IDEA-62374 Suggest ternary operation to avoid NPE implemented

This commit is contained in:
Danila Ponomarenko
2012-05-28 13:38:20 +04:00
parent ced3ee94b3
commit c152ac976f
12 changed files with 271 additions and 48 deletions

View File

@@ -0,0 +1,108 @@
/*
* Copyright 2000-2012 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.codeInspection;
import com.intellij.codeInsight.CodeInsightUtilBase;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.editor.ScrollType;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.util.PsiTypesUtil;
import com.intellij.psi.util.PsiUtilBase;
import org.jetbrains.annotations.NotNull;
/**
* @author Danila Ponomarenko
*/
public class ReplaceWithTernaryOperatorFix implements LocalQuickFix {
private final String myText;
@NotNull
public String getName() {
return InspectionsBundle.message("inspection.replace.ternary.quickfix", myText);
}
public ReplaceWithTernaryOperatorFix(@NotNull PsiExpression expressionToAssert) {
myText = expressionToAssert.getText();
}
@NotNull
@Override
public String getFamilyName() {
return InspectionsBundle.message("inspection.surround.if.family");
}
@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
final PsiElement element = descriptor.getPsiElement();
if (!(element instanceof PsiExpression)) {
return;
}
final PsiExpression expression = (PsiExpression)element;
final PsiFile file = expression.getContainingFile();
if (!CodeInsightUtilBase.prepareFileForWrite(file)) return;
final PsiConditionalExpression conditionalExpression = replaceWthConditionalExpression(project, myText + "!=null", expression, suggestDefaultValue(expression));
final PsiExpression elseExpression = conditionalExpression.getElseExpression();
if (elseExpression != null) {
selectInEditor(elseExpression);
}
}
private static void selectInEditor(@NotNull PsiElement element) {
final Editor editor = PsiUtilBase.findEditor(element);
if (editor == null) return;
final TextRange expressionRange = element.getTextRange();
editor.getCaretModel().moveToOffset(expressionRange.getStartOffset());
editor.getScrollingModel().scrollToCaret(ScrollType.RELATIVE);
editor.getSelectionModel().setSelection(expressionRange.getStartOffset(), expressionRange.getEndOffset());
}
@NotNull
private static PsiConditionalExpression replaceWthConditionalExpression(@NotNull Project project,
@NotNull String condition,
@NotNull PsiExpression expression,
@NotNull String defaultValue) {
final PsiElementFactory factory = JavaPsiFacade.getInstance(project).getElementFactory();
final PsiElement parent = expression.getParent();
final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)factory.createExpressionFromText(
condition + " ? " + expression.getText() + " : " + defaultValue,
parent
);
final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project);
codeStyleManager.reformat(conditionalExpression);
return (PsiConditionalExpression)expression.replace(conditionalExpression);
}
public static boolean isAvailable(@NotNull PsiExpression qualifier, @NotNull PsiExpression expression) {
if (!qualifier.isValid() || qualifier.getText() == null) {
return false;
}
return !(expression.getParent() instanceof PsiExpressionStatement);
}
private static String suggestDefaultValue(@NotNull PsiExpression expression) {
PsiType type = expression.getType();
return PsiTypesUtil.getDefaultValueOfType(type);
}
}

View File

@@ -81,8 +81,8 @@ public class SurroundWithIfFix implements LocalQuickFix {
return InspectionsBundle.message("inspection.surround.if.family");
}
public boolean isAvailable(PsiExpression qualifier) {
if (!qualifier.isValid() || myText == null) {
public static boolean isAvailable(PsiExpression qualifier) {
if (!qualifier.isValid() || qualifier.getText() == null) {
return false;
}
PsiStatement statement = PsiTreeUtil.getParentOfType(qualifier, PsiStatement.class);

View File

@@ -68,20 +68,28 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
return new OptionsPanel();
}
void test(@NotNull List l) {
final List list = null;
test(list);
}
@NotNull
public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, boolean isOnTheFly) {
return new JavaElementVisitor() {
@Override public void visitField(PsiField field) {
@Override
public void visitField(PsiField field) {
if (isNullLiteralExpression(field.getInitializer()) && NullableNotNullManager.isNotNull(field)) {
holder.registerProblem(field.getInitializer(), InspectionsBundle.message("dataflow.message.initializing.field.with.null"));
}
}
@Override public void visitMethod(PsiMethod method) {
@Override
public void visitMethod(PsiMethod method) {
analyzeCodeBlock(method.getBody(), holder);
}
@Override public void visitClassInitializer(PsiClassInitializer initializer) {
@Override
public void visitClassInitializer(PsiClassInitializer initializer) {
analyzeCodeBlock(initializer.getBody(), holder);
}
};
@@ -109,35 +117,38 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
}
@Nullable
private static LocalQuickFix[] createNPEFixes(PsiExpression qualifier) {
if (qualifier != null &&
!(qualifier instanceof PsiMethodCallExpression) &&
!(qualifier instanceof PsiLiteralExpression && ((PsiLiteralExpression)qualifier).getValue() == null)) {
try {
PsiBinaryExpression binary = (PsiBinaryExpression)JavaPsiFacade.getInstance(qualifier.getProject()).getElementFactory()
.createExpressionFromText("a != null", null);
binary.getLOperand().replace(qualifier);
List<LocalQuickFix> fixes = new SmartList<LocalQuickFix>();
private static LocalQuickFix[] createNPEFixes(PsiExpression qualifier, PsiExpression expression) {
if (qualifier == null || expression == null) return null;
if (qualifier instanceof PsiMethodCallExpression) return null;
if (qualifier instanceof PsiLiteralExpression && ((PsiLiteralExpression)qualifier).getValue() == null) return null;
if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_4)) {
fixes.add(new AddAssertStatementFix(binary));
}
SurroundWithIfFix ifFix = new SurroundWithIfFix(qualifier);
if (ifFix.isAvailable(qualifier)) {
fixes.add(ifFix);
}
return fixes.toArray(new LocalQuickFix[fixes.size()]);
try {
final List<LocalQuickFix> fixes = new SmartList<LocalQuickFix>();
if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_4)) {
final Project project = qualifier.getProject();
final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory();
final PsiBinaryExpression binary = (PsiBinaryExpression)elementFactory.createExpressionFromText("a != null", null);
binary.getLOperand().replace(qualifier);
fixes.add(new AddAssertStatementFix(binary));
}
catch (IncorrectOperationException e) {
LOG.error(e);
return null;
if (SurroundWithIfFix.isAvailable(qualifier)) {
fixes.add(new SurroundWithIfFix(qualifier));
}
if (ReplaceWithTernaryOperatorFix.isAvailable(qualifier, expression)) {
fixes.add(new ReplaceWithTernaryOperatorFix(qualifier));
}
return fixes.toArray(new LocalQuickFix[fixes.size()]);
}
catch (IncorrectOperationException e) {
LOG.error(e);
return null;
}
return null;
}
private void createDescription(StandardDataFlowRunner runner, ProblemsHolder holder, StandardInstructionVisitor visitor) {
Pair<Set<Instruction>,Set<Instruction>> constConditions = runner.getConstConditionalExpressions();
Pair<Set<Instruction>, Set<Instruction>> constConditions = runner.getConstConditionalExpressions();
Set<Instruction> trueSet = constConditions.getFirst();
Set<Instruction> falseSet = constConditions.getSecond();
Set<Instruction> npeSet = runner.getNPEInstructions();
@@ -164,7 +175,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
MethodCallInstruction mcInstruction = (MethodCallInstruction)instruction;
if (mcInstruction.getCallExpression() instanceof PsiMethodCallExpression) {
PsiMethodCallExpression callExpression = (PsiMethodCallExpression)mcInstruction.getCallExpression();
LocalQuickFix[] fix = createNPEFixes(callExpression.getMethodExpression().getQualifierExpression());
LocalQuickFix[] fix = createNPEFixes(callExpression.getMethodExpression().getQualifierExpression(), callExpression);
holder.registerProblem(callExpression,
InspectionsBundle.message("dataflow.message.npe.method.invocation"),
@@ -176,13 +187,13 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
PsiElement elementToAssert = frInstruction.getElementToAssert();
PsiExpression expression = frInstruction.getExpression();
if (expression instanceof PsiArrayAccessExpression) {
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert);
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert, expression);
holder.registerProblem(expression,
InspectionsBundle.message("dataflow.message.npe.array.access"),
fix);
}
else {
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert);
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert, expression);
holder.registerProblem(elementToAssert,
InspectionsBundle.message("dataflow.message.npe.field.access"),
fix);
@@ -207,7 +218,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, true);
holder.registerProblem(psiAnchor,
InspectionsBundle.message(underBinary ? "dataflow.message.constant.condition.whenriched" : "dataflow.message.constant.condition", Boolean.toString(true)),
localQuickFix==null?null:new LocalQuickFix[]{localQuickFix});
localQuickFix == null ? null : new LocalQuickFix[]{localQuickFix});
}
}
else if (psiAnchor instanceof PsiSwitchLabelStatement) {
@@ -228,7 +239,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue);
holder.registerProblem(psiAnchor, InspectionsBundle.message(underBinary ? "dataflow.message.constant.condition.whenriched" : "dataflow.message.constant.condition",
Boolean.toString(evaluatesToTrue)),
localQuickFix == null ? null : new LocalQuickFix[]{localQuickFix});
localQuickFix == null ? null : new LocalQuickFix[]{localQuickFix});
}
}
reportedAnchors.add(psiAnchor);
@@ -241,15 +252,15 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
final String text = isNullLiteralExpression(expr)
? InspectionsBundle.message("dataflow.message.passing.null.argument")
: InspectionsBundle.message("dataflow.message.passing.nullable.argument");
LocalQuickFix[] fixes = createNPEFixes(expr);
LocalQuickFix[] fixes = createNPEFixes(expr, expr);
holder.registerProblem(expr, text, fixes);
}
exprs = runner.getNullableAssignments();
for (PsiExpression expr : exprs) {
final String text = isNullLiteralExpression(expr)
? InspectionsBundle.message("dataflow.message.assigning.null")
: InspectionsBundle.message("dataflow.message.assigning.nullable");
? InspectionsBundle.message("dataflow.message.assigning.null")
: InspectionsBundle.message("dataflow.message.assigning.nullable");
holder.registerProblem(expr, text);
}
@@ -263,17 +274,16 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
final PsiExpression expr = statement.getReturnValue();
if (runner.isInNotNullMethod()) {
final String text = isNullLiteralExpression(expr)
? InspectionsBundle.message("dataflow.message.return.null.from.notnull")
: InspectionsBundle.message("dataflow.message.return.nullable.from.notnull");
? InspectionsBundle.message("dataflow.message.return.null.from.notnull")
: InspectionsBundle.message("dataflow.message.return.nullable.from.notnull");
holder.registerProblem(expr, text);
}
else if (AnnotationUtil.isAnnotatingApplicable(statement)) {
final String text = isNullLiteralExpression(expr)
? InspectionsBundle.message("dataflow.message.return.null.from.notnullable")
: InspectionsBundle.message("dataflow.message.return.nullable.from.notnullable");
? InspectionsBundle.message("dataflow.message.return.null.from.notnullable")
: InspectionsBundle.message("dataflow.message.return.nullable.from.notnullable");
final NullableNotNullManager manager = NullableNotNullManager.getInstance(expr.getProject());
holder.registerProblem(expr, text, new AnnotateMethodFix(manager.getDefaultNullable(), ArrayUtil.toStringArray(manager.getNotNulls())));
holder.registerProblem(expr, text, new AnnotateMethodFix(manager.getDefaultNullable(), ArrayUtil.toStringArray(manager.getNotNulls())));
}
}
}
@@ -343,7 +353,7 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
final PsiElement psiElement = descriptor.getPsiElement();
if (psiElement == null) return;
final SimplifyBooleanExpressionFix fix = createIntention(psiElement, value);
if (fix==null) return;
if (fix == null) return;
try {
LOG.assertTrue(psiElement.isValid());
fix.invoke(project, null, psiElement.getContainingFile());
@@ -480,8 +490,8 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
private static class DataFlowInstructionVisitor extends StandardInstructionVisitor {
protected void onAssigningToNotNullableVariable(AssignInstruction instruction, DataFlowRunner runner) {
((StandardDataFlowRunner)runner).onAssigningToNotNullableVariable(instruction.getRExpression());
}
((StandardDataFlowRunner)runner).onAssigningToNotNullableVariable(instruction.getRExpression());
}
protected void onNullableReturn(CheckReturnValueInstruction instruction, DataFlowRunner runner) {
((StandardDataFlowRunner)runner).onNullableReturn(instruction.getReturn());
@@ -496,15 +506,15 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
}
protected void onInstructionProducesNPE(MethodCallInstruction instruction, DataFlowRunner runner) {
((StandardDataFlowRunner) runner).onInstructionProducesNPE(instruction);
((StandardDataFlowRunner)runner).onInstructionProducesNPE(instruction);
}
protected void onUnboxingNullable(MethodCallInstruction instruction, DataFlowRunner runner) {
((StandardDataFlowRunner) runner).onUnboxingNullable(instruction.getContext());
((StandardDataFlowRunner)runner).onUnboxingNullable(instruction.getContext());
}
protected void onPassingNullParameter(DataFlowRunner runner, PsiExpression arg) {
((StandardDataFlowRunner) runner).onPassingNullParameter(arg); // Parameters on stack are reverted.
}
((StandardDataFlowRunner)runner).onPassingNullParameter(arg); // Parameters on stack are reverted.
}
}
}

View File

@@ -0,0 +1,10 @@
// "Replace with 'list != null ?:'" "true"
import org.jetbrains.annotations.NotNull;
class A{
void test(@NotNull List l) {
final List list = null;
test(list != null ? list : <selection>null</selection>);
}
}

View File

@@ -0,0 +1,10 @@
// "Replace with 'integer != null ?:'" "true"
import java.lang.Integer;
class A{
void test(){
Integer integer = null;
int i = integer != null ? integer.intValue() : <selection>0</selection>;
}
}

View File

@@ -0,0 +1,8 @@
// "Replace with 'list != null ?:'" "true"
class A{
void test(){
List list = null;
Object o = list != null ? list.get(0) : <selection>null</selection>;
}
}

View File

@@ -0,0 +1,7 @@
// "Replace with 'list != null ?:'" "false"
class A{
void test(){
List list = null;
li<caret>st.get(0);
}
}

View File

@@ -0,0 +1,10 @@
// "Replace with 'list != null ?:'" "true"
import org.jetbrains.annotations.NotNull;
class A{
void test(@NotNull List l) {
final List list = null;
test(li<caret>st);
}
}

View File

@@ -0,0 +1,10 @@
// "Replace with 'integer != null ?:'" "true"
import java.lang.Integer;
class A{
void test(){
Integer integer = null;
int i = in<caret>teger.intValue();
}
}

View File

@@ -0,0 +1,8 @@
// "Replace with 'list != null ?:'" "true"
class A{
void test(){
List list = null;
Object o = li<caret>st.get(0);
}
}

View File

@@ -0,0 +1,41 @@
/*
* Copyright 2000-2012 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.
*/
/*
* User: anna
* Date: 21-Mar-2008
*/
package com.intellij.codeInsight.daemon.quickFix;
import com.intellij.codeInspection.LocalInspectionTool;
import com.intellij.codeInspection.dataFlow.DataFlowInspection;
import com.intellij.codeInspection.nullable.NullableStuffInspection;
public class ReplaceWithTernaryOperatorTest extends LightQuickFixTestCase {
@Override
protected LocalInspectionTool[] configureLocalInspectionTools() {
return new LocalInspectionTool[]{new DataFlowInspection(), new NullableStuffInspection()};
}
public void test() throws Exception {
doAllTests();
}
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator";
}
}

View File

@@ -274,6 +274,7 @@ inspection.problem.resolution=Problem resolution
inspection.quickfix.assert.family=Assert
inspection.assert.quickfix=Assert ''{0}''
inspection.surround.if.quickfix=Surround with ''if ({0} != null)''
inspection.replace.ternary.quickfix=Replace with ''{0} != null ?:''
inspection.surround.if.family=Surround with if
inspection.dependency.configure.button.text=Configure dependency rules