Fix no-consecutive-blank-lines (palantir#2239)

Correctly apply fixes at EOF.
Fix tests.
Simplify collecting consecutive blank lines.

package.json

@@ -43,7 +43,7 @@
     "glob": "^7.1.1",
     "optimist": "~0.6.0",
     "resolve": "^1.1.7",
-    "tsutils": "^1.0.0",
+    "tsutils": "^1.1.0",
     "update-notifier": "^2.0.0"
   },
   "peerDependencies": {

src/rules/noConsecutiveBlankLinesRule.ts

@@ -15,13 +15,13 @@
  * limitations under the License.
  */
 
+import * as utils from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
 
 export class Rule extends Lint.Rules.AbstractRule {
     public static DEFAULT_ALLOWED_BLANKS = 1;
-    public static MINIMUM_ALLOWED_BLANKS = 1;
 
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
@@ -46,84 +46,81 @@ export class Rule extends Lint.Rules.AbstractRule {
         return allowed === 1
             ? "Consecutive blank lines are forbidden"
             : `Exceeds the ${allowed} allowed consecutive blank lines`;
-    };
+    }
 
     /**
      * Disable the rule if the option is provided but non-numeric or less than the minimum.
      */
     public isEnabled(): boolean {
-        if (!super.isEnabled()) {
-            return false;
-        }
-        const options = this.getOptions();
-        const allowedBlanks = options.ruleArguments && options.ruleArguments[0] || Rule.DEFAULT_ALLOWED_BLANKS;
-        return typeof allowedBlanks === "number" && allowedBlanks >= Rule.MINIMUM_ALLOWED_BLANKS;
+        return super.isEnabled() &&
+            (!this.ruleArguments[0] ||
+             typeof this.ruleArguments[0] === "number" && this.ruleArguments[0] > 0);
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new NoConsecutiveBlankLinesWalker(sourceFile, this.getOptions()));
+        const limit: number = this.ruleArguments[0] || Rule.DEFAULT_ALLOWED_BLANKS;
+        return this.applyWithFunction(sourceFile, walk, limit);
     }
 }
 
-class NoConsecutiveBlankLinesWalker extends Lint.RuleWalker {
-    public walk(node: ts.SourceFile) {
-        const templateIntervals = this.getTemplateIntervals(node);
-        const lineStarts = node.getLineStarts();
+function walk(ctx: Lint.WalkContext<number>) {
+    const sourceText = ctx.sourceFile.text;
+    const threshold = ctx.options + 1;
+    const possibleFailures: ts.TextRange[] = [];
+    let consecutiveBlankLines = 0;
 
-        const options = this.getOptions();
-        const allowedBlanks = options && options[0] || Rule.DEFAULT_ALLOWED_BLANKS;
-        const failureMessage = Rule.FAILURE_STRING_FACTORY(allowedBlanks);
-        const sourceFileText = node.getFullText();
-        const soureFileLines = sourceFileText.split(/\n/);
-
-        // find all the lines that are blank or only contain whitespace
-        const blankLineIndexes: number[] = [];
-        soureFileLines.forEach((txt, i) => {
-            if (txt.trim() === "") {
-                blankLineIndexes.push(i);
+    for (const line of utils.getLineRanges(ctx.sourceFile)) {
+        if (sourceText.substring(line.pos, line.end).search(/\S/) === -1) {
+            ++consecutiveBlankLines;
+            if (consecutiveBlankLines === threshold) {
+                possibleFailures.push({
+                    end: line.end,
+                    pos: line.pos,
+                });
+            } else if (consecutiveBlankLines > threshold) {
+                possibleFailures[possibleFailures.length - 1].end = line.end;
             }
-        });
-
-        // now only throw failures for the fisrt number from groups of consecutive blank line indexes
-        const sequences: number[][] = [];
-        let lastVal = -2;
-        for (const line of blankLineIndexes) {
-            line > lastVal + 1 ? sequences.push([line]) : sequences[sequences.length - 1].push(line);
-            lastVal = line;
+        } else {
+            consecutiveBlankLines = 0;
         }
+    }
 
-        for (const arr of sequences) {
-            if (arr.length <= allowedBlanks) {
-                continue;
-            }
-
-            const startLineNum = arr[0];
-            const endLineNum = arr[arr.length - allowedBlanks];
-            const pos = lineStarts[startLineNum + 1];
-            const end = lineStarts[endLineNum];
-            const isInTemplate = templateIntervals.some((interval) => pos >= interval.startPosition && pos < interval.endPosition);
-
-            if (!isInTemplate) {
-                const fix = this.createFix(this.deleteFromTo(pos, end));
-                this.addFailureAt(pos, 1, failureMessage, fix);
-            }
+    if (possibleFailures.length === 0) {
+        return;
+    }
+    const failureString = Rule.FAILURE_STRING_FACTORY(ctx.options);
+    const templateRanges = getTemplateRanges(ctx.sourceFile);
+    for (const possibleFailure of possibleFailures) {
+        if (!templateRanges.some((template) => template.pos < possibleFailure.pos && possibleFailure.pos < template.end)) {
+            ctx.addFailureAt(possibleFailure.pos, 1, failureString, ctx.createFix(
+                Lint.Replacement.deleteFromTo(
+                    // special handling for fixing blank lines at the end of the file
+                    // to fix this we need to cut off the line break of the last allowed blank line, too
+                    possibleFailure.end === sourceText.length ? getStartOfLineBreak(sourceText, possibleFailure.pos) : possibleFailure.pos,
+                    possibleFailure.end,
+                ),
+            ));
         }
     }
+}
 
-    private getTemplateIntervals(sourceFile: ts.SourceFile): Lint.IDisabledInterval[] {
-        const intervals: Lint.IDisabledInterval[] = [];
-        const cb = (node: ts.Node) => {
-            if (node.kind >= ts.SyntaxKind.FirstTemplateToken &&
-                node.kind <= ts.SyntaxKind.LastTemplateToken) {
-                intervals.push({
-                    endPosition: node.getEnd(),
-                    startPosition: node.getStart(sourceFile),
-                });
-            } else {
-                ts.forEachChild(node, cb);
-            }
-        };
-        ts.forEachChild(sourceFile, cb);
-        return intervals;
-    }
+function getStartOfLineBreak(sourceText: string, pos: number) {
+    return sourceText[pos - 2] === "\r" ? pos - 1 : pos - 1;
+}
+
+function getTemplateRanges(sourceFile: ts.SourceFile): ts.TextRange[] {
+    const intervals: ts.TextRange[] = [];
+    const cb = (node: ts.Node): void => {
+        if (node.kind >= ts.SyntaxKind.FirstTemplateToken &&
+            node.kind <= ts.SyntaxKind.LastTemplateToken) {
+            intervals.push({
+                end: node.end,
+                pos: node.getStart(sourceFile),
+            });
+        } else {
+            return ts.forEachChild(node, cb);
+        }
+    };
+    ts.forEachChild(sourceFile, cb);
+    return intervals;
 }

test/rules/no-consecutive-blank-lines/default/test.ts.fix

@@ -16,12 +16,12 @@ class Clazz { // comment
 
 //Begin whitespace
 // The next two lines of "code" contain only tabs or spaces, they are also considered "blank" lines
-		
+        
 let foo = `
 
 
 `;
 
 let bar = `${bar
-
+ 
 }`;

test/rules/no-consecutive-blank-lines/multiple/test.ts.fix

@@ -11,8 +11,8 @@ class Clazz { // comment
 
 
         // still allowed since 2 lines only
-        
-        
+
+
         // this one won't be allowed anymore
 
         console.log("test");
@@ -24,4 +24,5 @@ class Clazz { // comment
 
 //Begin whitespace
 // The next lines contain only tabs or spaces, they are also considered "blank" lines
-
+
+

test/rules/no-consecutive-blank-lines/multiple/test.ts.lint

@@ -5,22 +5,22 @@
 class Clazz { // comment
 
 
+
 ~nil
 
 ~nil [Exceeds the 2 allowed consecutive blank lines]
-
     public funcxion() {
 
         // also comment
 
 
         // still allowed since 2 lines only
-
+
+
 
 ~nil
-
-~nil [Exceeds the 2 allowed consecutive blank lines]
 
+~nil [Exceeds the 2 allowed consecutive blank lines]
 
         // this one won't be allowed anymore
 
@@ -35,6 +35,6 @@ class Clazz { // comment
 // The next lines contain only tabs or spaces, they are also considered "blank" lines
 
 
-~ [Exceeds the 2 allowed consecutive blank lines]
 
+~ [Exceeds the 2 allowed consecutive blank lines]
 

yarn.lock

@@ -1184,9 +1184,9 @@ tslint@latest:
     resolve "^1.1.7"
     update-notifier "^1.0.2"
 
-tsutils@^1.0.0:
-  version "1.0.0"
-  resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-1.0.0.tgz#a1f87d1092179987d13a1a8406deaec28ac3a0ad"
+tsutils@^1.1.0:
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-1.1.0.tgz#94e0c267624eeb1b63561ba8ec0bcff71b4e2872"
 
 [email protected]:
   version "0.1.1"