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

Check whether it is an expression before accessing the expression properties #80

Merged

Conversation

SamvelRaja
Copy link
Contributor

Need to check whether the node is a expression before getting the expression properties

Found while testing the issue #79

Feel free to reject the PR if not needed.

@SamvelRaja SamvelRaja changed the title Check whether it is a expression before accessing the expression properties Check whether it is an expression before accessing the expression properties Sep 2, 2015
var lastArgument = route.expression.arguments[route.expression.arguments.length-1];
if (types.FunctionExpression.check(lastArgument)) {
traverseExpressions(lastArgument.body.body);
if (route.expression) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also check if the AST type is a CallExpression with if (types.CallExpression.check(route.expression)) { ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@marcioj
Copy link
Collaborator

marcioj commented Sep 2, 2015

Looks good, can you add a test covering this issue?

@SamvelRaja SamvelRaja force-pushed the patch_added_expression_truthiness branch from 32a5f7d to d809d02 Compare September 3, 2015 13:46
@SamvelRaja SamvelRaja force-pushed the patch_added_expression_truthiness branch from d809d02 to 9700acd Compare September 3, 2015 13:48
@SamvelRaja
Copy link
Contributor Author

Shall i include the test for normal route structure also? or this is enough? @marcioj

marcioj added a commit that referenced this pull request Sep 4, 2015
Check whether it is an expression before accessing the expression properties
@marcioj marcioj merged commit 24ee5cc into abuiles:master Sep 4, 2015
@marcioj
Copy link
Collaborator

marcioj commented Sep 4, 2015

Shall i include the test for normal route structure also?

This is good to go, we have other tests with more common scenarios.

Thank you for your contribution!

@SamvelRaja
Copy link
Contributor Author

👍

@SamvelRaja SamvelRaja deleted the patch_added_expression_truthiness branch September 4, 2015 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants