From 5f26378cc2a473772ff1d78e37c44971827d04cb Mon Sep 17 00:00:00 2001 From: Shubh Karman Singh <54154054+qascade@users.noreply.github.com> Date: Sat, 29 Oct 2022 23:02:42 +0530 Subject: [PATCH] Comment spacing rule (#761) * added comment-spacing rule for revive * added test for comment-spacings rule * adds comment-spacings rule to the configuration * renames the source file to match rule name * adds some tests * refactor Comment-Spacings Rule to remove whiteList and avoid Panic at empty comment * refactoring and adds rule configuration * adds rule documentation Co-authored-by: chavacava --- README.md | 2 + RULES_DESCRIPTIONS.md | 19 ++++++++ config/config.go | 1 + rule/comment-spacings.go | 82 ++++++++++++++++++++++++++++++++ test/comment-spacings_test.go | 15 ++++++ test/disable-annotations_test.go | 2 +- test/utils.go | 2 +- testdata/comment-spacings.go | 24 ++++++++++ 8 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 rule/comment-spacings.go create mode 100644 test/comment-spacings_test.go create mode 100644 testdata/comment-spacings.go diff --git a/README.md b/README.md index e2b97b1dc..56f4b36ad 100644 --- a/README.md +++ b/README.md @@ -499,6 +499,8 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`optimize-operands-order`](./RULES_DESCRIPTIONS.md#optimize-operands-order) | n/a | Checks inefficient conditional expressions | no | no | | [`use-any`](./RULES_DESCRIPTIONS.md#use-any) | n/a | Proposes to replace `interface{}` with its alias `any` | no | no | | [`datarace`](./RULES_DESCRIPTIONS.md#datarace) | n/a | Spots potential dataraces | no | no | +| [`comment-spacings`](./RULES_DESCRIPTIONS.md#comment-spacings) | []string | Warns on malformed comments | no | no | + ## Configurable rules diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 79d9a4988..0494c29b0 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -13,6 +13,7 @@ List of all available rules. - [bool-literal-in-expr](#bool-literal-in-expr) - [call-to-gc](#call-to-gc) - [confusing-naming](#confusing-naming) + - [comment-spacings](#comment-spacings) - [confusing-results](#confusing-results) - [cognitive-complexity](#cognitive-complexity) - [constant-logical-expr](#constant-logical-expr) @@ -168,6 +169,24 @@ Example: arguments =[7] ``` +## comment-spacings +_Description_: Spots comments of the form: +```go +//This is a malformed comment: no space between // and the start of the sentence +``` + +_Configuration_: ([]string) list of exceptions. For example, to accept comments of the form +```go +//mypragma: activate something +``` +You need to add `"mypragma"` in the configuration + +Example: + +```toml +[rule.comment-spacings] + arguments =["mypragma","otherpragma"] +``` ## confusing-naming _Description_: Methods or fields of `struct` that have names different only by capitalization could be confusing. diff --git a/config/config.go b/config/config.go index 5c63f35b3..d6b4f4100 100644 --- a/config/config.go +++ b/config/config.go @@ -86,6 +86,7 @@ var allRules = append([]lint.Rule{ &rule.OptimizeOperandsOrderRule{}, &rule.UseAnyRule{}, &rule.DataRaceRule{}, + &rule.CommentSpacingsRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/rule/comment-spacings.go b/rule/comment-spacings.go new file mode 100644 index 000000000..abe2ad76d --- /dev/null +++ b/rule/comment-spacings.go @@ -0,0 +1,82 @@ +package rule + +import ( + "fmt" + "strings" + "sync" + + "github.com/mgechev/revive/lint" +) + +// CommentSpacings Rule check the whether there is a space between +// the comment symbol( // ) and the start of the comment text +type CommentSpacingsRule struct { + allowList []string + sync.Mutex +} + +func (r *CommentSpacingsRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() + + if r.allowList == nil { + r.allowList = []string{ + "//go:", + "//revive:", + } + + for _, arg := range arguments { + allow, ok := arg.(string) // Alt. non panicking version + if !ok { + panic(fmt.Sprintf("invalid argument %v for %s; expected string but got %T", arg, r.Name(), arg)) + } + r.allowList = append(r.allowList, `//`+allow+`:`) + } + } +} + +func (r *CommentSpacingsRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + r.configure(args) + + var failures []lint.Failure + + for _, cg := range file.AST.Comments { + for _, comment := range cg.List { + commentLine := comment.Text + if len(commentLine) < 3 { + continue // nothing to do + } + + isOK := commentLine[2] == ' ' + if isOK { + continue + } + + if r.isAllowed(commentLine) { + continue + } + + failures = append(failures, lint.Failure{ + Node: comment, + Confidence: 1, + Category: "style", + Failure: "no space between comment delimiter and comment text", + }) + } + } + return failures +} + +func (*CommentSpacingsRule) Name() string { + return "comment-spacings" +} + +func (r *CommentSpacingsRule) isAllowed(line string) bool { + for _, allow := range r.allowList { + if strings.HasPrefix(line, allow) { + return true + } + } + + return false +} diff --git a/test/comment-spacings_test.go b/test/comment-spacings_test.go new file mode 100644 index 000000000..d5685021b --- /dev/null +++ b/test/comment-spacings_test.go @@ -0,0 +1,15 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" +) + +func TestCommentSpacings(t *testing.T) { + + testRule(t, "comment-spacings", &rule.CommentSpacingsRule{}, &lint.RuleConfig{ + Arguments: []interface{}{"myOwnDirective"}}, + ) +} diff --git a/test/disable-annotations_test.go b/test/disable-annotations_test.go index afb88ef00..bb05b004c 100644 --- a/test/disable-annotations_test.go +++ b/test/disable-annotations_test.go @@ -17,4 +17,4 @@ func TestModifiedAnnotations(t *testing.T) { func TestDisableNextLineAnnotations(t *testing.T) { testRule(t, "disable-annotations3", &rule.VarNamingRule{}, &lint.RuleConfig{}) -} \ No newline at end of file +} diff --git a/test/utils.go b/test/utils.go index 536e8cdbe..3de676180 100644 --- a/test/utils.go +++ b/test/utils.go @@ -107,7 +107,7 @@ func assertFailures(t *testing.T, baseDir string, fi os.FileInfo, src []byte, ru copy(failures[i:], failures[i+1:]) failures = failures[:len(failures)-1] - // t.Logf("/%v/ matched at %s:%d", in.Match, fi.Name(), in.Line) + //t.Logf("/%v/ matched at %s:%d", in.Match, fi.Name(), in.Line) ok = true break } diff --git a/testdata/comment-spacings.go b/testdata/comment-spacings.go new file mode 100644 index 000000000..a5465fa24 --- /dev/null +++ b/testdata/comment-spacings.go @@ -0,0 +1,24 @@ +package commentspacings + +//revive:disable:cyclomatic +type some struct{} + +//revive:disable:cyclomatic High complexity score but easy to understand +type some1 struct{} + +// Some nice comment +// +// With an empty line in the middle will make the rule panic! +func itsATrap() {} + +// This is a well formed comment +type hello struct { + random `json:random` +} + +// MATCH:21 /no space between comment delimiter and comment text/ + +//This comment does not respect the spacing rule! +var a string + +//myOwnDirective: do something