Java: Replace incorrect usage of Literal.getLiteral()#6612
Java: Replace incorrect usage of Literal.getLiteral()#6612aschackmull merged 2 commits intogithub:mainfrom
Literal.getLiteral()#6612Conversation
| lit.getType().getName() = "float" or | ||
| lit.getType().getName() = "double" | ||
| } | ||
| predicate isNumber(Literal lit) { lit.getType() instanceof NumericOrCharType } |
There was a problem hiding this comment.
Changes behaviour by adding byte and boxed types
There was a problem hiding this comment.
That is what I mentioned in the description. Sorry, would probably have been better to add a comment instead.
Was it intentional that byte was excluded? (I can also revert the change though)
Boxed types should not be an issue (except for performance?) because literals are always primitive, except for String but that is not a numeric type.
There was a problem hiding this comment.
It's fine to include byte. The dependent queries are low-precision recommendation queries so it doesn't matter too much.
java/ql/src/Violations of Best Practice/Magic Constants/MagicConstants.qll
Outdated
Show resolved
Hide resolved
|
Additionally, and there's also an internal qltest that's failing. |
|
Have fixed the test failures, please let me know if the changes are fine. Does the change to |
d9ce781 to
37268bd
Compare
| result = | ||
| [ | ||
| "RSA", "SHA256", "SHA512", "CCM", "GCM", "AES([^a-zA-Z](?!ECB|CBC/PKCS[57]Padding)).*", | ||
| "RSA", "SHA256", "SHA512", "CCM", "GCM", "AES(?)", |
There was a problem hiding this comment.
.* does not seem to be necessary here because the algorithmRegex predicate will add that.
Also, is there a reason why these queries use rank[...] and max instead of concat to concat the algorithm names? Should I replace that?
There was a problem hiding this comment.
No good reason AFAICT, it's not important though
|
An internal test is failing because this changes the observed behaviour of annotations' get-value methods. Made an internal PR to update it. |
37268bd to
020aa4d
Compare
Literal.getLiteral()returns the string representation of the literal the way it was written in source. This is rarely desired by queries and there were a few cases where it was used erroneously.Notable changes:
Literal.getLiteral()Annotatable.suppressesWarningsAbout(string)usinggetLiteral().This might be a breaking change for third party queries, however the current state is somewhat broken:
BooleanLiteral.getBooleanValue()usinggetLiteral().This caused it to not work when a boolean literal was written using Unicode escapes (which are replaced during pre-processing), e.g.
boolean b = \u0074\u0072\u0075\u0065;. A test for this will be added by the follow-up pull request Java: Split literals tests #6613.StringLiteral.getValue()withgetRepresentedString(). The result is the same (getRepresentedStringdelegates togetValue), but it might make the intention clearer.MagicConstants.qll(please let me know if this should be a separate pull request):getLiteral()isNumberto check forNumericOrCharType. Note that this changes functionality because previously it did not considerbyte, not sure if that was an oversight.intTrivialandlongTrivialto remove_from literal and to check for lowercaselfor long literals.