Skip to content
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.

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

feat(rule-tester): check for parsing errors in suggestion fixes #9052

Merged
17 changes: 7 additions & 10 deletions packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,17 @@ export default createRule<Options, MessageIds>({
requiresQuoting(missingBranchName.toString(), compilerOptions.target)
) {
const escapedBranchName = missingBranchName
.replace(/'/g, "\\'")
.replace(/\n/g, '\\n')
.replace(/\r/g, '\\r');
.replaceAll("'", "\\'")
.replaceAll('\n', '\\n')
.replaceAll('\r', '\\r');

caseTest = `${symbolName}['${escapedBranchName}']`;
}

const errorMessage = `Not implemented yet: ${caseTest} case`;
const escapedErrorMessage = errorMessage.replace(/'/g, "\\'");

missingCases.push(
`case ${caseTest}: { throw new Error('${escapedErrorMessage}') }`,
`case ${caseTest}: { throw new Error('Not implemented yet: ${caseTest
.replaceAll('\\', '\\\\')
.replaceAll("'", "\\'")} case') }`,
);
}

Expand Down Expand Up @@ -305,9 +304,7 @@ export default createRule<Options, MessageIds>({
context.report({
node: node.discriminant,
messageId: 'switchIsNotExhaustive',
data: {
missingBranches: 'default',
},
data: { missingBranches: 'default' },
suggest: [
{
messageId: 'addMissingCases',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1964,7 +1964,7 @@ switch (value) {

switch (a) {
case Enum.a: { throw new Error('Not implemented yet: Enum.a case') }
case Enum['key-with\\n\\n new-line']: { throw new Error('Not implemented yet: Enum[\\'key-with\\n\\n new-line\\'] case') }
case Enum['key-with\\n\\n new-line']: { throw new Error('Not implemented yet: Enum[\\'key-with\\\\n\\\\n new-line\\'] case') }
}
`,
},
Expand Down Expand Up @@ -1999,7 +1999,7 @@ switch (value) {

switch (a) {
case Enum.a: { throw new Error('Not implemented yet: Enum.a case') }
case Enum['\\'a\\' \`b\` "c"']: { throw new Error('Not implemented yet: Enum[\\'\\\\'a\\\\' \`b\` "c"\\'] case') }
case Enum['\\'a\\' \`b\` "c"']: { throw new Error('Not implemented yet: Enum[\\'\\\\\\'a\\\\\\' \`b\` "c"\\'] case') }
}
`,
},
Expand Down
28 changes: 24 additions & 4 deletions packages/rule-tester/src/RuleTester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,7 @@ export class RuleTester extends TestFramework {
throw new Error(DUPLICATE_PARSER_ERROR_MESSAGE);
}
if (!test.filename) {
return {
...test,
filename: getFilename(test.parserOptions),
};
return { ...test, filename: getFilename(test.parserOptions) };
}
return test;
};
Expand Down Expand Up @@ -483,6 +480,8 @@ export class RuleTester extends TestFramework {
output: string;
beforeAST: TSESTree.Program;
afterAST: TSESTree.Program;
config: RuleTesterConfig;
filename?: string;
} {
let config: TesterConfigWithDefaults = merge({}, this.#testerConfig);
let code;
Expand Down Expand Up @@ -656,6 +655,8 @@ export class RuleTester extends TestFramework {
// is definitely assigned within the `rule-tester/validate-ast` rule
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
afterAST: cloneDeeplyExcludesParent(afterAST!),
config,
filename,
};
}

Expand Down Expand Up @@ -1046,6 +1047,25 @@ export class RuleTester extends TestFramework {
[actualSuggestion],
).output;

// Verify if suggestion fix makes a syntax error or not.
const errorMessageInSuggestion = this.#linter
.verify(
codeWithAppliedSuggestion,
result.config,
result.filename,
)
.find(m => m.fatal);

assert(
!errorMessageInSuggestion,
[
'A fatal parsing error occurred in suggestion fix.',
`Error: ${errorMessageInSuggestion?.message}`,
'Suggestion output:',
codeWithAppliedSuggestion,
].join('\n'),
);

assert.strictEqual(
codeWithAppliedSuggestion,
expectedSuggestion.output,
Expand Down
1 change: 1 addition & 0 deletions packages/rule-tester/tests/RuleTester.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ runRuleForItemSpy.mockImplementation((_1, _2, testCase) => {
output: testCase.code,
afterAST: EMPTY_PROGRAM,
beforeAST: EMPTY_PROGRAM,
config: { parser: '' },
};
});

Expand Down
42 changes: 42 additions & 0 deletions packages/rule-tester/tests/eslint-base/eslint-base.test.js
abrahamguo marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -2045,6 +2045,48 @@ describe("RuleTester", () => {
}, "Error should have 2 suggestions. Instead found 1 suggestions");
});

it("should throw if suggestion fix made a syntax error.", () => {
assert.throw(() => {
ruleTester.run(
"foo",
{
meta: { hasSuggestions: true },
create(context) {
return {
Identifier(node) {
context.report({
node,
message: "make a syntax error",
suggest: [
{
desc: "make a syntax error",
fix(fixer) {
return fixer.replaceText(node, "one two");
}
}
]
});
}
};
}
},
{
valid: [""],
invalid: [{
code: "one()",
errors: [{
message: "make a syntax error",
suggestions: [{
desc: "make a syntax error",
output: "one two()"
}]
}]
}]
}
);
}, /A fatal parsing error occurred in suggestion fix\.\nError: .+\nSuggestion output:\n.+/u);
});

it("should throw if the suggestion description doesn't match", () => {
assert.throws(() => {
ruleTester.run("suggestions-basic", require("./fixtures/suggestions").basic, {
Expand Down