Skip to content

Commit

Permalink
Comment spacing rule (#761)
Browse files Browse the repository at this point in the history
* 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 <salvadorcavadini+github@gmail.com>
  • Loading branch information
qascade and chavacava committed Oct 29, 2022
1 parent 5fd3b2c commit 5f26378
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 2 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
19 changes: 19 additions & 0 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ var allRules = append([]lint.Rule{
&rule.OptimizeOperandsOrderRule{},
&rule.UseAnyRule{},
&rule.DataRaceRule{},
&rule.CommentSpacingsRule{},
}, defaultRules...)

var allFormatters = []lint.Formatter{
Expand Down
82 changes: 82 additions & 0 deletions rule/comment-spacings.go
Original file line number Diff line number Diff line change
@@ -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
}
15 changes: 15 additions & 0 deletions test/comment-spacings_test.go
Original file line number Diff line number Diff line change
@@ -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"}},
)
}
2 changes: 1 addition & 1 deletion test/disable-annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ func TestModifiedAnnotations(t *testing.T) {

func TestDisableNextLineAnnotations(t *testing.T) {
testRule(t, "disable-annotations3", &rule.VarNamingRule{}, &lint.RuleConfig{})
}
}
2 changes: 1 addition & 1 deletion test/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
24 changes: 24 additions & 0 deletions testdata/comment-spacings.go
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 5f26378

Please sign in to comment.