diff --git a/src/angular/metadata.ts b/src/angular/metadata.ts index 54043bd7b..c01a812cf 100644 --- a/src/angular/metadata.ts +++ b/src/angular/metadata.ts @@ -1,5 +1,5 @@ -import * as ts from 'typescript'; import { RawSourceMap } from 'source-map'; +import * as ts from 'typescript'; export interface CodeWithSourceMap { code: string; @@ -12,6 +12,10 @@ interface PropertyMetadata { url?: string; } +export interface AnimationMetadata extends PropertyMetadata { + animation: CodeWithSourceMap; +} + export interface StyleMetadata extends PropertyMetadata { style: CodeWithSourceMap; } @@ -27,6 +31,7 @@ export class DirectiveMetadata { } export class ComponentMetadata extends DirectiveMetadata { + animations!: AnimationMetadata[]; styles!: StyleMetadata[]; template!: TemplateMetadata; } diff --git a/src/angular/metadataReader.ts b/src/angular/metadataReader.ts index a6a298acd..7e30d49c8 100644 --- a/src/angular/metadataReader.ts +++ b/src/angular/metadataReader.ts @@ -9,11 +9,11 @@ import { } from '../util/astQuery'; import { ifTrue, listToMaybe, Maybe, unwrapFirst } from '../util/function'; import { logger } from '../util/logger'; -import { getInlineStyle, getTemplate } from '../util/ngQuery'; +import { getAnimations, getInlineStyle, getTemplate } from '../util/ngQuery'; import { maybeNodeArray } from '../util/utils'; import { Config } from './config'; import { FileResolver } from './fileResolver/fileResolver'; -import { CodeWithSourceMap, ComponentMetadata, DirectiveMetadata, StyleMetadata, TemplateMetadata } from './metadata'; +import { AnimationMetadata, CodeWithSourceMap, ComponentMetadata, DirectiveMetadata, StyleMetadata, TemplateMetadata } from './metadata'; import { AbstractResolver, MetadataUrls } from './urlResolvers/abstractResolver'; import { PathResolver } from './urlResolvers/pathResolver'; import { UrlResolver } from './urlResolvers/urlResolver'; @@ -71,10 +71,12 @@ export class MetadataReader { const expr = this.getDecoratorArgument(dec); const directiveMetadata = this.readDirectiveMetadata(d, dec); const external_M = expr.fmap(() => this._urlResolver!.resolve(dec)); + const animations_M = external_M.bind(external => this.readComponentAnimationsMetadata(dec, external!)); const style_M = external_M.bind(external => this.readComponentStylesMetadata(dec, external!)); const template_M = external_M.bind(external => this.readComponentTemplateMetadata(dec, external!)); return Object.assign(new ComponentMetadata(), directiveMetadata, { + animations: animations_M.unwrap(), styles: style_M.unwrap(), template: template_M.unwrap() }); @@ -84,6 +86,17 @@ export class MetadataReader { return decoratorArgument(decorator).bind(ifTrue(hasProperties)); } + protected readComponentAnimationsMetadata(dec: ts.Decorator, external: MetadataUrls): Maybe { + return getAnimations(dec).fmap(inlineAnimations => + inlineAnimations!.elements + .filter(inlineAnimation => isSimpleTemplateString(inlineAnimation)) + .map(inlineAnimation => ({ + animation: normalizeTransformed({ code: (inlineAnimation as ts.StringLiteral | ts.NoSubstitutionTemplateLiteral).text }), + node: inlineAnimation as ts.Node + })) + ); + } + protected readComponentTemplateMetadata(dec: ts.Decorator, external: MetadataUrls): Maybe { // Resolve Inline template return getTemplate(dec) diff --git a/src/maxInlineDeclarationsRule.ts b/src/maxInlineDeclarationsRule.ts index 06fcad574..33eb2bc66 100644 --- a/src/maxInlineDeclarationsRule.ts +++ b/src/maxInlineDeclarationsRule.ts @@ -4,8 +4,10 @@ import { SourceFile } from 'typescript/lib/typescript'; import { CodeWithSourceMap, ComponentMetadata } from './angular/metadata'; import { NgWalker } from './angular/ngWalker'; +const DEFAULT_ANIMATIONS_LIMIT: number = 15; const DEFAULT_STYLES_LIMIT: number = 3; const DEFAULT_TEMPLATE_LIMIT: number = 3; +const OPTION_ANIMATIONS = 'animations'; const OPTION_STYLES = 'styles'; const OPTION_TEMPLATE = 'template'; @@ -13,10 +15,13 @@ export class Rule extends Rules.AbstractRule { static readonly metadata: IRuleMetadata = { description: 'Disallows having too many lines in inline template and styles. Forces separate template or styles file creation.', descriptionDetails: 'See more at https://angular.io/guide/styleguide#style-05-04.', - optionExamples: [true, [true, { [OPTION_STYLES]: 8, [OPTION_TEMPLATE]: 5 }]], + optionExamples: [true, [true, { [OPTION_ANIMATIONS]: 20, [OPTION_STYLES]: 8, [OPTION_TEMPLATE]: 5 }]], options: { items: { properties: { + [OPTION_ANIMATIONS]: { + type: 'number' + }, [OPTION_STYLES]: { type: 'number' }, @@ -31,7 +36,8 @@ export class Rule extends Rules.AbstractRule { type: 'array' }, optionsDescription: Utils.dedent` - It can take an optional object with the properties '${OPTION_STYLES}' and '${OPTION_TEMPLATE}': + It can take an optional object with the properties '${OPTION_ANIMATIONS}', '${OPTION_STYLES}' and '${OPTION_TEMPLATE}': + * \`${OPTION_ANIMATIONS}\` - number > 0 defining the maximum allowed inline lines for animations. Defaults to ${DEFAULT_ANIMATIONS_LIMIT}. * \`${OPTION_STYLES}\` - number > 0 defining the maximum allowed inline lines for styles. Defaults to ${DEFAULT_STYLES_LIMIT}. * \`${OPTION_TEMPLATE}\` - number > 0 defining the maximum allowed inline lines for template. Defaults to ${DEFAULT_TEMPLATE_LIMIT}. `, @@ -60,14 +66,17 @@ export class Rule extends Rules.AbstractRule { } } -type PropertyType = 'styles' | 'template'; - +type PropertyType = 'animations' | 'styles' | 'template'; export type PropertyPair = { [key in PropertyType]?: number }; const generateFailure = (type: PropertyType, limit: number, value: number): string => { return sprintf(Rule.FAILURE_STRING, type, limit, value); }; +export const getAnimationsFailure = (value: number, limit = DEFAULT_ANIMATIONS_LIMIT): string => { + return generateFailure(OPTION_ANIMATIONS, limit, value); +}; + export const getStylesFailure = (value: number, limit = DEFAULT_STYLES_LIMIT): string => { return generateFailure(OPTION_STYLES, limit, value); }; @@ -77,6 +86,7 @@ export const getTemplateFailure = (value: number, limit = DEFAULT_TEMPLATE_LIMIT }; export class MaxInlineDeclarationsWalker extends NgWalker { + private readonly animationsLinesLimit = DEFAULT_ANIMATIONS_LIMIT; private readonly stylesLinesLimit = DEFAULT_STYLES_LIMIT; private readonly templateLinesLimit = DEFAULT_TEMPLATE_LIMIT; private readonly newLineRegExp = /\r\n|\r|\n/; @@ -84,15 +94,17 @@ export class MaxInlineDeclarationsWalker extends NgWalker { constructor(sourceFile: SourceFile, options: IOptions) { super(sourceFile, options); - const { styles = -1, template = -1 } = (options.ruleArguments[0] || []) as PropertyPair; + const { animations = -1, styles = -1, template = -1 } = (options.ruleArguments[0] || []) as PropertyPair; + this.animationsLinesLimit = animations > -1 ? animations : this.animationsLinesLimit; this.stylesLinesLimit = styles > -1 ? styles : this.stylesLinesLimit; this.templateLinesLimit = template > -1 ? template : this.templateLinesLimit; } protected visitNgComponent(metadata: ComponentMetadata): void { - this.validateInlineTemplate(metadata); + this.validateInlineAnimations(metadata); this.validateInlineStyles(metadata); + this.validateInlineTemplate(metadata); super.visitNgComponent(metadata); } @@ -100,6 +112,28 @@ export class MaxInlineDeclarationsWalker extends NgWalker { return source!.trim().split(this.newLineRegExp).length; } + private getInlineAnimationsLinesCount(metadata: ComponentMetadata): number { + return (metadata.animations || []).reduce((previousValue, currentValue) => { + previousValue += this.getLinesCount(currentValue.animation.source); + + return previousValue; + }, 0); + } + + private validateInlineAnimations(metadata: ComponentMetadata): void { + const linesCount = this.getInlineAnimationsLinesCount(metadata); + + if (linesCount <= this.animationsLinesLimit) { + return; + } + + const failureMessage = getAnimationsFailure(linesCount, this.animationsLinesLimit); + + for (const animation of metadata.animations) { + this.addFailureAtNode(animation.node!, failureMessage); + } + } + private getInlineStylesLinesCount(metadata: ComponentMetadata): number { return (metadata.styles || []).reduce((previousValue, currentValue) => { if (!currentValue.url) { @@ -123,6 +157,7 @@ export class MaxInlineDeclarationsWalker extends NgWalker { this.addFailureAtNode(style.node!, failureMessage); } } + private getTemplateLinesCount(metadata: ComponentMetadata): number { return this.hasInlineTemplate(metadata) ? this.getLinesCount(metadata.template.template.source) : 0; } diff --git a/src/util/ngQuery.ts b/src/util/ngQuery.ts index 06330bdc5..56aa6111b 100644 --- a/src/util/ngQuery.ts +++ b/src/util/ngQuery.ts @@ -2,6 +2,14 @@ import * as ts from 'typescript'; import { decoratorArgument, getInitializer, getStringInitializerFromProperty, isProperty, WithStringInitializer } from './astQuery'; import { Maybe } from './function'; +export function getAnimations(dec: ts.Decorator): Maybe { + return decoratorArgument(dec).bind(expr => { + const property = expr!.properties.find(p => isProperty('animations', p))!; + + return getInitializer(property).fmap(expr => (ts.isArrayLiteralExpression(expr!) ? (expr as ts.ArrayLiteralExpression) : undefined)); + }); +} + export function getInlineStyle(dec: ts.Decorator): Maybe { return decoratorArgument(dec).bind(expr => { const property = expr!.properties.find(p => isProperty('styles', p))!; diff --git a/test/maxInlineDeclarationsRule.spec.ts b/test/maxInlineDeclarationsRule.spec.ts index 8126a4bde..3321d6aa3 100644 --- a/test/maxInlineDeclarationsRule.spec.ts +++ b/test/maxInlineDeclarationsRule.spec.ts @@ -1,5 +1,5 @@ import { createSourceFile, ScriptTarget, SourceFile } from 'typescript/lib/typescript'; -import { getStylesFailure, getTemplateFailure, PropertyPair, Rule } from '../src/maxInlineDeclarationsRule'; +import { getAnimationsFailure, getStylesFailure, getTemplateFailure, PropertyPair, Rule } from '../src/maxInlineDeclarationsRule'; import { assertFailure, assertFailures, assertSuccess } from './testHelper'; type PropertyPairArray = ReadonlyArray; @@ -14,44 +14,65 @@ const getSourceFile = (code: string): SourceFile => { }; describe(ruleName, () => { - describe('template', () => { + describe('animations', () => { describe('failure', () => { it('should fail when the number of lines exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', - template: \` -
first line
-
second line
-
third line
-
fourth line
- \` + animations: [ + \` + transformPanel: trigger('transformPanel', + [ + state('void', style({opacity: 0, transform: 'scale(1, 0)'})), + state('enter', style({opacity: 1, transform: 'scale(1, 1)'})), + transition('void => enter', group([ + query('@fadeInCalendar', animateChild()), + animate('400ms cubic-bezier(0.25, 0.8, 0.25, 1)') + ])), + transition('* => void', animate('100ms linear', style({opacity: 0}))) + ] + ), + + fadeInCalendar: trigger('fadeInCalendar', [ + state('void', style({opacity: 0})), + state('enter', style({opacity: 1})), + transition('void => *', animate('400ms 100ms cubic-bezier(0.55, 0, 0.55, 0.2)')) + ] + \`; + ] }) class Test {} `; assertFailure(ruleName, source, { - endPosition: { character: 13, line: 8 }, - message: getTemplateFailure(4), - startPosition: { character: 22, line: 3 } + endPosition: { character: 15, line: 21 }, + message: getAnimationsFailure(17), + startPosition: { character: 14, line: 3 } }); }); it('should fail when the number of lines exceeds a custom lines limit', () => { const source = ` @Component({ - selector: 'foobar', - template: '
first line
' + animations: [ + \` + transformPanel: trigger('transformPanel', + [ + state('void', style({opacity: 0, transform: 'scale(1, 0)'})) + ] + ) + \` + ] }) class Test {} `; - const options: PropertyPairArray = [{ template: 0 }]; + const options: PropertyPairArray = [{ animations: 2 }]; assertFailure( ruleName, source, { - endPosition: { character: 45, line: 3 }, - message: getTemplateFailure(1, options[0].template), - startPosition: { character: 22, line: 3 } + endPosition: { character: 15, line: 9 }, + message: getAnimationsFailure(5, options[0].animations), + startPosition: { character: 14, line: 3 } }, options ); @@ -62,8 +83,7 @@ describe(ruleName, () => { it('should succeed when the number of lines does not exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', - template: '
just one line template
' + animations: ['state('void', style({opacity: 0, transform: 'scale(1, 0)'}))'] }) class Test {} `; @@ -73,36 +93,21 @@ describe(ruleName, () => { it('should succeed when a negative limit is used and the number of lines does not exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', - template: '
first line
' + animations: ['state('void', style({opacity: 0, transform: 'scale(1, 0)'}))'] }) class Test {} `; - const options: PropertyPairArray = [{ template: -5 }]; + const options: PropertyPairArray = [{ animations: -5 }]; assertSuccess(ruleName, source, options); }); }); }); - describe('templateUrl', () => { - it('should succeed when the number of lines of a template file exceeds the default lines limit', () => { - const source = ` - @Component({ - selector: 'foobar', - templateUrl: './foobar.html' - }) - class Test {} - `; - assertSuccess(ruleName, getSourceFile(source)); - }); - }); - describe('styles', () => { describe('failure', () => { it('should fail when the number of lines exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', styles: [ \` div { @@ -115,16 +120,15 @@ describe(ruleName, () => { class Test {} `; assertFailure(ruleName, source, { - endPosition: { character: 15, line: 9 }, + endPosition: { character: 15, line: 8 }, message: getStylesFailure(4), - startPosition: { character: 14, line: 4 } + startPosition: { character: 14, line: 3 } }); }); it('should fail when the sum of lines (from separate inline styles) exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', styles: [ \` div { @@ -145,14 +149,14 @@ describe(ruleName, () => { const message = getStylesFailure(8); assertFailures(ruleName, source, [ { - endPosition: { character: 15, line: 9 }, + endPosition: { character: 15, line: 8 }, message, - startPosition: { character: 14, line: 4 } + startPosition: { character: 14, line: 3 } }, { - endPosition: { character: 15, line: 15 }, + endPosition: { character: 15, line: 14 }, message, - startPosition: { character: 14, line: 10 } + startPosition: { character: 14, line: 9 } } ]); }); @@ -160,7 +164,6 @@ describe(ruleName, () => { it('should fail when the number of lines exceeds a custom lines limit', () => { const source = ` @Component({ - selector: 'foobar', styles: ['div { display: none; }'] }) class Test {} @@ -170,9 +173,9 @@ describe(ruleName, () => { ruleName, source, { - endPosition: { character: 45, line: 3 }, + endPosition: { character: 45, line: 2 }, message: getStylesFailure(1, options[0].styles), - startPosition: { character: 21, line: 3 } + startPosition: { character: 21, line: 2 } }, options ); @@ -183,7 +186,6 @@ describe(ruleName, () => { it('should succeed when the number of lines does not exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', styles: ['div { display: none; }'] }) class Test {} @@ -194,7 +196,6 @@ describe(ruleName, () => { it('should succeed when a negative limit is used and the number of lines does not exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', styles: ['div { display: none; }'] }) class Test {} @@ -209,7 +210,6 @@ describe(ruleName, () => { it('should succeed when the number of lines of a styles file exceeds the default lines limit', () => { const source = ` @Component({ - selector: 'foobar', styleUrls: ['./foobar.css'] }) class Test {} @@ -222,7 +222,6 @@ describe(ruleName, () => { it('should succeed when neither the styles nor the template are present', () => { const source = ` @Component({ - selector: 'foobar', styleUrls: ['./foobar.scss'], templateUrl: './foobar.html' }) @@ -231,4 +230,82 @@ describe(ruleName, () => { assertSuccess(ruleName, getSourceFile(source)); }); }); + + describe('template', () => { + describe('failure', () => { + it('should fail when the number of lines exceeds the default lines limit', () => { + const source = ` + @Component({ + template: \` +
first line
+
second line
+
third line
+
fourth line
+ \` + }) + class Test {} + `; + assertFailure(ruleName, source, { + endPosition: { character: 13, line: 7 }, + message: getTemplateFailure(4), + startPosition: { character: 22, line: 2 } + }); + }); + + it('should fail when the number of lines exceeds a custom lines limit', () => { + const source = ` + @Component({ + template: '
first line
' + }) + class Test {} + `; + const options: PropertyPairArray = [{ template: 0 }]; + assertFailure( + ruleName, + source, + { + endPosition: { character: 45, line: 2 }, + message: getTemplateFailure(1, options[0].template), + startPosition: { character: 22, line: 2 } + }, + options + ); + }); + }); + + describe('success', () => { + it('should succeed when the number of lines does not exceeds the default lines limit', () => { + const source = ` + @Component({ + template: '
just one line template
' + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed when a negative limit is used and the number of lines does not exceeds the default lines limit', () => { + const source = ` + @Component({ + template: '
first line
' + }) + class Test {} + `; + const options: PropertyPairArray = [{ template: -5 }]; + assertSuccess(ruleName, source, options); + }); + }); + }); + + describe('templateUrl', () => { + it('should succeed when the number of lines of a template file exceeds the default lines limit', () => { + const source = ` + @Component({ + templateUrl: './foobar.html' + }) + class Test {} + `; + assertSuccess(ruleName, getSourceFile(source)); + }); + }); });