paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 30 Packages Projects Releases Wiki Activity

eslint-plugin: add auto fix for missing imports

Browse Source 
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
Patrik Oldsberg
2023-03-05 20:17:14 +01:00
parent 7aee063a27
commit 911c25de59
5 changed files with 232 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/twelve-parrots-camp.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/eslint-plugin': patch
---
Add support for auto-fixing missing imports detected by the `no-undeclared-imports` rule.
packages/eslint-plugin/lib/getPackages.js
+4
View File
@@ -31,6 +31,7 @@ const manypkg = require('@manypkg/get-packages');
* @property {ExtendedPackage} root
* @property {ExtendedPackage[]} list
* @property {Map<string, ExtendedPackage>} map
* @property {() => void} clearCache
* @property {(path: string) => ExtendedPackage | undefined} byPath
*/
@@ -64,6 +65,9 @@ module.exports = (function () {
pkg => !path.relative(pkg.dir, filePath).startsWith('..'),
);
},
clearCache() {
result = undefined;
},
};
lastLoadAt = Date.now();
return result;
packages/eslint-plugin/lib/visitImports.js
+27 -3
View File
@@ -24,6 +24,16 @@ const getPackages = require('./getPackages');
* @type {object}
* @property {'local'} type
* @property {'value' | 'type'} kind
* @property {import('estree').Node} node
* @property {string} path
*/
/**
* @typedef ImportDirective
* @type {object}
* @property {'directive'} type
* @property {'value' | 'type'} kind
* @property {import('estree').Node} node
* @property {string} path
*/
@@ -32,6 +42,7 @@ const getPackages = require('./getPackages');
* @type {object}
* @property {'internal'} type
* @property {'value' | 'type'} kind
* @property {import('estree').Node} node
* @property {string} path
* @property {import('./getPackages').ExtendedPackage} package
* @property {string} packageName
@@ -42,6 +53,7 @@ const getPackages = require('./getPackages');
* @type {object}
* @property {'external'} type
* @property {'value' | 'type'} kind
* @property {import('estree').Node} node
* @property {string} path
* @property {string} packageName
*/
@@ -51,6 +63,7 @@ const getPackages = require('./getPackages');
* @type {object}
* @property {'builtin'} type
* @property {'value' | 'type'} kind
* @property {import('estree').Literal} node
* @property {string} path
* @property {string} packageName
*/
@@ -58,7 +71,7 @@ const getPackages = require('./getPackages');
/**
* @callback ImportVisitor
* @param {ConsideredNode} node
* @param {LocalImport | InternalImport | ExternalImport | BuiltinImport} import
* @param {ImportDirective | LocalImport | InternalImport | ExternalImport | BuiltinImport} import
*/
/**
@@ -68,7 +81,7 @@ const getPackages = require('./getPackages');
/**
* @param {ConsideredNode} node
* @returns {undefined | {path: string, kind: 'type' | 'value'}}
* @returns {undefined | {path: string, node: import('estree').Literal, kind: 'type' | 'value'}}
*/
function getImportInfo(node) {
/** @type {import('estree').Expression | import('estree').SpreadElement | undefined | null} */
@@ -95,7 +108,11 @@ function getImportInfo(node) {
/** @type {any} */
const anyNode = node;
return { path: pathNode.value, kind: anyNode.importKind ?? 'value' };
return {
path: pathNode.value,
node: pathNode,
kind: anyNode.importKind ?? 'value',
};
}
/**
@@ -122,6 +139,10 @@ module.exports = function visitImports(context, visitor) {
return visitor(node, { type: 'local', ...info });
}
if (info.path.startsWith('directive:')) {
return visitor(node, { type: 'directive', ...info });
}
const pathParts = info.path.split('/');
// Check for match with plain name, then namespaced name
@@ -143,6 +164,7 @@ module.exports = function visitImports(context, visitor) {
return visitor(node, {
type: 'builtin',
kind: info.kind,
node: info.node,
path: subPath,
packageName,
});
@@ -151,6 +173,7 @@ module.exports = function visitImports(context, visitor) {
return visitor(node, {
type: 'external',
kind: info.kind,
node: info.node,
path: subPath,
packageName,
});
@@ -159,6 +182,7 @@ module.exports = function visitImports(context, visitor) {
return visitor(node, {
type: 'internal',
kind: info.kind,
node: info.node,
path: subPath,
package: pkg,
packageName,
packages/eslint-plugin/rules/no-undeclared-imports.js
+139 -49
View File
@@ -20,6 +20,7 @@ const path = require('path');
const getPackageMap = require('../lib/getPackages');
const visitImports = require('../lib/visitImports');
const minimatch = require('minimatch');
const { execFileSync } = require('child_process');
const depFields = {
dep: 'dependencies',
@@ -124,11 +125,13 @@ function getAddFlagForDepsField(depsField) {
module.exports = {
meta: {
type: 'problem',
fixable: 'code',
messages: {
undeclared:
"{{ packageName }} must be declared in {{ depsField }} of {{ packageJsonPath }}, run 'yarn --cwd {{ packagePath }} add{{ addFlag }} {{ packageName }}' from the project root.",
switch:
'{{ packageName }} is declared in {{ oldDepsField }}, but should be moved to {{ depsField }} in {{ packageJsonPath }}.',
switchBack: 'Switch back to import declaration',
},
docs: {
description:
@@ -150,63 +153,150 @@ module.exports = {
return {};
}
return visitImports(context, (node, imp) => {
// We leave checking of type imports to the repo-tools check,
// and we skip builtins and local imports
if (
imp.kind === 'type' ||
imp.type === 'builtin' ||
imp.type === 'local'
) {
return;
}
/** @type Array<{name: string, flag: string, node: import('estree').Node}> */
const importsToAdd = [];
// We skip imports for the package itself
if (imp.packageName === localPkg.packageJson.name) {
return;
}
return {
// All missing imports that we detect are collected as we traverse, and then we use
// the program exit to execute all install directives that have been found.
['Program:exit']() {
/** @type Record<string, Set<string>> */
const byFlag = {};
const modulePath = path.relative(localPkg.dir, filePath);
const expectedType = getExpectedDepType(
localPkg.packageJson,
imp.packageName,
modulePath,
);
for (const { name, flag } of importsToAdd) {
byFlag[flag] = byFlag[flag] ?? new Set();
byFlag[flag].add(name);
}
const conflict = findConflict(
localPkg.packageJson,
imp.packageName,
expectedType,
);
for (const name of byFlag[''] ?? []) {
byFlag['--dev']?.delete(name);
}
for (const name of byFlag['--peer'] ?? []) {
byFlag['']?.delete(name);
byFlag['--dev']?.delete(name);
}
if (conflict) {
try {
const fullImport = imp.path
? `${imp.packageName}/${imp.path}`
: imp.packageName;
require.resolve(fullImport, {
paths: [localPkg.dir],
for (const [flag, names] of Object.entries(byFlag)) {
// The security implication of this is a bit interesting, as crafted add-import
// directives could be used to install malicious packages. However, the same is true
// for adding malicious packages to package.json, so there's significant difference.
execFileSync('yarn', ['add', ...(flag || []), ...names], {
cwd: localPkg.dir,
stdio: 'inherit',
});
} catch {
// If the dependency doesn't resolve then it's likely a type import, ignore
}
// This switches all import directives back to the original import.
for (const added of importsToAdd) {
context.report({
node: added.node,
messageId: 'switchBack',
fix(fixer) {
return fixer.replaceText(added.node, `'${added.name}'`);
},
});
}
importsToAdd.length = 0;
packages.clearCache();
},
...visitImports(context, (node, imp) => {
// We leave checking of type imports to the repo-tools check,
// and we skip builtins and local imports
if (
imp.kind === 'type' ||
imp.type === 'builtin' ||
imp.type === 'local'
) {
return;
}
const packagePath = path.relative(packages.root.dir, localPkg.dir);
const packageJsonPath = path.join(packagePath, 'package.json');
// Any import directive that is found is collected for processing later
if (imp.type === 'directive') {
const parts = imp.path.split(':');
if (parts[1] !== 'add-import') {
return;
}
const [type, name] = parts.slice(2);
if (!name.match(/^(@[-\w\.~]+\/)?[-\w\.~]*$/i)) {
throw new Error(
`Invalid package name to add as dependency: '${name}'`,
);
}
context.report({
node,
messageId: conflict.oldDepsField ? 'switch' : 'undeclared',
data: {
...conflict,
packagePath,
addFlag: getAddFlagForDepsField(conflict.depsField),
packageName: imp.packageName,
packageJsonPath: packageJsonPath,
},
});
}
});
importsToAdd.push({
flag: getAddFlagForDepsField(type).trim(),
name,
node: imp.node,
});
return;
}
// We skip imports for the package itself
if (imp.packageName === localPkg.packageJson.name) {
return;
}
const modulePath = path.relative(localPkg.dir, filePath);
const expectedType = getExpectedDepType(
localPkg.packageJson,
imp.packageName,
modulePath,
);
const conflict = findConflict(
localPkg.packageJson,
imp.packageName,
expectedType,
);
if (conflict) {
try {
const fullImport = imp.path
? `${imp.packageName}/${imp.path}`
: imp.packageName;
require.resolve(fullImport, {
paths: [localPkg.dir],
});
} catch {
// If the dependency doesn't resolve then it's likely a type import, ignore
return;
}
const packagePath = path.relative(packages.root.dir, localPkg.dir);
const packageJsonPath = path.join(packagePath, 'package.json');
context.report({
node,
messageId: conflict.oldDepsField ? 'switch' : 'undeclared',
data: {
...conflict,
packagePath,
addFlag: getAddFlagForDepsField(conflict.depsField),
packageName: imp.packageName,
packageJsonPath: packageJsonPath,
},
// This fix callback is always executed, regardless of whether linting is run with
// fixes enabled or not. There is no way to determine if fixes are being applied, so
// instead our fix will replace the import with a directive that will be picked up
// on the next run. When ESLint applies fixes all rules are re-run to make sure the fixes
// applied correctly, which means that these directives will be picked up, executed,
// and switched back to the original import immediately.
// This is not true for all editor integrations. For example, VSCode translates there fixes
// to native editor commands, and does not re-run ESLint. This means that the import directive
// will end up in source code, and the import directive fix needs to be applied manually too.
// There is to my knowledge no way around this that doesn't get very hacky, so it will do for now.
fix: conflict.oldDepsField
? undefined
: fixer => {
return fixer.replaceText(
imp.node,
`'directive:add-import:${conflict.depsField}:${imp.packageName}'`,
);
},
});
}
}),
};
},
};
packages/eslint-plugin/src/no-undeclared-imports.test.ts
+57 -7
View File
@@ -18,6 +18,10 @@ import { RuleTester } from 'eslint';
import { join as joinPath } from 'path';
import rule from '../rules/no-undeclared-imports';
jest.mock('child_process', () => ({
execFileSync: jest.fn(),
}));
const RULE = 'no-undeclared-imports';
const FIXTURE = joinPath(__dirname, '__fixtures__/monorepo');
@@ -45,6 +49,9 @@ const ERR_SWITCHED = (
'package.json',
)}.`,
});
const ERR_SWITCH_BACK = () => ({
message: 'Switch back to import declaration',
});
process.chdir(FIXTURE);
@@ -130,6 +137,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import 'lodash'`,
output: `import 'directive:add-import:dependencies:lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED('lodash', 'dependencies', joinPath('packages', 'bar')),
@@ -137,6 +145,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import { debounce } from 'lodash'`,
output: `import { debounce } from 'directive:add-import:dependencies:lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED('lodash', 'dependencies', joinPath('packages', 'bar')),
@@ -144,6 +153,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import * as _ from 'lodash'`,
output: `import * as _ from 'directive:add-import:dependencies:lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED('lodash', 'dependencies', joinPath('packages', 'bar')),
@@ -151,6 +161,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import _ from 'lodash'`,
output: `import _ from 'directive:add-import:dependencies:lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED('lodash', 'dependencies', joinPath('packages', 'bar')),
@@ -158,6 +169,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import('lodash')`,
output: `import('directive:add-import:dependencies:lodash')`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED('lodash', 'dependencies', joinPath('packages', 'bar')),
@@ -165,6 +177,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `require('lodash')`,
output: `require('directive:add-import:dependencies:lodash')`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED('lodash', 'dependencies', joinPath('packages', 'bar')),
@@ -172,13 +185,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import 'lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED('lodash', 'dependencies', joinPath('packages', 'bar')),
],
},
{
code: `import 'lodash'`,
output: `import 'directive:add-import:devDependencies:lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.test.ts'),
errors: [
ERR_UNDECLARED(
@@ -191,6 +198,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import 'react'`,
output: `import 'directive:add-import:peerDependencies:react'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED(
@@ -203,6 +211,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import 'react'`,
output: `import 'directive:add-import:peerDependencies:react'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.test.ts'),
errors: [
ERR_UNDECLARED(
@@ -215,6 +224,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import 'react-dom'`,
output: `import 'directive:add-import:dependencies:react-dom'`,
filename: joinPath(FIXTURE, 'packages/foo/src/index.ts'),
errors: [
ERR_UNDECLARED(
@@ -226,6 +236,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import 'react-dom'`,
output: `import 'directive:add-import:devDependencies:react-dom'`,
filename: joinPath(FIXTURE, 'packages/foo/src/index.test.ts'),
errors: [
ERR_UNDECLARED(
@@ -238,6 +249,7 @@ ruleTester.run(RULE, rule, {
},
{
code: `import '@internal/foo'`,
output: `import 'directive:add-import:dependencies:@internal/foo'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [
ERR_UNDECLARED(
@@ -247,5 +259,43 @@ ruleTester.run(RULE, rule, {
),
],
},
// Switching back to original import declarations
{
code: `import 'directive:add-import:dependencies:lodash'`,
output: `import 'lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [ERR_SWITCH_BACK()],
},
{
code: `import { debounce } from 'directive:add-import:dependencies:lodash'`,
output: `import { debounce } from 'lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [ERR_SWITCH_BACK()],
},
{
code: `import * as _ from 'directive:add-import:dependencies:lodash'`,
output: `import * as _ from 'lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [ERR_SWITCH_BACK()],
},
{
code: `import _ from 'directive:add-import:dependencies:lodash'`,
output: `import _ from 'lodash'`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [ERR_SWITCH_BACK()],
},
{
code: `import('directive:add-import:dependencies:lodash')`,
output: `import('lodash')`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [ERR_SWITCH_BACK()],
},
{
code: `require('directive:add-import:dependencies:lodash')`,
output: `require('lodash')`,
filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
errors: [ERR_SWITCH_BACK()],
},
],
});