feature: new rule enforce-switch-default (#1390)

README.md

@@ -495,6 +495,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`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-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   |
 | [`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-switch-style`](./RULES_DESCRIPTIONS.md#enforce-switch-style) |  []string (defaults to enforce occurrence and position) |  Enforces consistent usage of `default` on `switch` statements. |    no    |  no   |
 | [`error-naming`](./RULES_DESCRIPTIONS.md#error-naming)        |  n/a   | Naming of error variables.                                       |   yes    |  no   |
 | [`error-return`](./RULES_DESCRIPTIONS.md#error-return)        |  n/a   | The error return parameter should be last.                       |   yes    |  no   |
 | [`error-strings`](./RULES_DESCRIPTIONS.md#error-strings)       |  []string   | Conventions around error strings.                                |   yes    |  no   |

RULES_DESCRIPTIONS.md

@@ -31,6 +31,7 @@ List of all available rules.
   - [enforce-map-style](#enforce-map-style)
   - [enforce-repeated-arg-type-style](#enforce-repeated-arg-type-style)
   - [enforce-slice-style](#enforce-slice-style)
+  - [enforce-switch-style](#enforce-switch-style)
   - [error-naming](#error-naming)
   - [error-return](#error-return)
   - [error-strings](#error-strings)
@@ -520,6 +521,40 @@ Example:
 arguments = ["make"]
 ```
 
+## enforce-switch-style
+
+_Description_: This rule enforces consistent usage of `default` on `switch` statements.
+It can check for `default` case clause occurrence and/or position in the list of case clauses.
+
+_Configuration_: ([]string) Specifies what to enforced: occurrence and/or position. The, non-mutually exclusive, options are:
+
+- "allowNoDefault": allows `switch` without `default` case clause.
+- "allowDefaultNotLast": allows `default` case clause to be not the last clause of the `switch`.
+
+Examples:
+
+To enforce that all `switch` statements have a `default` clause as its the last case clause:
+
+```toml
+[rule.enforce-switch-style]
+```
+
+To enforce that all `switch` statements have a `default` clause but its position is unimportant:
+
+```toml
+[rule.enforce-switch-style]
+arguments = ["allowDefaultNotLast"]
+```
+
+To enforce that in all `switch` statements with a `default` clause, the `default` is the last case clause:
+
+```toml
+[rule.enforce-switch-style]
+arguments = ["allowNoDefault"]
+```
+
+Notice that a configuration including both options will effectively deactivate the whole rule.
+
 ## error-naming
 
 _Description_: By convention, for the sake of readability, variables of type `error` must be named with the prefix `err`.

config/config.go

@@ -104,6 +104,7 @@ var allRules = append([]lint.Rule{
 	&rule.RedundantTestMainExitRule{},
 	&rule.UnnecessaryFormatRule{},
 	&rule.UseFmtPrintRule{},
+	&rule.EnforceSwitchStyleRule{},
 }, defaultRules...)
 
 // allFormatters is a list of all available formatters to output the linting results.

rule/enforce_switch_style.go

@@ -0,0 +1,106 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// EnforceSwitchStyleRule implements a rule to enforce default clauses use and/or position.
+type EnforceSwitchStyleRule struct {
+	allowNoDefault      bool // allow absence of default
+	allowDefaultNotLast bool // allow default, if present, not being the last case
+}
+
+// Configure validates the rule configuration, and configures the rule accordingly.
+//
+// Configuration implements the [lint.ConfigurableRule] interface.
+func (r *EnforceSwitchStyleRule) Configure(arguments lint.Arguments) error {
+	if len(arguments) < 1 {
+		return nil
+	}
+
+	for _, arg := range arguments {
+		argStr, ok := arg.(string)
+		if !ok {
+			return fmt.Errorf("invalid argument for rule %s; expected string but got %T", r.Name(), arg)
+		}
+		switch {
+		case isRuleOption(argStr, "allowNoDefault"):
+			r.allowNoDefault = true
+		case isRuleOption(argStr, "allowDefaultNotLast"):
+			r.allowDefaultNotLast = true
+		default:
+			return fmt.Errorf(`invalid argument %q for rule %s; expected "allowNoDefault" or "allowDefaultNotLast"`, argStr, r.Name())
+		}
+	}
+
+	return nil
+}
+
+// Apply applies the rule to given file.
+func (r *EnforceSwitchStyleRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+	astFile := file.AST
+	ast.Inspect(astFile, func(n ast.Node) bool {
+		switchNode, ok := n.(*ast.SwitchStmt)
+		if !ok {
+			return true // not a switch statement
+		}
+
+		defaultClause, isLast := r.seekDefaultCase(switchNode.Body)
+		hasDefault := defaultClause != nil
+
+		if !hasDefault && r.allowNoDefault {
+			return true // switch without default but the rule is configured to don´t care
+		}
+
+		if !hasDefault && !r.allowNoDefault {
+			// switch without default
+			failures = append(failures, lint.Failure{
+				Confidence: 1,
+				Node:       switchNode,
+				Category:   lint.FailureCategoryStyle,
+				Failure:    "switch must have a default case clause",
+			})
+
+			return true
+		}
+
+		// the switch has a default
+
+		if r.allowDefaultNotLast || isLast {
+			return true
+		}
+
+		failures = append(failures, lint.Failure{
+			Confidence: 1,
+			Node:       defaultClause,
+			Category:   lint.FailureCategoryStyle,
+			Failure:    "default case clause must be the last one",
+		})
+
+		return true
+	})
+
+	return failures
+}
+
+func (*EnforceSwitchStyleRule) seekDefaultCase(body *ast.BlockStmt) (defaultClause *ast.CaseClause, isLast bool) {
+	var last *ast.CaseClause
+	for _, stmt := range body.List {
+		cc, _ := stmt.(*ast.CaseClause) // no need to check for ok
+		last = cc
+		if cc.List == nil { // a nil List means "default"
+			defaultClause = cc
+		}
+	}
+
+	return defaultClause, defaultClause == last
+}
+
+// Name returns the rule name.
+func (*EnforceSwitchStyleRule) Name() string {
+	return "enforce-switch-style"
+}

rule/enforce_switch_style_test.go

@@ -0,0 +1,92 @@
+package rule
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/mgechev/revive/lint"
+)
+
+func TestEnforceSwitchStyleRule_Configure(t *testing.T) {
+	tests := []struct {
+		name                    string
+		arguments               lint.Arguments
+		wantErr                 error
+		wantAllowNoDefault      bool
+		wantAllowDefaultNotLast bool
+	}{
+		{
+			name:      "no arguments",
+			arguments: lint.Arguments{},
+			wantErr:   nil,
+		},
+		{
+			name: "valid argument: allowNoDefault",
+			arguments: lint.Arguments{
+				"allowNoDefault",
+			},
+			wantErr:            nil,
+			wantAllowNoDefault: true,
+		},
+		{
+			name: "valid argument: allowDefaultNotLast",
+			arguments: lint.Arguments{
+				"allowDefaultNotLast",
+			},
+			wantErr:                 nil,
+			wantAllowDefaultNotLast: true,
+		},
+		{
+			name: "valid lowercased arguments",
+			arguments: lint.Arguments{
+				"allowdefaultnotlast",
+				"allownodefault",
+			},
+			wantErr:                 nil,
+			wantAllowNoDefault:      true,
+			wantAllowDefaultNotLast: true,
+		},
+		{
+			name: "unknown argument: unknown",
+			arguments: lint.Arguments{
+				"unknown",
+			},
+			wantErr: errors.New(`invalid argument "unknown" for rule enforce-switch-style; expected "allowNoDefault" or "allowDefaultNotLast"`),
+		},
+		{
+			name: "unexpected type argument: 10",
+			arguments: lint.Arguments{
+				10,
+			},
+			wantErr: errors.New(`invalid argument for rule enforce-switch-style; expected string but got int`),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var rule EnforceSwitchStyleRule
+
+			err := rule.Configure(tt.arguments)
+
+			if tt.wantErr != nil {
+				if err == nil {
+					t.Errorf("unexpected error: got = nil, want = %v", tt.wantErr)
+					return
+				}
+				if err.Error() != tt.wantErr.Error() {
+					t.Errorf("unexpected error: got = %v, want = %v", err, tt.wantErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Errorf("unexpected error: got = %v, want = nil", err)
+			}
+			if tt.wantAllowNoDefault != rule.allowNoDefault {
+				t.Errorf("unexpected allowNoDefault: want = %v, got %v", tt.wantAllowNoDefault, rule.allowNoDefault)
+			}
+			if tt.wantAllowDefaultNotLast != rule.allowDefaultNotLast {
+				t.Errorf("unexpected funcRetValStyle: want = %v, got %v", tt.wantAllowDefaultNotLast, rule.allowDefaultNotLast)
+			}
+		})
+	}
+}

test/enforce_switch_style_test.go

@@ -0,0 +1,21 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/rule"
+)
+
+func TestEnforceSwitchStyle(t *testing.T) {
+	testRule(t, "enforce_switch_style", &rule.EnforceSwitchStyleRule{})
+	testRule(t, "enforce_switch_style_allow_no_default", &rule.EnforceSwitchStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{"allowNoDefault"},
+	})
+	testRule(t, "enforce_switch_style_allow_not_last", &rule.EnforceSwitchStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{"allowDefaultNotLast"},
+	})
+	testRule(t, "enforce_switch_style_allow_no_default_allow_not_last", &rule.EnforceSwitchStyleRule{}, &lint.RuleConfig{
+		Arguments: []any{"allowNoDefault", "allowDefaultNotLast"},
+	})
+}

testdata/enforce_switch_style.go

@@ -0,0 +1,18 @@
+package fixtures
+
+func enforceSwitchStyle3() {
+
+	switch expression {
+	case condition:
+	default:
+	}
+
+	switch expression {
+	default: // MATCH /default case clause must be the last one/
+	case condition:
+	}
+
+	switch expression { // MATCH /switch must have a default case clause/
+	case condition:
+	}
+}

testdata/enforce_switch_style_allow_no_default.go

@@ -0,0 +1,18 @@
+package fixtures
+
+func enforceSwitchStyle() {
+
+	switch expression {
+	case condition:
+	default:
+	}
+
+	switch expression {
+	default: // MATCH /default case clause must be the last one/
+	case condition:
+	}
+
+	switch expression {
+	case condition:
+	}
+}

testdata/enforce_switch_style_allow_no_default_allow_not_last.go

@@ -0,0 +1,18 @@
+package fixtures
+
+func enforceSwitchStyle3() {
+
+	switch expression {
+	case condition:
+	default:
+	}
+
+	switch expression {
+	default:
+	case condition:
+	}
+
+	switch expression {
+	case condition:
+	}
+}

testdata/enforce_switch_style_allow_not_last.go

@@ -0,0 +1,18 @@
+package fixtures
+
+func enforceSwitchStyle2() {
+
+	switch expression {
+	case condition:
+	default:
+	}
+
+	switch expression {
+	default:
+	case condition:
+	}
+
+	switch expression { // MATCH /switch must have a default case clause/
+	case condition:
+	}
+}