ci: add go-static-check #295

7h3-3mp7y-m4n · 2025-04-10T19:53:03Z

Add Go Static Check to CI Workflow

Description

This PR adds Staticcheck to the CI pipeline to perform static code analysis on the Go codebase. It helps catch bugs and enforce best practices automatically.

What does this PR do?

It integrates Staticcheck into the CI pipeline, improving code quality and catching potential issues early.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
New chore (expected functionality to be implemented)

Checklist:

[] My change requires a change to the documentation.
- I have updated the documentation accordingly.
I've signed off with an email address that matches the commit author.

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

7h3-3mp7y-m4n · 2025-04-10T20:38:31Z

@astromechza , what do you think of it? Do you think we should add this to our workflow?

astromechza · 2025-04-17T14:33:19Z

@7h3-3mp7y-m4n personally I'd prefer we go further an add golangci-lint with a config like

version: "2"
linters:
  default: standard
  enable:
    - bodyclose
    - contextcheck
    - goconst
    - gosec
    - mnd
    - testifylint

I use this in a number of other projects and found it very useful!

7h3-3mp7y-m4n · 2025-04-17T16:57:55Z

@7h3-3mp7y-m4n personally I'd prefer we go further an add golangci-lint with a config like
version: "2"
linters:
  default: standard
  enable:
    - bodyclose
    - contextcheck
    - goconst
    - gosec
    - mnd
    - testifylint
I use this in a number of other projects and found it very useful!

Yeah, makes sense golangci-lint seems like the better call, especially with all the extra checks it brings in. Having everything in one place with a shared config will be super handy. Let’s do it!
I’ll just fix the remaining staticCheck issues first so we can include it all in the same PR

7h3-3mp7y-m4n · 2025-04-17T17:27:28Z

I’m thinking we should remove the go vet step from our ci.yaml, because we can include go vet as a linter in golangci-lint. Since we're centralizing our linting configuration in .golangci.yml , why not add it there as well? What do you think, @astromechza and @mathieu-benoit?

mathieu-benoit · 2025-04-17T18:14:34Z

I think this makes sense if it's redundant.

@astromechza, does it work for you?

astromechza · 2025-05-20T09:20:49Z

@7h3-3mp7y-m4n @mathieu-benoit yes, we can remove vet if we're using the new linter.

7h3-3mp7y-m4n · 2025-05-21T20:43:27Z

@7h3-3mp7y-m4n @mathieu-benoit yes, we can remove vet if we're using the new linter.

I'll make the changes :)

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

7h3-3mp7y-m4n · 2025-06-10T19:06:27Z

here is a little breakdown of linters.

 make lint
Running golangci-lint...
golangci-lint run ./...
internal/command/init.go:229:19: Error return value of `f.Close` is not checked (errcheck)
                                        defer f.Close()
                                                     ^
internal/command/run.go:101:17: Error return value of `src.Close` is not checked (errcheck)
        defer src.Close()
                       ^
internal/command/run.go:115:19: Error return value of `ovr.Close` is not checked (errcheck)
                        defer ovr.Close()
                                       ^
internal/command/run.go:273:23: Error return value of `destFile.Close` is not checked (errcheck)
                defer destFile.Close()
                                    ^
internal/command/run.go:292:19: Error return value of `dest.Close` is not checked (errcheck)
                defer dest.Close()
                                ^
internal/command/provisioners.go:79:7: string `json` has 3 occurrences, make it a constant (goconst)
        case "json":
             ^
internal/command/generate.go:377:20: G306: Expect WriteFile permissions to be 0600 or less (gosec)
                } else if err := os.WriteFile(v+".temp", raw, 0644); err != nil {
                                 ^
internal/command/init.go:225:16: G302: Expect file permissions to be 0600 or less (gosec)
                                        f, err := os.OpenFile(defaultProvisioners, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0644)
                                                  ^
internal/command/run.go:235:22: G115: integer overflow conversion int -> uint32 (gosec)
                                Target:    uint32(util.DerefOr(pSpec.TargetPort, pSpec.Port)),
                                                 ^
internal/compose/convert.go:253:19: G304: Potential file inclusion via variable (gosec)
                        content, err = os.ReadFile(sourcePath)
                                       ^
internal/compose/convert.go:291:26: G115: integer overflow conversion int64 -> uint32 (gosec)
                        fileMode = os.FileMode(newMode)
                                              ^
internal/project/project.go:66:12: G301: Expect directory permissions to be 0750 or less (gosec)
        if err := os.Mkdir(sd.Path, 0755); err != nil && !errors.Is(err, os.ErrExist) {
                  ^
internal/project/project.go:69:12: G301: Expect directory permissions to be 0750 or less (gosec)
        if err := os.Mkdir(sd.State.Extras.MountsDirectory, 0755); err != nil && !errors.Is(err, os.ErrExist) {
                  ^
internal/project/project.go:80:12: G306: Expect WriteFile permissions to be 0600 or less (gosec)
        if err := os.WriteFile(filepath.Join(sd.Path, StateFileName+".temp"), out.Bytes(), 0755); err != nil {
                  ^
internal/project/project.go:91:18: G304: Potential file inclusion via variable (gosec)
        content, err := os.ReadFile(filepath.Join(d, StateFileName))
                        ^
internal/provisioners/cmdprov/commandprov.go:134:9: G204: Subprocess launched with variable (gosec)
        cmd := exec.CommandContext(ctx, bin, p.Args...)
               ^
internal/provisioners/core.go:212:14: G301: Expect directory permissions to be 0750 or less (gosec)
                        if err := os.MkdirAll(dst, 0755); err != nil && !errors.Is(err, os.ErrExist) {
                                  ^
internal/provisioners/core.go:234:14: G306: Expect WriteFile permissions to be 0600 or less (gosec)
                        if err := os.WriteFile(dst, []byte(*b), 0644); err != nil {
                                  ^
internal/provisioners/loader/load.go:87:16: G304: Potential file inclusion via variable (gosec)
                        raw, err := os.ReadFile(filepath.Join(path, item.Name()))
                                    ^
internal/provisioners/loader/load.go:114:99: G115: integer overflow conversion int64 -> uint64 (gosec)
        timePrefix := base64.RawURLEncoding.EncodeToString(binary.BigEndian.AppendUint64([]byte{}, uint64(math.MaxInt64-time.Now().UnixNano())))
                                                                                                         ^
internal/command/generate.go:172:46: Magic number: 2, in <argument> detected (mnd)
                                        parts := strings.SplitN(buildFlag, "=", 2)
                                                                                ^
internal/command/generate.go:173:23: Magic number: 2, in <condition> detected (mnd)
                                        if len(parts) != 2 {
                                                         ^
internal/command/generate.go:296:22: Magic number: 2, in <condition> detected (mnd)
                                if len(parts) <= 2 {
                                                 ^
internal/compose/convert.go:217:54: Magic number: 5, in <argument> detected (mnd)
                        Interval: util.Ref(compose.Duration(time.Second * 5)),
                                                                          ^
internal/compose/convert.go:218:54: Magic number: 5, in <argument> detected (mnd)
                        Timeout:  util.Ref(compose.Duration(time.Second * 5)),
                                                                          ^
internal/compose/convert.go:278:27: Magic number: 0644, in <argument> detected (mnd)
                fileMode := os.FileMode(0644)
                                        ^
internal/compose/convert.go:283:24: Magic number: 0777, in <condition> detected (mnd)
                        } else if newMode > 0777 {
                                            ^
internal/compose/convert.go:285:30: Magic number: 0400, in <condition> detected (mnd)
                        } else if newMode&0400 != 0400 {
                                                  ^
internal/compose/convert.go:287:30: Magic number: 0600, in <condition> detected (mnd)
                        } else if newMode&0600 != 0600 {
                                                  ^
internal/compose/convert.go:288:25: Magic number: 0600, in <operation> detected (mnd)
                                newMode = newMode | 0600
                                                    ^
internal/compose/write.go:27:16: Magic number: 2, in <argument> detected (mnd)
        enc.SetIndent(2)
                      ^
internal/project/project.go:74:16: Magic number: 2, in <argument> detected (mnd)
        enc.SetIndent(2)
                      ^
internal/provisioners/loader/load.go:118:40: Magic number: 0600, in <argument> detected (mnd)
        if err := os.WriteFile(tmpPath, data, 0600); err != nil {
                                              ^
internal/version/version.go:51:16: Magic number: 2, in <condition> detected (mnd)
        if len(cpm) > 2 {
                      ^
internal/version/version.go:53:17: Magic number: 3, in <condition> detected (mnd)
                if len(cpm) > 3 {
                              ^
internal/logging/logging.go:15:1: package-comments: should have a package comment (revive)
package logging
^
internal/logging/logging.go:25:6: exported: exported type SimpleHandler should have comment or be unexported (revive)
type SimpleHandler struct {
     ^
internal/logging/logging.go:32:33: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
func (h *SimpleHandler) Enabled(ctx context.Context, level slog.Level) bool {
                                ^
internal/logging/logging.go:36:32: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
func (h *SimpleHandler) Handle(ctx context.Context, record slog.Record) error {
                               ^
internal/logging/logging.go:43:35: unused-parameter: parameter 'attrs' seems to be unused, consider removing or renaming it as _ (revive)
func (h *SimpleHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
                                  ^
internal/logging/logging.go:48:35: unused-parameter: parameter 'name' seems to be unused, consider removing or renaming it as _ (revive)
func (h *SimpleHandler) WithGroup(name string) slog.Handler {
                                  ^
internal/patching/patching.go:36:6: exported: exported type PatchOperation should have comment or be unexported (revive)
type PatchOperation struct {
     ^
internal/project/project.go:15:1: package-comments: should have a package comment (revive)
package project
^
internal/project/project.go:30:2: exported: exported const DefaultRelativeStateDirectory should have comment (or a comment on this block) or be unexported (revive)
        DefaultRelativeStateDirectory = ".score-compose"
        ^
internal/project/project.go:39:6: exported: exported type StateExtras should have comment or be unexported (revive)
type StateExtras struct {
     ^
internal/project/project.go:45:6: exported: exported type WorkloadExtras should have comment or be unexported (revive)
type WorkloadExtras struct {
     ^
internal/provisioners/cmdprov/commandprov.go:36:6: exported: exported type Provisioner should have comment or be unexported (revive)
type Provisioner struct {
     ^
internal/provisioners/cmdprov/commandprov.go:37:2: var-naming: struct field ProvisionerUri should be ProvisionerURI (revive)
        ProvisionerUri string   `yaml:"uri"`
        ^
internal/provisioners/cmdprov/commandprov.go:40:2: var-naming: struct field ResId should be ResID (revive)
        ResId          *string  `yaml:"id,omitempty"`
        ^
internal/provisioners/cmdprov/commandprov.go:47:1: exported: exported method Provisioner.Description should have comment or be unexported (revive)
func (p *Provisioner) Description() string {
^
internal/provisioners/cmdprov/commandprov.go:55:1: exported: exported method Provisioner.Class should have comment or be unexported (revive)
func (p *Provisioner) Class() string {
^
internal/provisioners/cmdprov/commandprov.go:62:1: exported: exported method Provisioner.Type should have comment or be unexported (revive)
func (p *Provisioner) Type() string {
^
internal/provisioners/cmdprov/commandprov.go:66:1: exported: exported method Provisioner.Params should have comment or be unexported (revive)
func (p *Provisioner) Params() []string {
^
internal/provisioners/cmdprov/commandprov.go:70:1: exported: exported method Provisioner.Outputs should have comment or be unexported (revive)
func (p *Provisioner) Outputs() []string {
^
internal/provisioners/cmdprov/commandprov.go:122:1: exported: exported method Provisioner.Provision should have comment or be unexported (revive)
func (p *Provisioner) Provision(ctx context.Context, input *provisioners.Input) (*provisioners.ProvisionOutput, error) {
^
internal/provisioners/cmdprov/commandprov.go:154:1: exported: exported function Parse should have comment or be unexported (revive)
func Parse(raw map[string]interface{}) (*Provisioner, error) {
^
internal/provisioners/core.go:41:2: var-naming: struct field ResourceUid should be ResourceUID (revive)
        ResourceUid      string                 `json:"resource_uid"`
        ^
internal/provisioners/core.go:44:2: var-naming: struct field ResourceId should be ResourceID (revive)
        ResourceId       string                 `json:"resource_id"`
        ^
internal/provisioners/core.go:67:6: exported: exported type ServicePort should have comment or be unexported (revive)
type ServicePort struct {
     ^
internal/provisioners/core.go:86:2: var-naming: struct field ProvisionerUri should be ProvisionerURI (revive)
        ProvisionerUri       string                           `json:"-"`
        ^
internal/provisioners/core.go:100:6: exported: exported type Provisioner should have comment or be unexported (revive)
type Provisioner interface {
     ^
internal/provisioners/core.go:102:8: var-naming: interface method parameter resUid should be resUID (revive)
        Match(resUid framework.ResourceUid) bool
              ^
internal/provisioners/core.go:113:2: var-naming: struct field matchUid should be matchUID (revive)
        matchUid    framework.ResourceUid
        ^
internal/provisioners/core.go:126:32: var-naming: method Uri should be URI (revive)
func (e *ephemeralProvisioner) Uri() string {
                               ^
internal/provisioners/core.go:130:38: var-naming: method parameter resUid should be resUID (revive)
func (e *ephemeralProvisioner) Match(resUid framework.ResourceUid) bool {
                                     ^
internal/provisioners/core.go:154:42: var-naming: func parameter matchUid should be matchUID (revive)
func NewEphemeralProvisioner(uri string, matchUid framework.ResourceUid, inner func(ctx context.Context, input *Input) (*ProvisionOutput, error)) Provisioner {
                                         ^
internal/provisioners/core.go:160:73: var-naming: method parameter resUid should be resUID (revive)
func (po *ProvisionOutput) ApplyToStateAndProject(state *project.State, resUid framework.ResourceUid, project *compose.Project) (*project.State, error) {
                                                                        ^
internal/provisioners/core.go:300:1: exported: exported function ProvisionResources should have comment or be unexported (revive)
func ProvisionResources(ctx context.Context, state *project.State, provisioners []Provisioner, composeProject *compose.Project) (*project.State, error) {
^
internal/provisioners/core.go:311:9: var-naming: range var resUid should be resUID (revive)
        for _, resUid := range orderedResources {
               ^
internal/provisioners/envprov/envprov.go:41:23: var-naming: method Uri should be URI (revive)
func (e *Provisioner) Uri() string {
                      ^
internal/provisioners/envprov/envprov.go:45:29: var-naming: method parameter resUid should be resUID (revive)
func (e *Provisioner) Match(resUid framework.ResourceUid) bool {
                            ^
internal/provisioners/envprov/envprov.go:49:33: unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
func (e *Provisioner) Provision(ctx context.Context, input *provisioners.Input) (*provisioners.ProvisionOutput, error) {
                                ^
internal/provisioners/envprov/envprov.go:56:1: exported: exported method Provisioner.Accessed should have comment or be unexported (revive)
func (e *Provisioner) Accessed() map[string]string {
^
internal/provisioners/envprov/envprov.go:76:1: exported: exported method Provisioner.LookupOutput should have comment or be unexported (revive)
func (e *Provisioner) LookupOutput(keys ...string) (interface{}, error) {
^
internal/provisioners/envprov/envprov.go:118:33: var-naming: method Uri should be URI (revive)
func (e *envVarResourceTracker) Uri() string {
                                ^
internal/provisioners/envprov/envprov.go:156:1: receiver-naming: receiver name p should be consistent with previous receiver name e for Provisioner (revive)
func (p *Provisioner) Class() string {
        return p.Class()
}
internal/provisioners/envprov/envprov.go:160:1: receiver-naming: receiver name p should be consistent with previous receiver name e for Provisioner (revive)
func (p *Provisioner) Type() string {
        return p.Type()
}
internal/provisioners/envprov/envprov.go:164:1: receiver-naming: receiver name p should be consistent with previous receiver name e for Provisioner (revive)
func (p *Provisioner) Outputs() []string {
        return nil
}
internal/util/maps.go:15:1: package-comments: should have a package comment (revive)
package util
^
internal/util/ref.go:17:1: exported: exported function Ref should have comment or be unexported (revive)
func Ref[k any](input k) *k {
^
internal/util/ref.go:21:1: exported: exported function DerefOr should have comment or be unexported (revive)
func DerefOr[k any](input *k, def k) k {
^
internal/version/version.go:1:1: package-comments: package comment should be of the form "Package version ..." (revive)
/*
Apache Score
Copyright 2022 The Apache Software Foundation

This product includes software developed at
The Apache Software Foundation (http://www.apache.org/).
*/
internal/version/version.go:18:22: var-declaration: should omit type string from declaration of var Version; it will be inferred from the right-hand side (revive)
        Version             string = "0.0.0"
                            ^
internal/version/version.go:60:1: exported: exported function AssertVersion should have comment or be unexported (revive)
func AssertVersion(constraint string, current string) error {
^
internal/version/version.go:65:9: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (revive)
        } else {
                op := m[1]
                compareI, err := semverToI(m[0][len(op):])
                if err != nil {
                        return fmt.Errorf("failed to parse constraint: %w", err)
                }
                match := false
                switch op {
                case ">":
                        match = currentI > compareI
                case ">=":
                        match = currentI >= compareI
                case "=":
                        match = currentI == compareI
                }
                if !match {
                        return fmt.Errorf("current version %s does not match requested constraint %s", current, constraint)
                }
                return nil
        }
internal/logging/logging.go:39:12: QF1012: Use fmt.Fprintf(...) instead of Write([]byte(fmt.Sprintf(...))) (staticcheck)
        _, err := h.Writer.Write([]byte(fmt.Sprintf("%s: %s\n", record.Level.String(), record.Message)))
                  ^
internal/provisioners/envprov/envprov.go:157:9: SA5007: infinite recursive call (staticcheck)
        return p.Class()
               ^
internal/provisioners/envprov/envprov.go:161:9: SA5007: infinite recursive call (staticcheck)
        return p.Type()
               ^
internal/provisioners/envprov/envprov.go:181:9: SA5007: infinite recursive call (staticcheck)
        return p.Description()
               ^
89 issues:
* errcheck: 5
* goconst: 1
* gosec: 14
* mnd: 15
* revive: 50
* staticcheck: 4
make: *** [lint] Error 1

7h3-3mp7y-m4n · 2025-06-10T19:48:19Z

revive is a good linter, but it has strict rules and might discourage new contributors. It can also create noise, so let's not use it for now.

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

ci: add go-static-check

e63b6e5

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

Merge branch 'main' into ciStatic

4f1cdb8

7h3-3mp7y-m4n and others added 5 commits May 22, 2025 22:21

add golagci-lint

5b6556a

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

Merge branch 'main' into ciStatic

3a6f91f

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

minor fix for lint

02459e2

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

fix ci

116bbf2

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

Merge branch 'main' into ciStatic

9370645

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

minor changes and fix

11aef86

Signed-off-by: 7h3-3mp7y-m4n <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: add go-static-check #295

ci: add go-static-check #295

Uh oh!

7h3-3mp7y-m4n commented Apr 10, 2025

Uh oh!

7h3-3mp7y-m4n commented Apr 10, 2025

Uh oh!

astromechza commented Apr 17, 2025

Uh oh!

7h3-3mp7y-m4n commented Apr 17, 2025

Uh oh!

7h3-3mp7y-m4n commented Apr 17, 2025

Uh oh!

mathieu-benoit commented Apr 17, 2025

Uh oh!

astromechza commented May 20, 2025

Uh oh!

7h3-3mp7y-m4n commented May 21, 2025

Uh oh!

7h3-3mp7y-m4n commented Jun 10, 2025

Uh oh!

7h3-3mp7y-m4n commented Jun 10, 2025

Uh oh!

Uh oh!

ci: add go-static-check #295

Are you sure you want to change the base?

ci: add go-static-check #295

Uh oh!

Conversation

7h3-3mp7y-m4n commented Apr 10, 2025

Add Go Static Check to CI Workflow

Description

What does this PR do?

Types of changes

Checklist:

Uh oh!

7h3-3mp7y-m4n commented Apr 10, 2025

Uh oh!

astromechza commented Apr 17, 2025

Uh oh!

7h3-3mp7y-m4n commented Apr 17, 2025

Uh oh!

7h3-3mp7y-m4n commented Apr 17, 2025

Uh oh!

mathieu-benoit commented Apr 17, 2025

Uh oh!

astromechza commented May 20, 2025

Uh oh!

7h3-3mp7y-m4n commented May 21, 2025

Uh oh!

7h3-3mp7y-m4n commented Jun 10, 2025

Uh oh!

7h3-3mp7y-m4n commented Jun 10, 2025

Uh oh!

Uh oh!