Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: improve ngWalker by preventing an error when a class has no name #788

Merged
merged 3 commits into from Mar 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 24 additions & 11 deletions src/angular/metadata.ts
Expand Up @@ -25,22 +25,35 @@ export interface TemplateMetadata extends PropertyMetadata {
}

export class DirectiveMetadata {
constructor(
public readonly controller: ts.ClassDeclaration,
public readonly decorator: ts.Decorator,
public readonly selector?: string
) {}
constructor(readonly controller: ts.ClassDeclaration, readonly decorator: ts.Decorator, readonly selector?: string) {}
}

export class ComponentMetadata extends DirectiveMetadata {
constructor(
public readonly controller: ts.ClassDeclaration,
public readonly decorator: ts.Decorator,
public readonly selector?: string,
public readonly animations?: (AnimationMetadata | undefined)[],
public readonly styles?: (StyleMetadata | undefined)[],
public readonly template?: TemplateMetadata
readonly controller: ts.ClassDeclaration,
readonly decorator: ts.Decorator,
readonly selector?: string,
readonly animations?: (AnimationMetadata | undefined)[],
readonly styles?: (StyleMetadata | undefined)[],
readonly template?: TemplateMetadata
) {
super(controller, decorator, selector);
}
}

export class PipeMetadata {
constructor(
readonly controller: ts.ClassDeclaration,
readonly decorator: ts.Decorator,
readonly name?: string,
rafaelss95 marked this conversation as resolved.
Show resolved Hide resolved
readonly pure?: ts.BooleanLiteral
wKoza marked this conversation as resolved.
Show resolved Hide resolved
) {}
}

export class ModuleMetadata {
constructor(readonly controller: ts.ClassDeclaration, readonly decorator: ts.Decorator) {}
}

export class InjectableMetadata {
constructor(readonly controller: ts.ClassDeclaration, readonly decorator: ts.Decorator) {}
}
67 changes: 61 additions & 6 deletions src/angular/metadataReader.ts
Expand Up @@ -3,10 +3,20 @@ import { callExpression, decoratorArgument, hasProperties, withIdentifier } from
import { ifTrue, listToMaybe, Maybe, unwrapFirst } from '../util/function';
import { logger } from '../util/logger';
import { getAnimations, getInlineStyle, getTemplate } from '../util/ngQuery';
import { getDecoratorPropertyInitializer, isStringLiteralLike, maybeNodeArray } from '../util/utils';
import { getDecoratorPropertyInitializer, isBooleanLiteralLike, isStringLiteralLike, maybeNodeArray } from '../util/utils';
import { Config } from './config';
import { FileResolver } from './fileResolver/fileResolver';
import { AnimationMetadata, CodeWithSourceMap, ComponentMetadata, DirectiveMetadata, StyleMetadata, TemplateMetadata } from './metadata';
import {
AnimationMetadata,
CodeWithSourceMap,
ComponentMetadata,
DirectiveMetadata,
InjectableMetadata,
ModuleMetadata,
PipeMetadata,
StyleMetadata,
TemplateMetadata
} from './metadata';
import { AbstractResolver, MetadataUrls } from './urlResolvers/abstractResolver';
import { PathResolver } from './urlResolvers/pathResolver';
import { UrlResolver } from './urlResolvers/urlResolver';
Expand All @@ -26,9 +36,9 @@ export class MetadataReader {
this.urlResolver = this.urlResolver || new UrlResolver(new PathResolver());
}

read(d: ts.ClassDeclaration): DirectiveMetadata | undefined {
read(d: ts.ClassDeclaration): DirectiveMetadata | ComponentMetadata | PipeMetadata | ModuleMetadata | InjectableMetadata | undefined {
const componentMetadata = unwrapFirst<ComponentMetadata | undefined>(
maybeNodeArray(d.decorators!).map(dec => {
maybeNodeArray(ts.createNodeArray(d.decorators)).map(dec => {
return Maybe.lift(dec)
.bind(callExpression)
.bind(withIdentifier('Component') as any)
Expand All @@ -37,15 +47,42 @@ export class MetadataReader {
);

const directiveMetadata = unwrapFirst<DirectiveMetadata | undefined>(
maybeNodeArray(d.decorators!).map(dec =>
maybeNodeArray(ts.createNodeArray(d.decorators)).map(dec =>
Maybe.lift(dec)
.bind(callExpression)
.bind(withIdentifier('Directive') as any)
.fmap(() => this.readDirectiveMetadata(d, dec))
)
);

return directiveMetadata || componentMetadata || undefined;
const pipeMetadata = unwrapFirst<PipeMetadata | undefined>(
maybeNodeArray(ts.createNodeArray(d.decorators)).map(dec =>
Maybe.lift(dec)
.bind(callExpression)
.bind(withIdentifier('Pipe') as any)
.fmap(() => this.readPipeMetadata(d, dec))
)
);

const moduleMetadata = unwrapFirst<ModuleMetadata | undefined>(
maybeNodeArray(ts.createNodeArray(d.decorators)).map(dec =>
Maybe.lift(dec)
.bind(callExpression)
.bind(withIdentifier('NgModule') as any)
.fmap(() => this.readModuleMetadata(d, dec))
)
);

const injectableMetadata = unwrapFirst<InjectableMetadata | undefined>(
maybeNodeArray(ts.createNodeArray(d.decorators)).map(dec =>
Maybe.lift(dec)
.bind(callExpression)
.bind(withIdentifier('Injectable') as any)
.fmap(() => this.readInjectableMetadata(d, dec))
)
);

return directiveMetadata || componentMetadata || pipeMetadata || moduleMetadata || injectableMetadata;
}

protected readDirectiveMetadata(d: ts.ClassDeclaration, dec: ts.Decorator): DirectiveMetadata {
Expand All @@ -55,6 +92,24 @@ export class MetadataReader {
return new DirectiveMetadata(d, dec, selector);
}

protected readPipeMetadata(d: ts.ClassDeclaration, dec: ts.Decorator): DirectiveMetadata {
const nameExpression = getDecoratorPropertyInitializer(dec, 'name');
const name = nameExpression && isStringLiteralLike(nameExpression) ? nameExpression.text : undefined;

const pureExpression = getDecoratorPropertyInitializer(dec, 'pure');
const pure = pureExpression && isBooleanLiteralLike(pureExpression) ? pureExpression : undefined;

return new PipeMetadata(d, dec, name, pure);
}

protected readModuleMetadata(d: ts.ClassDeclaration, dec: ts.Decorator): DirectiveMetadata {
return new ModuleMetadata(d, dec);
}

protected readInjectableMetadata(d: ts.ClassDeclaration, dec: ts.Decorator): DirectiveMetadata {
return new InjectableMetadata(d, dec);
}

protected readComponentMetadata(d: ts.ClassDeclaration, dec: ts.Decorator): ComponentMetadata {
const expr = this.getDecoratorArgument(dec);
const directiveMetadata = this.readDirectiveMetadata(d, dec);
Expand Down
50 changes: 24 additions & 26 deletions src/angular/ngWalker.ts
Expand Up @@ -2,9 +2,9 @@ import * as compiler from '@angular/compiler';
import * as Lint from 'tslint';
import * as ts from 'typescript';
import { logger } from '../util/logger';
import { getDecoratorName, maybeNodeArray } from '../util/utils';
import { getClassName, getDecoratorName, maybeNodeArray } from '../util/utils';
import { Config } from './config';
import { ComponentMetadata, DirectiveMetadata, StyleMetadata } from './metadata';
import { ComponentMetadata, DirectiveMetadata, InjectableMetadata, ModuleMetadata, PipeMetadata, StyleMetadata } from './metadata';
import { MetadataReader } from './metadataReader';
import { ngWalkerFactoryUtils } from './ngWalkerFactoryUtils';
import { BasicCssAstVisitor, CssAstVisitorCtrl } from './styles/basicCssAstVisitor';
Expand Down Expand Up @@ -61,11 +61,19 @@ export class NgWalker extends Lint.RuleWalker {
}

visitClassDeclaration(declaration: ts.ClassDeclaration) {
const metadata = this._metadataReader!.read(declaration);
if (metadata instanceof ComponentMetadata) {
this.visitNgComponent(metadata);
} else if (metadata instanceof DirectiveMetadata) {
this.visitNgDirective(metadata);
if (this.hasClassName(declaration)) {
const metadata = this._metadataReader!.read(declaration);
if (metadata instanceof ComponentMetadata) {
this.visitNgComponent(metadata);
} else if (metadata instanceof DirectiveMetadata) {
this.visitNgDirective(metadata);
} else if (metadata instanceof PipeMetadata) {
this.visitNgPipe(metadata);
} else if (metadata instanceof ModuleMetadata) {
this.visitNgModule(metadata);
} else if (metadata instanceof InjectableMetadata) {
this.visitNgInjectable(metadata);
}
}
maybeNodeArray(ts.createNodeArray(declaration.decorators)).forEach(this.visitClassDecorator.bind(this));
super.visitClassDeclaration(declaration);
Expand Down Expand Up @@ -115,22 +123,6 @@ export class NgWalker extends Lint.RuleWalker {
}
}

protected visitClassDecorator(decorator: ts.Decorator) {
let name = getDecoratorName(decorator);

if (name === 'Injectable') {
this.visitNgInjectable(decorator.parent as ts.ClassDeclaration, decorator);
}

if (name === 'Pipe') {
this.visitNgPipe(decorator.parent as ts.ClassDeclaration, decorator);
}

if (name === 'NgModule') {
this.visitNgModule(decorator);
}
}

protected visitNgComponent(metadata: ComponentMetadata) {
const { styles = [] } = metadata;

Expand Down Expand Up @@ -165,13 +157,15 @@ export class NgWalker extends Lint.RuleWalker {
}
}

protected visitNgModule(decorator: ts.Decorator) {}
protected visitClassDecorator(decorator: ts.Decorator) {}

protected visitNgModule(metadata: ModuleMetadata) {}

protected visitNgDirective(metadata: DirectiveMetadata) {}

protected visitNgPipe(controller: ts.ClassDeclaration, decorator: ts.Decorator) {}
protected visitNgPipe(metadata: PipeMetadata) {}

protected visitNgInjectable(classDeclaration: ts.ClassDeclaration, decorator: ts.Decorator) {}
protected visitNgInjectable(metadata: InjectableMetadata) {}

protected visitNgInput(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) {}

Expand Down Expand Up @@ -237,4 +231,8 @@ export class NgWalker extends Lint.RuleWalker {

return sf;
}

private hasClassName(node: ts.Decorator | ts.ClassDeclaration) {
return (ts.isDecorator(node) && getClassName(node.parent)) || (ts.isClassDeclaration(node) && getClassName(node));
}
}
23 changes: 11 additions & 12 deletions src/contextualLifecycleRule.ts
Expand Up @@ -14,6 +14,7 @@ import {
MetadataTypeKeys,
MetadataTypes
} from './util/utils';
import { InjectableMetadata, PipeMetadata } from './angular';

interface FailureParameters {
readonly className: string;
Expand Down Expand Up @@ -45,26 +46,24 @@ export class Rule extends AbstractRule {
}

class ContextualLifecycleWalker extends NgWalker {
protected visitNgInjectable(controller: ClassDeclaration, decorator: Decorator): void {
this.validateDecorator(controller, decorator, METADATA_TYPE_LIFECYCLE_MAPPER.Injectable);
super.visitNgInjectable(controller, decorator);
protected visitNgInjectable(metadata: InjectableMetadata): void {
this.validateDecorator(metadata, METADATA_TYPE_LIFECYCLE_MAPPER.Injectable);
super.visitNgInjectable(metadata);
}

protected visitNgPipe(controller: ClassDeclaration, decorator: Decorator): void {
this.validateDecorator(controller, decorator, METADATA_TYPE_LIFECYCLE_MAPPER.Pipe);
super.visitNgPipe(controller, decorator);
protected visitNgPipe(metadata: PipeMetadata): void {
this.validateDecorator(metadata, METADATA_TYPE_LIFECYCLE_MAPPER.Pipe);
super.visitNgPipe(metadata);
}

private validateDecorator(controller: ClassDeclaration, decorator: Decorator, allowedMethods: ReadonlySet<LifecycleMethodKeys>): void {
const className = getClassName(controller);
private validateDecorator(metadata: PipeMetadata, allowedMethods: ReadonlySet<LifecycleMethodKeys>): void {
const className = getClassName(metadata.controller)!;

if (!className) return;
wKoza marked this conversation as resolved.
Show resolved Hide resolved

const metadataType = getDecoratorName(decorator);
const metadataType = getDecoratorName(metadata.decorator);

if (!metadataType || !isMetadataType(metadataType)) return;

for (const member of controller.members) {
for (const member of metadata.controller.members) {
const { name: memberName } = member;

if (!memberName) continue;
Expand Down
2 changes: 1 addition & 1 deletion src/noOutputRenameRule.ts
Expand Up @@ -29,7 +29,7 @@ export const getFailureMessage = (): string => {
export class OutputMetadataWalker extends NgWalker {
private directiveSelectors!: ReadonlySet<DirectiveMetadata['selector']>;

visitNgDirective(metadata: DirectiveMetadata): void {
protected visitNgDirective(metadata: DirectiveMetadata): void {
this.directiveSelectors = new Set((metadata.selector || '').replace(/[\[\]\s]/g, '').split(','));
super.visitNgDirective(metadata);
}
Expand Down
24 changes: 8 additions & 16 deletions src/noPipeImpureRule.ts
Expand Up @@ -4,6 +4,7 @@ import { AbstractRule } from 'tslint/lib/rules';
import { ClassDeclaration, Decorator, SourceFile, SyntaxKind } from 'typescript';
import { NgWalker } from './angular/ngWalker';
import { getClassName, getDecoratorPropertyInitializer } from './util/utils';
import { PipeMetadata } from './angular';

interface FailureParameters {
readonly className: string;
Expand Down Expand Up @@ -31,27 +32,18 @@ export class Rule extends AbstractRule {
}

export class ClassMetadataWalker extends NgWalker {
protected visitNgPipe(controller: ClassDeclaration, decorator: Decorator): void {
this.validatePipe(controller, decorator);
super.visitNgPipe(controller, decorator);
protected visitNgPipe(metadata: PipeMetadata): void {
this.validatePipe(metadata);
super.visitNgPipe(metadata);
}

private validatePipe(controller: ClassDeclaration, decorator: Decorator): void {
const pureExpression = getDecoratorPropertyInitializer(decorator, 'pure');
private validatePipe(metadata: PipeMetadata): void {
if (!metadata.pure || metadata.pure.kind !== SyntaxKind.FalseKeyword) return;

if (!pureExpression) return;

const { parent: parentExpression } = pureExpression;
const isNotFalseLiteral = pureExpression.kind !== SyntaxKind.FalseKeyword;

if (!parentExpression || isNotFalseLiteral) return;

const className = getClassName(controller);

if (!className) return;
wKoza marked this conversation as resolved.
Show resolved Hide resolved
const className = getClassName(metadata.controller)!;

const failure = getFailureMessage({ className });

this.addFailureAtNode(parentExpression, failure);
this.addFailureAtNode(metadata.pure, failure);
}
}
9 changes: 5 additions & 4 deletions src/pipePrefixRule.ts
Expand Up @@ -4,6 +4,7 @@ import * as ts from 'typescript';
import { NgWalker } from './angular/ngWalker';
import { SelectorValidator } from './util/selectorValidator';
import { getDecoratorArgument } from './util/utils';
import { PipeMetadata } from './angular';

export class Rule extends Lint.Rules.AbstractRule {
static readonly metadata: Lint.IRuleMetadata = {
Expand Down Expand Up @@ -69,10 +70,10 @@ export class ClassMetadataWalker extends NgWalker {
super(sourceFile, rule.getOptions());
}

protected visitNgPipe(controller: ts.ClassDeclaration, decorator: ts.Decorator) {
let className = controller.name!.text;
this.validateProperties(className, decorator);
super.visitNgPipe(controller, decorator);
protected visitNgPipe(metadata: PipeMetadata) {
let className = metadata.controller.name!.text;
this.validateProperties(className, metadata.decorator);
super.visitNgPipe(metadata);
}

private validateProperties(className: string, pipe: ts.Decorator) {
Expand Down
4 changes: 4 additions & 0 deletions src/util/utils.ts
Expand Up @@ -19,6 +19,7 @@ import {
ObjectLiteralExpression,
SourceFile,
StringLiteral,
BooleanLiteral,
SyntaxKind
} from 'typescript';
import { getDeclaredMethods } from './classDeclarationUtils';
Expand Down Expand Up @@ -202,6 +203,9 @@ export const isSameLine = (sourceFile: SourceFile, pos1: number, pos2: number):
export const isStringLiteralLike = (node: Node): node is StringLiteral | NoSubstitutionTemplateLiteral =>
isStringLiteral(node) || isNoSubstitutionTemplateLiteral(node);

export const isBooleanLiteralLike = (node: Node): node is BooleanLiteral =>
node.kind === SyntaxKind.FalseKeyword || node.kind === SyntaxKind.TrueKeyword;

export const maybeNodeArray = <T extends Node>(nodes: NodeArray<T>): ReadonlyArray<T> => nodes || [];

// Regex below matches any Unicode word and compatible with ES5. In ES2018 the same result
Expand Down