feat(linter): minor PR cleanup

nrwl · Mar 8, 2022 · d0d08b7 · d0d08b7
1 parent eec9fb9
commit d0d08b7
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 97 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
@@ -19,13 +19,18 @@ import {
   stringifyTags,
   hasNoneOfTheseTags,
   groupImports,
+  MappedProjectGraphNode,
 } 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 { joinPathFragments, normalizePath } from '@nrwl/devkit';
+import {
+  joinPathFragments,
+  normalizePath,
+  ProjectGraphExternalNode,
+} from '@nrwl/devkit';
 import {
   ProjectType,
   readCachedProjectGraph,
@@ -39,10 +44,7 @@ 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 { basename, dirname, relative } from 'path';
 import {
   getBarrelEntryPointByImportScope,
   getBarrelEntryPointProjectNode,
@@ -225,19 +227,22 @@ export default createESLintRule<Options, MessageIds>({
         projectPath
       );
 
-      // check for relative and absolute imports
       const sourceProject = findSourceProject(projectGraph, sourceFilePath);
-      const targetImportProject = getTargetProjectBasedOnRelativeImport(
-        imp,
-        projectPath,
-        projectGraph,
-        sourceFilePath
-      );
+      // If source is not part of an nx workspace, return.
+      if (!sourceProject) {
+        return;
+      }
 
+      // check for relative and absolute imports
+      let targetProject: MappedProjectGraphNode | ProjectGraphExternalNode =
+        getTargetProjectBasedOnRelativeImport(
+          imp,
+          projectPath,
+          projectGraph,
+          sourceFilePath
+        );
       if (
-        (sourceProject &&
-          targetImportProject &&
-          sourceProject !== targetImportProject) ||
+        (targetProject && sourceProject !== targetProject) ||
         isAbsoluteImportIntoAnotherProject(imp, workspaceLayout)
       ) {
         context.report({
@@ -247,20 +252,19 @@ export default createESLintRule<Options, MessageIds>({
             npmScope,
           },
           fix(fixer) {
-            if (targetImportProject) {
-              const indexTsPaths =
-                getBarrelEntryPointProjectNode(targetImportProject);
+            if (targetProject) {
+              const indexTsPaths = getBarrelEntryPointProjectNode(
+                targetProject as MappedProjectGraphNode
+              );
 
               if (indexTsPaths && indexTsPaths.length > 0) {
-                const imports = ((node as any).specifiers || []).map(
-                  (specifier) => specifier.imported.name
-                );
-
-                // ignore non-named imports for now, like "import '../../some-other-lib/src'"
-                if (imports.length === 0) {
+                const specifiers = (node as any).specifiers;
+                if (!specifiers || specifiers.length === 0) {
                   return;
                 }
 
+                const imports = specifiers.map((s) => s.imported.name);
+
                 // process each potential entry point and try to find the imports
                 const importsToRemap = [];
 
@@ -272,18 +276,10 @@ export default createESLintRule<Options, MessageIds>({
                       sourceProject.data.sourceRoot
                     );
 
-                    if (importPath) {
-                      importsToRemap.push({
-                        member: importMember,
-                        importPath: entryPointPath.importScope,
-                      });
-                    } else {
-                      // we cannot remap, so leave it as is
-                      importsToRemap.push({
-                        member: importMember,
-                        importPath: imp,
-                      });
-                    }
+                    importsToRemap.push({
+                      member: importMember,
+                      importPath: importPath ? entryPointPath.importScope : imp, // we cannot remap, so leave it as is
+                    });
                   }
                 }
 
@@ -302,24 +298,24 @@ export default createESLintRule<Options, MessageIds>({
         return;
       }
 
-      const targetProject = findProjectUsingImport(
-        projectGraph,
-        targetProjectLocator,
-        sourceFilePath,
-        imp,
-        npmScope
-      );
+      targetProject =
+        targetProject ||
+        findProjectUsingImport(
+          projectGraph,
+          targetProjectLocator,
+          sourceFilePath,
+          imp,
+          npmScope
+        );
 
-      // If source or target are not part of an nx workspace, return.
-      if (!sourceProject || !targetProject) {
+      // If target is not part of an nx workspace, return.
+      if (!targetProject) {
         return;
       }
 
-      // same project => allow
+      // we only allow relative paths within the same project
+      // and if it's not a secondary entrypoint in an angular lib
       if (sourceProject === targetProject) {
-        // we only allow relative paths within the same project
-        // and if it's not a secondary entrypoint in an angular lib
-
         if (
           !allowCircularSelfDependency &&
           !isRelativePath(imp) &&
@@ -332,18 +328,15 @@ export default createESLintRule<Options, MessageIds>({
               imp,
             },
             fix(fixer) {
-              // imp is equal to @myorg/someproject
+              // imp has form of @myorg/someproject/some/path
               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);
-
-                // ignore non-named imports for now, like "import '../../some-other-lib/src'"
-                if (imports.length === 0) {
+                const specifiers = (node as any).specifiers;
+                if (!specifiers || specifiers.length === 0) {
                   return;
                 }
+                // imported JS functions to remap
+                const imports = specifiers.map((s) => s.imported.name);
 
                 // process each potential entry point and try to find the imports
                 const importsToRemap = [];
@@ -357,17 +350,17 @@ export default createESLintRule<Options, MessageIds>({
                     );
                     if (importPath) {
                       // resolve the import path
-                      let importPathResolved = relative(
+                      const relativePath = relative(
                         dirname(context.getFilename()),
                         dirname(importPath)
                       );
 
                       // if the string is empty, it's the current file
-                      importPathResolved =
-                        importPathResolved === ''
+                      const importPathResolved =
+                        relativePath === ''
                           ? `./${basename(importPath)}`
                           : joinPathFragments(
-                              importPathResolved,
+                              relativePath,
                               basename(importPath)
                             );
 

diff --git a/packages/eslint-plugin-nx/src/utils/ast-utils.ts b/packages/eslint-plugin-nx/src/utils/ast-utils.ts
@@ -1,4 +1,4 @@
-import { joinPathFragments } from '@nrwl/devkit';
+import { joinPathFragments, readJsonFile } from '@nrwl/devkit';
 import { findNodes } from '@nrwl/workspace/src/utilities/typescript';
 import { MappedProjectGraphNode } from '@nrwl/workspace/src/utils/runtime-lint-utils';
 import { existsSync, readFileSync } from 'fs';
@@ -9,13 +9,9 @@ import { appRootPath } from '@nrwl/tao/src/utils/app-root';
 
 function tryReadBaseJson() {
   try {
-    return JSON.parse(
-      readFileSync(
-        joinPathFragments(appRootPath, 'tsconfig.base.json')
-      ).toString('utf-8')
-    );
+    return readJsonFile(joinPathFragments(appRootPath, 'tsconfig.base.json'));
   } catch (e) {
-    logger.warn(`Error reading tsconfig.base.json: \n${JSON.stringify(e)}`);
+    logger.warn(`Error reading "tsconfig.base.json": \n${JSON.stringify(e)}`);
     return null;
   }
 }
@@ -29,14 +25,7 @@ export function getBarrelEntryPointByImportScope(
   importScope: string
 ): string[] | null {
   const tsConfigBase = tryReadBaseJson();
-
-  if (tsConfigBase?.compilerOptions?.paths) {
-    const entryPoints = tsConfigBase.compilerOptions.paths[importScope];
-    if (entryPoints) {
-      return entryPoints;
-    }
-  }
-  return null;
+  return tsConfigBase?.compilerOptions?.paths[importScope] || null;
 }
 
 export function getBarrelEntryPointProjectNode(
@@ -157,7 +146,7 @@ export function getRelativeImportPath(exportedMember, filePath, basePath) {
              * starting from `SOME_CONSTANT` identifier usages.
              */
             if (parent.kind === ts.SyntaxKind.FunctionDeclaration) {
-              const parentName = (parent as any)?.name?.text;
+              const parentName = (parent as any).name?.text;
               if (parentName === exportedMember) {
                 hasExport = true;
                 break;

diff --git a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts
@@ -1,7 +1,10 @@
 import * as Lint from 'tslint';
 import { IOptions } from 'tslint';
 import * as ts from 'typescript';
-import type { NxJsonConfiguration } from '@nrwl/devkit';
+import type {
+  NxJsonConfiguration,
+  ProjectGraphExternalNode,
+} from '@nrwl/devkit';
 import { ProjectType, readCachedProjectGraph } from '../core/project-graph';
 import { appRootPath } from '@nrwl/tao/src/utils/app-root';
 import {
@@ -20,6 +23,7 @@ import {
   onlyLoadChildren,
   stringifyTags,
   hasNoneOfTheseTags,
+  MappedProjectGraphNode,
 } from '../utils/runtime-lint-utils';
 import { normalize } from 'path';
 import { readNxJson } from '../core/file-utils';
@@ -136,18 +140,21 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
       this.projectPath
     );
     const sourceProject = findSourceProject(this.projectGraph, filePath);
-    const targetImportProject = getTargetProjectBasedOnRelativeImport(
-      imp,
-      this.projectPath,
-      this.projectGraph,
-      filePath
-    );
+    if (!sourceProject) {
+      super.visitImportDeclaration(node);
+      return;
+    }
+    let targetProject: MappedProjectGraphNode | ProjectGraphExternalNode =
+      getTargetProjectBasedOnRelativeImport(
+        imp,
+        this.projectPath,
+        this.projectGraph,
+        filePath
+      );
 
     // check for relative and absolute imports
     if (
-      (sourceProject &&
-        targetImportProject &&
-        sourceProject !== targetImportProject) ||
+      (targetProject && sourceProject !== targetProject) ||
       isAbsoluteImportIntoAnotherProject(imp, this.workspaceLayout)
     ) {
       this.addFailureAt(
@@ -158,16 +165,18 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
       return;
     }
 
-    const targetProject = findProjectUsingImport(
-      this.projectGraph,
-      this.targetProjectLocator,
-      filePath,
-      imp,
-      this.npmScope
-    );
+    targetProject =
+      targetProject ||
+      findProjectUsingImport(
+        this.projectGraph,
+        this.targetProjectLocator,
+        filePath,
+        imp,
+        this.npmScope
+      );
 
     // If source or target are not part of an nx workspace, return.
-    if (!sourceProject || !targetProject || targetProject.type === 'npm') {
+    if (!targetProject || targetProject.type === 'npm') {
       super.visitImportDeclaration(node);
       return;
     }

diff --git a/packages/workspace/src/utils/runtime-lint-utils.ts b/packages/workspace/src/utils/runtime-lint-utils.ts
@@ -168,12 +168,12 @@ export function isAbsoluteImportIntoAnotherProject(
 }
 
 export function findProjectUsingImport(
-  projectGraph: ProjectGraph,
+  projectGraph: MappedProjectGraph,
   targetProjectLocator: TargetProjectLocator,
   filePath: string,
   imp: string,
   npmScope: string
-) {
+): MappedProjectGraphNode | ProjectGraphExternalNode {
   const target = targetProjectLocator.findProjectWithImport(
     imp,
     filePath,