Add names

fixtures/names.go

@@ -0,0 +1,116 @@
+// Test for name linting.
+
+// Package pkg_with_underscores ...
+package pkg_with_underscores // MATCH /don't use an underscore in package name/
+
+import (
+	"io"
+	"net"
+	net_http "net/http" // renamed deliberately
+	"net/url"
+)
+
+import "C"
+
+var var_name int // MATCH /don't use underscores in Go names; var var_name should be varName/
+
+type t_wow struct { // MATCH /don't use underscores in Go names; type t_wow should be tWow/
+	x_damn int      // MATCH /don't use underscores in Go names; struct field x_damn should be xDamn/
+	Url    *url.URL // MATCH /struct field Url should be URL/
+}
+
+const fooId = "blah" // MATCH /const fooId should be fooID/
+
+func f_it() { // MATCH /don't use underscores in Go names; func f_it should be fIt/
+	more_underscore := 4 // MATCH /don't use underscores in Go names; var more_underscore should be moreUnderscore/
+	_ = more_underscore
+	var err error
+	if isEof := (err == io.EOF); isEof { // MATCH /var isEof should be isEOF/
+		more_underscore = 7 // should be okay
+	}
+
+	x := net_http.Request{} // should be okay
+	_ = x
+
+	var ips []net.IP
+	for _, theIp := range ips { // MATCH /range var theIp should be theIP/
+		_ = theIp
+	}
+
+	switch myJson := g(); { // MATCH /var myJson should be myJSON/
+	default:
+		_ = myJson
+	}
+	var y net_http.ResponseWriter // an interface
+	switch tApi := y.(type) {     // MATCH /var tApi should be tAPI/
+	default:
+		_ = tApi
+	}
+
+	var c chan int
+	select {
+	case qId := <-c: // MATCH /var qId should be qID/
+		_ = qId
+	}
+}
+
+// Common styles in other languages that don't belong in Go.
+const (
+	CPP_CONST   = 1 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/
+	kLeadingKay = 2 // MATCH /don't use leading k in Go names; const kLeadingKay should be leadingKay/
+
+	HTML  = 3 // okay; no underscore
+	X509B = 4 // ditto
+)
+
+func f(bad_name int)                    {}            // MATCH /don't use underscores in Go names; func parameter bad_name should be badName/
+func g() (no_way int)                   { return 0 }  // MATCH /don't use underscores in Go names; func result no_way should be noWay/
+func (t *t_wow) f(more_under string)    {}            // MATCH /don't use underscores in Go names; method parameter more_under should be moreUnder/
+func (t *t_wow) g() (still_more string) { return "" } // MATCH /don't use underscores in Go names; method result still_more should be stillMore/
+
+type i interface {
+	CheckHtml() string // okay; interface method names are often constrained by the concrete types' method names
+
+	F(foo_bar int) // MATCH /don't use underscores in Go names; interface method parameter foo_bar should be fooBar/
+}
+
+// All okay; underscore between digits
+const case1_1 = 1
+
+type case2_1 struct {
+	case2_2 int
+}
+
+func case3_1(case3_2 int) (case3_3 string) {
+	case3_4 := 4
+	_ = case3_4
+
+	return ""
+}
+
+type t struct{}
+
+func (t) LastInsertId() (int64, error) { return 0, nil } // okay because it matches a known style violation
+
+//export exported_to_c
+func exported_to_c() {} // okay: https://github.com/golang/lint/issues/144
+
+//export exported_to_c_with_arg
+func exported_to_c_with_arg(but_use_go_param_names int) // MATCH /don't use underscores in Go names; func parameter but_use_go_param_names should be butUseGoParamNames/
+
+// This is an exported C function with a leading doc comment.
+//
+//export exported_to_c_with_comment
+func exported_to_c_with_comment() {} // okay: https://github.com/golang/lint/issues/144
+
+//export maybe_exported_to_CPlusPlusWithCamelCase
+func maybe_exported_to_CPlusPlusWithCamelCase() {} // okay: https://github.com/golang/lint/issues/144
+
+// WhyAreYouUsingCapitalLetters_InACFunctionName is a Go-exported function that
+// is also exported to C as a name with underscores.
+//
+// Don't do that. If you want to use a C-style name for a C export, make it
+// lower-case and leave it out of the Go-exported API.
+//
+//export WhyAreYouUsingCapitalLetters_InACFunctionName
+func WhyAreYouUsingCapitalLetters_InACFunctionName() {} // MATCH /don't use underscores in Go names; func WhyAreYouUsingCapitalLetters_InACFunctionName should be WhyAreYouUsingCapitalLettersInACFunctionName/

linter/utils.go

@@ -0,0 +1,118 @@
+package linter
+
+import (
+	"strings"
+	"unicode"
+)
+
+// LintName returns a different name if it should be different.
+func LintName(name string) (should string) {
+	// Fast path for simple cases: "_" and all lowercase.
+	if name == "_" {
+		return name
+	}
+	allLower := true
+	for _, r := range name {
+		if !unicode.IsLower(r) {
+			allLower = false
+			break
+		}
+	}
+	if allLower {
+		return name
+	}
+
+	// Split camelCase at any lower->upper transition, and split on underscores.
+	// Check each word for common initialisms.
+	runes := []rune(name)
+	w, i := 0, 0 // index of start of word, scan
+	for i+1 <= len(runes) {
+		eow := false // whether we hit the end of a word
+		if i+1 == len(runes) {
+			eow = true
+		} else if runes[i+1] == '_' {
+			// underscore; shift the remainder forward over any run of underscores
+			eow = true
+			n := 1
+			for i+n+1 < len(runes) && runes[i+n+1] == '_' {
+				n++
+			}
+
+			// Leave at most one underscore if the underscore is between two digits
+			if i+n+1 < len(runes) && unicode.IsDigit(runes[i]) && unicode.IsDigit(runes[i+n+1]) {
+				n--
+			}
+
+			copy(runes[i+1:], runes[i+n+1:])
+			runes = runes[:len(runes)-n]
+		} else if unicode.IsLower(runes[i]) && !unicode.IsLower(runes[i+1]) {
+			// lower->non-lower
+			eow = true
+		}
+		i++
+		if !eow {
+			continue
+		}
+
+		// [w,i) is a word.
+		word := string(runes[w:i])
+		if u := strings.ToUpper(word); commonInitialisms[u] {
+			// Keep consistent case, which is lowercase only at the start.
+			if w == 0 && unicode.IsLower(runes[w]) {
+				u = strings.ToLower(u)
+			}
+			// All the common initialisms are ASCII,
+			// so we can replace the bytes exactly.
+			copy(runes[w:], []rune(u))
+		} else if w > 0 && strings.ToLower(word) == word {
+			// already all lowercase, and not the first word, so uppercase the first character.
+			runes[w] = unicode.ToUpper(runes[w])
+		}
+		w = i
+	}
+	return string(runes)
+}
+
+// commonInitialisms is a set of common initialisms.
+// Only add entries that are highly unlikely to be non-initialisms.
+// For instance, "ID" is fine (Freudian code is rare), but "AND" is not.
+var commonInitialisms = map[string]bool{
+	"ACL":   true,
+	"API":   true,
+	"ASCII": true,
+	"CPU":   true,
+	"CSS":   true,
+	"DNS":   true,
+	"EOF":   true,
+	"GUID":  true,
+	"HTML":  true,
+	"HTTP":  true,
+	"HTTPS": true,
+	"ID":    true,
+	"IP":    true,
+	"JSON":  true,
+	"LHS":   true,
+	"QPS":   true,
+	"RAM":   true,
+	"RHS":   true,
+	"RPC":   true,
+	"SLA":   true,
+	"SMTP":  true,
+	"SQL":   true,
+	"SSH":   true,
+	"TCP":   true,
+	"TLS":   true,
+	"TTL":   true,
+	"UDP":   true,
+	"UI":    true,
+	"UID":   true,
+	"UUID":  true,
+	"URI":   true,
+	"URL":   true,
+	"UTF8":  true,
+	"VM":    true,
+	"XML":   true,
+	"XMPP":  true,
+	"XSRF":  true,
+	"XSS":   true,
+}

rule/argument-limit_test.go

@@ -1,34 +0,0 @@
-package rule
-
-import (
-	"testing"
-
-	"github.com/mgechev/revive/rule"
-	"github.com/mgechev/revive/testutil"
-)
-
-func TestArgumentLimit(t *testing.T) {
-	t.Parallel()
-
-	program := `
-	package foo
-
-	[@1]func foo(a int, b int, c int)[/@1] {
-		return a + b + c;
-	}
-	`
-	testutil.AssertFailures(t, program, &ArgumentsLimitRule{}, rule.Arguments{"2"})
-}
-
-func TestArgumentLimit2(t *testing.T) {
-	t.Parallel()
-
-	program := `
-	package foo
-
-	func foo(a int, b int, c int) {
-		return a + b + c;
-	}
-	`
-	testutil.AssertSuccess(t, program, &ArgumentsLimitRule{}, rule.Arguments{"3"})
-}

rule/names.go

@@ -5,7 +5,6 @@ import (
 	"go/ast"
 	"go/token"
 	"strings"
-	"unicode"
 
 	"github.com/mgechev/revive/linter"
 )
@@ -30,6 +29,17 @@ func (r *NamesRule) Apply(file *linter.File, arguments linter.Arguments) []linte
 		},
 	}
 
+	// Package names need slightly different handling than other names.
+	if strings.Contains(walker.fileAst.Name.Name, "_") && !strings.HasSuffix(walker.fileAst.Name.Name, "_test") {
+		walker.onFailure(linter.Failure{
+			Failure:    "don't use an underscore in package name",
+			Confidence: 1,
+			Node:       walker.fileAst,
+			Category:   "naming",
+			URL:        "http://golang.org/doc/effective_go.html#package-names",
+		})
+	}
+
 	ast.Walk(&walker, fileAst)
 
 	return failures
@@ -40,6 +50,71 @@ func (r *NamesRule) Name() string {
 	return "imports"
 }
 
+func checkList(fl *ast.FieldList, thing string, w *lintNames) {
+	if fl == nil {
+		return
+	}
+	for _, f := range fl.List {
+		for _, id := range f.Names {
+			check(id, thing, w)
+		}
+	}
+}
+
+func check(id *ast.Ident, thing string, w *lintNames) {
+	if id.Name == "_" {
+		return
+	}
+	if knownNameExceptions[id.Name] {
+		return
+	}
+
+	// Handle two common styles from other languages that don't belong in Go.
+	if len(id.Name) >= 5 && allCapsRE.MatchString(id.Name) && strings.Contains(id.Name, "_") {
+		w.onFailure(linter.Failure{
+			Failure:    "don't use ALL_CAPS in Go names; use CamelCase",
+			Confidence: 0.8,
+			Node:       id,
+			Category:   "naming",
+			URL:        "#mixed-caps",
+		})
+		return
+	}
+	if len(id.Name) > 2 && id.Name[0] == 'k' && id.Name[1] >= 'A' && id.Name[1] <= 'Z' {
+		should := string(id.Name[1]+'a'-'A') + id.Name[2:]
+		w.onFailure(linter.Failure{
+			Failure:    fmt.Sprintf("don't use leading k in Go names; %s %s should be %s", thing, id.Name, should),
+			Confidence: 0.8,
+			Node:       id,
+			Category:   "naming",
+			URL:        "#mixed-caps",
+		})
+	}
+
+	should := linter.LintName(id.Name)
+	if id.Name == should {
+		return
+	}
+
+	if len(id.Name) > 2 && strings.Contains(id.Name[1:], "_") {
+		w.onFailure(linter.Failure{
+			Failure:    fmt.Sprintf("don't use underscores in Go names; %s %s should be %s", thing, id.Name, should),
+			Confidence: 0.9,
+			Node:       id,
+			Category:   "naming",
+			URL:        "http://golang.org/doc/effective_go.html#mixed-caps",
+		})
+		return
+	}
+	w.onFailure(linter.Failure{
+		Failure:    fmt.Sprintf("%s %s should be %s", thing, id.Name, should),
+		Confidence: 0.8,
+		Node:       id,
+		Category:   "naming",
+		URL:        "#initialisms",
+	})
+}
+
 type lintNames struct {
 	file                   *linter.File
 	fileAst                *ast.File
@@ -49,12 +124,6 @@ type lintNames struct {
 }
 
 func (w *lintNames) Visit(n ast.Node) ast.Visitor {
-
-	// Package names need slightly different handling than other names.
-	// if strings.Contains(f.f.Name.Name, "_") && !strings.HasSuffix(f.f.Name.Name, "_test") {
-	// 	f.errorf(f.f, 1, link("http://golang.org/doc/effective_go.html#package-names"), category("naming"), "don't use an underscore in package name")
-	// }
-
 	switch v := n.(type) {
 	case *ast.AssignStmt:
 		if v.Tok == token.ASSIGN {
@@ -62,11 +131,11 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor {
 		}
 		for _, exp := range v.Lhs {
 			if id, ok := exp.(*ast.Ident); ok {
-				w.check(id, "var")
+				check(id, "var", w)
 			}
 		}
 	case *ast.FuncDecl:
-		if isTest(w.file) && (strings.HasPrefix(v.Name.Name, "Example") || strings.HasPrefix(v.Name.Name, "Test") || strings.HasPrefix(v.Name.Name, "Benchmark")) {
+		if w.file.IsTest() && (strings.HasPrefix(v.Name.Name, "Example") || strings.HasPrefix(v.Name.Name, "Test") || strings.HasPrefix(v.Name.Name, "Benchmark")) {
 			return w
 		}
 
@@ -79,11 +148,11 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor {
 		// not exported in the Go API.
 		// See https://github.com/golang/lint/issues/144.
 		if ast.IsExported(v.Name.Name) || !isCgoExported(v) {
-			w.check(v.Name, thing)
+			check(v.Name, thing, w)
 		}
 
-		w.checkList(v.Type.Params, thing+" parameter")
-		w.checkList(v.Type.Results, thing+" result")
+		checkList(v.Type.Params, thing+" parameter", w)
+		checkList(v.Type.Results, thing+" result", w)
 	case *ast.GenDecl:
 		if v.Tok == token.IMPORT {
 			return w
@@ -100,10 +169,10 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor {
 		for _, spec := range v.Specs {
 			switch s := spec.(type) {
 			case *ast.TypeSpec:
-				w.check(s.Name, thing)
+				check(s.Name, thing, w)
 			case *ast.ValueSpec:
 				for _, id := range s.Names {
-					w.check(id, thing)
+					check(id, thing, w)
 				}
 			}
 		}
@@ -115,150 +184,25 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor {
 			if !ok { // might be an embedded interface name
 				continue
 			}
-			w.checkList(ft.Params, "interface method parameter")
-			w.checkList(ft.Results, "interface method result")
+			checkList(ft.Params, "interface method parameter", w)
+			checkList(ft.Results, "interface method result", w)
 		}
 	case *ast.RangeStmt:
 		if v.Tok == token.ASSIGN {
 			return w
 		}
 		if id, ok := v.Key.(*ast.Ident); ok {
-			w.check(id, "range var")
+			check(id, "range var", w)
 		}
 		if id, ok := v.Value.(*ast.Ident); ok {
-			w.check(id, "range var")
+			check(id, "range var", w)
 		}
 	case *ast.StructType:
 		for _, f := range v.Fields.List {
 			for _, id := range f.Names {
-				w.check(id, "struct field")
+				check(id, "struct field", w)
 			}
 		}
 	}
 	return w
 }
-
-func (w *lintNames) check(id *ast.Ident, thing string) {
-	if id.Name == "_" {
-		return
-	}
-	if knownNameExceptions[id.Name] {
-		return
-	}
-
-	// Handle two common styles from other languages that don't belong in Go.
-	if len(id.Name) >= 5 && allCapsRE.MatchString(id.Name) && strings.Contains(id.Name, "_") {
-		w.onFailure(linter.Failure{
-			Failure:    fmt.Sprintf("don't use ALL_CAPS in Go names; use CamelCase"),
-			Confidence: 0.8,
-			Node:       id,
-		})
-		return
-	}
-	if len(id.Name) > 2 && id.Name[0] == 'k' && id.Name[1] >= 'A' && id.Name[1] <= 'Z' {
-		should := string(id.Name[1]+'a'-'A') + id.Name[2:]
-		w.onFailure(linter.Failure{
-			Failure:    fmt.Sprintf("don't use leading k in Go names; %s %s should be %s", thing, id.Name, should),
-			Confidence: 0.8,
-			Node:       id,
-		})
-	}
-
-	should := lintName(id.Name)
-	if id.Name == should {
-		return
-	}
-
-	if len(id.Name) > 2 && strings.Contains(id.Name[1:], "_") {
-		w.onFailure(linter.Failure{
-			Failure:    fmt.Sprintf("don't use underscores in Go names; %s %s should be %s", thing, id.Name, should),
-			Confidence: 0.9,
-			Node:       id,
-		})
-		return
-	}
-	w.onFailure(linter.Failure{
-		Failure:    fmt.Sprintf("%s %s should be %s", thing, id.Name, should),
-		Confidence: 0.8,
-		Node:       id,
-	})
-}
-
-func (w *lintNames) checkList(fl *ast.FieldList, thing string) {
-	if fl == nil {
-		return
-	}
-	for _, f := range fl.List {
-		for _, id := range f.Names {
-			w.check(id, thing)
-		}
-	}
-}
-
-// lintName returns a different name if it should be different.
-func lintName(name string) (should string) {
-	// Fast path for simple cases: "_" and all lowercase.
-	if name == "_" {
-		return name
-	}
-	allLower := true
-	for _, r := range name {
-		if !unicode.IsLower(r) {
-			allLower = false
-			break
-		}
-	}
-	if allLower {
-		return name
-	}
-
-	// Split camelCase at any lower->upper transition, and split on underscores.
-	// Check each word for common initialisms.
-	runes := []rune(name)
-	w, i := 0, 0 // index of start of word, scan
-	for i+1 <= len(runes) {
-		eow := false // whether we hit the end of a word
-		if i+1 == len(runes) {
-			eow = true
-		} else if runes[i+1] == '_' {
-			// underscore; shift the remainder forward over any run of underscores
-			eow = true
-			n := 1
-			for i+n+1 < len(runes) && runes[i+n+1] == '_' {
-				n++
-			}
-
-			// Leave at most one underscore if the underscore is between two digits
-			if i+n+1 < len(runes) && unicode.IsDigit(runes[i]) && unicode.IsDigit(runes[i+n+1]) {
-				n--
-			}
-
-			copy(runes[i+1:], runes[i+n+1:])
-			runes = runes[:len(runes)-n]
-		} else if unicode.IsLower(runes[i]) && !unicode.IsLower(runes[i+1]) {
-			// lower->non-lower
-			eow = true
-		}
-		i++
-		if !eow {
-			continue
-		}
-
-		// [w,i) is a word.
-		word := string(runes[w:i])
-		if u := strings.ToUpper(word); commonInitialisms[u] {
-			// Keep consistent case, which is lowercase only at the start.
-			if w == 0 && unicode.IsLower(runes[w]) {
-				u = strings.ToLower(u)
-			}
-			// All the common initialisms are ASCII,
-			// so we can replace the bytes exactly.
-			copy(runes[w:], []rune(u))
-		} else if w > 0 && strings.ToLower(word) == word {
-			// already all lowercase, and not the first word, so uppercase the first character.
-			runes[w] = unicode.ToUpper(runes[w])
-		}
-		w = i
-	}
-	return string(runes)
-}

testutil/lint_test.go

@@ -24,7 +24,6 @@ import (
 	"strconv"
 	"strings"
 	"testing"
-	"unicode"
 
 	"github.com/mgechev/revive/rule"
 
@@ -39,6 +38,7 @@ var rules = []linter.Rule{
 	&rule.DotImportsRule{},
 	&rule.BlankImportsRule{},
 	&rule.ExportedRule{},
+	&rule.NamesRule{},
 }
 
 func TestAll(t *testing.T) {
@@ -242,118 +242,6 @@ func TestLine(t *testing.T) {
 	}
 }
 
-// commonInitialisms is a set of common initialisms.
-// Only add entries that are highly unlikely to be non-initialisms.
-// For instance, "ID" is fine (Freudian code is rare), but "AND" is not.
-var commonInitialisms = map[string]bool{
-	"ACL":   true,
-	"API":   true,
-	"ASCII": true,
-	"CPU":   true,
-	"CSS":   true,
-	"DNS":   true,
-	"EOF":   true,
-	"GUID":  true,
-	"HTML":  true,
-	"HTTP":  true,
-	"HTTPS": true,
-	"ID":    true,
-	"IP":    true,
-	"JSON":  true,
-	"LHS":   true,
-	"QPS":   true,
-	"RAM":   true,
-	"RHS":   true,
-	"RPC":   true,
-	"SLA":   true,
-	"SMTP":  true,
-	"SQL":   true,
-	"SSH":   true,
-	"TCP":   true,
-	"TLS":   true,
-	"TTL":   true,
-	"UDP":   true,
-	"UI":    true,
-	"UID":   true,
-	"UUID":  true,
-	"URI":   true,
-	"URL":   true,
-	"UTF8":  true,
-	"VM":    true,
-	"XML":   true,
-	"XMPP":  true,
-	"XSRF":  true,
-	"XSS":   true,
-}
-
-// lintName returns a different name if it should be different.
-func lintName(name string) (should string) {
-	// Fast path for simple cases: "_" and all lowercase.
-	if name == "_" {
-		return name
-	}
-	allLower := true
-	for _, r := range name {
-		if !unicode.IsLower(r) {
-			allLower = false
-			break
-		}
-	}
-	if allLower {
-		return name
-	}
-
-	// Split camelCase at any lower->upper transition, and split on underscores.
-	// Check each word for common initialisms.
-	runes := []rune(name)
-	w, i := 0, 0 // index of start of word, scan
-	for i+1 <= len(runes) {
-		eow := false // whether we hit the end of a word
-		if i+1 == len(runes) {
-			eow = true
-		} else if runes[i+1] == '_' {
-			// underscore; shift the remainder forward over any run of underscores
-			eow = true
-			n := 1
-			for i+n+1 < len(runes) && runes[i+n+1] == '_' {
-				n++
-			}
-
-			// Leave at most one underscore if the underscore is between two digits
-			if i+n+1 < len(runes) && unicode.IsDigit(runes[i]) && unicode.IsDigit(runes[i+n+1]) {
-				n--
-			}
-
-			copy(runes[i+1:], runes[i+n+1:])
-			runes = runes[:len(runes)-n]
-		} else if unicode.IsLower(runes[i]) && !unicode.IsLower(runes[i+1]) {
-			// lower->non-lower
-			eow = true
-		}
-		i++
-		if !eow {
-			continue
-		}
-
-		// [w,i) is a word.
-		word := string(runes[w:i])
-		if u := strings.ToUpper(word); commonInitialisms[u] {
-			// Keep consistent case, which is lowercase only at the start.
-			if w == 0 && unicode.IsLower(runes[w]) {
-				u = strings.ToLower(u)
-			}
-			// All the common initialisms are ASCII,
-			// so we can replace the bytes exactly.
-			copy(runes[w:], []rune(u))
-		} else if w > 0 && strings.ToLower(word) == word {
-			// already all lowercase, and not the first word, so uppercase the first character.
-			runes[w] = unicode.ToUpper(runes[w])
-		}
-		w = i
-	}
-	return string(runes)
-}
-
 func TestLintName(t *testing.T) {
 	tests := []struct {
 		name, want string
@@ -387,7 +275,7 @@ func TestLintName(t *testing.T) {
 		{"IEEE802_16Bit", "IEEE802_16Bit"},
 	}
 	for _, test := range tests {
-		got := lintName(test.name)
+		got := linter.LintName(test.name)
 		if got != test.want {
 			t.Errorf("lintName(%q) = %q, want %q", test.name, got, test.want)
 		}