Add new -default-case-required flag

comment.go

@@ -7,8 +7,10 @@ import (
 )
 
 const (
-	ignoreComment  = "//exhaustive:ignore"
-	enforceComment = "//exhaustive:enforce"
+	ignoreComment                     = "//exhaustive:ignore"
+	enforceComment                    = "//exhaustive:enforce"
+	ignoreDefaultCaseRequiredComment  = "//exhaustive:ignore-default-case-required"
+	enforceDefaultCaseRequiredComment = "//exhaustive:enforce-default-case-required"
 )
 
 func hasCommentPrefix(comments []*ast.CommentGroup, comment string) bool {

exhaustive.go

@@ -19,6 +19,7 @@ func registerFlags() {
 	Analyzer.Flags.BoolVar(&fExplicitExhaustiveMap, ExplicitExhaustiveMapFlag, false, `check map literal only if associated with "//exhaustive:enforce" comment`)
 	Analyzer.Flags.BoolVar(&fCheckGenerated, CheckGeneratedFlag, false, "check generated files")
 	Analyzer.Flags.BoolVar(&fDefaultSignifiesExhaustive, DefaultSignifiesExhaustiveFlag, false, "switch statement is unconditionally exhaustive if it has a default case")
+	Analyzer.Flags.BoolVar(&fDefaultCaseRequired, DefaultCaseRequiredFlag, false, "switch statement requires default case even if exhaustive")
 	Analyzer.Flags.Var(&fIgnoreEnumMembers, IgnoreEnumMembersFlag, "ignore constants matching `regexp`")
 	Analyzer.Flags.Var(&fIgnoreEnumTypes, IgnoreEnumTypesFlag, "ignore types matching `regexp`")
 	Analyzer.Flags.BoolVar(&fPackageScopeOnly, PackageScopeOnlyFlag, false, "only discover enums declared in file-level blocks")
@@ -36,6 +37,7 @@ const (
 	ExplicitExhaustiveMapFlag      = "explicit-exhaustive-map"
 	CheckGeneratedFlag             = "check-generated"
 	DefaultSignifiesExhaustiveFlag = "default-signifies-exhaustive"
+	DefaultCaseRequiredFlag        = "default-case-required"
 	IgnoreEnumMembersFlag          = "ignore-enum-members"
 	IgnoreEnumTypesFlag            = "ignore-enum-types"
 	PackageScopeOnlyFlag           = "package-scope-only"
@@ -52,6 +54,7 @@ var (
 	fExplicitExhaustiveMap      bool
 	fCheckGenerated             bool
 	fDefaultSignifiesExhaustive bool
+	fDefaultCaseRequired        bool
 	fIgnoreEnumMembers          regexpFlag
 	fIgnoreEnumTypes            regexpFlag
 	fPackageScopeOnly           bool
@@ -65,6 +68,7 @@ func resetFlags() {
 	fExplicitExhaustiveMap = false
 	fCheckGenerated = false
 	fDefaultSignifiesExhaustive = false
+	fDefaultCaseRequired = false
 	fIgnoreEnumMembers = regexpFlag{}
 	fIgnoreEnumTypes = regexpFlag{}
 	fPackageScopeOnly = false
@@ -121,6 +125,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			conf := switchConfig{
 				explicit:                   fExplicitExhaustiveSwitch,
 				defaultSignifiesExhaustive: fDefaultSignifiesExhaustive,
+				defaultCaseRequired:        fDefaultCaseRequired,
 				checkGenerated:             fCheckGenerated,
 				ignoreConstant:             fIgnoreEnumMembers.re,
 				ignoreType:                 fIgnoreEnumTypes.re,

exhaustive_test.go

@@ -46,6 +46,10 @@ func TestExhaustive(t *testing.T) {
 	runTest(t, "default-signifies-exhaustive/default-absent/...", func() { fDefaultSignifiesExhaustive = true })
 	runTest(t, "default-signifies-exhaustive/default-present/...", func() { fDefaultSignifiesExhaustive = true })
 
+	// These tests exercise the default-case-required flag and its escape comment
+	runTest(t, "default-case-required/default-required/...", func() { fDefaultCaseRequired = true })
+	runTest(t, "default-case-required/default-not-required/...", func() { fDefaultCaseRequired = false })
+
 	// Tests for -ignore-enum-members and -ignore-enum-types flags.
 	runTest(t, "ignore-pattern/...", func() {
 		fIgnoreEnumMembers = regexpFlag{

switch.go

@@ -5,6 +5,7 @@ import (
 	"go/ast"
 	"go/types"
 	"regexp"
+	"strings"
 
 	"golang.org/x/tools/go/analysis"
 )
@@ -44,6 +45,7 @@ const (
 	resultNoEnforceComment     = "has no enforce comment"
 	resultEnumMembersAccounted = "required enum members accounted for"
 	resultDefaultCaseSuffices  = "default case satisfies exhaustiveness"
+	resultMissingDefaultCase   = "missing required default case"
 	resultReportedDiagnostic   = "reported diagnostic"
 	resultEnumTypes            = "invalid or empty composing enum types"
 )
@@ -52,11 +54,47 @@ const (
 type switchConfig struct {
 	explicit                   bool
 	defaultSignifiesExhaustive bool
+	defaultCaseRequired        bool
 	checkGenerated             bool
 	ignoreConstant             *regexp.Regexp // can be nil
 	ignoreType                 *regexp.Regexp // can be nil
 }
 
+// There are few possibilities, and often none, so we use a possibly-nil slice
+func userDirectives(comments []*ast.CommentGroup) []string {
+	var directives []string
+	for _, c := range comments {
+		for _, cc := range c.List {
+			// The order matters here: we always want to check the longest first.
+			for _, d := range []string{
+				enforceDefaultCaseRequiredComment,
+				ignoreDefaultCaseRequiredComment,
+				enforceComment,
+				ignoreComment,
+			} {
+				if strings.HasPrefix(cc.Text, d) {
+					directives = append(directives, d)
+					// The break here is important: once we associate a comment
+					// with a particular (longest-possible) directive, we don't want
+					// to map to another!
+					break
+				}
+			}
+		}
+	}
+	return directives
+}
+
+// Can be replaced with slices.Contains with go1.21
+func directivesIncludes(directives []string, d string) bool {
+	for _, ud := range directives {
+		if ud == d {
+			return true
+		}
+	}
+	return false
+}
+
 // switchChecker returns a node visitor that checks exhaustiveness of
 // enum switch statements for the supplied pass, and reports
 // diagnostics. The node visitor expects only *ast.SwitchStmt nodes.
@@ -80,17 +118,27 @@ func switchChecker(pass *analysis.Pass, cfg switchConfig, generated boolCache, c
 		sw := n.(*ast.SwitchStmt)
 
 		switchComments := comments.get(pass.Fset, file)[sw]
-		if !cfg.explicit && hasCommentPrefix(switchComments, ignoreComment) {
+		uDirectives := userDirectives(switchComments)
+		if !cfg.explicit && directivesIncludes(uDirectives, ignoreComment) {
 			// Skip checking of this switch statement due to ignore
 			// comment. Still return true because there may be nested
 			// switch statements that are not to be ignored.
 			return true, resultIgnoreComment
 		}
-		if cfg.explicit && !hasCommentPrefix(switchComments, enforceComment) {
+		if cfg.explicit && !directivesIncludes(uDirectives, enforceComment) {
 			// Skip checking of this switch statement due to missing
 			// enforce comment.
 			return true, resultNoEnforceComment
 		}
+		requireDefaultCase := cfg.defaultCaseRequired
+		if directivesIncludes(uDirectives, ignoreDefaultCaseRequiredComment) {
+			requireDefaultCase = false
+		}
+		if directivesIncludes(uDirectives, enforceDefaultCaseRequiredComment) {
+			// We have "if" instead of "else if" here in case of conflicting ignore/enforce directives.
+			// In that case, because this is second, we will default to enforcing.
+			requireDefaultCase = true
+		}
 
 		if sw.Tag == nil {
 			return true, resultNoSwitchTag // never possible for valid Go program?
@@ -114,13 +162,21 @@ func switchChecker(pass *analysis.Pass, cfg switchConfig, generated boolCache, c
 			checkl.add(e.typ, e.members, pass.Pkg == e.typ.Pkg())
 		}
 
-		def := analyzeSwitchClauses(sw, pass.TypesInfo, checkl.found)
+		defaultCaseExists := analyzeSwitchClauses(sw, pass.TypesInfo, checkl.found)
+		if !defaultCaseExists && requireDefaultCase {
+			// Even if the switch explicitly enumerates all the
+			// enum values, the user has still required all switches
+			// to have a default case. We check this first to avoid
+			// early-outs
+			pass.Report(makeMissingDefaultDiagnostic(sw, dedupEnumTypes(toEnumTypes(es))))
+			return true, resultMissingDefaultCase
+		}
 		if len(checkl.remaining()) == 0 {
 			// All enum members accounted for.
 			// Nothing to report.
 			return true, resultEnumMembersAccounted
 		}
-		if def && cfg.defaultSignifiesExhaustive {
+		if defaultCaseExists && cfg.defaultSignifiesExhaustive {
 			// Though enum members are not accounted for, the
 			// existence of the default case signifies
 			// exhaustiveness.  So don't report.
@@ -167,3 +223,14 @@ func makeSwitchDiagnostic(sw *ast.SwitchStmt, enumTypes []enumType, missing map[
 		),
 	}
 }
+
+func makeMissingDefaultDiagnostic(sw *ast.SwitchStmt, enumTypes []enumType) analysis.Diagnostic {
+	return analysis.Diagnostic{
+		Pos: sw.Pos(),
+		End: sw.End(),
+		Message: fmt.Sprintf(
+			"missing default case in switch of type %s",
+			diagnosticEnumTypes(enumTypes),
+		),
+	}
+}

testdata/src/default-case-required/default-not-required/default_not_required.go

@@ -0,0 +1,46 @@
+package notrequired
+
+import "default-case-required"
+
+func _a(t dcr.T) {
+	// No diagnostic because default-require is not set.
+	switch t {
+	case dcr.A:
+	case dcr.B:
+	}
+}
+
+func _b(t dcr.T) {
+	//exhaustive:enforce-default-case-required this is a comment showing that we can turn it on for select switches
+	switch t { // want "^missing default in switch of type dcr.T$"
+	case dcr.A:
+	case dcr.B:
+	}
+}
+
+func _c(t dcr.T) {
+	//exhaustive:ignore-default-case-required this comment is discarded in facvor of the enforcement
+	//exhaustive:enforce-default-case-required this is a comment showing that we can turn it on for select switches
+	switch t { // want "^missing default in switch of type dcr.T$"
+	case dcr.A:
+	case dcr.B:
+	}
+}
+
+func _d(t dcr.T) {
+	//exhaustive:enforce-default-case-required this is a comment showing that we can turn it on for select switches
+	//exhaustive:ignore-default-case-required this comment is discarded in facvor of the enforcement
+	switch t { // want "^missing default in switch of type dcr.T$"
+	case dcr.A:
+	case dcr.B:
+	}
+}
+
+func _e(t dcr.T) {
+	//exhaustive:enforce-default-case-required this is happy because it has a default
+	switch t {
+	case dcr.A:
+	case dcr.B:
+	default:
+	}
+}

testdata/src/default-case-required/default-required/default_required.go

@@ -0,0 +1,37 @@
+package required
+
+import "default-case-required"
+
+func _a(t dcr.T) {
+	// expect a diagnostic when fDefaultCaseRequired is true.
+	switch t { // want "^missing default in switch of type dcr.T$"
+	case dcr.A:
+	case dcr.B:
+	}
+}
+
+func _b(t dcr.T) {
+	//exhaustive:ignore-default-case-required this is a comment showing that we can turn it off for select switches
+	switch t {
+	case dcr.A:
+	case dcr.B:
+	}
+}
+
+func _c(t dcr.T) {
+	//exhaustive:ignore-default-case-required this comment is discarded in facvor of the enforcement
+	//exhaustive:enforce-default-case-required this helps override the above
+	switch t { // want "^missing default in switch of type dcr.T$"
+	case dcr.A:
+	case dcr.B:
+	}
+}
+
+func _d(t dcr.T) {
+	// this is happy even with enforcement because we have a default
+	switch t {
+	case dcr.A:
+	case dcr.B:
+	default:
+	}
+}

testdata/src/default-case-required/default_case_required.go

@@ -0,0 +1,8 @@
+package dcr
+
+type T int
+
+const (
+	A T = iota
+	B
+)