diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 9b18df6d8..273c127a9 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -475,12 +475,17 @@ Available flags are: * _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 +* _disableChecksOnConstants_ disable all checks on constant declarations +* _disableChecksOnFunctions_ disable all checks on function declarations +* _disableChecksOnMethods_ disable all checks on method declarations +* _disableChecksOnTypes_ disable all checks on type declarations +* _disableChecksOnVariables_ disable all checks on variable declarations Example: ```toml [rule.exported] - arguments = ["checkPrivateReceivers", "disableStutteringCheck", "checkPublicInterface"] + arguments = ["checkPrivateReceivers", "disableStutteringCheck", "checkPublicInterface", "disableChecksOnFunctions"] ``` ## file-header diff --git a/rule/exported.go b/rule/exported.go index 7d7150529..e3972d40e 100644 --- a/rule/exported.go +++ b/rule/exported.go @@ -13,40 +13,92 @@ import ( "github.com/mgechev/revive/lint" ) +// disabledChecks store ignored warnings types +type disabledChecks struct { + Const bool + Function bool + Method bool + PrivateReceivers bool + PublicInterfaces bool + Stuttering bool + Type bool + Var bool +} + +const checkNamePrivateReceivers = "privateReceivers" +const checkNamePublicInterfaces = "publicInterfaces" +const checkNameStuttering = "stuttering" + +// isDisabled returns true if the given check is disabled, false otherwise +func (dc *disabledChecks) isDisabled(checkName string) bool { + switch checkName { + case "var": + return dc.Var + case "const": + return dc.Const + case "function": + return dc.Function + case "method": + return dc.Method + case checkNamePrivateReceivers: + return dc.PrivateReceivers + case checkNamePublicInterfaces: + return dc.PublicInterfaces + case checkNameStuttering: + return dc.Stuttering + case "type": + return dc.Type + default: + return false + } +} + // ExportedRule lints given else constructs. type ExportedRule struct { - configured bool - checkPrivateReceivers bool - disableStutteringCheck bool - checkPublicInterface bool - stuttersMsg string + configured bool + stuttersMsg string + disabledChecks disabledChecks sync.Mutex } func (r *ExportedRule) configure(arguments lint.Arguments) { r.Lock() defer r.Unlock() - if !r.configured { - r.stuttersMsg = "stutters" - 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 { + if r.configured { + return + } + r.configured = true + + r.disabledChecks = disabledChecks{PrivateReceivers: true, PublicInterfaces: true} + r.stuttersMsg = "stutters" + for _, flag := range arguments { + switch flag := flag.(type) { + case string: + switch flag { case "checkPrivateReceivers": - r.checkPrivateReceivers = true + r.disabledChecks.PrivateReceivers = false case "disableStutteringCheck": - r.disableStutteringCheck = true + r.disabledChecks.Stuttering = true case "sayRepetitiveInsteadOfStutters": r.stuttersMsg = "is repetitive" case "checkPublicInterface": - r.checkPublicInterface = true + r.disabledChecks.PublicInterfaces = false + case "disableChecksOnConstants": + r.disabledChecks.Const = true + case "disableChecksOnFunctions": + r.disabledChecks.Function = true + case "disableChecksOnMethods": + r.disabledChecks.Method = true + case "disableChecksOnTypes": + r.disabledChecks.Type = true + case "disableChecksOnVariables": + r.disabledChecks.Var = true default: - panic(fmt.Sprintf("Unknown configuration flag %s for %s rule", flagStr, r.Name())) + panic(fmt.Sprintf("Unknown configuration flag %s for %s rule", flag, r.Name())) } + default: + panic(fmt.Sprintf("Invalid argument for the %s rule: expecting a string, got %T", r.Name(), flag)) } - r.configured = true } } @@ -68,10 +120,8 @@ func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failur failures = append(failures, failure) }, genDeclMissingComments: make(map[*ast.GenDecl]bool), - checkPrivateReceivers: r.checkPrivateReceivers, - disableStutteringCheck: r.disableStutteringCheck, - checkPublicInterface: r.checkPublicInterface, stuttersMsg: r.stuttersMsg, + disabledChecks: r.disabledChecks, } ast.Walk(&walker, fileAst) @@ -90,30 +140,30 @@ type lintExported struct { lastGen *ast.GenDecl genDeclMissingComments map[*ast.GenDecl]bool onFailure func(lint.Failure) - checkPrivateReceivers bool - disableStutteringCheck bool - checkPublicInterface bool stuttersMsg string + disabledChecks disabledChecks } func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { if !ast.IsExported(fn.Name.Name) { - // func is unexported - return + return // func is unexported, nothing to do } + kind := "function" name := fn.Name.Name - if fn.Recv != nil && len(fn.Recv.List) > 0 { - // method + isMethod := fn.Recv != nil && len(fn.Recv.List) > 0 + if isMethod { kind = "method" recv := typeparams.ReceiverType(fn) - if !w.checkPrivateReceivers && !ast.IsExported(recv) { - // receiver is unexported + + if !ast.IsExported(recv) && w.disabledChecks.PrivateReceivers { return } + if commonMethods[name] { return } + switch name { case "Len", "Less", "Swap": sortables := w.file.Pkg.Sortable() @@ -123,6 +173,11 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { } name = recv + "." + name } + + if w.disabledChecks.isDisabled(kind) { + return + } + if fn.Doc == nil { w.onFailure(lint.Failure{ Node: fn, @@ -132,6 +187,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { }) return } + s := normalizeText(fn.Doc.Text()) prefix := fn.Name.Name + " " if !strings.HasPrefix(s, prefix) { @@ -145,7 +201,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { } func (w *lintExported) checkStutter(id *ast.Ident, thing string) { - if w.disableStutteringCheck { + if w.disabledChecks.Stuttering { return } @@ -179,9 +235,14 @@ func (w *lintExported) checkStutter(id *ast.Ident, thing string) { } func (w *lintExported) lintTypeDoc(t *ast.TypeSpec, doc *ast.CommentGroup) { + if w.disabledChecks.isDisabled("type") { + return + } + if !ast.IsExported(t.Name.Name) { return } + if doc == nil { w.onFailure(lint.Failure{ Node: t, @@ -203,18 +264,19 @@ func (w *lintExported) lintTypeDoc(t *ast.TypeSpec, doc *ast.CommentGroup) { break } } + // if comment starts with 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) { @@ -223,6 +285,10 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD kind = "const" } + if w.disabledChecks.isDisabled(kind) { + return + } + if len(vs.Names) > 1 { // Check that none are exported except for the first. for _, n := range vs.Names[1:] { @@ -324,7 +390,7 @@ func (w *lintExported) Visit(n ast.Node) ast.Visitor { w.lintTypeDoc(v, doc) w.checkStutter(v.Name, "type") - if w.checkPublicInterface { + if !w.disabledChecks.PublicInterfaces { if iface, ok := v.Type.(*ast.InterfaceType); ok { if ast.IsExported(v.Name.Name) { w.doCheckPublicInterface(v.Name.Name, iface) diff --git a/test/exported_test.go b/test/exported_test.go index da298c588..b028eb5df 100644 --- a/test/exported_test.go +++ b/test/exported_test.go @@ -30,3 +30,9 @@ func TestCheckPublicInterfaceOption(t *testing.T) { testRule(t, "exported-issue-1002", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args}) } + +func TestCheckDisablingOnDeclarationTypes(t *testing.T) { + args := []any{"disableChecksOnConstants", "disableChecksOnFunctions", "disableChecksOnMethods", "disableChecksOnTypes", "disableChecksOnVariables"} + + testRule(t, "exported-issue-1045", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args}) +} diff --git a/testdata/exported-issue-1045.go b/testdata/exported-issue-1045.go new file mode 100644 index 000000000..f6b06030f --- /dev/null +++ b/testdata/exported-issue-1045.go @@ -0,0 +1,25 @@ +// Package golint comment +package golint + + +// path separator defined by os.Separator. +const FilePath = "xyz" + + +// Rewrite string to remove non-standard path characters +func UnicodeSanitize(s string) string {} + + +// Tags returns a slice of tags. The order is the original tag order unless it +// was changed. +func (t *Tags) Keys() []string {} + +// A value which may be passed to the which parameter for Getitimer and +type ItimerWhich int + + +/* +// PropertyBag +*/ +// Rectangle An area within an image. +type Rectangle struct {}