Golangci-lint: Comments issues(golint) should not be ignored by default

[pityonline]

(should have comment|comment on exported method) - golint: annoying issues about not having a comment. The rare codebase has such comments.

In my opinion, comments in Go code are not only just comments but also the documents of the code in Golang design. It's pretty familiar with the here documents in Python. If this issue is ignored by default, the code might be without the document in some careless actions, and might be unmaintainable. Also, this will cause some different reaction with the original golint, just like below.

$ golangci-lint run --disable-all -E golint ./...
Congrats! No issues were found.
$ golint ./...
pkg/enabled_linters.go:15:2: exported const PresetFormatting should have comment (or a comment on this block) or be unexported
pkg/enabled_linters.go:23:1: exported function AllPresets should have comment or be unexported
pkg/enabled_linters.go:35:6: exported type LinterConfig should have comment or be unexported
pkg/enabled_linters.go:44:1: exported method LinterConfig.WithFullImport should have comment or be unexported
pkg/enabled_linters.go:49:1: exported method LinterConfig.WithSSA should have comment or be unexported
pkg/enabled_linters.go:55:1: exported method LinterConfig.WithPresets should have comment or be unexported
pkg/enabled_linters.go:60:1: exported method LinterConfig.WithSpeed should have comment or be unexported
pkg/enabled_linters.go:74:1: exported function GetLinterConfig should have comment or be unexported
pkg/enabled_linters.go:100:1: exported function GetAllSupportedLinterConfigs should have comment or be unexported
pkg/enabled_linters.go:283:1: exported function GetAllLintersForPreset should have comment or be unexported
...

[ryboe]

Just for the record, if you set min-confidence: 0, it will show every issue from golint.

$ golangci-lint run --golint.min-confidence 0

# .golangci.yml
linters-settings:
  golint:
    min-confidence: 0

[pityonline]

Thank you for your reply. I take some small tests with the confidence configuration. This is my .golangci.yml:

run:
  deadline: 30s
  tests: true

linters-settings:
  govet:
    check-shadowing: true
  golint:
    min-confidence: 0
  gocyclo:
    min-complexity: 10
  maligned:
    suggest-new: true
  dupl:
    threshold: 100
  goconst:
    min-len: 2
    min-occurrences: 2

linters:
  enable-all: false

Note that min-confidence is set to 0 in the config file(no rules excluded) or command line arguments, I got 3 warnings.

$ golangci-lint run --disable-all -E golint --golint.min-confidence 0 ./...
pkg/golinters/context.go:1:1: should have a package comment, unless it's in another file for this package (golint)
package golinters
^
pkg/golinters/deadcode.go:1:1: should have a package comment, unless it's in another file for this package (golint)
package golinters
^
pkg/golinters/dupl.go:1:1: should have a package comment, unless it's in another file for this package (golint)
package golinters
^
$ golangci-lint run --disable-all -E golint ./...
cmd/golangci-lint/main.go:1:1: should have a package comment, unless it's in another file for this package (golint)
package main
^
pkg/config/config.go:1:1: should have a package comment, unless it's in another file for this package (golint)
package config
^
pkg/fsutils/fsutils.go:1:1: should have a package comment, unless it's in another file for this package (golint)
package fsutils
^

The original golint with default -min_confidence(default to 0.8) returns 175 warnings, if set to 0, it returns 223 warnings.

$ golint -min_confidence 0 ./... | wc -l
     223
$ golint ./... | wc -l
     175

As far as I can see, golangci-lint still has different reactions with original golint.

$ golint ./...
pkg/enabled_linters.go:15:2: exported const PresetFormatting should have comment (or a comment on this block) or be unexported
pkg/enabled_linters.go:23:1: exported function AllPresets should have comment or be unexported
pkg/enabled_linters.go:35:6: exported type LinterConfig should have comment or be unexported
pkg/enabled_linters.go:44:1: exported method LinterConfig.WithFullImport should have comment or be unexported
pkg/enabled_linters.go:49:1: exported method LinterConfig.WithSSA should have comment or be unexported
pkg/enabled_linters.go:55:1: exported method LinterConfig.WithPresets should have comment or be unexported
pkg/enabled_linters.go:60:1: exported method LinterConfig.WithSpeed should have comment or be unexported
pkg/enabled_linters.go:74:1: exported function GetLinterConfig should have comment or be unexported
pkg/enabled_linters.go:100:1: exported function GetAllSupportedLinterConfigs should have comment or be unexported
pkg/enabled_linters.go:283:1: exported function GetAllLintersForPreset should have comment or be unexported
pkg/enabled_linters.go:373:1: exported function GetEnabledLinters should have comment or be unexported
pkg/enabled_linters_test.go:264:102: don't use leading k in Go names; func parameter kLOC should be lOC
pkg/linter.go:10:6: exported type Linter should have comment or be unexported
pkg/runner.go:19:6: exported type SimpleRunner should have comment or be unexported
pkg/runner.go:190:1: exported method SimpleRunner.Run should have comment or be unexported
pkg/commands/executor.go:8:6: exported type Executor should have comment or be unexported
pkg/commands/executor.go:16:1: exported function NewExecutor should have comment or be unexported
pkg/commands/executor.go:28:1: exported method Executor.Execute should have comment or be unexported
pkg/config/config.go:7:6: exported type OutFormat should have comment or be unexported
pkg/config/config.go:10:2: exported const OutFormatJSON should have comment (or a comment on this block) or be unexported
pkg/config/config.go:15:5: exported var OutFormats should have comment or be unexported

[golangci]

@pityonline thank you the issue!

1. golangci-lint ignores some issues, thay are documented in README and --help:

      --exclude-use-default              Use or not use default excludes: (Error return value of .((os\.)?std(out|err)\..*|.*Close|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked|should have comment|comment on exported method|
G103:|G104:|G204:|G301:|G302:|G304:|possible misuse of unsafe.Pointer|should have signature|ineffective break statement. Did you mean to break out of the outer loop) (default true)

1. You can use option --exclude-use-default=no to disable all default excludes. Is it ok for you?

In my opinion, the most of Go programmers treat issues about comments as false-positives. Do you have any ideas how to not being annoying for them?

[pityonline]

@golangci Thanks for your reply. I think comments above the exported types, functions, methods, etc, are useful and important documents for better understanding the code, that's the design of Golang. I've taken a glance through the code of this project, no documents found. So would you mind if someone adds docs to the code in the future?

[golangci]

we don't mind :)

[jirfag]

Closing, I won't enable comments warnings by default

[rgeronimi]

1. You can use option --exclude-use-default=no to disable all default excludes. Is it ok for you?

In my opinion, the most of Go programmers treat issues about comments as false-positives. Do you have any ideas how to not being annoying for them?

If these are considered false positives, they do not seem to be on the same level than false positives from other linters (e.g., gas). Because this is what is recommended by several googlers, and several projects (e.g., all the professional projects I have been contributing to) check comments by default.

The exclude-use-default workaround as it stands now brings 2 challenges:

• It disables all filters for false positives from other linters (e.g., gas), making golangci-lint unusable
• It does not benefit from first-class test & support - e.g., the current version of golangci-lint (0c82b64) does not take into account this flag (using it in config file or command line does not disable the filter, as we dont see the warnings appearing, while we see them with a direct call to golint)

The only workaround that works currently is to run golint in addition to golangci-lint, which runs contrary to our objective of going 100% golangci-lint.