@backstage/eslint-plugin - npm Package Compare versions

Comparing version 0.0.0-nightly-20230314021848 to 0.0.0-nightly-20230315022408

CHANGELOG.md

 # @backstage/eslint-plugin
 
-## 0.0.0-nightly-20230314021848
+## 0.0.0-nightly-20230315022408
 
 ### Patch Changes
 
+- 911c25de59c: Add support for auto-fixing missing imports detected by the `no-undeclared-imports` rule.
+
+## 0.1.2
+
+### Patch Changes
+
 - a061c466d66: Fixing a bug that we should check internal dependencies too
@@ -8,0 +14,0 @@

lib/getPackages.js

@@ -34,2 +34,3 @@ /*
  * @property {Map<string, ExtendedPackage>} map
+ * @property {() => void} clearCache
  * @property {(path: string) => ExtendedPackage | undefined} byPath
@@ -68,2 +69,5 @@ */
       },
+      clearCache() {
+        result = undefined;
+      },
     };
@@ -70,0 +74,0 @@ lastLoadAt = Date.now();

lib/visitImports.js

@@ -27,2 +27,3 @@ /*
  * @property {'value' | 'type'} kind
+ * @property {import('estree').Node} node
  * @property {string} path
@@ -32,2 +33,11 @@ */
 /**
+ * @typedef ImportDirective
+ * @type {object}
+ * @property {'directive'} type
+ * @property {'value' | 'type'} kind
+ * @property {import('estree').Node} node
+ * @property {string} path
+ */
+
+/**
  * @typedef InternalImport
@@ -37,2 +47,3 @@ * @type {object}
  * @property {'value' | 'type'} kind
+ * @property {import('estree').Node} node
  * @property {string} path
@@ -48,2 +59,3 @@ * @property {import('./getPackages').ExtendedPackage} package
  * @property {'value' | 'type'} kind
+ * @property {import('estree').Node} node
  * @property {string} path
@@ -58,2 +70,3 @@ * @property {string} packageName
  * @property {'value' | 'type'} kind
+ * @property {import('estree').Literal} node
  * @property {string} path
@@ -66,3 +79,3 @@ * @property {string} packageName
  * @param {ConsideredNode} node
- * @param {LocalImport | InternalImport | ExternalImport | BuiltinImport} import
+ * @param {ImportDirective | LocalImport | InternalImport | ExternalImport | BuiltinImport} import
  */
@@ -77,3 +90,3 @@
  * @param {ConsideredNode} node
- * @returns {undefined | {path: string, kind: 'type' | 'value'}}
+ * @returns {undefined | {path: string, node: import('estree').Literal, kind: 'type' | 'value'}}
  */
@@ -105,3 +118,7 @@ function getImportInfo(node) {
   const anyNode = node;
-  return { path: pathNode.value, kind: anyNode.importKind ?? 'value' };
+  return {
+    path: pathNode.value,
+    node: pathNode,
+    kind: anyNode.importKind ?? 'value',
+  };
 }
@@ -133,2 +150,6 @@
 
+    if (info.path.startsWith('directive:')) {
+      return visitor(node, { type: 'directive', ...info });
+    }
+
     const pathParts = info.path.split('/');
@@ -155,2 +176,3 @@
           kind: info.kind,
+          node: info.node,
           path: subPath,
@@ -164,2 +186,3 @@ packageName,
         kind: info.kind,
+        node: info.node,
         path: subPath,
@@ -173,2 +196,3 @@ packageName,
       kind: info.kind,
+      node: info.node,
       path: subPath,
@@ -175,0 +199,0 @@ package: pkg,

package.json

 {
   "name": "@backstage/eslint-plugin",
   "description": "Backstage ESLint plugin",
-  "version": "0.0.0-nightly-20230314021848",
+  "version": "0.0.0-nightly-20230315022408",
   "publishConfig": {
@@ -25,5 +25,5 @@ "access": "public"
   "devDependencies": {
-    "@backstage/cli": "^0.0.0-nightly-20230314021848",
+    "@backstage/cli": "^0.0.0-nightly-20230315022408",
     "eslint": "^8.33.0"
   }
 }

rules/no-undeclared-imports.js

@@ -23,2 +23,3 @@ /*
 const minimatch = require('minimatch');
+const { execFileSync } = require('child_process');
 
@@ -128,2 +129,3 @@ const depFields = {
     type: 'problem',
+    fixable: 'code',
     messages: {
@@ -134,2 +136,3 @@ undeclared:
         '{{ packageName }} is declared in {{ oldDepsField }}, but should be moved to {{ depsField }} in {{ packageJsonPath }}.',
+      switchBack: 'Switch back to import declaration',
     },
@@ -156,61 +159,148 @@ docs: {
 
-    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}'`,
+                  );
+                },
+          });
+        }
+      }),
+    };
   },
 };

src/no-undeclared-imports.test.ts

@@ -21,2 +21,6 @@ /*
 
+jest.mock('child_process', () => ({
+  execFileSync: jest.fn(),
+}));
+
 const RULE = 'no-undeclared-imports';
@@ -49,2 +53,5 @@ const FIXTURE = joinPath(__dirname, '__fixtures__/monorepo');
 });
+const ERR_SWITCH_BACK = () => ({
+  message: 'Switch back to import declaration',
+});
 
@@ -135,2 +142,3 @@ process.chdir(FIXTURE);
       code: `import 'lodash'`,
+      output: `import 'directive:add-import:dependencies:lodash'`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -143,2 +151,3 @@ errors: [
       code: `import { debounce } from 'lodash'`,
+      output: `import { debounce } from 'directive:add-import:dependencies:lodash'`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -151,2 +160,3 @@ errors: [
       code: `import * as _ from 'lodash'`,
+      output: `import * as _ from 'directive:add-import:dependencies:lodash'`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -159,2 +169,3 @@ errors: [
       code: `import _ from 'lodash'`,
+      output: `import _ from 'directive:add-import:dependencies:lodash'`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -167,2 +178,3 @@ errors: [
       code: `import('lodash')`,
+      output: `import('directive:add-import:dependencies:lodash')`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -175,2 +187,3 @@ errors: [
       code: `require('lodash')`,
+      output: `require('directive:add-import:dependencies:lodash')`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -183,9 +196,3 @@ errors: [
       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'),
@@ -203,2 +210,3 @@ errors: [
       code: `import 'react'`,
+      output: `import 'directive:add-import:peerDependencies:react'`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -216,2 +224,3 @@ errors: [
       code: `import 'react'`,
+      output: `import 'directive:add-import:peerDependencies:react'`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.test.ts'),
@@ -229,2 +238,3 @@ errors: [
       code: `import 'react-dom'`,
+      output: `import 'directive:add-import:dependencies:react-dom'`,
       filename: joinPath(FIXTURE, 'packages/foo/src/index.ts'),
@@ -241,2 +251,3 @@ errors: [
       code: `import 'react-dom'`,
+      output: `import 'directive:add-import:devDependencies:react-dom'`,
       filename: joinPath(FIXTURE, 'packages/foo/src/index.test.ts'),
@@ -254,2 +265,3 @@ errors: [
       code: `import '@internal/foo'`,
+      output: `import 'directive:add-import:dependencies:@internal/foo'`,
       filename: joinPath(FIXTURE, 'packages/bar/src/index.ts'),
@@ -264,3 +276,41 @@ errors: [
     },
+
+    // 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()],
+    },
   ],
 });

New alerts

Shell access

Supply chain risk

This module accesses the system shell. Accessing the system shell increases the risk of executing arbitrary code.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

46251

increased by17.65%

Lines of code

1201

increased by14.93%

Worsened metrics

Number of medium supply chain risk alerts

1

increased byInfinity%

No dependency changes