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.

Sign up for GitHub

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

fix: error accessing invalid property #406

Merged
merged 2 commits into from
Sep 8, 2017
Merged

fix: error accessing invalid property #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e3fafb
fix: error accessing invalid property
mgechev Sep 8, 2017
57b11d7
style: add trailing newline
mgechev Sep 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 30 additions & 23 deletions src/pipeNamingRule.ts
View file
Open in desktop
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
Open in desktop
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']);
});
});
});