Skip to content

Commit

Permalink
4️⃣ fix false negative case in unique-fragment-name rule (#535)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dimitri POSTOLOV committed Jul 30, 2021
1 parent 5e8ebd8 commit c0b12a5
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/four-coats-vanish.md
@@ -0,0 +1,5 @@
---
'@graphql-eslint/eslint-plugin': patch
---

fix(siblings): return virtual path for code files instead of real path
8 changes: 5 additions & 3 deletions packages/plugin/src/rules/unique-fragment-name.ts
Expand Up @@ -22,11 +22,11 @@ export const checkNode = (
const siblings = requireSiblingsOperations(ruleName, context);
const siblingDocuments: (FragmentSource | OperationSource)[] =
node.kind === Kind.FRAGMENT_DEFINITION ? siblings.getFragment(documentName) : siblings.getOperation(documentName);
const realFilepath = getOnDiskFilepath(context.getFilename());
const filepath = context.getFilename();

const conflictingDocuments = siblingDocuments.filter(f => {
const isSameName = f.document.name?.value === documentName;
const isSamePath = normalizePath(f.filePath) === normalizePath(realFilepath);
const isSamePath = normalizePath(f.filePath) === normalizePath(filepath);
return isSameName && !isSamePath;
});

Expand All @@ -36,7 +36,9 @@ export const checkNode = (
messageId,
data: {
documentName,
summary: conflictingDocuments.map(f => `\t${relative(process.cwd(), f.filePath)}`).join('\n'),
summary: conflictingDocuments
.map(f => `\t${relative(process.cwd(), getOnDiskFilepath(f.filePath))}`)
.join('\n'),
},
loc: {
start: {
Expand Down
20 changes: 19 additions & 1 deletion packages/plugin/src/sibling-operations.ts
@@ -1,3 +1,4 @@
import { resolve } from 'path';
import {
FragmentDefinitionNode,
FragmentSpreadNode,
Expand Down Expand Up @@ -29,6 +30,23 @@ export type SiblingOperations = {
getOperationByType(operationType: OperationTypeNode): OperationSource[];
};

const handleVirtualPath = (documents: Source[]): Source[] => {
const filepathMap: Record<string, number> = {};

return documents.map(source => {
const { location } = source;
if (['.gql', '.graphql'].some(extension => location.endsWith(extension))) {
return source;
}
filepathMap[location] = filepathMap[location] ?? -1;
const index = filepathMap[location] += 1;
return {
...source,
location: resolve(location, `${index}_document.graphql`)
};
});
};

const operationsCache: Map<string, Source[]> = new Map();
const siblingOperationsCache: Map<Source[], SiblingOperations> = new Map();

Expand Down Expand Up @@ -56,7 +74,7 @@ const getSiblings = (filePath: string, gqlConfig: GraphQLConfig): Source[] => {
};

export function getSiblingOperations(options: ParserOptions, gqlConfig: GraphQLConfig): SiblingOperations {
const siblings = getSiblings(options.filePath, gqlConfig);
const siblings = handleVirtualPath(getSiblings(options.filePath, gqlConfig));

if (siblings.length === 0) {
let printed = false;
Expand Down
16 changes: 16 additions & 0 deletions packages/plugin/tests/mocks/two-fragments-in-code-file.js
@@ -0,0 +1,16 @@
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const USER_FIELDS = /* GraphQL */ `
fragment UserFields on User {
id
firstName
}
`;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const ALL_USER_FIELDS = /* GraphQL */ `
fragment UserFields on User {
id
firstName
lastName
}
`;
2 changes: 1 addition & 1 deletion packages/plugin/tests/unique-fragment-name.spec.ts
Expand Up @@ -31,7 +31,7 @@ ruleTester.runGraphQLTests('unique-fragment-name', rule, {
{
// Compare filepath of code as real instead of virtual with siblings
...SIBLING_FRAGMENTS(join(__dirname, 'mocks/unique-fragment.js')),
filename: join(__dirname, 'mocks/unique-fragment.js/0_document.graphql '),
filename: join(__dirname, 'mocks/unique-fragment.js/0_document.graphql'),
code: /* GraphQL */ `
fragment UserFields on User {
id
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin/tests/unique-operation-name.spec.ts
Expand Up @@ -21,7 +21,7 @@ ruleTester.runGraphQLTests('unique-operation-name', rule, {
{
// Compare filepath of code as real instead of virtual with siblings
...SIBLING_OPERATIONS(join(__dirname, 'mocks/unique-fragment.js')),
filename: join(__dirname, 'mocks/unique-fragment.js/1_document.graphql '),
filename: join(__dirname, 'mocks/unique-fragment.js/1_document.graphql'),
code: /* GraphQL */ `
query User {
user {
Expand Down

0 comments on commit c0b12a5

Please sign in to comment.