feat(linter): optimize lint performance. resolves #5210 (#5576)

* feat(linter): optimize containsFile function

related to #5210

* feat(linter): optimize find project function

related to #5210

* feat(core): cleanup target project locator

* fix(core): fix false positive match on resolvedModule

* chore(core): mark old code for removal

* feat(linter): move npm check before expensive typescript resolution

* feat(linter): improve performance of extension removal

* feat(linter): improve I/O operations

* feat(linter): read ts config only once in project locator

* feat(linter): remove double path normalization from rule

normalization is already part of getSourceFilePath function

* feat(linter): run find source only once

* feat(linter): defer source file path calculation will after whitelist check

* feat(linter): map project graph node files to hash map

* fix(linter): map projectGraph only once per runtime

* feat(linter): introduce mapped project graph type

packages/devkit/src/project-graph/interfaces.ts

@@ -19,8 +19,8 @@ export interface ProjectFileMap {
 /**
  * A Graph of projects in the workspace and dependencies between them
  */
-export interface ProjectGraph {
-  nodes: Record<string, ProjectGraphNode>;
+export interface ProjectGraph<T = any> {
+  nodes: Record<string, ProjectGraphNode<T>>;
   dependencies: Record<string, ProjectGraphDependency[]>;
 
   // this is optional otherwise it might break folks who use project graph creation

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

@@ -9,8 +9,10 @@ import {
   hasNoneOfTheseTags,
   isAbsoluteImportIntoAnotherProject,
   isRelativeImportIntoAnotherProject,
+  mapProjectGraphFiles,
   matchImportWithWildcard,
   onlyLoadChildren,
+  MappedProjectGraph,
 } from '@nrwl/workspace/src/utils/runtime-lint-utils';
 import {
   AST_NODE_TYPES,
@@ -20,7 +22,6 @@ import { createESLintRule } from '../utils/create-eslint-rule';
 import { normalizePath } from '@nrwl/devkit';
 import {
   isNpmProject,
-  ProjectGraph,
   ProjectType,
 } from '@nrwl/workspace/src/core/project-graph';
 import { readNxJson } from '@nrwl/workspace/src/core/file-utils';
@@ -121,7 +122,9 @@ export default createESLintRule<Options, MessageIds>({
     if (!(global as any).projectGraph) {
       const nxJson = readNxJson();
       (global as any).npmScope = nxJson.npmScope;
-      (global as any).projectGraph = readCurrentProjectGraph();
+      (global as any).projectGraph = mapProjectGraphFiles(
+        readCurrentProjectGraph()
+      );
     }
 
     if (!(global as any).projectGraph) {
@@ -129,7 +132,7 @@ export default createESLintRule<Options, MessageIds>({
     }
 
     const npmScope = (global as any).npmScope;
-    const projectGraph = (global as any).projectGraph as ProjectGraph;
+    const projectGraph = (global as any).projectGraph as MappedProjectGraph;
 
     if (!(global as any).targetProjectLocator) {
       (global as any).targetProjectLocator = new TargetProjectLocator(
@@ -159,23 +162,25 @@ export default createESLintRule<Options, MessageIds>({
 
       const imp = node.source.value as string;
 
-      const sourceFilePath = getSourceFilePath(
-        normalizePath(context.getFilename()),
-        projectPath
-      );
-
       // whitelisted import
       if (allow.some((a) => matchImportWithWildcard(a, imp))) {
         return;
       }
 
+      const sourceFilePath = getSourceFilePath(
+        context.getFilename(),
+        projectPath
+      );
+
       // check for relative and absolute imports
+      const sourceProject = findSourceProject(projectGraph, sourceFilePath);
       if (
         isRelativeImportIntoAnotherProject(
           imp,
           projectPath,
           projectGraph,
-          sourceFilePath
+          sourceFilePath,
+          sourceProject
         ) ||
         isAbsoluteImportIntoAnotherProject(imp)
       ) {
@@ -189,7 +194,6 @@ export default createESLintRule<Options, MessageIds>({
         return;
       }
 
-      const sourceProject = findSourceProject(projectGraph, sourceFilePath);
       const targetProject = findProjectUsingImport(
         projectGraph,
         targetProjectLocator,

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

@@ -11,6 +11,7 @@ import enforceModuleBoundaries, {
   RULE_NAME as enforceModuleBoundariesRuleName,
 } from '../../src/rules/enforce-module-boundaries';
 import { TargetProjectLocator } from '@nrwl/workspace/src/core/target-project-locator';
+import { mapProjectGraphFiles } from '@nrwl/workspace/src/utils/runtime-lint-utils';
 jest.mock('fs', () => require('memfs').fs);
 jest.mock('../../../workspace/src/utilities/app-root', () => ({
   appRootPath: '/root',
@@ -72,7 +73,7 @@ const fileSys = {
   './tsconfig.base.json': JSON.stringify(tsconfig),
 };
 
-describe('Enforce Module Boundaries', () => {
+describe('Enforce Module Boundaries (eslint)', () => {
   beforeEach(() => {
     vol.fromJSON(fileSys, '/root');
   });
@@ -1536,7 +1537,7 @@ function runRule(
 ): TSESLint.Linter.LintMessage[] {
   (global as any).projectPath = `${process.cwd()}/proj`;
   (global as any).npmScope = 'mycompany';
-  (global as any).projectGraph = projectGraph;
+  (global as any).projectGraph = mapProjectGraphFiles(projectGraph);
   (global as any).targetProjectLocator = new TargetProjectLocator(
     projectGraph.nodes
   );

packages/workspace/src/command-line/affected.ts

@@ -149,7 +149,7 @@ function projectsToRun(nxArgs: NxArgs, projectGraph: ProjectGraph) {
 }
 
 function applyExclude(
-  projects: Record<string, ProjectGraphNode<any>>,
+  projects: Record<string, ProjectGraphNode>,
   nxArgs: NxArgs
 ) {
   return Object.keys(projects)
@@ -161,7 +161,7 @@ function applyExclude(
 }
 
 function applyOnlyFailed(
-  projectsNotExcluded: Record<string, ProjectGraphNode<any>>,
+  projectsNotExcluded: Record<string, ProjectGraphNode>,
   nxArgs: NxArgs,
   env: Environment
 ) {

packages/workspace/src/command-line/run-many.ts

@@ -80,7 +80,7 @@ function applyExclude(
 }
 
 function applyOnlyFailed(
-  projectsNotExcluded: Record<string, ProjectGraphNode<any>>,
+  projectsNotExcluded: Record<string, ProjectGraphNode>,
   nxArgs: NxArgs,
   env: Environment
 ) {

packages/workspace/src/core/file-utils.ts

@@ -151,7 +151,7 @@ function getIgnoredGlobs() {
   return ig;
 }
 
-function readFileIfExisting(path: string) {
+export function readFileIfExisting(path: string) {
   return existsSync(path) ? readFileSync(path, 'utf-8') : '';
 }
 

packages/workspace/src/core/project-graph/operators.ts

@@ -1,9 +1,5 @@
 import { ProjectGraphBuilder } from './project-graph-builder';
-import {
-  ProjectGraph,
-  ProjectGraphNode,
-  ProjectGraphNodeRecords,
-} from './project-graph-models';
+import { ProjectGraph, ProjectGraphNode } from './project-graph-models';
 
 const reverseMemo = new Map<ProjectGraph, ProjectGraph>();
 
@@ -61,7 +57,7 @@ export function isNpmProject(
   return project.type === 'npm';
 }
 
-export function getSortedProjectNodes(nodes: ProjectGraphNodeRecords) {
+export function getSortedProjectNodes(nodes: Record<string, ProjectGraphNode>) {
   return Object.values(nodes).sort((nodeA, nodeB) => {
     // If a or b is not a nx project, leave them in the same spot
     if (!isWorkspaceProject(nodeA) && !isWorkspaceProject(nodeB)) {

packages/workspace/src/core/target-project-locator.ts

@@ -1,9 +1,6 @@
 import { resolveModuleByImport } from '../utilities/typescript';
-import { defaultFileRead, normalizedProjectRoot } from './file-utils';
-import {
-  ProjectGraphNode,
-  ProjectGraphNodeRecords,
-} from './project-graph/project-graph-models';
+import { normalizedProjectRoot, readFileIfExisting } from './file-utils';
+import { ProjectGraphNode } from './project-graph/project-graph-models';
 import {
   getSortedProjectNodes,
   isNpmProject,
@@ -29,14 +26,12 @@ export class TargetProjectLocator {
         } as ProjectGraphNode)
     );
   private npmProjects = this.sortedProjects.filter(isNpmProject);
-  private tsConfigPath = this.getRootTsConfigPath();
-  private absTsConfigPath = join(appRootPath, this.tsConfigPath);
-  private paths = parseJsonWithComments(defaultFileRead(this.tsConfigPath))
-    ?.compilerOptions?.paths;
+  private tsConfig = this.getRootTsConfig();
+  private paths = this.tsConfig.config?.compilerOptions?.paths;
   private typescriptResolutionCache = new Map<string, string | null>();
   private npmResolutionCache = new Map<string, string | null>();
 
-  constructor(private nodes: ProjectGraphNodeRecords) {}
+  constructor(private readonly nodes: Record<string, ProjectGraphNode>) {}
 
   /**
    * Find a project based on its import
@@ -73,6 +68,12 @@ export class TargetProjectLocator {
       }
     }
 
+    // try to find npm package before using expensive typescript resolution
+    const npmProject = this.findNpmPackage(importExpr);
+    if (npmProject || this.npmResolutionCache.has(importExpr)) {
+      return npmProject;
+    }
+
     let resolvedModule: string;
     if (this.typescriptResolutionCache.has(normalizedImportExpr)) {
       resolvedModule = this.typescriptResolutionCache.get(normalizedImportExpr);
@@ -80,7 +81,7 @@ export class TargetProjectLocator {
       resolvedModule = resolveModuleByImport(
         normalizedImportExpr,
         filePath,
-        this.absTsConfigPath
+        this.tsConfig.absolutePath
       );
       this.typescriptResolutionCache.set(
         normalizedImportExpr,
@@ -89,12 +90,13 @@ export class TargetProjectLocator {
     }
 
     // TODO: vsavkin temporary workaround. Remove it once we reworking handling of npm packages.
-    if (resolvedModule && resolvedModule.indexOf('/node_modules/') === -1) {
+    if (resolvedModule && resolvedModule.indexOf('node_modules/') === -1) {
       const resolvedProject = this.findProjectOfResolvedModule(resolvedModule);
       if (resolvedProject) {
         return resolvedProject;
       }
     }
+    // TODO: meeroslav this block should be probably removed
     const importedProject = this.sortedWorkspaceProjects.find((p) => {
       const projectImport = `@${npmScope}/${p.data.normalizedRoot}`;
       return (
@@ -102,23 +104,28 @@ export class TargetProjectLocator {
         normalizedImportExpr.startsWith(`${projectImport}/`)
       );
     });
-    if (importedProject) return importedProject.name;
+    if (importedProject) {
+      return importedProject.name;
+    }
 
-    const npmProject = this.findNpmPackage(importExpr);
-    return npmProject ? npmProject : null;
+    // nothing found, cache for later
+    this.npmResolutionCache.set(importExpr, undefined);
+    return null;
   }
 
-  private findNpmPackage(npmImport: string) {
+  private findNpmPackage(npmImport: string): string | undefined {
     if (this.npmResolutionCache.has(npmImport)) {
       return this.npmResolutionCache.get(npmImport);
     } else {
-      const pkgName = this.npmProjects.find(
+      const pkg = this.npmProjects.find(
         (pkg) =>
           npmImport === pkg.data.packageName ||
           npmImport.startsWith(`${pkg.data.packageName}/`)
-      )?.name;
-      this.npmResolutionCache.set(npmImport, pkgName);
-      return pkgName;
+      );
+      if (pkg) {
+        this.npmResolutionCache.set(npmImport, pkg.name);
+        return pkg.name;
+      }
     }
   }
 
@@ -127,15 +134,18 @@ export class TargetProjectLocator {
       return resolvedModule.startsWith(p.data.root);
     });
 
-    return importedProject?.name;
+    return importedProject ? importedProject.name : void 0;
   }
 
-  private getRootTsConfigPath() {
-    try {
-      defaultFileRead('tsconfig.base.json');
-      return 'tsconfig.base.json';
-    } catch (e) {
-      return 'tsconfig.json';
+  private getRootTsConfig() {
+    let path = 'tsconfig.base.json';
+    let absolutePath = join(appRootPath, path);
+    let content = readFileIfExisting(absolutePath);
+    if (!content) {
+      path = 'tsconfig.json';
+      absolutePath = join(appRootPath, path);
+      content = readFileIfExisting(absolutePath);
     }
+    return { path, absolutePath, config: parseJsonWithComments(content) };
   }
 }

packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.spec.ts

@@ -9,6 +9,7 @@ import {
 } from '../core/project-graph';
 import { Rule } from './nxEnforceModuleBoundariesRule';
 import { TargetProjectLocator } from '../core/target-project-locator';
+import { mapProjectGraphFiles } from '../utils/runtime-lint-utils';
 
 jest.mock('fs', () => require('memfs').fs);
 jest.mock('../utilities/app-root', () => ({ appRootPath: '/root' }));
@@ -69,7 +70,7 @@ const fileSys = {
   './tsconfig.base.json': JSON.stringify(tsconfig),
 };
 
-describe('Enforce Module Boundaries', () => {
+describe('Enforce Module Boundaries (tslint)', () => {
   beforeEach(() => {
     vol.fromJSON(fileSys, '/root');
   });
@@ -1177,12 +1178,14 @@ function runRule(
     true
   );
 
+  const mappedProjectGraph = mapProjectGraphFiles(projectGraph);
+
   const rule = new Rule(
     options,
     `${process.cwd()}/proj`,
     'mycompany',
-    projectGraph,
-    new TargetProjectLocator(projectGraph.nodes)
+    mappedProjectGraph,
+    new TargetProjectLocator(mappedProjectGraph.nodes)
   );
   return rule.apply(sourceFile);
 }

packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts

@@ -13,6 +13,8 @@ import {
   hasNoneOfTheseTags,
   isAbsoluteImportIntoAnotherProject,
   isRelativeImportIntoAnotherProject,
+  MappedProjectGraph,
+  mapProjectGraphFiles,
   matchImportWithWildcard,
   onlyLoadChildren,
 } from '../utils/runtime-lint-utils';
@@ -28,7 +30,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     options: IOptions,
     private readonly projectPath?: string,
     private readonly npmScope?: string,
-    private readonly projectGraph?: ProjectGraph,
+    private readonly projectGraph?: MappedProjectGraph,
     private readonly targetProjectLocator?: TargetProjectLocator
   ) {
     super(options);
@@ -38,10 +40,12 @@ export class Rule extends Lint.Rules.AbstractRule {
       if (!(global as any).projectGraph) {
         const nxJson = readNxJson();
         (global as any).npmScope = nxJson.npmScope;
-        (global as any).projectGraph = readCurrentProjectGraph();
+        (global as any).projectGraph = mapProjectGraphFiles(
+          readCurrentProjectGraph()
+        );
       }
       this.npmScope = (global as any).npmScope;
-      this.projectGraph = (global as any).projectGraph;
+      this.projectGraph = (global as any).projectGraph as MappedProjectGraph;
 
       if (!(global as any).targetProjectLocator && this.projectGraph) {
         (global as any).targetProjectLocator = new TargetProjectLocator(
@@ -109,16 +113,20 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
       return;
     }
 
+    const filePath = getSourceFilePath(
+      this.getSourceFile().fileName,
+      this.projectPath
+    );
+    const sourceProject = findSourceProject(this.projectGraph, filePath);
+
     // check for relative and absolute imports
     if (
       isRelativeImportIntoAnotherProject(
         imp,
         this.projectPath,
         this.projectGraph,
-        getSourceFilePath(
-          normalize(this.getSourceFile().fileName),
-          this.projectPath
-        )
+        filePath,
+        sourceProject
       ) ||
       isAbsoluteImportIntoAnotherProject(imp)
     ) {
@@ -130,12 +138,6 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
       return;
     }
 
-    const filePath = getSourceFilePath(
-      this.getSourceFile().fileName,
-      this.projectPath
-    );
-
-    const sourceProject = findSourceProject(this.projectGraph, filePath);
     const targetProject = findProjectUsingImport(
       this.projectGraph,
       this.targetProjectLocator,

packages/workspace/src/utilities/fileutils.ts

@@ -47,7 +47,11 @@ export function readJsonFile<T = any>(path: string): T {
 }
 
 export function parseJsonWithComments<T = any>(content: string): T {
-  return JSON.parse(stripJsonComments(content));
+  try {
+    return JSON.parse(content);
+  } catch {
+    return JSON.parse(stripJsonComments(content));
+  }
 }
 
 export function writeJsonFile(path: string, json: any) {

packages/workspace/src/utils/runtime-lint-utils.ts

@@ -7,7 +7,22 @@ import {
   ProjectGraphNode,
 } from '../core/project-graph';
 import { TargetProjectLocator } from '../core/target-project-locator';
-import { normalizePath } from '@nrwl/devkit';
+import { normalizePath, TargetConfiguration } from '@nrwl/devkit';
+
+export interface MappedProjectGraphNode<T = any> {
+  type: string;
+  name: string;
+  data: T & {
+    root?: string;
+    targets?: { [targetName: string]: TargetConfiguration };
+    files: Record<string, FileData>;
+  };
+}
+export interface MappedProjectGraph<T = any> {
+  nodes: Record<string, MappedProjectGraphNode<T>>;
+  dependencies: Record<string, ProjectGraphDependency[]>;
+  allWorkspaceFiles?: FileData[];
+}
 
 export type Deps = { [projectName: string]: ProjectGraphDependency[] };
 export type DepConstraint = {
@@ -15,7 +30,10 @@ export type DepConstraint = {
   onlyDependOnLibsWithTags: string[];
 };
 
-export function hasNoneOfTheseTags(proj: ProjectGraphNode, tags: string[]) {
+export function hasNoneOfTheseTags(
+  proj: ProjectGraphNode<any>,
+  tags: string[]
+) {
   return tags.filter((allowedTag) => hasTag(proj, allowedTag)).length === 0;
 }
 
@@ -23,15 +41,6 @@ function hasTag(proj: ProjectGraphNode, tag: string) {
   return (proj.data.tags || []).indexOf(tag) > -1 || tag === '*';
 }
 
-function containsFile(
-  files: FileData[],
-  targetFileWithoutExtension: string
-): boolean {
-  return !!files.filter(
-    (f) => removeExt(f.file) === targetFileWithoutExtension
-  )[0];
-}
-
 function removeExt(file: string): string {
   return file.replace(/\.[^/.]+$/, '');
 }
@@ -66,7 +75,8 @@ export function isRelativeImportIntoAnotherProject(
   imp: string,
   projectPath: string,
   projectGraph: ProjectGraph,
-  sourceFilePath: string
+  sourceFilePath: string,
+  sourceProject: ProjectGraphNode
 ): boolean {
   if (!isRelative(imp)) return false;
 
@@ -74,19 +84,19 @@ export function isRelativeImportIntoAnotherProject(
     path.resolve(path.join(projectPath, path.dirname(sourceFilePath)), imp)
   ).substring(projectPath.length + 1);
 
-  const sourceProject = findSourceProject(projectGraph, sourceFilePath);
   const targetProject = findTargetProject(projectGraph, targetFile);
   return sourceProject && targetProject && sourceProject !== targetProject;
 }
 
-export function findProjectUsingFile(projectGraph: ProjectGraph, file: string) {
-  return Object.values(projectGraph.nodes).filter((n) =>
-    containsFile(n.data.files, file)
-  )[0];
+export function findProjectUsingFile<T>(
+  projectGraph: MappedProjectGraph<T>,
+  file: string
+): MappedProjectGraphNode {
+  return Object.values(projectGraph.nodes).find((n) => n.data.files[file]);
 }
 
 export function findSourceProject(
-  projectGraph: ProjectGraph,
+  projectGraph: MappedProjectGraph,
   sourceFilePath: string
 ) {
   const targetFile = removeExt(sourceFilePath);
@@ -180,3 +190,26 @@ export function hasBuildExecutor(projectGraph: ProjectGraphNode): boolean {
     projectGraph.data.targets.build.executor !== ''
   );
 }
+
+export function mapProjectGraphFiles<T>(
+  projectGraph: ProjectGraph<T>
+): MappedProjectGraph | null {
+  if (!projectGraph) {
+    return null;
+  }
+  const nodes: Record<string, MappedProjectGraphNode> = {};
+  Object.entries(projectGraph.nodes).forEach(([name, node]) => {
+    const files: Record<string, FileData> = {};
+    node.data.files.forEach(({ file, hash, ext }) => {
+      files[file.slice(0, -ext.length)] = { file, hash, ext };
+    });
+    const data = { ...node.data, files };
+
+    nodes[name] = { ...node, data };
+  });
+
+  return {
+    ...projectGraph,
+    nodes,
+  };
+}