diff --git a/.changeset/empty-horses-relate.md b/.changeset/empty-horses-relate.md new file mode 100644 index 00000000000..4048ca86a54 --- /dev/null +++ b/.changeset/empty-horses-relate.md @@ -0,0 +1,5 @@ +--- +'@graphql-eslint/eslint-plugin': patch +--- + +fix false positive cases for `require-import-fragment` on Windows, when `graphql-config`'s `documents` key contained glob pattern => source file path of document contained always forward slashes diff --git a/packages/plugin/__tests__/mocks/import-fragments/bar-fragment.gql b/packages/plugin/__tests__/mocks/import-fragments/fragments/bar-fragment.gql similarity index 100% rename from packages/plugin/__tests__/mocks/import-fragments/bar-fragment.gql rename to packages/plugin/__tests__/mocks/import-fragments/fragments/bar-fragment.gql diff --git a/packages/plugin/__tests__/mocks/import-fragments/foo-fragment.gql b/packages/plugin/__tests__/mocks/import-fragments/fragments/foo-fragment.gql similarity index 100% rename from packages/plugin/__tests__/mocks/import-fragments/foo-fragment.gql rename to packages/plugin/__tests__/mocks/import-fragments/fragments/foo-fragment.gql diff --git a/packages/plugin/__tests__/mocks/import-fragments/invalid-query-default.gql b/packages/plugin/__tests__/mocks/import-fragments/invalid-query-default.gql index a3f5a5f7302..a1c14d55c45 100644 --- a/packages/plugin/__tests__/mocks/import-fragments/invalid-query-default.gql +++ b/packages/plugin/__tests__/mocks/import-fragments/invalid-query-default.gql @@ -1,4 +1,4 @@ -#import 'bar-fragment.gql' +#import './fragments/bar-fragment.gql' query { foo { ...FooFields diff --git a/packages/plugin/__tests__/mocks/import-fragments/invalid-query.gql b/packages/plugin/__tests__/mocks/import-fragments/invalid-query.gql index 6fdcc59f030..f5e4fd6a8cc 100644 --- a/packages/plugin/__tests__/mocks/import-fragments/invalid-query.gql +++ b/packages/plugin/__tests__/mocks/import-fragments/invalid-query.gql @@ -1,4 +1,4 @@ -#import FooFields from "bar-fragment.gql" +#import FooFields from "./fragments/bar-fragment.gql" query { foo { ...FooFields diff --git a/packages/plugin/__tests__/mocks/import-fragments/valid-query-default.gql b/packages/plugin/__tests__/mocks/import-fragments/valid-query-default.gql index 0470e1b771b..2e4a42730f4 100644 --- a/packages/plugin/__tests__/mocks/import-fragments/valid-query-default.gql +++ b/packages/plugin/__tests__/mocks/import-fragments/valid-query-default.gql @@ -1,6 +1,6 @@ # Imports could have extra whitespace and double/single quotes -# import 'foo-fragment.gql' +# import './fragments/foo-fragment.gql' query { foo { diff --git a/packages/plugin/__tests__/mocks/import-fragments/valid-query.gql b/packages/plugin/__tests__/mocks/import-fragments/valid-query.gql index 11d46b7cd37..8018aa77226 100644 --- a/packages/plugin/__tests__/mocks/import-fragments/valid-query.gql +++ b/packages/plugin/__tests__/mocks/import-fragments/valid-query.gql @@ -1,6 +1,6 @@ # Imports could have extra whitespace and double/single quotes -# import FooFields from "foo-fragment.gql" +# import FooFields from "./fragments/foo-fragment.gql" query { foo { diff --git a/packages/plugin/src/documents.ts b/packages/plugin/src/documents.ts index 7399c9593e1..8b98b57729b 100644 --- a/packages/plugin/src/documents.ts +++ b/packages/plugin/src/documents.ts @@ -1,4 +1,4 @@ -import { resolve } from 'node:path'; +import path from 'node:path'; import debugFactory from 'debug'; import fg from 'fast-glob'; import { GraphQLProjectConfig } from 'graphql-config'; @@ -15,13 +15,18 @@ const handleVirtualPath = (documents: Source[]): Source[] => { return documents.map(source => { const location = source.location!; if (['.gql', '.graphql'].some(extension => location.endsWith(extension))) { - return source; + return { + ...source, + // When using glob pattern e.g. `**/*.gql` location contains always forward slashes even on + // Windows + location: path.resolve(location), + }; } filepathMap[location] ??= -1; const index = (filepathMap[location] += 1); return { ...source, - location: resolve(location, `${index}_document.graphql`), + location: path.resolve(location, `${index}_document.graphql`), }; }); }; diff --git a/packages/plugin/src/rules/require-import-fragment/index.test.ts b/packages/plugin/src/rules/require-import-fragment/index.test.ts index 426c096520e..0c2ba5b3920 100644 --- a/packages/plugin/src/rules/require-import-fragment/index.test.ts +++ b/packages/plugin/src/rules/require-import-fragment/index.test.ts @@ -1,55 +1,60 @@ -import { join } from 'node:path'; -import { CWD } from '@/utils.js'; +import path from 'node:path'; import { ParserOptionsForTests, ruleTester } from '../../../__tests__/test-utils.js'; import { rule } from './index.js'; -function withMocks({ name, filename, errors }: { name: string; filename: string; errors?: any }) { +function withMocks({ + name, + filename, + errors, + only = false, +}: { + name: string; + filename: string; + errors?: any; + only?: boolean; +}) { return { name, filename, code: ruleTester.fromMockFile(filename.split('mocks')[1]), parserOptions: { graphQLConfig: { - documents: [ - filename, - join(CWD, '__tests__', 'mocks', 'import-fragments', 'foo-fragment.gql'), - join(CWD, '__tests__', 'mocks', 'import-fragments', 'bar-fragment.gql'), - ], + documents: [filename, './__tests__/mocks/import-fragments/fragments/**/*.gql'], }, } satisfies ParserOptionsForTests, errors, + only, }; } - ruleTester.run('require-import-fragment', rule, { valid: [ withMocks({ name: 'should not report with named import', - filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'valid-query.gql'), + filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'valid-query.gql'), }), withMocks({ name: 'should not report with default import', - filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'valid-query-default.gql'), + filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'valid-query-default.gql'), }), withMocks({ name: 'should not report fragments from the same file', - filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'same-file.gql'), + filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'same-file.gql'), }), ], invalid: [ withMocks({ name: 'should report with named import', - filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'invalid-query.gql'), + filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'invalid-query.gql'), errors: [{ message: 'Expected "FooFields" fragment to be imported.' }], }), withMocks({ name: 'should report with default import', - filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'invalid-query-default.gql'), + filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'invalid-query-default.gql'), errors: [{ message: 'Expected "FooFields" fragment to be imported.' }], }), withMocks({ name: 'should report fragments when there are no import expressions', - filename: join(CWD, '__tests__', 'mocks', 'import-fragments', 'missing-import.gql'), + filename: path.resolve('__tests__', 'mocks', 'import-fragments', 'missing-import.gql'), errors: [{ message: 'Expected "FooFields" fragment to be imported.' }], }), ], diff --git a/packages/plugin/src/rules/require-import-fragment/index.ts b/packages/plugin/src/rules/require-import-fragment/index.ts index f9710845578..41a27c7220e 100644 --- a/packages/plugin/src/rules/require-import-fragment/index.ts +++ b/packages/plugin/src/rules/require-import-fragment/index.ts @@ -2,7 +2,7 @@ import path from 'node:path'; import { NameNode } from 'graphql'; import { GraphQLESTreeNode } from '../../estree-converter/index.js'; import { GraphQLESLintRule } from '../../types.js'; -import { requireSiblingsOperations } from '../../utils.js'; +import { requireSiblingsOperations, slash } from '../../utils.js'; const RULE_ID = 'require-import-fragment'; const SUGGESTION_ID = 'add-import-expression'; @@ -89,21 +89,21 @@ export const rule: GraphQLESLintRule = { const extractedImportPath = comment.value.match(/(["'])((?:\1|.)*?)\1/)?.[2]; if (!extractedImportPath) continue; - - const importPath = path.join(path.dirname(filePath), extractedImportPath); + const importPath = path.join(filePath, '..', extractedImportPath); const hasInSiblings = fragmentsFromSiblings.some( source => source.filePath === importPath, ); if (hasInSiblings) return; } - const fragmentInSameFile = fragmentsFromSiblings.some( source => source.filePath === filePath, ); if (fragmentInSameFile) return; - const suggestedFilePaths = fragmentsFromSiblings.length - ? fragmentsFromSiblings.map(o => path.relative(path.dirname(filePath), o.filePath)) + ? fragmentsFromSiblings.map(o => + // Use always forward slash for suggested import path + slash(path.relative(path.dirname(filePath), o.filePath)), + ) : ['CHANGE_ME.graphql']; context.report({ diff --git a/packages/plugin/src/rules/require-import-fragment/snapshot.md b/packages/plugin/src/rules/require-import-fragment/snapshot.md index 27391745144..c0ced7a5b9c 100644 --- a/packages/plugin/src/rules/require-import-fragment/snapshot.md +++ b/packages/plugin/src/rules/require-import-fragment/snapshot.md @@ -18,7 +18,7 @@ exports[`require-import-fragment > invalid > should report fragments when there #### 💡 Suggestion: Add import expression for "FooFields". - 1 | # import FooFields from 'foo-fragment.gql' + 1 | # import FooFields from 'fragments/foo-fragment.gql' 2 | { 3 | foo { 4 | ...FooFields @@ -29,7 +29,7 @@ exports[`require-import-fragment > invalid > should report fragments when there exports[`require-import-fragment > invalid > should report with default import 1`] = ` #### ⌨️ Code - 1 | #import 'bar-fragment.gql' + 1 | #import './fragments/bar-fragment.gql' 2 | query { 3 | foo { 4 | ...FooFields @@ -45,8 +45,8 @@ exports[`require-import-fragment > invalid > should report with default import 1 #### 💡 Suggestion: Add import expression for "FooFields". - 1 | # import FooFields from 'foo-fragment.gql' - 2 | #import 'bar-fragment.gql' + 1 | # import FooFields from 'fragments/foo-fragment.gql' + 2 | #import './fragments/bar-fragment.gql' 3 | query { 4 | foo { 5 | ...FooFields @@ -57,7 +57,7 @@ exports[`require-import-fragment > invalid > should report with default import 1 exports[`require-import-fragment > invalid > should report with named import 1`] = ` #### ⌨️ Code - 1 | #import FooFields from "bar-fragment.gql" + 1 | #import FooFields from "./fragments/bar-fragment.gql" 2 | query { 3 | foo { 4 | ...FooFields @@ -73,8 +73,8 @@ exports[`require-import-fragment > invalid > should report with named import 1`] #### 💡 Suggestion: Add import expression for "FooFields". - 1 | # import FooFields from 'foo-fragment.gql' - 2 | #import FooFields from "bar-fragment.gql" + 1 | # import FooFields from 'fragments/foo-fragment.gql' + 2 | #import FooFields from "./fragments/bar-fragment.gql" 3 | query { 4 | foo { 5 | ...FooFields diff --git a/packages/plugin/src/utils.ts b/packages/plugin/src/utils.ts index bbf63523b4d..40f1eea5bf3 100644 --- a/packages/plugin/src/utils.ts +++ b/packages/plugin/src/utils.ts @@ -49,8 +49,7 @@ export const logger = { export const slash = (path: string): string => path.replaceAll('\\', '/'); // Match slash or backslash for Windows -// eslint-disable-next-line no-useless-escape -export const VIRTUAL_DOCUMENT_REGEX = /[\/\\]\d+_document.graphql$/; +export const VIRTUAL_DOCUMENT_REGEX = /[/\\]\d+_document.graphql$/; export const CWD = process.cwd();