diff --git a/src/angular/sourceMappingVisitor.ts b/src/angular/sourceMappingVisitor.ts index 0e767538e..8d52e403a 100644 --- a/src/angular/sourceMappingVisitor.ts +++ b/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(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 }; } } diff --git a/src/noUnusedCssRule.ts b/src/noUnusedCssRule.ts index e05746398..114e2eeee 100644 --- a/src/noUnusedCssRule.ts +++ b/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); diff --git a/src/useLifeCycleInterfaceRule.ts b/src/useLifeCycleInterfaceRule.ts index 09008973d..94d8f78b0 100644 --- a/src/useLifeCycleInterfaceRule.ts +++ b/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 = (m.name).text; if(n && this.isMethodValidHook(m,interfaces)){ let hookName = n.substr(2, n.lenght); diff --git a/test/angular/sourceMappingVisitor.spec.ts b/test/angular/sourceMappingVisitor.spec.ts index d480cf4bd..30f04a692 100644 --- a/test/angular/sourceMappingVisitor.spec.ts +++ b/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); }); }); diff --git a/test/noUnusedCssRule.spec.ts b/test/noUnusedCssRule.spec.ts index b91366be8..0392f1e98 100644 --- a/test/noUnusedCssRule.spec.ts +++ b/test/noUnusedCssRule.spec.ts @@ -1,3 +1,4 @@ +import {expect} from 'chai'; import {Decorator} from 'typescript'; import * as sass from 'node-sass'; @@ -257,7 +258,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 12, - character: 12 + character: 13 } }); }); @@ -295,7 +296,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 17, - character: 12 + character: 13 } }); }); @@ -328,7 +329,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 12, - character: 12 + character: 13 } }); }); @@ -396,7 +397,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 12, - character: 14 + character: 15 } }); }); @@ -456,7 +457,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 12, - character: 12 + character: 13 } }); }); @@ -514,7 +515,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 12, - character: 12 + character: 13 } }); }); @@ -570,7 +571,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 12, - character: 12 + character: 13 } }); }); @@ -629,7 +630,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 12, - character: 12 + character: 13 } }); }); @@ -695,7 +696,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 9, - character: 12 + character: 13 } }); }); @@ -723,7 +724,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 9, - character: 12 + character: 13 } }); }); @@ -751,7 +752,7 @@ describe('no-unused-css', () => { }, endPosition: { line: 9, - character: 12 + character: 13 } }); }); @@ -794,12 +795,12 @@ describe('no-unused-css', () => { assertFailure('no-unused-css', source, { message: 'Unused styles', startPosition: { - line: 9, - character: 8 + line: 8, + character: 7 }, endPosition: { - line: 13, - character: 11 + line: 12, + character: 12 } }); Config.transformStyle = (code: string) => ({ code, map: null }); @@ -830,4 +831,96 @@ describe('no-unused-css', () => { }); + describe('autofixes', () => { + + it('should work with regular CSS', () => { + let source = ` + @Component({ + selector: 'foobar', + encapsulation: prefix.foo.ViewEncapsulation.Emulated, + template: \`
\`, + styles: [ + \` + p { + color: red; + } + \` + ] + }) + class Test {}`; + const failures = assertFailure('no-unused-css', source, { + message: 'Unused styles', + startPosition: { + line: 7, + character: 12 + }, + endPosition: { + line: 9, + character: 13 + } + }, null); + const fix = failures[0].getFix(); + const replacements = fix.replacements; + expect(replacements.length).to.eq(1); + const replacement = replacements[0]; + expect(replacement.text).to.eq(''); + expect(replacement.start).to.eq(197); + expect(replacement.end).to.eq(240); + }); + + it('should work with SASS', () => { + Config.transformStyle = (source: string, url: string, d: Decorator) => { + const res = sass.renderSync({ + sourceMap: true, data: source, sourceMapEmbed: true + }); + const code = res.css.toString(); + const base64Map = code.match(/\/\*(.*?)\*\//)[1].replace('# sourceMappingURL=data:application/json;base64,', ''); + const map = JSON.parse(new Buffer(base64Map, 'base64').toString('ascii')); + return { code, source, map }; + }; + + let source = ` + @Component({ + selector: 'hero-cmp', + template: \` +

Hello {{ hero.name }}

+ \`, + styles: [ + \` + h1 { + spam { + baz { + color: red; + } + } + } + \` + ] + }) + class HeroComponent { + private hero: Hero; + }`; + const failures = assertFailure('no-unused-css', source, { + message: 'Unused styles', + startPosition: { + line: 8, + character: 9 + }, + endPosition: { + line: 12, + character: 14 + } + }); + Config.transformStyle = (code: string) => ({ code, map: null }); + const fix = failures[0].getFix(); + const replacements = fix.replacements; + expect(replacements.length).to.eq(1); + const replacement = replacements[0]; + expect(replacement.text).to.eq(''); + expect(replacement.start).to.eq(174); + expect(replacement.end).to.eq(261); // should be 276 + }); + + }); + }); diff --git a/test/testHelper.ts b/test/testHelper.ts index ae83207eb..6619c67e6 100644 --- a/test/testHelper.ts +++ b/test/testHelper.ts @@ -1,4 +1,5 @@ import * as tslint from 'tslint'; +import * as Lint from 'tslint'; import chai = require('chai'); interface ISourcePosition { @@ -12,7 +13,7 @@ export interface IExpectedFailure { endPosition: ISourcePosition; } -function lint(ruleName: string, source: string, options): tslint.LintResult { +function lint(ruleName: string, source: string, options: any): tslint.LintResult { let configuration = { rules: {} }; @@ -95,8 +96,8 @@ export function assertAnnotated(config: AssertConfig) { } }; -export function assertFailure(ruleName: string, source: string, fail: IExpectedFailure, options = null) { - let result; +export function assertFailure(ruleName: string, source: string, fail: IExpectedFailure, options = null): Lint.RuleFailure[] { + let result: Lint.LintResult; try { result = lint(ruleName, source, options); } catch (e) { @@ -108,6 +109,10 @@ export function assertFailure(ruleName: string, source: string, fail: IExpectedF chai.assert.deepEqual(fail.startPosition, ruleFail.getStartPosition().getLineAndCharacter(), 'start char doesn\'t match'); chai.assert.deepEqual(fail.endPosition, ruleFail.getEndPosition().getLineAndCharacter(), 'end char doesn\'t match'); }); + if (result) { + return result.failures; + } + return undefined; }; export function assertFailures(ruleName: string, source: string, fails: IExpectedFailure[], options = null) { @@ -120,7 +125,8 @@ export function assertFailures(ruleName: string, source: string, fails: IExpecte chai.assert(result.failureCount > 0, 'no failures'); result.failures.forEach((ruleFail,index) => { chai.assert.equal(fails[index].message, ruleFail.getFailure(), 'error messages dont\'t match'); - chai.assert.deepEqual(fails[index].startPosition, ruleFail.getStartPosition().getLineAndCharacter(), 'start char doesn\'t match'); + chai.assert.deepEqual(fails[index].startPosition, ruleFail.getStartPosition().getLineAndCharacter(), + 'start char doesn\'t match'); chai.assert.deepEqual(fails[index].endPosition, ruleFail.getEndPosition().getLineAndCharacter(), 'end char doesn\'t match'); }); };