Skip to content

JS ES6 Support and Template Literals Support #1493

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

apple5775
Copy link

For javascript, add ES6 Support for rhino 1.7.15, and support Template Literals as a concatenation/addition of strings. Also includes a few tests for Template Literals in "es6.js" file. Tagged Template Literals are not yet supported.

Copy link
Member

@msridhar msridhar left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution! I have a few comments and questions.

}

@Override
public CAstNode visitTemplateLiteral(TemplateLiteral node, WalkContext arg) {
Copy link
Member

Choose a reason for hiding this comment

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

I can't really understand the logic here; I think we need some comments to explain. Is the code in this method handling just the string concatenation?

Comment on lines +1632 to +1635
public CAstNode visitTaggedTemplateLiteral(TaggedTemplateLiteral node, WalkContext arg) {
// TODO Auto-generated method stub
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm concerned this will cause a crash if we get an input with a tagged template literal. If we don't handle this case yet, maybe we should not add this method and just skip visiting the nodes?

@@ -158,6 +161,12 @@ public R visit(AstNode node, A arg) {
return visitSwitchCase((SwitchCase) node, arg);
} else if (node instanceof SwitchStatement) {
return visitSwitchStatement((SwitchStatement) node, arg);
} else if (node instanceof TaggedTemplateLiteral) {
return visitTaggedTemplateLiteral((TaggedTemplateLiteral) node, arg);
Copy link
Member

Choose a reason for hiding this comment

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

See comment above about not visiting these nodes if we don't yet handle them; or is the issue that we'll get an unexpected node type?

In general, what happens right now when we try to pass an ES6 input to WALA, given that we haven't set the language version? Do we just get a parse error in Rhino?

@@ -1071,4 +1071,38 @@ public void testComplexFinally()

CAstCallGraphUtil.dumpCG(B.getCFAContextInterpreter(), B.getPointerAnalysis(), CG);
}

private static final Object[][] assertionsES6Args =
Copy link
Member

Choose a reason for hiding this comment

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

For the test cases where the template substitution has a function call, can we assert that the appropriate call edge(s) are present?

Copy link

Test Results

  827 files  ±0    827 suites  ±0   5h 33m 0s ⏱️ + 1h 5m 9s
  766 tests +1    749 ✅ +1   17 💤 ±0  0 ❌ ±0 
5 246 runs  +7  5 125 ✅ +7  121 💤 ±0  0 ❌ ±0 

Results for commit b513d4d. ± Comparison against base commit c85f543.

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