From c5035101957fe1223915968272c42feb47a7c6fa Mon Sep 17 00:00:00 2001 From: mgechev Date: Tue, 20 Jun 2017 18:47:58 -0700 Subject: [PATCH] fix: proper position in html/css source file Fix #343 --- src/angular/ngWalker.ts | 8 +- src/angularWhitespaceRule.ts | 2 +- test/angularWhitespaceRule.spec.ts | 23 +++- test/fixtures/angularWhitespace/valid.html | 1 + test/testHelper.ts | 26 +++- test/utils.ts | 133 +++++++++++++++++++++ 6 files changed, 187 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/angularWhitespace/valid.html create mode 100644 test/utils.ts diff --git a/src/angular/ngWalker.ts b/src/angular/ngWalker.ts index abae0d757..a324fa639 100644 --- a/src/angular/ngWalker.ts +++ b/src/angular/ngWalker.ts @@ -193,7 +193,13 @@ export class NgWalker extends Lint.RuleWalker { if (!path) { return current; } - return ts.createSourceFile(path, `\`${content}\``, ts.ScriptTarget.ES5); + const sf = ts.createSourceFile(path, `\`${content}\``, ts.ScriptTarget.ES5); + const original = sf.getFullText; + sf.getFullText = function () { + const text = original.apply(sf); + return text.substring(1, text.length - 1); + }.bind(sf); + return sf; } } diff --git a/src/angularWhitespaceRule.ts b/src/angularWhitespaceRule.ts index 77f505b7b..7bfc77209 100644 --- a/src/angularWhitespaceRule.ts +++ b/src/angularWhitespaceRule.ts @@ -179,7 +179,7 @@ export class Rule extends Lint.Rules.AbstractRule { static FAILURE: string = 'The %s "%s" that you\'re trying to access does not exist in the class declaration.'; - public apply(sourceFile:ts.SourceFile): Lint.RuleFailure[] { + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithWalker( new NgWalker(sourceFile, this.getOptions(), { diff --git a/test/angularWhitespaceRule.spec.ts b/test/angularWhitespaceRule.spec.ts index bc34ae90e..6958032ff 100644 --- a/test/angularWhitespaceRule.spec.ts +++ b/test/angularWhitespaceRule.spec.ts @@ -1,6 +1,15 @@ import { assertSuccess, assertAnnotated, assertMultipleAnnotated } from './testHelper'; import { Replacement } from 'tslint'; import { expect } from 'chai'; +import { FsFileResolver } from '../src/angular/fileResolver/fsFileResolver'; +import { MetadataReader } from '../src/angular/metadataReader'; +import * as ts from 'typescript'; +import { ComponentMetadata } from '../src/angular/metadata'; +import chai = require('chai'); + +const getAst = (code: string, file = 'file.ts') => { + return ts.createSourceFile(file, code, ts.ScriptTarget.ES5, true); +}; describe('angular-whitespace', () => { describe('success', () => { @@ -86,7 +95,19 @@ describe('angular-whitespace', () => { assertSuccess('angular-whitespace', source, ['check-pipe']); }); - + it('should work with external templates', () => { + const code = ` + @Component({ + selector: 'foo', + moduleId: module.id, + templateUrl: 'valid.html', + }) + class Bar {} + `; + const reader = new MetadataReader(new FsFileResolver()); + const ast = getAst(code, __dirname + '/../../test/fixtures/angularWhitespace/component.ts'); + assertSuccess('angular-whitespace', ast, ['check-pipe']); + }); }); }); diff --git a/test/fixtures/angularWhitespace/valid.html b/test/fixtures/angularWhitespace/valid.html new file mode 100644 index 000000000..ddb82bdff --- /dev/null +++ b/test/fixtures/angularWhitespace/valid.html @@ -0,0 +1 @@ +
{{ foo | async | uppercase }}
diff --git a/test/testHelper.ts b/test/testHelper.ts index 4a9a081f5..acab0069c 100644 --- a/test/testHelper.ts +++ b/test/testHelper.ts @@ -1,6 +1,12 @@ import * as tslint from 'tslint'; import * as Lint from 'tslint'; import chai = require('chai'); +import * as ts from 'typescript'; +import { IOptions } from 'tslint'; +import { loadRules, convertRuleOptions } from './utils'; + +const fs = require('fs'); +const path = require('path'); interface ISourcePosition { line: number; @@ -26,7 +32,7 @@ export interface IExpectedFailure { * @param options additional options for the lint rule * @returns {LintResult} the result of linting */ -function lint(ruleName: string, source: string, options: any): tslint.LintResult { +function lint(ruleName: string, source: string | ts.SourceFile, options: any): tslint.LintResult { let configuration = { extends: [], rules: new Map>(), @@ -46,7 +52,21 @@ function lint(ruleName: string, source: string, options: any): tslint.LintResult }; let linter = new tslint.Linter(linterOptions, undefined); - linter.lint('file.ts', source, configuration); + if (typeof source === 'string') { + linter.lint('file.ts', source, configuration); + } else { + const rules = loadRules(convertRuleOptions(configuration.rules), linterOptions.rulesDirectory, false); + const res = [].concat.apply([], rules.map(r => r.apply(source))) as tslint.RuleFailure[]; + const errCount = res.filter(r => !r.getRuleSeverity || r.getRuleSeverity() === 'error').length; + return { + errorCount: errCount, + warningCount: res.length - errCount, + output: '', + format: null, + fixes: [].concat.apply(res.map(r => r.getFix())), + failures: res + }; + } return linter.getResult(); } @@ -235,7 +255,7 @@ export function assertFailures(ruleName: string, source: string, fails: IExpecte * @param source * @param options */ -export function assertSuccess(ruleName: string, source: string, options = null) { +export function assertSuccess(ruleName: string, source: string | ts.SourceFile, options = null) { const result = lint(ruleName, source, options); chai.assert.isTrue(result && result.failures.length === 0); } diff --git a/test/utils.ts b/test/utils.ts new file mode 100644 index 000000000..e6e82640d --- /dev/null +++ b/test/utils.ts @@ -0,0 +1,133 @@ +import { IOptions, IRule } from 'tslint'; +import * as fs from 'fs'; +import * as path from 'path'; + +export function convertRuleOptions(ruleConfiguration: Map>): IOptions[] { + const output: IOptions[] = []; + ruleConfiguration.forEach(({ ruleArguments, ruleSeverity }, ruleName) => { + const options: IOptions = { + disabledIntervals: [], // deprecated, so just provide an empty array. + ruleArguments: ruleArguments !== null ? ruleArguments : [], + ruleName, + ruleSeverity: ruleSeverity !== null ? ruleSeverity : 'error', + }; + output.push(options); + }); + return output; +} + +const cachedRules = new Map(); + +export function camelize(stringWithHyphens: string): string { + return stringWithHyphens.replace(/-(.)/g, (_, nextLetter) => (nextLetter as string).toUpperCase()); +} + +function transformName(name: string): string { + // camelize strips out leading and trailing underscores and dashes, so make sure they aren't passed to camelize + // the regex matches the groups (leading underscores and dashes)(other characters)(trailing underscores and dashes) + const nameMatch = name.match(/^([-_]*)(.*?)([-_]*)$/); + if (nameMatch === null) { + return `${name}Rule`; + } + return `${nameMatch[1]}${camelize(nameMatch[2])}${nameMatch[3]}Rule`; +} + +/** + * @param directory - An absolute path to a directory of rules + * @param ruleName - A name of a rule in filename format. ex) "someLintRule" + */ +function loadRule(directory: string, ruleName: string): any | 'not-found' { + const fullPath = path.join(directory, ruleName); + if (fs.existsSync(`${fullPath}.js`)) { + const ruleModule = require(fullPath) as { Rule: any } | undefined; + if (ruleModule !== undefined) { + return ruleModule.Rule; + } + } + return 'not-found'; +} + +export function getRelativePath(directory?: string | null, relativeTo?: string) { + if (directory !== null) { + const basePath = relativeTo !== undefined ? relativeTo : process.cwd(); + return path.resolve(basePath, directory); + } + return undefined; +} + +export function arrayify(arg?: T | T[]): T[] { + if (Array.isArray(arg)) { + return arg; + } else if (arg !== null) { + return [arg]; + } else { + return []; + } +} + +function loadCachedRule(directory: string, ruleName: string, isCustomPath?: boolean): any | undefined { + // use cached value if available + const fullPath = path.join(directory, ruleName); + const cachedRule = cachedRules.get(fullPath); + if (cachedRule !== undefined) { + return cachedRule === 'not-found' ? undefined : cachedRule; + } + + // get absolute path + let absolutePath: string | undefined = directory; + if (isCustomPath) { + absolutePath = getRelativePath(directory); + if (absolutePath !== undefined && !fs.existsSync(absolutePath)) { + throw new Error(`Could not find custom rule directory: ${directory}`); + } + } + + const Rule = absolutePath === undefined ? 'not-found' : loadRule(absolutePath, ruleName); + + cachedRules.set(fullPath, Rule); + return Rule === 'not-found' ? undefined : Rule; +} + +export function find(inputs: T[], getResult: (t: T) => U | undefined): U | undefined { + for (const element of inputs) { + const result = getResult(element); + if (result !== undefined) { + return result; + } + } + return undefined; +} + +function findRule(name: string, rulesDirectories?: string | string[]): any | undefined { + const camelizedName = transformName(name); + return find(arrayify(rulesDirectories), (dir) => loadCachedRule(dir, camelizedName, true)); +} + +export function loadRules(ruleOptionsList: IOptions[], + rulesDirectories?: string | string[], + isJs = false): IRule[] { + const rules: IRule[] = []; + const notFoundRules: string[] = []; + const notAllowedInJsRules: string[] = []; + + for (const ruleOptions of ruleOptionsList) { + if (ruleOptions.ruleSeverity === 'off') { + // Perf: don't bother finding the rule if it's disabled. + continue; + } + + const ruleName = ruleOptions.ruleName; + const Rule = findRule(ruleName, rulesDirectories); + if (Rule === undefined) { + notFoundRules.push(ruleName); + } else if (isJs && Rule.metadata !== undefined && Rule.metadata.typescriptOnly) { + notAllowedInJsRules.push(ruleName); + } else { + const rule = new Rule(ruleOptions); + if (rule.isEnabled()) { + rules.push(rule); + } + } + } + return rules; +}