From 9644840e4c5f4790605850b85d9c532ca4437aa5 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 29 Aug 2017 01:29:57 +0200 Subject: [PATCH] build: write custom tslint rules in typescript (#6650) Currently the custom TSLint rules of the repository had to be pushed as JavaScript files because TSLint was only able to run them as JavaScript files. Recently with TSLint 5.7.0 (https://github.com/palantir/tslint/commit/90dd3f472199c68662eaecc47663969cb8ea6ed1), custom rules can be also loaded directly as TypeScript files in combination with ts-node. --- package-lock.json | 22 ++-- package.json | 2 +- tools/gulp/tasks/lint.ts | 15 ++- ...alsRule.js => missingRollupGlobalsRule.ts} | 110 +++++++++--------- ...xposedTodoRule.js => noExposedTodoRule.ts} | 18 ++- ...portsRule.js => noRxjsPatchImportsRule.ts} | 85 +++++++------- ...tionRule.js => noViewEncapsulationRule.ts} | 29 +++-- ...nerRule.js => requireLicenseBannerRule.ts} | 31 +++-- 8 files changed, 164 insertions(+), 148 deletions(-) rename tools/tslint-rules/{missingRollupGlobalsRule.js => missingRollupGlobalsRule.ts} (58%) rename tools/tslint-rules/{noExposedTodoRule.js => noExposedTodoRule.ts} (66%) rename tools/tslint-rules/{noRxjsPatchImportsRule.js => noRxjsPatchImportsRule.ts} (57%) rename tools/tslint-rules/{noViewEncapsulationRule.js => noViewEncapsulationRule.ts} (67%) rename tools/tslint-rules/{requireLicenseBannerRule.js => requireLicenseBannerRule.ts} (73%) diff --git a/package-lock.json b/package-lock.json index 0adba8c06cc..6170f0d3035 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1166,9 +1166,9 @@ } }, "babel-code-frame": { - "version": "6.22.0", - "resolved": "https://registry.npmjs.org/babel-code-frame/-/babel-code-frame-6.22.0.tgz", - "integrity": "sha1-AnYgvuVnqIwyVhV05/0IAdMxGOQ=", + "version": "6.26.0", + "resolved": "https://registry.npmjs.org/babel-code-frame/-/babel-code-frame-6.26.0.tgz", + "integrity": "sha1-Y/1D99weO7fONZR9uP42mj9Yx0s=", "dev": true, "requires": { "chalk": "1.1.3", @@ -15512,12 +15512,12 @@ "integrity": "sha1-vIAEFkaRkjp5/oN4u+s9ogF1OOw=" }, "tslint": { - "version": "5.6.0", - "resolved": "https://registry.npmjs.org/tslint/-/tslint-5.6.0.tgz", - "integrity": "sha1-CIqmxgJmIzOGULKQCCirPt9Z9s8=", + "version": "5.7.0", + "resolved": "https://registry.npmjs.org/tslint/-/tslint-5.7.0.tgz", + "integrity": "sha1-wl4NDJL6EgHCvDDoROCOaCtPNVI=", "dev": true, "requires": { - "babel-code-frame": "6.22.0", + "babel-code-frame": "6.26.0", "colors": "1.1.2", "commander": "2.11.0", "diff": "3.3.0", @@ -15526,7 +15526,7 @@ "resolve": "1.3.3", "semver": "5.3.0", "tslib": "1.7.1", - "tsutils": "2.8.0" + "tsutils": "2.8.1" }, "dependencies": { "colors": { @@ -15536,9 +15536,9 @@ "dev": true }, "tsutils": { - "version": "2.8.0", - "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-2.8.0.tgz", - "integrity": "sha1-AWAXNymzvxOGKN0UoVN+AIUdgUo=", + "version": "2.8.1", + "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-2.8.1.tgz", + "integrity": "sha1-N3FATnyp8L7fXZGaR6SxiQpo7/8=", "dev": true, "requires": { "tslib": "1.7.1" diff --git a/package.json b/package.json index a84fc6435c9..3ad917bf39e 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,7 @@ "stylelint": "^7.12.0", "ts-node": "^3.0.4", "tsconfig-paths": "^2.2.0", - "tslint": "~5.6.0", + "tslint": "^5.7.0", "tsutils": "^2.6.0", "typescript": "~2.2.1", "uglify-js": "^2.8.14", diff --git a/tools/gulp/tasks/lint.ts b/tools/gulp/tasks/lint.ts index b51b84ba654..b81ce409387 100644 --- a/tools/gulp/tasks/lint.ts +++ b/tools/gulp/tasks/lint.ts @@ -6,6 +6,7 @@ import {red} from 'chalk'; // These types lack of type definitions const madge = require('madge'); +const resolveBin = require('resolve-bin'); /** Glob that matches all SCSS or CSS files that should be linted. */ const stylesGlob = '+(tools|src)/**/!(*.bundle).+(css|scss)'; @@ -27,10 +28,10 @@ task('stylelint', execNodeTask( )); /** Task to run TSLint against the e2e/ and src/ directories. */ -task('tslint', execNodeTask('tslint', tsLintBaseFlags)); +task('tslint', execTsLintTask()); /** Task that automatically fixes TSLint warnings. */ -task('tslint:fix', execNodeTask('tslint', [...tsLintBaseFlags, '--fix'])); +task('tslint:fix', execTsLintTask('--fix')); /** Task that runs madge to detect circular dependencies. */ task('madge', ['material:clean-build'], () => { @@ -50,3 +51,13 @@ task('madge', ['material:clean-build'], () => { function formatMadgeCircularModules(circularModules: string[][]): string { return circularModules.map((modulePaths: string[]) => `\n - ${modulePaths.join(' > ')}`).join(''); } + +/** Creates a gulp task function that will run TSLint together with ts-node. */ +function execTsLintTask(...flags: string[]) { + const tslintBinPath = resolveBin.sync('tslint'); + const tsNodeOptions = ['-O', '{"module": "commonjs"}']; + + // TS-Node needs the module compiler option to be set to `commonjs` because the transpiled + // TypeScript files will be running inside of NodeJS. + return execNodeTask('ts-node', [...tsNodeOptions, tslintBinPath, ...tsLintBaseFlags, ...flags]); +} diff --git a/tools/tslint-rules/missingRollupGlobalsRule.js b/tools/tslint-rules/missingRollupGlobalsRule.ts similarity index 58% rename from tools/tslint-rules/missingRollupGlobalsRule.js rename to tools/tslint-rules/missingRollupGlobalsRule.ts index da87f5811c3..ef85610473e 100644 --- a/tools/tslint-rules/missingRollupGlobalsRule.js +++ b/tools/tslint-rules/missingRollupGlobalsRule.ts @@ -1,54 +1,56 @@ -const path = require('path'); -const Lint = require('tslint'); -const minimatch = require('minimatch'); - -// Since the packaging is based on TypeScript and is only compiled at run-time using ts-node, the -// custom TSLint rule is not able to read the map of rollup globals. Because the custom rules -// for TSLint are written in JavaScript we also need to use ts-node here to read the globals. -require('ts-node').register({ - project: path.join(__dirname, '../gulp/tsconfig.json') -}); - -/** - * Rule that enforces that the specified external packages have been included in our Rollup config. - * Usage: [true, './path/to/rollup/config.json'] - */ -class Rule extends Lint.Rules.AbstractRule { - apply(file) { - return this.applyWithWalker(new Walker(file, this.getOptions())); - } -} - -class Walker extends Lint.RuleWalker { - constructor(file, options) { - super(...arguments); - - if (!options.ruleArguments.length) { - throw Error('missing-rollup-globals: The Rollup config path has to be specified.'); - } - - const [configPath, ...fileGlobs] = options.ruleArguments; - - // Relative path for the current TypeScript source file. - const relativeFilePath = path.relative(process.cwd(), file.fileName); - - this._configPath = path.resolve(process.cwd(), configPath); - this._config = require(this._configPath).rollupGlobals; - this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); - } - - visitImportDeclaration(node) { - // Parse out the module name. The first and last characters are the quote marks. - const module = node.moduleSpecifier.getText().slice(1, -1); - const isExternal = !module.startsWith('.') && !module.startsWith('/'); - - // Check whether the module is external and whether it's in our config. - if (this._enabled && isExternal && !this._config[module]) { - this.addFailureAtNode(node, `Module "${module}" is missing from file ${this._configPath}.`); - } - - super.visitImportDeclaration(node); - } -} - -exports.Rule = Rule; +import * as path from 'path'; +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as minimatch from 'minimatch'; + +/** + * Rule that enforces that the specified external packages have been included in our Rollup config. + * Usage: [true, './path/to/rollup/config.json'] + */ +export class Rule extends Lint.Rules.AbstractRule { + apply(sourceFile: ts.SourceFile) { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions())); + } +} + +class Walker extends Lint.RuleWalker { + + /** Path to the rollup globals configuration file. */ + private _configPath: string; + + /** Rollup globals configuration object. */ + private _config: {[globalName: string]: string}; + + /** Whether the walker should check the current source file. */ + private _enabled: boolean; + + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { + super(sourceFile, options); + + if (!options.ruleArguments.length) { + throw Error('missing-rollup-globals: The Rollup config path has to be specified.'); + } + + const [configPath, ...fileGlobs] = options.ruleArguments; + + // Relative path for the current TypeScript source file. + const relativeFilePath = path.relative(process.cwd(), sourceFile.fileName); + + this._configPath = path.resolve(process.cwd(), configPath); + this._config = require(this._configPath).rollupGlobals; + this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); + } + + visitImportDeclaration(node: ts.ImportDeclaration) { + // Parse out the module name. The first and last characters are the quote marks. + const module = node.moduleSpecifier.getText().slice(1, -1); + const isExternal = !module.startsWith('.') && !module.startsWith('/'); + + // Check whether the module is external and whether it's in our config. + if (this._enabled && isExternal && !this._config[module]) { + this.addFailureAtNode(node, `Module "${module}" is missing from file ${this._configPath}.`); + } + + super.visitImportDeclaration(node); + } +} diff --git a/tools/tslint-rules/noExposedTodoRule.js b/tools/tslint-rules/noExposedTodoRule.ts similarity index 66% rename from tools/tslint-rules/noExposedTodoRule.js rename to tools/tslint-rules/noExposedTodoRule.ts index 01e96b593f3..ab090f82ab8 100644 --- a/tools/tslint-rules/noExposedTodoRule.js +++ b/tools/tslint-rules/noExposedTodoRule.ts @@ -1,6 +1,6 @@ -const ts = require('typescript'); -const utils = require('tsutils'); -const Lint = require('tslint'); +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as utils from 'tsutils'; const ERROR_MESSAGE = 'A TODO may only appear in inline (//) style comments. ' + @@ -11,18 +11,18 @@ const ERROR_MESSAGE = * detects TODO's inside of multi-line comments. TODOs need to be placed inside of single-line * comments. */ -class Rule extends Lint.Rules.AbstractRule { +export class Rule extends Lint.Rules.AbstractRule { - apply(sourceFile) { + apply(sourceFile: ts.SourceFile) { return this.applyWithWalker(new NoExposedTodoWalker(sourceFile, this.getOptions())); } } class NoExposedTodoWalker extends Lint.RuleWalker { - visitSourceFile(sourceFile) { - utils.forEachComment(sourceFile, (fullText, commentRange) => { - let isTodoComment = fullText.substring(commentRange.pos, commentRange.end).includes('TODO'); + visitSourceFile(sourceFile: ts.SourceFile) { + utils.forEachComment(sourceFile, (text, commentRange) => { + const isTodoComment = text.substring(commentRange.pos, commentRange.end).includes('TODO:'); if (commentRange.kind === ts.SyntaxKind.MultiLineCommentTrivia && isTodoComment) { this.addFailureAt(commentRange.pos, commentRange.end - commentRange.pos, ERROR_MESSAGE); @@ -32,5 +32,3 @@ class NoExposedTodoWalker extends Lint.RuleWalker { super.visitSourceFile(sourceFile); } } - -exports.Rule = Rule; diff --git a/tools/tslint-rules/noRxjsPatchImportsRule.js b/tools/tslint-rules/noRxjsPatchImportsRule.ts similarity index 57% rename from tools/tslint-rules/noRxjsPatchImportsRule.js rename to tools/tslint-rules/noRxjsPatchImportsRule.ts index 7afb1fa248e..233e346d93a 100644 --- a/tools/tslint-rules/noRxjsPatchImportsRule.js +++ b/tools/tslint-rules/noRxjsPatchImportsRule.ts @@ -1,41 +1,44 @@ -const Lint = require('tslint'); -const minimatch = require('minimatch'); -const path = require('path'); - -const ERROR_MESSAGE = 'Uses of RxJS patch imports are forbidden.'; - -/** - * Rule that prevents uses of RxJS patch imports (e.g. `import 'rxjs/add/operator/map'). - * Supports whitelisting via `"no-patch-imports": [true, "\.spec\.ts$"]`. - */ -class Rule extends Lint.Rules.AbstractRule { - apply(file) { - return this.applyWithWalker(new Walker(file, this.getOptions())); - } -} - -class Walker extends Lint.RuleWalker { - constructor(file, options) { - super(...arguments); - - // 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(), file.fileName); - - // Whether the file should be checked at all. - this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); - } - - visitImportDeclaration(node) { - // Walk through the imports and check if they start with `rxjs/add`. - if (this._enabled && node.moduleSpecifier.getText().startsWith('rxjs/add', 1)) { - this.addFailureAtNode(node, ERROR_MESSAGE); - } - - super.visitImportDeclaration(node); - } -} - -exports.Rule = Rule; +import * as path from 'path'; +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as minimatch from 'minimatch'; + +const ERROR_MESSAGE = 'Uses of RxJS patch imports are forbidden.'; + +/** + * Rule that prevents uses of RxJS patch imports (e.g. `import 'rxjs/add/operator/map'). + * Supports whitelisting via `"no-patch-imports": [true, "\.spec\.ts$"]`. + */ +export class Rule extends Lint.Rules.AbstractRule { + apply(sourceFile: ts.SourceFile) { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions())); + } +} + +class Walker extends Lint.RuleWalker { + + /** Whether the walker should check the current source file. */ + private _enabled: boolean; + + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { + 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); + + // Whether the file should be checked at all. + this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); + } + + visitImportDeclaration(node: ts.ImportDeclaration) { + // Walk through the imports and check if they start with `rxjs/add`. + if (this._enabled && node.moduleSpecifier.getText().startsWith('rxjs/add', 1)) { + this.addFailureAtNode(node, ERROR_MESSAGE); + } + + super.visitImportDeclaration(node); + } +} diff --git a/tools/tslint-rules/noViewEncapsulationRule.js b/tools/tslint-rules/noViewEncapsulationRule.ts similarity index 67% rename from tools/tslint-rules/noViewEncapsulationRule.js rename to tools/tslint-rules/noViewEncapsulationRule.ts index 60254387bd8..9b130174d50 100644 --- a/tools/tslint-rules/noViewEncapsulationRule.js +++ b/tools/tslint-rules/noViewEncapsulationRule.ts @@ -1,6 +1,7 @@ -const Lint = require('tslint'); -const path = require('path'); -const minimatch = require('minimatch'); +import * as path from 'path'; +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as minimatch from 'minimatch'; const ERROR_MESSAGE = 'Components must turn off view encapsulation.'; @@ -10,28 +11,34 @@ const ERROR_MESSAGE = 'Components must turn off view encapsulation.'; * Rule that enforces that view encapsulation is turned off on all components. * Files can be whitelisted via `"no-view-encapsulation": [true, "\.spec\.ts$"]`. */ -class Rule extends Lint.Rules.AbstractRule { - apply(file) { - return this.applyWithWalker(new Walker(file, this.getOptions())); +export class Rule extends Lint.Rules.AbstractRule { + apply(sourceFile: ts.SourceFile) { + return this.applyWithWalker(new Walker(sourceFile, this.getOptions())); } } class Walker extends Lint.RuleWalker { - constructor(file, options) { - super(...arguments); + + /** Whether the walker should check the current source file. */ + private _enabled: boolean; + + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { + 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(), file.fileName); + const relativeFilePath = path.relative(process.cwd(), sourceFile.fileName); // Whether the file should be checked at all. this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); } visitClassDeclaration(node) { - if (!this._enabled || !node.decorators) return; + if (!this._enabled || !node.decorators) { + return; + } node.decorators .map(decorator => decorator.expression) @@ -50,5 +57,3 @@ class Walker extends Lint.RuleWalker { } } - -exports.Rule = Rule; diff --git a/tools/tslint-rules/requireLicenseBannerRule.js b/tools/tslint-rules/requireLicenseBannerRule.ts similarity index 73% rename from tools/tslint-rules/requireLicenseBannerRule.js rename to tools/tslint-rules/requireLicenseBannerRule.ts index 668dbb09fe2..11a5dbfbaf1 100644 --- a/tools/tslint-rules/requireLicenseBannerRule.js +++ b/tools/tslint-rules/requireLicenseBannerRule.ts @@ -1,13 +1,9 @@ -const Lint = require('tslint'); -const path = require('path'); -const minimatch = require('minimatch'); -const buildConfig = require('../../build-config'); +import * as path from 'path'; +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as minimatch from 'minimatch'; -/** Paths to the directories that are public packages and should be validated. */ -const packageDirs = [ - path.join(buildConfig.packagesDir, 'lib'), - path.join(buildConfig.packagesDir, 'cdk') -]; +const buildConfig = require('../../build-config'); /** License banner that is placed at the top of every public TypeScript file. */ const licenseBanner = buildConfig.licenseBanner; @@ -23,29 +19,32 @@ const tslintFix = Lint.Replacement.appendText(0, licenseBanner + '\n\n'); * Rule that walks through all TypeScript files of public packages and shows failures if a * file does not have the license banner at the top of the file. */ -class Rule extends Lint.Rules.AbstractRule { +export class Rule extends Lint.Rules.AbstractRule { - apply(sourceFile) { + apply(sourceFile: ts.SourceFile) { return this.applyWithWalker(new RequireLicenseBannerWalker(sourceFile, this.getOptions())); } } class RequireLicenseBannerWalker extends Lint.RuleWalker { - constructor(file, options) { - super(...arguments); + /** Whether the walker should check the current source file. */ + private _enabled: boolean; + + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { + 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(), file.fileName); + const relativeFilePath = path.relative(process.cwd(), sourceFile.fileName); // Whether the file should be checked at all. this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p)); } - visitSourceFile(sourceFile) { + visitSourceFile(sourceFile: ts.SourceFile) { if (!this._enabled) { return; } @@ -60,5 +59,3 @@ class RequireLicenseBannerWalker extends Lint.RuleWalker { super.visitSourceFile(sourceFile); } } - -exports.Rule = Rule;