-
Notifications
You must be signed in to change notification settings - Fork 1.9k
JS: Add model of Angular2 #4450
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
Conversation
| import UnvalidatedDynamicMethodCallCustomizations::UnvalidatedDynamicMethodCall | ||
| private import DataFlow::FlowLabel | ||
|
|
||
| // Materialize flow labels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could add the below as a test somewhere:
import javascript
select any(DataFlow::FlowLabel label)That way we have a failing test if new flow-labels are added globally.
Also, this PR could use a merge.
And do you intend to rebase this PR on top of the recent ApiGraph changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And do you intend to rebase this PR on top of the recent ApiGraph changes?
No, mentioned in the PR description, it took a lot of evaluations to get here, so I'll defer integration to a later PR.
javascript/ql/test/library-tests/FlowLabels/DefaulFlowLabels.ql
Outdated
Show resolved
Hide resolved
|
I split the |
Adds a model of Angular, also known as Angular2 (not to be confused with AngularJS; we already have a model of that).
The model relies heavily on type annotations, as Angular's dependency injection depends on type information (which seemed impossible until I read this blog post). This mean types aren't just a substitute for missing data flow in this model.
The model doesn't currently use API graphs as there isn't currently a drop-in replacement for
hasUnderlyingTypewhich is pretty important for this model. Some improvements for this exact use-case are underway, though given the number of evaluation I've had to run I'd prefer to postpone integration to a later PR.It also includes two commits to resolve some performance issues that arose since some more
Customizationsfiles are now transitively reachable fromjavascript.qll. In particular, we should avoid creating flow labels in such files as they affect all data flow configurations.Evaluations: (internal links)