-
Notifications
You must be signed in to change notification settings - Fork 1.7k
JS: Address some code that weren't affecting any query result #8422
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
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.
Interesting findings. Thanks once again for doing this kind of cleanup!
One nit: Should all of the deprecations have a qldoc string pointing the users to the alternative?
Also, please confirm that https://github.com/github/codeql/blob/4fc85a791d45d931ec6dac0304e8c314eb0ba23a/javascript/ql/test/library-tests/Classes/tests.ql contains the inlined query predicate version of the files deleted in 5e52a71.
👍
Confirmed 👍 |
We need a change note for the same reason as in #8323 (comment):
|
As a followup to #8347 I checked if some code had become dead.
There was no new dead code, but I found some interesting things regardless.
That is partly expected (e.g. a lot of classes in
Expr.qll
/TypeScript.qll
are not used anywhere).But there were also some unexpected result, like a few
Sanitizer
classes that weren't referenced anywhere.(Not this PR, I'll address that in another PR).
This query partially addresses 1) in JS.
There's still plenty of code that doesn't affect any query, but all the remaining seems benign.