eslint-plugin: refactor to ES2021 and add no-undeclared-imports rule

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

packages/eslint-plugin/.eslintrc.js

@@ -17,8 +17,9 @@
 // Custom lint config here since source is ES5
 module.exports = {
   plugins: ['import'],
-  parserOptions: {
-    ecmaVersion: 5,
+  env: {
+    node: true,
+    es2021: true,
   },
   rules: {
     'import/no-extraneous-dependencies': [
@@ -30,6 +31,6 @@ module.exports = {
         bundledDependencies: true,
       },
     ],
-    'no-unused-expressions': 'off',
+    // 'no-unused-expressions': 'off',
   },
 };

packages/eslint-plugin/index.js

@@ -20,10 +20,12 @@ module.exports = {
       plugins: ['@backstage'],
       rules: {
         '@backstage/no-forbidden-package-imports': 'error',
+        '@backstage/no-undeclared-imports': 'error',
       },
     },
   },
   rules: {
     'no-forbidden-package-imports': require('./rules/no-forbidden-package-imports'),
+    'no-undeclared-imports': require('./rules/no-undeclared-imports'),
   },
 };

packages/eslint-plugin/lib/getPackages.js

@@ -0,0 +1,63 @@
+/*
+ * Copyright 2023 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// @ts-check
+
+/** @type {import('@manypkg/get-packages')} */
+const manypkg = require('@manypkg/get-packages');
+
+/**
+ * @typedef ExtendedPackage
+ * @type {import('@manypkg/get-packages').Package & { packageJson: { exports?: Record<string, string> }}} packageJson
+ */
+
+/**
+ * @typedef PackageMap
+ * @type object
+ *
+ * @property {ExtendedPackage} root
+ * @property {ExtendedPackage[]} list
+ * @property {Map<string, ExtendedPackage>} map
+ */
+
+// Loads all packages in the monorepo once, and caches the result
+module.exports = (function () {
+  let result = undefined;
+  let lastLoadAt = undefined;
+
+  /** @returns {PackageMap | undefined} */
+  return function getPackages(/** @type {string} */ dir) {
+    if (result) {
+      // Only cache for 5 seconds, to avoid the need to reload ESLint servers
+      if (Date.now() - lastLoadAt > 5000) {
+        result = undefined;
+      } else {
+        return result;
+      }
+    }
+    const packages = manypkg.getPackagesSync(dir);
+    if (!packages) {
+      return undefined;
+    }
+    result = {
+      map: new Map(packages.packages.map(pkg => [pkg.packageJson.name, pkg])),
+      list: packages.packages,
+      root: packages.root,
+    };
+    lastLoadAt = Date.now();
+    return result;
+  };
+})();

packages/eslint-plugin/lib/visitImports.js

@@ -0,0 +1,100 @@
+/*
+ * Copyright 2023 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// @ts-check
+
+const getPackages = require('./getPackages');
+
+/**
+ * @typedef LocalImport
+ * @type {object}
+ * @property {'local'} type
+ * @property {string} path
+ */
+
+/**
+ * @typedef InternalImport
+ * @type {object}
+ * @property {'internal'} type
+ * @property {string} path
+ * @property {import('./getPackages').ExtendedPackage} package
+ */
+
+/**
+ * @typedef ExternalImport
+ * @type {object}
+ * @property {'external'} type
+ * @property {string} path
+ * @property {string} packageName
+ */
+
+/**
+ * @callback ImportVisitor
+ * @param {import('eslint').Rule.Node} node
+ * @param {LocalImport | InternalImport | ExternalImport} import
+ */
+
+/**
+ * @param visitor - Visitor callback
+ * @param {import('eslint').Rule.RuleContext} context
+ * @param {ImportVisitor} visitor
+ */
+module.exports = function visitImports(context, visitor) {
+  const packages = getPackages(context.getCwd());
+  if (!packages) {
+    return;
+  }
+
+  function visit(node) {
+    if (!node.source) {
+      return;
+    }
+    const importPath = node.source.value;
+    if (importPath[0] === '.') {
+      return visitor(node, { type: 'local', path: importPath });
+    }
+
+    const pathParts = importPath.split('/');
+
+    // Check for match with plain name, then namespaced name
+    let packageName;
+    let subPath;
+    if (importPath[0] === '@') {
+      packageName = pathParts.slice(0, 2).join('/');
+      subPath = pathParts.slice(2).join('/');
+    } else {
+      packageName = pathParts[0];
+      subPath = pathParts.slice(1).join('/');
+    }
+    const pkg = packages?.map.get(packageName);
+    if (!pkg) {
+      return visitor(node, {
+        type: 'external',
+        path: subPath,
+        packageName: packageName,
+      });
+    }
+
+    return visitor(node, { type: 'internal', path: subPath, package: pkg });
+  }
+
+  return {
+    ImportDeclaration: visit,
+    ExportAllDeclaration: visit,
+    ExportNamedDeclaration: visit,
+    ImportExpression: visit,
+  };
+};

packages/eslint-plugin/package.json

@@ -17,7 +17,8 @@
     "lint": "backstage-cli package lint"
   },
   "dependencies": {
-    "@manypkg/get-packages": "^1.1.3"
+    "@manypkg/get-packages": "^1.1.3",
+    "minimatch": "^5.1.2"
   },
   "devDependencies": {
     "@backstage/cli": "workspace:^"

packages/eslint-plugin/rules/no-forbidden-package-imports.js

@@ -1,4 +1,3 @@
-'use strict';
 /*
  * Copyright 2023 The Backstage Authors
  *
@@ -17,41 +16,9 @@
 
 // @ts-check
 
-// Note: this file is ES5, because we want to avoid having to transpile it for local development
-
-var fs = require('fs');
-var path = require('path');
-/** @type {import('@manypkg/get-packages')} */
-var manypkg = require('@manypkg/get-packages');
-
-// Loads all packages in the monorepo once, and caches the result
-var getPackageMap = (function () {
-  var map = undefined;
-  var lastLoadAt = undefined;
-
-  /** @returns {Record<string, {dir: string, packageJson: unknown}>} */
-  return function (/** @type {string} */ dir) {
-    if (map) {
-      // Only cache for 5 seconds, to avoid the need to reload ESLint servers
-      if (Date.now() - lastLoadAt > 5000) {
-        map = undefined;
-      } else {
-        return map;
-      }
-    }
-    var packages = manypkg.getPackagesSync(dir).packages;
-    if (!packages) {
-      return undefined;
-    }
-    map = {};
-    lastLoadAt = Date.now();
-    packages.forEach(function (pkg) {
-      map[pkg.packageJson.name] = pkg;
-    });
-    return map;
-  };
-})();
+const visitImports = require('../lib/visitImports');
 
+/** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
   meta: {
     type: 'problem',
@@ -59,48 +26,19 @@ module.exports = {
       forbidden: '{{packageName}} does not export {{subPath}}',
     },
   },
-  create: function (/** @type import('eslint').Rule.RuleContext */ context) {
-    var packageMap = getPackageMap(context.getCwd());
-    if (!packageMap) {
-      return;
-    }
-
-    function checkImport(node) {
-      if (!node.source) {
+  create(context) {
+    return visitImports(context, (node, imp) => {
+      if (imp.type !== 'internal') {
         return;
       }
-      var importPath = node.source.value;
-      if (importPath[0] === '.') {
-        return;
-      }
-
-      var pathParts = importPath.split('/');
-
-      // Check for match with plain name, then namespaced name
-      var name = pathParts.shift();
-      if (!packageMap[name]) {
-        name += '/' + pathParts.shift();
-      }
-      if (!packageMap[name]) {
-        return;
-      }
-
       // Empty subpaths are always allowed
-      var subPath = pathParts.join('/');
-      if (!subPath) {
+      if (!imp.path) {
         return;
       }
 
-      var pkg = packageMap[name];
-
       // If the import is listed in the package.json exports field, we allow it
-      var exp = pkg.packageJson.exports;
-      if (exp && (exp[subPath] || exp['./' + subPath])) {
-        return;
-      }
-
-      // If there's a package.json in the target directory, we allow the import
-      if (fs.existsSync(path.resolve(pkg.dir, subPath, 'package.json'))) {
+      const exp = imp.package.packageJson.exports;
+      if (exp && (exp[imp.path] || exp['./' + imp.path])) {
         return;
       }
 
@@ -108,17 +46,10 @@ module.exports = {
         node: node,
         messageId: 'forbidden',
         data: {
-          packageName: pkg.packageJson.name || '<unknown-package>',
-          subPath: subPath,
+          packageName: imp.package.packageJson.name || imp.package.dir,
+          subPath: imp.path,
         },
       });
-    }
-
-    return {
-      ImportDeclaration: checkImport,
-      ExportAllDeclaration: checkImport,
-      ExportNamedDeclaration: checkImport,
-      ImportExpression: checkImport,
-    };
+    });
   },
 };

packages/eslint-plugin/rules/no-undeclared-imports.js

@@ -0,0 +1,156 @@
+/*
+ * Copyright 2023 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// @ts-check
+
+const path = require('path');
+const getPackageMap = require('../lib/getPackages');
+const visitImports = require('../lib/visitImports');
+const minimatch = require('minimatch');
+
+const depFields = {
+  dep: 'dependencies',
+  dev: 'devDependencies',
+  peer: 'peerDependencies',
+};
+
+const peerDeps = ['react', 'react-dom', 'react-router', 'react-router-dom'];
+const devModulePatterns = [
+  new minimatch.Minimatch('!src/**'),
+  new minimatch.Minimatch('src/**/*.test.*'),
+  new minimatch.Minimatch('src/**/*.stories.*'),
+  new minimatch.Minimatch('src/**/__testUtils__/**'),
+  new minimatch.Minimatch('src/**/__mocks__/**'),
+  new minimatch.Minimatch('src/setupTests.*'),
+];
+
+function getExpectedDepType(
+  /** @type {string} */ impPath,
+  /** @type {string} */ modulePath,
+) {
+  if (peerDeps.includes(impPath)) {
+    return 'peer';
+  }
+  for (const pattern of devModulePatterns) {
+    if (pattern.match(modulePath)) {
+      return 'dev';
+    }
+  }
+  return 'dep';
+}
+
+/**
+ *
+ * @param {import('@manypkg/get-packages').Package['packageJson']} pkg
+ * @param {string} name
+ * @param {ReturnType<typeof getExpectedDepType>} expectedType
+ * @returns {{oldDepsField?: string, depsField: string} | undefined}
+ */
+function findConflict(pkg, name, expectedType) {
+  const isDep = pkg.dependencies?.[name];
+  const isDevDep = pkg.devDependencies?.[name];
+  const isPeerDep = pkg.peerDependencies?.[name];
+  const depsField = depFields[expectedType];
+
+  if (expectedType === 'dep' && !isDep) {
+    const oldDepsField = isDevDep ? depFields.dev : undefined;
+    return { oldDepsField, depsField };
+  } else if (expectedType === 'dev' && !isDevDep && !isDep && !isPeerDep) {
+    return { oldDepsField: undefined, depsField };
+  } else if (expectedType === 'peer' && !isPeerDep) {
+    const oldDepsField = isDep
+      ? depFields.dep
+      : isDevDep
+      ? depFields.dev
+      : undefined;
+
+    return { oldDepsField, depsField };
+  }
+}
+
+function getAddFlagForConflict(conflict) {
+  switch (conflict?.depsField) {
+    case depFields.dep:
+      return '';
+    case depFields.dev:
+      return ' --dev';
+    case depFields.peer:
+      return ' --peer';
+    default:
+      return '';
+  }
+}
+
+/** @type {import('eslint').Rule.RuleModule} */
+module.exports = {
+  meta: {
+    type: 'problem',
+    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 }}.',
+    },
+  },
+  create(context) {
+    const packages = getPackageMap(context.getCwd());
+    if (!packages) {
+      return {};
+    }
+    const filePath = context.getPhysicalFilename
+      ? context.getPhysicalFilename()
+      : context.getFilename();
+
+    const localPkg = packages.list.find(p =>
+      filePath.startsWith(p.dir + path.sep),
+    );
+    if (!localPkg) {
+      return {};
+    }
+
+    return visitImports(context, (node, imp) => {
+      if (imp.type !== 'external') {
+        return;
+      }
+
+      const modulePath = path.relative(localPkg.dir, filePath);
+      const expectedType = getExpectedDepType(imp.packageName, modulePath);
+
+      const conflict = findConflict(
+        localPkg.packageJson,
+        imp.packageName,
+        expectedType,
+      );
+
+      if (conflict) {
+        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: getAddFlagForConflict(conflict),
+            packageName: imp.packageName,
+            packageJsonPath: packageJsonPath,
+          },
+        });
+      }
+    });
+  },
+};

yarn.lock

@@ -4101,6 +4101,7 @@ __metadata:
   dependencies:
     "@backstage/cli": "workspace:^"
     "@manypkg/get-packages": ^1.1.3
+    minimatch: ^5.1.2
   languageName: unknown
   linkType: soft