From d6f04c17253e673ca4b2d6ffbc8815bd995b0c5a Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 9 Feb 2018 15:50:58 +0700 Subject: [PATCH] ContractInspection: warn about mutability violations (part of IDEA-182125) --- .../dataFlow/ContractInspection.java | 19 ++++++++ .../dataFlow/MutationSignature.java | 18 ++++++-- .../MutationSignatureProblems.java | 43 +++++++++++++++++++ .../codeInspection/ContractCheckTest.java | 17 +------- 4 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/contractCheck/MutationSignatureProblems.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java index 40f8d8dba6cf..dd6900de02cc 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractInspection.java @@ -50,6 +50,25 @@ public class ContractInspection extends AbstractBaseJavaLocalInspectionTool { holder.registerProblem(value, error); } } + checkMutationContract(annotation, method); + } + + private void checkMutationContract(PsiAnnotation annotation, PsiMethod method) { + String mutationContract = AnnotationUtil.getStringAttributeValue(annotation, MutationSignature.ATTR_MUTATES); + if (StringUtil.isNotEmpty(mutationContract)) { + boolean pure = Boolean.TRUE.equals(AnnotationUtil.getBooleanAttributeValue(annotation, "pure")); + String error; + if (pure) { + error = "Pure method cannot have mutation contract"; + } else { + error = MutationSignature.checkSignature(mutationContract, method); + } + if (error != null) { + PsiAnnotationMemberValue value = annotation.findAttributeValue(MutationSignature.ATTR_MUTATES); + assert value != null; + holder.registerProblem(value, error); + } + } } }; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java index 4174d988a406..29c8e87e998d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java @@ -1,15 +1,16 @@ -// Copyright 2000-2017 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. +// Copyright 2000-2018 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.codeInspection.dataFlow; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.psi.*; +import com.siyeh.ig.psiutils.ClassUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Arrays; public class MutationSignature { - private static final String ATTR_MUTATES = "mutates"; + public static final String ATTR_MUTATES = "mutates"; private static final String CONTRACT_ANNOTATION = "org.jetbrains.annotations.Contract"; private static final MutationSignature UNKNOWN = new MutationSignature(false, new boolean[0]); private static final MutationSignature PURE = new MutationSignature(false, new boolean[0]); @@ -91,8 +92,17 @@ public class MutationSignature { if (ms.myThis && method.hasModifierProperty(PsiModifier.STATIC)) { return "Static method cannot mutate 'this'"; } - if (ms.myArgs.length > method.getParameterList().getParametersCount()) { - return "Reference to argument #" + ms.myArgs.length + " is invalid"; + PsiParameter[] parameters = method.getParameterList().getParameters(); + if (ms.myArgs.length > parameters.length) { + return "Reference to parameter #" + ms.myArgs.length + " is invalid"; + } + for (int i = 0; i < ms.myArgs.length; i++) { + if (ms.myArgs[i]) { + PsiType type = parameters[i].getType(); + if (ClassUtils.isImmutable(type)) { + return "Parameter #" + (i + 1) + " has immutable type '" + type.getPresentableText() + "'"; + } + } } } catch (IllegalArgumentException ex) { diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/MutationSignatureProblems.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/MutationSignatureProblems.java new file mode 100644 index 000000000000..684ef30edf4d --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/MutationSignatureProblems.java @@ -0,0 +1,43 @@ +package org.jetbrains.annotations; + +import java.lang.annotation.*; +import java.util.*; + +@Target({ElementType.METHOD, ElementType.CONSTRUCTOR}) +@interface Contract { + String value() default ""; + boolean pure() default false; + String mutates() default ""; +} + +class Test { + @Contract(mutates = "this") + public static void test1(List list) {} + + @Contract(mutates = "arg3") + public static void test2(List list) {} + + @Contract(mutates = "blahblahblah") + public static void test3(List list) {} + + @Contract(mutates = "arg") + public static void test4(List list) {} + + @Contract(mutates = "arg", pure = true) + public static void test5(List list) {} + + @Contract(mutates = "arg", pure = false) + public static void test6(List list) {} + + @Contract(mutates = "", pure = true) + public static void test7(List list) {} + + @Contract(mutates = "arg1") + public static void test8(String s, int i, List list) {} + + @Contract(mutates = "arg2") + public static void test9(String s, int i, List list) {} + + @Contract(mutates = "arg3") + public static void test10(String s, int i, List list) {} +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java index e66febed0068..54937b4b2a6a 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java @@ -1,18 +1,4 @@ -/* - * 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. - */ +// Copyright 2000-2018 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.java.codeInspection; import com.intellij.JavaTestUtil; @@ -64,4 +50,5 @@ public class ContractCheckTest extends LightCodeInsightFixtureTestCase { public void testPassingVarargsToDelegate() { doTest(); } public void testUnknownIfCondition() { doTest(); } public void testCallingNotNullMethod() { doTest(); } + public void testMutationSignatureProblems() { doTest(); } }