[java] Improve quick-fixes on expression statements:

1. Put 'Iterate' and 'Introduce local variable' above 'Insert semicolon' where possible
2. Add 'Insert return' fix where possible (IDEA-267682)
3. Display type in 'Iterate' fix name (IDEA-267796)

GitOrigin-RevId: 08aa00adc0f7f1144c306a5dfdd279319b0af2ca
This commit is contained in:
Tagir Valeev
2021-04-23 15:04:02 +07:00
committed by intellij-monorepo-bot
parent c8e020b146
commit 62f9692d64
26 changed files with 231 additions and 45 deletions

View File

@@ -528,4 +528,8 @@ public abstract class QuickFixFactory {
public abstract @NotNull IntentionAction createUnwrapArrayInitializerMemberValueAction(@NotNull PsiArrayInitializerMemberValue arrayValue);
public abstract @NotNull IntentionAction createIntroduceVariableAction(@NotNull PsiExpression expression);
public abstract @NotNull IntentionAction createInsertReturnFix(@NotNull PsiExpression expression);
public abstract @NotNull IntentionAction createIterateFix(@NotNull PsiExpression expression);
}

View File

@@ -22,6 +22,7 @@ import com.intellij.psi.controlFlow.*;
import com.intellij.psi.util.*;
import com.intellij.util.ArrayUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.SwitchUtils;
import com.siyeh.ig.psiutils.VariableAccessUtils;
@@ -398,4 +399,29 @@ public final class HighlightFixUtil {
);
}
}
static void registerFixesForExpressionStatement(HighlightInfo info, PsiElement statement) {
if (info == null) return;
if (!(statement instanceof PsiExpressionStatement)) return;
PsiCodeBlock block = ObjectUtils.tryCast(statement.getParent(), PsiCodeBlock.class);
if (block == null) return;
PsiExpression expression = ((PsiExpressionStatement)statement).getExpression();
if (expression instanceof PsiAssignmentExpression) return;
PsiType type = expression.getType();
if (type == null) return;
if (!type.equals(PsiType.VOID)) {
QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createIntroduceVariableAction(expression));
QuickFixAction.registerQuickFixAction(info, PriorityIntentionActionWrapper
.highPriority(QUICK_FIX_FACTORY.createIterateFix(expression)));
}
if (PsiTreeUtil.skipWhitespacesAndCommentsForward(statement) == block.getRBrace()) {
PsiElement blockParent = block.getParent();
if (blockParent instanceof PsiMethod) {
PsiType returnType = ((PsiMethod)blockParent).getReturnType();
if (returnType != null && returnType.isAssignableFrom(type)) {
QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createInsertReturnFix(expression));
}
}
}
}
}

View File

@@ -1566,12 +1566,9 @@ public final class HighlightUtil {
HighlightInfo error =
HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(anchor).descriptionAndTooltip(description).create();
if (statement instanceof PsiExpressionStatement) {
PsiExpression expression = ((PsiExpressionStatement)statement).getExpression();
if (statement.getParent() instanceof PsiCodeBlock) {
QuickFixAction.registerQuickFixAction(error, PriorityIntentionActionWrapper
.highPriority(getFixFactory().createIntroduceVariableAction(expression)));
}
QuickFixAction.registerQuickFixAction(error, getFixFactory().createDeleteSideEffectAwareFix((PsiExpressionStatement)statement));
HighlightFixUtil.registerFixesForExpressionStatement(error, statement);
QuickFixAction.registerQuickFixAction(error, PriorityIntentionActionWrapper
.lowPriority(getFixFactory().createDeleteSideEffectAwareFix((PsiExpressionStatement)statement)));
}
return error;
}

View File

@@ -18,6 +18,7 @@ public class JavaErrorQuickFixProvider implements ErrorQuickFixProvider {
String description = errorElement.getErrorDescription();
if (description.equals(JavaPsiBundle.message("expected.semicolon"))) {
QuickFixAction.registerQuickFixAction(info, new InsertMissingTokenFix(";"));
HighlightFixUtil.registerFixesForExpressionStatement(info, parent);
}
if (parent instanceof PsiTryStatement && description.equals(JavaPsiBundle.message("expected.catch.or.finally"))) {
QuickFixAction.registerQuickFixAction(info, new AddExceptionToCatchFix(false));

View File

@@ -0,0 +1,44 @@
// Copyright 2000-2021 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.impl.actions.IntentionActionWithFixAllOption;
import com.intellij.codeInsight.intention.HighPriorityAction;
import com.intellij.codeInspection.CommonQuickFixBundle;
import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiFile;
import com.intellij.psi.PsiKeyword;
import com.siyeh.ig.psiutils.CommentTracker;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class ConvertExpressionToReturnFix extends LocalQuickFixAndIntentionActionOnPsiElement
implements IntentionActionWithFixAllOption, HighPriorityAction {
public ConvertExpressionToReturnFix(@NotNull PsiExpression expression) {
super(expression);
}
@Override
public void invoke(@NotNull Project project,
@NotNull PsiFile file,
@Nullable Editor editor,
@NotNull PsiElement startElement,
@NotNull PsiElement endElement) {
PsiExpression expression = (PsiExpression)startElement;
CommentTracker tracker = new CommentTracker();
tracker.replaceAndRestoreComments(expression.getParent(), "return " + tracker.text(expression) + ";");
}
@Override
public @NotNull String getText() {
return getFamilyName();
}
@Override
public @NotNull String getFamilyName() {
return CommonQuickFixBundle.message("fix.insert.x", PsiKeyword.RETURN);
}
}

View File

@@ -2,6 +2,7 @@
package com.intellij.codeInsight.daemon.impl.quickfix;
import com.intellij.codeInsight.daemon.impl.actions.IntentionActionWithFixAllOption;
import com.intellij.codeInsight.intention.LowPriorityAction;
import com.intellij.ide.IdeBundle;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
@@ -9,7 +10,7 @@ import com.intellij.psi.PsiFile;
import com.intellij.util.IncorrectOperationException;
import org.jetbrains.annotations.NotNull;
public class InsertMissingTokenFix implements IntentionActionWithFixAllOption {
public class InsertMissingTokenFix implements IntentionActionWithFixAllOption, LowPriorityAction {
private final String myToken;
public InsertMissingTokenFix(String token) {

View File

@@ -21,6 +21,8 @@ import com.intellij.codeInsight.template.impl.InvokeTemplateAction;
import com.intellij.codeInsight.template.impl.TemplateImpl;
import com.intellij.codeInsight.template.impl.TemplateManagerImpl;
import com.intellij.codeInsight.template.impl.TemplateSettings;
import com.intellij.codeInspection.util.IntentionName;
import com.intellij.java.JavaBundle;
import com.intellij.openapi.application.WriteAction;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.editor.Editor;
@@ -37,36 +39,55 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.HashSet;
import java.util.Objects;
public class IterateOverIterableIntention implements IntentionAction {
private static final Logger LOG = Logger.getInstance(IterateOverIterableIntention.class);
private final @Nullable PsiExpression myExpression;
private @IntentionName String myText;
public IterateOverIterableIntention() {
this(null);
}
public IterateOverIterableIntention(@Nullable PsiExpression expression) {
myExpression = expression;
}
@Override
public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) {
final TemplateImpl template = file instanceof PsiJavaFile ? getTemplate() : null;
if (template != null) {
int offset = editor.getCaretModel().getOffset();
int startOffset = offset;
if (editor.getSelectionModel().hasSelection()) {
final int selStart = editor.getSelectionModel().getSelectionStart();
final int selEnd = editor.getSelectionModel().getSelectionEnd();
startOffset = (offset == selStart) ? selEnd : selStart;
}
PsiElement element = file.findElementAt(startOffset);
while (element instanceof PsiWhiteSpace) {
element = element.getPrevSibling();
}
PsiStatement psiStatement = PsiTreeUtil.getParentOfType(element, PsiStatement.class, false);
if (psiStatement != null) {
startOffset = psiStatement.getTextRange().getStartOffset();
}
if (!template.isDeactivated() &&
(TemplateManagerImpl.isApplicable(file, offset, template) ||
(TemplateManagerImpl.isApplicable(file, startOffset, template)))) {
return getIterableExpression(editor, file) != null;
}
if (template == null) return false;
int offset = editor.getCaretModel().getOffset();
int startOffset = offset;
if (editor.getSelectionModel().hasSelection()) {
final int selStart = editor.getSelectionModel().getSelectionStart();
final int selEnd = editor.getSelectionModel().getSelectionEnd();
startOffset = (offset == selStart) ? selEnd : selStart;
}
return false;
PsiElement element = file.findElementAt(startOffset);
while (element instanceof PsiWhiteSpace) {
element = element.getPrevSibling();
}
PsiStatement psiStatement = PsiTreeUtil.getParentOfType(element, PsiStatement.class, false);
if (myExpression == null && psiStatement instanceof PsiExpressionStatement &&
startOffset == offset && startOffset == psiStatement.getTextRange().getEndOffset() &&
psiStatement.getLastChild() instanceof PsiErrorElement) {
// At this position, we provide a quick-fix for error
return false;
}
if (psiStatement != null) {
startOffset = psiStatement.getTextRange().getStartOffset();
}
if (template.isDeactivated() ||
(!TemplateManagerImpl.isApplicable(file, offset, template) &&
(!TemplateManagerImpl.isApplicable(file, startOffset, template)))) {
return false;
}
PsiExpression expression = getIterableExpression(editor, file);
if (expression == null) return false;
myText = JavaBundle.message("intention.name.iterate.over", Objects.requireNonNull(expression.getType()).getPresentableText());
return true;
}
@Nullable
@@ -74,15 +95,20 @@ public class IterateOverIterableIntention implements IntentionAction {
return TemplateSettings.getInstance().getTemplate("I", "Java");
}
@NotNull
@Override
public String getText() {
return QuickFixBundle.message("iterate.iterable");
return myText == null ? getFamilyName() : myText;
}
@Nullable
private static PsiExpression getIterableExpression(Editor editor, PsiFile file) {
private PsiExpression getIterableExpression(Editor editor, PsiFile file) {
if (myExpression != null) {
if (!myExpression.isValid()) return null;
final PsiType type = myExpression.getType();
if (type instanceof PsiArrayType || InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_LANG_ITERABLE)) return myExpression;
return null;
}
final SelectionModel selectionModel = editor.getSelectionModel();
if (selectionModel.hasSelection()) {
PsiElement elementAtStart = file.findElementAt(selectionModel.getSelectionStart());
@@ -150,6 +176,6 @@ public class IterateOverIterableIntention implements IntentionAction {
@NotNull
@Override
public String getFamilyName() {
return getText();
return QuickFixBundle.message("iterate.iterable");
}
}

View File

@@ -1,7 +1,6 @@
// Copyright 2000-2021 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.intention.impl;
import com.intellij.codeInsight.intention.HighPriorityAction;
import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement;
import com.intellij.java.JavaBundle;
import com.intellij.openapi.editor.Editor;
@@ -13,7 +12,7 @@ import com.intellij.refactoring.introduceVariable.IntroduceVariableHandler;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class IntroduceVariableErrorFixAction extends LocalQuickFixAndIntentionActionOnPsiElement implements HighPriorityAction {
public class IntroduceVariableErrorFixAction extends LocalQuickFixAndIntentionActionOnPsiElement {
public IntroduceVariableErrorFixAction(@NotNull PsiExpression expression) {
super(expression);
}

View File

@@ -61,12 +61,16 @@ public class IntroduceVariableIntentionAction extends BaseRefactoringIntentionAc
return false;
}
if (expression.getParent() instanceof PsiExpressionStatement &&
!PsiUtil.isStatement(expression.getParent())) {
// Same action is available as an error quick-fix
return false;
if (expression.getParent() instanceof PsiExpressionStatement) {
if (!PsiUtil.isStatement(expression.getParent()) ||
expression.getParent().getLastChild() instanceof PsiErrorElement &&
editor.getCaretModel().getOffset() == expression.getParent().getTextRange().getEndOffset()) {
// Same action is available as an error quick-fix
return false;
}
}
final PsiType expressionType = expression.getType();
return expressionType != null && !PsiType.VOID.equals(expressionType) && !(expression instanceof PsiAssignmentExpression);
}

View File

@@ -1034,4 +1034,14 @@ public final class QuickFixFactoryImpl extends QuickFixFactory {
public @NotNull IntentionAction createIntroduceVariableAction(@NotNull PsiExpression expression) {
return new IntroduceVariableErrorFixAction(expression);
}
@Override
public @NotNull IntentionAction createInsertReturnFix(@NotNull PsiExpression expression) {
return new ConvertExpressionToReturnFix(expression);
}
@Override
public @NotNull IntentionAction createIterateFix(@NotNull PsiExpression expression) {
return new IterateOverIterableIntention(expression);
}
}

View File

@@ -0,0 +1,6 @@
// "Insert 'return'" "true"
class Test {
int x() {
return 2 + 2;
}
}

View File

@@ -0,0 +1,6 @@
// "Insert 'return'" "true"
class Test {
int x(int y) {
return Math.abs(y);
}
}

View File

@@ -0,0 +1,6 @@
// "Insert 'return'" "false"
class Test {
int x() {
int x = 2+2<caret>
}
}

View File

@@ -0,0 +1,7 @@
// "Insert 'return'" "false"
class Test {
int x() {
2+2<caret>
3+3
}
}

View File

@@ -0,0 +1,6 @@
// "Insert 'return'" "false"
class Test {
int x() {
" xyz ".trim()<caret>
}
}

View File

@@ -0,0 +1,6 @@
// "Insert 'return'" "true"
class Test {
int x() {
2+2<caret>
}
}

View File

@@ -0,0 +1,6 @@
// "Insert 'return'" "true"
class Test {
int x(int y) {
Math.abs(y)<caret>
}
}

View File

@@ -1,6 +1,6 @@
import java.lang.annotation.Annotation;
// "Iterate" "true"
// "Iterate over Annotation[]" "true"
class Test {
void foo() {
for (Annotation annotation : getClass().getAnnotations()) {

View File

@@ -1,6 +1,6 @@
import java.lang.annotation.Annotation;
// "Iterate" "true"
// "Iterate over Annotation[]" "true"
class Test {
void foo() {
for (Annotation annotation : getClass().getAnnotations()) {

View File

@@ -1,4 +1,4 @@
// "Iterate" "true"
// "Iterate over Annotation[]" "true"
class Test {
void foo() {
getClass().getAnnotatio<caret>ns();

View File

@@ -1,4 +1,4 @@
// "Iterate" "true"
// "Iterate over Annotation[]" "true"
class Test {
void foo() {
getClass().getAnnotations()<caret>

View File

@@ -1,4 +1,4 @@
// "Iterate" "false"
// "Disable 'Iterate'" "false"
class Test {
void foo() {
final Annotation[] annotations = getClass().getAnnotat<caret>ions();

View File

@@ -1,4 +1,4 @@
// "Iterate" "false"
// "Disable 'Iterate'" "false"
class Test {
void foo() {
java.lang.thi<caret>s

View File

@@ -0,0 +1,28 @@
/*
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.intellij.java.codeInsight.daemon.quickFix;
import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
public class InsertReturnFixTest extends LightQuickFixParameterizedTestCase {
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/insertReturn";
}
}

View File

@@ -1600,3 +1600,4 @@ link.configure.classes.excluded.from.completion=Configure classes excluded from
exception.navigation.fetching.target.position=Fetching target position
inspection.preview.feature.0.is.preview.api.message={0} is a preview API and may be removed in a future release
progress.title.detect.overridden.methods=Check overriding methods
intention.name.iterate.over=Iterate over {0}

View File

@@ -28,3 +28,5 @@ fix.simplify=Simplify
fix.create.title=Create {0}
fix.create.title.x=Create {0} ''{1}''
fix.insert.x=Insert ''{0}''