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 <[email protected]>

Author: qascade

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
 

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.

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{

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
+}

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"}},
+	)
+}

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{})
-}
+}

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
 			}

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