From 6d22f628b57f06fb05c6c6beeb2ccb832b33bedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Jona=C5=A1?= Date: Tue, 11 May 2021 18:39:12 +0200 Subject: [PATCH] fix(linter): add missing self-circular check for tslint (#5613) * fix(linter): add missing self-circular check for tslint * feat(linter): add test for tslint self-circular deps --- .../src/rules/enforce-module-boundaries.ts | 1 + .../nxEnforceModuleBoundariesRule.spec.ts | 118 +++++++++++++++++- .../tslint/nxEnforceModuleBoundariesRule.ts | 19 +-- 3 files changed, 129 insertions(+), 9 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 index fbd9ab220b..6939598f35 100644 --- a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts +++ b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts @@ -64,6 +64,7 @@ export default createESLintRule({ type: 'object', properties: { enforceBuildableLibDependency: { type: 'boolean' }, + allowCircularSelfDependency: { type: 'boolean' }, allow: [{ type: 'string' }], depConstraints: [ { diff --git a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.spec.ts b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.spec.ts index 619a6b1677..0006c22ce8 100644 --- a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.spec.ts +++ b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.spec.ts @@ -1,5 +1,5 @@ import { vol } from 'memfs'; -import { extname, join } from 'path'; +import { extname } from 'path'; import { RuleFailure } from 'tslint'; import * as ts from 'typescript'; import { @@ -9,7 +9,6 @@ import { } from '../core/project-graph'; import { Rule } from './nxEnforceModuleBoundariesRule'; import { TargetProjectLocator } from '../core/target-project-locator'; -import { readFileSync } from 'fs'; jest.mock('fs', () => require('memfs').fs); jest.mock('../utilities/app-root', () => ({ appRootPath: '/root' })); @@ -726,6 +725,121 @@ describe('Enforce Module Boundaries', () => { ); }); + it('should error when absolute path within project detected', () => { + const failures = runRule( + {}, + `${process.cwd()}/proj/libs/mylib/src/main.ts`, + 'import "@mycompany/mylib"', + { + nodes: { + mylibName: { + name: 'mylibName', + type: ProjectType.lib, + data: { + root: 'libs/mylib', + tags: [], + implicitDependencies: [], + targets: {}, + files: [createFile(`libs/mylib/src/main.ts`)], + }, + }, + anotherlibName: { + name: 'anotherlibName', + type: ProjectType.lib, + data: { + root: 'libs/anotherlib', + tags: [], + implicitDependencies: [], + targets: {}, + files: [createFile(`libs/anotherlib/src/main.ts`)], + }, + }, + myappName: { + name: 'myappName', + type: ProjectType.app, + data: { + root: 'apps/myapp', + tags: [], + implicitDependencies: [], + targets: {}, + files: [createFile(`apps/myapp/src/index.ts`)], + }, + }, + }, + dependencies: { + mylibName: [ + { + source: 'mylibName', + target: 'anotherlibName', + type: DependencyType.static, + }, + ], + }, + } + ); + const message = + 'Only relative imports are allowed within the project. Absolute import found: @mycompany/mylib'; + expect(failures.length).toEqual(1); + expect(failures[0].getFailure()).toEqual(message); + }); + + it('should ignore detected absolute path within project if allowCircularSelfDependency flag is set', () => { + const failures = runRule( + { + allowCircularSelfDependency: true, + }, + `${process.cwd()}/proj/libs/mylib/src/main.ts`, + 'import "@mycompany/mylib"', + { + nodes: { + mylibName: { + name: 'mylibName', + type: ProjectType.lib, + data: { + root: 'libs/mylib', + tags: [], + implicitDependencies: [], + targets: {}, + files: [createFile(`libs/mylib/src/main.ts`)], + }, + }, + anotherlibName: { + name: 'anotherlibName', + type: ProjectType.lib, + data: { + root: 'libs/anotherlib', + tags: [], + implicitDependencies: [], + targets: {}, + files: [createFile(`libs/anotherlib/src/main.ts`)], + }, + }, + myappName: { + name: 'myappName', + type: ProjectType.app, + data: { + root: 'apps/myapp', + tags: [], + implicitDependencies: [], + targets: {}, + files: [createFile(`apps/myapp/src/index.ts`)], + }, + }, + }, + dependencies: { + mylibName: [ + { + source: 'mylibName', + target: 'anotherlibName', + type: DependencyType.static, + }, + ], + }, + } + ); + expect(failures.length).toEqual(0); + }); + it('should error when circular dependency detected', () => { const failures = runRule( {}, diff --git a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts index 2cb0f55375..a34d091111 100644 --- a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts +++ b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts @@ -1,12 +1,7 @@ import * as Lint from 'tslint'; import { IOptions } from 'tslint'; import * as ts from 'typescript'; -import { - createProjectGraph, - isNpmProject, - ProjectGraph, - ProjectType, -} from '../core/project-graph'; +import { isNpmProject, ProjectGraph, ProjectType } from '../core/project-graph'; import { appRootPath } from '../utilities/app-root'; import { DepConstraint, @@ -26,6 +21,7 @@ import { readNxJson } from '@nrwl/workspace/src/core/file-utils'; import { TargetProjectLocator } from '../core/target-project-locator'; import { checkCircularPath } from '@nrwl/workspace/src/utils/graph-utils'; import { readCurrentProjectGraph } from '@nrwl/workspace/src/core/project-graph/project-graph'; +import { isRelativePath } from '../utilities/fileutils'; export class Rule extends Lint.Rules.AbstractRule { constructor( @@ -75,6 +71,7 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker { private readonly allow: string[]; private readonly enforceBuildableLibDependency: boolean = false; // for backwards compat private readonly depConstraints: DepConstraint[]; + private readonly allowCircularSelfDependency: boolean = false; constructor( sourceFile: ts.SourceFile, @@ -96,6 +93,9 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker { this.enforceBuildableLibDependency = this.getOptions()[0].enforceBuildableLibDependency === true; + + this.allowCircularSelfDependency = + this.getOptions()[0].allowCircularSelfDependency === true; } public visitImportDeclaration(node: ts.ImportDeclaration) { @@ -152,7 +152,12 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker { // same project => allow if (sourceProject === targetProject) { - super.visitImportDeclaration(node); + if (!this.allowCircularSelfDependency && !isRelativePath(imp)) { + const error = `Only relative imports are allowed within the project. Absolute import found: ${imp}`; + this.addFailureAt(node.getStart(), node.getWidth(), error); + } else { + super.visitImportDeclaration(node); + } return; }