Skip to content

Commit

Permalink
rule.exported: add feature to disable checking on const,method,functi…
Browse files Browse the repository at this point in the history
…on, variable (#1047)



Co-authored-by: chavacava <salvadorcavadini+github@gmail.com>
  • Loading branch information
mfederowicz and chavacava authored Oct 2, 2024
1 parent ca2a32b commit ff7b0ad
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 34 deletions.
7 changes: 6 additions & 1 deletion RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
132 changes: 99 additions & 33 deletions rule/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -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
}

Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand All @@ -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:] {
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions test/exported_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
25 changes: 25 additions & 0 deletions testdata/exported-issue-1045.go
Original file line number Diff line number Diff line change
@@ -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 {}

0 comments on commit ff7b0ad

Please # to comment.