Repository: mgechev/revive
Pull request #1072 (2024-10-24 21:43:11)

feat: add `file-length-limit` rule
This PR adds a new rule `file-length-limit`.
The rule is inspired on the eslint rule [max-length](https://eslint.org/docs/latest/rules/max-lines).

This rule has options:

- `max` (default `0`) enforces a maximum number of lines in a file, `0` means the rule is disabled;
- `skipBlankLines` (default `false`) if `true` ignore lines made up purely of whitespace;
- `skipComments` (default `false`) if `true` ignore lines containing just comments.



ccoVeille: For record, archives and context

The following linter made by @DanilXO has been merged into [golangci-lint](https://github.com/golangci/golangci-lint/pull/5081)

https://github.com/DanilXO/filen


It also was discussed here
https://github.com/golangci/golangci-lint/discussions/2881
DanilXO: SkipBlankLines is an interesting addition. I'll add it to my linter too. I was thinking about revive as a place to add this, but I liked the idea of ​​golangci-lint with little disabling linters better.
chavacava: IMO the default max must be `-1` meaning the rule is not activated. That way we will not pollute the results of those who activate all rules.
alexandear: > IMO the default max must be `-1` meaning the rule is not activated. That way we will not pollute the results of those who activate all rules.

@mgechev Changed.

But I choose `0` in `max` to disable the rule. This is for consistency with the `function-length` rule.

Patch:
diff --git a/README.md b/README.md
index 6c66f17..254bd4b 100644
--- a/README.md
+++ b/README.md
@@ -547,6 +547,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
 
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index 273c127..3ef2acd 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -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)
@@ -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).
diff --git a/config/config.go b/config/config.go
index fed1d19..5da64b6 100644
--- a/config/config.go
+++ b/config/config.go
@@ -96,6 +96,7 @@ var allRules = append([]lint.Rule{
 	&rule.EnforceSliceStyleRule{},
 	&rule.MaxControlNestingRule{},
 	&rule.CommentsDensityRule{},
+	&rule.FileLengthLimitRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{
diff --git a/revivelib/core.go b/revivelib/core.go
index 4e076e9..31463fe 100644
--- a/revivelib/core.go
+++ b/revivelib/core.go
@@ -82,7 +82,7 @@ func (r *Revive) Lint(patterns ...*LintPattern) (<-chan lint.Failure, error) {
 	if len(excludePatterns) == 0 { // if no excludes were set
 		excludePatterns = r.config.Exclude // use those from the configuration
 	}
-	
+
 	// by default if no excludes exclude vendor
 	if len(excludePatterns) == 0 {
 		excludePatterns = []string{"vendor/..."}
@@ -95,7 +95,6 @@ func (r *Revive) Lint(patterns ...*LintPattern) (<-chan lint.Failure, error) {
 
 	revive := lint.New(func(file string) ([]byte, error) {
 		contents, err := os.ReadFile(file)
-
 		if err != nil {
 			return nil, fmt.Errorf("reading file %v: %w", file, err)
 		}
diff --git a/rule/enforce-map-style.go b/rule/enforce-map-style.go
index 1c4717c..c698c40 100644
--- a/rule/enforce-map-style.go
+++ b/rule/enforce-map-style.go
@@ -65,7 +65,6 @@ func (r *EnforceMapStyleRule) configure(arguments lint.Arguments) {
 
 	var err error
 	r.enforceMapStyle, err = mapStyleFromString(enforceMapStyle)
-
 	if err != nil {
 		panic(fmt.Sprintf("Invalid argument to the enforce-map-style rule: %v", err))
 	}
diff --git a/rule/enforce-slice-style.go b/rule/enforce-slice-style.go
index df72c8c..14be258 100644
--- a/rule/enforce-slice-style.go
+++ b/rule/enforce-slice-style.go
@@ -69,7 +69,6 @@ func (r *EnforceSliceStyleRule) configure(arguments lint.Arguments) {
 
 	var err error
 	r.enforceSliceStyle, err = sliceStyleFromString(enforceSliceStyle)
-
 	if err != nil {
 		panic(fmt.Sprintf("Invalid argument to the enforce-slice-style rule: %v", err))
 	}
diff --git a/rule/file-length-limit.go b/rule/file-length-limit.go
new file mode 100644
index 0000000..c5a5641
--- /dev/null
+++ b/rule/file-length-limit.go
@@ -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()
+
+	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] {
+			case '/': // single-line comment
+				count++
+			case '*': // multi-line comment
+				count += strings.Count(comment.Text, "\n") + 1
+			}
+		}
+	}
+	return count
+}
diff --git a/rule/line-length-limit.go b/rule/line-length-limit.go
index 1e91646..a154b7a 100644
--- a/rule/line-length-limit.go
+++ b/rule/line-length-limit.go
@@ -38,7 +38,6 @@ func (r *LineLengthLimitRule) configure(arguments lint.Arguments) {
 	}
 
 	r.max = int(maxLength)
-
 }
 
 // Apply applies the rule to given file.
diff --git a/rule/max-public-structs.go b/rule/max-public-structs.go
index ea93402..70840e7 100644
--- a/rule/max-public-structs.go
+++ b/rule/max-public-structs.go
@@ -35,7 +35,6 @@ func (r *MaxPublicStructsRule) configure(arguments lint.Arguments) {
 		panic(`invalid value passed as argument number to the "max-public-structs" rule`)
 	}
 	r.max = maxStructs
-
 }
 
 // Apply applies the rule to given file.
diff --git a/rule/redefines-builtin-id.go b/rule/redefines-builtin-id.go
index 8aaaf03..10ea16a 100644
--- a/rule/redefines-builtin-id.go
+++ b/rule/redefines-builtin-id.go
@@ -163,7 +163,7 @@ func (w *lintRedefinesBuiltinID) Visit(node ast.Node) ast.Visitor {
 				if !isTypeOrName {
 					continue
 				}
-				
+
 				id := obj.Name
 				if ok, bt := w.isBuiltIn(id); ok {
 					w.addFailure(name, fmt.Sprintf("redefinition of the built-in %s %s", bt, id))
diff --git a/rule/var-naming.go b/rule/var-naming.go
index e93159b..9d9c739 100644
--- a/rule/var-naming.go
+++ b/rule/var-naming.go
@@ -79,7 +79,6 @@ func (r *VarNamingRule) applyPackageCheckRules(walker *lintNames) {
 			Category:   "naming",
 		})
 	}
-
 }
 
 // Apply applies the rule to given file.


Test patch:
diff --git a/cli/main_test.go b/cli/main_test.go
index b2fabaf..be1f97d 100644
--- a/cli/main_test.go
+++ b/cli/main_test.go
@@ -40,7 +40,6 @@ func TestXDGConfigDirIsPrefferedFirst(t *testing.T) {
 }
 
 func TestHomeConfigDir(t *testing.T) {
-
 	homeDirPath := "/tmp-iofs/home/tester"
 	AppFs.MkdirAll(homeDirPath, 0755)
 
diff --git a/lint/filefilter_test.go b/lint/filefilter_test.go
index 6c20afd..11da313 100644
--- a/lint/filefilter_test.go
+++ b/lint/filefilter_test.go
@@ -95,7 +95,6 @@ func TestFileFilter(t *testing.T) {
 				t.Fatalf("should not match %s", fn)
 			}
 		}
-
 	})
 
 	t.Run("just *", func(t *testing.T) {
@@ -109,7 +108,6 @@ func TestFileFilter(t *testing.T) {
 				t.Fatalf("should match %s", fn)
 			}
 		}
-
 	})
 
 	t.Run("just ~", func(t *testing.T) {
@@ -123,6 +121,5 @@ func TestFileFilter(t *testing.T) {
 				t.Fatalf("should match %s", fn)
 			}
 		}
-
 	})
 }
diff --git a/test/file-length-limit_test.go b/test/file-length-limit_test.go
new file mode 100644
index 0000000..c000daa
--- /dev/null
+++ b/test/file-length-limit_test.go
@@ -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}},
+	})
+}
diff --git a/test/json-data-format_test.go b/test/json-data-format_test.go
index 3914b31..41b154c 100644
--- a/test/json-data-format_test.go
+++ b/test/json-data-format_test.go
@@ -9,9 +9,8 @@ import (
 
 func TestJsonDataFormat(t *testing.T) {
 	testRule(t, "json-data-format-atomic", &rule.AtomicRule{})
-
 }
+
 func TestJsonDataFormatVarNaming(t *testing.T) {
 	testRule(t, "json-data-format-var-naming", &rule.VarNamingRule{}, &lint.RuleConfig{})
-
 }
diff --git a/test/utils_test.go b/test/utils_test.go
index ad36869..56af842 100644
--- a/test/utils_test.go
+++ b/test/utils_test.go
@@ -107,11 +107,9 @@ func assertFailures(t *testing.T, baseDir string, fi os.FileInfo, src []byte, ru
 				}
 
 				if in.Confidence > 0 {
-
 					if in.Confidence != p.Confidence {
 						t.Errorf("Lint failed at %s:%d; got confidence %f, want %f", fi.Name(), in.Line, p.Confidence, in.Confidence)
 					}
-
 				}
 
 				// remove this problem from ps
@@ -224,11 +222,9 @@ func extractInstructionFromJSON(line string, lineNumber int) (instruction, error
 		Line:       lineNumber,
 	}
 	return ins, nil
-
 }
 
 func extractDataMode(line string) string {
-
 	if strings.HasPrefix(line, "json") {
 		return "json"
 	}
@@ -237,7 +233,6 @@ func extractDataMode(line string) string {
 	}
 
 	return ""
-
 }
 
 func extractPattern(line string) (string, error) {
diff --git a/test/var-naming_test.go b/test/var-naming_test.go
index f554286..9347385 100644
--- a/test/var-naming_test.go
+++ b/test/var-naming_test.go
@@ -23,5 +23,4 @@ func TestVarNaming(t *testing.T) {
 	testRule(t, "var-naming_skipPackageNameChecks-true", &rule.VarNamingRule{}, &lint.RuleConfig{
 		Arguments: []any{[]any{}, []any{}, []any{map[string]any{"skipPackageNameChecks": true}}},
 	})
-
 }
diff --git a/testdata/file-length-limit-4-skip-comments-skip-blank.go b/testdata/file-length-limit-4-skip-comments-skip-blank.go
new file mode 100644
index 0000000..ce086d0
--- /dev/null
+++ b/testdata/file-length-limit-4-skip-comments-skip-blank.go
@@ -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/
diff --git a/testdata/file-length-limit-6-skip-blank.go b/testdata/file-length-limit-6-skip-blank.go
new file mode 100644
index 0000000..9e9641c
--- /dev/null
+++ b/testdata/file-length-limit-6-skip-blank.go
@@ -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/
diff --git a/testdata/file-length-limit-7-skip-comments.go b/testdata/file-length-limit-7-skip-comments.go
new file mode 100644
index 0000000..ef67590
--- /dev/null
+++ b/testdata/file-length-limit-7-skip-comments.go
@@ -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/
diff --git a/testdata/file-length-limit-9.go b/testdata/file-length-limit-9.go
new file mode 100644
index 0000000..649a201
--- /dev/null
+++ b/testdata/file-length-limit-9.go
@@ -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/
diff --git a/testdata/file-length-limit-disabled.go b/testdata/file-length-limit-disabled.go
new file mode 100644
index 0000000..1066985
--- /dev/null
+++ b/testdata/file-length-limit-disabled.go
@@ -0,0 +1,8 @@
+package fixtures
+
+import "fmt"
+
+// Foo is a function.
+func Foo(a, b int) {
+	fmt.Println("Hello, world!")
+}