OptionalRefactoringUtil: do not suggest opt.map().stream() -> opt.stream().flatMap() if operation may have side-effect

Side-effect is allowed for Optional.map, but not allowed for Stream.flatMap, and actually may cause a semantics change
Fixes IDEA-207178 Incorrect simplification of optional suggested by inspection
This commit is contained in:
Tagir Valeev
2019-02-15 12:27:18 +07:00
parent cf6d1885ed
commit a8259d29d2
4 changed files with 28 additions and 6 deletions
@@ -9,10 +9,7 @@ import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.psiutils.BoolUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.JavaPsiMathUtil;
import com.siyeh.ig.psiutils.StreamApiUtil;
import com.siyeh.ig.psiutils.*;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -122,7 +119,7 @@ public class OptionalRefactoringUtil {
}
}
String flatMapOperationName = StreamRefactoringUtil.getFlatMapOperationName(var.getType(), elementType);
if(flatMapOperationName != null) {
if(flatMapOperationName != null && !SideEffectChecker.mayHaveSideEffects(trueExpression)) {
return qualifier + ".stream()."+flatMapOperationName+"(" + LambdaUtil.createLambda(var, trueExpression) + ")";
}
}
@@ -15,7 +15,19 @@ public final class MyClass {
p.name().stream().mapToInt(String::length).forEach(System.out::println);
p.name().stream().flatMapToInt(String::chars).forEach(System.out::println);
p.name().filter(n -> !n.isEmpty()).stream().forEach(System.out::println);
p.name().stream().flatMap(MyClass::createStream);
p.name().map(m -> createStreamSideEffect(m)).orElse(Stream.empty());
Stream<Integer> stream = opt.stream();
}
static Stream<String> createStream(String s) {
return Stream.of(s, s);
}
static Stream<String> createStreamSideEffect(String s) {
System.out.println(s);
return Stream.of(s, s);
}
}
@@ -15,7 +15,19 @@ public final class MyClass {
p.name().map(n -> IntStream.of(n.length())).orElseGet(IntStream::empty).forEach(System.out::println);
p.name().map(String::chars).orElseGet(IntStream::empty).forEach(System.out::println);
p.name().map(n -> n.isEmpty() ? Stream.empty() : Stream.of(n)).orElseGet(Stream::empty).forEach(System.out::println);
p.name().map(m -> createStream(m)).orElse(Stream.empty());
p.name().map(m -> createStreamSideEffect(m)).orElse(Stream.empty());
Stream<Integer> stream = opt.map(Stream::of).orElseGet(Stream::empty);
}
static Stream<String> createStream(String s) {
return Stream.of(s, s);
}
static Stream<String> createStreamSideEffect(String s) {
System.out.println(s);
return Stream.of(s, s);
}
}
@@ -20,12 +20,13 @@ import com.intellij.codeInspection.LocalInspectionTool;
import com.intellij.codeInspection.SimplifyOptionalCallChainsInspection;
import com.intellij.openapi.projectRoots.Sdk;
import com.intellij.testFramework.IdeaTestUtil;
import com.intellij.testFramework.PsiTestUtil;
import org.jetbrains.annotations.NotNull;
public class SimplifyOptionalCallChainsInspectionTest extends LightQuickFixParameterizedTestCase {
@Override
protected Sdk getProjectJDK() {
return IdeaTestUtil.getMockJdk9();
return PsiTestUtil.addJdkAnnotations(IdeaTestUtil.getMockJdk9());
}
@NotNull