new rule: unexported-naming (#443)

* new rule: unexported-naming

* better failure message

README.md

@@ -349,6 +349,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`unconditional-recursion`](./RULES_DESCRIPTIONS.md#unconditional-recursion)          |  n/a   | Warns on function calls that will lead to (direct) infinite recursion |    no    |  no   |
 | [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches)          |  n/a   | Spots if-then-else statements with identical `then` and `else` branches       |    no    |  no   |
 | [`defer`](./RULES_DESCRIPTIONS.md#defer)          |  map   |  Warns on some [defer gotchas](https://blog.learngoprogramming.com/5-gotchas-of-defer-in-go-golang-part-iii-36a1ab3d6ef1)       |    no    |  no   |
+| [`unexported-naming`](./RULES_DESCRIPTIONS.md#unexported-naming)          |  n/a   |  Warns on wrongly named un-exported symbols       |    no    |  no   |
 
 ## Configurable rules
 

RULES_DESCRIPTIONS.md

@@ -57,6 +57,7 @@ List of all available rules.
   - [var-naming](#var-naming)
   - [var-declaration](#var-declaration)
   - [unconditional-recursion](#unconditional-recursion)
+  - [unexported-naming](#unexported-naming)
   - [unexported-return](#unexported-return)
   - [unhandled-error](#unhandled-error)
   - [unnecessary-stmt](#unnecessary-stmt)
@@ -517,6 +518,12 @@ _Description_: Unconditional recursive calls will produce infinite recursion, th
 
 _Configuration_: N/A
 
+## unexported-naming
+
+_Description_: this rule warns on wrongly named un-exported symbols, i.e. un-exported symbols whose name start with a capital letter.
+
+_Configuration_: N/A
+
 ## unexported-return
 
 _Description_: This rule warns when an exported function or method returns a value of an un-exported type.

config.go

@@ -88,6 +88,7 @@ var allRules = append([]lint.Rule{
 	&rule.UnconditionalRecursionRule{},
 	&rule.IdenticalBranchesRule{},
 	&rule.DeferRule{},
+	&rule.UnexportedNamingRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

rule/unexported-naming.go

@@ -0,0 +1,115 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// UnexportedNamingRule lints wrongly named unexported symbols.
+type UnexportedNamingRule struct{}
+
+// Apply applies the rule to given file.
+func (r *UnexportedNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	ba := &unexportablenamingLinter{onFailure}
+	ast.Walk(ba, file.AST)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (r *UnexportedNamingRule) Name() string {
+	return "unexported-naming"
+}
+
+type unexportablenamingLinter struct {
+	onFailure func(lint.Failure)
+}
+
+func (unl unexportablenamingLinter) Visit(node ast.Node) ast.Visitor {
+	switch n := node.(type) {
+	case *ast.FuncDecl:
+		unl.lintFunction(n.Type, n.Body)
+		return nil
+	case *ast.FuncLit:
+		unl.lintFunction(n.Type, n.Body)
+
+		return nil
+	case *ast.AssignStmt:
+		if n.Tok != token.DEFINE {
+			return nil
+		}
+
+		ids := []*ast.Ident{}
+		for _, e := range n.Lhs {
+			id, ok := e.(*ast.Ident)
+			if !ok {
+				continue
+			}
+			ids = append(ids, id)
+		}
+
+		unl.lintIDs(ids)
+
+	case *ast.DeclStmt:
+		gd, ok := n.Decl.(*ast.GenDecl)
+		if !ok {
+			return nil
+		}
+
+		if len(gd.Specs) < 1 {
+			return nil
+		}
+
+		vs, ok := gd.Specs[0].(*ast.ValueSpec)
+		if !ok {
+			return nil
+		}
+
+		unl.lintIDs(vs.Names)
+	}
+
+	return unl
+}
+
+func (unl unexportablenamingLinter) lintFunction(ft *ast.FuncType, body *ast.BlockStmt) {
+	unl.lintFields(ft.Params)
+	unl.lintFields(ft.Results)
+
+	if body != nil {
+		ast.Walk(unl, body)
+	}
+}
+
+func (unl unexportablenamingLinter) lintFields(fields *ast.FieldList) {
+	if fields == nil {
+		return
+	}
+
+	ids := []*ast.Ident{}
+	for _, field := range fields.List {
+		ids = append(ids, field.Names...)
+	}
+
+	unl.lintIDs(ids)
+}
+
+func (unl unexportablenamingLinter) lintIDs(ids []*ast.Ident) {
+	for _, id := range ids {
+		if id.IsExported() {
+			unl.onFailure(lint.Failure{
+				Node:       id,
+				Confidence: 1,
+				Category:   "naming",
+				Failure:    fmt.Sprintf("the symbol %s is local, its name should start with a lowercase letter", id.String()),
+			})
+		}
+	}
+}

test/unxeported-naming_test.go

@@ -0,0 +1,11 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestUnexportednaming(t *testing.T) {
+	testRule(t, "unexported-naming", &rule.UnexportedNamingRule{})
+}

testdata/unexported-naming.go

@@ -0,0 +1,21 @@
+package fixtures
+
+var unexported string
+var Exported string
+
+func unexportednaming(
+	S int, // MATCH /the symbol S is local, its name should start with a lowercase letter/
+	s int,
+) (
+	Result bool, // MATCH /the symbol Result is local, its name should start with a lowercase letter/
+	result bool,
+) {
+	var NotExportable int // MATCH /the symbol NotExportable is local, its name should start with a lowercase letter/
+	var local float32
+	{
+		OtherNotExportable := 0 // MATCH /the symbol OtherNotExportable is local, its name should start with a lowercase letter/
+	}
+	const NotExportableConstant = "local" // MATCH /the symbol NotExportableConstant is local, its name should start with a lowercase letter/
+
+	return
+}