Skip to content

Commit

Permalink
fix: regressions in 4.4.0 (#671)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mgechev committed Jun 24, 2018
1 parent a3f8679 commit d922dcb
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 72 deletions.
76 changes: 34 additions & 42 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -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": {
Expand Down
16 changes: 7 additions & 9 deletions src/angular/metadataReader.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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<AnimationMetadata>(inlineAnimation => ({
animation: normalizeTransformed({ code: (inlineAnimation as ts.StringLiteralLike).text }),
node: inlineAnimation as ts.Node
}))
inlineAnimations!.elements.filter(isStringLiteralLike).map<AnimationMetadata>(inlineAnimation => ({
animation: normalizeTransformed({ code: (inlineAnimation as ts.StringLiteral).text }),
node: inlineAnimation as ts.Node
}))
);
}

Expand Down Expand Up @@ -113,9 +111,9 @@ export class MetadataReader {
return getInlineStyle(dec)
.fmap(inlineStyles =>
// Resolve Inline styles
inlineStyles!.elements.filter(inlineStyle => isSimpleTemplateString(inlineStyle)).map<StyleMetadata>(inlineStyle => ({
inlineStyles!.elements.filter(isStringLiteralLike).map<StyleMetadata>(inlineStyle => ({
node: inlineStyle,
style: normalizeTransformed(Config.transformStyle((inlineStyle as ts.StringLiteralLike).text))
style: normalizeTransformed(Config.transformStyle((inlineStyle as ts.StringLiteral).text))
}))
)
.catch(() =>
Expand Down
10 changes: 5 additions & 5 deletions 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;
Expand All @@ -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[] {
Expand All @@ -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 [];
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Expand Up @@ -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';
Expand Down
5 changes: 3 additions & 2 deletions 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',
Expand Down
4 changes: 2 additions & 2 deletions src/selectorNameBase.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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[] {
Expand Down
8 changes: 4 additions & 4 deletions 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<ts.CallExpression | undefined> {
return Maybe.lift(dec!.expression).fmap(expr => (expr && ts.isCallExpression(expr) ? expr : undefined));
Expand Down Expand Up @@ -29,13 +29,13 @@ export function getInitializer(p: ts.ObjectLiteralElement): Maybe<ts.Expression
export function getStringInitializerFromProperty(
propertyName: string,
ps: ts.NodeArray<ts.ObjectLiteralElement>
): Maybe<ts.StringLiteralLike | undefined> {
): Maybe<ts.StringLiteral | undefined> {
const property = ps.find(p => isProperty(propertyName, p))!;

return (
getInitializer(property)
// A little wrinkle to return Maybe<ts.StringLiteralLike>
.fmap(expr => (expr && isSimpleTemplateString(expr) ? (expr as ts.StringLiteralLike) : undefined))
// A little wrinkle to return Maybe<ts.StringLiteral>
.fmap(expr => (expr && isStringLiteralLike(expr) ? (expr as ts.StringLiteral) : undefined))
);
}

Expand Down
4 changes: 2 additions & 2 deletions src/util/ngQuery.ts
Expand Up @@ -18,10 +18,10 @@ export function getInlineStyle(dec: ts.Decorator): Maybe<ts.ArrayLiteralExpressi
});
}

export function getTemplate(dec: ts.Decorator): Maybe<ts.StringLiteralLike | undefined> {
export function getTemplate(dec: ts.Decorator): Maybe<ts.StringLiteral | undefined> {
return decoratorArgument(dec).bind(expr => getStringInitializerFromProperty('template', expr!.properties));
}

export function getTemplateUrl(dec: ts.Decorator): Maybe<ts.StringLiteralLike | undefined> {
export function getTemplateUrl(dec: ts.Decorator): Maybe<ts.StringLiteral | undefined> {
return decoratorArgument(dec).bind(expr => getStringInitializerFromProperty('templateUrl', expr!.properties));
}
12 changes: 8 additions & 4 deletions 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;
Expand Down Expand Up @@ -61,3 +57,11 @@ export const getSymbolName = (expression: ts.ExpressionWithTypeArguments): strin
export const maybeNodeArray = <T extends ts.Node>(nodes: ts.NodeArray<T>): ReadonlyArray<T> => {
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;
};
1 change: 1 addition & 0 deletions test/componentSelectorRule.spec.ts
Expand Up @@ -144,6 +144,7 @@ describe('component-selector-prefix', () => {
});
});
});

describe('component-selector-type', () => {
describe('invalid component selectors', () => {
it('should fail when component used as attribute', () => {
Expand Down

0 comments on commit d922dcb

Please sign in to comment.