From 1412d0c6ed29a581e6258480ac8bc39823152bbd Mon Sep 17 00:00:00 2001 From: SalvadorC Date: Thu, 23 Jul 2020 01:17:20 +0200 Subject: [PATCH] new rule: unexported-naming (#443) * new rule: unexported-naming * better failure message --- README.md | 1 + RULES_DESCRIPTIONS.md | 7 ++ config.go | 1 + rule/unexported-naming.go | 115 +++++++++++++++++++++++++++++++++ test/unxeported-naming_test.go | 11 ++++ testdata/unexported-naming.go | 21 ++++++ 6 files changed, 156 insertions(+) create mode 100644 rule/unexported-naming.go create mode 100644 test/unxeported-naming_test.go create mode 100644 testdata/unexported-naming.go diff --git a/README.md b/README.md index e3ce248..db02f15 100644 --- a/README.md +++ b/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 diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 62fd2a5..b7dfd76 100644 --- a/RULES_DESCRIPTIONS.md +++ b/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. diff --git a/config.go b/config.go index 65f5bf9..5a45377 100644 --- a/config.go +++ b/config.go @@ -88,6 +88,7 @@ var allRules = append([]lint.Rule{ &rule.UnconditionalRecursionRule{}, &rule.IdenticalBranchesRule{}, &rule.DeferRule{}, + &rule.UnexportedNamingRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/rule/unexported-naming.go b/rule/unexported-naming.go new file mode 100644 index 0000000..96cec3e --- /dev/null +++ b/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()), + }) + } + } +} diff --git a/test/unxeported-naming_test.go b/test/unxeported-naming_test.go new file mode 100644 index 0000000..d689bef --- /dev/null +++ b/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{}) +} diff --git a/testdata/unexported-naming.go b/testdata/unexported-naming.go new file mode 100644 index 0000000..880a3d7 --- /dev/null +++ b/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 +}