From d922dcb5c7fdb6bb99efb8b6cb6000dfc0c12f1c Mon Sep 17 00:00:00 2001 From: Minko Gechev Date: Sun, 24 Jun 2018 10:18:56 +1000 Subject: [PATCH] fix: regressions in 4.4.0 (#671) * fix: regressions in 4.4.0 - Downgrade to TypeScript 2.7 which is the officially supported on by `@angular/core` - Export `preferInlineDecoratorRule` We should introduce a public API guard to make sure we don't hit regressions like the second one. Fix #669 #670 * refactor: address comments * fix: regression in selector type --- package-lock.json | 76 +++++++++----------- package.json | 4 +- src/angular/metadataReader.ts | 16 ++--- src/angular/urlResolvers/abstractResolver.ts | 10 +-- src/index.ts | 1 + src/preferInlineDecoratorRule.ts | 5 +- src/selectorNameBase.ts | 4 +- src/util/astQuery.ts | 8 +-- src/util/ngQuery.ts | 4 +- src/util/utils.ts | 12 ++-- test/componentSelectorRule.spec.ts | 1 + 11 files changed, 69 insertions(+), 72 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9be44e433..2bf5a95ce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "codelyzer", - "version": "4.3.0", + "version": "4.4.0", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -1013,6 +1013,12 @@ "integrity": "sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o=", "dev": true }, + "diff": { + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-3.5.0.tgz", + "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA==", + "dev": true + }, "domexception": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/domexception/-/domexception-1.0.1.tgz", @@ -1573,6 +1579,12 @@ "ansi-regex": "^2.0.0" } }, + "has-flag": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", + "integrity": "sha1-tdRU3CGZriJWmfNGfloH87lVuv0=", + "dev": true + }, "has-unicode": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/has-unicode/-/has-unicode-2.0.1.tgz", @@ -4559,9 +4571,9 @@ "dev": true }, "resolve": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/resolve/-/resolve-1.6.0.tgz", - "integrity": "sha512-mw7JQNu5ExIkcw4LPih0owX/TZXjD/ZUF/ZQ/pDnkw3ZKhDcZZw5klmBlj6gVMwjQ3Pz5Jgu7F3d0jcDVuEWdw==", + "version": "1.8.1", + "resolved": "https://registry.npmjs.org/resolve/-/resolve-1.8.1.tgz", + "integrity": "sha512-AicPrAC7Qu1JxPCZ9ZgCZlY35QgFnNqc+0LtbRNxnVw4TXvjQ72wnuL9JQcEBgXkI9JM8MsT9kaQoHcpCRJOYA==", "dev": true, "requires": { "path-parse": "^1.0.5" @@ -5092,6 +5104,15 @@ "get-stdin": "^4.0.1" } }, + "supports-color": { + "version": "5.4.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.4.0.tgz", + "integrity": "sha512-zjaXglF5nnWpsq470jSv6P9DwPvgLkuapYmfDm3JWOm0vkNTVF2tI4UrN2r6jH1qM/uc/WtxYY1hYoA2dOKj5w==", + "dev": true, + "requires": { + "has-flag": "^3.0.0" + } + }, "symbol-observable": { "version": "0.2.4", "resolved": "https://registry.npmjs.org/symbol-observable/-/symbol-observable-0.2.4.tgz", @@ -5306,9 +5327,9 @@ "dev": true }, "tslint": { - "version": "5.10.0", - "resolved": "https://registry.npmjs.org/tslint/-/tslint-5.10.0.tgz", - "integrity": "sha1-EeJrzLiK+gLdDZlWyuPUVAtfVMM=", + "version": "5.9.1", + "resolved": "https://registry.npmjs.org/tslint/-/tslint-5.9.1.tgz", + "integrity": "sha1-ElX4ej/1frCw4fDmEKi0dIBGya4=", "dev": true, "requires": { "babel-code-frame": "^6.22.0", @@ -5351,12 +5372,6 @@ "integrity": "sha512-VlfT9F3V0v+jr4yxPc5gg9s62/fIVWsd2Bk2iD435um1NlGMYdVCq+MjcXnhYq2icNOizHr1kK+5TI6H0Hy0ag==", "dev": true }, - "diff": { - "version": "3.5.0", - "resolved": "https://registry.npmjs.org/diff/-/diff-3.5.0.tgz", - "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA==", - "dev": true - }, "glob": { "version": "7.1.2", "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.2.tgz", @@ -5370,39 +5385,16 @@ "once": "^1.3.0", "path-is-absolute": "^1.0.0" } - }, - "has-flag": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", - "integrity": "sha1-tdRU3CGZriJWmfNGfloH87lVuv0=", - "dev": true - }, - "supports-color": { - "version": "5.4.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.4.0.tgz", - "integrity": "sha512-zjaXglF5nnWpsq470jSv6P9DwPvgLkuapYmfDm3JWOm0vkNTVF2tI4UrN2r6jH1qM/uc/WtxYY1hYoA2dOKj5w==", - "dev": true, - "requires": { - "has-flag": "^3.0.0" - } } } }, "tsutils": { - "version": "2.24.0", - "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-2.24.0.tgz", - "integrity": "sha512-rOIkvoe17acR3r96IPnqwa1+Z7zx9AroEtEKl20IeExXtoWptqG/zb806cYOvdbQGcxh1eOaZQNruOQ716Edig==", + "version": "2.27.1", + "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-2.27.1.tgz", + "integrity": "sha512-AE/7uzp32MmaHvNNFES85hhUDHFdFZp6OAiZcd6y4ZKKIg6orJTm8keYWBhIhrJQH3a4LzNKat7ZPXZt5aTf6w==", "dev": true, "requires": { "tslib": "^1.8.1" - }, - "dependencies": { - "tslib": { - "version": "1.9.0", - "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.9.0.tgz", - "integrity": "sha512-f/qGG2tUkrISBlQZEjEqoZ3B2+npJjIf04H1wuAv9iA8i04Icp+61KRXxFdha22670NJopsZCIjhC3SnjPRKrQ==", - "dev": true - } } }, "tunnel-agent": { @@ -5434,9 +5426,9 @@ "dev": true }, "typescript": { - "version": "2.8.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-2.8.3.tgz", - "integrity": "sha512-K7g15Bb6Ra4lKf7Iq2l/I5/En+hLIHmxWZGq3D4DIRNFxMNV6j2SHSvDOqs2tGd4UvD/fJvrwopzQXjLrT7Itw==", + "version": "2.7.2", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-2.7.2.tgz", + "integrity": "sha512-p5TCYZDAO0m4G344hD+wx/LATebLWZNkkh2asWUFqSsD2OrDNhbAHuSjobrmsUmdzjJjEeZVU9g1h3O6vpstnw==", "dev": true }, "union-value": { diff --git a/package.json b/package.json index dc05a19d1..03030e775 100644 --- a/package.json +++ b/package.json @@ -85,8 +85,8 @@ "rxjs": "^6.0.0", "rxjs-compat": "^6.1.0", "ts-node": "^6.0.2", - "tslint": "^5.10.0", - "typescript": "^2.8.0", + "tslint": "~5.9.1", + "typescript": "~2.7.0", "zone.js": "^0.8.26" }, "peerDependencies": { diff --git a/src/angular/metadataReader.ts b/src/angular/metadataReader.ts index ca53265ae..dbf2a2cbc 100644 --- a/src/angular/metadataReader.ts +++ b/src/angular/metadataReader.ts @@ -3,7 +3,7 @@ import { callExpression, decoratorArgument, getStringInitializerFromProperty, ha import { ifTrue, listToMaybe, Maybe, unwrapFirst } from '../util/function'; import { logger } from '../util/logger'; import { getAnimations, getInlineStyle, getTemplate } from '../util/ngQuery'; -import { isSimpleTemplateString, maybeNodeArray } from '../util/utils'; +import { isStringLiteralLike, maybeNodeArray } from '../util/utils'; import { Config } from './config'; import { FileResolver } from './fileResolver/fileResolver'; import { AnimationMetadata, CodeWithSourceMap, ComponentMetadata, DirectiveMetadata, StyleMetadata, TemplateMetadata } from './metadata'; @@ -80,12 +80,10 @@ export class MetadataReader { protected readComponentAnimationsMetadata(dec: ts.Decorator): Maybe<(AnimationMetadata | undefined)[] | undefined> { return getAnimations(dec).fmap(inlineAnimations => - inlineAnimations!.elements - .filter(inlineAnimation => isSimpleTemplateString(inlineAnimation)) - .map(inlineAnimation => ({ - animation: normalizeTransformed({ code: (inlineAnimation as ts.StringLiteralLike).text }), - node: inlineAnimation as ts.Node - })) + inlineAnimations!.elements.filter(isStringLiteralLike).map(inlineAnimation => ({ + animation: normalizeTransformed({ code: (inlineAnimation as ts.StringLiteral).text }), + node: inlineAnimation as ts.Node + })) ); } @@ -113,9 +111,9 @@ export class MetadataReader { return getInlineStyle(dec) .fmap(inlineStyles => // Resolve Inline styles - inlineStyles!.elements.filter(inlineStyle => isSimpleTemplateString(inlineStyle)).map(inlineStyle => ({ + inlineStyles!.elements.filter(isStringLiteralLike).map(inlineStyle => ({ node: inlineStyle, - style: normalizeTransformed(Config.transformStyle((inlineStyle as ts.StringLiteralLike).text)) + style: normalizeTransformed(Config.transformStyle((inlineStyle as ts.StringLiteral).text)) })) ) .catch(() => diff --git a/src/angular/urlResolvers/abstractResolver.ts b/src/angular/urlResolvers/abstractResolver.ts index c0e37ec02..277709858 100644 --- a/src/angular/urlResolvers/abstractResolver.ts +++ b/src/angular/urlResolvers/abstractResolver.ts @@ -1,5 +1,5 @@ import * as ts from 'typescript'; -import { getDecoratorArgument, isSimpleTemplateString } from '../../util/utils'; +import { getDecoratorArgument, isStringLiteralLike } from '../../util/utils'; export interface MetadataUrls { templateUrl: string; @@ -17,12 +17,12 @@ export abstract class AbstractResolver { } const prop = arg.properties.find( - p => (p.name as ts.StringLiteralLike).text === 'templateUrl' && isSimpleTemplateString((p as ts.PropertyAssignment).initializer) + p => (p.name as ts.StringLiteral).text === 'templateUrl' && isStringLiteralLike((p as ts.PropertyAssignment).initializer) ); // We know that it's has an initializer because it's either // a template string or a string literal. - return prop ? ((prop as ts.PropertyAssignment).initializer as ts.StringLiteralLike).text : undefined; + return prop ? ((prop as ts.PropertyAssignment).initializer as ts.StringLiteral).text : undefined; } protected getStyleUrls(decorator: ts.Decorator): string[] { @@ -38,8 +38,8 @@ export abstract class AbstractResolver { if (prop) { return ((prop as ts.PropertyAssignment).initializer as ts.ArrayLiteralExpression).elements - .filter(isSimpleTemplateString) - .map(e => (e as ts.StringLiteralLike).text); + .filter(isStringLiteralLike) + .map(e => (e as ts.StringLiteral).text); } return []; diff --git a/src/index.ts b/src/index.ts index e4b31362e..d050a2a28 100644 --- a/src/index.ts +++ b/src/index.ts @@ -24,6 +24,7 @@ export { Rule as NoTemplateCallExpressionRule } from './noTemplateCallExpression export { Rule as NoUnusedCssRule } from './noUnusedCssRule'; export { Rule as PipeImpureRule } from './pipeImpureRule'; export { Rule as PipeNamingRule } from './pipeNamingRule'; +export { Rule as PreferInlineDecorator } from './preferInlineDecoratorRule'; export { Rule as PreferOutputReadonlyRule } from './preferOutputReadonlyRule'; export { Rule as TemplateConditionalComplexityRule } from './templateConditionalComplexityRule'; export { Rule as TemplateCyclomaticComplexityRule } from './templateCyclomaticComplexityRule'; diff --git a/src/preferInlineDecoratorRule.ts b/src/preferInlineDecoratorRule.ts index 3e0172b7b..6bca66db6 100644 --- a/src/preferInlineDecoratorRule.ts +++ b/src/preferInlineDecoratorRule.ts @@ -1,8 +1,9 @@ +import * as ts from 'typescript'; + import { IOptions, IRuleMetadata, Replacement, RuleFailure, Rules } from 'tslint/lib'; -import { isSameLine } from 'tsutils'; import { Decorator, Node, PropertyAccessExpression, SourceFile } from 'typescript'; import { NgWalker } from './angular/ngWalker'; -import { getDecoratorName } from './util/utils'; +import { getDecoratorName, isSameLine } from './util/utils'; enum Decorators { ContentChild = 'ContentChild', diff --git a/src/selectorNameBase.ts b/src/selectorNameBase.ts index 3e0a8b2f5..eec3fb828 100644 --- a/src/selectorNameBase.ts +++ b/src/selectorNameBase.ts @@ -3,7 +3,7 @@ import { sprintf } from 'sprintf-js'; import * as Lint from 'tslint'; import * as ts from 'typescript'; import { SelectorValidator } from './util/selectorValidator'; -import { getDecoratorArgument, getDecoratorName } from './util/utils'; +import { getDecoratorArgument, getDecoratorName, isStringLiteralLike } from './util/utils'; export type SelectorType = 'element' | 'attribute'; export type SelectorTypeInternal = 'element' | 'attrs'; @@ -148,7 +148,7 @@ export class SelectorValidatorWalker extends Lint.RuleWalker { } private validateProperty(p: ts.PropertyAssignment): boolean { - return ts.isStringLiteralLike(p.initializer) && ts.isIdentifier(p.name) && p.name.text === 'selector'; + return isStringLiteralLike(p.initializer) && ts.isIdentifier(p.name) && p.name.text === 'selector'; } private extractMainSelector(expression: ts.StringLiteral): compiler.CssSelector[] { diff --git a/src/util/astQuery.ts b/src/util/astQuery.ts index d48418203..e3891c0e2 100644 --- a/src/util/astQuery.ts +++ b/src/util/astQuery.ts @@ -1,6 +1,6 @@ import * as ts from 'typescript'; import { Maybe, ifTrue } from './function'; -import { isSimpleTemplateString } from './utils'; +import { isStringLiteralLike } from './utils'; export function callExpression(dec?: ts.Decorator): Maybe { return Maybe.lift(dec!.expression).fmap(expr => (expr && ts.isCallExpression(expr) ? expr : undefined)); @@ -29,13 +29,13 @@ export function getInitializer(p: ts.ObjectLiteralElement): Maybe -): Maybe { +): Maybe { const property = ps.find(p => isProperty(propertyName, p))!; return ( getInitializer(property) - // A little wrinkle to return Maybe - .fmap(expr => (expr && isSimpleTemplateString(expr) ? (expr as ts.StringLiteralLike) : undefined)) + // A little wrinkle to return Maybe + .fmap(expr => (expr && isStringLiteralLike(expr) ? (expr as ts.StringLiteral) : undefined)) ); } diff --git a/src/util/ngQuery.ts b/src/util/ngQuery.ts index 14d9d78ac..d4a3f177e 100644 --- a/src/util/ngQuery.ts +++ b/src/util/ngQuery.ts @@ -18,10 +18,10 @@ export function getInlineStyle(dec: ts.Decorator): Maybe { +export function getTemplate(dec: ts.Decorator): Maybe { return decoratorArgument(dec).bind(expr => getStringInitializerFromProperty('template', expr!.properties)); } -export function getTemplateUrl(dec: ts.Decorator): Maybe { +export function getTemplateUrl(dec: ts.Decorator): Maybe { return decoratorArgument(dec).bind(expr => getStringInitializerFromProperty('templateUrl', expr!.properties)); } diff --git a/src/util/utils.ts b/src/util/utils.ts index 8d47bdf6d..09727c6a7 100644 --- a/src/util/utils.ts +++ b/src/util/utils.ts @@ -1,9 +1,5 @@ import * as ts from 'typescript'; -export const isSimpleTemplateString = (e: any): e is ts.SyntaxKind.FirstTemplateToken | ts.StringLiteralLike => { - return ts.isStringLiteralLike(e) || e.kind === ts.SyntaxKind.FirstTemplateToken; -}; - export const getClassName = (property: ts.PropertyDeclaration): string | undefined => { const { parent } = property; const identifier = parent && ts.isClassDeclaration(parent) ? parent.name : undefined; @@ -61,3 +57,11 @@ export const getSymbolName = (expression: ts.ExpressionWithTypeArguments): strin export const maybeNodeArray = (nodes: ts.NodeArray): ReadonlyArray => { return nodes || []; }; + +export const isSameLine = (sourceFile: ts.SourceFile, pos1: number, pos2: number) => { + return ts.getLineAndCharacterOfPosition(sourceFile, pos1).line === ts.getLineAndCharacterOfPosition(sourceFile, pos2).line; +}; + +export const isStringLiteralLike = (node: ts.Node) => { + return node.kind === ts.SyntaxKind.StringLiteral || node.kind === ts.SyntaxKind.NoSubstitutionTemplateLiteral; +}; diff --git a/test/componentSelectorRule.spec.ts b/test/componentSelectorRule.spec.ts index 9f7c74e6f..158950d0f 100644 --- a/test/componentSelectorRule.spec.ts +++ b/test/componentSelectorRule.spec.ts @@ -144,6 +144,7 @@ describe('component-selector-prefix', () => { }); }); }); + describe('component-selector-type', () => { describe('invalid component selectors', () => { it('should fail when component used as attribute', () => {