Skip to content

fix(graphql): apply root field and base rules together in naming-convention #1256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
malept wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(graphql): apply root field and base rules together in naming-convention #1256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 22 additions & 20 deletions internal/graphql/lint/naming_convention.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,24 +224,26 @@ func parseNamingConventionOptions(opts map[string]any) namingConventionOptions {
return o
}

// ruleFor resolves the namingRule that applies to s, if any: s's own
// root-field selector (if it has one) or base selector, whichever
// scripts/devbase.yaml actually configured, falling back to the
// shared "types" rule only for a type-definition site whose own kind
// key was not configured individually.
func (o namingConventionOptions) ruleFor(s *namingSite) (namingRule, bool) {
// rulesFor resolves the namingRules that apply to s, in check order:
// its base selector (falling back to the shared "types" rule only for
// a type-definition site whose own kind key was not configured
// individually), then its root-field selector, if configured. The
// root-field rule adds to the base rule instead of replacing it, because
// @graphql-eslint registers one listener per option key and runs the
// least specific first.
func (o namingConventionOptions) rulesFor(s *namingSite) []namingRule {
var rules []namingRule
if r, ok := o.selectors[s.baseSelector]; ok {
rules = append(rules, r)
} else if s.typeDefKindKey != "" && o.types != nil {
rules = append(rules, *o.types)
}
if s.rootSelector != "" {
if r, ok := o.selectors[s.rootSelector]; ok {
return r, true
rules = append(rules, r)
}
}
if r, ok := o.selectors[s.baseSelector]; ok {
return r, true
}
if s.typeDefKindKey != "" && o.types != nil {
return *o.types, true
}
return namingRule{}, false
return rules
}

// namingSite is one name in a schema file that naming-convention
Expand Down Expand Up @@ -384,15 +386,15 @@ func namingRuleMessage(name string, rule *namingRule) string {
}

// namingConventionViolations reports a RuleNamingConvention violation
// for every site in sites that either has a configured namingRule it
// fails (per namingRuleMessage) or an underscore opts forbids, in
// document order. Both checks run for the same site independently, and
// neither one skips the other, matching @graphql-eslint's own behavior
// of registering them as separate listeners.
// for each configured namingRule a site fails (per namingRuleMessage)
// and for each underscore opts forbids, in document order. A site can
// fail several of these. The checks run independently, and none skips
// another, matching @graphql-eslint's own behavior of registering them
// as separate listeners.
func namingConventionViolations(sites []namingSite, opts namingConventionOptions) []Violation {
var violations []Violation
for _, s := range sites {
if rule, ok := opts.ruleFor(&s); ok {
for _, rule := range opts.rulesFor(&s) {
if msg := namingRuleMessage(s.name, &rule); msg != "" {
violations = append(violations, Violation{
err: gqlerror.ErrorPosf(s.pos, "%s %q %s", s.kindLabel, s.name, msg),
Expand Down
69 changes: 61 additions & 8 deletions internal/graphql/lint/naming_convention_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,6 @@ func TestNamingConventionValidCases(t *testing.T) {
"EnumValueDefinition": map[string]any{"style": "UPPER_CASE", "prefix": "ENUM_VALUE_"},
},
},
{
"Query field selector overrides the general FieldDefinition rule",
`type One { fieldA: String } type Query { QUERY_A(id: ID!): String }`,
map[string]any{
"FieldDefinition[parent.name.value=Query]": map[string]any{"style": "UPPER_CASE", "prefix": "QUERY"},
"FieldDefinition": map[string]any{"style": "camelCase", "prefix": "field"},
},
},
{
"should ignore fields matching ignorePattern",
`type Test { EU: ID EUIntlFlag: ID UPC: ID }`,
Expand Down Expand Up @@ -202,6 +194,67 @@ func TestNamingConventionInvalidCases(t *testing.T) {
`Field "getC" should not have "get" prefix`,
},
},
{
// @graphql-eslint registers one listener per option key, so
// the Query selector adds to the FieldDefinition rule
// instead of replacing it (verified against
// @graphql-eslint/eslint-plugin 3.13.1 and 3.20.1).
"Query field selector adds to the general FieldDefinition rule",
`type One { fieldA: String } type Query { QUERY_A(id: ID!): String }`,
opts(map[string]any{
"FieldDefinition[parent.name.value=Query]": map[string]any{"style": "UPPER_CASE", "prefix": "QUERY"},
"FieldDefinition": map[string]any{"style": "camelCase", "prefix": "field"},
}),
[]string{`Field "QUERY_A" should have "field" prefix`},
},
{
"a root field's casing is still checked when only forbiddenPrefixes is set on the Query selector",
`type Query { Bad_name: String, getThing: String }`,
opts(map[string]any{
"FieldDefinition": map[string]any{"style": "camelCase"},
"FieldDefinition[parent.name.value=Query]": map[string]any{"forbiddenPrefixes": []any{"get"}},
}),
[]string{
`Field "Bad_name" should be in camelCase format`,
`Field "getThing" should not have "get" prefix`,
},
},
{
"Mutation and Subscription fields get both rules too",
`type Query { a: String } type Mutation { Bad_m: String, getM: String } type Subscription { Bad_s: String, getS: String }`,
opts(map[string]any{
"FieldDefinition": map[string]any{"style": "camelCase"},
"FieldDefinition[parent.name.value=Mutation]": map[string]any{"forbiddenPrefixes": []any{"get"}},
"FieldDefinition[parent.name.value=Subscription]": map[string]any{"forbiddenPrefixes": []any{"get"}},
}),
[]string{
`Field "Bad_m" should be in camelCase format`,
`Field "getM" should not have "get" prefix`,
`Field "Bad_s" should be in camelCase format`,
`Field "getS" should not have "get" prefix`,
},
},
{
"the types rule never applies to a field, only to type definitions",
`type Query { getX: String, lower: String }`,
opts(map[string]any{
"types": "PascalCase",
"FieldDefinition[parent.name.value=Query]": map[string]any{"forbiddenPrefixes": []any{"get"}},
}),
[]string{`Field "getX" should not have "get" prefix`},
},
{
"a root field that breaks both rules is reported for each, general rule first",
`type Query { Get_x: String }`,
opts(map[string]any{
"FieldDefinition": map[string]any{"style": "camelCase"},
"FieldDefinition[parent.name.value=Query]": map[string]any{"forbiddenPrefixes": []any{"Get"}},
}),
[]string{
`Field "Get_x" should be in camelCase format`,
`Field "Get_x" should not have "Get" prefix`,
},
},
})
}

Expand Down
Loading