From 0664beb76da87cb0c8b39c0029dffb3959faadcc Mon Sep 17 00:00:00 2001 From: Stephen Cavaliere Date: Sat, 28 Jan 2017 12:34:53 -0500 Subject: [PATCH] refactor(lint): use tslint api for linting (#4248) Closes #867, #3993 BREAKING CHANGE: In order to use the updated `ng lint` command, the following section will have to be added to the project's `angular-cli.json` at the root level of the json object. ```json "lint": [ { "files": "src/**/*.ts", "project": "src/tsconfig.json" }, { "files": "e2e/**/*.ts", "project": "e2e/tsconfig.json" } ], ``` Alternatively, you can run `ng update`. --- docs/documentation/lint.md | 12 +++- .../blueprints/ng2/files/angular-cli.json | 10 +++ .../blueprints/ng2/files/package.json | 1 - packages/angular-cli/commands/lint.ts | 17 ++++- packages/angular-cli/lib/config/schema.json | 28 ++++++++- packages/angular-cli/tasks/lint.ts | 63 +++++++++++++++---- packages/angular-cli/tasks/test.ts | 8 +-- .../utilities/require-project-module.ts | 8 +++ .../e2e/tests/lint/lint-no-config-section.ts | 28 +++++++++ tests/e2e/tests/lint/lint-with-fix.ts | 16 +++++ tests/e2e/tests/lint/lint-with-force.ts | 26 ++++++++ tests/e2e/tests/lint/lint-with-format.ts | 19 ++++++ tests/e2e/tests/lint/lint.ts | 14 +++++ tests/e2e/tests/test/lint.ts | 6 -- 14 files changed, 224 insertions(+), 32 deletions(-) create mode 100644 packages/angular-cli/utilities/require-project-module.ts create mode 100644 tests/e2e/tests/lint/lint-no-config-section.ts create mode 100644 tests/e2e/tests/lint/lint-with-fix.ts create mode 100644 tests/e2e/tests/lint/lint-with-force.ts create mode 100644 tests/e2e/tests/lint/lint-with-format.ts create mode 100644 tests/e2e/tests/lint/lint.ts delete mode 100644 tests/e2e/tests/test/lint.ts diff --git a/docs/documentation/lint.md b/docs/documentation/lint.md index 189451c4d7cc..16ee05dd9dc2 100644 --- a/docs/documentation/lint.md +++ b/docs/documentation/lint.md @@ -1,8 +1,14 @@ + + # ng lint ## Overview -`ng lint` will lint your app code. +`ng lint` will lint you app code using tslint. + +## Options + +`--fix` will attempt to fix lint errors -This will use the `lint` npm script that in generated projects uses `tslint`. +`--force` will always return error code 0 even with lint errors -You can modify the these scripts in `package.json` to run whatever tool you prefer. \ No newline at end of file +`--format` (`-t`) the output formatter to use diff --git a/packages/angular-cli/blueprints/ng2/files/angular-cli.json b/packages/angular-cli/blueprints/ng2/files/angular-cli.json index ea13f8072b9f..406e9a620962 100644 --- a/packages/angular-cli/blueprints/ng2/files/angular-cli.json +++ b/packages/angular-cli/blueprints/ng2/files/angular-cli.json @@ -33,6 +33,16 @@ "config": "./protractor.conf.js" } }, + "lint": [ + { + "files": "<%= sourceDir %>/**/*.ts", + "project": "<%= sourceDir %>/tsconfig.json" + }, + { + "files": "e2e/**/*.ts", + "project": "e2e/tsconfig.json" + } + ], "test": { "karma": { "config": "./karma.conf.js" diff --git a/packages/angular-cli/blueprints/ng2/files/package.json b/packages/angular-cli/blueprints/ng2/files/package.json index 99cd9bcec0f7..aae643b81390 100644 --- a/packages/angular-cli/blueprints/ng2/files/package.json +++ b/packages/angular-cli/blueprints/ng2/files/package.json @@ -6,7 +6,6 @@ "scripts": { "ng": "ng", "start": "ng serve", - "lint": "tslint \"<%= sourceDir %>/**/*.ts\" --project src/tsconfig.json --type-check && tslint \"e2e/**/*.ts\" --project e2e/tsconfig.json --type-check", "test": "ng test", "pree2e": "webdriver-manager update --standalone false --gecko false", "e2e": "protractor" diff --git a/packages/angular-cli/commands/lint.ts b/packages/angular-cli/commands/lint.ts index cfda6c8fd0ad..e8bc20a05fa3 100644 --- a/packages/angular-cli/commands/lint.ts +++ b/packages/angular-cli/commands/lint.ts @@ -1,16 +1,29 @@ const Command = require('../ember-cli/lib/models/command'); +export interface LintCommandOptions { + fix?: boolean; + format?: string; + force?: boolean; +} + export default Command.extend({ name: 'lint', + aliases: ['l'], description: 'Lints code in existing project', works: 'insideProject', - run: function () { + availableOptions: [ + { name: 'fix', type: Boolean, default: false }, + { name: 'force', type: Boolean, default: false }, + { name: 'format', alias: 't', type: String, default: 'prose' } + ], + run: function (commandOptions: LintCommandOptions) { const LintTask = require('../tasks/lint').default; + const lintTask = new LintTask({ ui: this.ui, project: this.project }); - return lintTask.run(); + return lintTask.run(commandOptions); } }); diff --git a/packages/angular-cli/lib/config/schema.json b/packages/angular-cli/lib/config/schema.json index 67b0473f88e5..cb8bd5442051 100644 --- a/packages/angular-cli/lib/config/schema.json +++ b/packages/angular-cli/lib/config/schema.json @@ -124,7 +124,9 @@ } }, "additionalProperties": true, - "required": ["input"] + "required": [ + "input" + ] } ] }, @@ -173,6 +175,30 @@ }, "additionalProperties": false }, + "lint": { + "description": "Properties to be passed to TSLint.", + "type": "array", + "items": { + "type": "object", + "properties": { + "files": { + "type": "string" + }, + "project": { + "type": "string" + }, + "tslintConfig": { + "type": "string", + "default": "tslint.json" + } + }, + "required": [ + "files", + "project" + ], + "additionalProperties": false + } + }, "test": { "type": "object", "properties": { diff --git a/packages/angular-cli/tasks/lint.ts b/packages/angular-cli/tasks/lint.ts index 08f9d5a653e2..6e4e3618acfa 100644 --- a/packages/angular-cli/tasks/lint.ts +++ b/packages/angular-cli/tasks/lint.ts @@ -1,22 +1,61 @@ const Task = require('../ember-cli/lib/models/task'); import * as chalk from 'chalk'; -import {exec} from 'child_process'; +import * as path from 'path'; +import { requireDependency } from '../utilities/require-project-module'; +import { CliConfig } from '../models/config'; +import { LintCommandOptions } from '../commands/lint'; +import { oneLine } from 'common-tags'; export default Task.extend({ - run: function () { + run: function (commandOptions: LintCommandOptions) { const ui = this.ui; + const projectRoot = this.project.root; - return new Promise(function(resolve, reject) { - exec('npm run lint', (err, stdout) => { - ui.writeLine(stdout); - if (err) { - ui.writeLine(chalk.red('Lint errors found in the listed files.')); - reject(); - } else { - ui.writeLine(chalk.green('All files pass linting.')); - resolve(); - } + return new Promise(function (resolve, reject) { + const tslint = requireDependency(projectRoot, 'tslint'); + const Linter = tslint.Linter; + const Configuration = tslint.Configuration; + + const lintConfigs = CliConfig.fromProject().config.lint || []; + + if (lintConfigs.length === 0) { + ui.writeLine(chalk.yellow(oneLine` + No lint config(s) found. + If this is not intended, run "ng update". + `)); + return resolve(0); + } + + let errors = 0; + + lintConfigs.forEach((config) => { + const program = Linter.createProgram(config.project); + const files: string[] = Linter.getFileNames(program); + + const linter = new Linter({ + fix: commandOptions.fix, + formatter: commandOptions.format + }, program); + + files.forEach((file) => { + const fileContents = program.getSourceFile(file).getFullText(); + const configLoad = Configuration.findConfiguration(config.tslintConfig, file); + linter.lint(file, fileContents, configLoad.results); + }); + + const result = linter.getResult(); + errors += result.failureCount; + + ui.writeLine(result.output.trim().concat('\n')); }); + + if (errors > 0) { + ui.writeLine(chalk.red('Lint errors found in the listed files.')); + return commandOptions.force ? resolve(0) : resolve(2); + } + + ui.writeLine(chalk.green('All files pass linting.')); + return resolve(0); }); } }); diff --git a/packages/angular-cli/tasks/test.ts b/packages/angular-cli/tasks/test.ts index 648314d88764..b54c85273e0d 100644 --- a/packages/angular-cli/tasks/test.ts +++ b/packages/angular-cli/tasks/test.ts @@ -1,13 +1,7 @@ const Task = require('../ember-cli/lib/models/task'); import { TestOptions } from '../commands/test'; import * as path from 'path'; - -// require dependencies within the target project -function requireDependency(root: string, moduleName: string) { - const packageJson = require(path.join(root, 'node_modules', moduleName, 'package.json')); - const main = path.normalize(packageJson.main); - return require(path.join(root, 'node_modules', moduleName, main)); -} +import { requireDependency } from '../utilities/require-project-module'; export default Task.extend({ run: function (options: TestOptions) { diff --git a/packages/angular-cli/utilities/require-project-module.ts b/packages/angular-cli/utilities/require-project-module.ts new file mode 100644 index 000000000000..464cb8120297 --- /dev/null +++ b/packages/angular-cli/utilities/require-project-module.ts @@ -0,0 +1,8 @@ +import * as path from 'path'; + +// require dependencies within the target project +export function requireDependency(root: string, moduleName: string) { + const packageJson = require(path.join(root, 'node_modules', moduleName, 'package.json')); + const main = path.normalize(packageJson.main); + return require(path.join(root, 'node_modules', moduleName, main)); +} diff --git a/tests/e2e/tests/lint/lint-no-config-section.ts b/tests/e2e/tests/lint/lint-no-config-section.ts new file mode 100644 index 000000000000..f30f80aba08c --- /dev/null +++ b/tests/e2e/tests/lint/lint-no-config-section.ts @@ -0,0 +1,28 @@ +import { ng } from '../../utils/process'; +import { oneLine } from 'common-tags'; + +export default function () { + return Promise.resolve() + .then(() => ng('set', 'lint', '[]')) + .then(() => ng('lint')) + .then((output) => { + if (!output.match(/No lint config\(s\) found\./)) { + throw new Error(oneLine` + Expected to match "No lint configs found." + in ${output}. + `); + } + + return output; + }) + .then((output) => { + if (!output.match(/If this is not intended, run "ng update"\./)) { + throw new Error(oneLine` + Expected to match "If this is not intended, run "ng update"." + in ${output}. + `); + } + + return output; + }); +} diff --git a/tests/e2e/tests/lint/lint-with-fix.ts b/tests/e2e/tests/lint/lint-with-fix.ts new file mode 100644 index 000000000000..0dfb3fffe72d --- /dev/null +++ b/tests/e2e/tests/lint/lint-with-fix.ts @@ -0,0 +1,16 @@ +import { ng } from '../../utils/process'; +import { readFile, writeFile } from '../../utils/fs'; + +export default function () { + const fileName = 'src/app/foo.ts'; + + return Promise.resolve() + .then(() => writeFile(fileName, 'const foo = "";\n')) + .then(() => ng('lint', '--fix', '--force')) + .then(() => readFile(fileName)) + .then(content => { + if (!content.match(/const foo = '';/)) { + throw new Error(`Expected to match "const foo = '';" in ${content}.`); + } + }); +} diff --git a/tests/e2e/tests/lint/lint-with-force.ts b/tests/e2e/tests/lint/lint-with-force.ts new file mode 100644 index 000000000000..f5b82f041c9d --- /dev/null +++ b/tests/e2e/tests/lint/lint-with-force.ts @@ -0,0 +1,26 @@ +import { ng } from '../../utils/process'; +import { writeFile } from '../../utils/fs'; +import { oneLine } from 'common-tags'; + +export default function () { + const fileName = 'src/app/foo.ts'; + + return Promise.resolve() + .then(() => writeFile(fileName, 'const foo = "";\n')) + .then(() => ng('lint', '--force')) + .then((output) => { + if (!output.match(/" should be '/)) { + throw new Error(`Expected to match "" should be '" in ${output}.`); + } + + return output; + }) + .then((output) => { + if (!output.match(/Lint errors found in the listed files\./)) { + throw new Error(oneLine` + Expected to match "Lint errors found in the listed files." + in ${output}. + `); + } + }); +} diff --git a/tests/e2e/tests/lint/lint-with-format.ts b/tests/e2e/tests/lint/lint-with-format.ts new file mode 100644 index 000000000000..01e602e61b08 --- /dev/null +++ b/tests/e2e/tests/lint/lint-with-format.ts @@ -0,0 +1,19 @@ +import { ng } from '../../utils/process'; +import { writeFile } from '../../utils/fs'; +import { oneLine } from 'common-tags'; + +export default function () { + const fileName = 'src/app/foo.ts'; + + return Promise.resolve() + .then(() => writeFile(fileName, 'const foo = "";\n')) + .then(() => ng('lint', '--format=stylish', '--force')) + .then((output) => { + if (!output.match(/1:13 quotemark " should be '/)) { + throw new Error(oneLine` + Expected to match "1:13 quotemark " should be '" + in ${output}. + `); + } + }); +} diff --git a/tests/e2e/tests/lint/lint.ts b/tests/e2e/tests/lint/lint.ts new file mode 100644 index 000000000000..8de6eb9f07c0 --- /dev/null +++ b/tests/e2e/tests/lint/lint.ts @@ -0,0 +1,14 @@ +import { ng } from '../../utils/process'; +import { oneLine } from 'common-tags'; + +export default function () { + return ng('lint') + .then((output) => { + if (!output.match(/All files pass linting\./)) { + throw new Error(oneLine` + Expected to match "All files pass linting." + in ${output}. + `); + } + }); +} diff --git a/tests/e2e/tests/test/lint.ts b/tests/e2e/tests/test/lint.ts deleted file mode 100644 index 55242cc11cd2..000000000000 --- a/tests/e2e/tests/test/lint.ts +++ /dev/null @@ -1,6 +0,0 @@ -import {ng} from '../../utils/process'; - - -export default function() { - return ng('lint'); -}