diff --git a/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java b/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java index 81dc20c800ee..5327524333c4 100644 --- a/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java +++ b/java/java-analysis-api/src/com/intellij/codeInsight/intention/QuickFixFactory.java @@ -60,6 +60,13 @@ public abstract class QuickFixFactory { @NotNull PsiType toReturn, boolean fixWholeHierarchy); + /** + * Provides fix for changing method return type when it's not clear with which type method return type should be replaced.
+ * For example, in case when void method returns null this fix should be used.
+ */
+ @NotNull
+ public abstract LocalQuickFixAndIntentionActionOnPsiElement createMethodReturnUnknownTypeFix(@NotNull PsiMethod method);
+
@NotNull
public abstract LocalQuickFixAndIntentionActionOnPsiElement createAddMethodFix(@NotNull PsiMethod method, @NotNull PsiClass toClass);
@@ -174,6 +181,16 @@ public abstract class QuickFixFactory {
@NotNull
public abstract IntentionAction createConvertToStringLiteralAction();
+ /**
+ * Provides fix to remove return statement or return value in case when return statement is not last statement in block.
+ * This fix won't be available if return value may have side effects.
+ *
+ * @param codeBlock code block with return statement
+ * @param statement return statement to remove
+ */
+ @NotNull
+ public abstract IntentionAction createDeleteReturnFix(@NotNull PsiCodeBlock codeBlock, @NotNull PsiReturnStatement statement);
+
@NotNull
public abstract IntentionAction createDeleteCatchFix(@NotNull PsiParameter parameter);
diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java
index f1dafbf4745e..880f727515f1 100644
--- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java
+++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java
@@ -559,11 +559,7 @@ public class HighlightUtil extends HighlightUtilBase {
if (returnValue != null) {
PsiType valueType = RefactoringChangeUtil.getTypeByExpression(returnValue);
if (isMethodVoid) {
- description = JavaErrorMessages.message("return.from.void.method");
- errorResult = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(statement).descriptionAndTooltip(description).create();
- if (valueType != null && method != null) {
- QuickFixAction.registerQuickFixAction(errorResult, QUICK_FIX_FACTORY.createMethodReturnFix(method, valueType, true));
- }
+ return generateReturnValueFromVoidMethodInfo(method, statement, valueType);
}
else {
TextRange textRange = statement.getTextRange();
@@ -593,6 +589,36 @@ public class HighlightUtil extends HighlightUtilBase {
return errorResult;
}
+ private static boolean isValidConstructor(@NotNull PsiMethod method) {
+ PsiClass aClass = method.getContainingClass();
+ if (aClass == null) return false;
+ return method.getName().equals(aClass.getName());
+ }
+
+ private static HighlightInfo generateReturnValueFromVoidMethodInfo(@Nullable PsiMethod method,
+ @NotNull PsiReturnStatement returnStatement,
+ @Nullable PsiType returnValueType) {
+ String description = JavaErrorMessages.message("return.from.void.method");
+ HighlightInfo info =
+ HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(returnStatement).descriptionAndTooltip(description).create();
+
+ boolean canCreateFixes = method != null && returnValueType != null && (!method.isConstructor() || isValidConstructor(method));
+ if (!canCreateFixes) return info;
+
+ PsiCodeBlock codeBlock = method.getBody();
+ if (codeBlock == null) return info;
+ QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createDeleteReturnFix(codeBlock, returnStatement));
+ if (method.isConstructor()) return info;
+ boolean isKnownReturnType = returnValueType.isValid() && !TypeConversionUtil.isNullType(returnValueType);
+ if (isKnownReturnType) {
+ QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createMethodReturnFix(method, returnValueType, true));
+ }
+ else {
+ QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createMethodReturnUnknownTypeFix(method));
+ }
+ return info;
+ }
+
@NotNull
public static String getUnhandledExceptionsDescriptor(@NotNull final Collection extends PsiClassType> unhandled) {
return getUnhandledExceptionsDescriptor(unhandled, null);
diff --git a/java/java-analysis-impl/src/messages/QuickFixBundle.properties b/java/java-analysis-impl/src/messages/QuickFixBundle.properties
index 0cc407f15bb3..7326e99cbb39 100644
--- a/java/java-analysis-impl/src/messages/QuickFixBundle.properties
+++ b/java/java-analysis-impl/src/messages/QuickFixBundle.properties
@@ -146,6 +146,7 @@ fix.parameter.type.family=Fix Parameter Type
fix.parameter.type.text=Make ''{0}'' take parameter of type ''{1}'' here
fix.return.type.family=Fix return type
fix.return.type.text=Make ''{0}'' return ''{1}''
+fix.return.type.change.type.text=Change return type for method ''{0}''
fix.throws.list.family=Fix throws list
fix.throws.list.add.exception=Add ''{0}'' to ''{1}'' throws list
fix.throws.list.remove.exception=Remove ''{0}'' from ''{1}'' throws list
@@ -311,6 +312,9 @@ wrap.with.optional.parameter.text=Wrap {0, choice, 1#1st|2#2nd|3#3rd|4#{0,number
wrap.with.optional.single.parameter.text=Wrap using 'java.util.Optional'
move.file.to.source.root.text=Move file to a source root
+delete.return.fix.family=Delete return
+delete.return.fix.statement.text=Delete return statement
+delete.return.fix.value.text=Delete return value ''{0}''
delete.element.fix.text=Delete element
delete.reference.fix.text=Delete reference
diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddReturnFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddReturnFix.java
index b60060eedeb3..a38733580367 100644
--- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddReturnFix.java
+++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddReturnFix.java
@@ -81,7 +81,7 @@ public class AddReturnFix implements IntentionAction {
assert body != null;
returnStatement = (PsiReturnStatement) body.addBefore(returnStatement, body.getRBrace());
- MethodReturnTypeFix.selectReturnValueInEditor(returnStatement, editor);
+ MethodReturnTypeFix.selectInEditor(returnStatement.getReturnValue(), editor);
}
private String suggestReturnValue() {
diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/DeleteReturnFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/DeleteReturnFix.java
new file mode 100644
index 000000000000..11e6e3875565
--- /dev/null
+++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/DeleteReturnFix.java
@@ -0,0 +1,69 @@
+// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.codeInsight.daemon.impl.quickfix;
+
+import com.intellij.codeInsight.daemon.QuickFixBundle;
+import com.intellij.codeInsight.intention.IntentionAction;
+import com.intellij.codeInsight.intention.impl.BaseIntentionAction;
+import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.*;
+import com.intellij.util.IncorrectOperationException;
+import com.siyeh.ig.psiutils.CommentTracker;
+import com.siyeh.ig.psiutils.ControlFlowUtils;
+import com.siyeh.ig.psiutils.SideEffectChecker;
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+
+public class DeleteReturnFix implements IntentionAction {
+
+ private final PsiReturnStatement myReturnStatement;
+ private final boolean myIsLastStatement;
+
+ @Contract(pure = true)
+ public DeleteReturnFix(@NotNull PsiCodeBlock codeBlock, @NotNull PsiReturnStatement statement) {
+ myReturnStatement = statement;
+ myIsLastStatement = ControlFlowUtils.blockCompletesWithStatement(codeBlock, statement);
+ }
+
+ @Nls(capitalization = Nls.Capitalization.Sentence)
+ @NotNull
+ @Override
+ public String getText() {
+ if (!myIsLastStatement) {
+ PsiExpression value = myReturnStatement.getReturnValue();
+ if (value != null) return QuickFixBundle.message("delete.return.fix.value.text", value.getText());
+ }
+ return QuickFixBundle.message("delete.return.fix.statement.text");
+ }
+
+ @Nls(capitalization = Nls.Capitalization.Sentence)
+ @NotNull
+ @Override
+ public String getFamilyName() {
+ return QuickFixBundle.message("delete.return.fix.family");
+ }
+
+ @Override
+ public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) {
+ if (!myReturnStatement.isValid() || !BaseIntentionAction.canModify(myReturnStatement)) return false;
+ PsiExpression returnValue = myReturnStatement.getReturnValue();
+ return returnValue == null || !SideEffectChecker.mayHaveSideEffects(returnValue);
+ }
+
+ @Override
+ public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException {
+ if (myIsLastStatement) {
+ new CommentTracker().deleteAndRestoreComments(myReturnStatement);
+ }
+ else {
+ PsiElement toDelete = myReturnStatement.getReturnValue();
+ new CommentTracker().deleteAndRestoreComments(toDelete == null ? myReturnStatement : toDelete);
+ }
+ }
+
+ @Override
+ public boolean startInWriteAction() {
+ return true;
+ }
+}
diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodReturnTypeFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodReturnTypeFix.java
index 21c5c92b3587..cef4fedeadcd 100644
--- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodReturnTypeFix.java
+++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/MethodReturnTypeFix.java
@@ -49,6 +49,14 @@ public class MethodReturnTypeFix extends LocalQuickFixAndIntentionActionOnPsiEle
private final String myName;
private final String myCanonicalText;
+ public MethodReturnTypeFix(@NotNull PsiMethod method) {
+ super(method);
+ myReturnTypePointer = null;
+ myFixWholeHierarchy = false;
+ myName = method.getName();
+ myCanonicalText = null;
+ }
+
public MethodReturnTypeFix(@NotNull PsiMethod method, @NotNull PsiType returnType, boolean fixWholeHierarchy) {
super(method);
myReturnTypePointer = SmartTypePointerManager.getInstance(method.getProject()).createSmartTypePointer(returnType);
@@ -67,7 +75,8 @@ public class MethodReturnTypeFix extends LocalQuickFixAndIntentionActionOnPsiEle
@NotNull
@Override
public String getText() {
- return QuickFixBundle.message("fix.return.type.text", myName, myCanonicalText);
+ return myCanonicalText == null ? QuickFixBundle.message("fix.return.type.change.type.text", myName) :
+ QuickFixBundle.message("fix.return.type.text", myName, myCanonicalText);
}
@Override
@@ -82,10 +91,10 @@ public class MethodReturnTypeFix extends LocalQuickFixAndIntentionActionOnPsiEle
@NotNull PsiElement startElement,
@NotNull PsiElement endElement) {
final PsiMethod myMethod = (PsiMethod)startElement;
-
+ if (!BaseIntentionAction.canModify(myMethod)) return false;
+ if (myReturnTypePointer == null) return myMethod.getReturnTypeElement() != null;
final PsiType myReturnType = myReturnTypePointer.getType();
- if (BaseIntentionAction.canModify(myMethod) &&
- myReturnType != null &&
+ if (myReturnType != null &&
myReturnType.isValid() &&
!TypeConversionUtil.isNullType(myReturnType)) {
final PsiType returnType = myMethod.getReturnType();
@@ -105,6 +114,11 @@ public class MethodReturnTypeFix extends LocalQuickFixAndIntentionActionOnPsiEle
final PsiMethod myMethod = (PsiMethod)startElement;
if (!FileModificationService.getInstance().prepareFileForWrite(myMethod.getContainingFile())) return;
+ if (myReturnTypePointer == null) {
+ Editor editorForMethod = getEditorForMethod(myMethod, project, editor, file);
+ if (editorForMethod != null) selectInEditor(myMethod.getReturnTypeElement(), editorForMethod);
+ return;
+ }
final PsiType myReturnType = myReturnTypePointer.getType();
if (myReturnType == null) return;
if (myFixWholeHierarchy) {
@@ -135,7 +149,7 @@ public class MethodReturnTypeFix extends LocalQuickFixAndIntentionActionOnPsiEle
if (statementToSelect != null) {
Editor editorForMethod = getEditorForMethod(myMethod, project, editor, file);
if (editorForMethod != null) {
- selectReturnValueInEditor(statementToSelect, editorForMethod);
+ selectInEditor(statementToSelect.getReturnValue(), editorForMethod);
}
}
}
@@ -308,10 +322,9 @@ public class MethodReturnTypeFix extends LocalQuickFixAndIntentionActionOnPsiEle
}
}
- static void selectReturnValueInEditor(final PsiReturnStatement returnStatement, final Editor editor) {
- final PsiExpression returnValue = returnStatement.getReturnValue();
- LOG.assertTrue(returnValue != null, returnStatement);
- TextRange range = returnValue.getTextRange();
+ static void selectInEditor(@Nullable PsiElement element, Editor editor) {
+ LOG.assertTrue(element != null);
+ TextRange range = element.getTextRange();
int offset = range.getStartOffset();
editor.getCaretModel().moveToOffset(offset);
diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java
index f7358ea52bb0..6d5385682f8a 100644
--- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java
+++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java
@@ -84,6 +84,12 @@ public class QuickFixFactoryImpl extends QuickFixFactory {
return new MethodReturnTypeFix(method, toReturn, fixWholeHierarchy);
}
+ @NotNull
+ @Override
+ public LocalQuickFixAndIntentionActionOnPsiElement createMethodReturnUnknownTypeFix(@NotNull PsiMethod method) {
+ return new MethodReturnTypeFix(method);
+ }
+
@NotNull
@Override
public LocalQuickFixAndIntentionActionOnPsiElement createAddMethodFix(@NotNull PsiMethod method, @NotNull PsiClass toClass) {
@@ -269,6 +275,12 @@ public class QuickFixFactoryImpl extends QuickFixFactory {
return new ConvertToStringLiteralAction();
}
+ @NotNull
+ @Override
+ public IntentionAction createDeleteReturnFix(@NotNull PsiCodeBlock codeBlock, @NotNull PsiReturnStatement statement) {
+ return new DeleteReturnFix(codeBlock, statement);
+ }
+
@NotNull
@Override
public IntentionAction createDeleteCatchFix(@NotNull PsiParameter parameter) {
diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterKnownTypeLastStatement.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterKnownTypeLastStatement.java
new file mode 100644
index 000000000000..2b7bd60e463d
--- /dev/null
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterKnownTypeLastStatement.java
@@ -0,0 +1,7 @@
+// "Delete return statement" "true"
+
+class Test {
+
+ void foo(boolean b) {
+ }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterKnownTypeUsefulReturn.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterKnownTypeUsefulReturn.java
new file mode 100644
index 000000000000..3737b0725382
--- /dev/null
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterKnownTypeUsefulReturn.java
@@ -0,0 +1,9 @@
+// "Delete return value '"foo"'" "true"
+
+class Test {
+
+ void foo(boolean b) {
+ if (b) return;
+ System.out.println("bar");
+ }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterUnknownTypeLastStatement.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterUnknownTypeLastStatement.java
new file mode 100644
index 000000000000..2b7bd60e463d
--- /dev/null
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterUnknownTypeLastStatement.java
@@ -0,0 +1,7 @@
+// "Delete return statement" "true"
+
+class Test {
+
+ void foo(boolean b) {
+ }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterUnknownTypeUsefulReturn.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterUnknownTypeUsefulReturn.java
new file mode 100644
index 000000000000..fc07cd799810
--- /dev/null
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/afterUnknownTypeUsefulReturn.java
@@ -0,0 +1,9 @@
+// "Delete return value 'null'" "true"
+
+class Test {
+
+ void foo(boolean b) {
+ if (b) return;
+ System.out.println("bar");
+ }
+}
\ No newline at end of file
diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/beforeKnownTypeLastStatement.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/beforeKnownTypeLastStatement.java
new file mode 100644
index 000000000000..368e319817f5
--- /dev/null
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteReturn/beforeKnownTypeLastStatement.java
@@ -0,0 +1,8 @@
+// "Delete return statement" "true"
+
+class Test {
+
+ void foo(boolean b) {
+ return