Merge pull request #246 from mgechev/unused-css-fix

Unused css auto-fix & better source position mapping

src/angular/sourceMappingVisitor.ts

@@ -1,57 +1,139 @@
 import * as ts from 'typescript';
-import {RuleWalker, RuleFailure, IOptions} from 'tslint';
+import {RuleWalker, RuleFailure, IOptions, Fix, Replacement} from 'tslint';
 import {ComponentMetadata, CodeWithSourceMap} from './metadata';
 import {SourceMapConsumer} from 'source-map';
 
-const findLineAndColumnNumber = (pos: number, code: string) => {
-  code = code.replace('\r\n', '\n').replace('\r', '\n');
-  let line = 1;
-  let column = 0;
-  for (let i = 0; i < pos; i += 1) {
-    column += 1;
-    if (code[i] === '\n') {
-      line += 1;
-      column = 0;
+const LineFeed = 0x0A;
+const CarriageReturn = 0x0D;
+const MaxAsciiCharacter = 0x7F;
+const LineSeparator = 0x2028;
+const ParagraphSeparator = 0x2029;
+
+export function isLineBreak(ch: number): boolean {
+  return ch === LineFeed ||
+      ch === CarriageReturn ||
+      ch === LineSeparator ||
+      ch === ParagraphSeparator;
+}
+
+function binarySearch<T>(array: T[], value: T, comparer?: (v1: T, v2: T) => number, offset?: number): number {
+  if (!array || array.length === 0) {
+      return -1;
+  }
+
+  let low = offset || 0;
+  let high = array.length - 1;
+  comparer = comparer !== undefined
+      ? comparer
+      : (v1, v2) => (v1 < v2 ? -1 : (v1 > v2 ? 1 : 0));
+
+  while (low <= high) {
+    const middle = low + ((high - low) >> 1);
+    const midValue = array[middle];
+
+    if (comparer(midValue, value) === 0) {
+      return middle;
+    } else if (comparer(midValue, value) > 0) {
+      high = middle - 1;
+    } else {
+      low = middle + 1;
     }
   }
-  return { line, column };
-};
 
-const findCharNumberFromLineAndColumn = ({ line, column }: { line: number, column: number }, code: string) => {
-  code = code.replace('\r\n', '\n').replace('\r', '\n');
-  let char = 0;
-  while (line) {
-    if (code[char] === '\n') {
-      line -= 1;
+  return ~low;
+}
+
+// Apply caching and do not recompute every time
+function getLineAndCharacterOfPosition(sourceFile: string, position: number) {
+  return computeLineAndCharacterOfPosition(computeLineStarts(sourceFile), position);
+}
+
+// Apply caching and do not recompute every time
+function getPositionOfLineAndCharacter(sourceFile: string, line: number, character: number): number {
+  return computePositionOfLineAndCharacter(computeLineStarts(sourceFile), line, character);
+}
+
+function computePositionOfLineAndCharacter(lineStarts: number[], line: number, character: number): number {
+  return lineStarts[line] + character;
+}
+
+function computeLineAndCharacterOfPosition(lineStarts: number[], position: number) {
+  let lineNumber = binarySearch(lineStarts, position);
+  if (lineNumber < 0) {
+    lineNumber = ~lineNumber - 1;
+  }
+  return {
+    line: lineNumber,
+    character: position - lineStarts[lineNumber]
+  };
+}
+
+function computeLineStarts(text: string): number[] {
+  const result: number[] = new Array();
+  let pos = 0;
+  let lineStart = 0;
+  while (pos < text.length) {
+    const ch = text.charCodeAt(pos);
+    pos++;
+    switch (ch) {
+      case CarriageReturn:
+        if (text.charCodeAt(pos) === LineFeed) {
+          pos++;
+        }
+      case LineFeed:
+        result.push(lineStart);
+        lineStart = pos;
+        break;
+      default:
+        if (ch > MaxAsciiCharacter && isLineBreak(ch)) {
+          result.push(lineStart);
+          lineStart = pos;
+        }
+        break;
     }
-    char += 1;
   }
-  return char + column;
-};
+  result.push(lineStart);
+  return result;
+}
 
 export class SourceMappingVisitor extends RuleWalker {
+  private consumer: SourceMapConsumer;
 
   constructor(sourceFile: ts.SourceFile, options: IOptions, protected codeWithMap: CodeWithSourceMap, protected basePosition: number) {
     super(sourceFile, options);
+    if (this.codeWithMap.map) {
+      this.consumer = new SourceMapConsumer(this.codeWithMap.map);
+    }
   }
 
-  createFailure(start: number, length: number, message: string): RuleFailure {
-    let end = start + length;
-    if (this.codeWithMap.map) {
-      const consumer = new SourceMapConsumer(this.codeWithMap.map);
-      start = this.getMappedPosition(start, consumer);
-      end = this.getMappedPosition(end, consumer);
-    } else {
-      start += this.basePosition;
-      end = start + length;
+  createFailure(s: number, l: number, message: string, fix?: Fix): RuleFailure {
+    const { start, length } = this.getMappedInterval(s, l);
+    return super.createFailure(start, length, message, fix);
+  }
+
+  createReplacement(s: number, l: number, replacement: string): Replacement {
+    const { start, length } = this.getMappedInterval(s, l);
+    return super.createReplacement(start, length, replacement);
+  }
+
+  getSourcePosition(pos: number) {
+    if (this.consumer) {
+      try {
+        let absPos = getLineAndCharacterOfPosition(this.codeWithMap.code, pos);
+        const result = this.consumer.originalPositionFor({ line: absPos.line + 1, column: absPos.character + 1 });
+        absPos = { line: result.line - 1, character: result.column - 1 };
+        pos = getPositionOfLineAndCharacter(this.codeWithMap.source, absPos.line, absPos.character);
+      } catch (e) {
+        console.log(e);
+      }
     }
-    return super.createFailure(start, end - start, message);
+    return pos + this.basePosition;
   }
 
-  private getMappedPosition(pos: number, consumer: SourceMapConsumer) {
-    const absPos = findLineAndColumnNumber(pos, this.codeWithMap.code);
-    const mappedPos = consumer.originalPositionFor(absPos);
-    const char = findCharNumberFromLineAndColumn(mappedPos, this.codeWithMap.source);
-    return char + this.basePosition;
+  private getMappedInterval(start: number, length: number) {
+    let end = start + length;
+    start = this.getSourcePosition(start);
+    end = this.getSourcePosition(end);
+    return { start, length: end - start };
   }
 }

src/noUnusedCssRule.ts

@@ -12,7 +12,7 @@ import {
   PropertyBindingType
 } from '@angular/compiler';
 import {parseTemplate} from './angular/templates/templateParser';
-import {CssAst, CssSelectorRuleAst, CssSelectorAst} from './angular/styles/cssAst';
+import { CssAst, CssSelectorRuleAst, CssSelectorAst, CssBlockAst } from './angular/styles/cssAst';
 
 import {ComponentMetadata, StyleMetadata} from './angular/metadata';
 import {ng2WalkerFactoryUtils} from './angular/ng2WalkerFactoryUtils';
@@ -153,12 +153,25 @@ export class Rule extends Lint.Rules.AbstractRule {
 class UnusedCssVisitor extends BasicCssAstVisitor {
   templateAst: TemplateAst;
 
+  constructor(sourceFile: ts.SourceFile,
+    originalOptions: Lint.IOptions,
+    context: ComponentMetadata,
+    protected style: StyleMetadata,
+    templateStart: number) {
+      super(sourceFile, originalOptions, context, style, templateStart);
+    }
+
   visitCssSelectorRule(ast: CssSelectorRuleAst) {
     try {
       const match = ast.selectors.some(s => this.visitCssSelector(s));
       if (!match) {
-        this.addFailure(this.createFailure(ast.start.offset,
-          ast.end.offset - ast.start.offset, 'Unused styles'));
+        // We need this because of eventual source maps
+        const start = ast.start.offset;
+        const end = ast.end.offset;
+        const length = end - ast.start.offset + 1;
+        // length + 1 because we want to drop the '}'
+        const fix = this.createFix(this.createReplacement(start, length, ''));
+        this.addFailure(this.createFailure(start, length, 'Unused styles', fix));
       }
     } catch (e) {
       logger.error(e);

src/useLifeCycleInterfaceRule.ts

@@ -57,7 +57,7 @@ export class ClassMetadataWalker extends Lint.RuleWalker {
     }
 
     private validateMethods( methods:any[],interfaces:string[],className:string){
-        methods.forEach(m=>{
+        methods.forEach(m => {
             let n = (<any>m.name).text;
             if(n && this.isMethodValidHook(m,interfaces)){
                 let hookName = n.substr(2, n.lenght);

test/angular/sourceMappingVisitor.spec.ts

@@ -25,7 +25,7 @@ const fixture1 =
 export class Foo {}
 `;
 
-describe('metadataReader', () => {
+describe('SourceMappingVisitor', () => {
 
   it('should map to correct position', () => {
     const ast = getAst(fixture1);
@@ -39,8 +39,8 @@ describe('metadataReader', () => {
       map: JSON.parse(result.map.toString()),
       source: scss
     }, styleNode.getStart() + 1);
-    const failure = visitor.createFailure(0, 4, 'bar');
-    chai.expect(failure.getStartPosition().getPosition()).eq(46);
-    chai.expect(failure.getEndPosition().getPosition()).eq(50);
+    const failure = visitor.createFailure(0, 3, 'bar');
+    chai.expect(failure.getStartPosition().getPosition()).eq(34);
+    chai.expect(failure.getEndPosition().getPosition()).eq(38);
   });
 });