Skip to content

Commit

Permalink
stylecheck: validate docstrings on exported identifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
dominikh committed Jul 29, 2019
1 parent ad29248 commit 2c70f90
Show file tree
Hide file tree
Showing 9 changed files with 302 additions and 1 deletion.
2 changes: 1 addition & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (c Config) String() string {
}

var DefaultConfig = Config{
Checks: []string{"all", "-ST1000", "-ST1003", "-ST1016"},
Checks: []string{"all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"},
Initialisms: []string{
"ACL", "API", "ASCII", "CPU", "CSS", "DNS",
"EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID",
Expand Down
12 changes: 12 additions & 0 deletions stylecheck/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,16 @@ var Analyzers = lintutil.InitializeAnalyzers(Docs, map[string]*analysis.Analyzer
Run: CheckDuplicatedImports,
Requires: []*analysis.Analyzer{facts.Generated, config.Analyzer},
},
"ST1020": {
Run: CheckExportedFunctionDocs,
Requires: []*analysis.Analyzer{inspect.Analyzer},
},
"ST1021": {
Run: CheckExportedTypeDocs,
Requires: []*analysis.Analyzer{inspect.Analyzer},
},
"ST1022": {
Run: CheckExportedVarDocs,
Requires: []*analysis.Analyzer{inspect.Analyzer},
},
})
51 changes: 51 additions & 0 deletions stylecheck/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,55 @@ bug, we prefer the more idiomatic 'if x == 42'.`,
Title: `Importing the same package multiple times`,
Since: "Unreleased",
},

"ST1020": &lint.Documentation{
Title: "The documentation of an exported function should start with the function's name",
Text: `Doc comments work best as complete sentences, which
allow a wide variety of automated presentations. The first sentence
should be a one-sentence summary that starts with the name being
declared.
If every doc comment begins with the name of the item it describes,
you can use the doc subcommand of the go tool and run the output
through grep.
See https://golang.org/doc/effective_go.html#commentary for more
information on how to write good documentation.`,
Since: "Unreleased",
NonDefault: true,
},

"ST1021": &lint.Documentation{
Title: "The documentation of an exported type should start with type's name",
Text: `Doc comments work best as complete sentences, which
allow a wide variety of automated presentations. The first sentence
should be a one-sentence summary that starts with the name being
declared.
If every doc comment begins with the name of the item it describes,
you can use the doc subcommand of the go tool and run the output
through grep.
See https://golang.org/doc/effective_go.html#commentary for more
information on how to write good documentation.`,
Since: "Unreleased",
NonDefault: true,
},

"ST1022": &lint.Documentation{
Title: "The documentation of an exported variable or constant should start with variable's name",
Text: `Doc comments work best as complete sentences, which
allow a wide variety of automated presentations. The first sentence
should be a one-sentence summary that starts with the name being
declared.
If every doc comment begins with the name of the item it describes,
you can use the doc subcommand of the go tool and run the output
through grep.
See https://golang.org/doc/effective_go.html#commentary for more
information on how to write good documentation.`,
Since: "Unreleased",
NonDefault: true,
},
}
154 changes: 154 additions & 0 deletions stylecheck/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,3 +664,157 @@ func CheckInvisibleCharacters(pass *analysis.Pass) (interface{}, error) {
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.BasicLit)(nil)}, fn)
return nil, nil
}

func CheckExportedFunctionDocs(pass *analysis.Pass) (interface{}, error) {
fn := func(node ast.Node) {
if IsInTest(pass, node) {
return
}

decl := node.(*ast.FuncDecl)
if decl.Doc == nil {
return
}
if !ast.IsExported(decl.Name.Name) {
return
}
kind := "function"
if decl.Recv != nil {
kind = "method"
switch T := decl.Recv.List[0].Type.(type) {
case *ast.StarExpr:
if !ast.IsExported(T.X.(*ast.Ident).Name) {
return
}
case *ast.Ident:
if !ast.IsExported(T.Name) {
return
}
default:
ExhaustiveTypeSwitch(T)
}
}
prefix := decl.Name.Name + " "
if !strings.HasPrefix(decl.Doc.Text(), prefix) {
ReportNodef(pass, decl.Doc, `comment on exported %s %s should be of the form "%s..."`, kind, decl.Name.Name, prefix)
}
}

pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.FuncDecl)(nil)}, fn)
return nil, nil
}

func CheckExportedTypeDocs(pass *analysis.Pass) (interface{}, error) {
var genDecl *ast.GenDecl
fn := func(node ast.Node, push bool) bool {
if !push {
genDecl = nil
return false
}
if IsInTest(pass, node) {
return false
}

switch node := node.(type) {
case *ast.GenDecl:
if node.Tok == token.IMPORT {
return false
}
genDecl = node
return true
case *ast.TypeSpec:
if !ast.IsExported(node.Name.Name) {
return false
}

doc := node.Doc
if doc == nil {
if len(genDecl.Specs) != 1 {
// more than one spec in the GenDecl, don't validate the
// docstring
return false
}
if genDecl.Lparen.IsValid() {
// 'type ( T )' is weird, don't guess the user's intention
return false
}
doc = genDecl.Doc
if doc == nil {
return false
}
}

s := doc.Text()
articles := [...]string{"A", "An", "The"}
for _, a := range articles {
if strings.HasPrefix(s, a+" ") {
s = s[len(a)+1:]
break
}
}
if !strings.HasPrefix(s, node.Name.Name+" ") {
ReportNodef(pass, doc, `comment on exported type %s should be of the form "%s ..." (with optional leading article)`, node.Name.Name, node.Name.Name)
}
return false
case *ast.FuncLit, *ast.FuncDecl:
return false
default:
ExhaustiveTypeSwitch(node)
return false
}
}

pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes([]ast.Node{(*ast.GenDecl)(nil), (*ast.TypeSpec)(nil), (*ast.FuncLit)(nil), (*ast.FuncDecl)(nil)}, fn)
return nil, nil
}

func CheckExportedVarDocs(pass *analysis.Pass) (interface{}, error) {
var genDecl *ast.GenDecl
fn := func(node ast.Node, push bool) bool {
if !push {
genDecl = nil
return false
}
if IsInTest(pass, node) {
return false
}

switch node := node.(type) {
case *ast.GenDecl:
if node.Tok == token.IMPORT {
return false
}
genDecl = node
return true
case *ast.ValueSpec:
if genDecl.Lparen.IsValid() || len(node.Names) > 1 {
// Don't try to guess the user's intention
return false
}
name := node.Names[0].Name
if !ast.IsExported(name) {
return false
}
if genDecl.Doc == nil {
return false
}
prefix := name + " "
if !strings.HasPrefix(genDecl.Doc.Text(), prefix) {
kind := "var"
if genDecl.Tok == token.CONST {
kind = "const"
}
ReportNodef(pass, genDecl.Doc, `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
}
return false
case *ast.FuncLit, *ast.FuncDecl:
return false
default:
ExhaustiveTypeSwitch(node)
return false
}
}

pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes([]ast.Node{(*ast.GenDecl)(nil), (*ast.ValueSpec)(nil), (*ast.FuncLit)(nil), (*ast.FuncDecl)(nil)}, fn)
return nil, nil
}
3 changes: 3 additions & 0 deletions stylecheck/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ func TestAll(t *testing.T) {
"ST1017": {{Dir: "CheckYodaConditions"}},
"ST1018": {{Dir: "CheckInvisibleCharacters"}},
"ST1019": {{Dir: "CheckDuplicatedImports"}},
"ST1020": {{Dir: "CheckExportedFunctionDocs"}},
"ST1021": {{Dir: "CheckExportedTypeDocs"}},
"ST1022": {{Dir: "CheckExportedVarDocs"}},
}

testutil.Run(t, Analyzers, checks)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package pkg

// whatever
func foo() {}

// Foo is amazing
func Foo() {}

// Whatever // want `comment on exported function`
func Bar() {}

type T struct{}

// Whatever
func (T) foo() {}

// Foo is amazing
func (T) Foo() {}

// Whatever // want `comment on exported method`
func (T) Bar() {}
6 changes: 6 additions & 0 deletions stylecheck/testdata/src/CheckExportedFunctionDocs/foo_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package pkg

import "testing"

// This is a test
func TestFoo(t *testing.T) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package pkg

// Some type
type t1 struct{}

// Some type // want `comment on exported type`
type T2 struct{}

// T3 is amazing
type T3 struct{}

type (
// Some type // want `comment on exported type`
T4 struct{}
// The T5 type is amazing
T5 struct{}
// Some type
t6 struct{}
)

// Some types
type (
T7 struct{}
T8 struct{}
)

// Some types
type (
T9 struct{}
)

func fn() {
// Some type
type T1 struct{}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package pkg

// Whatever
var a int

// Whatever // want `should be of the form`
var B int

// Whatever
var (
// Whatever
C int
)

func fn() {
// Whatever
var D int
_ = D
}

0 comments on commit 2c70f90

Please sign in to comment.