fixup! feat(linter): automatic fixes for noRelativeOrAbsoluteImportsA…

…crossLibraries and noSelfCircularDependencies
nrwl · Mar 2, 2022 · ebb34b2 · ebb34b2
1 parent 183b150
commit ebb34b2
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 100 deletions.
diff --git a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts
@@ -8,7 +8,6 @@ import {
   hasBuildExecutor,
   hasNoneOfTheseTags,
   isAbsoluteImportIntoAnotherProject,
-  isRelativeImportIntoAnotherProject,
   mapProjectGraphFiles,
   matchImportWithWildcard,
   onlyLoadChildren,
@@ -17,6 +16,7 @@ import {
   isDirectDependency,
   getTargetProjectBasedOnRelativeImport,
   isTerminalRun,
+  groupImports,
 } from '@nrwl/workspace/src/utils/runtime-lint-utils';
 import {
   AST_NODE_TYPES,
@@ -222,14 +222,17 @@ export default createESLintRule<Options, MessageIds>({
 
       // check for relative and absolute imports
       const sourceProject = findSourceProject(projectGraph, sourceFilePath);
+      const targetImportProject = getTargetProjectBasedOnRelativeImport(
+        imp,
+        projectPath,
+        projectGraph,
+        sourceFilePath
+      );
+
       if (
-        isRelativeImportIntoAnotherProject(
-          imp,
-          projectPath,
-          projectGraph,
-          sourceFilePath,
-          sourceProject
-        ) ||
+        (sourceProject &&
+          targetImportProject &&
+          sourceProject !== targetImportProject) ||
         isAbsoluteImportIntoAnotherProject(imp, workspaceLayout)
       ) {
         context.report({
@@ -239,13 +242,6 @@ export default createESLintRule<Options, MessageIds>({
             npmScope,
           },
           fix(fixer) {
-            const targetImportProject = getTargetProjectBasedOnRelativeImport(
-              imp,
-              projectPath,
-              projectGraph,
-              sourceFilePath
-            );
-
             if (targetImportProject) {
               const indexTsPaths =
                 getBarrelEntryPointProjectNode(targetImportProject);
@@ -288,35 +284,7 @@ export default createESLintRule<Options, MessageIds>({
 
                 // group imports together, like having "import { Foo, Bar } from './foo'"
                 // instead of individual ones
-                const importsToRemapGrouped = importsToRemap.reduce(
-                  (acc, curr) => {
-                    const existing = acc.find(
-                      (i) =>
-                        i.importScope === curr.importScope &&
-                        i.member !== curr.member
-                    );
-                    if (existing) {
-                      if (existing.member) {
-                        existing.member += `, ${curr.member}`;
-                      }
-                    } else {
-                      acc.push({
-                        importScope: curr.importScope,
-                        member: curr.member,
-                      });
-                    }
-                    return acc;
-                  },
-                  []
-                );
-
-                // create the new import expressions
-                const adjustedRelativeImports = importsToRemapGrouped
-                  .map(
-                    (entry) =>
-                      `import { ${entry.member} } from '${entry.importScope}';`
-                  )
-                  .join('\n');
+                const adjustedRelativeImports = groupImports(importsToRemap);
 
                 if (adjustedRelativeImports !== '') {
                   return fixer.replaceTextRange(
@@ -375,7 +343,8 @@ export default createESLintRule<Options, MessageIds>({
                 }
 
                 // process each potential entry point and try to find the imports
-                const importsToRemap = [];
+                const importsToRemap: { member: string; importPath: string }[] =
+                  [];
 
                 for (const entryPointPath of indexTsPaths) {
                   for (const importMember of imports) {
@@ -402,41 +371,14 @@ export default createESLintRule<Options, MessageIds>({
 
                       importsToRemap.push({
                         member: importMember,
-                        relativePath: importPathResolved.replace('.ts', ''),
+                        importPath: importPathResolved.replace('.ts', ''),
                       });
                     }
                   }
                 }
 
                 // group imports from the same relative path
-                const importsToRemapGrouped = importsToRemap.reduce(
-                  (acc, curr) => {
-                    const existing = acc.find(
-                      (i) =>
-                        i.relativePath === curr.relativePath &&
-                        i.member !== curr.member
-                    );
-                    if (existing) {
-                      if (existing.member) {
-                        existing.member += `, ${curr.member}`;
-                      }
-                    } else {
-                      acc.push({
-                        relativePath: curr.relativePath,
-                        member: curr.member,
-                      });
-                    }
-                    return acc;
-                  },
-                  []
-                );
-
-                const adjustedRelativeImports = importsToRemapGrouped
-                  .map(
-                    (entry) =>
-                      `import { ${entry.member} } from '${entry.relativePath}';`
-                  )
-                  .join('\n');
+                const adjustedRelativeImports = groupImports(importsToRemap);
                 if (adjustedRelativeImports !== '') {
                   return fixer.replaceTextRange(
                     node.range,

diff --git a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts
@@ -10,10 +10,10 @@ import {
   findProjectUsingImport,
   findSourceProject,
   getSourceFilePath,
+  getTargetProjectBasedOnRelativeImport,
   hasBuildExecutor,
   hasNoneOfTheseTags,
   isAbsoluteImportIntoAnotherProject,
-  isRelativeImportIntoAnotherProject,
   MappedProjectGraph,
   mapProjectGraphFiles,
   matchImportWithWildcard,
@@ -134,16 +134,18 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
       this.projectPath
     );
     const sourceProject = findSourceProject(this.projectGraph, filePath);
+    const targetImportProject = getTargetProjectBasedOnRelativeImport(
+      imp,
+      this.projectPath,
+      this.projectGraph,
+      filePath
+    );
 
     // check for relative and absolute imports
     if (
-      isRelativeImportIntoAnotherProject(
-        imp,
-        this.projectPath,
-        this.projectGraph,
-        filePath,
-        sourceProject
-      ) ||
+      (sourceProject &&
+        targetImportProject &&
+        sourceProject !== targetImportProject) ||
       isAbsoluteImportIntoAnotherProject(imp, this.workspaceLayout)
     ) {
       this.addFailureAt(

diff --git a/packages/workspace/src/utils/runtime-lint-utils.ts b/packages/workspace/src/utils/runtime-lint-utils.ts
@@ -75,8 +75,10 @@ export function getTargetProjectBasedOnRelativeImport(
   projectPath: string,
   projectGraph: MappedProjectGraph,
   sourceFilePath: string
-) {
-  if (!isRelative(imp)) return false;
+): MappedProjectGraphNode<any> | undefined {
+  if (isRelative(imp)) {
+    return undefined;
+  }
 
   const targetFile = normalizePath(
     path.resolve(path.join(projectPath, path.dirname(sourceFilePath)), imp)
@@ -85,22 +87,6 @@ export function getTargetProjectBasedOnRelativeImport(
   return findTargetProject(projectGraph, targetFile);
 }
 
-export function isRelativeImportIntoAnotherProject(
-  imp: string,
-  projectPath: string,
-  projectGraph: MappedProjectGraph,
-  sourceFilePath: string,
-  sourceProject: ProjectGraphProjectNode
-): boolean {
-  const targetProject = getTargetProjectBasedOnRelativeImport(
-    imp,
-    projectPath,
-    projectGraph,
-    sourceFilePath
-  );
-  return sourceProject && targetProject && sourceProject !== targetProject;
-}
-
 export function findProjectUsingFile<T>(
   projectGraph: MappedProjectGraph<T>,
   file: string
@@ -289,3 +275,35 @@ export function isTerminalRun(): boolean {
     !!process.argv[1].match(/@nrwl\/cli\/lib\/run-cli\.js$/)
   );
 }
+
+/**
+ * Takes an array of imports and tries to group them, so rather than having
+ * `import { A } from './some-location'` and `import { B } from './some-location'` you get
+ * `import { A, B } from './some-location'`
+ * @param importsToRemap
+ * @returns
+ */
+export function groupImports(
+  importsToRemap: { member: string; importPath: string }[]
+): string {
+  const importsToRemapGrouped = importsToRemap.reduce((acc, curr) => {
+    const existing = acc.find(
+      (i) => i.importPath === curr.importPath && i.member !== curr.member
+    );
+    if (existing) {
+      if (existing.member) {
+        existing.member += `, ${curr.member}`;
+      }
+    } else {
+      acc.push({
+        importPath: curr.importPath,
+        member: curr.member,
+      });
+    }
+    return acc;
+  }, []);
+
+  return importsToRemapGrouped
+    .map((entry) => `import { ${entry.member} } from '${entry.importPath}';`)
+    .join('\n');
+}