diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index e1fe3225d..803005324 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -474,12 +474,13 @@ Available flags are: * _checkPrivateReceivers_ enables checking public methods of private types * _disableStutteringCheck_ disables checking for method names that stutter with the package name (i.e. avoid failure messages of the form _type name will be used as x.XY by other packages, and that stutters; consider calling this Y_) * _sayRepetitiveInsteadOfStutters_ replaces the use of the term _stutters_ by _repetitive_ in failure messages +* _checkPublicInterface_ enabled checking public method definitions in public interface types Example: ```toml [rule.exported] - arguments = ["checkPrivateReceivers", "disableStutteringCheck"] + arguments = ["checkPrivateReceivers", "disableStutteringCheck", "checkPublicInterface"] ``` ## file-header diff --git a/rule/exported.go b/rule/exported.go index b8663c48c..a1566b9a3 100644 --- a/rule/exported.go +++ b/rule/exported.go @@ -18,23 +18,36 @@ type ExportedRule struct { configured bool checkPrivateReceivers bool disableStutteringCheck bool + checkPublicInterface bool stuttersMsg string sync.Mutex } func (r *ExportedRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if !r.configured { - var sayRepetitiveInsteadOfStutters bool - r.checkPrivateReceivers, r.disableStutteringCheck, sayRepetitiveInsteadOfStutters = r.getConf(arguments) r.stuttersMsg = "stutters" - if sayRepetitiveInsteadOfStutters { - r.stuttersMsg = "is repetitive" + for _, flag := range arguments { + flagStr, ok := flag.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument for the %s rule: expecting a string, got %T", r.Name(), flag)) + } + switch flagStr { + case "checkPrivateReceivers": + r.checkPrivateReceivers = true + case "disableStutteringCheck": + r.disableStutteringCheck = true + case "sayRepetitiveInsteadOfStutters": + r.stuttersMsg = "is repetitive" + case "checkPublicInterface": + r.checkPublicInterface = true + default: + panic(fmt.Sprintf("Unknown configuration flag %s for %s rule", flagStr, r.Name())) + } } - r.configured = true } - r.Unlock() } // Apply applies the rule to given file. @@ -57,6 +70,7 @@ func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failur genDeclMissingComments: make(map[*ast.GenDecl]bool), checkPrivateReceivers: r.checkPrivateReceivers, disableStutteringCheck: r.disableStutteringCheck, + checkPublicInterface: r.checkPublicInterface, stuttersMsg: r.stuttersMsg, } @@ -70,32 +84,6 @@ func (*ExportedRule) Name() string { return "exported" } -func (r *ExportedRule) getConf(args lint.Arguments) (checkPrivateReceivers, disableStutteringCheck, sayRepetitiveInsteadOfStutters bool) { - // if any, we expect a slice of strings as configuration - if len(args) < 1 { - return - } - for _, flag := range args { - flagStr, ok := flag.(string) - if !ok { - panic(fmt.Sprintf("Invalid argument for the %s rule: expecting a string, got %T", r.Name(), flag)) - } - - switch flagStr { - case "checkPrivateReceivers": - checkPrivateReceivers = true - case "disableStutteringCheck": - disableStutteringCheck = true - case "sayRepetitiveInsteadOfStutters": - sayRepetitiveInsteadOfStutters = true - default: - panic(fmt.Sprintf("Unknown configuration flag %s for %s rule", flagStr, r.Name())) - } - } - - return -} - type lintExported struct { file *lint.File fileAst *ast.File @@ -104,6 +92,7 @@ type lintExported struct { onFailure func(lint.Failure) checkPrivateReceivers bool disableStutteringCheck bool + checkPublicInterface bool stuttersMsg string } @@ -214,14 +203,18 @@ func (w *lintExported) lintTypeDoc(t *ast.TypeSpec, doc *ast.CommentGroup) { break } } - if !strings.HasPrefix(s, t.Name.Name+" ") { - w.onFailure(lint.Failure{ - Node: doc, - Confidence: 1, - Category: "comments", - Failure: fmt.Sprintf(`comment on exported type %v should be of the form "%v ..." (with optional leading article)`, t.Name, t.Name), - }) + // if comment starts wih name of type and has some text after - it's ok + expectedPrefix := t.Name.Name+" " + if strings.HasPrefix(s, expectedPrefix){ + return } + w.onFailure(lint.Failure{ + Node: doc, + Confidence: 1, + Category: "comments", + Failure: fmt.Sprintf(`comment on exported type %v should be of the form "%s..." (with optional leading article)`, t.Name, expectedPrefix), + }) + } func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genDeclMissingComments map[*ast.GenDecl]bool) { @@ -301,7 +294,7 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD // // This function is needed because ast.CommentGroup.Text() does not handle //-style and /*-style comments uniformly func normalizeText(t string) string { - return strings.TrimPrefix(t, " ") + return strings.TrimSpace(t) } func (w *lintExported) Visit(n ast.Node) ast.Visitor { @@ -330,7 +323,15 @@ func (w *lintExported) Visit(n ast.Node) ast.Visitor { } w.lintTypeDoc(v, doc) w.checkStutter(v.Name, "type") - // Don't proceed inside types. + + if w.checkPublicInterface { + if iface, ok := v.Type.(*ast.InterfaceType); ok { + if ast.IsExported(v.Name.Name) { + w.doCheckPublicInterface(v.Name.Name, iface) + } + } + } + return nil case *ast.ValueSpec: w.lintValueSpecDoc(v, w.lastGen, w.genDeclMissingComments) @@ -338,3 +339,38 @@ func (w *lintExported) Visit(n ast.Node) ast.Visitor { } return w } + +func (w *lintExported) doCheckPublicInterface(typeName string, iface *ast.InterfaceType) { + for _, m := range iface.Methods.List { + w.lintInterfaceMethod(typeName, m) + } +} + +func (w *lintExported) lintInterfaceMethod(typeName string, m *ast.Field) { + if len(m.Names) == 0 { + return + } + if !ast.IsExported(m.Names[0].Name) { + return + } + name := m.Names[0].Name + if m.Doc == nil { + w.onFailure(lint.Failure{ + Node: m, + Confidence: 1, + Category: "comments", + Failure: fmt.Sprintf("public interface method %s.%s should be commented", typeName, name), + }) + return + } + s := normalizeText(m.Doc.Text()) + expectedPrefix := m.Names[0].Name + " " + if !strings.HasPrefix(s, expectedPrefix) { + w.onFailure(lint.Failure{ + Node: m.Doc, + Confidence: 0.8, + Category: "comments", + Failure: fmt.Sprintf(`comment on exported interface method %s.%s should be of the form "%s..."`, typeName, name, expectedPrefix), + }) + } +} diff --git a/test/exported_test.go b/test/exported_test.go index 841ab4a01..da298c588 100644 --- a/test/exported_test.go +++ b/test/exported_test.go @@ -24,3 +24,9 @@ func TestExportedReplacingStuttersByRepetitive(t *testing.T) { testRule(t, "exported-issue-519", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args}) } + +func TestCheckPublicInterfaceOption(t *testing.T) { + args := []any{"checkPublicInterface"} + + testRule(t, "exported-issue-1002", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args}) +} diff --git a/testdata/exported-issue-1002.go b/testdata/exported-issue-1002.go new file mode 100644 index 000000000..3610fb6fe --- /dev/null +++ b/testdata/exported-issue-1002.go @@ -0,0 +1,24 @@ +// Package golint comment +package golint + +// by default code bellow is valid, +// but if checkPublicInterface is switched on - it should check documentation in interfaces + +// Some - some interface +type Some interface { + Other // should not fail + // Correct - should do all correct + Correct() + // MATCH /comment on exported interface method Some.SemiCorrect should be of the form "SemiCorrect ..."/ + SemiCorrect() + NonCorrect() // MATCH /public interface method Some.NonCorrect should be commented/ +} + +// Other - just to check names compatibility +type Other interface {} + +// for private interfaces it doesn't check docs anyway + +type somePrivate interface { + AllGood() +} \ No newline at end of file diff --git a/testdata/golint/exported.go b/testdata/golint/exported.go index 9b93503f6..022d422c3 100644 --- a/testdata/golint/exported.go +++ b/testdata/golint/exported.go @@ -33,6 +33,13 @@ const FirstLetter = "A" /*Bar2 something */ type Bar2 struct{} + +/* Bar3 */ // MATCH /comment on exported type Bar3 should be of the form "Bar3 ..." (with optional leading article)/ +type Bar3 struct{} + +/* BarXXX invalid */ // MATCH /comment on exported type Bar4 should be of the form "Bar4 ..." (with optional leading article)/ +type Bar4 struct{} + /*Toto2 something */ func Toto2() {}