Resolves #70: add stricter comment directive scan

comment.go

@@ -7,21 +7,52 @@ import (
 )
 
 const (
-	ignoreComment                     = "//exhaustive:ignore"
-	enforceComment                    = "//exhaustive:enforce"
-	ignoreDefaultCaseRequiredComment  = "//exhaustive:ignore-default-case-required"
-	enforceDefaultCaseRequiredComment = "//exhaustive:enforce-default-case-required"
+	exhaustiveComment                 = "//exhaustive:"
+	ignoreComment                     = "ignore"
+	enforceComment                    = "enforce"
+	ignoreDefaultCaseRequiredComment  = "ignore-default-case-required"
+	enforceDefaultCaseRequiredComment = "enforce-default-case-required"
 )
 
-func hasCommentPrefix(comments []*ast.CommentGroup, comment string) bool {
-	for _, c := range comments {
-		for _, cc := range c.List {
-			if strings.HasPrefix(cc.Text, comment) {
-				return true
+type directive int64
+
+const (
+	ignoreDirective = 1 << iota
+	enforceDirective
+	ignoreDefaultCaseRequiredDirective
+	enforceDefaultCaseRequiredDirective
+)
+
+type directiveSet int64
+
+func parseDirectiveSet(commentGroups []*ast.CommentGroup) (out directiveSet) {
+	for _, commentGroup := range commentGroups {
+		for _, comment := range commentGroup.List {
+			commentLine := comment.Text
+			if !strings.HasPrefix(commentLine, exhaustiveComment) {
+				continue
+			}
+			directive := commentLine[len(exhaustiveComment):]
+			if whiteSpaceIndex := strings.IndexAny(directive, " \t"); whiteSpaceIndex != -1 {
+				directive = directive[:whiteSpaceIndex]
+			}
+			switch directive {
+			case ignoreComment:
+				out |= ignoreDirective
+			case enforceComment:
+				out |= enforceDirective
+			case ignoreDefaultCaseRequiredComment:
+				out |= ignoreDefaultCaseRequiredDirective
+			case enforceDefaultCaseRequiredComment:
+				out |= enforceDefaultCaseRequiredDirective
 			}
 		}
 	}
-	return false
+	return
+}
+
+func (d directiveSet) hasDirective(directive directive) bool {
+	return int64(d)&int64(directive) != 0
 }
 
 func fileCommentMap(fset *token.FileSet, file *ast.File) ast.CommentMap {

comment_test.go

@@ -1,6 +1,7 @@
 package exhaustive
 
 import (
+	"go/ast"
 	"go/parser"
 	"go/token"
 	"testing"
@@ -32,3 +33,95 @@ func f() {
 		}
 	})
 }
+
+func TestDirectives(t *testing.T) {
+	t.Run("no directives", func(t *testing.T) {
+		commentGroups := []*ast.CommentGroup{
+			{
+				List: []*ast.Comment{
+					{
+						Text: "//exhaustive:foo",
+					},
+				},
+			},
+			{
+				List: []*ast.Comment{
+					{
+						Text: "//bar:value",
+					},
+					{
+						Text: "Another comment",
+					},
+				},
+			},
+		}
+		directives := parseDirectiveSet(commentGroups)
+		if directives != 0 {
+			t.Errorf("unexpected directives: %d", directives)
+		}
+	})
+
+	t.Run("several conflicting directives", func(t *testing.T) {
+		commentGroups := []*ast.CommentGroup{
+			{
+				List: []*ast.Comment{
+					{
+						Text: "//exhaustive:ignore",
+					},
+					{
+						Text: "//exhaustive:enforce",
+					},
+				},
+			},
+			{
+				List: []*ast.Comment{
+					{
+						Text: "//exhaustive:ignore-default-case-required",
+					},
+					{
+						Text: "//exhaustive:enforce-default-case-required",
+					},
+					{
+						Text: "//exhaustive:ignore-default-case-required",
+					},
+				},
+			},
+		}
+		directives := parseDirectiveSet(commentGroups)
+		expected := ignoreDirective | enforceDirective |
+			ignoreDefaultCaseRequiredDirective | enforceDefaultCaseRequiredDirective
+
+		if directives != directiveSet(expected) {
+			t.Errorf("unexpected directives: %d", directives)
+		}
+	})
+
+	t.Run("directives with trailing text and surrounding non-directives", func(t *testing.T) {
+		commentGroups := []*ast.CommentGroup{
+			{
+				List: []*ast.Comment{
+					{
+						Text: "Nevermind this comment",
+					},
+					{
+						Text: "//exhaustive:ignore foo",
+					},
+					{
+						Text: "//exhaustive:enforce-default-case-required\tbar",
+					},
+					{
+						Text: "//exhaustive:ignore-default-case-garbage",
+					},
+					{
+						Text: "This comment doesn't matter",
+					},
+				},
+			},
+		}
+
+		directives := parseDirectiveSet(commentGroups)
+		if directives != ignoreDirective|enforceDefaultCaseRequiredDirective {
+			t.Errorf("unexpected directives: %d", directives)
+		}
+	})
+}

map.go

@@ -77,13 +77,15 @@ func mapChecker(pass *analysis.Pass, cfg mapConfig, generated boolCache, comment
 			}
 		}
 
-		if !cfg.explicit && hasCommentPrefix(relatedComments, ignoreComment) {
+		directives := parseDirectiveSet(relatedComments)
+
+		if !cfg.explicit && directives.hasDirective(ignoreDirective) {
 			// Skip checking of this map literal due to ignore
 			// comment. Still return true because there may be nested
 			// map literals that are not to be ignored.
 			return true, resultIgnoreComment
 		}
-		if cfg.explicit && !hasCommentPrefix(relatedComments, enforceComment) {
+		if cfg.explicit && !directives.hasDirective(enforceDirective) {
 			return true, resultNoEnforceComment
 		}
 

switch.go

@@ -5,7 +5,6 @@ import (
 	"go/ast"
 	"go/types"
 	"regexp"
-	"strings"
 
 	"golang.org/x/tools/go/analysis"
 )
@@ -60,41 +59,6 @@ type switchConfig struct {
 	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.
@@ -118,30 +82,31 @@ func switchChecker(pass *analysis.Pass, cfg switchConfig, generated boolCache, c
 		sw := n.(*ast.SwitchStmt)
 
 		switchComments := comments.get(pass.Fset, file)[sw]
-		uDirectives := userDirectives(switchComments)
-		if !cfg.explicit && directivesIncludes(uDirectives, ignoreComment) {
+		uDirectives := parseDirectiveSet(switchComments)
+
+		if !cfg.explicit && uDirectives.hasDirective(ignoreDirective) {
 			// 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 && !directivesIncludes(uDirectives, enforceComment) {
+		if cfg.explicit && !uDirectives.hasDirective(enforceDirective) {
 			// Skip checking of this switch statement due to missing
 			// enforce comment.
 			return true, resultNoEnforceComment
 		}
 		requireDefaultCase := cfg.defaultCaseRequired
-		if directivesIncludes(uDirectives, ignoreDefaultCaseRequiredComment) {
+		if uDirectives.hasDirective(ignoreDefaultCaseRequiredDirective) {
 			requireDefaultCase = false
 		}
-		if directivesIncludes(uDirectives, enforceDefaultCaseRequiredComment) {
+		if uDirectives.hasDirective(enforceDefaultCaseRequiredDirective) {
 			// 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?
+			return true, resultNoSwitchTag
 		}
 
 		t := pass.TypesInfo.Types[sw.Tag]

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

@@ -1,10 +1,11 @@
 package notrequired
 
-import "default-case-required"
+import dcr "default-case-required"
 
 func _a(t dcr.T) {
 	// No diagnostic because neither fDefaultCaseRequired is true
 	// nor the enforcement comment is present.
+	//exhaustive:enforce-default-case-required-typo this invalid directive should be ignored
 	switch t {
 	case dcr.A:
 	case dcr.B:

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

@@ -1,9 +1,10 @@
 package required
 
-import "default-case-required"
+import dcr "default-case-required"
 
 func _a(t dcr.T) {
 	// expect a diagnostic when fDefaultCaseRequired is true.
+	//exhaustive:ignore-default-case-required-typo This invalid directive should be ignored
 	switch t { // want "^missing default case in switch of type dcr.T$"
 	case dcr.A:
 	case dcr.B:
@@ -12,6 +13,7 @@ func _a(t dcr.T) {
 
 func _b(t dcr.T) {
 	//exhaustive:ignore-default-case-required this is a comment showing that we can turn it off for select switches
+	//exhaustive:enforce-default-case-required-typo This invalid directive should be ignored
 	switch t {
 	case dcr.A:
 	case dcr.B: