-
Notifications
You must be signed in to change notification settings - Fork 1.9k
simplify expressions that could be type-casts #7668
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
nickrolfe
left a comment
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.
👍 for Ruby
smowton
left a comment
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.
Java 👍
tausbn
left a comment
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.
Python 👍
| exists(DataFlow::Node r | r = requestNode.getAMethodCall("body") | result = r) | ||
| or | ||
| // Otherwise, treat the response as the response body. | ||
| not exists(DataFlow::Node r | r = requestNode.getAMethodCall("body")) and |
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.
I wonder if the line
exists(DataFlow::Node r | r = requestNode.getAMethodCall("body") | result = r)
could also be simplified to
result = requestNode.getAMethodCall("body")
if the query was extended slightly? (We know that result has type DataFlow::Node in this case.)
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.
It could, and originally my query did just that (see the discussion in the PR).
But that had too many FPs, so I dropped it.
hvitved
left a comment
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.
C# 👍
Fixes the issues identified by this QL-for-QL query.
Consider the below:
That is basically just a type-cast, so we might as well simplify it as such:
There are also some cases where the resulting type-cast would be redundant.
Here's an example:
The
getAttributepredicate hasXMLAttributeas its result type, so adding a type-cast would be redundant.It is therefore simplified as follows:
The patch was generated automatically by a tool.
(GitHub people: you can see how in the backref below).