From 18fa2bc0f07120147fac2e8eafea1a1508166593 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 22 Aug 2019 11:48:45 +0700 Subject: [PATCH] SimplifyOptionalCallChainsInspection.RewrappingCase: check orElse argument (IDEA-220982) Also disabled for `orElseGet(() -> null)` as it would require more logic to check the lambda content and we already suggest to replace this with `orElse(null)` GitOrigin-RevId: ec86f715ef0f78c637447d5379a239016ecfd3b6 --- .../SimplifyOptionalCallChainsInspection.java | 7 +++++-- .../inspection/optionalChains/afterRewrapOrElseGet.java | 8 -------- .../inspection/optionalChains/beforeRewrapNotNull.java | 8 ++++++++ .../inspection/optionalChains/beforeRewrapOrElseGet.java | 4 +++- 4 files changed, 16 insertions(+), 11 deletions(-) delete mode 100644 java/java-tests/testData/inspection/optionalChains/afterRewrapOrElseGet.java create mode 100644 java/java-tests/testData/inspection/optionalChains/beforeRewrapNotNull.java diff --git a/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java b/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java index 190d85a5ce71..60fe941d46c6 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java @@ -470,9 +470,12 @@ public class SimplifyOptionalCallChainsInspection extends AbstractBaseJavaLocalI !EquivalenceChecker.getCanonicalPsiEquivalence().typesAreEquivalent(qualifier.getType(), parentCall.getType())) { return null; } - if ("get".equals(call.getMethodExpression().getReferenceName())) { + String name = call.getMethodExpression().getReferenceName(); + if ("get".equals(name)) { SpecialFieldValue fact = CommonDataflow.getExpressionFact(qualifier, DfaFactType.SPECIAL_FIELD_VALUE); if (DfaFactType.NULLABILITY.fromDfaValue(SpecialField.OPTIONAL_VALUE.extract(fact)) != DfaNullability.NOT_NULL) return null; + } else if ("orElse".equals(name)) { + if (!ExpressionUtils.isNullLiteral(call.getArgumentList().getExpressions()[0])) return null; } return new Context(qualifier, parentCall); } @@ -490,7 +493,7 @@ public class SimplifyOptionalCallChainsInspection extends AbstractBaseJavaLocalI if (myType == Type.OptionalGet) { return OPTIONAL_GET; } - return OPTIONAL_OR_ELSE_OR_ELSE_GET; + return OPTIONAL_OR_ELSE; } private static class Context { diff --git a/java/java-tests/testData/inspection/optionalChains/afterRewrapOrElseGet.java b/java/java-tests/testData/inspection/optionalChains/afterRewrapOrElseGet.java deleted file mode 100644 index 7c9217cfd6a0..000000000000 --- a/java/java-tests/testData/inspection/optionalChains/afterRewrapOrElseGet.java +++ /dev/null @@ -1,8 +0,0 @@ -// "Unwrap" "true" -import java.util.*; - -public class Tests { - void test(List list) { - Optional opt = list.stream().filter(Objects::nonNull).findFirst(); - } -} diff --git a/java/java-tests/testData/inspection/optionalChains/beforeRewrapNotNull.java b/java/java-tests/testData/inspection/optionalChains/beforeRewrapNotNull.java new file mode 100644 index 000000000000..e100e3e59798 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/beforeRewrapNotNull.java @@ -0,0 +1,8 @@ +// "Unwrap" "false" +import java.util.*; + +public class Tests { + private Optional test(Optional testOptional, String defaultVal) { + return Optional.ofNullable(testOptional.orElse(defaultVal)); + } +} diff --git a/java/java-tests/testData/inspection/optionalChains/beforeRewrapOrElseGet.java b/java/java-tests/testData/inspection/optionalChains/beforeRewrapOrElseGet.java index ade2df7bb1a4..4a2428cb9f6e 100644 --- a/java/java-tests/testData/inspection/optionalChains/beforeRewrapOrElseGet.java +++ b/java/java-tests/testData/inspection/optionalChains/beforeRewrapOrElseGet.java @@ -1,8 +1,10 @@ -// "Unwrap" "true" +// "Unwrap" "false" import java.util.*; public class Tests { void test(List list) { + // Will be reported as "Excessive lambda usage", changing to orElse which will trigger rewrapping inspection + // no need to do this in single step Optional opt = Optional.ofNullable(list.stream().filter(Objects::nonNull).findFirst().orElseGet(() -> null)); } }