diff --git a/README.md b/README.md index 80dfd38..2c710ce 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | `file-header` | string | Header which each file should have. | no | no | | `empty-block` | n/a | Warns on empty code blocks | no | no | | `superfluous-else` | n/a | Prevents redundant else statements (extends `indent-error-flow`) | no | no | +| `confusing-naming` | n/a | Warns on methods with names that differ only by capitalization | no | no | ## Available Formatters diff --git a/config.go b/config.go index 31a0cdc..8a22ef3 100644 --- a/config.go +++ b/config.go @@ -49,6 +49,7 @@ var allRules = append([]lint.Rule{ &rule.FileHeaderRule{}, &rule.EmptyBlockRule{}, &rule.SuperfluousElseRule{}, + &rule.ConfusingNamingRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/fixtures/confusing-naming.go b/fixtures/confusing-naming.go new file mode 100644 index 0000000..bb5dc2a --- /dev/null +++ b/fixtures/confusing-naming.go @@ -0,0 +1,51 @@ +// Test of confusing-naming rule. + +// Package pkg ... +package pkg + +type foo struct {} + +func (t foo) aFoo() { + return +} + + + +func (t *foo) AFoo() { // MATCH /Method 'AFoo' differs only by capitalization to method 'aFoo'/ + return +} + +type bar struct {} + +func (t *bar) aBar() { + return +} + +func (t *bar) aFoo() { // Should not warn + return +} + + + +func aGlobal(){ + +} + +func AGlobal(){ // MATCH /Method 'AGlobal' differs only by capitalization to method 'aGlobal'/ +} + +func ABar() { // Should not warn + +} + +func aFoo() { // Should not warn + +} + +func (t foo) ABar() { // Should not warn + return +} + +func (t bar) ABar() { // MATCH /Method 'ABar' differs only by capitalization to method 'aBar'/ + return +} \ No newline at end of file diff --git a/rule/confusing-naming.go b/rule/confusing-naming.go new file mode 100644 index 0000000..d8899a6 --- /dev/null +++ b/rule/confusing-naming.go @@ -0,0 +1,102 @@ +package rule + +import ( + "fmt" + "go/ast" + "strings" + + "github.com/mgechev/revive/lint" +) + +// ConfusingNamingRule lints method names that differ only by capitalization +type ConfusingNamingRule struct{} + +// Apply applies the rule to given file. +func (r *ConfusingNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + var failures []lint.Failure + + fileAst := file.AST + walker := lintConfusingNames{ + methodNames: make(map[string][]*ast.Ident), + onFailure: func(failure lint.Failure) { + failures = append(failures, failure) + }, + } + + ast.Walk(&walker, fileAst) + + return failures +} + +// Name returns the rule name. +func (r *ConfusingNamingRule) Name() string { + return "confusing-naming" +} + +//checkMethodName checks if a given method/function name is similar (just case differences) to other method/function of the same struct/file. +func checkMethodName(holder string, id *ast.Ident, w *lintConfusingNames) { + name := strings.ToUpper(id.Name) + if w.methodNames[holder] != nil { + blackList := w.methodNames[holder] + for _, n := range blackList { + if strings.ToUpper(n.Name) == name { + // confusing names + w.onFailure(lint.Failure{ + Failure: fmt.Sprintf("Method '%s' differs only by capitalization to method '%s'", id.Name, n.Name), + Confidence: 1, + Node: id, + Category: "naming", + URL: "#TODO", + }) + + return + } + } + } + // update the black list + w.methodNames[holder] = append(w.methodNames[holder], id) +} + +type lintConfusingNames struct { + methodNames map[string][]*ast.Ident // a map from struct names to method id nodes + onFailure func(lint.Failure) +} + +const defaultStructName = "_" // used to map functions + +//getStructName of a function receiver. Defaults to defaultStructName +func getStructName(r *ast.FieldList) string { + result := defaultStructName + + if r == nil || len(r.List) < 1 { + return result + } + + t := r.List[0].Type + + if p, _ := t.(*ast.StarExpr); p != nil { // if a pointer receiver => dereference pointer receiver types + t = p.X + } + + if p, _ := t.(*ast.Ident); p != nil { + result = p.Name + } + + return result +} + +func (w *lintConfusingNames) Visit(n ast.Node) ast.Visitor { + switch v := n.(type) { + case *ast.FuncDecl: + // Exclude naming warnings for functions that are exported to C but + // not exported in the Go API. + // See https://github.com/golang/lint/issues/144. + if ast.IsExported(v.Name.Name) || !isCgoExported(v) { + checkMethodName(getStructName(v.Recv), v.Name, w) + } + default: + // will add other checks like field names, struct names, etc. + } + + return w +} diff --git a/test/confusing-naming_test.go b/test/confusing-naming_test.go new file mode 100644 index 0000000..b14e6ed --- /dev/null +++ b/test/confusing-naming_test.go @@ -0,0 +1,12 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +// TestConfusingNaming rule. +func TestConfusingNaming(t *testing.T) { + testRule(t, "confusing-naming", &rule.ConfusingNamingRule{}) +}