diff --git a/internal/graphql/lint/hashtag.go b/internal/graphql/lint/hashtag.go index 7809f1a8..03672cc7 100644 --- a/internal/graphql/lint/hashtag.go +++ b/internal/graphql/lint/hashtag.go @@ -21,7 +21,9 @@ // example "type Query { # trailing"). noHashtagDescriptionViolations // recovers both distinctions the same way @graphql-eslint's own rule // does: by comparing line numbers, using trailingCommentStarts for the -// second one. +// second one. Like that rule, it also skips a comment whose text starts +// with "eslint", so a directive such as "# eslint-disable ..." above a +// definition is not mistaken for a description. // // Unlike require-description, no-hashtag-description has no per-kind // options: it applies unconditionally to every site groupDescriptionSites @@ -37,6 +39,7 @@ package lint import ( "fmt" + "strings" "github.com/getoutreach/devbase/v2/internal/graphql/config" "github.com/vektah/gqlparser/v2/ast" @@ -73,6 +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") +} + // noHashtagDescriptionViolations reports a RuleNoHashtagDescription // violation for every site in sites whose afterDescriptionComment is // attached to it with no blank line in between. Two comment tokens can @@ -90,7 +100,12 @@ func noHashtagDescriptionViolations(sites []descriptionSite, trailing map[int]bo continue } + // 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) { + continue + } if len(group.List) == 1 && trailing[last.Position.Start] { continue } diff --git a/internal/graphql/lint/hashtag_test.go b/internal/graphql/lint/hashtag_test.go index d0bf1806..9583521d 100644 --- a/internal/graphql/lint/hashtag_test.go +++ b/internal/graphql/lint/hashtag_test.go @@ -4,16 +4,12 @@ // Cases below are ported from @graphql-eslint/eslint-plugin's own test // suite (packages/plugin/src/rules/no-hashtag-description/index.test.ts, -// graphql-hive/graphql-eslint@master as of 2026-08-22), with two -// adaptations. Cases exercising an OperationDefinition or +// graphql-hive/graphql-eslint@master as of 2026-08-22), with one +// adaptation: cases exercising an OperationDefinition or // FragmentDefinition document are dropped, since devbase graphql lint // has no notion of either: it only ever parses standalone *.graphql // schema files (see lint.go's Files), never query/mutation/subscription/ -// fragment documents. And the "# eslint-disable-next-line" case is -// inverted from valid to invalid: devbase has no inline-suppression -// comment syntax anywhere in its lint package, so to it that line is -// just an ordinary attached comment like any other; see -// TestNoHashtagDescriptionInvalidCases' last case. +// fragment documents. package lint @@ -89,6 +85,28 @@ func TestNoHashtagDescriptionValidCases(t *testing.T) { ): User }`, }, + { + "eslint directive immediately before the type", + `# eslint-disable-next-line + type Query { + foo: String + }`, + }, + { + "eslint directive with no space after the hash", + `#eslint-disable @graphql-eslint/naming-convention -- Reason: Existing enum. + enum Kind { + Foo + } + #eslint-enable @graphql-eslint/naming-convention`, + }, + { + "eslint directive immediately before a field", + `type Query { + # eslint-disable-next-line + foo: String + }`, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -146,8 +164,16 @@ func TestNoHashtagDescriptionInvalidCases(t *testing.T) { }`, }, { - "a comment shaped like an eslint directive is still an ordinary comment to devbase", - `# eslint-disable-next-line + "only the last comment of a group decides, and it is prose", + `# eslint-foo + # prose + type Query { + foo: String + }`, + }, + { + "comment that only mentions eslint is still a description", + `# Not an eslint directive type Query { foo: String }`, @@ -171,3 +197,14 @@ func TestNoHashtagDescriptionDisabledByDefault(t *testing.T) { assert.NilError(t, err) assert.Equal(t, len(violations), 0) } + +// TestNoHashtagDescriptionIgnoresGroupEndingInDirective confirms a group of +// comments is judged by its last comment only, as @graphql-eslint does: +// prose followed by a directive is not reported. +func TestNoHashtagDescriptionIgnoresGroupEndingInDirective(t *testing.T) { + path := writeFile(t, t.TempDir(), "schema.graphql", "# prose\n# eslint-foo\ntype Query {\n\tfoo: String\n}\n") + + violations, err := Files([]string{path}, enableRule(config.RuleNoHashtagDescription)) + assert.NilError(t, err) + assert.Equal(t, len(violations), 0) +} diff --git a/internal/graphql/lint/typename_prefix_test.go b/internal/graphql/lint/typename_prefix_test.go index b5bd0615..a5e6aa35 100644 --- a/internal/graphql/lint/typename_prefix_test.go +++ b/internal/graphql/lint/typename_prefix_test.go @@ -6,11 +6,8 @@ // suite (packages/plugin/tests/no-typename-prefix.spec.ts, // @graphql-eslint/eslint-plugin@3.13.1, a version still in real use // downstream), dropping the "eslint-disable-next-line" -// valid case: devbase has no inline-suppression comment syntax -// anywhere in its lint package, and unlike no-hashtag-description's -// own such case (hashtag_test.go), that comment has nothing to do with -// no-typename-prefix's own logic, so there is nothing meaningful to -// invert it into. +// valid case: that comment has nothing to do with no-typename-prefix's +// own logic, so there is nothing meaningful to test with it. package lint