From 2acc27b2f42b2b450daa455be343a87bbb74e258 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 5 May 2018 05:22:48 -0300 Subject: [PATCH] feat(import-destructuring-spacing): add fixer (#595) --- src/importDestructuringSpacingRule.ts | 81 +++++--- test/importDestructuringSpacingRule.spec.ts | 197 ++++++++++++++++---- 2 files changed, 209 insertions(+), 69 deletions(-) diff --git a/src/importDestructuringSpacingRule.ts b/src/importDestructuringSpacingRule.ts index 94956bb06..6f328de2d 100644 --- a/src/importDestructuringSpacingRule.ts +++ b/src/importDestructuringSpacingRule.ts @@ -1,52 +1,73 @@ -import * as ts from 'typescript'; -import * as Lint from 'tslint'; +import { Fix, IOptions, IRuleMetadata, Replacement, RuleFailure, Rules, RuleWalker } from 'tslint/lib'; +import { NamedImports, SourceFile } from 'typescript/lib/typescript'; -export class Rule extends Lint.Rules.AbstractRule { - public static metadata: Lint.IRuleMetadata = { - ruleName: 'import-destructuring-spacing', - type: 'style', +export class Rule extends Rules.AbstractRule { + static readonly metadata: IRuleMetadata = { description: 'Ensure consistent and tidy imports.', - rationale: "Imports are easier for the reader to look at when they're tidy.", + hasFix: true, options: null, optionsDescription: 'Not configurable.', + rationale: "Imports are easier for the reader to look at when they're tidy.", + ruleName: 'import-destructuring-spacing', + type: 'style', typescriptOnly: true }; - public static FAILURE_STRING = "You need to leave whitespaces inside of the import statement's curly braces"; + static readonly FAILURE_STRING = "Import statement's curly braces must be spaced exactly by a space to the right and a space to the left"; - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + apply(sourceFile: SourceFile): RuleFailure[] { return this.applyWithWalker(new ImportDestructuringSpacingWalker(sourceFile, this.getOptions())); } } -// The walker takes care of all the work. -class ImportDestructuringSpacingWalker extends Lint.RuleWalker { - private scanner: ts.Scanner; +export const getFailureMessage = (): string => { + return Rule.FAILURE_STRING; +}; + +const isBlankOrMultilineImport = (value: string): boolean => { + return value.indexOf('\n') !== -1 || /^\{\s*\}$/.test(value); +}; - constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { +class ImportDestructuringSpacingWalker extends RuleWalker { + constructor(sourceFile: SourceFile, options: IOptions) { super(sourceFile, options); - this.scanner = ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, sourceFile.text); } - public visitImportDeclaration(node: ts.ImportDeclaration) { - const importClause = node.importClause; - if (importClause && importClause.namedBindings) { - const text = importClause.namedBindings.getText(); + protected visitNamedImports(node: NamedImports): void { + this.validateNamedImports(node); + super.visitNamedImports(node); + } + + private validateNamedImports(node: NamedImports): void { + const nodeText = node.getText(); - if (!this.checkForWhiteSpace(text)) { - this.addFailure( - this.createFailure(importClause.namedBindings.getStart(), importClause.namedBindings.getWidth(), Rule.FAILURE_STRING) - ); - } + if (isBlankOrMultilineImport(nodeText)) { + return; + } + + const totalLeadingSpaces = nodeText.match(/^\{(\s*)/)[1].length; + const totalTrailingSpaces = nodeText.match(/(\s*)}$/)[1].length; + + if (totalLeadingSpaces === 1 && totalTrailingSpaces === 1) { + return; } - // call the base version of this visitor to actually parse this node - super.visitImportDeclaration(node); - } - private checkForWhiteSpace(text: string) { - if (/\s*\*\s+as\s+[^\s]/.test(text)) { - return true; + const nodeStartPos = node.getStart(); + const nodeEndPos = node.getEnd(); + let fix: Fix = []; + + if (totalLeadingSpaces === 0) { + fix.push(Replacement.appendText(nodeStartPos + 1, ' ')); + } else if (totalLeadingSpaces > 1) { + fix.push(Replacement.deleteText(nodeStartPos + 1, totalLeadingSpaces - 1)); + } + + if (totalTrailingSpaces === 0) { + fix.push(Replacement.appendText(nodeEndPos - 1, ' ')); + } else if (totalTrailingSpaces > 1) { + fix.push(Replacement.deleteText(nodeEndPos - totalTrailingSpaces, totalTrailingSpaces - 1)); } - return /{\s[^]*\s}/.test(text); + + this.addFailureAtNode(node, getFailureMessage(), fix); } } diff --git a/test/importDestructuringSpacingRule.spec.ts b/test/importDestructuringSpacingRule.spec.ts index 350d1521c..5a77d41f6 100644 --- a/test/importDestructuringSpacingRule.spec.ts +++ b/test/importDestructuringSpacingRule.spec.ts @@ -1,77 +1,196 @@ +import { expect } from 'chai'; +import { Replacement } from 'tslint/lib'; +import { getFailureMessage, Rule } from '../src/importDestructuringSpacingRule'; import { assertAnnotated, assertSuccess } from './testHelper'; -describe('import-destructuring-spacing', () => { - describe('invalid import spacing', () => { - it('should fail when the imports have no spaces', () => { - let source = ` +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail when a single import has no spaces', () => { + const source = ` import {Foo} from './foo' ~~~~~ `; assertAnnotated({ - ruleName: 'import-destructuring-spacing', - message: "You need to leave whitespaces inside of the import statement's curly braces", + message: getFailureMessage(), + ruleName, source }); }); - it('should fail with multiple items to import', () => { - let source = ` + it('should fail when multiple imports have no spaces', () => { + const source = ` import {Foo,Bar} from './foo' ~~~~~~~~~ `; - assertAnnotated({ - ruleName: 'import-destructuring-spacing', - message: "You need to leave whitespaces inside of the import statement's curly braces", - source - }); + assertAnnotated({ message: getFailureMessage(), ruleName, source }); }); - it('should fail with spaces between items', () => { - let source = ` - import {Foo, Bar} from './foo' - ~~~~~~~~~~~ - `; - assertAnnotated({ - ruleName: 'import-destructuring-spacing', - message: "You need to leave whitespaces inside of the import statement's curly braces", - source - }); - }); - - it('should fail with only one whitespace in the left', () => { - let source = ` + it('should fail when there are no trailing spaces', () => { + const source = ` import { Foo} from './foo'; ~~~~~~ `; assertAnnotated({ - ruleName: 'import-destructuring-spacing', - message: "You need to leave whitespaces inside of the import statement's curly braces", + message: getFailureMessage(), + ruleName, source }); }); - it('should fail with only one whitespace in the right', () => { - let source = ` + it('should fail when there are no leading spaces', () => { + const source = ` import {Foo } from './foo'; ~~~~~~ `; assertAnnotated({ - ruleName: 'import-destructuring-spacing', - message: "You need to leave whitespaces inside of the import statement's curly braces", + message: getFailureMessage(), + ruleName, source }); }); }); - describe('valid import spacing', () => { + describe('failure with replacements', () => { + it('should fail and apply proper replacements when there are no spaces', () => { + const source = ` + import {Foo} from './foo'; + ~~~~~ + `; + const failures = assertAnnotated({ message: getFailureMessage(), ruleName, source }); + + if (!Array.isArray(failures)) { + return; + } + + const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix())); + + expect(replacement).to.eq(` + import { Foo } from './foo'; + ~~~~~ + `); + }); + + it('should fail and apply proper replacements when there is more than one leading space', () => { + const source = ` + import { Bar, BarFoo, Foo } from './foo'; + ~~~~~~~~~~~~~~~~~~~~~~~~ + `; + const failures = assertAnnotated({ message: getFailureMessage(), ruleName, source }); + + if (!Array.isArray(failures)) { + return; + } + + const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix())); + + expect(replacement).to.eq(` + import { Bar, BarFoo, Foo } from './foo'; + ~~~~~~~~~~~~~~~~~~~~~~~~ + `); + }); + + it('should fail and apply proper replacements when there is more than one trailing space', () => { + const source = ` + import { Bar, BarFoo, Foo } from './foo'; + ~~~~~~~~~~~~~~~~~~~~~~~~ + `; + const failures = assertAnnotated({ message: getFailureMessage(), ruleName, source }); + + if (!Array.isArray(failures)) { + return; + } + + const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix())); + + expect(replacement).to.eq(` + import { Bar, BarFoo, Foo } from './foo'; + ~~~~~~~~~~~~~~~~~~~~~~~~ + `); + }); + + it('should fail and apply proper replacements when there is more than one space left and right', () => { + const source = ` + import { Bar, BarFoo, Foo } from './foo'; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `; + const failures = assertAnnotated({ message: getFailureMessage(), ruleName, source }); + + if (!Array.isArray(failures)) { + return; + } + + const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix())); + + expect(replacement).to.eq(` + import { Bar, BarFoo, Foo } from './foo'; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `); + }); + }); + + describe('success', () => { it('should succeed with valid spacing', () => { - let source = "import { Foo } from './foo';"; - assertSuccess('import-destructuring-spacing', source); + const source = ` + import { Foo } from './foo'; + `; + assertSuccess(ruleName, source); + }); + + it('should succeed with multiple spaces between imports', () => { + const source = ` + import { Bar, Foo } from './foo'; + `; + assertSuccess(ruleName, source); + }); + + it('should succeed for blank imports', () => { + const source = ` + import {} from 'foo'; + `; + assertSuccess(ruleName, source); + }); + + it('should succeed for module imports', () => { + const source = ` + import foo = require('./foo'); + `; + assertSuccess(ruleName, source); + }); + + it('should succeed for patch imports', () => { + const source = ` + import 'rxjs/add/operator/map'; + `; + assertSuccess(ruleName, source); }); - it('should work with alias imports', () => { - let source = "import * as Foo from './foo';"; - assertSuccess('import-destructuring-spacing', source); + it('should succeed with alias imports', () => { + const source = ` + import * as Foo from './foo'; + `; + assertSuccess(ruleName, source); + }); + + it('should succeed for alias imports inside braces', () => { + const source = ` + import { default as _rollupMoment, Moment } from 'moment'; + `; + assertSuccess(ruleName, source); + }); + + it('should succeed for multiline imports', () => { + const source = ` + import { + Bar, + BarFoo, + Foo + } from './foo'; + `; + assertSuccess(ruleName, source); }); }); });