Skip to content

Commit

Permalink
feat: support testing autofixes
Browse files Browse the repository at this point in the history
  • Loading branch information
jwbay committed Oct 2, 2021
1 parent 7d3504c commit 52645a8
Show file tree
Hide file tree
Showing 8 changed files with 281 additions and 38 deletions.
50 changes: 49 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,54 @@ const foo = 'fails';
```
### Testing fixes
Use the `acceptFix` JSDoc tag for a test to run your rule's fixer against the source code. The
snapshot will contain both the before and after code.

Example: `my-autofix-rule.fixture`

```
/**
* @test replaces foo with bar
* @acceptFix
*/
const somethingFoo = 'something';

```
If your rule replaces text "foo" with "bar" in variables (note: renaming variables is generally
unsafe for a linter and should not be done in a real rule), the following snapshot will be
generated:
```
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`replaces foo with bar`] = `
"Original code:
========================

const somethingFoo = 'something';
~~~~~~~~~~~~ [1]


[1] variable name 'somethingFoo' should not include 'foo'.

Code after applying fixes:
==========================

const somethingBar = 'something'

"
`;
```
If the rule still reports errors after your fixer runs, those errors are serialized just like the
initial errors.
> Note: at time of writing, ESLint runs fixes in a finite loop to allow fixes across rules to
> stabilize. An unstable or incomplete rule fixer may still report errors.
### Syntax highlighting in fixtures
Fixture names ending in any extension are supported. Examples of valid fixture names:
Expand Down Expand Up @@ -239,4 +287,4 @@ function lint(source) {
## Limitations and tradeoffs

- Double quotes in source code are subject to noise in snapshots due to escaping
- Testing lint rule auto-fixes and suggestions is not yet supported
- Testing lint rule suggestions is not yet supported
6 changes: 5 additions & 1 deletion src/fixtureParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ interface FixtureEntry {
testName: string
fileName: string
testSource: string
acceptFix?: boolean
ruleOptions?: any[]
}

const knownTags = ['test', 'filename', 'ruleOptions'] as const
const knownTags = ['test', 'filename', 'ruleOptions', 'acceptFix'] as const
type KnownTags = typeof knownTags[number]

export function parseFixture(fixtureContent: string, fixturePath: string) {
Expand Down Expand Up @@ -51,6 +52,9 @@ export function parseFixture(fixtureContent: string, fixturePath: string) {
case 'ruleoptions' as string:
entry.ruleOptions = parseRuleOptionsFromJSDoc(instruction)
continue
case 'acceptfix' as string:
entry.acceptFix = true
break
default:
const supported = knownTags.join(', ')
console.warn(
Expand Down
47 changes: 35 additions & 12 deletions src/ruleSnapshotTester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,44 @@ export function runLintFixtureTests({
test(entry.testName, () => {
const linter = new Linter({})
linter.defineRule(ruleName, rule)
const result = linter.verify(
entry.testSource,
{
...eslintConfig,
rules: {
[ruleName]: entry.ruleOptions ? ['error', ...entry.ruleOptions] : 'error',
},
const lintConfig: Linter.Config = {
...eslintConfig,
rules: {
[ruleName]: entry.ruleOptions ? ['error', ...entry.ruleOptions] : 'error',
},
entry.fileName
)
const serialized = serializeLintResult({
lintMessages: result,
}

const errorsResult = linter.verify(entry.testSource, lintConfig, entry.fileName)
const serializedErrors = serializeLintResult({
lintMessages: errorsResult,
lintedSource: entry.testSource,
})
expect(serialized).toMatchSnapshot()

if (!entry.acceptFix) {
expect(serializedErrors).toMatchSnapshot()
return
}

const fixed = linter.verifyAndFix(entry.testSource, lintConfig, entry.fileName)
const errorsAfterFixing = linter.verify(fixed.output, lintConfig, entry.fileName)
let postFixCode = fixed.output
if (errorsAfterFixing.length > 0) {
postFixCode = serializeLintResult({
lintedSource: fixed.output,
lintMessages: errorsAfterFixing,
})
}

const result = [
'Original code:',
'========================',
serializedErrors,
'',
'Code after applying fixes:',
'==========================',
postFixCode,
].join('\n')
expect(result).toMatchSnapshot()
})
})
}
117 changes: 117 additions & 0 deletions src/tests/__snapshots__/autofix.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`rule with accepted autofix should display errors before and code after fix 1`] = `
"Original code:
========================
const namedFooSecond = 'fixed'
~~~~~~~~~~~~~~ [1]
[1] variable name 'namedFooSecond' should not include 'foo'.
Code after applying fixes:
==========================
const namedBarSecond = 'fixed'
"
`;

exports[`rule with nested reports and accepted autofix should display errors before and code after fix 1`] = `
"Original code:
========================
class SomethingFooSecond {
~~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
multiline() {
~~~~~~~~~~~~~~~~~ [1]
const foo = 42;
~~~~~~~~~~~~~~~~~~~~~~~ [1]
~~~ [2]
}
~~~~~ [1]
}
~ [1]
[1] class name 'SomethingFooSecond' should not include 'foo'.
[2] variable name 'foo' should not include 'foo'.
Code after applying fixes:
==========================
class SomethingBarSecond {
multiline() {
const bar = 42;
}
}
"
`;

exports[`rule with nested reports and unaccepted autofix should display errors normally 1`] = `
"
class SomethingFooFirst {
~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
multiline() {
~~~~~~~~~~~~~~~~~ [1]
const foo = 42;
~~~~~~~~~~~~~~~~~~~~~~~ [1]
~~~ [2]
}
~~~~~ [1]
}
~ [1]
[1] class name 'SomethingFooFirst' should not include 'foo'.
[2] variable name 'foo' should not include 'foo'."
`;

exports[`rule with unaccepted autofix should display errors normally 1`] = `
"
const namedFooFirst = 'not fixed'
~~~~~~~~~~~~~ [1]
[1] variable name 'namedFooFirst' should not include 'foo'."
`;

exports[`rule with unstable fix should show remaining errors after applying fix 1`] = `
"Original code:
========================
class SomethingFooThird {
~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
multiline() {
~~~~~~~~~~~~~~~~~ [1]
const foo = 42;
~~~~~~~~~~~~~~~~~~~~~~~ [1]
~~~ [2]
}
~~~~~ [1]
}
~ [1]
[1] class name 'SomethingFooThird' should not include 'foo'.
[2] variable name 'foo' should not include 'foo'.
Code after applying fixes:
==========================
class SomethingFooBarBarBarBarBarBarBarBarBarBarThird {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
multiline() {
~~~~~~~~~~~~~~~~~ [1]
const foobarbarbarbarbarbarbarbarbarbar = 42;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [2]
}
~~~~~ [1]
}
~ [1]
[1] class name 'SomethingFooBarBarBarBarBarBarBarBarBarBarThird' should not include 'foo'.
[2] variable name 'foobarbarbarbarbarbarbarbarbarbar' should not include 'foo'."
`;
40 changes: 40 additions & 0 deletions src/tests/autofix.fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* @test rule with unaccepted autofix should display errors normally
*/
const namedFooFirst = 'not fixed'

/**
* @test rule with accepted autofix should display errors before and code after fix
* @acceptFix
*/
const namedFooSecond = 'fixed'

/**
* @test rule with nested reports and unaccepted autofix should display errors normally
*/
class SomethingFooFirst {
multiline() {
const foo = 42;
}
}

/**
* @test rule with nested reports and accepted autofix should display errors before and code after fix
* @acceptFix
*/
class SomethingFooSecond {
multiline() {
const foo = 42;
}
}

/**
* @test rule with unstable fix should show remaining errors after applying fix
* @ruleOptions [{ "badFix": true }]
* @acceptFix
*/
class SomethingFooThird {
multiline() {
const foo = 42;
}
}
7 changes: 7 additions & 0 deletions src/tests/autofix.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { runLintFixtureTests } from '../ruleSnapshotTester'
import { noFooAllowed } from './rules/no-foo-allowed'

runLintFixtureTests({
rule: noFooAllowed,
ruleName: 'autofix',
})
50 changes: 27 additions & 23 deletions src/tests/rules/no-foo-allowed.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Rule } from 'eslint'
import { ClassDeclaration, VariableDeclarator } from 'estree'
import { ClassDeclaration, Identifier, SourceLocation, VariableDeclarator } from 'estree'

export const noFooAllowed: Rule.RuleModule = {
meta: {
type: 'problem',
fixable: 'code',
messages: {
noFooClasses: "class name '{{name}}' should not include '{{forbidden}}'.",
noFooVars: "variable name '{{name}}' should not include '{{forbidden}}'.",
Expand All @@ -15,39 +16,42 @@ export const noFooAllowed: Rule.RuleModule = {
}

const denyList: string[] = context.options?.[0]?.forbidden ?? ['foo']
const misbehavingFix = context.options?.[0]?.badFix ?? false
return {
ClassDeclaration(node) {
const classDeclaration = node as ClassDeclaration

if (classDeclaration.id?.type === 'Identifier') {
const checkedName = classDeclaration.id.name.toLowerCase()
const forbidden = denyList.find((forbidden) => checkedName.includes(forbidden))
if (forbidden) {
const name = classDeclaration.id.name
context.report({
loc: classDeclaration.loc!,
messageId: 'noFooClasses',
data: { name, forbidden },
})
}
checkIdentifier(classDeclaration.id, classDeclaration.loc!, 'noFooClasses')
}
},
VariableDeclarator(node) {
const variableDeclarator = node as VariableDeclarator

if (variableDeclarator.id?.type === 'Identifier') {
const checkedName = variableDeclarator.id.name.toLowerCase()
const forbidden = denyList.find((forbidden) => checkedName.includes(forbidden))
if (forbidden) {
const name = variableDeclarator.id.name
context.report({
loc: variableDeclarator.id.loc!,
messageId: 'noFooVars',
data: { name, forbidden },
})
}
checkIdentifier(variableDeclarator.id, variableDeclarator.id.loc!, 'noFooVars')
}
},
}

function checkIdentifier(
identifier: Identifier,
location: SourceLocation,
messageId: string
) {
const name = identifier.name
const forbidden = denyList.find((forbidden) => name.toLowerCase().includes(forbidden))
if (forbidden) {
context.report({
loc: location,
messageId: messageId,
data: { name, forbidden },
fix: (fixer) => {
const fixedName = name.includes('Foo')
? name.replace('Foo', misbehavingFix ? 'FooBar' : 'Bar')
: name.replace('foo', misbehavingFix ? 'foobar' : 'bar')
return fixer.replaceText(identifier, fixedName)
},
})
}
}
},
}
2 changes: 1 addition & 1 deletion src/tests/runtime-error-unsupported-jsdoc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ runLintFixtureTests({
it('should give a helpful warning when encountering an unsupported jsdoc tag in a fixture', () => {
const warning = consoleWarn.mock.calls[0][0].trim()
expect(warning).toMatchInlineSnapshot(
`"Unrecognized tag '@something' in fixture JSDoc. Supported tags: test, filename, ruleOptions"`
`"Unrecognized tag '@something' in fixture JSDoc. Supported tags: test, filename, ruleOptions, acceptFix"`
)
})

0 comments on commit 52645a8

Please sign in to comment.