False positive "should have a package comment" from golint

[ernado]

Having following config:

linters-settings:
  golint:
    min-confidence: 0
issues:
  exclude-use-default: false

I'm getting an error for each file in package (expect one with actual documentation):

bin/bytes.go:1:1: should have a package comment, unless it's in another file for this package (golint)
package bin

Looks like golint is not package-aware?

Related: #456 #464

Update:
If min-confidence: 0 is removed, those issues are not reported 🤔

[ldez]

Hello,

golint is package-aware but the linting applies at file level.

func (f *file) lintPackageComment()

func (f *file) lintPackageComment() {
	// ...

	if f.f.Doc == nil {
		f.errorf(f.f, 0.2, link(ref), category("comments"), "should have a package comment, unless it's in another file for this package")
		return
    }

	// ...
}

https://github.com/golang/lint/blob/738671d3881b9731cc63024d5d88cf28db875626/lint.go#L379

func (f *file) lint()

func (f *file) lint() {
	f.lintPackageComment()
	// ...
}

https://github.com/golang/lint/blob/738671d3881b9731cc63024d5d88cf28db875626/lint.go#L195

func (p *pkg) lint() []Problem

func (p *pkg) lint() []Problem {
	// ...

	for _, f := range p.files {
		f.lint()
	}

	/ ...
}

https://github.com/golang/lint/blob/738671d3881b9731cc63024d5d88cf28db875626/lint.go#L176

func (f *file) errorf(...)

func (f *file) errorf(n ast.Node, confidence float64, args ...interface{}) *Problem {
	pos := f.fset.Position(n.Pos())
	if pos.Filename == "" {
		pos.Filename = f.filename
	}
	return f.pkg.errorfAt(pos, confidence, args...)
}

https://github.com/golang/lint/blob/738671d3881b9731cc63024d5d88cf28db875626/lint.go#L221-L227

Knowing that golint can only display errors at the file level, it is not possible to display a message related to a package. Furthermore, it is not possible to know which file in a package should contain the package comment.

[ldez]

The default min_confidence is 0.8, and the "package comment" rule has confidence of 0.2

So by default, those messages are not displayed by golint.

https://github.com/golang/lint/blob/738671d3881b9731cc63024d5d88cf28db875626/golint/golint.go#L23

golangci-lint/pkg/commands/run.go

Line 139 in 0370d3a

fs.Float64Var(&lsc.Golint.MinConfidence, "golint.min-confidence", 0.8,

[ldez]

IMHO the EXC0002 exclude rule can be dropped as the default min_confidence is 0.8

[ernado]

Should we investigate whether it is possible to make golint understand that other file in package contains package comment?

AFAIK EXC0002 is not only about package comments so we can't disable it without breaking backward compatibility.

[ldez]

The only rule with a confidence lower than 0.8 is EXC0002.

$ grep "f.errorf(" lint.go 
f.errorf(f.f, 0.2, link(ref), category("comments"), "should have a package comment, unless it's in another file for this package")
f.errorf(f.f.Doc, 1, link(ref), category("comments"), "package comment should not have leading space")
f.errorf(f.f.Doc, 1, link(ref), category("comments"), `package comment should be of the form "%s..."`, prefix)
f.errorf(imp, 1, link(ref), category("imports"), "a blank import should be only in a main or test package, or have a comment justifying it")
f.errorf(is, 1, link(styleGuideBase+"#import-dot"), category("imports"), "should not use dot imports")
f.errorf(f.f, 1, link("http://golang.org/doc/effective_go.html#package-names"), category("naming"), "don't use an underscore in package name")
f.errorf(f.f, 1, link("http://golang.org/doc/effective_go.html#package-names"), category("mixed-caps"), "don't use MixedCaps in package name; %s should be %s", f.f.Name.Name, strings.ToLower(f.f.Name.Name))
f.errorf(id, 0.8, link(styleGuideBase+"#mixed-caps"), category("naming"), "don't use ALL_CAPS in Go names; use CamelCase")
f.errorf(id, 0.8, link(styleGuideBase+"#mixed-caps"), category("naming"), "don't use leading k in Go names; %s %s should be %s", thing, id.Name, should)
f.errorf(id, 0.9, link("http://golang.org/doc/effective_go.html#mixed-caps"), category("naming"), "don't use underscores in Go names; %s %s should be %s", thing, id.Name, should)
f.errorf(id, 0.8, link(styleGuideBase+"#initialisms"), category("naming"), "%s %s should be %s", thing, id.Name, should)
f.errorf(t, 1, link(docCommentsLink), category("comments"), "exported type %v should have comment or be unexported", t.Name)
f.errorf(doc, 1, link(docCommentsLink), category("comments"), `comment on exported type %v should be of the form "%v ..." (with optional leading article)`, t.Name, t.Name)
f.errorf(fn, 1, link(docCommentsLink), category("comments"), "exported %s %s should have comment or be unexported", kind, name)
f.errorf(fn.Doc, 1, link(docCommentsLink), category("comments"), `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
f.errorf(vs, 1, category("comments"), "exported %s %s should have its own declaration", kind, n.Name)
f.errorf(vs, 1, link(docCommentsLink), category("comments"), "exported %s %s should have comment%s or be unexported", kind, name, block)
f.errorf(doc, 1, link(docCommentsLink), category("comments"), `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
f.errorf(id, 0.8, link(styleGuideBase+"#package-names"), category("naming"), "%s name will be used as %s.%s by other packages, and that stutters; consider calling this %s", thing, pkg, name, rem)
f.errorf(ifStmt.Else, 1, link(styleGuideBase+"#indent-error-flow"), category("indent"), "if block ends with a return statement, so drop this else and outdent its block"+extra)
p := f.errorf(rs.Key, 1, category("range-loop"), "should omit values from range; this loop is equivalent to `for range ...`")
p := f.errorf(rs.Value, 1, category("range-loop"), "should omit 2nd value from range; this loop is equivalent to `for %s %s range ...`", f.render(rs.Key), rs.Tok)
p := f.errorf(node, 1, category("errors"), "should replace %s(fmt.Sprintf(...)) with %s.Errorf(...)", f.render(se), errorfPrefix)
f.errorf(id, 0.9, category("naming"), "error var %s should have name of the form %sFoo", id.Name, prefix)
f.errorf(str, conf, link(styleGuideBase+"#error-strings"), category("errors"),
f.errorf(n, 1, link(ref), category("naming"), `receiver name should not be an underscore, omit the name if it is unused`)
f.errorf(n, 1, link(ref), category("naming"), `receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"`)
f.errorf(n, 1, link(ref), category("naming"), "receiver name %s should be consistent with previous receiver name %s for %s", name, prev, recv)
f.errorf(as, 0.8, category("unary-op"), "should replace %s with %s%s", f.render(as), f.render(as.Lhs[0]), suffix)
f.errorf(fn, 0.9, category("arg-order"), "error should be the last type when returning multiple items")
f.errorf(ret.Type, 0.8, category("unexported-type-in-api"),
f.errorf(v, 0.9, category("time"), "var %s is of type %v; don't use unit-specific suffix %q", name.Name, origTyp, suffix)
f.errorf(x, 1.0, category("context"), fmt.Sprintf("should not use basic type %s as key in context.WithValue", key.Type))
f.errorf(fn, 0.9, link("https://golang.org/pkg/context/"), category("arg-order"), "context.Context should be the first parameter of a function")

If you set explicitly the confidence to 0 it's only for EXC0002, so EXC0002 can be dropped without any impact.

[ldez]

sorry yes EXC0002 is also about the other comment rules, so this rule cannot be dropped.

[ldez]

But EXC0002 could be divided into 2 or 3 rules:

• comment on package
• comment on method and function
• comment on type on const

also in EXC0002 we can remove the package part.

[ldez]

Should we investigate whether it is possible to make golint understand that other file in package contains package comment?

yes I think we could investigate. I think that the current golint code was not designed to do that.

[ldez]

FYI stylecheck handle that rule by package instead of file:

ST1000: at least one file in a package should have a package comment (stylecheck)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.