From 11a8762e26a667efb697837446503296d80084ab Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 28 Nov 2018 20:17:21 +1000 Subject: [PATCH 1/4] Allowed the "disable rule" fix to be shown, even when there are no TSLint fixes available. --- src/plugin.ts | 51 +++++++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/src/plugin.ts b/src/plugin.ts index 981c472..99d1e15 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -9,15 +9,20 @@ import { getNonOverlappingReplacements, filterProblemsForFile } from './runner/f const isTsLintLanguageServiceMarker = Symbol('__isTsLintLanguageServiceMarker__'); -class FailureMap { - private readonly _map = new Map(); +interface Problem { + failure: tslint.RuleFailure; + fixable: boolean; +} + +class ProblemMap { + private readonly _map = new Map(); public get(start: number, end: number) { return this._map.get(this.key(start, end)); } - public set(start: number, end: number, failure: tslint.RuleFailure): void { - this._map.set(this.key(start, end), failure); + public set(start: number, end: number, problem: Problem): void { + this._map.set(this.key(start, end), problem); } public values() { @@ -31,7 +36,7 @@ class FailureMap { } export class TSLintPlugin { - private readonly codeFixActions = new Map(); + private readonly codeFixActions = new Map(); private readonly configFileWatcher: ConfigFileWatcher; private readonly runner: TsLintRunner; @@ -182,12 +187,12 @@ export class TSLintPlugin { const documentFixes = this.codeFixActions.get(fileName); if (documentFixes) { const problem = documentFixes.get(start, end); - if (problem) { - const fix = problem.getFix(); + if (problem && problem.fixable) { + const fix = problem.failure.getFix(); if (fix) { - fixes.push(this.getRuleFailureQuickFix(problem, fileName)); + fixes.push(this.getRuleFailureQuickFix(problem.failure, fileName)); - const fixAll = this.getRuleFailureFixAllQuickFix(problem.getRuleName(), documentFixes, fileName); + const fixAll = this.getRuleFailureFixAllQuickFix(problem.failure.getRuleName(), documentFixes, fileName); if (fixAll) { fixes.push(fixAll); } @@ -197,7 +202,7 @@ export class TSLintPlugin { fixes.push(this.getFixAllAutoFixableQuickFix(documentFixes, fileName)); if (problem) { - fixes.push(this.getDisableRuleQuickFix(problem, fileName, this.getProgram().getSourceFile(fileName)!)); + fixes.push(this.getDisableRuleQuickFix(problem.failure, fileName, this.getProgram().getSourceFile(fileName)!)); } } @@ -205,23 +210,15 @@ export class TSLintPlugin { } private recordCodeAction(problem: tslint.RuleFailure, file: ts.SourceFile) { - let fix: tslint.Fix | undefined; - // tslint can return a fix with an empty replacements array, these fixes are ignored - if (problem.getFix && problem.getFix() && !replacementsAreEmpty(problem.getFix())) { // tslint fixes are not available in tslint < 3.17 - fix = problem.getFix(); // createAutoFix(problem, document, problem.getFix()); - } - - if (!fix) { - return; - } + const fixable = !!(problem.getFix && problem.getFix() && !replacementsAreEmpty(problem.getFix())); let documentAutoFixes = this.codeFixActions.get(file.fileName); if (!documentAutoFixes) { - documentAutoFixes = new FailureMap(); + documentAutoFixes = new ProblemMap(); this.codeFixActions.set(file.fileName, documentAutoFixes); } - documentAutoFixes.set(problem.getStartPosition().getPosition(), problem.getEndPosition().getPosition(), problem); + documentAutoFixes.set(problem.getStartPosition().getPosition(), problem.getEndPosition().getPosition(), { failure: problem, fixable }); } private getRuleFailureQuickFix(problem: tslint.RuleFailure, fileName: string): ts_module.CodeFixAction { @@ -235,12 +232,14 @@ export class TSLintPlugin { /** * Generate a code action that fixes all instances of ruleName. */ - private getRuleFailureFixAllQuickFix(ruleName: string, problems: FailureMap, fileName: string): ts_module.CodeFixAction | undefined { + private getRuleFailureFixAllQuickFix(ruleName: string, problems: ProblemMap, fileName: string): ts_module.CodeFixAction | undefined { const changes: ts_module.FileTextChanges[] = []; for (const problem of problems.values()) { - if (problem.getRuleName() === ruleName) { - changes.push(problemToFileTextChange(problem, fileName)); + if (problem.fixable) { + if (problem.failure.getRuleName() === ruleName) { + changes.push(problemToFileTextChange(problem.failure, fileName)); + } } } @@ -270,8 +269,8 @@ export class TSLintPlugin { }; } - private getFixAllAutoFixableQuickFix(documentFixes: FailureMap, fileName: string): ts_module.CodeFixAction { - const allReplacements = getNonOverlappingReplacements(Array.from(documentFixes.values())); + private getFixAllAutoFixableQuickFix(documentFixes: ProblemMap, fileName: string): ts_module.CodeFixAction { + const allReplacements = getNonOverlappingReplacements(Array.from(documentFixes.values()).filter(x => x.fixable).map(x => x.failure)); return { description: `Fix all auto-fixable tslint failures`, fixName: '', From ced270ceb75ad8dd0356341173a30538c3762fda Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 28 Nov 2018 20:20:34 +1000 Subject: [PATCH 2/4] Renamed variables called "problem" to "failure" to be consistent and avoid confusion. --- src/plugin.ts | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/plugin.ts b/src/plugin.ts index 99d1e15..268d391 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -209,23 +209,23 @@ export class TSLintPlugin { return fixes; } - private recordCodeAction(problem: tslint.RuleFailure, file: ts.SourceFile) { + private recordCodeAction(failure: tslint.RuleFailure, file: ts.SourceFile) { // tslint can return a fix with an empty replacements array, these fixes are ignored - const fixable = !!(problem.getFix && problem.getFix() && !replacementsAreEmpty(problem.getFix())); + const fixable = !!(failure.getFix && failure.getFix() && !replacementsAreEmpty(failure.getFix())); let documentAutoFixes = this.codeFixActions.get(file.fileName); if (!documentAutoFixes) { documentAutoFixes = new ProblemMap(); this.codeFixActions.set(file.fileName, documentAutoFixes); } - documentAutoFixes.set(problem.getStartPosition().getPosition(), problem.getEndPosition().getPosition(), { failure: problem, fixable }); + documentAutoFixes.set(failure.getStartPosition().getPosition(), failure.getEndPosition().getPosition(), { failure, fixable }); } - private getRuleFailureQuickFix(problem: tslint.RuleFailure, fileName: string): ts_module.CodeFixAction { + private getRuleFailureQuickFix(failure: tslint.RuleFailure, fileName: string): ts_module.CodeFixAction { return { - description: `Fix: ${problem.getFailure()}`, + description: `Fix: ${failure.getFailure()}`, fixName: '', - changes: [problemToFileTextChange(problem, fileName)], + changes: [failureToFileTextChange(failure, fileName)], }; } @@ -238,7 +238,7 @@ export class TSLintPlugin { for (const problem of problems.values()) { if (problem.fixable) { if (problem.failure.getRuleName() === ruleName) { - changes.push(problemToFileTextChange(problem.failure, fileName)); + changes.push(failureToFileTextChange(problem.failure, fileName)); } } } @@ -255,15 +255,15 @@ export class TSLintPlugin { }; } - private getDisableRuleQuickFix(problem: tslint.RuleFailure, fileName: string, file: ts_module.SourceFile): ts_module.CodeFixAction { + private getDisableRuleQuickFix(failure: tslint.RuleFailure, fileName: string, file: ts_module.SourceFile): ts_module.CodeFixAction { return { - description: `Disable rule '${problem.getRuleName()}'`, + description: `Disable rule '${failure.getRuleName()}'`, fixName: '', changes: [{ fileName, textChanges: [{ - newText: `// tslint:disable-next-line: ${problem.getRuleName()}\n`, - span: { start: file.getLineStarts()[problem.getStartPosition().getLineAndCharacter().line], length: 0 }, + newText: `// tslint:disable-next-line: ${failure.getRuleName()}\n`, + span: { start: file.getLineStarts()[failure.getStartPosition().getLineAndCharacter().line], length: 0 }, }], }], }; @@ -285,17 +285,17 @@ export class TSLintPlugin { return this.project.getLanguageService().getProgram()!; } - private makeDiagnostic(problem: tslint.RuleFailure, file: ts.SourceFile): ts.Diagnostic { - const message = (problem.getRuleName() !== null) - ? `${problem.getFailure()} (${problem.getRuleName()})` - : `${problem.getFailure()}`; + private makeDiagnostic(failure: tslint.RuleFailure, file: ts.SourceFile): ts.Diagnostic { + const message = (failure.getRuleName() !== null) + ? `${failure.getFailure()} (${failure.getRuleName()})` + : `${failure.getFailure()}`; - const category = this.getDiagnosticCategory(problem); + const category = this.getDiagnosticCategory(failure); return { file, - start: problem.getStartPosition().getPosition(), - length: problem.getEndPosition().getPosition() - problem.getStartPosition().getPosition(), + start: failure.getStartPosition().getPosition(), + length: failure.getEndPosition().getPosition() - failure.getStartPosition().getPosition(), messageText: message, category, source: TSLINT_ERROR_SOURCE, @@ -303,10 +303,10 @@ export class TSLintPlugin { }; } - private getDiagnosticCategory(problem: tslint.RuleFailure): ts.DiagnosticCategory { + private getDiagnosticCategory(failure: tslint.RuleFailure): ts.DiagnosticCategory { if (this.configurationManager.config.alwaysShowRuleFailuresAsWarnings === true) { return this.ts.DiagnosticCategory.Warning; - } else if (problem.getRuleSeverity && problem.getRuleSeverity() === 'error') { + } else if (failure.getRuleSeverity && failure.getRuleSeverity() === 'error') { // tslint5 supports to assign severities to rules return this.ts.DiagnosticCategory.Error; } @@ -338,8 +338,8 @@ function convertReplacementToTextChange(repl: tslint.Replacement): ts_module.Tex }; } -function problemToFileTextChange(problem: tslint.RuleFailure, fileName: string): ts_module.FileTextChanges { - const fix = problem.getFix(); +function failureToFileTextChange(failure: tslint.RuleFailure, fileName: string): ts_module.FileTextChanges { + const fix = failure.getFix(); const replacements: tslint.Replacement[] = getReplacements(fix); return { From c63b757b153fa9975201711a4896812f4dd73bf4 Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 28 Nov 2018 20:21:20 +1000 Subject: [PATCH 3/4] Added a test file that has a problem without a quick fix to test the "disable rule" quick fix. --- test-workspace/examples/eval.ts | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 test-workspace/examples/eval.ts diff --git a/test-workspace/examples/eval.ts b/test-workspace/examples/eval.ts new file mode 100644 index 0000000..72de39e --- /dev/null +++ b/test-workspace/examples/eval.ts @@ -0,0 +1,3 @@ +// There is no TSLint fix for the "no-eval" rule, +// but the "disable rule" fix is still available. +let x = eval("1 + 1"); From 1be5cd1bb611e68e0e41f21c86133f6aea8f79d8 Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 28 Nov 2018 20:21:34 +1000 Subject: [PATCH 4/4] Ensured that the TSLint VS Code plugin is turned off in the test workspace. --- test-workspace/.vscode/settings.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test-workspace/.vscode/settings.json b/test-workspace/.vscode/settings.json index 55712c1..031840f 100644 --- a/test-workspace/.vscode/settings.json +++ b/test-workspace/.vscode/settings.json @@ -1,3 +1,4 @@ { - "typescript.tsdk": "node_modules/typescript/lib" + "typescript.tsdk": "node_modules/typescript/lib", + "tslint.enable": false } \ No newline at end of file