From ecb8cd5b20abf60f334ec43a80f3b929343c6b95 Mon Sep 17 00:00:00 2001 From: WilliamKoza Date: Fri, 23 Nov 2018 21:20:57 +0100 Subject: [PATCH 1/3] feat(rule): add new Rule RelativePathExternalResourcesRule --- src/index.ts | 1 + src/relativeUrlPrefixRule.ts | 66 +++++++++++++++++ test/relativeUrlPrefixRule.spec.ts | 110 +++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+) create mode 100644 src/relativeUrlPrefixRule.ts create mode 100644 test/relativeUrlPrefixRule.spec.ts diff --git a/src/index.ts b/src/index.ts index 88b2cd4a2..d9dd0f6cd 100644 --- a/src/index.ts +++ b/src/index.ts @@ -38,6 +38,7 @@ export { Rule as UseOutputPropertyDecoratorRule } from './useOutputPropertyDecor export { Rule as UsePipeDecoratorRule } from './usePipeDecoratorRule'; export { Rule as UsePipeTransformInterfaceRule } from './usePipeTransformInterfaceRule'; export { Rule as UseViewEncapsulationRule } from './useViewEncapsulationRule'; +export { Rule as RelativePathExternalResourcesRule } from './relativeUrlPrefixRule'; export * from './angular'; diff --git a/src/relativeUrlPrefixRule.ts b/src/relativeUrlPrefixRule.ts new file mode 100644 index 000000000..bb196b8e8 --- /dev/null +++ b/src/relativeUrlPrefixRule.ts @@ -0,0 +1,66 @@ +import { IOptions, IRuleMetadata, RuleFailure, Rules } from 'tslint/lib'; +import { SourceFile } from 'typescript/lib/typescript'; +import { NgWalker } from './angular/ngWalker'; +import * as ts from 'typescript'; + +export class Rule extends Rules.AbstractRule { + static readonly metadata: IRuleMetadata = { + description: "The ./ prefix is standard syntax for relative URLs; don't depend on Angular's current ability to do without that prefix.", + descriptionDetails: 'See more at https://angular.io/guide/styleguide#style-05-04.', + rationale: 'A component relative URL requires no change when you move the component files, as long as the files stay together.', + ruleName: 'relative-url-prefix', + options: null, + optionsDescription: 'Not configurable.', + type: 'maintainability', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'The ./ prefix is standard syntax for relative URLs. (https://angular.io/guide/styleguide#style-05-04)'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new RelativePathExternalResourcesRuleWalker(sourceFile, this.getOptions())); + } +} + +export class RelativePathExternalResourcesRuleWalker extends NgWalker { + constructor(sourceFile: SourceFile, options: IOptions) { + super(sourceFile, options); + } + + visitClassDecorator(decorator: ts.Decorator) { + if (ts.isCallExpression(decorator.expression) && decorator.expression.arguments) { + decorator.expression.arguments.forEach(arg => { + if (ts.isObjectLiteralExpression(arg) && arg.properties) { + arg.properties.forEach((prop: any) => { + if (prop && prop.name.text === 'templateUrl') { + const url = prop.initializer.text; + this.checkTemplateUrl(url, prop.initializer); + } else if (prop && prop.name.text === 'styleUrls') { + if (prop.initializer.elements.length > 0) { + prop.initializer.elements.forEach(e => { + const url = e.text; + this.checkStyleUrls(e); + }); + } + } + }); + } + }); + } + super.visitClassDecorator(decorator); + } + + private checkTemplateUrl(url: string, initializer: ts.StringLiteral) { + if (url && !/^.\/./i.test(url)) { + this.addFailureAtNode(initializer, Rule.FAILURE_STRING); + } + } + + private checkStyleUrls(token: ts.StringLiteral) { + if (token && token.text) { + if (!/^.\/./i.test(token.text)) { + this.addFailureAtNode(token, Rule.FAILURE_STRING); + } + } + } +} diff --git a/test/relativeUrlPrefixRule.spec.ts b/test/relativeUrlPrefixRule.spec.ts new file mode 100644 index 000000000..efa4bc411 --- /dev/null +++ b/test/relativeUrlPrefixRule.spec.ts @@ -0,0 +1,110 @@ +import { Rule } from '../src/relativeUrlPrefixRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('styleUrls', () => { + describe('success', () => { + it('should succeed when a relative URL is prefixed by ./', () => { + const source = ` + @Component({ + styleUrls: ['./foobar.css'] + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed when all relative URLs is prefixed by ./', () => { + const source = ` + @Component({ + styleUrls: ['./foo.css', './bar.css', './whatyouwant.css'] + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + }); + + describe('failure', () => { + it("should fail when a relative URL isn't prefixed by ./", () => { + const source = ` + @Component({ + styleUrls: ['foobar.css'] + ~~~~~~~~~~~~ + }) + class Test {} + `; + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source + }); + }); + + it("should fail when one relative URLs isn't prefixed by ./", () => { + const source = ` + @Component({ + styleUrls: ['./foo.css', 'bar.css', './whatyouwant.css'] + ~~~~~~~~~ + }) + class Test {} + `; + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source + }); + }); + }); + }); + + describe('templateUrl', () => { + describe('success', () => { + it('should succeed when a relative URL is prefixed by ./', () => { + const source = ` + @Component({ + templateUrl: './foobar.html' + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + }); + + describe('failure', () => { + it("should succeed when a relative URL isn't prefixed by ./", () => { + const source = ` + @Component({ + templateUrl: 'foobar.html' + ~~~~~~~~~~~~~ + }) + class Test {} + `; + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source + }); + }); + + it('should fail when a relative URL is prefixed by ../', () => { + const source = ` + @Component({ + templateUrl: '../foobar.html' + ~~~~~~~~~~~~~~~~ + }) + class Test {} + `; + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source + }); + }); + }); + }); +}); From 253ab524d6eeab9f9ba1255e3da00a70eddf4a2a Mon Sep 17 00:00:00 2001 From: WilliamKoza Date: Sat, 24 Nov 2018 09:46:24 +0100 Subject: [PATCH 2/3] feat(rule): fix regexp --- src/relativeUrlPrefixRule.ts | 6 +++--- test/relativeUrlPrefixRule.spec.ts | 32 +++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/relativeUrlPrefixRule.ts b/src/relativeUrlPrefixRule.ts index bb196b8e8..9c9d41945 100644 --- a/src/relativeUrlPrefixRule.ts +++ b/src/relativeUrlPrefixRule.ts @@ -6,7 +6,7 @@ import * as ts from 'typescript'; export class Rule extends Rules.AbstractRule { static readonly metadata: IRuleMetadata = { description: "The ./ prefix is standard syntax for relative URLs; don't depend on Angular's current ability to do without that prefix.", - descriptionDetails: 'See more at https://angular.io/guide/styleguide#style-05-04.', + descriptionDetails: 'See more at https://angular.io/styleguide#style-05-04.', rationale: 'A component relative URL requires no change when you move the component files, as long as the files stay together.', ruleName: 'relative-url-prefix', options: null, @@ -51,14 +51,14 @@ export class RelativePathExternalResourcesRuleWalker extends NgWalker { } private checkTemplateUrl(url: string, initializer: ts.StringLiteral) { - if (url && !/^.\/./i.test(url)) { + if (url && !/^\.\/[^\.\/|\.\.\/]/.test(url)) { this.addFailureAtNode(initializer, Rule.FAILURE_STRING); } } private checkStyleUrls(token: ts.StringLiteral) { if (token && token.text) { - if (!/^.\/./i.test(token.text)) { + if (!/^\.\/[^\.\/|\.\.\/]/.test(token.text)) { this.addFailureAtNode(token, Rule.FAILURE_STRING); } } diff --git a/test/relativeUrlPrefixRule.spec.ts b/test/relativeUrlPrefixRule.spec.ts index efa4bc411..3e9f12d20 100644 --- a/test/relativeUrlPrefixRule.spec.ts +++ b/test/relativeUrlPrefixRule.spec.ts @@ -45,6 +45,21 @@ describe(ruleName, () => { }); }); + it("should fail when a relative URL isn't prefixed by ./", () => { + const source = ` + @Component({ + styleUrls: ['./../foobar.css'] + ~~~~~~~~~~~~~~~~~ + }) + class Test {} + `; + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source + }); + }); + it("should fail when one relative URLs isn't prefixed by ./", () => { const source = ` @Component({ @@ -56,7 +71,7 @@ describe(ruleName, () => { assertAnnotated({ ruleName, message: Rule.FAILURE_STRING, - source + source }); }); }); @@ -105,6 +120,21 @@ describe(ruleName, () => { source }); }); + + it('should fail when a relative URL is prefixed by ../', () => { + const source = ` + @Component({ + templateUrl: '.././foobar.html' + ~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `; + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source + }); + }); }); }); }); From a42360d56894d1d22d6cf676d3f22397d9fbfa0d Mon Sep 17 00:00:00 2001 From: WilliamKoza Date: Sat, 24 Nov 2018 09:47:56 +0100 Subject: [PATCH 3/3] feat(rule): fix message --- src/relativeUrlPrefixRule.ts | 2 +- test/relativeUrlPrefixRule.spec.ts | 30 +++++++++++++++--------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/relativeUrlPrefixRule.ts b/src/relativeUrlPrefixRule.ts index 9c9d41945..a43fe1083 100644 --- a/src/relativeUrlPrefixRule.ts +++ b/src/relativeUrlPrefixRule.ts @@ -15,7 +15,7 @@ export class Rule extends Rules.AbstractRule { typescriptOnly: true }; - static readonly FAILURE_STRING = 'The ./ prefix is standard syntax for relative URLs. (https://angular.io/guide/styleguide#style-05-04)'; + static readonly FAILURE_STRING = 'The ./ prefix is standard syntax for relative URLs. (https://angular.io/styleguide#style-05-04)'; apply(sourceFile: SourceFile): RuleFailure[] { return this.applyWithWalker(new RelativePathExternalResourcesRuleWalker(sourceFile, this.getOptions())); diff --git a/test/relativeUrlPrefixRule.spec.ts b/test/relativeUrlPrefixRule.spec.ts index 3e9f12d20..cb934bc04 100644 --- a/test/relativeUrlPrefixRule.spec.ts +++ b/test/relativeUrlPrefixRule.spec.ts @@ -45,20 +45,20 @@ describe(ruleName, () => { }); }); - it("should fail when a relative URL isn't prefixed by ./", () => { - const source = ` + it("should fail when a relative URL isn't prefixed by ./", () => { + const source = ` @Component({ styleUrls: ['./../foobar.css'] ~~~~~~~~~~~~~~~~~ }) class Test {} `; - assertAnnotated({ - ruleName, - message: Rule.FAILURE_STRING, - source - }); + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source }); + }); it("should fail when one relative URLs isn't prefixed by ./", () => { const source = ` @@ -71,7 +71,7 @@ describe(ruleName, () => { assertAnnotated({ ruleName, message: Rule.FAILURE_STRING, - source + source }); }); }); @@ -121,20 +121,20 @@ describe(ruleName, () => { }); }); - it('should fail when a relative URL is prefixed by ../', () => { - const source = ` + it('should fail when a relative URL is prefixed by ../', () => { + const source = ` @Component({ templateUrl: '.././foobar.html' ~~~~~~~~~~~~~~~~~~ }) class Test {} `; - assertAnnotated({ - ruleName, - message: Rule.FAILURE_STRING, - source - }); + assertAnnotated({ + ruleName, + message: Rule.FAILURE_STRING, + source }); + }); }); }); });