-
Notifications
You must be signed in to change notification settings - Fork 688
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
SONARJAVA-5147 FP S1905 when the type cast expression is used to call "getClass()" #5013
base: master
Are you sure you want to change the base?
Conversation
e387b20
to
4c29038
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, we can fine a tune a coupe of things and we should be good to merge!
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.
Thanks for adding the new cases with ambiguous .getClass
references. Is there a case where the rules is supposed to raise an issue with a .getClass()
call? I cannot see any example in this sample
Tree parent = typeCastTree.parent(); | ||
while (parent.is(Tree.Kind.PARENTHESIZED_EXPRESSION)) { | ||
parent = parent.parent(); | ||
} |
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.
Can we replace this skipParentheses?
"getClass".equals(memberSelect.identifier().name()) && | ||
memberSelect.parent() instanceof MethodInvocationTree methodInvocation && |
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.
What is most efficient here? To check that the right-hand side of the member select is part of a method invocation first or to match the name?
@@ -81,9 +83,25 @@ private boolean isUnnecessaryCast(TypeCastTree typeCastTree) { | |||
Tree parentTree = skipParentheses(typeCastTree.parent()); | |||
return !parentTree.is(Tree.Kind.ARGUMENTS); | |||
} | |||
if (isMethodInvocationReceiverOfGetClass(typeCastTree)) { | |||
// java.lang.Object#getClass() is a very specific method declared to return Class<?> but in fact | |||
// returns Class<? extends (receiver type)> at compile time. To prevent false-positive, we ignore 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.
// returns Class<? extends (receiver type)> at compile time. To prevent false-positive, we ignore this case. | |
// returns Class<? extends (receiver type)> at compile time. To prevent false-positives, we ignore this case. |
SONARJAVA-5147