diff --git a/internal/graphql/config/config.go b/internal/graphql/config/config.go index 72c7d21d..3d8b904e 100644 --- a/internal/graphql/config/config.go +++ b/internal/graphql/config/config.go @@ -194,6 +194,21 @@ const ( RuleNoUnreachableTypes = "no-unreachable-types" ) +// These constants name the 2 diagnostics raised about "devbase-lint-" +// suppression comments. Unlike the Tier 2 and Tier 3 rules, they run +// without being enabled, at "error" severity; scripts/devbase.yaml can +// lower either to "warn" or turn it "off". They are never themselves +// suppressible. +const ( + // RuleInvalidSuppression reports a suppression comment that is + // malformed, names no rule or an unknown one, or has no reason. + RuleInvalidSuppression = "invalid-suppression" + + // RuleUnusedSuppression reports a suppression comment that + // suppressed no violation of a rule it names. + RuleUnusedSuppression = "unused-suppression" +) + // Rule is the per-rule override for a single lint rule. It accepts // two YAML shapes: // @@ -316,6 +331,17 @@ func (c *Lint) MergeExcludes(extra ...string) []string { return merged } +// Disabled reports whether scripts/devbase.yaml sets the rule to "off". +// A rule absent from the config is not disabled, so diagnostics that run +// by default stay on. c may be nil. +func (c *Lint) Disabled(rule string) bool { + if c == nil { + return false + } + rc, ok := c.Rules[rule] + return ok && rc.Severity == SeverityOff +} + // Enabled reports whether the rule should run at all. A Tier 2 or Tier 3 // rule is enabled only once scripts/devbase.yaml gives it a severity // other than SeverityOff -- it does not run by default, matching diff --git a/internal/graphql/config/config_test.go b/internal/graphql/config/config_test.go index 6eac35c6..fe663a1c 100644 --- a/internal/graphql/config/config_test.go +++ b/internal/graphql/config/config_test.go @@ -295,6 +295,33 @@ func TestEnabledRequiresAnExplicitNonOffSeverity(t *testing.T) { } } +func TestDisabledIsFalseByDefault(t *testing.T) { + var nilConfig *Lint + assert.Equal(t, nilConfig.Disabled(RuleUnusedSuppression), false) + + c := &Lint{} + assert.Equal(t, c.Disabled(RuleUnusedSuppression), false) +} + +func TestDisabledRequiresAnExplicitOffSeverity(t *testing.T) { + cases := []struct { + name string + severity Severity + want bool + }{ + {name: "off", severity: SeverityOff, want: true}, + {name: "warn", severity: SeverityWarn, want: false}, + {name: "error", severity: SeverityError, want: false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := &Lint{Rules: map[string]Rule{RuleUnusedSuppression: {Severity: tc.severity}}} + assert.Equal(t, c.Disabled(RuleUnusedSuppression), tc.want) + }) + } +} + func TestSeverityOfDefaultsToErrorWhenRuleIsNotInTheMap(t *testing.T) { var nilConfig *Lint assert.Equal(t, nilConfig.SeverityOf("unique-type-names"), SeverityError) diff --git a/internal/graphql/lint/eslint_directives.go b/internal/graphql/lint/eslint_directives.go index 9fa8578d..3ae2e635 100644 --- a/internal/graphql/lint/eslint_directives.go +++ b/internal/graphql/lint/eslint_directives.go @@ -164,11 +164,14 @@ func suppresses(directives []eslintDirective, rule string, line int) bool { } // unsuppressibleRules are the rules a directive comment never silences. -// Tier 1 violations currently return from FilesFromSources before -// suppression runs, so this guard only protects against that changing. +// Tier 1 violations return from FilesFromSources before suppression runs, +// so for those this guard only protects against that changing. The two +// suppression diagnostics are added before the "eslint-" directives are +// applied, so this set is what keeps them visible. // //nolint:gochecknoglobals // Why: built once, never mutated. -var unsuppressibleRules = set.Of(append(config.Tier1RuleNames(), UnclassifiedRule)...) +var unsuppressibleRules = set.Of(append(config.Tier1RuleNames(), UnclassifiedRule, + config.RuleInvalidSuppression, config.RuleUnusedSuppression)...) // withoutSuppressed returns violations minus every Tier 2 or Tier 3 // violation silenced by a directive comment in the source it was found diff --git a/internal/graphql/lint/hashtag.go b/internal/graphql/lint/hashtag.go index 03672cc7..97631bb3 100644 --- a/internal/graphql/lint/hashtag.go +++ b/internal/graphql/lint/hashtag.go @@ -76,11 +76,13 @@ func trailingCommentStarts(src *ast.Source) (map[int]bool, error) { return trailing, nil } -// isEslintDirective reports whether c is an ESLint directive comment, -// such as "# eslint-disable ...". @graphql-eslint never treats these as -// descriptions. -func isEslintDirective(c *ast.Comment) bool { - return strings.HasPrefix(strings.TrimSpace(c.Text()), "eslint") +// isDirectiveComment reports whether c is a directive comment, such as +// "# eslint-disable ..." or "# devbase-lint-disable-next-line ...". A +// directive is never a description: @graphql-eslint skips the ESLint ones, +// and the same holds for devbase's own. +func isDirectiveComment(c *ast.Comment) bool { + text := strings.TrimSpace(c.Text()) + return strings.HasPrefix(text, "eslint") || strings.HasPrefix(text, devbaseDirectivePrefix) } // noHashtagDescriptionViolations reports a RuleNoHashtagDescription @@ -103,7 +105,7 @@ func noHashtagDescriptionViolations(sites []descriptionSite, trailing map[int]bo // Only the last comment of a group is reported, so only it is // checked for being a directive. last := group.List[len(group.List)-1] - if isEslintDirective(last) { + if isDirectiveComment(last) { continue } if len(group.List) == 1 && trailing[last.Position.Start] { diff --git a/internal/graphql/lint/lint.go b/internal/graphql/lint/lint.go index bf0dcc67..6698a702 100644 --- a/internal/graphql/lint/lint.go +++ b/internal/graphql/lint/lint.go @@ -156,6 +156,10 @@ func FilesFromSources(fileSources []*ast.Source, cfg *config.Lint) ([]Violation, if err != nil { return nil, err } + violations, err = withNativeSuppression(fileSources, violations, cfg) + if err != nil { + return nil, err + } return withoutSuppressed(fileSources, violations) } diff --git a/internal/graphql/lint/suppression_comments.go b/internal/graphql/lint/suppression_comments.go new file mode 100644 index 00000000..53fb113b --- /dev/null +++ b/internal/graphql/lint/suppression_comments.go @@ -0,0 +1,431 @@ +// Copyright 2026 Outreach Corporation. All Rights Reserved. + +// Description: Honors "# devbase-lint-disable" style suppression comments. + +// suppression_comments.go implements devbase's own suppression comments, +// the native counterpart of the "eslint-" ones in eslint_directives.go: +// +// # devbase-lint-disable-next-line rule[, rule]... [-- reason] +// # devbase-lint-disable-line rule[, rule]... [-- reason] +// # devbase-lint-disable rule[, rule]... [-- reason] +// # devbase-lint-enable [rule[, rule]...] +// +// Unlike the "eslint-" ones, a native directive is strict: it names at +// least one rule, by its bare devbase name, and it gives a reason, either +// inline after "--" or as a "# Why:" comment block directly above a +// disable-next-line or disable directive. A directive that is malformed, +// names an unknown or unsuppressible rule, or has no reason suppresses +// nothing and is reported as invalid-suppression. A valid directive that +// suppresses no violation of a rule it names is reported once for that +// rule as unused-suppression. Both default to "error" and can be lowered +// or turned off in scripts/devbase.yaml. +// +// disable-next-line covers the next line. A description below the +// directive is skipped, because violations are reported on the name's +// line. + +package lint + +import ( + "fmt" + "slices" + "strings" + "unicode" + + "github.com/getoutreach/devbase/v2/internal/graphql/config" + "github.com/vektah/gqlparser/v2/ast" + "github.com/vektah/gqlparser/v2/gqlerror" + "github.com/vektah/gqlparser/v2/lexer" +) + +// devbaseDirectivePrefix starts every native suppression comment. +const devbaseDirectivePrefix = "devbase-lint-" + +// whyPrefix starts the first line of a reason written above a directive. +const whyPrefix = "Why:" + +// suppressionKind identifies which native suppression comment a +// directive is. +type suppressionKind int + +// Supported kinds of native suppression comment. +const ( + // suppressDisable silences rules from the line after it until a + // matching suppressEnable. + suppressDisable suppressionKind = iota + + // suppressEnable ends a suppressDisable for the rules it names. + suppressEnable + + // suppressDisableNextLine silences rules on the next line only. + suppressDisableNextLine + + // suppressDisableLine silences rules on its own line only. + suppressDisableLine +) + +// suppressionKinds maps a directive's keyword, without the prefix, to +// its kind. +// +//nolint:gochecknoglobals // Why: a fixed lookup table, never mutated. +var suppressionKinds = map[string]suppressionKind{ + "disable": suppressDisable, + "enable": suppressEnable, + "disable-next-line": suppressDisableNextLine, + "disable-line": suppressDisableLine, +} + +// suppression is one parsed native suppression comment. +type suppression struct { + kind suppressionKind + keyword string + pos ast.Position + rules []string + reason string + + // target is the line a disable-next-line or disable-line directive + // covers. + target int + + // problems lists why the directive is invalid, if it is. + problems []string + + // ownLine is whether the directive is the only thing on its line, + // not a trailing comment after code. + ownLine bool + + // used records the rules the directive suppressed a violation of. + used map[string]bool + + // shadowed records the rules for which another directive also covered + // a violation the directive would have suppressed. + shadowed map[string]bool +} + +// parseSuppression parses the text of a "#" comment, without the leading +// "#", as a native suppression directive. ok is false for any other +// comment. A comment with the prefix but an unknown keyword is returned +// with a problem so a typo is reported instead of silently ignored. +func parseSuppression(text string) (s suppression, ok bool) { + text = strings.TrimSpace(text) + if !strings.HasPrefix(text, devbaseDirectivePrefix) { + return s, false + } + + keyword, rest := text, "" + if i := strings.IndexFunc(text, unicode.IsSpace); i >= 0 { + keyword, rest = text[:i], text[i:] + } + s.keyword = keyword + + kind, known := suppressionKinds[strings.TrimPrefix(keyword, devbaseDirectivePrefix)] + if !known { + s.problems = []string{"unknown directive"} + return s, true + } + s.kind = kind + + if loc := eslintReasonSeparator.FindStringIndex(rest); loc != nil { + s.reason = strings.TrimSpace(rest[loc[1]:]) + rest = rest[:loc[0]] + } + for name := range strings.SplitSeq(rest, ",") { + name = strings.TrimSpace(name) + if name != "" && !slices.Contains(s.rules, name) { + s.rules = append(s.rules, name) + } + } + return s, true +} + +// acceptsWhy reports whether s may take its reason from a "# Why:" block +// above it: only a disable-next-line or disable directive on its own line. +func (s *suppression) acceptsWhy() bool { + return s.ownLine && (s.kind == suppressDisableNextLine || s.kind == suppressDisable) +} + +// validate records the problems that make s invalid. +func (s *suppression) validate() { + if s.kind != suppressEnable && len(s.rules) == 0 { + s.problems = append(s.problems, "names no rule") + } + for _, rule := range s.rules { + switch { + case configurableRules.Contains(rule): + case unsuppressibleRules.Contains(rule): + s.problems = append(s.problems, fmt.Sprintf("rule %q cannot be suppressed", rule)) + default: + problem := fmt.Sprintf("unknown rule %q", rule) + if strings.Contains(rule, "--") { + problem += ` (separate the reason with " -- ")` + } + s.problems = append(s.problems, problem) + } + } + if s.kind == suppressEnable || s.reason != "" { + return + } + if s.acceptsWhy() { + s.problems = append(s.problems, `has no reason (add "-- reason" or a "# Why:" comment directly above)`) + } else { + s.problems = append(s.problems, `has no reason (add "-- reason")`) + } +} + +// isTrailing reports whether the comment at tokens[i] follows code on its +// own line. +func isTrailing(tokens []lexer.Token, i int) bool { + return i > 0 && tokens[i-1].Kind != lexer.Comment && tokens[i-1].Pos.Line == tokens[i].Pos.Line +} + +// nextCode returns the index of the first token at or after from that is +// not a comment, or -1. +func nextCode(tokens []lexer.Token, from int) int { + for i := from; i < len(tokens); i++ { + if tokens[i].Kind != lexer.Comment { + return i + } + } + return -1 +} + +// lineFinder returns a function giving the 1-based line that holds the rune +// at a rune offset in input. A lexer token's Pos.Line is not the line it +// starts on when the token spans lines, as a block string does, so the +// start line comes from the offset. Like the lexer, it ends a line at "\n", +// "\r\n" or a lone "\r". +func lineFinder(input string) func(offset int) int { + runes := []rune(input) + starts := []int{0} + for i, r := range runes { + if r == '\n' || (r == '\r' && (i+1 == len(runes) || runes[i+1] != '\n')) { + starts = append(starts, i+1) + } + } + return func(offset int) int { + line, _ := slices.BinarySearch(starts, offset+1) + return line + } +} + +// nextLineTarget returns the line a disable-next-line directive at +// tokens[i] covers: the next line, or, when a description starts there, +// the line of the token after the description. lineAt gives the line a +// rune offset is on. +func nextLineTarget(tokens []lexer.Token, i int, lineAt func(int) int) int { + line := tokens[i].Pos.Line + 1 + j := nextCode(tokens, i+1) + if j < 0 || (tokens[j].Kind != lexer.String && tokens[j].Kind != lexer.BlockString) || + lineAt(tokens[j].Pos.Start) != line { + return line + } + if k := nextCode(tokens, j+1); k >= 0 { + return tokens[k].Pos.Line + } + return line +} + +// whyReason returns the reason written as a "# Why:" block directly above +// the directive at tokens[i]: the run of comment-only lines ending on the +// line above it, from the first line whose text starts with "Why:". Another +// directive, a blank line or code ends the run. +func whyReason(tokens []lexer.Token, i int) string { + var run []string + line := tokens[i].Pos.Line + for j := i - 1; j >= 0; j-- { + t := tokens[j] + if t.Kind != lexer.Comment || t.Pos.Line != line-1 || isTrailing(tokens, j) { + break + } + text := strings.TrimSpace(strings.TrimPrefix(t.Value, "#")) + if strings.HasPrefix(text, devbaseDirectivePrefix) { + break + } + run = append(run, text) + line = t.Pos.Line + } + slices.Reverse(run) + + for k, text := range run { + first, ok := strings.CutPrefix(text, whyPrefix) + if !ok { + continue + } + parts := slices.Concat([]string{strings.TrimSpace(first)}, run[k+1:]) + return strings.Join(slices.DeleteFunc(parts, func(p string) bool { return p == "" }), " ") + } + return "" +} + +// suppressionsIn lexes src for native suppression comments, in source +// order. +func suppressionsIn(src *ast.Source) ([]*suppression, error) { + lx := lexer.New(src) + + var tokens []lexer.Token + for { + tok, err := lx.ReadToken() + if err != nil { + return nil, fmt.Errorf("lex %s: %w", src.Name, err) + } + if tok.Kind == lexer.EOF { + break + } + tokens = append(tokens, tok) + } + + lineAt := lineFinder(src.Input) + + var out []*suppression + for i, tok := range tokens { + if tok.Kind != lexer.Comment { + continue + } + s, ok := parseSuppression(strings.TrimPrefix(tok.Value, "#")) + if !ok { + continue + } + s.pos = tok.Pos + s.ownLine = !isTrailing(tokens, i) + s.used = make(map[string]bool) + s.shadowed = make(map[string]bool) + + if len(s.problems) == 0 { + switch s.kind { + case suppressDisableNextLine: + s.target = nextLineTarget(tokens, i, lineAt) + case suppressDisableLine: + s.target = tok.Pos.Line + case suppressDisable, suppressEnable: + } + if s.reason == "" && s.acceptsWhy() { + s.reason = whyReason(tokens, i) + } + s.validate() + } + out = append(out, &s) + } + return out, nil +} + +// suppressedBy reports whether a valid directive in ss silences rule on +// line. The first directive that covers it is recorded as used for rule, +// a line directive before a range one, and any others as shadowed. +func suppressedBy(ss []*suppression, rule string, line int) bool { + if len(ss) == 0 { + return false + } + + var covering []*suppression + activeBy := make(map[string]*suppression) + for _, s := range ss { + if len(s.problems) > 0 { + continue + } + switch s.kind { + case suppressDisableNextLine, suppressDisableLine: + if s.target == line && slices.Contains(s.rules, rule) { + covering = append(covering, s) + } + case suppressDisable: + if s.pos.Line < line { + for _, r := range s.rules { + activeBy[r] = s + } + } + case suppressEnable: + if s.pos.Line >= line { + continue + } + if len(s.rules) == 0 { + clear(activeBy) + } + for _, r := range s.rules { + delete(activeBy, r) + } + } + } + + if active := activeBy[rule]; active != nil { + covering = append(covering, active) + } + if len(covering) == 0 { + return false + } + covering[0].used[rule] = true + for _, s := range covering[1:] { + s.shadowed[rule] = true + } + return true +} + +// suppressionDiagnostics reports the invalid directives in ss, and the +// valid ones that suppressed nothing for a rule they name, as violations +// positioned at the directive. cfg can turn either diagnostic off. +func suppressionDiagnostics(ss []*suppression, cfg *config.Lint) []Violation { + var out []Violation + report := func(s *suppression, rule, format string, args ...any) { + pos := s.pos + out = append(out, Violation{err: gqlerror.ErrorPosf(&pos, format, args...), Rule: rule}) + } + + for _, s := range ss { + if len(s.problems) > 0 { + if !cfg.Disabled(config.RuleInvalidSuppression) { + report(s, config.RuleInvalidSuppression, "Invalid suppression comment %s: %s", + s.keyword, strings.Join(s.problems, "; ")) + } + continue + } + if s.kind == suppressEnable || cfg.Disabled(config.RuleUnusedSuppression) { + continue + } + for _, rule := range s.rules { + if s.used[rule] { + continue + } + hint := "" + if s.shadowed[rule] { + hint = " (another directive already covers it)" + } + report(s, config.RuleUnusedSuppression, "Unused suppression comment %s: rule %q suppressed no violation%s", + s.keyword, rule, hint) + } + } + return out +} + +// withNativeSuppression returns violations minus every Tier 2 or Tier 3 +// violation silenced by a valid native directive in the source it was found +// in, plus the diagnostics about the directives themselves. sources must be +// the sources violations were produced from, and violations must not be used +// after the call, since it is filtered in place. +func withNativeSuppression(sources []*ast.Source, violations []Violation, cfg *config.Lint) ([]Violation, error) { + byFile := make(map[string][]*suppression, len(sources)) + for _, src := range sources { + // Every directive contains the prefix, so files without it are not + // lexed. + if !strings.Contains(src.Input, devbaseDirectivePrefix) { + continue + } + ss, err := suppressionsIn(src) + if err != nil { + return nil, err + } + if len(ss) > 0 { + byFile[src.Name] = ss + } + } + if len(byFile) == 0 { + return violations, nil + } + + kept := slices.DeleteFunc(violations, func(v Violation) bool { + return !unsuppressibleRules.Contains(v.Rule) && len(v.err.Locations) > 0 && + suppressedBy(byFile[v.File()], v.Rule, v.err.Locations[0].Line) + }) + for _, src := range sources { + kept = append(kept, suppressionDiagnostics(byFile[src.Name], cfg)...) + } + return kept, nil +} diff --git a/internal/graphql/lint/suppression_comments_test.go b/internal/graphql/lint/suppression_comments_test.go new file mode 100644 index 00000000..be24085b --- /dev/null +++ b/internal/graphql/lint/suppression_comments_test.go @@ -0,0 +1,345 @@ +// Copyright 2026 Outreach Corporation. All Rights Reserved. + +// Description: Tests for "# devbase-lint-disable" style suppression comments. + +package lint + +import ( + "fmt" + "slices" + "strings" + "testing" + + "github.com/getoutreach/devbase/v2/internal/graphql/config" + "gotest.tools/v3/assert" +) + +func invalidSuppression(keyword, problem string) string { + return fmt.Sprintf("invalid-suppression Invalid suppression comment %s: %s", keyword, problem) +} + +func unusedSuppression(keyword, rule string) string { + return fmt.Sprintf("unused-suppression Unused suppression comment %s: rule %q suppressed no violation", keyword, rule) +} + +// shadowedSuppression is unusedSuppression for a directive whose violations +// another directive already covers. +func shadowedSuppression(keyword, rule string) string { + return unusedSuppression(keyword, rule) + " (another directive already covers it)" +} + +func TestDevbaseLintSuppression(t *testing.T) { + naming := func(v string) string { return fmt.Sprintf(namingFor, v) } + desc := func(v string) string { return fmt.Sprintf(descFor, v) } + + const ( + next = "devbase-lint-disable-next-line" + both = "naming-convention, require-description" + noReas = "has no reason (add \"-- reason\" or a \"# Why:\" comment directly above)" + noRules = "names no rule" + + // A disable-line directive takes only an inline reason. + noReasLine = "has no reason (add \"-- reason\")" + ) + + cases := []struct { + name string + sdl string + want []string + }{ + { + "next-line with an inline reason", + `enum E { + # devbase-lint-disable-next-line naming-convention, require-description -- Legacy value. + first + second + }`, + []string{naming("second"), desc("second")}, + }, + { + "next-line with a Why block above", + `enum E { + # Why: Legacy value, + # kept for old clients. + # devbase-lint-disable-next-line naming-convention, require-description + first + }`, + nil, + }, + { + "comment lines before the Why line are ordinary comments", + `enum E { + # prose about first + # Why: Legacy value. + # devbase-lint-disable-next-line naming-convention, require-description + first + }`, + nil, + }, + { + "a blank line ends the Why block, so the directive has no reason and suppresses nothing", + `enum E { + # Why: Legacy value. + + # devbase-lint-disable-next-line naming-convention, require-description + first + }`, + []string{invalidSuppression(next, noReas), naming("first"), desc("first")}, + }, + { + "disable and enable bound a range", + `enum E { + # devbase-lint-disable naming-convention, require-description -- Generated section. + a + b + # devbase-lint-enable naming-convention, require-description + c + }`, + []string{naming("c"), desc("c")}, + }, + { + "enable with no rule re-enables every rule", + `enum E { + # devbase-lint-disable naming-convention, require-description -- Section. + a + # devbase-lint-enable + b + }`, + []string{naming("b"), desc("b")}, + }, + { + "disable-line covers its own line", + `enum E { + a # devbase-lint-disable-line naming-convention, require-description -- Public name. + b + }`, + []string{naming("b"), desc("b")}, + }, + { + "disable-line takes only an inline reason", + `enum E { + # Why: Public name. + a # devbase-lint-disable-line naming-convention, require-description + }`, + []string{invalidSuppression("devbase-lint-disable-line", noReasLine), naming("a"), desc("a")}, + }, + { + "a directive without a reason fails closed", + `enum E { + # devbase-lint-disable-next-line ` + both + ` + first + }`, + []string{invalidSuppression(next, noReas), naming("first"), desc("first")}, + }, + { + "a directive that names no rule fails closed", + `enum E { + # devbase-lint-disable-next-line -- Because. + first + }`, + []string{invalidSuppression(next, noRules), naming("first"), desc("first")}, + }, + { + "an unknown rule name fails closed", + `enum E { + # devbase-lint-disable-next-line nameing-convention -- Typo. + first + }`, + []string{invalidSuppression(next, `unknown rule "nameing-convention"`), naming("first"), desc("first")}, + }, + { + "a Tier 1 rule cannot be named", + `enum E { + # devbase-lint-disable-next-line known-type-names -- Not allowed. + first + }`, + []string{invalidSuppression(next, `rule "known-type-names" cannot be suppressed`), naming("first"), desc("first")}, + }, + { + "an unknown directive keyword is reported", + `enum E { + # devbase-lint-disabel-next-line ` + both + ` -- Typo. + first + }`, + []string{invalidSuppression("devbase-lint-disabel-next-line", "unknown directive"), naming("first"), desc("first")}, + }, + { + "a directive that suppresses nothing for a rule is reported for that rule", + `enum E { + # devbase-lint-disable-next-line naming-convention, require-description -- Stale. + FIRST + }`, + []string{unusedSuppression(next, "naming-convention")}, + }, + { + "a rule named twice is reported once", + `enum E { + # devbase-lint-disable-next-line naming-convention, naming-convention -- Stale. + FIRST + }`, + []string{desc("FIRST"), unusedSuppression(next, "naming-convention")}, + }, + { + "a trailing disable takes only an inline reason", + `enum E { + # Why: Public name. + a # devbase-lint-disable naming-convention, require-description + }`, + []string{invalidSuppression("devbase-lint-disable", noReasLine), naming("a"), desc("a")}, + }, + { + "a disable with no enable runs to the end of the file", + `enum E { + before + # devbase-lint-disable naming-convention, require-description -- Rest of the file. + a + b + }`, + []string{naming("before"), desc("before")}, + }, + { + "a directive another one already covers is reported with that hint", + `enum E { + # devbase-lint-disable naming-convention -- Whole enum. + a # devbase-lint-disable-line naming-convention, require-description -- Public. + }`, + []string{shadowedSuppression("devbase-lint-disable", "naming-convention")}, + }, + { + "a reason separator without whitespace is called out", + `enum E { + # devbase-lint-disable-next-line naming-convention-- Typo. + first + }`, + []string{ + invalidSuppression(next, `unknown rule "naming-convention-- Typo." (separate the reason with " -- "); `+noReas), + naming("first"), desc("first"), + }, + }, + { + "a description below the directive is skipped", + `enum E { + # devbase-lint-disable-next-line naming-convention -- Legacy. + """ + A described value. + """ + legacy + }`, + nil, + }, + { + "a one-line description below the directive is skipped", + `enum E { + # devbase-lint-disable-next-line naming-convention -- Legacy. + "A described value." + legacy + }`, + nil, + }, + { + "a blank line before the description means the directive covers nothing", + `enum E { + # devbase-lint-disable-next-line naming-convention -- Legacy. + + "A described value." + legacy + }`, + []string{naming("legacy"), unusedSuppression(next, "naming-convention")}, + }, + { + "an eslint-disable cannot silence the suppression diagnostics", + `# eslint-disable + enum E { + # devbase-lint-disable-next-line ` + both + ` + first + }`, + []string{invalidSuppression(next, noReas)}, + }, + { + "a Tier 1 violation returns before any suppression diagnostics", + `# devbase-lint-disable-next-line naming-convention -- Anything. + type Foo { a: Missing }`, + []string{"known-type-names Undefined type Missing."}, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := ruleMessages(t, c.sdl, namingAndDescriptionCfg()) + if c.want == nil { + assert.Equal(t, len(got), 0, "got %v", got) + return + } + want := slices.Clone(c.want) + slices.Sort(want) + assert.DeepEqual(t, got, want) + }) + } +} + +// TestDevbaseLintSuppressionDiagnosticsCanBeTurnedOff confirms +// scripts/devbase.yaml can set either diagnostic to "off". +func TestDevbaseLintSuppressionDiagnosticsCanBeTurnedOff(t *testing.T) { + cfg := namingAndDescriptionCfg() + cfg.Rules[config.RuleInvalidSuppression] = config.Rule{Severity: config.SeverityOff} + cfg.Rules[config.RuleUnusedSuppression] = config.Rule{Severity: config.SeverityOff} + + got := ruleMessages(t, `enum E { + # devbase-lint-disable-next-line naming-convention, require-description + first + # devbase-lint-disable-next-line naming-convention, require-description -- Stale. + SECOND_NAMED + }`, cfg) + + // The directive without a reason suppresses nothing, so "first" keeps + // both violations. The stale directive's unused report is silenced. + want := []string{fmt.Sprintf(namingFor, "first"), fmt.Sprintf(descFor, "first")} + slices.Sort(want) + assert.DeepEqual(t, got, want) +} + +// TestDevbaseLintSuppressionIsPerFile confirms a directive in one file does +// not suppress a violation in another file. +func TestDevbaseLintSuppressionIsPerFile(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "a.graphql", "enum A {\n\t# devbase-lint-disable naming-convention, require-description -- Why.\n\tx\n}\n") + b := writeFile(t, dir, "b.graphql", "enum B {\n\ty\n}\n") + + violations, err := Files([]string{a, b}, namingAndDescriptionCfg()) + assert.NilError(t, err) + assert.Equal(t, len(violations), 2) + for _, v := range violations { + assert.Equal(t, v.File(), b) + } +} + +// TestDevbaseLintDirectiveIsNotAHashtagDescription confirms +// no-hashtag-description does not report a native directive, or the Why +// block above it, as a description. +func TestDevbaseLintDirectiveIsNotAHashtagDescription(t *testing.T) { + cfg := &config.Lint{Rules: map[string]config.Rule{ + config.RuleNamingConvention: {Severity: config.SeverityError, Options: map[string]any{"types": "PascalCase"}}, + config.RuleNoHashtagDescription: {Severity: config.SeverityError}, + }} + + got := ruleMessages(t, `# Why: Legacy name. +# devbase-lint-disable-next-line naming-convention +type bad_name { a: String }`, cfg) + + assert.Equal(t, len(got), 0, "got %v", got) +} + +// TestDevbaseLintSuppressionLineEndings confirms a description below a +// directive is skipped whichever line ending the file uses, including the +// lone "\r" the lexer also treats as a line break, and with non-ASCII text +// before it. +func TestDevbaseLintSuppressionLineEndings(t *testing.T) { + const sdl = "# café ☃\nenum E {\n\t# devbase-lint-disable-next-line naming-convention -- Legacy.\n" + + "\t\"\"\"\n\tA described value.\n\t\"\"\"\n\tlegacy\n}\n" + + for name, newline := range map[string]string{"LF": "\n", "CRLF": "\r\n", "CR": "\r"} { + t.Run(name, func(t *testing.T) { + got := ruleMessages(t, strings.ReplaceAll(sdl, "\n", newline), namingAndDescriptionCfg()) + assert.Equal(t, len(got), 0, "got %v", got) + }) + } +}