From f3ea582cf043708d285895bcd4850a60759ad52c Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Fri, 23 Oct 2026 10:40:44 -0700 Subject: [PATCH 1/2] feat(graphql): honor eslint inline rule configuration comments A schema file can reconfigure an @graphql-eslint rule for itself with a comment such as "# eslint @graphql-eslint/naming-convention: [\"error\", { ... }]". The devbase port ignored these, so it kept applying the repository config to files that override it. Honor them for Tier 2 and Tier 3 rules the way ESLint does: a comment applies to its whole file, replaces the rule's options instead of merging with them, and the last comment for a rule wins. A rule can be named with or without the "@graphql-eslint/" prefix, and the value is a severity or a [severity, options] list in YAML flow syntax. Tier 1 rules cannot be configured, and a file's own "warn" or "error" still reports at the severity scripts/devbase.yaml gives the rule. Each distinct override reruns just its rule with that configuration and keeps the results for the files that declared it, so no rule needs a per-file configuration of its own. Assisted-By: claude-sonnet 5.5 via opencode --- .../graphql/lint/eslint_config_comments.go | 237 ++++++++++++++++ .../lint/eslint_config_comments_test.go | 253 ++++++++++++++++++ internal/graphql/lint/lint.go | 19 +- 3 files changed, 506 insertions(+), 3 deletions(-) create mode 100644 internal/graphql/lint/eslint_config_comments.go create mode 100644 internal/graphql/lint/eslint_config_comments_test.go diff --git a/internal/graphql/lint/eslint_config_comments.go b/internal/graphql/lint/eslint_config_comments.go new file mode 100644 index 00000000..41143918 --- /dev/null +++ b/internal/graphql/lint/eslint_config_comments.go @@ -0,0 +1,237 @@ +// Copyright 2026 Outreach Corporation. All Rights Reserved. + +// Description: Honors "# eslint : " inline configuration +// comments. + +// eslint_config_comments.go lets a schema file reconfigure a Tier 2 or +// Tier 3 rule for itself with the comment @graphql-eslint honors: +// +// # eslint @graphql-eslint/naming-convention: ["error", { FieldDefinition: "camelCase" }] +// # eslint @graphql-eslint/naming-convention: "off", @graphql-eslint/require-description: "error" +// +// As in ESLint, such a comment applies to its whole file wherever it +// appears, replaces the rule's configured options entirely instead of +// merging with them (so a severity alone leaves the rule with no +// options), and the last comment for a rule wins. A rule is named with +// or without the "@graphql-eslint/" prefix. The value is a severity +// ("off", "warn", "error", 0, 1 or 2) or a [severity, options] list, +// written as YAML flow syntax so unquoted keys work as they do in ESLint. +// A "-- reason" suffix is ignored. +// +// Only the rule's on/off state and options can differ per file: a file's +// "warn" or "error" still reports at the severity scripts/devbase.yaml +// gives the rule, or at "error" for a rule that file leaves unconfigured +// and a comment enables. Tier 1 rules cannot be configured, and an entry +// that names no configurable rule or has an invalid value is ignored. +// +// Rules take a single configuration for the whole schema, so each +// distinct override reruns only its rule over the whole schema and keeps +// the results for the files that declared it. A schema where every file +// carries a different override therefore pays one rerun per file. + +package lint + +import ( + "fmt" + "maps" + "slices" + "strings" + "unicode" + "unicode/utf8" + + "github.com/getoutreach/devbase/v2/internal/graphql/config" + "github.com/getoutreach/gobox/pkg/set" + "github.com/vektah/gqlparser/v2/ast" + "github.com/vektah/gqlparser/v2/lexer" + "go.yaml.in/yaml/v3" +) + +// configurableRules are the rules an inline configuration comment may +// change: every Tier 2 and Tier 3 rule. +// +//nolint:gochecknoglobals // Why: built once, never mutated. +var configurableRules = set.Of( + config.RuleUniqueDirectiveNamesPerLocation, config.RulePossibleTypeExtension, + config.RuleRequireDescription, config.RuleDescriptionStyle, config.RuleNoHashtagDescription, + config.RuleRequireDeprecationReason, config.RuleRequireDeprecationDate, config.RuleNoTypenamePrefix, + config.RuleNamingConvention, config.RuleNoCaseInsensitiveEnumValuesDuplicates, + config.RuleAlphabetize, config.RuleNoUnreachableTypes, +) + +// eslintSeverities maps ESLint's numeric severities to devbase's names. +// +//nolint:gochecknoglobals // Why: a fixed lookup table, never mutated. +var eslintSeverities = map[string]string{"0": "off", "1": "warn", "2": "error"} + +// splitTopLevel splits s at each comma that is neither inside brackets +// or braces nor inside a quoted string. +func splitTopLevel(s string) []string { + var parts []string + depth, start := 0, 0 + var quote rune + escaped := false + for i, r := range s { + switch { + case escaped: + escaped = false + case quote != 0: + escaped = r == '\\' + if r == quote { + quote = 0 + } + case r == '"' || r == '\'': + quote = r + case r == '[' || r == '{': + depth++ + case r == ']' || r == '}': + depth-- + case r == ',' && depth == 0: + parts = append(parts, s[start:i]) + start = i + 1 + } + } + return append(parts, s[start:]) +} + +// parseEslintRuleConfig parses the value of one "rule: value" entry, +// reporting false if it is not a valid severity or [severity, options]. +func parseEslintRuleConfig(value string) (config.Rule, bool) { + var doc yaml.Node + if err := yaml.Unmarshal([]byte(value), &doc); err != nil || len(doc.Content) == 0 { + return config.Rule{}, false + } + + root := doc.Content[0] + severity := root + if root.Kind == yaml.SequenceNode && len(root.Content) > 0 { + severity = root.Content[0] + } + if name, ok := eslintSeverities[severity.Value]; ok && severity.Kind == yaml.ScalarNode { + severity.Value, severity.Tag = name, "!!str" + } + + var rule config.Rule + if err := root.Decode(&rule); err != nil { + return config.Rule{}, false + } + return rule, true +} + +// parseEslintConfigComment parses the text of a "#" comment, without the +// leading "#", as an inline configuration comment, returning the rules +// it configures. It returns nil for any other comment. +func parseEslintConfigComment(text string) map[string]config.Rule { + rest, ok := strings.CutPrefix(strings.TrimSpace(text), "eslint") + if first, _ := utf8.DecodeRuneInString(rest); !ok || !unicode.IsSpace(first) { + return nil + } + if loc := eslintReasonSeparator.FindStringIndex(rest); loc != nil { + rest = rest[:loc[0]] + } + + var rules map[string]config.Rule + for _, entry := range splitTopLevel(rest) { + name, value, found := strings.Cut(entry, ":") + name = strings.TrimPrefix(strings.TrimSpace(name), eslintRulePrefix) + if !found || !configurableRules.Contains(name) { + continue + } + if rule, ok := parseEslintRuleConfig(value); ok { + if rules == nil { + rules = make(map[string]config.Rule) + } + rules[name] = rule + } + } + return rules +} + +// eslintConfigComments lexes src for inline configuration comments and +// returns the rules they configure, the last comment for a rule winning. +func eslintConfigComments(src *ast.Source) (map[string]config.Rule, error) { + lx := lexer.New(src) + + overrides := make(map[string]config.Rule) + for { + tok, err := lx.ReadToken() + if err != nil { + return nil, fmt.Errorf("lex %s: %w", src.Name, err) + } + if tok.Kind == lexer.EOF { + return overrides, nil + } + if tok.Kind == lexer.Comment { + maps.Copy(overrides, parseEslintConfigComment(strings.TrimPrefix(tok.Value, "#"))) + } + } +} + +// ruleOverride is one rule configuration declared by a set of files. +type ruleOverride struct { + name string + rule config.Rule + files set.Set[string] +} + +// withConfigComments returns violations adjusted for the inline +// configuration comments in fileSources: a file's violations of a rule it +// reconfigures are replaced with the rule's results under that +// configuration. violations must be the result of running runRules on +// fileSources and parsed. +func withConfigComments(fileSources []*ast.Source, parsed *parsedSchema, violations []Violation) ([]Violation, error) { + overridden := make(map[string]map[string]config.Rule) + for _, src := range fileSources { + // Skip lexing files that do not contain "eslint": they cannot hold a + // configuration comment. + if !strings.Contains(src.Input, "eslint") { + continue + } + overrides, err := eslintConfigComments(src) + if err != nil { + return nil, err + } + if len(overrides) > 0 { + overridden[src.Name] = overrides + } + } + if len(overridden) == 0 { + return violations, nil + } + + var kept []Violation + for _, v := range violations { + if _, ok := overridden[v.File()][v.Rule]; !ok { + kept = append(kept, v) + } + } + + groups := make(map[string]*ruleOverride) + for file, overrides := range overridden { + for name, rule := range overrides { + // %#v keeps option values of different types, such as 1 and "1", + // in separate groups. + key := fmt.Sprintf("%s\x00%s\x00%#v", name, rule.Severity, rule.Options) + if groups[key] == nil { + groups[key] = &ruleOverride{name: name, rule: rule, files: make(set.Set[string])} + } + groups[key].files.Insert(file) + } + } + + for _, key := range slices.Sorted(maps.Keys(groups)) { + g := groups[key] + if g.rule.Severity == config.SeverityOff { + continue + } + results, err := runRules(fileSources, parsed, &config.Lint{Rules: map[string]config.Rule{g.name: g.rule}}) + if err != nil { + return nil, err + } + for _, v := range results { + if v.Rule == g.name && g.files.Contains(v.File()) { + kept = append(kept, v) + } + } + } + return kept, nil +} diff --git a/internal/graphql/lint/eslint_config_comments_test.go b/internal/graphql/lint/eslint_config_comments_test.go new file mode 100644 index 00000000..899ad08f --- /dev/null +++ b/internal/graphql/lint/eslint_config_comments_test.go @@ -0,0 +1,253 @@ +// Copyright 2026 Outreach Corporation. All Rights Reserved. + +// Description: Tests for "# eslint : " inline configuration +// comments. + +// Expected results below were checked against +// @graphql-eslint/eslint-plugin 3.20.1 running under ESLint 8: such a +// comment applies to its whole file wherever it appears, replaces the +// rule's configured options entirely rather than merging with them, and +// the last of several conflicting comments wins. + +package lint + +import ( + "fmt" + "testing" + + "github.com/getoutreach/devbase/v2/internal/graphql/config" + "gotest.tools/v3/assert" +) + +// namingOnlyCfg enables naming-convention the way the stencil-graphql +// template does, and nothing else. +func namingOnlyCfg() *config.Lint { + return &config.Lint{Rules: map[string]config.Rule{ + config.RuleNamingConvention: { + Severity: config.SeverityError, + Options: map[string]any{ + "types": "PascalCase", + "FieldDefinition": "camelCase", + "EnumValueDefinition": "UPPER_CASE", + }, + }, + }} +} + +func TestEslintConfigComments(t *testing.T) { + enumValue := func(v string) string { return fmt.Sprintf(namingFor, v) } + + cases := []struct { + name string + sdl string + want []string + }{ + { + "options are replaced for the whole file, wherever the comment is", + `enum E { + lower + } + # eslint @graphql-eslint/naming-convention: ["error", { FieldDefinition: "camelCase" }] + type t { + Bad_field: String + }`, + []string{`naming-convention Field "Bad_field" should be in camelCase format`}, + }, + { + "off disables the rule for the file", + `# eslint @graphql-eslint/naming-convention: "off" + enum E { + lower + }`, + nil, + }, + { + "a numeric severity of 0 disables the rule", + `# eslint @graphql-eslint/naming-convention: 0 + enum E { + lower + }`, + nil, + }, + { + "a severity alone replaces the options with none", + `# eslint @graphql-eslint/naming-convention: "warn" + enum E { + lower + }`, + nil, + }, + { + "the last of several comments wins", + `# eslint @graphql-eslint/naming-convention: ["error", { EnumValueDefinition: "UPPER_CASE" }] + enum E { + lower + UPPER + } + # eslint @graphql-eslint/naming-convention: ["error", { EnumValueDefinition: "camelCase" }]`, + []string{`naming-convention Enumeration value "UPPER" should be in camelCase format`}, + }, + { + "one comment configures several rules, including one the config leaves off", + `# eslint @graphql-eslint/naming-convention: "off", @graphql-eslint/require-description: ["error", { EnumValueDefinition: true }] + enum E { + lower + }`, + []string{fmt.Sprintf(descFor, "lower")}, + }, + { + "a bare devbase rule name is accepted", + `# eslint naming-convention: "off" + enum E { + lower + }`, + nil, + }, + { + "a Tier 1 rule cannot be configured", + `# eslint @graphql-eslint/known-type-names: "off" + type Foo { a: Missing }`, + []string{"known-type-names Undefined type Missing."}, + }, + { + "a comment that names no known rule is ignored", + `# eslint is great: really + enum E { + lower + }`, + []string{enumValue("lower")}, + }, + { + "text inside a block string is not a comment", + `""" + # eslint @graphql-eslint/naming-convention: "off" + """ + enum E { + lower + }`, + []string{enumValue("lower")}, + }, + { + "a reason after -- is ignored", + `# eslint @graphql-eslint/naming-convention: "off" -- Reason: Existing enum. + enum E { + lower + }`, + nil, + }, + { + "a numeric severity of 1 in a list keeps the options", + `# eslint @graphql-eslint/naming-convention: [1, { EnumValueDefinition: "camelCase" }] + enum E { + UPPER + }`, + []string{`naming-convention Enumeration value "UPPER" should be in camelCase format`}, + }, + { + "a numeric severity of 2 in a list keeps the options", + `# eslint @graphql-eslint/naming-convention: [2, { EnumValueDefinition: "camelCase" }] + enum E { + UPPER + }`, + []string{`naming-convention Enumeration value "UPPER" should be in camelCase format`}, + }, + { + "a one-element list replaces the options with none", + `# eslint @graphql-eslint/naming-convention: ["error"] + enum E { + lower + }`, + nil, + }, + { + "an eslint-disable-next-line still applies under a configuration comment", + `# eslint @graphql-eslint/naming-convention: ["error", { EnumValueDefinition: "camelCase" }] + enum E { + # eslint-disable-next-line @graphql-eslint/naming-convention + UPPER + SHOUT + }`, + []string{`naming-convention Enumeration value "SHOUT" should be in camelCase format`}, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := ruleMessages(t, c.sdl, namingOnlyCfg()) + if c.want == nil { + assert.Equal(t, len(got), 0, "got %v", got) + return + } + assert.DeepEqual(t, got, c.want) + }) + } +} + +// TestEslintConfigCommentIsPerFile confirms a comment in one file does not +// change the rule's configuration in another file. +func TestEslintConfigCommentIsPerFile(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "a.graphql", "# eslint @graphql-eslint/naming-convention: \"off\"\nenum A {\n\tx\n}\n") + b := writeFile(t, dir, "b.graphql", "enum B {\n\ty\n}\n") + + violations, err := Files([]string{a, b}, namingOnlyCfg()) + assert.NilError(t, err) + assert.Equal(t, len(violations), 1) + assert.Equal(t, violations[0].File(), b) +} + +// TestEslintConfigCommentEnablesRuleWithNoConfig confirms a comment can +// turn on a rule when scripts/devbase.yaml configures nothing at all. +func TestEslintConfigCommentEnablesRuleWithNoConfig(t *testing.T) { + got := ruleMessages(t, `# eslint @graphql-eslint/naming-convention: ["error", { EnumValueDefinition: "UPPER_CASE" }] + enum E { + lower + }`, nil) + + assert.DeepEqual(t, got, []string{fmt.Sprintf(namingFor, "lower")}) +} + +// TestEslintConfigCommentsApplyPerConfiguration confirms files that share +// an override, and files that declare a different one, each get results +// from their own configuration. +func TestEslintConfigCommentsApplyPerConfiguration(t *testing.T) { + dir := t.TempDir() + const prefix = `# eslint @graphql-eslint/naming-convention: ["error", { EnumValueDefinition: ` + a := writeFile(t, dir, "a.graphql", prefix+"\"camelCase\" }]\nenum A {\n\tX\n}\n") + b := writeFile(t, dir, "b.graphql", prefix+"\"camelCase\" }]\nenum B {\n\tY\n}\n") + c := writeFile(t, dir, "c.graphql", prefix+"\"snake_case\" }]\nenum C {\n\tZ\n}\n") + + violations, err := Files([]string{a, b, c}, namingOnlyCfg()) + assert.NilError(t, err) + + got := make(map[string]string, len(violations)) + for _, v := range violations { + got[v.File()] = v.Message() + } + assert.DeepEqual(t, got, map[string]string{ + a: `Enumeration value "X" should be in camelCase format`, + b: `Enumeration value "Y" should be in camelCase format`, + c: `Enumeration value "Z" should be in snake_case format`, + }) +} + +func TestSplitTopLevel(t *testing.T) { + cases := []struct { + name string + in string + want []string + }{ + {"empty", ``, []string{""}}, + {"plain entries", `a, b`, []string{"a", " b"}}, + {"commas inside brackets and braces", `a: [1, 2], b: {c: 3, d: 4}`, []string{"a: [1, 2]", " b: {c: 3, d: 4}"}}, + {"comma inside double quotes", `a: "x, y", b`, []string{`a: "x, y"`, " b"}}, + {"comma inside single quotes", `a: 'x, y', b`, []string{`a: 'x, y'`, " b"}}, + {"escaped quote inside double quotes", `a: "say \"hi, there\"", b`, []string{`a: "say \"hi, there\""`, " b"}}, + {"bracket inside a quoted string", `a: "[", b`, []string{`a: "["`, " b"}}, + {"unclosed bracket keeps the rest together", `a: [1, 2`, []string{"a: [1, 2"}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.DeepEqual(t, splitTopLevel(c.in), c.want) + }) + } +} diff --git a/internal/graphql/lint/lint.go b/internal/graphql/lint/lint.go index 0326c3bc..bf0dcc67 100644 --- a/internal/graphql/lint/lint.go +++ b/internal/graphql/lint/lint.go @@ -148,6 +148,21 @@ func FilesFromSources(fileSources []*ast.Source, cfg *config.Lint) ([]Violation, return []Violation{*tier1Violation}, nil } + violations, err := runRules(fileSources, parsed, cfg) + if err != nil { + return nil, err + } + violations, err = withConfigComments(fileSources, parsed, violations) + if err != nil { + return nil, err + } + + return withoutSuppressed(fileSources, violations) +} + +// runRules runs the Tier 2 gap-fill passes and the Tier 3 rules cfg +// enables against a schema that already passed Tier 1 validation. +func runRules(fileSources []*ast.Source, parsed *parsedSchema, cfg *config.Lint) ([]Violation, error) { violations := gapFillDirectivesPerLocation(parsed, cfg) violations = append(violations, gapFillPossibleTypeExtension(parsed, cfg)...) @@ -155,9 +170,7 @@ func FilesFromSources(fileSources []*ast.Source, cfg *config.Lint) ([]Violation, if err != nil { return nil, err } - violations = append(violations, tier3Violations...) - - return withoutSuppressed(fileSources, violations) + return append(violations, tier3Violations...), nil } // withoutOperationDocuments returns sources minus every source that From cbaf567b94ae67d608544a8f1d5cdf67bc2cdf3e Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Mon, 26 Oct 2026 08:38:42 -0700 Subject: [PATCH 2/2] docs(graphql): note that eslint config comments follow ESLint 8 Say in the package comment that inline configuration comments follow ESLint 8, and list the two ways ESLint 9 differs. Assisted-By: claude-sonnet 5.5 via opencode --- internal/graphql/lint/eslint_config_comments.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/graphql/lint/eslint_config_comments.go b/internal/graphql/lint/eslint_config_comments.go index 41143918..994375a2 100644 --- a/internal/graphql/lint/eslint_config_comments.go +++ b/internal/graphql/lint/eslint_config_comments.go @@ -9,7 +9,7 @@ // # eslint @graphql-eslint/naming-convention: ["error", { FieldDefinition: "camelCase" }] // # eslint @graphql-eslint/naming-convention: "off", @graphql-eslint/require-description: "error" // -// As in ESLint, such a comment applies to its whole file wherever it +// As in ESLint 8, such a comment applies to its whole file wherever it // appears, replaces the rule's configured options entirely instead of // merging with them (so a severity alone leaves the rule with no // options), and the last comment for a rule wins. A rule is named with @@ -18,6 +18,11 @@ // written as YAML flow syntax so unquoted keys work as they do in ESLint. // A "-- reason" suffix is ignored. // +// ESLint 9 differs: a severity-only comment keeps the configured +// options, and the first comment for a rule wins while later ones are +// reported as errors. See +// https://eslint.org/docs/latest/use/migrate-to-9.0.0. +// // Only the rule's on/off state and options can differ per file: a file's // "warn" or "error" still reports at the severity scripts/devbase.yaml // gives the rule, or at "error" for a rule that file leaves unconfigured