From 8f5e04b1d741fa01aea9cd264dcead1368af3041 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 23 Nov 2023 14:10:13 +0100 Subject: [PATCH] build: custom lint rules not working locally We have some custom lint rules that only apply to specific files. Currently the filtering is implemented by opting in specific files through a glob pattern. We do so by converting the absolute `SourceFile.fileName` to a relative one and checking it against the pattern. The problem is that in some recent version either the OS or TypeScript started returning lower-cased paths in `SourceFile.fileName` while `process.cwd()` which we resolve the path against is case-sensitive. This ends up producing an invalid path which doesn't get covered by the lint rules. These changes fix the issue by having custom rules apply to all files and using the glob pattern to _exclude_ some of them. --- test/angular-test-init-spec.ts | 8 ++++++ tools/tslint-rules/lightweightTokensRule.ts | 9 ++----- .../noCrossEntryPointRelativeImportsRule.ts | 11 +++----- .../tslint-rules/noLifecycleInvocationRule.ts | 6 ++--- .../tslint-rules/requireLicenseBannerRule.ts | 8 ++---- tools/tslint-rules/validateDecoratorsRule.ts | 11 +++----- tslint.json | 27 +++++++++++++------ 7 files changed, 41 insertions(+), 39 deletions(-) diff --git a/test/angular-test-init-spec.ts b/test/angular-test-init-spec.ts index dd6d36c2acfb..3e661ed09465 100644 --- a/test/angular-test-init-spec.ts +++ b/test/angular-test-init-spec.ts @@ -1,3 +1,11 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + import {TestBed} from '@angular/core/testing'; import { BrowserDynamicTestingModule, diff --git a/tools/tslint-rules/lightweightTokensRule.ts b/tools/tslint-rules/lightweightTokensRule.ts index e79beac5c2f5..dfad5089ed54 100644 --- a/tools/tslint-rules/lightweightTokensRule.ts +++ b/tools/tslint-rules/lightweightTokensRule.ts @@ -1,7 +1,5 @@ import ts from 'typescript'; import minimatch from 'minimatch'; - -import * as path from 'path'; import * as Lint from 'tslint'; /** Arguments this rule supports. */ @@ -43,10 +41,7 @@ export class Rule extends Lint.Rules.TypedRule { * for which lightweight tokens are suitable (for optimized tree shaking). */ function checkSourceFileForLightweightTokens(ctx: Context, typeChecker: ts.TypeChecker): void { - // Relative path for the current TypeScript source file. This allows for - // relative globs being used in the rule options. - const relativeFilePath = path.relative(process.cwd(), ctx.sourceFile.fileName); - const [enabledFilesGlobs] = ctx.options; + const [disabledFileGlobs] = ctx.options; const visitNode = (node: ts.Node) => { if (ts.isClassDeclaration(node)) { checkClassDeclarationForLightweightToken(node, ctx, typeChecker); @@ -55,7 +50,7 @@ function checkSourceFileForLightweightTokens(ctx: Context, typeChecker: ts.TypeC } }; - if (enabledFilesGlobs.some(g => minimatch(relativeFilePath, g))) { + if (!disabledFileGlobs.some(g => minimatch(ctx.sourceFile.fileName, g))) { ts.forEachChild(ctx.sourceFile, visitNode); } } diff --git a/tools/tslint-rules/noCrossEntryPointRelativeImportsRule.ts b/tools/tslint-rules/noCrossEntryPointRelativeImportsRule.ts index 5d8fa2d0f731..46c0197262d7 100644 --- a/tools/tslint-rules/noCrossEntryPointRelativeImportsRule.ts +++ b/tools/tslint-rules/noCrossEntryPointRelativeImportsRule.ts @@ -2,7 +2,7 @@ import ts from 'typescript'; import minimatch from 'minimatch'; import {existsSync} from 'fs'; -import {dirname, join, normalize, relative, resolve} from 'path'; +import {dirname, join, normalize, resolve} from 'path'; import * as Lint from 'tslint'; const BUILD_BAZEL_FILE = 'BUILD.bazel'; @@ -16,7 +16,7 @@ const BUILD_BAZEL_FILE = 'BUILD.bazel'; */ export class Rule extends Lint.Rules.AbstractRule { apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, checkSourceFile, this.getOptions().ruleArguments); + return this.applyWithFunction(sourceFile, checkSourceFile, this.getOptions().ruleArguments[0]); } } @@ -25,10 +25,7 @@ export class Rule extends Lint.Rules.AbstractRule { * with relative cross entry-point references. */ function checkSourceFile(ctx: Lint.WalkContext) { - const filePath = ctx.sourceFile.fileName; - const relativeFilePath = relative(process.cwd(), filePath); - - if (!ctx.options.every(o => minimatch(relativeFilePath, o))) { + if (ctx.options.some(o => minimatch(ctx.sourceFile.fileName, o))) { return; } @@ -43,7 +40,7 @@ function checkSourceFile(ctx: Lint.WalkContext) { } const modulePath = node.moduleSpecifier.text; - const basePath = dirname(filePath); + const basePath = dirname(ctx.sourceFile.fileName); const currentPackage = findClosestBazelPackage(basePath); const resolvedPackage = findClosestBazelPackage(resolve(basePath, modulePath)); diff --git a/tools/tslint-rules/noLifecycleInvocationRule.ts b/tools/tslint-rules/noLifecycleInvocationRule.ts index e97d712a6add..d0a7de6f5a80 100644 --- a/tools/tslint-rules/noLifecycleInvocationRule.ts +++ b/tools/tslint-rules/noLifecycleInvocationRule.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as Lint from 'tslint'; import ts from 'typescript'; import minimatch from 'minimatch'; @@ -28,9 +27,8 @@ class Walker extends Lint.RuleWalker { constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); - const fileGlobs = options.ruleArguments; - const relativeFilePath = path.relative(process.cwd(), sourceFile.fileName); - this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); + const fileGlobs: string[] = options.ruleArguments[0]; + this._enabled = !fileGlobs.some(p => minimatch(sourceFile.fileName, p)); } override visitPropertyAccessExpression(node: ts.PropertyAccessExpression) { diff --git a/tools/tslint-rules/requireLicenseBannerRule.ts b/tools/tslint-rules/requireLicenseBannerRule.ts index 17275f217686..1f74e093e316 100644 --- a/tools/tslint-rules/requireLicenseBannerRule.ts +++ b/tools/tslint-rules/requireLicenseBannerRule.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as Lint from 'tslint'; import minimatch from 'minimatch'; import ts from 'typescript'; @@ -38,13 +37,10 @@ class RequireLicenseBannerWalker extends Lint.RuleWalker { super(sourceFile, options); // Globs that are used to determine which files to lint. - const fileGlobs = options.ruleArguments; - - // Relative path for the current TypeScript source file. - const relativeFilePath = path.relative(process.cwd(), sourceFile.fileName); + const fileGlobs: string[] = options.ruleArguments[0]; // Whether the file should be checked at all. - this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); + this._enabled = !fileGlobs.some(p => minimatch(sourceFile.fileName, p)); } override visitSourceFile(sourceFile: ts.SourceFile) { diff --git a/tools/tslint-rules/validateDecoratorsRule.ts b/tools/tslint-rules/validateDecoratorsRule.ts index da60f19424cf..df19cc0ac835 100644 --- a/tools/tslint-rules/validateDecoratorsRule.ts +++ b/tools/tslint-rules/validateDecoratorsRule.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import * as Lint from 'tslint'; import ts from 'typescript'; import minimatch from 'minimatch'; @@ -69,15 +68,13 @@ class Walker extends Lint.RuleWalker { constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); - // Globs that are used to determine which files to lint. - const fileGlobs = options.ruleArguments.slice(1) || []; - - // Relative path for the current TypeScript source file. - const relativeFilePath = path.relative(process.cwd(), sourceFile.fileName); + // Globs that are used to determine which files to exclude from linting. + const fileGlobs: string[] = options.ruleArguments[1] || []; this._rules = this._generateRules(options.ruleArguments[0]); this._enabled = - Object.keys(this._rules).length > 0 && fileGlobs.some(p => minimatch(relativeFilePath, p)); + Object.keys(this._rules).length > 0 && + !fileGlobs.some(p => minimatch(sourceFile.fileName, p)); } override visitClassDeclaration(node: ts.ClassDeclaration) { diff --git a/tslint.json b/tslint.json index 30188ce90a60..f334365f7c03 100644 --- a/tslint.json +++ b/tslint.json @@ -69,10 +69,10 @@ "require-breaking-change-version": true, "no-nested-ternary": true, "prefer-plain-enum": true, - "no-lifecycle-invocation": [true, "**/!(*.spec).ts"], + "no-lifecycle-invocation": [true, ["**/*.spec.ts"]], "lightweight-tokens": [ true, - ["src/**/!(*.spec).ts"], + ["**/*.spec.ts"], // Directionality is always used in Angular Material and therefore does not // need a lightweight token. Date Adapter is not very significant and does not // need a dedicated token. @@ -120,18 +120,29 @@ } } }, - "src/!(e2e-app|components-examples|universal-app|dev-app)/**/!(*.spec).ts" + [ + // Internal code that doesn't need to follow the same standards. + "**/+(e2e-app|components-examples|universal-app|dev-app|integration)/**", + "**/*.spec.ts" + ] ], "require-license-banner": [ true, - "src/!(e2e-app|components-examples|universal-app)/**/!(*.spec).ts" + [ + // Internal files that don't need license banners. + "**/+(e2e-app|components-examples|universal-app|dev-app|integration|tools|scripts)/**", + "**/*.spec.ts" + ] ], "no-cross-entry-point-relative-imports": [ true, - "src/!(e2e-app|components-examples|universal-app|dev-app)/**/!(*.spec).ts", - "!src/+(cdk|material)/schematics/**/*", - "!src/cdk/testing/+(private|tests)/**/*", - "!src/google-maps/testing/**/*" + [ + // Files that we don't publish to npm so the relative imports don't matter. + "**/+(dev-app|components-examples|schematics|tools)/**", + "**/google-maps/testing/**", + "**/cdk/testing/+(tests|private)/**", + "**/*.spec.ts" + ] ], "file-name-casing": [ true,