Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add file-length-limit rule #1072

Merged
merged 3 commits into from
Oct 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
| [`enforce-repeated-arg-type-style`](./RULES_DESCRIPTIONS.md#enforce-repeated-arg-type-style) | string (defaults to "any") | Enforces consistent style for repeated argument and/or return value types. | no | no |
| [`max-control-nesting`](./RULES_DESCRIPTIONS.md#max-control-nesting) | int (defaults to 5) | Sets restriction for maximum nesting of control structures. | no | no |
| [`comments-density`](./RULES_DESCRIPTIONS.md#comments-density) | int (defaults to 0) | Enforces a minimum comment / code relation | no | no |
| [`file-length-limit`](./RULES_DESCRIPTIONS.md#file-length-limit) | map (optional)| Enforces a maximum number of lines per file | no | no |


## Configurable rules
Expand Down
18 changes: 18 additions & 0 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ List of all available rules.
- [errorf](#errorf)
- [exported](#exported)
- [file-header](#file-header)
- [file-length-limit](#file-length-limit)
- [flag-parameter](#flag-parameter)
- [function-length](#function-length)
- [function-result-limit](#function-result-limit)
Expand Down Expand Up @@ -501,6 +502,23 @@ Example:
arguments = ["This is the text that must appear at the top of source files."]
```

## file-length-limit

_Description_: This rule enforces a maximum number of lines per file, in order to aid in maintainability and reduce complexity.

_Configuration_:

* `max` (int) a maximum number of lines in a file. Must be non-negative integers. 0 means the rule is disabled (default `0`);
* `skipComments` (bool) if true ignore and do not count lines containing just comments (default `false`);
* `skipBlankLines` (bool) if true ignore and do not count lines made up purely of whitespace (default `false`).

Example:

```toml
[rule.file-length-limit]
arguments = [{max=100,skipComments=true,skipBlankLines=true}]
```

## flag-parameter

_Description_: If a function controls the flow of another by passing it information on what to do, both functions are said to be [control-coupled](https://en.wikipedia.org/wiki/Coupling_(computer_programming)#Procedural_programming).
Expand Down
1 change: 1 addition & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ var allRules = append([]lint.Rule{
&rule.EnforceSliceStyleRule{},
&rule.MaxControlNestingRule{},
&rule.CommentsDensityRule{},
&rule.FileLengthLimitRule{},
}, defaultRules...)

var allFormatters = []lint.Formatter{
Expand Down
138 changes: 138 additions & 0 deletions rule/file-length-limit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package rule

import (
"bufio"
"bytes"
"fmt"
"go/ast"
"go/token"
"strings"
"sync"

"github.com/mgechev/revive/lint"
)

// FileLengthLimitRule lints the number of lines in a file.
type FileLengthLimitRule struct {
// max is the maximum number of lines allowed in a file. 0 means the rule is disabled.
max int
// skipComments indicates whether to skip comment lines when counting lines.
skipComments bool
// skipBlankLines indicates whether to skip blank lines when counting lines.
skipBlankLines bool
sync.Mutex
}

// Apply applies the rule to given file.
func (r *FileLengthLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
r.configure(arguments)

if r.max <= 0 {
// when max is negative or 0 the rule is disabled
return nil
}

all := 0
blank := 0
scanner := bufio.NewScanner(bytes.NewReader(file.Content()))
for scanner.Scan() {
all++
if len(bytes.TrimSpace(scanner.Bytes())) == 0 {
blank++
}
}

if err := scanner.Err(); err != nil {
panic(err.Error())
}

lines := all
if r.skipComments {
lines -= countCommentLines(file.AST.Comments)
}

if r.skipBlankLines {
lines -= blank
}

if lines <= r.max {
return nil
}

return []lint.Failure{
{
Category: "code-style",
Confidence: 1,
Position: lint.FailurePosition{
Start: token.Position{
Filename: file.Name,
Line: all,
},
},
Failure: fmt.Sprintf("file length is %d lines, which exceeds the limit of %d", lines, r.max),
},
}
}

func (r *FileLengthLimitRule) configure(arguments lint.Arguments) {
r.Lock()
defer r.Unlock()

alexandear marked this conversation as resolved.
Show resolved Hide resolved
if r.max != 0 {
return // already configured
}

if len(arguments) < 1 {
return // use default
}

argKV, ok := arguments[0].(map[string]any)
if !ok {
panic(fmt.Sprintf(`invalid argument to the "file-length-limit" rule. Expecting a k,v map, got %T`, arguments[0]))
}
for k, v := range argKV {
switch k {
case "max":
maxLines, ok := v.(int64)
if !ok || maxLines < 0 {
panic(fmt.Sprintf(`invalid configuration value for max lines in "file-length-limit" rule; need positive int64 but got %T`, arguments[0]))
}
r.max = int(maxLines)
case "skipComments":
skipComments, ok := v.(bool)
if !ok {
panic(fmt.Sprintf(`invalid configuration value for skip comments in "file-length-limit" rule; need bool but got %T`, arguments[1]))
}
r.skipComments = skipComments
case "skipBlankLines":
skipBlankLines, ok := v.(bool)
if !ok {
panic(fmt.Sprintf(`invalid configuration value for skip blank lines in "file-length-limit" rule; need bool but got %T`, arguments[2]))
}
r.skipBlankLines = skipBlankLines
}
}
}

// Name returns the rule name.
func (*FileLengthLimitRule) Name() string {
return "file-length-limit"
}

func countCommentLines(comments []*ast.CommentGroup) int {
count := 0
for _, cg := range comments {
for _, comment := range cg.List {
if len(comment.Text) < 2 {
continue
}
switch comment.Text[1] {
alexandear marked this conversation as resolved.
Show resolved Hide resolved
case '/': // single-line comment
count++
case '*': // multi-line comment
count += strings.Count(comment.Text, "\n") + 1
}
}
}
return count
}
32 changes: 32 additions & 0 deletions test/file-length-limit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package test

import (
"testing"

"github.com/mgechev/revive/lint"
"github.com/mgechev/revive/rule"
)

func TestFileLengthLimit(t *testing.T) {
testRule(t, "file-length-limit-disabled", &rule.FileLengthLimitRule{}, &lint.RuleConfig{
Arguments: []any{},
})
testRule(t, "file-length-limit-disabled", &rule.FileLengthLimitRule{}, &lint.RuleConfig{
Arguments: []any{map[string]any{"max": int64(0)}},
})
testRule(t, "file-length-limit-disabled", &rule.FileLengthLimitRule{}, &lint.RuleConfig{
Arguments: []any{map[string]any{"skipComments": true, "skipBlankLines": true}},
})
testRule(t, "file-length-limit-9", &rule.FileLengthLimitRule{}, &lint.RuleConfig{
Arguments: []any{map[string]any{"max": int64(9)}},
})
testRule(t, "file-length-limit-7-skip-comments", &rule.FileLengthLimitRule{}, &lint.RuleConfig{
Arguments: []any{map[string]any{"max": int64(7), "skipComments": true}},
})
testRule(t, "file-length-limit-6-skip-blank", &rule.FileLengthLimitRule{}, &lint.RuleConfig{
Arguments: []any{map[string]any{"max": int64(6), "skipBlankLines": true}},
})
testRule(t, "file-length-limit-4-skip-comments-skip-blank", &rule.FileLengthLimitRule{}, &lint.RuleConfig{
Arguments: []any{map[string]any{"max": int64(4), "skipComments": true, "skipBlankLines": true}},
})
}
10 changes: 10 additions & 0 deletions testdata/file-length-limit-4-skip-comments-skip-blank.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package fixtures

import "fmt"

// Foo is a function.
func Foo(a, b int) {
fmt.Println("Hello, world!")
}

// MATCH /file length is 5 lines, which exceeds the limit of 4/
10 changes: 10 additions & 0 deletions testdata/file-length-limit-6-skip-blank.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package fixtures

import "fmt"

// Foo is a function.
func Foo(a, b int) {
fmt.Println("Hello, world!")
}

// MATCH /file length is 7 lines, which exceeds the limit of 6/
20 changes: 20 additions & 0 deletions testdata/file-length-limit-7-skip-comments.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package fixtures

import "fmt"

// Foo is a function.
func Foo(a, b int) {
// This
/* is
a
*/
// a comment.
fmt.Println("Hello, world!")
/*
This is
multiline
comment.
*/
}

// MATCH /file length is 8 lines, which exceeds the limit of 7/
10 changes: 10 additions & 0 deletions testdata/file-length-limit-9.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package fixtures

import "fmt"

// Foo is a function.
func Foo(a, b int) {
fmt.Println("Hello, world!")
}

// MATCH /file length is 10 lines, which exceeds the limit of 9/
8 changes: 8 additions & 0 deletions testdata/file-length-limit-disabled.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package fixtures

import "fmt"

// Foo is a function.
func Foo(a, b int) {
fmt.Println("Hello, world!")
}