feat(linter): automatic fixes for noRelativeOrAbsoluteImportsAcrossLi…

…braries and noSelfCircularDependencies

e2e/linter/src/linter.test.ts

@@ -1,6 +1,7 @@
 import * as path from 'path';
 import {
   checkFilesExist,
+  createFile,
   newProject,
   readFile,
   readJson,
@@ -9,6 +10,7 @@ import {
   updateFile,
 } from '@nrwl/e2e/utils';
 import * as ts from 'typescript';
+import { names } from '@nrwl/devkit';
 
 describe('Linter', () => {
   describe('linting errors', () => {
@@ -191,6 +193,160 @@ describe('Linter', () => {
       expect(lintOutput).toContain(knownLintErrorMessage);
     }, 1000000);
   });
+
+  it('lint plugin should ensure module boundaries', () => {
+    const proj = newProject();
+    const myapp = uniq('myapp');
+    const myapp2 = uniq('myapp2');
+    const mylib = uniq('mylib');
+    const lazylib = uniq('lazylib');
+    const invalidtaglib = uniq('invalidtaglib');
+    const validtaglib = uniq('validtaglib');
+
+    runCLI(`generate @nrwl/angular:app ${myapp} --tags=validtag`);
+    runCLI(`generate @nrwl/angular:app ${myapp2}`);
+    runCLI(`generate @nrwl/angular:lib ${mylib}`);
+    runCLI(`generate @nrwl/angular:lib ${lazylib}`);
+    runCLI(`generate @nrwl/angular:lib ${invalidtaglib} --tags=invalidtag`);
+    runCLI(`generate @nrwl/angular:lib ${validtaglib} --tags=validtag`);
+
+    const eslint = readJson('.eslintrc.json');
+    eslint.overrides[0].rules[
+      '@nrwl/nx/enforce-module-boundaries'
+    ][1].depConstraints = [
+      { sourceTag: 'validtag', onlyDependOnLibsWithTags: ['validtag'] },
+      ...eslint.overrides[0].rules['@nrwl/nx/enforce-module-boundaries'][1]
+        .depConstraints,
+    ];
+    updateFile('.eslintrc.json', JSON.stringify(eslint, null, 2));
+
+    const tsConfig = readJson('tsconfig.base.json');
+
+    /**
+     * apps do not add themselves to the tsconfig file.
+     *
+     * Let's add it so that we can trigger the lint failure
+     */
+    tsConfig.compilerOptions.paths[`@${proj}/${myapp2}`] = [
+      `apps/${myapp2}/src/main.ts`,
+    ];
+
+    tsConfig.compilerOptions.paths[`@secondScope/${lazylib}`] =
+      tsConfig.compilerOptions.paths[`@${proj}/${lazylib}`];
+    delete tsConfig.compilerOptions.paths[`@${proj}/${lazylib}`];
+    updateFile('tsconfig.base.json', JSON.stringify(tsConfig, null, 2));
+
+    updateFile(
+      `apps/${myapp}/src/main.ts`,
+      `
+      import '../../../libs/${mylib}';
+      import '@secondScope/${lazylib}';
+      import '@${proj}/${myapp2}';
+      import '@${proj}/${invalidtaglib}';
+      import '@${proj}/${validtaglib}';
+
+      const s = {loadChildren: '@${proj}/${lazylib}'};
+    `
+    );
+
+    const out = runCLI(`lint ${myapp}`, { silenceError: true });
+    expect(out).toContain(
+      'Projects cannot be imported by a relative or absolute path, and must begin with a npm scope'
+    );
+    // expect(out).toContain('Imports of lazy-loaded libraries are forbidden');
+    expect(out).toContain('Imports of apps are forbidden');
+    expect(out).toContain(
+      'A project tagged with "validtag" can only depend on libs tagged with "validtag"'
+    );
+  }, 1000000);
+
+  describe('workspace boundary rules', () => {
+    const libA = uniq('lib-a');
+    const libB = uniq('lib-b');
+    const libC = uniq('lib-c');
+    let projScope;
+
+    beforeAll(() => {
+      projScope = newProject();
+      runCLI(`generate @nrwl/workspace:lib ${libA}`);
+      runCLI(`generate @nrwl/workspace:lib ${libB}`);
+      runCLI(`generate @nrwl/workspace:lib ${libC}`);
+
+      // updateFile(`apps/${myapp}/src/main.ts`, `console.log("should fail");`);
+    });
+
+    xdescribe('aaashould autofix noSelfCircularDependencies', () => {
+      beforeAll(() => {
+        /*
+          import { func1, func2 } from '@scope/same-lib';
+
+          should be transformed into
+
+          import { func1 } from './func1';
+          import { func2 } from './func2';
+        */
+
+        createFile(
+          `libs/${libC}/src/lib/another-func.ts`,
+          `
+          export function anotherFunc() {
+            return 'hi';
+          }
+          `
+        );
+
+        updateFile(
+          `libs/${libC}/src/lib/index.ts`,
+          `
+        export * from './lib/${names(libC).fileName}';
+        export * from './lib/another-func';
+        `
+        );
+
+        createFile(
+          `libs/${libC}/src/lib/lib-c-another.ts`,
+          `
+import { ${
+            names(libC).propertyName
+          }, anotherFunc } from '@${projScope}/${libC}';
+
+export function someStuff() {
+  anotherFunc();
+  return ${names(libC).propertyName}();
+}
+        `
+        );
+
+        // scenario 2
+      });
+
+      it('should fix a circular self reference', () => {
+        const stdout = runCLI(`lint ${libC}`, {
+          silenceError: true,
+        });
+        expect(stdout).toContain(
+          'Projects should use relative imports to import from other files within the same project'
+        );
+
+        // fix them
+        const fixedStout = runCLI(`lint ${libC} --fix`, {
+          silenceError: true,
+        });
+        expect(fixedStout).not.toContain(
+          'Projects should use relative imports to import from other files within the same project'
+        );
+        const fileContent = readFile(`libs/${libC}/src/lib/lib-c-another.ts`);
+        expect(fileContent).toContain(
+          `import { ${names(libC).propertyName} } from './${
+            names(libC).fileName
+          }';`
+        );
+        expect(fileContent).toContain(
+          `import { anotherFunc } from './another-func';`
+        );
+      });
+    });
+  });
 });
 
 /**

packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts

@@ -15,13 +15,14 @@ import {
   MappedProjectGraph,
   hasBannedImport,
   isDirectDependency,
+  getTargetProjectBasedOnRelativeImport,
 } from '@nrwl/workspace/src/utils/runtime-lint-utils';
 import {
   AST_NODE_TYPES,
   TSESTree,
 } from '@typescript-eslint/experimental-utils';
 import { createESLintRule } from '../utils/create-eslint-rule';
-import { normalizePath } from '@nrwl/devkit';
+import { joinPathFragments, normalizePath } from '@nrwl/devkit';
 import {
   ProjectType,
   readCachedProjectGraph,
@@ -35,6 +36,15 @@ import {
 import { isRelativePath } from '@nrwl/workspace/src/utilities/fileutils';
 import { isSecondaryEntrypoint as isAngularSecondaryEntrypoint } from '../utils/angular';
 import * as chalk from 'chalk';
+import { existsSync, readFileSync } from 'fs';
+import { findNodes } from '@nrwl/workspace/src/utilities/typescript';
+import ts = require('typescript');
+import { basename, dirname, join, relative, resolve } from 'path';
+import {
+  getBarrelEntryPointByImportScope,
+  getBarrelEntryPointProjectNode,
+  getRelativeImportPath,
+} from '../utils/ast-utils';
 
 type Options = [
   {
@@ -223,6 +233,90 @@ export default createESLintRule<Options, MessageIds>({
           data: {
             npmScope,
           },
+          fix(fixer) {
+            const targetImportProject = getTargetProjectBasedOnRelativeImport(
+              imp,
+              projectPath,
+              projectGraph,
+              sourceFilePath
+            );
+
+            if (targetImportProject) {
+              const indexTsPaths =
+                getBarrelEntryPointProjectNode(targetImportProject);
+
+              if (indexTsPaths && indexTsPaths.length > 0) {
+                const imports = (node as any).specifiers.map(
+                  (specifier) => specifier.imported.name
+                );
+
+                // process each potential entry point and try to find the imports
+                const importsToRemap = [];
+
+                for (const entryPointPath of indexTsPaths) {
+                  for (const importMember of imports) {
+                    const importPath = getRelativeImportPath(
+                      importMember,
+                      entryPointPath.path,
+                      sourceProject.data.sourceRoot
+                    );
+
+                    if (importPath) {
+                      importsToRemap.push({
+                        member: importMember,
+                        importScope: entryPointPath.importScope,
+                      });
+                    } else {
+                      // we cannot remap, so leave it as is
+                      importsToRemap.push({
+                        member: importMember,
+                        importScope: imp,
+                      });
+                    }
+                  }
+                }
+
+                // 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');
+
+                if (adjustedRelativeImports !== '') {
+                  return fixer.replaceTextRange(
+                    node.range,
+                    adjustedRelativeImports
+                  );
+                }
+              }
+            }
+          },
         });
         return;
       }
@@ -256,6 +350,86 @@ export default createESLintRule<Options, MessageIds>({
             data: {
               imp,
             },
+            fix(fixer) {
+              // imp is equal to @myorg/someproject
+              const indexTsPaths = getBarrelEntryPointByImportScope(imp);
+              if (indexTsPaths && indexTsPaths.length > 0) {
+                // imported JS functions to remap
+                const imports = (node.source.parent as any).specifiers.map(
+                  (specifier) => specifier.imported.name
+                );
+
+                // process each potential entry point and try to find the imports
+                const importsToRemap = [];
+
+                for (const entryPointPath of indexTsPaths) {
+                  for (const importMember of imports) {
+                    const importPath = getRelativeImportPath(
+                      importMember,
+                      entryPointPath,
+                      sourceProject.data.sourceRoot
+                    );
+                    if (importPath) {
+                      // resolve the import path
+                      let importPathResolved = relative(
+                        dirname(context.getFilename()),
+                        dirname(importPath)
+                      );
+
+                      // if the string is empty, it's the current file
+                      importPathResolved =
+                        importPathResolved === ''
+                          ? `./${basename(importPath)}`
+                          : joinPathFragments(
+                              importPathResolved,
+                              basename(importPath)
+                            );
+
+                      importsToRemap.push({
+                        member: importMember,
+                        relativePath: 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');
+                if (adjustedRelativeImports !== '') {
+                  return fixer.replaceTextRange(
+                    node.range,
+                    adjustedRelativeImports
+                  );
+                }
+              }
+            },
           });
         }
         return;