From f0c78aea1559c054043511b148ce04b0d4048b4e Mon Sep 17 00:00:00 2001 From: Artemiy Sartakov Date: Tue, 21 Apr 2020 15:41:55 +0700 Subject: [PATCH] OptionalToIfInspection: misc fixes 1. generate if with curly braces when it contains single declaration inside 2. restore else branch after generating code for flatMap 3. do not reassign source variable for nested source operations GitOrigin-RevId: 061fbcc8d8dda586dcecb9a448752709a43370da --- .../codeInspection/optionalToIf/Instruction.java | 2 +- .../optionalToIf/IntermediateOperation.java | 5 ++++- .../optionalToIf/OptionalToIfContext.java | 4 ++++ .../optionalToIf/SourceOperation.java | 4 ++-- .../quickFix/optionalToIf/afterFlatMap.java | 14 ++++++++++++++ .../quickFix/optionalToIf/beforeFlatMap.java | 12 ++++++++++++ 6 files changed, 37 insertions(+), 4 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/Instruction.java b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/Instruction.java index c4c437792887..fa2324d4fc5f 100644 --- a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/Instruction.java +++ b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/Instruction.java @@ -143,7 +143,7 @@ interface Instruction { @Override public String generate() { - if (myInstructions.size() == 1 && !hasElseBranch()) { + if (myInstructions.size() == 1 && !hasElseBranch() && !(myInstructions.get(0) instanceof Declaration)) { return "if(" + myCondition.getText() + ")" + myInstructions.get(0).generate(); } String thenBranch = "if(" + myCondition.getText() + "){\n" + diff --git a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/IntermediateOperation.java b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/IntermediateOperation.java index 52aba104e0ab..1eb6c0208261 100644 --- a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/IntermediateOperation.java +++ b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/IntermediateOperation.java @@ -204,8 +204,11 @@ abstract class IntermediateOperation implements Operation { @NotNull ChainVariable outVar, @NotNull String code, @NotNull OptionalToIfContext context) { + String elseBranch = context.getElseBranch(); List records = StreamEx.of(myRecords).map(r -> replaceFnVariable(r, inVar, context)).collect(Collectors.toList()); - return OptionalToIfInspection.wrapCode(context, records, code); + String wrapped = OptionalToIfInspection.wrapCode(context, records, code); + context.setElseBranch(elseBranch); + return wrapped; } @NotNull diff --git a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/OptionalToIfContext.java b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/OptionalToIfContext.java index 8e4184911d74..6e51f6077821 100644 --- a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/OptionalToIfContext.java +++ b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/OptionalToIfContext.java @@ -57,6 +57,10 @@ class OptionalToIfContext extends ChainContext { myElseBranch = elseBranch; } + String getElseBranch() { + return myElseBranch; + } + @NotNull String generateNotNullCondition(@NotNull String arg, @NotNull String code) { if (myElseBranch == null) { diff --git a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/SourceOperation.java b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/SourceOperation.java index c5041da1dd1d..f908de731e4a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/optionalToIf/SourceOperation.java +++ b/java/java-impl/src/com/intellij/codeInspection/optionalToIf/SourceOperation.java @@ -67,7 +67,7 @@ abstract class SourceOperation implements Operation { @NotNull ChainVariable outVar, @NotNull String code, @NotNull OptionalToIfContext context) { - if (SourceOperation.getSourceName(myArg) != null) { + if (SourceOperation.getSourceName(myArg) != null || myArg.getText().equals(outVar.getName())) { return "if(" + outVar.getName() + "==null)throw new java.lang.NullPointerException();" + code; } @@ -111,7 +111,7 @@ abstract class SourceOperation implements Operation { @NotNull ChainVariable outVar, @NotNull String code, @NotNull OptionalToIfContext context) { - if (SourceOperation.getSourceName(myArg) != null) { + if (SourceOperation.getSourceName(myArg) != null || myArg.getText().equals(outVar.getName())) { return context.generateNotNullCondition(outVar.getName(), code); } return outVar.getDeclaration(myArg.getText()) + diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/afterFlatMap.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/afterFlatMap.java index ebfac1e73806..2aa60e71fc33 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/afterFlatMap.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/afterFlatMap.java @@ -50,6 +50,20 @@ class Test { } } + void nestedFlatMap(String var0) { + boolean b = false; + if (var0 != null) { + String var2 = var0.toLowerCase(); + b = true; + } + } + + String flatMapWithOrInside() { + Object o1 = null; + String empty = null; + throw new NoSuchElementException("No value present"); + } + T id(T t) { return t; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/beforeFlatMap.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/beforeFlatMap.java index 50936db2533f..b8adf24ee1a7 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/beforeFlatMap.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalToIf/beforeFlatMap.java @@ -32,6 +32,18 @@ class Test { String out = Optional.ofNullable(in).flatMap(s1 -> Optional.of(p)).orElse("bar"); } + void nestedFlatMap(String var0) { + boolean b = Optional.ofNullable(var0) + .flatMap(var1 -> + Optional.of(var1).map(s -> s.toLowerCase()) + .flatMap(var2 -> Optional.ofNullable(var2))) + .isPresent(); + } + + String flatMapWithOrInside() { + return Optional.empty().flatMap(s1 -> Optional.empty().or(() -> Optional.empty())).get(); + } + T id(T t) { return t; }