diff --git a/docs/graphql-linting.md b/docs/graphql-linting.md index 25186d7..4629980 100644 --- a/docs/graphql-linting.md +++ b/docs/graphql-linting.md @@ -77,23 +77,24 @@ The custom GraphQL linter includes the following built-in rules: ### Core GraphQL Rules -| Rule | Severity | Description | -| ---------------------------- | -------- | ------------------------------------- | -| `no-anonymous-operations` | error | Prevent anonymous GraphQL operations | -| `no-duplicate-fields` | error | Prevent duplicate field definitions | -| `require-description` | warn | Require descriptions for types/fields | -| `require-deprecation-reason` | warn | Require reason for deprecated fields | -| `field-naming-convention` | warn | Enforce camelCase field naming | -| `root-fields-nullable` | warn | Suggest nullable root field types | +| Rule | Severity | Description | +| ---------------------------- | -------- | --------------------------------------------------- | +| `no-anonymous-operations` | error | Prevent anonymous GraphQL operations | +| `no-duplicate-fields` | error | Prevent duplicate field definitions | +| `require-description` | warn | Require descriptions for types/fields | +| `require-deprecation-reason` | warn | Require reason for deprecated fields | +| `node-interface-structure` | error | Node interface must have exactly one field: id: ID! | +| `field-naming-convention` | warn | Enforce camelCase field naming | +| `root-fields-nullable` | warn | Suggest nullable root field types | ### Pagination Rules -| Rule | Severity | Description | -| ------------------------------ | -------- | ---------------------------------------------- | -| `connection-structure` | error | Ensure Connection types have edges/pageInfo | -| `edge-structure` | error | Ensure Edge types have node/cursor fields | -| `connection-arguments` | warn | Suggest pagination arguments for connections | -| `pagination-argument-types` | error | Enforce correct types for pagination arguments | +| Rule | Severity | Description | +| --------------------------- | -------- | ---------------------------------------------- | +| `connection-structure` | error | Ensure Connection types have edges/pageInfo | +| `edge-structure` | error | Ensure Edge types have node/cursor fields | +| `connection-arguments` | warn | Suggest pagination arguments for connections | +| `pagination-argument-types` | error | Enforce correct types for pagination arguments | ### Rule Details @@ -101,6 +102,7 @@ The custom GraphQL linter includes the following built-in rules: - **no-duplicate-fields**: Prevents duplicate field definitions within the same type - **require-description**: Suggests adding descriptions to types and fields for better documentation - **require-deprecation-reason**: Ensures deprecated fields include a reason for deprecation +- **node-interface-structure**: Ensures Node interface follows the standard pattern with exactly one field: `id: ID!` - **field-naming-convention**: Enforces camelCase naming for field names (ignores special fields like `__typename`) - **root-fields-nullable**: Suggests making root type fields nullable for better error handling - **connection-structure**: Ensures Connection types follow the Relay pagination pattern with `edges` and `pageInfo` fields diff --git a/src/services/graphqlLinter.ts b/src/services/graphqlLinter.ts index 2ad9008..91d447b 100644 --- a/src/services/graphqlLinter.ts +++ b/src/services/graphqlLinter.ts @@ -12,6 +12,7 @@ import { ObjectTypeDefinitionNode, FieldDefinitionNode, OperationDefinitionNode, + InterfaceTypeDefinitionNode, } from "graphql"; import { logger } from "./logger"; import { StepZenError } from "../errors"; @@ -60,7 +61,7 @@ export class GraphQLLinterService { private initializeRules(): void { // Get enabled rules from configuration const config = vscode.workspace.getConfiguration("stepzen"); - const enabledRules = config.get("graphqlLintRules", { + const defaultRules = { "no-anonymous-operations": true, "no-duplicate-fields": true, "require-description": true, @@ -71,7 +72,15 @@ export class GraphQLLinterService { "edge-structure": true, "connection-arguments": true, "pagination-argument-types": true, - }); + "node-interface-structure": true, + }; + + // In test environment, config might be undefined, so use defaults + let enabledRules = defaultRules; + if (config) { + const configRules = config.get("graphqlLintRules", {}); + enabledRules = { ...defaultRules, ...configRules }; + } const allRules: GraphQLLintRule[] = []; @@ -535,6 +544,92 @@ export class GraphQLLinterService { }); } + // Rule: Node interface must have exactly one field: id: ID! + if (enabledRules["node-interface-structure"]) { + allRules.push({ + name: "node-interface-structure", + severity: "error" as const, + check: (ast: DocumentNode): GraphQLLintIssue[] => { + const issues: GraphQLLintIssue[] = []; + + visit(ast, { + // Check for Node defined as object type (should be interface) + ObjectTypeDefinition(node: ObjectTypeDefinitionNode) { + if (node.name.value === "Node" && node.loc) { + issues.push({ + message: "Node must be defined as an interface, not a type", + line: node.loc.startToken.line, + column: node.loc.startToken.column, + endLine: node.loc.endToken.line, + endColumn: node.loc.endToken.column, + rule: "node-interface-structure", + severity: "error", + }); + } + }, + // Check for Node interface structure + InterfaceTypeDefinition(node: InterfaceTypeDefinitionNode) { + if (node.name.value === "Node") { + const fields = node.fields || []; + + // Not exactly one field: error + if (fields.length !== 1 && node.loc) { + issues.push({ + message: + "Node interface must have exactly one field: id: ID!", + line: node.loc.startToken.line, + column: node.loc.startToken.column, + endLine: node.loc.endToken.line, + endColumn: node.loc.endToken.column, + rule: "node-interface-structure", + severity: "error", + }); + } + + // Check the field name and type + if (fields.length >= 1) { + const idField = fields[0]; + + if (idField.name.value !== "id" && idField.loc) { + issues.push({ + message: "Node interface must have a field named 'id'", + line: idField.loc.startToken.line, + column: idField.loc.startToken.column, + endLine: idField.loc.endToken.line, + endColumn: idField.loc.endToken.column, + rule: "node-interface-structure", + severity: "error", + }); + } + + // Check if the field type is ID! + const fieldType = idField.type; + + const isNonNullID = + fieldType.kind === "NonNullType" && + fieldType.type.kind === "NamedType" && + fieldType.type.name.value === "ID"; + if (!isNonNullID && idField.loc) { + issues.push({ + message: + "Node interface 'id' field must be of type 'ID!'", + line: idField.loc.startToken.line, + column: idField.loc.startToken.column, + endLine: idField.loc.endToken.line, + endColumn: idField.loc.endToken.column, + rule: "node-interface-structure", + severity: "error", + }); + } + } + } + }, + }); + return issues; + }, + }); + } + this.rules = allRules; } diff --git a/src/test/unit/graphqlLinter.test.ts b/src/test/unit/graphqlLinter.test.ts index f278091..c768a0b 100644 --- a/src/test/unit/graphqlLinter.test.ts +++ b/src/test/unit/graphqlLinter.test.ts @@ -560,6 +560,7 @@ suite("GraphQL Linter Test Suite", () => { "edge-structure": false, "connection-arguments": false, "pagination-argument-types": false, + "node-interface-structure": false, }; } return defaultValue; diff --git a/src/test/unit/services/graphqlLinter.test.ts b/src/test/unit/services/graphqlLinter.test.ts new file mode 100644 index 0000000..09d022c --- /dev/null +++ b/src/test/unit/services/graphqlLinter.test.ts @@ -0,0 +1,195 @@ +/** + * Copyright IBM Corp. 2025 + * Assisted by CursorAI + */ + +import * as assert from "assert"; +import { GraphQLLinterService } from "../../../services/graphqlLinter"; + +suite("GraphQL Linter Test Suite", () => { + let linter: GraphQLLinterService; + + suiteSetup(() => { + linter = new GraphQLLinterService(); + }); + + suiteTeardown(() => { + linter.dispose(); + }); + + test("should initialize GraphQL linter service", async () => { + await linter.initialize(); + assert.ok( + linter.getDiagnosticCollection(), + "Diagnostic collection should be created", + ); + }); + + test("should create diagnostic collection with correct name", () => { + const collection = linter.getDiagnosticCollection(); + assert.strictEqual( + collection.name, + "stepzen-graphql-lint", + "Diagnostic collection should have correct name", + ); + }); + + test("should clear diagnostics", () => { + const collection = linter.getDiagnosticCollection(); + linter.clearDiagnostics(); + let filesWithIssues = 0; + collection.forEach(() => { + filesWithIssues++; + }); + assert.strictEqual(filesWithIssues, 0, "Should clear all diagnostics"); + }); + + test("should dispose service correctly", () => { + linter.dispose(); + // Test that dispose doesn't throw errors + assert.ok(true, "Dispose should complete without errors"); + }); + + test("should detect Node interface with wrong field name", async () => { + await linter.initialize(); + const testFile = "test-node-wrong-field.graphql"; + const content = "interface Node { identifier: ID! }"; + + const fs = require("fs"); + fs.writeFileSync(testFile, content); + + try { + const diagnostics = await linter.lintFile(testFile); + + const nodeInterfaceErrors = diagnostics.filter( + (d) => + d.message.includes("Node interface must have a field named 'id'") && + d.code === "node-interface-structure", + ); + + assert.strictEqual( + nodeInterfaceErrors.length, + 1, + "Should detect Node interface with wrong field name", + ); + } finally { + fs.unlinkSync(testFile); + } + }); + + test("should detect Node interface with wrong field type", async () => { + await linter.initialize(); + const testFile = "test-node-wrong-type.graphql"; + const content = "interface Node { id: String! }"; + + const fs = require("fs"); + fs.writeFileSync(testFile, content); + + try { + const diagnostics = await linter.lintFile(testFile); + + const nodeInterfaceErrors = diagnostics.filter( + (d) => + d.message.includes( + "Node interface 'id' field must be of type 'ID!'", + ) && d.code === "node-interface-structure", + ); + + assert.strictEqual( + nodeInterfaceErrors.length, + 1, + "Should detect Node interface with wrong field type", + ); + } finally { + fs.unlinkSync(testFile); + } + }); + + test("should accept correct Node interface", async () => { + await linter.initialize(); + const testFile = "test-node-correct.graphql"; + const content = "interface Node { id: ID! }"; + + const fs = require("fs"); + fs.writeFileSync(testFile, content); + + try { + const diagnostics = await linter.lintFile(testFile); + const nodeInterfaceErrors = diagnostics.filter( + (d) => d.code === "node-interface-structure", + ); + + assert.strictEqual( + nodeInterfaceErrors.length, + 0, + "Should accept correct Node interface", + ); + } finally { + fs.unlinkSync(testFile); + } + }); + + test("should not affect other interfaces", async () => { + await linter.initialize(); + const testFile = "test-other-interfaces.graphql"; + const content = ` + interface User { + id: ID! + name: String! + email: String! + } + + interface Product { + id: ID! + name: String! + price: Float! + } + `; + + const fs = require("fs"); + fs.writeFileSync(testFile, content); + + try { + const diagnostics = await linter.lintFile(testFile); + const nodeInterfaceErrors = diagnostics.filter( + (d) => d.code === "node-interface-structure", + ); + + assert.strictEqual( + nodeInterfaceErrors.length, + 0, + "Should not affect other interfaces", + ); + } finally { + fs.unlinkSync(testFile); + } + }); + + test("should detect Node defined as object type instead of interface", async () => { + await linter.initialize(); + const testFile = "test-node-as-type.graphql"; + const content = "type Node { id: ID! }"; + + const fs = require("fs"); + fs.writeFileSync(testFile, content); + + try { + const diagnostics = await linter.lintFile(testFile); + + const nodeInterfaceErrors = diagnostics.filter( + (d) => + d.message.includes( + "Node must be defined as an interface, not a type", + ) && d.code === "node-interface-structure", + ); + + assert.strictEqual( + nodeInterfaceErrors.length, + 1, + "Should detect Node defined as object type instead of interface", + ); + } finally { + fs.unlinkSync(testFile); + } + }); +}); diff --git a/test-node-interface.graphql b/test-node-interface.graphql new file mode 100644 index 0000000..f7a17e9 --- /dev/null +++ b/test-node-interface.graphql @@ -0,0 +1,45 @@ +# Test file for Node interface rule + +# ❌ Wrong: Node interface with no fields +interface Node { +} + +# ❌ Wrong: Node interface with wrong field name +interface Node { + identifier: ID! +} + +# ❌ Wrong: Node interface with wrong field type +interface Node { + id: String! +} + +# ❌ Wrong: Node interface with nullable ID +interface Node { + id: ID +} + +# ❌ Wrong: Node interface with multiple fields +interface Node { + id: ID! + name: String! +} + +# ✅ Correct: Node interface with exactly id: ID! +interface Node { + id: ID! +} + +# Test that other interfaces are not affected +interface User { + id: ID! + name: String! + email: String! +} + +# Test that types are not affected +type Product { + id: ID! + name: String! + price: Float! +} \ No newline at end of file