Skip to content

Commit

Permalink
FEATURE mgechev#1002 - "checkPublicInterface" option for "exported" r…
Browse files Browse the repository at this point in the history
…ule - to check public interface method comments
  • Loading branch information
comdiv committed Jun 28, 2024
1 parent bdba5d9 commit 47f336c
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 3 deletions.
62 changes: 59 additions & 3 deletions rule/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type ExportedRule struct {
configured bool
checkPrivateReceivers bool
disableStutteringCheck bool
checkPublicInterface bool
stuttersMsg string
sync.Mutex
}
Expand All @@ -26,7 +27,12 @@ func (r *ExportedRule) configure(arguments lint.Arguments) {
r.Lock()
if !r.configured {
var sayRepetitiveInsteadOfStutters bool
r.checkPrivateReceivers, r.disableStutteringCheck, sayRepetitiveInsteadOfStutters = r.getConf(arguments)

r.checkPrivateReceivers,
r.disableStutteringCheck,
sayRepetitiveInsteadOfStutters,
r.checkPublicInterface = r.getConf(arguments)

r.stuttersMsg = "stutters"
if sayRepetitiveInsteadOfStutters {
r.stuttersMsg = "is repetitive"
Expand Down Expand Up @@ -57,7 +63,9 @@ 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,

}

ast.Walk(&walker, fileAst)
Expand All @@ -70,7 +78,12 @@ func (*ExportedRule) Name() string {
return "exported"
}

func (r *ExportedRule) getConf(args lint.Arguments) (checkPrivateReceivers, disableStutteringCheck, sayRepetitiveInsteadOfStutters bool) {
func (r *ExportedRule) getConf(args lint.Arguments) (
checkPrivateReceivers,
disableStutteringCheck,
sayRepetitiveInsteadOfStutters bool,
checkPublicInterface bool,
) {
// if any, we expect a slice of strings as configuration
if len(args) < 1 {
return
Expand All @@ -88,6 +101,8 @@ func (r *ExportedRule) getConf(args lint.Arguments) (checkPrivateReceivers, disa
disableStutteringCheck = true
case "sayRepetitiveInsteadOfStutters":
sayRepetitiveInsteadOfStutters = true
case "checkPublicInterface":
checkPublicInterface = true
default:
panic(fmt.Sprintf("Unknown configuration flag %s for %s rule", flagStr, r.Name()))
}
Expand All @@ -104,6 +119,7 @@ type lintExported struct {
onFailure func(lint.Failure)
checkPrivateReceivers bool
disableStutteringCheck bool
checkPublicInterface bool
stuttersMsg string
}

Expand Down Expand Up @@ -330,11 +346,51 @@ 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)
return nil
}
return w
}


func (w *lintExported) doCheckPublicInterface(typeName string, iface *ast.InterfaceType) {
for _, m := range iface.Methods.List {
if ast.IsExported(m.Names[0].Name) {
w.lintInterfaceMethod(typeName, m)
}
}
}

func (w *lintExported) lintInterfaceMethod(typeName string, m *ast.Field) {
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())
prefix := m.Names[0].Name + " "
if !strings.HasPrefix(s, prefix) {
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, prefix),
})
}
}
7 changes: 7 additions & 0 deletions test/exported_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,10 @@ 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})
}
20 changes: 20 additions & 0 deletions testdata/exported-issue-1002.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// 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 {
// 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/
}

// for private interfaces it doesn't check docs anyway

type somePrivate interface {
AllGood()
}

0 comments on commit 47f336c

Please sign in to comment.