-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
JS: Support for matchAll #17910
JS: Support for matchAll #17910
Conversation
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.
A small suggestion, otherwise LGTM assuming the evaluations look good.
I think we should also look at MembershipCandidates.qll
and TaintTracking.qll
.
But lets do that in a followup PR.
Co-authored-by: Erik Krogh Kristensen <[email protected]>
93c2dad
to
810eddc
Compare
810eddc
to
449cee9
Compare
d7dab8b
to
bca14d7
Compare
9809499
to
a96f9fc
Compare
a96f9fc
to
a4fe728
Compare
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.
Looks good 👍
Just one small comment.
javascript/ql/test/experimental/Security/CWE-918/check-regex.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Erik Krogh Kristensen <[email protected]>
Is there a DCA run with all the changes? But it looks like they're only for the first half of the PR. Could you start one with source-suite: nightly-old, query-suite: code-scanning, and add |
DCA experiment looks good, no new unexpected sinks. |
Added support for ES2020 feature, matchAll.
Related issue: github/codeql-javascript-team/issues/435