Improve IsInstanceLambdaUsage check
#401
Conversation
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
1 similar comment
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
| @@ -23,6 +23,7 @@ void identification() { | |||
| " // BUG: Diagnostic contains:", | |||
| " Stream.of(1).filter(i -> i instanceof Integer);", | |||
There was a problem hiding this comment.
The following expression is also flagged, while it shouldn't:
Stream.of(1).filter(i -> someOtherLocalVariable instanceof Integer);Similarly, we should not flag BiPredicates such as in:
Flux.just(1, "foo").distinctUntilChanged(v -> v, (a, b) -> a instanceof Integer);There was a problem hiding this comment.
While there: in identification tests we often list the "good" cases first; I'll shuffle om things.
| if (tree.getKind() != Kind.LAMBDA_EXPRESSION || tree.getBody().getKind() != Kind.INSTANCE_OF) { | ||
| if (tree.getKind() != Kind.LAMBDA_EXPRESSION | ||
| || tree.getBody().getKind() != Kind.INSTANCE_OF | ||
| || ((InstanceOfTree) tree.getBody()).getExpression().getKind() != Kind.IDENTIFIER) { | ||
| return Description.NO_MATCH; |
There was a problem hiding this comment.
While we're here, let's kill the highlighted mutants.
| @@ -39,7 +39,9 @@ public IsInstanceLambdaUsage() {} | |||
|
|
|||
| @Override | |||
| public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { | |||
| if (tree.getKind() != Kind.LAMBDA_EXPRESSION || tree.getBody().getKind() != Kind.INSTANCE_OF) { | |||
| if (tree.getKind() != Kind.LAMBDA_EXPRESSION | |||
| || tree.getBody().getKind() != Kind.INSTANCE_OF | |||
There was a problem hiding this comment.
Side-remark: at some point we should take a stance around testing the Kind versus performing an instanceof check. I expect the former to be faster (didn't check), but the latter is easier to understand for new users. Also, once we set JDK 17 as a baseline we may naturally start to use the latter i.c.w. pattern matching.
| if (tree.getKind() != Kind.LAMBDA_EXPRESSION | ||
| || tree.getBody().getKind() != Kind.INSTANCE_OF |
There was a problem hiding this comment.
It looks like tree.getBody().getKind() == Kind.INSTANCE_OF implies tree.getKind() == Kind.LAMBDA_EXPRESSION, so we can skip the latter check.
There was a problem hiding this comment.
I was looking at this but I think this is not correct. Isn't it because we are implementing a LambdaExpressionTreeMatcher, that makes the first condition always true?
There was a problem hiding this comment.
^ Discussed offline: in case of a "block lambda" we do hit this code, but tree.getBody().getKind() will then be BLOCK.
|
Looks good. All 5 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
|
Oh well, thanks a lot @Stephan202 :D |
By checking `isinstanceof` usages only against identifiers.
rickie
force-pushed
the
bdiederichs/is-instance-fix
branch
from
1220e5b to
3c2c657
Compare
|
Rebased. Will merge once 🍏. |
|
Oh and btw, thanks for the epic fixes @Stephan202. Shows us we need to think even more about all the edge cases. |
|
Looks good. All 5 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
By checking
isinstanceofusages only against identifiers.Fixes this issue.