Skip to content

Commit

Permalink
Add support for linting comments on structs (#11)
Browse files Browse the repository at this point in the history
Much like linting comments on interfaces but
allows a leading "A" or "An" in front of the
struct name in the comment. This matches
much of the stdlib comments for structs

Pass the new `--comment-structs` flag to
enable struct linting
  • Loading branch information
ashmrtn authored Apr 8, 2023
2 parents f33762b + d94fc12 commit 322c4d3
Show file tree
Hide file tree
Showing 6 changed files with 475 additions and 121 deletions.
76 changes: 53 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,32 +1,35 @@
# CommentMimic

CommentMimic is a golang linter that enforces starting comments for interfaces
and functions with the name of the interface or function. The case of the first
word must match the case of the element being commented. There are also a set of
flags for requiring comments on exported interfaces or functions.
CommentMimic is a golang linter that enforces starting comments for interfaces,
functions, and structs with the name of the interface, function, or struct. The
case of the first word must match the case of the element being commented. There
are also a set of flags for requiring comments on exported interfaces,
functions, or structs. Struct comments may start with an "A" or "An" followed by
the struct name (in the proper case).

## Why should I use CommentMimic?
Some go best-practices, like
[effective-go](https://github.com/golovers/effective-go#comment-sentences), say
comments should be written in full sentences and should start with the thing
being commented. Following the best-practices can make reading documentation
easier, but it makes upkeep a lot harder, especially when code is refactored or
a well-meaning colleague asks you to rename an interface or function to better
match what it does. When that happens, it's easy to forget to update the comment
to match the new interface or function name. In the end, the code ends up with
some elements having comments that match their names and others are clearly the
left-overs of previous iterations of the code.
a well-meaning colleague asks you to rename an interface, function, or struct to
better match what it does. When that happens, it's easy to forget to update the
comment to match the new name. In the end, the code ends up with some elements
having comments that match their names and others are clearly the left-overs of
previous iterations of the code.

CommentMimic was created to help developers by scanning the code and comments
and outputting notices when the first word of a comment for an interface or
function does not match the item being commented. It can be run from the command
line like other go tools, making it easy to integrate into your workflow.
and outputting notices when the first word of a comment for an interface,
function, or struct doesn't match the item being commented. It can be run from
the command line like other go tools, making it easy to integrate into your
workflow.

CommentMimic aims to be unobtrusive by only warning about things that already
have comments attached to them. Some other linters expect comments on all
exported functions and only output warnings about unmatched comment first words
and item names on exported functions. While that's a step in the right
direction, those linters can be overwhelming, because not many codebases have
direction, those linters can be overwhelming because not many codebases have
all exported items commented, and only find issues on exported items.

## Installing
Expand All @@ -41,8 +44,8 @@ would with other tools. For example, to check your whole project with
CommentMimic just run `commentmimic ./...`.

### Flags
CommentMimic optionally enforces comments on all exported interfaces and
functions depending flags passed to it.
CommentMimic optionally enforces comments on all exported interfaces,
functions, and structs depending flags passed to it.

`--comment-exported` requires comments on all functions that have exported
interfaces or exported receivers (i.e. the struct the function belongs to is
Expand All @@ -53,22 +56,49 @@ whether their receiver is exported.

`--comment-interfaces` requires comments on all exported interfaces.

`--comment-structs` requires comments on all exported structs.

## Limitations
CommentMimic has the following limitations and oddities:

* ignores leading whitespace in comments
* only checks interfaces and functions
* doesn't lint comments on consts or vars
* comments on a type-block with a single type definition won't be applied to the
type defined in the block

Leading whitespace is ignored because it doesn't play well with comments
starting with `/* text starts after a space...`. Go expects comments using
`/* */` to have the `/*` and `*/` on lines of their own. To be a little more
flexible, CommentMimic can handle when they share a line with comment text, but
the linter can no longer check if there's leading whitespace.

Currently CommentMimic only checks interface and function comments. This may be
expanded later on, but the [official pages](https://tip.golang.org/doc/comment)
on golang doc comments don't explicitly state standards for comment formats. The
type declaration comments written on that page consistently start with `A` or
`An` but comments on `var` and `const` declarations show much more variability
in their format. Eventually CommentMimic may be expanded to cover type
declaration comments as well, but support for others seems unlikely.
Currently CommentMimic only checks interface, function, and struct comments.
This may be expanded later on, but the
[official pages](https://tip.golang.org/doc/comment) on golang doc comments
don't explicitly state standards for comment formats. The `var` and `const`
declarations on that page show lots of variability in their format, making it
hard to enforce a standard.

Golang allows declaring one or more types in a type-block like shown below.
Comments can also be associated with the type-block, in addition to or as a
replacement for comments on the individual type declarations. CommentMimic won't
apply comments on the type-block to the type definition(s) in the block, even if
there's only one of them.

```go
// A someType is a struct used for passing data.
//
// This comment layout will cause a lint error because the comment is on the
// type-block instead of the type declaration.
type (
someType struct{}
)

type (
// A someOtherType is a struct used for passing data.
//
// This comment layout will not cause a lint error because the comment is
// associated with the type declaration.
someOtherType struct{}
)
```
83 changes: 71 additions & 12 deletions pkg/analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,27 @@ import (
)

const (
//nolint:lll
commentMismatchTmpl = "first word of comment is '%s' instead of '%s'"
commentEmptyTmpl = "empty comment on '%s'"
commentMissingTmpl = "exported element '%s' should be commented"

testFileNameSuffix = "_test.go"
)

var testCommentPrefix = []string{
"Benchmark",
"Example",
"Fuzz",
"Test",
}
var (
testCommentPrefix = []string{
"Benchmark",
"Example",
"Fuzz",
"Test",
}

// This will enforce capitalization of the first word as well.
structLeadWords = map[string]struct{}{
"A": {},
"An": {},
}
)

func checkComment(
pass *analysis.Pass,
Expand All @@ -36,12 +43,14 @@ func checkComment(
comment *ast.CommentGroup,
elementExported bool,
recvExported bool,
leadWords map[string]struct{},
) {
checkCommentMismatch(
pass,
elementName,
comment,
elementPos,
leadWords,
)
checkExported(
pass,
Expand Down Expand Up @@ -83,11 +92,21 @@ func containsOnlyMachineReadableComment(comment *ast.CommentGroup) bool {
return onlyMachine
}

// checkCommentMismatch checks if the element with the given name has a first or
// second word that matches the element name. If it doesn't it reports the
// result to pass.
//
// Comments that are only machine readable comments are ignored.
//
// leadWords denotes the set of leading words that are allowed. If leadWords is
// non-nil and non-empty this function will check if the second word matches the
// element name if the first word doesn't match.
func checkCommentMismatch(
pass *analysis.Pass,
elementName string,
comment *ast.CommentGroup,
elementPos token.Pos,
leadWords map[string]struct{},
) {
if comment == nil {
return
Expand Down Expand Up @@ -123,6 +142,16 @@ func checkCommentMismatch(
return
}

// See if this comment has a first word that matches a lead word and then
// check if the second word matches the element name.
if len(words) > 1 {
secondWord := words[1]

if _, ok := leadWords[firstWord]; ok && secondWord == elementName {
return
}
}

pass.Reportf(
comment.Pos(),
commentMismatchTmpl,
Expand Down Expand Up @@ -221,6 +250,7 @@ func (m mimic) checkFuncDecl(pass *analysis.Pass, fun *ast.FuncDecl) {
fun.Doc,
fun.Name.IsExported(),
exportedRecv,
nil,
)
}

Expand All @@ -231,13 +261,25 @@ func (m mimic) checkGenDecl(pass *analysis.Pass, decl *ast.GenDecl) {
continue
}

exportedRecv := ts.Name.IsExported()
var (
commentFlag bool
leadWords map[string]struct{}
)

iface, ok := ts.Type.(*ast.InterfaceType)
if !ok {
switch ts.Type.(type) {
case *ast.StructType:
commentFlag = m.commentStructs
leadWords = structLeadWords

case *ast.InterfaceType:
commentFlag = m.commentInterfaces

default:
continue
}

exportedRecv := ts.Name.IsExported()

// If the type-declaration a single declaration (i.e. not grouped by
// parentheses), then the doc comment is attached to the GenDecl node. If
// the interface is part of a grouped declaration, it's attached to the
Expand All @@ -250,19 +292,27 @@ func (m mimic) checkGenDecl(pass *analysis.Pass, decl *ast.GenDecl) {
pos = ts.Pos()
}

// Check if interface is commented properly.
// Check if struct or interface is commented properly.
checkComment(
pass,
// Set to false so the flag completely controls output behavior.
false,
m.commentInterfaces,
commentFlag,
ts.Name.Name,
pos,
doc,
exportedRecv,
true,
leadWords,
)

iface, ok := ts.Type.(*ast.InterfaceType)
if !ok {
// Structs can't embed other type definitions or function names so it's
// safe to return after just checking the comment.
continue
}

for _, field := range iface.Methods.List {
_, ok := field.Type.(*ast.FuncType)
if !ok {
Expand All @@ -278,6 +328,7 @@ func (m mimic) checkGenDecl(pass *analysis.Pass, decl *ast.GenDecl) {
field.Doc,
field.Names[0].IsExported(),
exportedRecv,
nil,
)
}
}
Expand Down Expand Up @@ -314,6 +365,7 @@ type mimic struct {
commentExportedFuncs bool
commentAllExportedFuncs bool
commentInterfaces bool
commentStructs bool
noTestComments bool
}

Expand Down Expand Up @@ -349,6 +401,13 @@ func NewCommentMimic() *analysis.Analyzer {
"don't require comments on tests, benchmarks, examples, and fuzz tests",
)

fs.BoolVar(
&m.commentStructs,
"comment-structs",
false,
"require comments on all exported structs",
)

return &analysis.Analyzer{
Name: "commentmimic",
//nolint:lll
Expand Down
Loading

0 comments on commit 322c4d3

Please sign in to comment.