Skip to content

Commit

Permalink
Merge pull request #406 from mgechev/minko/issue-399
Browse files Browse the repository at this point in the history 
fix: error accessing invalid property
  • Loading branch information
@mgechev
mgechev authored Sep 8, 2017
2 parents bef790b + 57b11d7 commit 5c460be
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 90 deletions.
53 changes: 30 additions & 23 deletions src/pipeNamingRule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export class Rule extends Lint.Rules.AbstractRule {
options: {
'type': 'array',
'items': [
{'enum': ['kebab-case', 'attribute']},
{'type': 'string'}
{ 'enum': ['kebab-case', 'attribute'] },
{ 'type': 'string' }
],
'minItems': 1
},
Expand All @@ -32,11 +32,11 @@ export class Rule extends Lint.Rules.AbstractRule {
};


static FAILURE_WITHOUT_PREFIX: string = 'The name of the Pipe decorator of class %s should' +
' be named camelCase, however its value is "%s".';
static FAILURE_WITHOUT_PREFIX: string = 'The name of the Pipe decorator of class %s should' +
' be named camelCase, however its value is "%s".';

static FAILURE_WITH_PREFIX: string = 'The name of the Pipe decorator of class %s should' +
' be named camelCase with prefix %s, however its value is "%s".';
static FAILURE_WITH_PREFIX: string = 'The name of the Pipe decorator of class %s should' +
' be named camelCase with prefix %s, however its value is "%s".';

public prefix: string;
public hasPrefix: boolean;
Expand Down Expand Up @@ -86,30 +86,37 @@ export class ClassMetadataWalker extends NgWalker {
this.validateProperties(className, decorator);
}

private validateProperties(className: string, pipe: any) {
private validateProperties(className: string, pipe: ts.Decorator) {
let argument = this.extractArgument(pipe);
if (argument.kind === SyntaxKind.current().ObjectLiteralExpression) {
argument.properties.filter(n => n.name.text === 'name')
if (argument && argument.kind === SyntaxKind.current().ObjectLiteralExpression) {
(<ts.ObjectLiteralExpression>argument).properties
.filter(n => n.name && (<ts.StringLiteral>n.name).text === 'name')
.forEach(this.validateProperty.bind(this, className));
}
}

private extractArgument(pipe: any) {
let baseExpr = <any>pipe.expression || {};
let args = baseExpr.arguments || [];
return args[0];
private extractArgument(pipe: ts.Decorator): ts.Expression | undefined {
const baseExpr = <ts.CallExpression>pipe.expression;
if (baseExpr.arguments) {
const args = baseExpr.arguments;
return args[0];
}
return undefined;
}

private validateProperty(className: string, property: any) {
let propName: string = property.initializer.text;
let isValidName: boolean = this.rule.validateName(propName);
let isValidPrefix: boolean = (this.rule.hasPrefix ? this.rule.validatePrefix(propName) : true);
if (!isValidName || !isValidPrefix) {
this.addFailure(
this.createFailure(
property.getStart(),
property.getWidth(),
sprintf.apply(this, this.createFailureArray(className, propName))));
private validateProperty(className: string, property: ts.Node) {
const init = (<ts.PropertyAssignment>property).initializer;
if (init && (<ts.StringLiteral>init).text) {
let propName: string = (<ts.StringLiteral>init).text;
let isValidName: boolean = this.rule.validateName(propName);
let isValidPrefix: boolean = (this.rule.hasPrefix ? this.rule.validatePrefix(propName) : true);
if (!isValidName || !isValidPrefix) {
this.addFailure(
this.createFailure(
property.getStart(),
property.getWidth(),
sprintf.apply(this, this.createFailureArray(className, propName))));
}
}
}

Expand Down
155 changes: 88 additions & 67 deletions test/pipeNamingRule.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,122 +1,143 @@
import { assertSuccess, assertAnnotated } from './testHelper';

describe('pipe-naming', () => {
describe('invalid pipe name', () => {
it('should fail when Pipe is named camelCase without prefix ng', () => {
let source = `
describe('invalid pipe name', () => {
it('should fail when Pipe is named camelCase without prefix ng', () => {
let source = `
@Pipe({
name: 'foo-bar'
~~~~~~~~~~~~~~~
})
class Test {}`;
assertAnnotated({
ruleName: 'pipe-naming',
message: 'The name of the Pipe decorator of class Test should be named ' +
'camelCase with prefix ng, however its value is "foo-bar".',
source,
options: ['camelCase', 'ng']
});
});
assertAnnotated({
ruleName: 'pipe-naming',
message:
'The name of the Pipe decorator of class Test should be named ' +
'camelCase with prefix ng, however its value is "foo-bar".',
source,
options: ['camelCase', 'ng']
});
});

it('should fail when Pipe is named camelCase without prefix applying multiple prefixes', () => {
let source = `
it('should fail when Pipe is named camelCase without prefix applying multiple prefixes', () => {
let source = `
@Pipe({
name: 'foo-bar'
~~~~~~~~~~~~~~~
})
class Test {}`;
assertAnnotated({
ruleName: 'pipe-naming',
message: 'The name of the Pipe decorator of class Test should be named camelCase' +
' with prefix ng,mg,sg, however its value is "foo-bar".',
source,
options: ['camelCase', 'ng', 'mg', 'sg']
});
});
assertAnnotated({
ruleName: 'pipe-naming',
message:
'The name of the Pipe decorator of class Test should be named camelCase' +
' with prefix ng,mg,sg, however its value is "foo-bar".',
source,
options: ['camelCase', 'ng', 'mg', 'sg']
});
});

it('should fail when Pipe is named camelCase and has longer prefix', () => {
let source = `
it('should fail when Pipe is named camelCase and has longer prefix', () => {
let source = `
@Pipe({
name: 'fooBar'
~~~~~~~~~~~~~~
})
class Test {}`;
assertAnnotated({
ruleName: 'pipe-naming',
message: 'The name of the Pipe decorator of class Test should be named camelCase ' +
'with prefix fo,mg,sg, however its value is "fooBar".',
source,
options: ['camelCase', 'fo', 'mg', 'sg']
});
});
assertAnnotated({
ruleName: 'pipe-naming',
message:
'The name of the Pipe decorator of class Test should be named camelCase ' +
'with prefix fo,mg,sg, however its value is "fooBar".',
source,
options: ['camelCase', 'fo', 'mg', 'sg']
});
});

it('should fail when Pipe is not named camelCase without prefix', () => {
let source = `
it('should fail when Pipe is not named camelCase without prefix', () => {
let source = `
@Pipe({
name: 'foo-bar'
~~~~~~~~~~~~~~~
})
class Test {}`;
assertAnnotated({
ruleName: 'pipe-naming',
message: 'The name of the Pipe decorator of class Test should be named camelCase,' +
' however its value is "foo-bar".',
source,
options: 'camelCase'
});
});
assertAnnotated({
ruleName: 'pipe-naming',
message:
'The name of the Pipe decorator of class Test should be named camelCase,' +
' however its value is "foo-bar".',
source,
options: 'camelCase'
});
});
});

describe('empty pipe', () => {
it('should not fail when @Pipe not invoked', () => {
let source = `
describe('empty pipe', () => {
it('should not fail when @Pipe not invoked', () => {
let source = `
@Pipe
class Test {}`;
assertSuccess('pipe-naming', source, ['camelCase', 'ng']);
});
assertSuccess('pipe-naming', source, ['camelCase', 'ng']);
});
});

describe('pipe with name as variable', () => {
it('should ignore the rule when the name is a variable', () => {
const source = `
export function mockPipe(name: string): any {
@Pipe({ name })
class MockPipe implements PipeTransform {
transform(input: any): any {
return input;
}
}
return MockPipe;
}
`;
assertSuccess('pipe-naming', source, ['camelCase', 'ng']);
});
});

describe('valid pipe name', () => {
it('should succeed when set valid name with prefix ng in @Pipe', () => {
let source = `
describe('valid pipe name', () => {
it('should succeed when set valid name with prefix ng in @Pipe', () => {
let source = `
@Pipe({
name: 'ngBarFoo'
})
class Test {}`;
assertSuccess('pipe-naming', source, ['camelCase', 'ng']);
});
assertSuccess('pipe-naming', source, ['camelCase', 'ng']);
});

it('should succeed when set valid name applying multiple prefixes in @Pipe', () => {
let source = `
it('should succeed when set valid name applying multiple prefixes in @Pipe', () => {
let source = `
@Pipe({
name: 'ngBarFoo'
})
class Test {}`;
assertSuccess('pipe-naming', source, ['camelCase', 'ng', 'sg', 'mg']);
});
assertSuccess('pipe-naming', source, ['camelCase', 'ng', 'sg', 'mg']);
});

it('should succeed when set valid name in @Pipe', () => {
let source = `
it('should succeed when set valid name in @Pipe', () => {
let source = `
@Pipe({
name: 'barFoo'
})
class Test {}`;
assertSuccess('pipe-naming', source, ['camelCase']);
});
assertSuccess('pipe-naming', source, ['camelCase']);
});

it('should succeed when the class is not a Pipe', () => {
let source = `
it('should succeed when the class is not a Pipe', () => {
let source = `
class Test {}`;
assertSuccess('pipe-naming', source, ['camelCase']);
});
it('should do nothing if the name of the pipe is not a literal', () => {
let source = `
assertSuccess('pipe-naming', source, ['camelCase']);
});
it('should do nothing if the name of the pipe is not a literal', () => {
let source = `
const pipeName = 'foo-bar';
@Pipe({
name: pipeName
})
class Test {}`;
assertSuccess('pipe-naming', source, ['camelCase']);
});
assertSuccess('pipe-naming', source, ['camelCase']);
});
});
});

0 comments on commit 5c460be

Please sign in to comment.