diff --git a/src/rules/groupedImportsRule.ts b/src/rules/groupedImportsRule.ts index c0d11c516fd..46298166a91 100644 --- a/src/rules/groupedImportsRule.ts +++ b/src/rules/groupedImportsRule.ts @@ -16,7 +16,7 @@ */ import * as Lint from "tslint"; -import { isImportDeclaration } from "tsutils"; +import { isImportDeclaration, isTextualLiteral } from "tsutils"; import * as ts from "typescript"; export class Rule extends Lint.Rules.AbstractRule { @@ -47,8 +47,8 @@ export class Rule extends Lint.Rules.AbstractRule { enum ImportStatementType { LIBRARY_IMPORT = 1, - PARENT_DIRECTORY_IMPORT = 2, - CURRENT_DIRECTORY_IMPORT = 3, + PARENT_DIRECTORY_IMPORT = 2, // starts with "../" + CURRENT_DIRECTORY_IMPORT = 3, // starts with "./" } interface ImportStatement { @@ -60,82 +60,112 @@ interface ImportStatement { class Walker extends Lint.AbstractWalker { private lastImportStatement: ImportStatement; + private newLine: string; + private allImportsFix: boolean; - private static getImportStatementType(statement: ts.Statement): ImportStatementType { - const path = Walker.getImportPath(statement); - if (path.charAt(0) === ".") { - if (path.charAt(1) === ".") { - return ImportStatementType.PARENT_DIRECTORY_IMPORT; - } else { - return ImportStatementType.CURRENT_DIRECTORY_IMPORT; + public walk(sourceFile: ts.SourceFile): void { + this.newLine = this.getEofChar(sourceFile); + sourceFile.statements + .filter(isImportDeclaration) + .forEach(this.checkStatement); + } + + private getEofChar(sourceFile: ts.SourceFile): string { + const lineEnd = sourceFile.getLineEndOfPosition(0); + let newLine; + if (lineEnd > 0) { + if (lineEnd > 1 && sourceFile.text[lineEnd - 1] === "\r") { + newLine = "\r\n"; + } else if (sourceFile.text[lineEnd] === "\n") { + newLine = "\n"; } - } else { - return ImportStatementType.LIBRARY_IMPORT; } + return newLine || ts.sys.newLine; } - private static getImportPath(statement: ts.Statement): string { - const str = statement.getText(); - let index; - let lastIndex; - index = str.indexOf("'"); - if (index > 0) { - lastIndex = str.lastIndexOf("'"); - } else { - index = str.indexOf("\""); - lastIndex = str.lastIndexOf("\""); + private checkStatement = (statement: ts.ImportDeclaration): void => { + if (this.allImportsFix) { + return; } - if (index < 0 || lastIndex < 0) { - throw new Error(`Unable to extract path from import statement \`${statement.getText()}\``); + const importStatement = this.toImportStatement(statement); + if (this.lastImportStatement) { + this.checkForFailure(importStatement); } - return str.substring(index + 1, lastIndex); - } - - public walk(sourceFile: ts.SourceFile): void { - sourceFile.statements - .filter(isImportDeclaration) - .forEach((st) => this.checkStatement(st)); + this.lastImportStatement = importStatement; } - private toImportStatement(statement: ts.Statement): ImportStatement { + private toImportStatement(statement: ts.ImportDeclaration): ImportStatement { return { lineEnd: this.sourceFile.getLineAndCharacterOfPosition(statement.getEnd()).line, lineStart: this.sourceFile.getLineAndCharacterOfPosition(statement.getStart()).line, statement, - type: Walker.getImportStatementType(statement), + type: this.getImportStatementType(statement), }; } - private checkStatement(statement: ts.Statement): void { - const importStatement = this.toImportStatement(statement); - if (this.lastImportStatement) { - this.checkImportStatement(importStatement); + private getImportStatementType(statement: ts.ImportDeclaration): ImportStatementType { + const path = this.getImportPath(statement); + if (path.charAt(0) === ".") { + if (path.charAt(1) === ".") { + return ImportStatementType.PARENT_DIRECTORY_IMPORT; + } else { + return ImportStatementType.CURRENT_DIRECTORY_IMPORT; + } + } else { + return ImportStatementType.LIBRARY_IMPORT; } - this.lastImportStatement = importStatement; } - private checkImportStatement(importStatement: ImportStatement) { + private getImportPath(statement: ts.ImportDeclaration): string { + if (isTextualLiteral(statement.moduleSpecifier)) { + return statement.moduleSpecifier.text; + } + return ""; + } + + private checkForFailure(importStatement: ImportStatement): void { if (importStatement.type === this.lastImportStatement.type) { if (importStatement.lineStart !== this.lastImportStatement.lineEnd + 1) { - const replacement = Lint.Replacement.deleteFromTo( - this.lastImportStatement.statement.getEnd() + 1, importStatement.statement.getStart()); - this.addFailureAtNode(importStatement.statement, Rule.IMPORT_SOURCES_SEPARATED, replacement); + this.addSeparatedImportsFailure(importStatement); } - } else if (importStatement.type.valueOf() < this.lastImportStatement.type.valueOf()) { - this.addFailureAtNode(importStatement.statement, Rule.IMPORT_SOURCES_ORDER, this.getAllImportsFix()); } else { - if (importStatement.lineStart !== this.lastImportStatement.lineEnd + 2) { - const replacement = Lint.Replacement.appendText(importStatement.statement.getStart(), ts.sys.newLine); - this.addFailureAtNode(importStatement.statement, Rule.IMPORT_SOURCES_NOT_SEPARATED, replacement); + if (importStatement.type < this.lastImportStatement.type) { + this.addIncorrectlyOrderedImportsFailure(importStatement); + } else if (importStatement.lineStart !== this.lastImportStatement.lineEnd + 2) { + this.addNotSeparatedImportsFailure(importStatement); } } } + private addSeparatedImportsFailure(importStatement: ImportStatement): void { + const text = [this.lastImportStatement, importStatement] + .map((st) => st.statement.getText()) + .join(this.newLine); + const replacement = Lint.Replacement.replaceFromTo( + this.lastImportStatement.statement.getStart(), importStatement.statement.getEnd(), text); + this.addFailureAtNode(importStatement.statement, Rule.IMPORT_SOURCES_SEPARATED, replacement); + } + + private addIncorrectlyOrderedImportsFailure(importStatement: ImportStatement): void { + this.allImportsFix = true; + this.failures.length = 0; + this.addFailureAtNode(importStatement.statement, Rule.IMPORT_SOURCES_ORDER, this.getAllImportsFix()); + } + + private addNotSeparatedImportsFailure(importStatement: ImportStatement): void { + const replacement = Lint.Replacement.replaceFromTo(this.lastImportStatement.statement.getEnd(), + importStatement.statement.getStart(), this.newLine + this.newLine); + this.addFailureAtNode(importStatement.statement, Rule.IMPORT_SOURCES_NOT_SEPARATED, replacement); + } + private getAllImportsFix(): Lint.Fix { const importStatements = this.sourceFile.statements.filter(isImportDeclaration); - const libs = importStatements.filter((st) => Walker.getImportStatementType(st) === ImportStatementType.LIBRARY_IMPORT); - const parent = importStatements.filter((st) => Walker.getImportStatementType(st) === ImportStatementType.PARENT_DIRECTORY_IMPORT); - const current = importStatements.filter((st) => Walker.getImportStatementType(st) === ImportStatementType.CURRENT_DIRECTORY_IMPORT); + const libs = importStatements.filter( + (st) => this.getImportStatementType(st) === ImportStatementType.LIBRARY_IMPORT); + const parent = importStatements.filter( + (st) => this.getImportStatementType(st) === ImportStatementType.PARENT_DIRECTORY_IMPORT); + const current = importStatements.filter( + (st) => this.getImportStatementType(st) === ImportStatementType.CURRENT_DIRECTORY_IMPORT); let imports: string[] = []; [libs, parent, current].forEach((statements) => { if (statements.length) { @@ -146,7 +176,7 @@ class Walker extends Lint.AbstractWalker { return Lint.Replacement.replaceFromTo( importStatements[0].getStart(), importStatements[importStatements.length - 1].getEnd(), - imports.join(ts.sys.newLine), + imports.join(this.newLine), ); } } diff --git a/test/rules/grouped-imports/mixed.ts.fix b/test/rules/grouped-imports/mixed.ts.fix new file mode 100644 index 00000000000..f55e667cb8b --- /dev/null +++ b/test/rules/grouped-imports/mixed.ts.fix @@ -0,0 +1,6 @@ +import {foo} from 'foo'; + +import {bar} from '../bar'; + +import './baz'; + diff --git a/test/rules/grouped-imports/mixed.ts.lint b/test/rules/grouped-imports/mixed.ts.lint new file mode 100644 index 00000000000..159c86bbfec --- /dev/null +++ b/test/rules/grouped-imports/mixed.ts.lint @@ -0,0 +1,5 @@ +import {bar} from '../bar'; + +import {foo} from 'foo'; +~~~~~~~~~~~~~~~~~~~~~~~~ [Import sources of different groups must be sorted by: libraries, parent directories, current directory] +import './baz'; diff --git a/test/rules/grouped-imports/single-line.ts.fix b/test/rules/grouped-imports/single-line.ts.fix new file mode 100644 index 00000000000..d77726dc514 --- /dev/null +++ b/test/rules/grouped-imports/single-line.ts.fix @@ -0,0 +1,4 @@ +import bar from "bar"; +import foo from "foo"; + +import {baz} from "./baz"; diff --git a/test/rules/grouped-imports/single-line.ts.lint b/test/rules/grouped-imports/single-line.ts.lint new file mode 100644 index 00000000000..c510b921691 --- /dev/null +++ b/test/rules/grouped-imports/single-line.ts.lint @@ -0,0 +1,3 @@ +import bar from "bar";import foo from "foo";import {baz} from "./baz"; + ~~~~~~~~~~~~~~~~~~~~~~ [Import sources within a group must not be separated by blank lines] + ~~~~~~~~~~~~~~~~~~~~~~~~~~ [Import sources of different groups must be separated by a single blank line] diff --git a/tslint.json b/tslint.json index 50be550274f..17017f357ae 100644 --- a/tslint.json +++ b/tslint.json @@ -32,6 +32,7 @@ // TODO: Enable these "completed-docs": false, + "grouped-imports": false, "no-any": false, "no-magic-numbers": false, "no-non-null-assertion": false, diff --git a/yarn.lock b/yarn.lock index 3c85b6d7faa..2d0dfc47b4e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1502,9 +1502,9 @@ type-detect@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-1.0.0.tgz#762217cc06db258ec48908a1298e8b95121e8ea2" -typescript@^2.4.1: - version "2.4.1" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.4.1.tgz#c3ccb16ddaa0b2314de031e7e6fee89e5ba346bc" +typescript@~2.4.1: + version "2.4.2" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.4.2.tgz#f8395f85d459276067c988aa41837a8f82870844" uglify-js@^2.6: version "2.8.28"