indent: Simplify and avoid scanning (palantir#2826)

[bugfix] `indent` now checks indentation of expressions inside template strings
no23reason · May 31, 2017 · 6a46287 · 6a46287
1 parent 5251926
commit 6a46287
Show file tree

Hide file tree

Showing 12 changed files with 61 additions and 91 deletions.
diff --git a/package.json b/package.json
@@ -44,7 +44,7 @@
     "resolve": "^1.3.2",
     "semver": "^5.3.0",
     "tslib": "^1.7.1",
-    "tsutils": "^2.1.0"
+    "tsutils": "^2.3.0"
   },
   "peerDependencies": {
     "typescript": ">=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev"

diff --git a/src/rules/indentRule.ts b/src/rules/indentRule.ts
@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { getLineRanges, getTokenAtPosition, isPositionInComment } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
@@ -96,85 +97,37 @@ interface Options {
     readonly size?: 2 | 4;
 }
 
-// visit every token and enforce that only the right character is used for indentation
 function walk(ctx: Lint.WalkContext<Options>): void {
     const { sourceFile, options: { tabs, size } } = ctx;
     const regExp = tabs ? new RegExp(" ".repeat(size === undefined ? 1 : size)) : /\t/;
-
-    let endOfComment = -1;
-    let endOfTemplateString = -1;
-    const scanner = ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, sourceFile.text);
-    for (const lineStart of sourceFile.getLineStarts()) {
-        if (lineStart < endOfComment || lineStart < endOfTemplateString) {
-            // skip checking lines inside multi-line comments or template strings
-            continue;
-        }
-
-        scanner.setTextPos(lineStart);
-
-        let currentScannedType = scanner.scan();
-        let fullLeadingWhitespace = "";
-        let lastStartPos = -1;
-
-        while (currentScannedType === ts.SyntaxKind.WhitespaceTrivia) {
-            const startPos = scanner.getStartPos();
-            if (startPos === lastStartPos) {
-                break;
-            }
-            lastStartPos = startPos;
-
-            fullLeadingWhitespace += scanner.getTokenText();
-            currentScannedType = scanner.scan();
+    const failure = Rule.FAILURE_STRING(tabs ? "tab" : size === undefined ? "space" : `${size} space`);
+
+    for (const {pos, contentLength} of getLineRanges(sourceFile)) {
+        if (contentLength === 0) { continue; }
+        const line = sourceFile.text.substr(pos, contentLength);
+        let indentEnd = line.search(/\S/);
+        if (indentEnd === 0) { continue; }
+        if (indentEnd === -1) {
+            indentEnd = contentLength;
         }
-
-        const commentRanges = ts.getTrailingCommentRanges(sourceFile.text, lineStart);
-        if (commentRanges !== undefined) {
-            endOfComment = commentRanges[commentRanges.length - 1].end;
-        } else {
-            let scanType = currentScannedType;
-
-            // scan until we reach end of line, skipping over template strings
-            while (scanType !== ts.SyntaxKind.NewLineTrivia && scanType !== ts.SyntaxKind.EndOfFileToken) {
-                if (scanType === ts.SyntaxKind.NoSubstitutionTemplateLiteral) {
-                    // template string without expressions - skip past it
-                    endOfTemplateString = scanner.getStartPos() + scanner.getTokenText().length;
-                } else if (scanType === ts.SyntaxKind.TemplateHead) {
-                    // find end of template string containing expressions...
-                    while (scanType !== ts.SyntaxKind.TemplateTail && scanType !== ts.SyntaxKind.EndOfFileToken) {
-                        scanType = scanner.scan();
-                        if (scanType === ts.SyntaxKind.CloseBraceToken) {
-                            scanType = scanner.reScanTemplateToken();
-                        }
-                    }
-                    // ... and skip past it
-                    endOfTemplateString = scanner.getStartPos() + scanner.getTokenText().length;
-                }
-                scanType = scanner.scan();
-            }
-        }
-
-        switch (currentScannedType) {
-            case ts.SyntaxKind.SingleLineCommentTrivia:
-            case ts.SyntaxKind.MultiLineCommentTrivia:
-            case ts.SyntaxKind.NewLineTrivia:
-                // ignore lines that have comments before the first token
-                continue;
-        }
-
-        if (regExp.test(fullLeadingWhitespace)) {
-            const failure = Rule.FAILURE_STRING(tabs ? "tab" : size === undefined ? "space" : `${size} space`);
-            ctx.addFailureAt(lineStart, fullLeadingWhitespace.length, failure, createFix(lineStart, fullLeadingWhitespace));
+        const whitespace = line.slice(0, indentEnd);
+        if (!regExp.test(whitespace)) { continue; }
+        const token = getTokenAtPosition(sourceFile, pos)!;
+        if (token.kind !== ts.SyntaxKind.JsxText &&
+            (pos >= token.getStart(sourceFile) || isPositionInComment(sourceFile, pos, token))) {
+            continue;
         }
+        ctx.addFailureAt(pos, indentEnd, failure, createFix(pos, whitespace, tabs, size));
     }
+}
 
-    function createFix(lineStart: number, fullLeadingWhitespace: string): Lint.Fix | undefined {
-        if (size === undefined) { return undefined; }
-        const replaceRegExp = tabs
-            // we want to find every group of `size` spaces, plus up to one 'incomplete' group
-            ? new RegExp(`^( {${size}})+( {1,${size - 1}})?`, "g")
-            : /\t/g;
-        const replacement = fullLeadingWhitespace.replace(replaceRegExp, (match) =>
-            (tabs ? "\t" : " ".repeat(size)).repeat(Math.ceil(match.length / size)));
-        return new Lint.Replacement(lineStart, fullLeadingWhitespace.length, replacement);
-    }
+function createFix(lineStart: number, fullLeadingWhitespace: string, tabs: boolean, size?: number): Lint.Fix | undefined {
+    if (size === undefined) { return undefined; }
+    const replaceRegExp = tabs
+        // we want to find every group of `size` spaces, plus up to one 'incomplete' group
+        ? new RegExp(`^( {${size}})+( {1,${size - 1}})?`, "g")
+        : /\t/g;
+    const replacement = fullLeadingWhitespace.replace(replaceRegExp, (match) =>
+        (tabs ? "\t" : " ".repeat(size)).repeat(Math.ceil(match.length / size)));
+    return new Lint.Replacement(lineStart, fullLeadingWhitespace.length, replacement);
 }
diff --git a/test/rules/indent/spaces-2/test.ts.fix b/test/rules/indent/spaces-2/test.ts.fix
@@ -25,7 +25,7 @@ module TestModule {
   var s2 = `
 		multiline ${ "A" }
 		template ${ "B"
-		+ "C" }
+    + "C" }
 		string`;
 
   export enum TestEnum {

diff --git a/test/rules/indent/spaces-2/test.ts.lint b/test/rules/indent/spaces-2/test.ts.lint
@@ -26,6 +26,7 @@ module TestModule {
 		multiline ${ "A" }
 		template ${ "B"
 		+ "C" }
+~~ [0]
 		string`;
 
   export enum TestEnum {

diff --git a/test/rules/indent/spaces-2/test.tsx.lint b/test/rules/indent/spaces-2/test.tsx.lint
@@ -0,0 +1,17 @@
+<foo>
+	<bar>
+~ [0]
+		baz
+~~ [0]
+	</bar>
+~ [0]
+
+	{
+~ [0]
+
+~~ [0]
+	}
+~ [0]
+</foo>
+
+[0]: 2 space indentation expected
diff --git a/test/rules/indent/spaces-4/test.ts.fix b/test/rules/indent/spaces-4/test.ts.fix
@@ -25,7 +25,7 @@ module TestModule {
     var s2 = `
 		multiline ${ "A" }
 		template ${ "B"
-		+ "C" }
+        + "C" }
 		string`;
 
     export enum TestEnum {

diff --git a/test/rules/indent/spaces-4/test.ts.lint b/test/rules/indent/spaces-4/test.ts.lint
@@ -26,6 +26,7 @@ module TestModule {
 		multiline ${ "A" }
 		template ${ "B"
 		+ "C" }
+~~ [0]
 		string`;
 
     export enum TestEnum {

diff --git a/test/rules/indent/tabs-2/test.ts.fix b/test/rules/indent/tabs-2/test.ts.fix
@@ -28,7 +28,7 @@ module TestModule {
 	var s2 = `
      multiline ${ "A" }
      template ${ "B"
-     + "C" }
+			+ "C" }
      string`;
 
 	export enum TestEnum {

diff --git a/test/rules/indent/tabs-2/test.ts.lint b/test/rules/indent/tabs-2/test.ts.lint
@@ -29,6 +29,7 @@ module TestModule {
      multiline ${ "A" }
      template ${ "B"
      + "C" }
+~~~~~ [0]
      string`;
 
 	export enum TestEnum {

diff --git a/test/rules/indent/tabs-4/test.ts.fix b/test/rules/indent/tabs-4/test.ts.fix
@@ -28,7 +28,7 @@ module TestModule {
 	var s2 = `
      multiline ${ "A" }
      template ${ "B"
-     + "C" }
+		+ "C" }
      string`;
 
 	export enum TestEnum {

diff --git a/test/rules/indent/tabs-4/test.ts.lint b/test/rules/indent/tabs-4/test.ts.lint
@@ -29,6 +29,7 @@ module TestModule {
      multiline ${ "A" }
      template ${ "B"
      + "C" }
+~~~~~ [0]
      string`;
 
 	export enum TestEnum {

diff --git a/yarn.lock b/yarn.lock
@@ -58,8 +58,8 @@
   resolved "https://registry.yarnpkg.com/@types/semver/-/semver-5.3.31.tgz#b999d7d935f43f5207b01b00d3de20852f4ca75f"
 
 agent-base@2:
-  version "2.1.0"
-  resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-2.1.0.tgz#193455e4347bca6b05847cb81e939bb325446da8"
+  version "2.1.1"
+  resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-2.1.1.tgz#d6de10d5af6132d5bd692427d46fc538539094c7"
   dependencies:
     extend "~3.0.0"
     semver "~5.0.1"
@@ -981,14 +981,10 @@ minimatch@^3.0.2, minimatch@^3.0.4:
   dependencies:
     brace-expansion "^1.1.7"
 
-[email protected]:
+[email protected], minimist@~0.0.1:
   version "0.0.8"
   resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.8.tgz#857fcabfc3397d2625b8228262e86aa7a011b05d"
 
-minimist@~0.0.1:
-  version "0.0.10"
-  resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.10.tgz#de3f98543dbf96082be48ad1a0c7cda836301dcf"
-
 [email protected], mkdirp@^0.5.0, mkdirp@^0.5.1:
   version "0.5.1"
   resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-0.5.1.tgz#30057438eac6cf7f8c4767f38648d6697d75c903"
@@ -1479,9 +1475,9 @@ tslint@latest:
     tslib "^1.6.0"
     tsutils "^2.0.0"
 
-tsutils@^2.0.0, tsutils@^2.1.0:
-  version "2.2.0"
-  resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-2.2.0.tgz#218614657f21c677e4536b4ba75daf8ebce1b367"
+tsutils@2, tsutils@^2.0.0:
+  version "2.3.0"
+  resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-2.3.0.tgz#96e661d7c2363f31adc8992ac67bbe7b7fc175e5"
 
 [email protected]:
   version "0.1.1"
@@ -1492,8 +1488,8 @@ type-detect@^1.0.0:
   resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-1.0.0.tgz#762217cc06db258ec48908a1298e8b95121e8ea2"
 
 typescript@^2.3.0:
-  version "2.3.3"
-  resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.3.3.tgz#9639f3c3b40148e8ca97fe08a51dd1891bb6be22"
+  version "2.3.4"
+  resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.3.4.tgz#3d38321828231e434f287514959c37a82b629f42"
 
 uglify-js@^2.6:
   version "2.8.27"