From 6682d9efc3ab33ccd4c18ed6e9b05641b1cdc07c Mon Sep 17 00:00:00 2001 From: Bartek Pacia Date: Fri, 22 Nov 2024 16:05:37 +0100 Subject: [PATCH] [java-inspections] Non functional style `Optional.isPresent` - missed opportunity IDEA-360860 fixed * split tests into individual methods, to make debugging easier * add javadoc for VariableAccessUtils#isVariableTypeChangeSafeForReference (cherry picked from commit f6b3468079959f7adec737e0b8ec3ad1d97e7ad2) IJ-MR-150176 GitOrigin-RevId: 4b430e49819c7cba1e08ecee47b881f05be570d4 --- .../ig/psiutils/VariableAccessUtils.java | 28 ++++- .../OptionalIsPresentInspection.java | 103 ++++++++++++++---- .../psi/util/MethodSignatureUtil.java | 2 +- .../afterConsumerIndirect.java | 12 ++ .../afterConsumerIndirectComplex_1.java | 12 ++ .../afterConsumerIndirectOverload.java | 18 +++ .../beforeConsumerIndirect.java | 15 +++ .../beforeConsumerIndirectComplex_1.java | 15 +++ .../beforeConsumerIndirectOverload.java | 21 ++++ ...OptionalIsPresent.java => Assignment.java} | 15 +-- .../optionalIsPresent/Consumer.java | 10 ++ .../optionalIsPresent/ConsumerIndirect.java | 13 +++ .../ConsumerIndirectAmbiguousOverload.java | 15 +++ .../ConsumerIndirectComment.java | 14 +++ .../ConsumerIndirectFieldInaccessible.java | 20 ++++ .../ConsumerIndirectTooComplex_1.java | 12 ++ .../ConsumerIndirectTooComplex_2.java | 13 +++ .../ConsumerIndirectTypeMismatch.java | 14 +++ .../optionalIsPresent/Optional.java | 13 +++ .../OptionalIsPresentInspectionTest.java | 23 +++- 20 files changed, 349 insertions(+), 39 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirect.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectComplex_1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectOverload.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirect.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectComplex_1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectOverload.java rename java/java-tests/testData/inspection/optionalIsPresent/{OptionalIsPresent.java => Assignment.java} (55%) create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/Consumer.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirect.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectAmbiguousOverload.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectComment.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectFieldInaccessible.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_1.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_2.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTypeMismatch.java create mode 100644 java/java-tests/testData/inspection/optionalIsPresent/Optional.java diff --git a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/VariableAccessUtils.java b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/VariableAccessUtils.java index e1be59bb950c..aeb61707ad1d 100644 --- a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/VariableAccessUtils.java +++ b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/VariableAccessUtils.java @@ -598,7 +598,31 @@ public final class VariableAccessUtils { .allMatch(context -> context == null || PsiTreeUtil.isAncestor(context, block, false)); } - static boolean isVariableTypeChangeSafeForReference(@NotNull PsiType targetType, @NotNull PsiReferenceExpression reference) { + /// Returns true if the type of element that `reference` refers to can be safely changed to `targetType`. + /// + /// ### Example + /// + /// Consider the following (obviously wrong, but good enough for demo purpose) code: + /// + /// ```java + /// void foo(Optional opt) { + /// Object obj = opt.get(); + /// if (obj instanceof Integer) { + /// doStuff(); + /// } + /// // ... + /// } + /// ``` + /// + /// Let's say you'd like to know if it's "safe" to change the type of local variable `obj` from `Object` to `String`. + /// "Safe" means that there likely will be no compile-time errors or behavior difference after such a type change is performed. + /// + /// In our example, this method returns false because changing the type of `obj` + /// to `String` will cause a compile-time error like `cannot cast 'java.lang.String' to 'java.lang.Integer'`. + /// + /// It's possible there are cases not covered by this method. + /// If you discover such a case, consider updating the implementation. + public static boolean isVariableTypeChangeSafeForReference(@NotNull PsiType targetType, @NotNull PsiReferenceExpression reference) { PsiElement parent = PsiUtil.skipParenthesizedExprUp(reference.getParent()); if (PsiUtil.isAccessedForWriting(reference)) { PsiAssignmentExpression assignmentExpression = tryCast(parent, PsiAssignmentExpression.class); @@ -670,4 +694,4 @@ public final class VariableAccessUtils { return usedVariables; } } -} \ No newline at end of file +} diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/OptionalIsPresentInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/OptionalIsPresentInspection.java index fc62965c6954..2016f7bb6c9e 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/OptionalIsPresentInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/OptionalIsPresentInspection.java @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection; import com.intellij.codeInsight.Nullability; @@ -27,7 +27,9 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import static com.intellij.codeInsight.PsiEquivalenceUtil.areElementsEquivalent; @@ -47,12 +49,12 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp WARNING, INFO, NONE; void registerProblem(@NotNull ProblemsHolder holder, @NotNull PsiExpression condition, OptionalIsPresentCase scenario) { - if(this != NONE) { + if (this != NONE) { if (this == INFO && !holder.isOnTheFly()) { return; //don't register fixes in batch mode } holder.registerProblem(condition, JavaBundle.message( - "inspection.message.can.be.replaced.with.single.expression.in.functional.style"), + "inspection.message.can.be.replaced.with.single.expression.in.functional.style"), this == INFO ? ProblemHighlightType.INFORMATION : ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new OptionalIsPresentFix(scenario)); } @@ -137,7 +139,8 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp PsiMethodCallExpression call = ObjectUtils.tryCast(condition, PsiMethodCallExpression.class); if (!OPTIONAL_IS_PRESENT.matches(call) && !OPTIONAL_IS_EMPTY.matches(call)) return null; PsiReferenceExpression qualifier = - ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression()), PsiReferenceExpression.class); + ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression()), + PsiReferenceExpression.class); if (qualifier == null) return null; PsiElement element = qualifier.resolve(); if (!(element instanceof PsiVariable) || isRaw((PsiVariable)element)) return null; @@ -174,6 +177,7 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp areElementsEquivalent(lambdaCandidate, optionalRef) && OptionalUtil.isOptionalEmptyCall(falseExpression)) { return ProblemType.WARNING; } + if (!LambdaGenerationUtil.canBeUncheckedLambda(lambdaCandidate, optionalRef::isReferenceTo)) return ProblemType.NONE; Ref hasOptionalReference = new Ref<>(Boolean.FALSE); boolean hasNoBadRefs = PsiTreeUtil.processElements(lambdaCandidate, e -> { @@ -183,10 +187,15 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp hasOptionalReference.set(Boolean.TRUE); return isOptionalGetCall(e.getParent().getParent(), optionalRef); }); + + if (hasOptionalReference.get() && getOnlyLocalVariable(lambdaCandidate, optionalRef) != null) { + return ProblemType.WARNING; + } + if (!hasNoBadRefs) return ProblemType.NONE; if (!hasOptionalReference.get() || !(lambdaCandidate instanceof PsiExpression expression)) return ProblemType.INFO; if (falseExpression != null) { - // falseExpression == null is "consumer" case (to be replaced with ifPresent()) + // falseExpression == null is a "consumer" case (to be replaced with ifPresent()) if (!ExpressionUtils.isNullLiteral(falseExpression) && NullabilityUtil.getExpressionNullability(expression, true) != Nullability.NOT_NULL) { // if falseExpression is null literal, then semantics is preserved @@ -197,7 +206,7 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp if (falseType == null || trueType == null) return ProblemType.NONE; if (falseType instanceof PsiPrimitiveType && trueType instanceof PsiPrimitiveType) { if (falseType.equals(trueType) || JavaPsiMathUtil.getNumberFromLiteral(falseExpression) != null) { - // like x ? double_expression : integer_expression; support only if integer_expression is simple literal, + // like x ? double_expression : integer_expression; support only if integer_expression is a simple literal, // so could be converted explicitly to double return ProblemType.WARNING; } @@ -210,6 +219,42 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp return ProblemType.WARNING; } + /// Returns non-null if `lambdaCandidate` is a special case from IDEA-360860. + /// + /// If the returned [PsiLocalVariable] is non-null, then this local variable: + /// * is the only local variable declaration inside `lambdaCandidate` + /// * it's initialized with a call to [Optional#get()] on `optionalRef` + private static @Nullable PsiLocalVariable getOnlyLocalVariable(@NotNull PsiElement lambdaCandidate, + @NotNull PsiReferenceExpression optionalRef) { + if (!(lambdaCandidate instanceof PsiBlockStatement blockStmt)) return null; + PsiStatement[] statements = blockStmt.getCodeBlock().getStatements(); + + if (statements.length == 2 && + statements[0] instanceof PsiDeclarationStatement declStmt && + declStmt.getFirstChild() instanceof PsiLocalVariable localVariable && + statements[1] instanceof PsiExpressionStatement exprStmt) { + + // Check that the Optional variable is referenced indirectly: first assigned to variable with get() call, then the variable is used + PsiExpression localVariableInitializer = localVariable.getInitializer(); + if (localVariableInitializer == null) return null; + if (!VariableAccessUtils.variableIsUsed(localVariable, exprStmt)) return null; + if (!isOptionalGetCall(localVariableInitializer, optionalRef)) return null; + List varReferences = VariableAccessUtils.getVariableReferences(localVariable); + + PsiType unwrappedOptionalType = PsiUtil.substituteTypeParameter(optionalRef.getType(), CommonClassNames.JAVA_UTIL_OPTIONAL, 0, false); + if (unwrappedOptionalType == null) return null; + + for (PsiReferenceExpression varReference : varReferences) { + if (!VariableAccessUtils.isVariableTypeChangeSafeForReference(unwrappedOptionalType, varReference)) { + return null; + } + } + + return localVariable; + } + return null; + } + @NotNull static String generateOptionalLambda(@NotNull PsiElementFactory factory, @NotNull CommentTracker ct, @@ -217,8 +262,19 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp PsiElement trueValue) { PsiType type = optionalRef.getType(); String paramName = new VariableNameGenerator(trueValue, VariableKind.PARAMETER) - .byType(OptionalUtil.getOptionalElementType(type)).byName("value").generate(true); - if(trueValue instanceof PsiExpressionStatement) { + .byType(OptionalUtil.getOptionalElementType(type)) + .byName("value") + .generate(true); + + PsiLocalVariable theOnlyVariable = getOnlyLocalVariable(trueValue, optionalRef); + if (theOnlyVariable != null && trueValue instanceof PsiBlockStatement blockStmt) { + paramName = theOnlyVariable.getName(); + PsiStatement varDecl = blockStmt.getCodeBlock().getStatements()[0]; + varDecl.delete(); + trueValue = blockStmt.getCodeBlock().getStatements()[0]; + } + + if (trueValue instanceof PsiExpressionStatement) { trueValue = ((PsiExpressionStatement)trueValue).getExpression(); } ct.markUnchanged(trueValue); @@ -227,8 +283,8 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp PsiElement result = getCall.replace(factory.createIdentifier(paramName)); if (copy == getCall) copy = result; } - if(copy instanceof PsiStatement && !(copy instanceof PsiBlockStatement)) { - return paramName + "->{" + copy.getText()+"}"; + if (copy instanceof PsiStatement && !(copy instanceof PsiBlockStatement)) { + return paramName + "->{" + copy.getText() + "}"; } return paramName + "->" + copy.getText(); } @@ -314,7 +370,9 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp public ProblemType getProblemType(@NotNull PsiReferenceExpression optionalRef, @Nullable PsiElement trueElement, @Nullable PsiElement falseElement) { - if (!(trueElement instanceof PsiReturnStatement trueReturn) || !(falseElement instanceof PsiReturnStatement falseReturn)) return ProblemType.NONE; + if (!(trueElement instanceof PsiReturnStatement trueReturn) || !(falseElement instanceof PsiReturnStatement falseReturn)) { + return ProblemType.NONE; + } PsiExpression falseValue = falseReturn.getReturnValue(); PsiExpression trueValue = trueReturn.getReturnValue(); if (!isSimpleOrUnchecked(falseValue)) return ProblemType.NONE; @@ -332,7 +390,8 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp LOG.assertTrue(trueValue != null); LOG.assertTrue(falseValue != null); return "return " + - generateOptionalUnwrap(factory, ct, optionalVariable, trueValue, falseValue, PsiTypesUtil.getMethodReturnType(trueElement)) + + generateOptionalUnwrap(factory, ct, optionalVariable, trueValue, falseValue, + PsiTypesUtil.getMethodReturnType(trueElement)) + ";"; } }, @@ -381,7 +440,9 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp public ProblemType getProblemType(@NotNull PsiReferenceExpression optionalVariable, @Nullable PsiElement trueElement, @Nullable PsiElement falseElement) { - if(!(trueElement instanceof PsiExpression trueExpression) || !(falseElement instanceof PsiExpression falseExpression)) return ProblemType.NONE; + if (!(trueElement instanceof PsiExpression trueExpression) || !(falseElement instanceof PsiExpression falseExpression)) { + return ProblemType.NONE; + } PsiType trueType = trueExpression.getType(); PsiType falseType = falseExpression.getType(); if (trueType == null || falseType == null || !trueType.isAssignableFrom(falseType) || !isSimpleOrUnchecked(falseExpression)) { @@ -413,8 +474,8 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp @Nullable PsiElement falseElement) { if (falseElement != null && !(falseElement instanceof PsiEmptyStatement)) return ProblemType.NONE; if (!(trueElement instanceof PsiStatement)) return ProblemType.NONE; - if (trueElement instanceof PsiExpressionStatement) { - PsiExpression expression = ((PsiExpressionStatement)trueElement).getExpression(); + if (trueElement instanceof PsiExpressionStatement expressionStmt) { + PsiExpression expression = expressionStmt.getExpression(); if (isOptionalGetCall(expression, optionalRef)) return ProblemType.NONE; trueElement = expression; } @@ -434,14 +495,14 @@ public final class OptionalIsPresentInspection extends AbstractBaseJavaLocalInsp @NotNull abstract ProblemType getProblemType(@NotNull PsiReferenceExpression optionalVariable, - @Nullable PsiElement trueElement, - @Nullable PsiElement falseElement); + @Nullable PsiElement trueElement, + @Nullable PsiElement falseElement); @NotNull abstract String generateReplacement(@NotNull PsiElementFactory factory, - @NotNull CommentTracker ct, - @NotNull PsiReferenceExpression optionalVariable, - PsiElement trueElement, - PsiElement falseElement); + @NotNull CommentTracker ct, + @NotNull PsiReferenceExpression optionalVariable, + PsiElement trueElement, + PsiElement falseElement); } } diff --git a/java/java-psi-api/src/com/intellij/psi/util/MethodSignatureUtil.java b/java/java-psi-api/src/com/intellij/psi/util/MethodSignatureUtil.java index 54e936468b8c..bbf2ca8dd46a 100644 --- a/java/java-psi-api/src/com/intellij/psi/util/MethodSignatureUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/util/MethodSignatureUtil.java @@ -1,4 +1,4 @@ -// Copyright 2000-2021 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.psi.util; import com.intellij.openapi.util.Comparing; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirect.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirect.java new file mode 100644 index 000000000000..7b64c330577c --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirect.java @@ -0,0 +1,12 @@ +// "Replace Optional presence condition with functional style expression" "GENERIC_ERROR_OR_WARNING" + +import java.util.*; + +public class Main { + public void testOptional(Optional str) { + if (str == null) str = Optional.empty(); + str.ifPresent(this::use); + } + + void use(Object obj) { System.out.println("Object"); } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectComplex_1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectComplex_1.java new file mode 100644 index 000000000000..f7274b339ed3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectComplex_1.java @@ -0,0 +1,12 @@ +// "Replace Optional presence condition with functional style expression" "GENERIC_ERROR_OR_WARNING" + +import java.util.*; + +public class Main { + public void testOptional(Optional str) { + if (str == null) str = Optional.empty(); + str.ifPresent(obj -> use(obj + ":" + obj)); + } + + void use(String obj) { System.out.println("String"); } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectOverload.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectOverload.java new file mode 100644 index 000000000000..df59a9c0b9c9 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerIndirectOverload.java @@ -0,0 +1,18 @@ +// "Replace Optional presence condition with functional style expression" "GENERIC_ERROR_OR_WARNING" + +import java.util.*; + +public class Main { + public void testOptional(Optional str) { + if (str == null) str = Optional.empty(); + str.ifPresent(this::use); + } + + void use(Object obj) { + System.out.println("Object"); + } + + void use(String obj, int i) { + System.out.println("String"); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirect.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirect.java new file mode 100644 index 000000000000..7365ea28ba11 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirect.java @@ -0,0 +1,15 @@ +// "Replace Optional presence condition with functional style expression" "GENERIC_ERROR_OR_WARNING" + +import java.util.*; + +public class Main { + public void testOptional(Optional str) { + if (str == null) str = Optional.empty(); + if (str.isPresent()) { + String obj = str.get(); + use(obj); + } + } + + void use(Object obj) { System.out.println("Object"); } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectComplex_1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectComplex_1.java new file mode 100644 index 000000000000..ff0fc5221a6b --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectComplex_1.java @@ -0,0 +1,15 @@ +// "Replace Optional presence condition with functional style expression" "GENERIC_ERROR_OR_WARNING" + +import java.util.*; + +public class Main { + public void testOptional(Optional str) { + if (str == null) str = Optional.empty(); + if (str.isPresent()) { + String obj = str.get(); + use(obj + ":" + obj); + } + } + + void use(String obj) { System.out.println("String"); } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectOverload.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectOverload.java new file mode 100644 index 000000000000..8e5ec378d8b5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerIndirectOverload.java @@ -0,0 +1,21 @@ +// "Replace Optional presence condition with functional style expression" "GENERIC_ERROR_OR_WARNING" + +import java.util.*; + +public class Main { + public void testOptional(Optional str) { + if (str == null) str = Optional.empty(); + if (str.isPresent()) { + Object obj = str.get(); + use(obj); + } + } + + void use(Object obj) { + System.out.println("Object"); + } + + void use(String obj, int i) { + System.out.println("String"); + } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/OptionalIsPresent.java b/java/java-tests/testData/inspection/optionalIsPresent/Assignment.java similarity index 55% rename from java/java-tests/testData/inspection/optionalIsPresent/OptionalIsPresent.java rename to java/java-tests/testData/inspection/optionalIsPresent/Assignment.java index 43adb64f7d52..0d78c07fd2ee 100644 --- a/java/java-tests/testData/inspection/optionalIsPresent/OptionalIsPresent.java +++ b/java/java-tests/testData/inspection/optionalIsPresent/Assignment.java @@ -1,9 +1,8 @@ // Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -import java.lang.invoke.MethodHandle; import java.util.Optional; -public class OptionalIsPresent { +class OptionalIsPresent { public void testIsPresent(Optional str) { String val; if (str.isPresent()) { @@ -13,14 +12,4 @@ public class OptionalIsPresent { } System.out.println(val); } - - public void testOptional(Optional str) { - String val; - if (str.isEmpty()) { - val = ""; - } else { - val = str.get(); - } - System.out.println(val); - } -} \ No newline at end of file +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/Consumer.java b/java/java-tests/testData/inspection/optionalIsPresent/Consumer.java new file mode 100644 index 000000000000..507757d58d36 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/Consumer.java @@ -0,0 +1,10 @@ +import java.util.Optional; + +class OptionalIsPresent_2 { + public void foo() { + Optional opt = Optional.ofNullable(Math.random() > 0.5 ? new Object() : null); + if (opt.isPresent()) { + System.out.println(opt.get()); + } + } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirect.java b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirect.java new file mode 100644 index 000000000000..7116a8e47942 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirect.java @@ -0,0 +1,13 @@ +import java.util.Optional; + +class OptionalIsPresent { + public void foo() { + Optional opt = Optional.ofNullable(Math.random() > 0.5 ? new Object() : null); + if (opt.isPresent()) { + Object obj = opt.get(); + use(obj); + } + } + + void use(Object obj) { System.out.println("Object"); } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectAmbiguousOverload.java b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectAmbiguousOverload.java new file mode 100644 index 000000000000..b5b3beab0487 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectAmbiguousOverload.java @@ -0,0 +1,15 @@ +import java.util.Optional; + +class OptionalIsPresent { + public void testOptional(Optional str) { + if (str == null) str = Optional.empty(); + if (str.isPresent()) { + Object obj = str.get(); + use(obj, 1); // resolves to use(Object) + } + } + + void use(Object obj, int i) { System.out.println("Object"); } + + void use(String obj, int i) { System.out.println("String"); } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectComment.java b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectComment.java new file mode 100644 index 000000000000..992aae68f7f9 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectComment.java @@ -0,0 +1,14 @@ +import java.util.Optional; + +class OptionalIsPresent { + public void foo() { + Optional opt = Optional.ofNullable(Math.random() > 0.5 ? new Object() : null); + if (opt.isPresent()) { + // who dares wins + Object obj = opt.get(); + use(obj); + } + } + + void use(Object obj) { System.out.println("Object"); } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectFieldInaccessible.java b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectFieldInaccessible.java new file mode 100644 index 000000000000..b68de4aa44d1 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectFieldInaccessible.java @@ -0,0 +1,20 @@ +import java.util.Optional; + +class Super { + private int x; + + public void foo(Optional opt) { + if (opt == null) opt = Optional.empty(); + if (opt.isPresent()) { + // Changing type of `obj` from Super to Sub will cause a compile-time error (field x is private). Hence, don't suggest fix. + Super obj = opt.get(); + use(obj.x); + } + } + + void use(Object obj) { + System.out.println("Object"); + } +} + +class Sub extends Super {} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_1.java b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_1.java new file mode 100644 index 000000000000..88287c6d9a4a --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_1.java @@ -0,0 +1,12 @@ +import java.util.Optional; + +class OptionalIsPresent { + public void foo() { + Optional opt = Optional.ofNullable(Math.random() > 0.5 ? new Object() : null); + if (opt.isPresent()) { + Object obj = opt.get(); + Object a = opt.get(); + System.out.println(opt.get()); + } + } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_2.java b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_2.java new file mode 100644 index 000000000000..fe571450b16f --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTooComplex_2.java @@ -0,0 +1,13 @@ +import java.util.Optional; + +class OptionalIsPresent { + public void foo() { + Optional opt = Optional.ofNullable(Math.random() > 0.5 ? new Object() : null); + if (opt.isPresent()) { + Object obj = opt.get(); + Object obj2 = opt.get(); + System.out.println(obj); + System.out.println(obj2); + } + } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTypeMismatch.java b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTypeMismatch.java new file mode 100644 index 000000000000..a9d6e8bc665d --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/ConsumerIndirectTypeMismatch.java @@ -0,0 +1,14 @@ +import java.util.Optional; + +class OptionalIsPresent { + public void testOptional(Optional opt) { + if (opt == null) opt = Optional.empty(); + if (opt.isPresent()) { + // Changing type of `obj` from Object to String will cause a compile-time error on instanceof. Hence, don't suggest fix. + Object obj = opt.get(); + use(obj instanceof Integer ? "foo" : "bar"); + } + } + + void use(String obj) { System.out.println("String"); } +} diff --git a/java/java-tests/testData/inspection/optionalIsPresent/Optional.java b/java/java-tests/testData/inspection/optionalIsPresent/Optional.java new file mode 100644 index 000000000000..fb60df224f6d --- /dev/null +++ b/java/java-tests/testData/inspection/optionalIsPresent/Optional.java @@ -0,0 +1,13 @@ +import java.util.Optional; + +class OptionalIsPresent { + public void foo(Optional str) { + String val; + if (str.isEmpty()) { + val = ""; + } else { + val = str.get(); + } + System.out.println(val); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalIsPresentInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalIsPresentInspectionTest.java index e637234ceaa4..a984eea3c4e4 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalIsPresentInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalIsPresentInspectionTest.java @@ -1,4 +1,4 @@ -// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.java.codeInspection; import com.intellij.JavaTestUtil; @@ -27,5 +27,24 @@ public class OptionalIsPresentInspectionTest extends LightJavaInspectionTestCase return JAVA_11_ANNOTATED; } - public void testOptionalIsPresent() { doTest(); } + + public void testAssignment() { doTest(); } + + public void testConsumer() { doTest(); } + + public void testConsumerIndirect() { doTest(); } + + public void testConsumerIndirectAmbiguousOverload() { doTest(); } + + public void testConsumerIndirectComment() { doTest(); } + + public void testConsumerIndirectFieldInaccessible() { doTest(); } + + public void testConsumerIndirectTooComplex_1() { doTest(); } + + public void testConsumerIndirectTooComplex_2() { doTest(); } + + public void testConsumerIndirectTypeMismatch() { doTest(); } + + public void testOptional() { doTest(); } }