feat: add support for enforce-repeated-arg-type-style rule (#953)

mgechev · Dec 27, 2023 · 8d5724f · 8d5724f
1 parent 90b2112
commit 8d5724f
Show file tree

Hide file tree

Showing 10 changed files with 449 additions and 1 deletion.
diff --git a/README.md b/README.md
@@ -537,6 +537,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`import-alias-naming`](./RULES_DESCRIPTIONS.md#import-alias-naming)         | string or map[string]string (defaults to allow regex pattern ^[a-z][a-z0-9]{0,}$) | Conventions around the naming of import aliases.                              |    no    |  no   |
 | [`enforce-map-style`](./RULES_DESCRIPTIONS.md#enforce-map-style) |  string (defaults to "any")  |  Enforces consistent usage of `make(map[type]type)` or `map[type]type{}` for map initialization. Does not affect `make(map[type]type, size)` constructions. |    no    |  no   |
 | [`enforce-slice-style`](./RULES_DESCRIPTIONS.md#enforce-slice-style) |  string (defaults to "any")  |  Enforces consistent usage of `make([]type, 0)` or `[]type{}` for slice initialization. Does not affect `make(map[type]type, non_zero_len, or_non_zero_cap)` constructions. |    no    |  no   |
+| [`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   |
 
 
 ## Configurable rules

diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
@@ -386,6 +386,44 @@ Example:
   arguments = ["make"]
 ```
 
+
+## enforce-repeated-arg-type-style
+
+**Description**: This rule is designed to maintain consistency in the declaration
+of repeated argument and return value types in Go functions. It supports three styles:
+'any', 'short', and 'full'. The 'any' style is lenient and allows any form of type
+declaration. The 'short' style encourages omitting repeated types for conciseness,
+whereas the 'full' style mandates explicitly stating the type for each argument
+and return value, even if they are repeated, promoting clarity.
+
+**Configuration (1)**: (string) as a single string, it configures both argument
+and return value styles. Accepts 'any', 'short', or 'full' (default: 'any').
+
+**Configuration (2)**: (map[string]any) as a map, allows separate configuration
+for function arguments and return values. Valid keys are "funcArgStyle" and
+"funcRetValStyle", each accepting 'any', 'short', or 'full'. If a key is not
+specified, the default value of 'any' is used.
+
+**Note**: The rule applies checks based on the specified styles. For 'full' style,
+it flags instances where types are omitted in repeated arguments or return values.
+For 'short' style, it highlights opportunities to omit repeated types for brevity.
+Incorrect or unknown configuration values will result in an error.
+
+**Example (1)**:
+
+```toml
+[rule.enforce-repeated-arg-type-style]
+arguments = ["short"]
+```
+
+**Example (2):**
+
+```toml
+[rule.enforce-repeated-arg-type-style]
+arguments = [ { funcArgStyle = "full", funcRetValStyle = "short" } ]
+```
+
+
 ## enforce-slice-style
 
 _Description_: This rule enforces consistent usage of `make([]type, 0)` or `[]type{}` for slice initialization.
@@ -499,7 +537,8 @@ _Description_: Aligns with Go's naming conventions, as outlined in the official
 the principles of good package naming. Users can follow these guidelines by default or define a custom regex rule.
 Importantly, aliases with underscores ("_") are always allowed.
 
-_Configuration_ (1): (string) as plain string accepts allow regexp pattern for aliases (default: ^[a-z][a-z0-9]{0,}$),
+_Configuration_ (1): (string) as plain string accepts allow regexp pattern for aliases (default: ^[a-z][a-z0-9]{0,}$).
+
 _Configuration_ (2): (map[string]string) as a map accepts two values:
 * for a key "allowRegex" accepts allow regexp pattern
 * for a key "denyRegex deny regexp pattern

diff --git a/rule/enforce-repeated-arg-type-style.go b/rule/enforce-repeated-arg-type-style.go
@@ -0,0 +1,191 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"go/types"
+	"sync"
+
+	"github.com/mgechev/revive/lint"
+)
+
+type enforceRepeatedArgTypeStyleType string
+
+const (
+	enforceRepeatedArgTypeStyleTypeAny   enforceRepeatedArgTypeStyleType = "any"
+	enforceRepeatedArgTypeStyleTypeShort enforceRepeatedArgTypeStyleType = "short"
+	enforceRepeatedArgTypeStyleTypeFull  enforceRepeatedArgTypeStyleType = "full"
+)
+
+func repeatedArgTypeStyleFromString(s string) enforceRepeatedArgTypeStyleType {
+	switch s {
+	case string(enforceRepeatedArgTypeStyleTypeAny), "":
+		return enforceRepeatedArgTypeStyleTypeAny
+	case string(enforceRepeatedArgTypeStyleTypeShort):
+		return enforceRepeatedArgTypeStyleTypeShort
+	case string(enforceRepeatedArgTypeStyleTypeFull):
+		return enforceRepeatedArgTypeStyleTypeFull
+	default:
+		err := fmt.Errorf(
+			"invalid repeated arg type style: %s (expecting one of %v)",
+			s,
+			[]enforceRepeatedArgTypeStyleType{
+				enforceRepeatedArgTypeStyleTypeAny,
+				enforceRepeatedArgTypeStyleTypeShort,
+				enforceRepeatedArgTypeStyleTypeFull,
+			},
+		)
+
+		panic(fmt.Sprintf("Invalid argument to the enforce-repeated-arg-type-style rule: %v", err))
+	}
+}
+
+// EnforceRepeatedArgTypeStyleRule implements a rule to enforce repeated argument type style.
+type EnforceRepeatedArgTypeStyleRule struct {
+	configured      bool
+	funcArgStyle    enforceRepeatedArgTypeStyleType
+	funcRetValStyle enforceRepeatedArgTypeStyleType
+
+	sync.Mutex
+}
+
+func (r *EnforceRepeatedArgTypeStyleRule) configure(arguments lint.Arguments) {
+	r.Lock()
+	defer r.Unlock()
+
+	if r.configured {
+		return
+	}
+	r.configured = true
+
+	r.funcArgStyle = enforceRepeatedArgTypeStyleTypeAny
+	r.funcRetValStyle = enforceRepeatedArgTypeStyleTypeAny
+
+	if len(arguments) == 0 {
+		return
+	}
+
+	switch funcArgStyle := arguments[0].(type) {
+	case string:
+		r.funcArgStyle = repeatedArgTypeStyleFromString(funcArgStyle)
+		r.funcRetValStyle = repeatedArgTypeStyleFromString(funcArgStyle)
+	case map[string]any: // expecting map[string]string
+		for k, v := range funcArgStyle {
+			switch k {
+			case "funcArgStyle":
+				val, ok := v.(string)
+				if !ok {
+					panic(fmt.Sprintf("Invalid map value type for 'enforce-repeated-arg-type-style' rule. Expecting string, got %T", v))
+				}
+				r.funcArgStyle = repeatedArgTypeStyleFromString(val)
+			case "funcRetValStyle":
+				val, ok := v.(string)
+				if !ok {
+					panic(fmt.Sprintf("Invalid map value '%v' for 'enforce-repeated-arg-type-style' rule. Expecting string, got %T", v, v))
+				}
+				r.funcRetValStyle = repeatedArgTypeStyleFromString(val)
+			default:
+				panic(fmt.Sprintf("Invalid map key for 'enforce-repeated-arg-type-style' rule. Expecting 'funcArgStyle' or 'funcRetValStyle', got %v", k))
+			}
+		}
+	default:
+		panic(fmt.Sprintf("Invalid argument '%v' for 'import-alias-naming' rule. Expecting string or map[string]string, got %T", arguments[0], arguments[0]))
+	}
+}
+
+// Apply applies the rule to a given file.
+func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	r.configure(arguments)
+
+	if r.funcArgStyle == enforceRepeatedArgTypeStyleTypeAny && r.funcRetValStyle == enforceRepeatedArgTypeStyleTypeAny {
+		// This linter is not configured, return no failures.
+		return nil
+	}
+
+	var failures []lint.Failure
+
+	err := file.Pkg.TypeCheck()
+	if err != nil {
+		// the file has other issues
+		return nil
+	}
+	typesInfo := file.Pkg.TypesInfo()
+
+	astFile := file.AST
+	ast.Inspect(astFile, func(n ast.Node) bool {
+		switch fn := n.(type) {
+		case *ast.FuncDecl:
+			if r.funcArgStyle == enforceRepeatedArgTypeStyleTypeFull {
+				if fn.Type.Params != nil {
+					for _, field := range fn.Type.Params.List {
+						if len(field.Names) > 1 {
+							failures = append(failures, lint.Failure{
+								Confidence: 1,
+								Node:       field,
+								Category:   "style",
+								Failure:    "argument types should not be omitted",
+							})
+						}
+					}
+				}
+			}
+
+			if r.funcArgStyle == enforceRepeatedArgTypeStyleTypeShort {
+				var prevType ast.Expr
+				if fn.Type.Params != nil {
+					for _, field := range fn.Type.Params.List {
+						if types.Identical(typesInfo.Types[field.Type].Type, typesInfo.Types[prevType].Type) {
+							failures = append(failures, lint.Failure{
+								Confidence: 1,
+								Node:       field,
+								Category:   "style",
+								Failure:    "repeated argument type can be omitted",
+							})
+						}
+						prevType = field.Type
+					}
+				}
+			}
+
+			if r.funcRetValStyle == enforceRepeatedArgTypeStyleTypeFull {
+				if fn.Type.Results != nil {
+					for _, field := range fn.Type.Results.List {
+						if len(field.Names) > 1 {
+							failures = append(failures, lint.Failure{
+								Confidence: 1,
+								Node:       field,
+								Category:   "style",
+								Failure:    "return types should not be omitted",
+							})
+						}
+					}
+				}
+			}
+
+			if r.funcRetValStyle == enforceRepeatedArgTypeStyleTypeShort {
+				var prevType ast.Expr
+				if fn.Type.Results != nil {
+					for _, field := range fn.Type.Results.List {
+						if field.Names != nil && types.Identical(typesInfo.Types[field.Type].Type, typesInfo.Types[prevType].Type) {
+							failures = append(failures, lint.Failure{
+								Confidence: 1,
+								Node:       field,
+								Category:   "style",
+								Failure:    "repeated return type can be omitted",
+							})
+						}
+						prevType = field.Type
+					}
+				}
+			}
+		}
+		return true
+	})
+
+	return failures
+}
+
+// Name returns the name of the linter rule.
+func (*EnforceRepeatedArgTypeStyleRule) Name() string {
+	return "enforce-repeated-arg-type-style"
+}
diff --git a/test/enforce-repeated-arg-type-style_test.go b/test/enforce-repeated-arg-type-style_test.go
@@ -0,0 +1,123 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/rule"
+)
+
+func TestEnforceRepeatedArgTypeStyleShort(t *testing.T) {
+	testRule(t, "enforce-repeated-arg-type-style-short-args", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{"short"},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-short-return", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{"short"},
+	})
+
+	testRule(t, "enforce-repeated-arg-type-style-short-args", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle": `short`,
+			},
+		},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-short-return", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcRetValStyle": `short`,
+			},
+		},
+	})
+}
+
+func TestEnforceRepeatedArgTypeStyleFull(t *testing.T) {
+	testRule(t, "enforce-repeated-arg-type-style-full-args", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{"full"},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-full-return", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{"full"},
+	})
+
+	testRule(t, "enforce-repeated-arg-type-style-full-args", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle": `full`,
+			},
+		},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-full-return", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcRetValStyle": `full`,
+			},
+		},
+	})
+}
+
+func TestEnforceRepeatedArgTypeStyleMixed(t *testing.T) {
+	testRule(t, "enforce-repeated-arg-type-style-full-args", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle": `full`,
+			},
+		},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-full-args", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle":    `full`,
+				"funcRetValStyle": `any`,
+			},
+		},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-full-args", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle":    `full`,
+				"funcRetValStyle": `short`,
+			},
+		},
+	})
+
+	testRule(t, "enforce-repeated-arg-type-style-full-return", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcRetValStyle": `full`,
+			},
+		},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-full-return", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle":    `any`,
+				"funcRetValStyle": `full`,
+			},
+		},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-full-return", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle":    `short`,
+				"funcRetValStyle": `full`,
+			},
+		},
+	})
+
+	testRule(t, "enforce-repeated-arg-type-style-mixed-full-short", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle":    `full`,
+				"funcRetValStyle": `short`,
+			},
+		},
+	})
+	testRule(t, "enforce-repeated-arg-type-style-mixed-short-full", &rule.EnforceRepeatedArgTypeStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{
+			map[string]any{
+				"funcArgStyle":    `short`,
+				"funcRetValStyle": `full`,
+			},
+		},
+	})
+}