Repository: mgechev/revive
Pull request #761 (2022-10-17 14:43:11)

Comment spacing rule
<!-- ### IMPORTANT ### -->
<!-- Please do not create a Pull Request without creating an issue first.** -->
<!-- If you're fixing a typo or improving the documentation, you may not have to open an issue. -->

<!-- ### CHECKLIST ### -->
<!-- Please, describe in details what's your motivation for this PR -->
This PR will close issue #735. 
 
- Did you add tests? 
Yes, tests were added
- Does your code follow the coding style of the rest of the repository?
Yes, the code was written while keeping the coding style of the repo in mind. But, if you find any decrepancy, please feel free to request a change in the code review. 

- Does the Travis build passes? 
Yes, the build is passing along with all the tests. 
<!-- ### FOOTER (OPTIONAL) ### -->
<!-- If you're closing an issue, add "Closes #XXXX" in your comment. This way, the PR will be linked to the issue automatically. -->



Patch:
diff --git a/README.md b/README.md
index e2b97b1..56f4b36 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 114275f..0494c29 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.
@@ -565,11 +584,14 @@ This is geared towards user facing applications where string literals are often
 
 _Configuration_: Each argument is a slice containing 2-3 strings: a scope, a regex, and an optional error message.
 
-1. The first string defines a scope. This controls which string literals the regex will apply to, and is defined as a function argument. It must contain at least a function name (`core.WriteError`). Scopes may optionally contain a number specifying which argument in the function to check (`core.WriteError[1]`), as well as a struct field (`core.WriteError[1].Message`, only works for top level fields). Function arguments are counted starting at 0, so `[0]` would refer to the first argument, `[1]` would refer to the second, etc. If no argument number is provided, the first argument will be used (same as `[0]`).
+1. The first string defines a **scope**. This controls which string literals the regex will apply to, and is defined as a function argument. It must contain at least a function name (`core.WriteError`). Scopes may optionally contain a number specifying which argument in the function to check (`core.WriteError[1]`), as well as a struct field (`core.WriteError[1].Message`, only works for top level fields). Function arguments are counted starting at 0, so `[0]` would refer to the first argument, `[1]` would refer to the second, etc. If no argument number is provided, the first argument will be used (same as `[0]`).
+
+2. The second string is a **regular expression** (beginning and ending with a `/` character), which will be used to check the string literals in the scope. The default semantics is "_strings matching the regular expression are OK_". If you need to inverse the semantics you can add a `!` just before the first `/`. Examples:
 
-2. The second string is a regular expression (beginning and ending with a `/` character), which will be used to check the string literals in the scope.
+    * with `"/^[A-Z].*$/"` the rule will **accept** strings starting with capital letters
+    * with `"!/^[A-Z].*$/"` the rule will a **fail** on strings starting with capital letters
 
-3. The third string (optional) is a message containing the purpose for the regex, which will be used in lint errors.
+3. The third string (optional) is a **message** containing the purpose for the regex, which will be used in lint errors.
 
 Example:
 
diff --git a/config/config.go b/config/config.go
index 5c63f35..d6b4f41 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/go.mod b/go.mod
index c6590e0..a02ae88 100644
--- a/go.mod
+++ b/go.mod
@@ -3,20 +3,20 @@ module github.com/mgechev/revive
 go 1.19
 
 require (
-	github.com/BurntSushi/toml v1.2.0
-	github.com/chavacava/garif v0.0.0-20220630083739-93517212f375
+	github.com/BurntSushi/toml v1.2.1
+	github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348
 	github.com/fatih/color v1.13.0
 	github.com/fatih/structtag v1.2.0
 	github.com/mgechev/dots v0.0.0-20210922191527-e955255bf517
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/pkg/errors v0.9.1
-	golang.org/x/tools v0.1.12
+	golang.org/x/tools v0.2.0
 )
 
 require (
 	github.com/mattn/go-colorable v0.1.9 // indirect
 	github.com/mattn/go-isatty v0.0.14 // indirect
 	github.com/mattn/go-runewidth v0.0.9 // indirect
-	golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect
+	golang.org/x/sys v0.1.0 // indirect
 )
diff --git a/go.sum b/go.sum
index 1780e76..c10681b 100644
--- a/go.sum
+++ b/go.sum
@@ -1,7 +1,11 @@
 github.com/BurntSushi/toml v1.2.0 h1:Rt8g24XnyGTyglgET/PRUNlrUeu9F5L+7FilkXfZgs0=
 github.com/BurntSushi/toml v1.2.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
+github.com/BurntSushi/toml v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
+github.com/BurntSushi/toml v1.2.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
 github.com/chavacava/garif v0.0.0-20220630083739-93517212f375 h1:E7LT642ysztPWE0dfz43cWOvMiF42DyTRC+eZIaO4yI=
 github.com/chavacava/garif v0.0.0-20220630083739-93517212f375/go.mod h1:4m1Rv7xfuwWPNKXlThldNuJvutYM6J95wNuuVmn55To=
+github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348 h1:cy5GCEZLUCshCGCRRUjxHrDUqkB4l5cuUt3ShEckQEo=
+github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348/go.mod h1:f/miWtG3SSuTxKsNK3o58H1xl+XV6ZIfbC6p7lPPB8U=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -28,16 +32,22 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
+github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U=
+golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU=
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
+golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE=
+golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
diff --git a/rule/comment-spacings.go b/rule/comment-spacings.go
new file mode 100644
index 0000000..abe2ad7
--- /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/rule/identical-branches.go b/rule/identical-branches.go
index b1a6909..9222c8a 100644
--- a/rule/identical-branches.go
+++ b/rule/identical-branches.go
@@ -63,8 +63,10 @@ func (lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool {
 	}
 
 	ref := gofmt(branches[0])
+	refSize := len(branches[0].List)
 	for i := 1; i < len(branches); i++ {
-		if gofmt(branches[i]) != ref {
+		currentSize := len(branches[i].List)
+		if currentSize != refSize || gofmt(branches[i]) != ref {
 			return false
 		}
 	}
diff --git a/rule/string-format.go b/rule/string-format.go
index e7841e8..0e30ebf 100644
--- a/rule/string-format.go
+++ b/rule/string-format.go
@@ -68,6 +68,7 @@ type stringFormatSubrule struct {
 	parent       *lintStringFormatRule
 	scope        stringFormatSubruleScope
 	regexp       *regexp.Regexp
+	negated      bool
 	errorMessage string
 }
 
@@ -89,17 +90,18 @@ var parseStringFormatScope = regexp.MustCompile(
 
 func (w *lintStringFormatRule) parseArguments(arguments lint.Arguments) {
 	for i, argument := range arguments {
-		scope, regex, errorMessage := w.parseArgument(argument, i)
+		scope, regex, negated, errorMessage := w.parseArgument(argument, i)
 		w.rules = append(w.rules, stringFormatSubrule{
 			parent:       w,
 			scope:        scope,
 			regexp:       regex,
+			negated:      negated,
 			errorMessage: errorMessage,
 		})
 	}
 }
 
-func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (scope stringFormatSubruleScope, regex *regexp.Regexp, errorMessage string) {
+func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (scope stringFormatSubruleScope, regex *regexp.Regexp, negated bool, errorMessage string) {
 	g, ok := argument.([]interface{}) // Cast to generic slice first
 	if !ok {
 		w.configError("argument is not a slice", ruleNum, 0)
@@ -146,7 +148,12 @@ func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (
 	}
 
 	// Strip / characters from the beginning and end of rule[1] before compiling
-	regex, err := regexp.Compile(rule[1][1 : len(rule[1])-1])
+	negated = rule[1][0] == '!'
+	offset := 1
+	if negated {
+		offset++
+	}
+	regex, err := regexp.Compile(rule[1][offset : len(rule[1])-1])
 	if err != nil {
 		w.parseError(fmt.Sprintf("unable to compile %s as regexp", rule[1]), ruleNum, 1)
 	}
@@ -155,7 +162,7 @@ func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (
 	if len(rule) == 3 {
 		errorMessage = rule[2]
 	}
-	return scope, regex, errorMessage
+	return scope, regex, negated, errorMessage
 }
 
 // Report an invalid config, this is specifically the user's fault
@@ -261,7 +268,26 @@ func (r *stringFormatSubrule) Apply(call *ast.CallExpr) {
 }
 
 func (r *stringFormatSubrule) lintMessage(s string, node ast.Node) {
-	// Fail if the string doesn't match the user's regex
+	if r.negated {
+		if !r.regexp.MatchString(s) {
+			return
+		}
+		// Fail if the string does match the user's regex
+		var failure string
+		if len(r.errorMessage) > 0 {
+			failure = r.errorMessage
+		} else {
+			failure = fmt.Sprintf("string literal matches user defined regex /%s/", r.regexp.String())
+		}
+		r.parent.onFailure(lint.Failure{
+			Confidence: 1,
+			Failure:    failure,
+			Node:       node,
+		})
+		return
+	}
+
+	// Fail if the string does NOT match the user's regex
 	if r.regexp.MatchString(s) {
 		return
 	}


Test patch:
diff --git a/test/comment-spacings_test.go b/test/comment-spacings_test.go
new file mode 100644
index 0000000..d568502
--- /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 afb88ef..bb05b00 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/string-format_test.go b/test/string-format_test.go
index 7f23d7e..f58c10c 100644
--- a/test/string-format_test.go
+++ b/test/string-format_test.go
@@ -20,7 +20,7 @@ func TestStringFormat(t *testing.T) {
 				"/[^\\.]$/"}, // Must not end with a period
 			[]interface{}{
 				"s.Method3[2]",
-				"/^[^Tt][^Hh]/",
+				"!/^[Tt][Hh]/",
 				"must not start with 'th'"}}})
 }
 
diff --git a/test/utils.go b/test/utils.go
index 536e8cd..3de6761 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 0000000..a5465fa
--- /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