Implement suggestions from linter

[mrwormhole]

Signed-off-by: F. Talha Altinel [email protected]

Description

I have run golangci-lint v1.49.0 and used this config to eliminate false positives

linters-settings:
  govet:
    check-shadowing: true
    settings:
      printf:
        funcs:
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
  funlen:
    lines: 100
    statements: 50
  goconst:
    min-len: 2
    min-occurrences: 3
    ignore-tests: true
  gocritic:
    enabled-tags:
      - diagnostic
      - experimental
      - opinionated
      - performance
      - style
    disabled-checks:
      - ifElseChain
      - whyNoLint
      - hugeParam
      - octalLiteral
  gocyclo:
    min-complexity: 20
  gomnd:
    settings:
      mnd:
        checks:
          - argument
          - case
          - condition
          - return
          - operation
          # assign is optional to enable
        ignored-numbers: "2,10,100,500,8080,0600,0660"
  lll:
    line-length: 144
  misspell:
    locale: UK
  nolintlint:
    allow-unused: false # report any unused nolint directives
    allow-leading-space: false # disable to ensure that nolint directives don't have a leading space. Default is true.)
    require-explanation: false # don't require an explanation for nolint directives
    require-specific: true # require nolint directives to be specific about which linter is being skipped

linters:
  disable-all: true
  enable:
    # ESSENTIALS AND ENABLED BY DEFAULT
    - errcheck
    - gosimple
    - govet
    - ineffassign
    - staticcheck
    - typecheck
    - unused
    # BEGINNER GO DEV SLAYER PACK
    - bodyclose
    - dogsled
    - dupl
    # - errname enable if your version is  +v1.42.0
    - exportloopref
    - forcetypeassert
    - funlen
    - gochecknoinits
    - goconst
    - gocritic
    - gocyclo
    - gofmt
    - goimports
    - revive
    - gomnd
    - gosec
    - lll
    - misspell
    - nakedret
    - nilerr
    - nolintlint
    # - paralleltest enable this to enforce table driven tests fast with t.Parallel()
    - predeclared
    - stylecheck
    - thelper
    # - tparallel enable this to enforce correct usage of t.Parallel()
    - unconvert
    - unparam

issues:
  # Excluding configuration per-path, per-linter, per-text and per-source
  exclude-rules:
    # Exclude duplicates and type assertions in the tests
    - path: _test\.go
      linters:
        - dupl
        - forcetypeassert
        - lll

    # Exclude shadowing on common Go conventions
    - linters:
        - govet
      text: "shadow: declaration of \"(err|ok|ctx)\""

    # Exclude init from main.go files
    - linters:
        - gochecknoinits
      path: main\.go

    # Exclude lll issues for long lines with go:generate
    - linters:
        - lll
      source: "^//go:generate"

    # Exclude assert for shadowing
    - linters:
        - gocritic
      text: "importShadow: shadow of imported from 'github.com/stretchr/testify/assert' package 'assert'"

    - linters:
        - gocritic
      text: "unnamedResult: consider giving a name to these results"

    - linters:
        - gosec
      text: "G204: Subprocess launched with a potential tainted input or cmd arguments"

    - linters:
        - gosec
      text: "G306: Expect WriteFile permissions to be 0600 or less"

    - linters:
        - gocritic
      text: "httpNoBody: http.NoBody should be preferred to the nil request body"

    - linters:
        - gocritic
      text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
      path: _test\.go

    - linters:
        - gocritic
      text: "unnecessaryDefer: defer res.Body.Close() is placed just before return"
      path: _test\.go

    - linters:
        - revive
      text: "var-naming: func NewHttp should be NewHTTP"

    - linters:
        - revive
      text: "var-naming: type Http should be HTTP"

    - linters:
        - revive
      text: "errorf: should replace t.Error"
      path: _test\.go

    - linters:
        - revive
      text: "exported: type name will be used as metrics.MetricsServer by other packages, and that stutters; consider calling this Server"

    - linters:
        - gosimple
      text: "S1002: should omit comparison"

    - linters:
        - gosimple
      text: "S1038: should use t.Errorf(...)"
      path: _test\.go

    - linters:
        - nilerr
      text: "error is not nil (line 142) but it returns nil"

    - linters:
        - misspell
      text: "`serializing` is a misspelling of `serialising`"

    - linters:
        - stylecheck
      text: "ST1003: func NewHttp should be NewHTTP"

    - linters:
        - stylecheck
      text: "ST1003: type Http should be HTTP"

run:
  timeout: 4m
  skip-dirs:
    - bigquery

  skip-files:
    - lib/bad.go

rules:
  - linters:
      - dupl
    severity: info

Motivation and Context

• I have raised an issue to propose this change (required)
  raised under Suggestion: Implement Go linting #55

How Has This Been Tested?

no this is not tested yet

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)
• Code style change of linter suggestions

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• [] I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have signed-off my commits with git commit -s
• I have added tests to cover my changes.
• All new and existing tests passed.

Additional Note

I have found these errors as well but due to my limited knowledge, I didn't take action on these to not break the repository.
Would be cool to discuss it after this PR change

[mrwormhole]

not agreed as a scope, will be parked

[alexellis]

Will you suggest any fixes for these warnings? We can review it for you.

[alexellis]

Why do you feel this is "wrong"?

[alexellis]

Both are used in the stdlib

https://stackoverflow.com/questions/18594330/what-is-the-best-way-to-test-for-an-empty-string-in-go

[mrwormhole]

I think you are right, I assumed ASM part to be different but it is the same, will rollback this and ignore this gosimple rule

https://godbolt.org/z/h83vaqE1x

[alexellis]

Added comments, thanks for running this.

[alexellis]

/set title: Implement suggestions from linter

[derek]

Thank you for your contribution. unfortunately, one or more of your commits are missing the required "Signed-off-by:" statement. Signing off is part of the Developer Certificate of Origin (DCO) which is used by this project.

Read the DCO and project contributing guide carefully, and amend your commits using the git CLI. Note that this does not require any cryptography, keys or special steps to be taken.

💡 Shall we fix this?

This will only take a few moments.

First, clone your fork and checkout this branch using the git CLI.

Next, set up your real name and email address:

git config --global user.name "Your Full Name"
git config --global user.email "[email protected]"

Finally, run one of these commands to add the "Signed-off-by" line to your commits.

If you only have one commit so far then run: git commit --amend --signoff and then git push --force.
If you have multiple commits, watch this video.

Check that the message has been added properly by running "git log".

[mrwormhole]

closing this as I am working on a less style based linter configuration that only focuses basic style and more of real bugs/issues with eliminated false positives, will probably do a gist and a blog on this as I have been dealing with linter configs for long time