From e027f98d6a219012c281761ca06715d66dbac9da Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 22 Jun 2020 17:22:13 +0700 Subject: [PATCH] Disable field assignments in record compact constructors Last piece of IDEA-239088 GitOrigin-RevId: 23c467c6c525d7908922bc2ebd8928205368dff7 --- .../analysis/HighlightControlFlowUtil.java | 10 ++++- .../RedundantRecordConstructorInspection.java | 2 + .../RecordCompactConstructorsJava15.java | 8 ++++ .../beforeCanonicalAssignSame.java | 2 +- .../beforeCanonicalAssignViceVersa.java | 2 +- ...eforeCanonicalNotAnnotatedParameterOk.java | 2 +- ...fterCanonicalAssignSameJava14Preview.java} | 0 ...anonicalAssignViceVersaJava14Preview.java} | 0 ...NotAnnotatedParameterOkJava14Preview.java} | 0 ...eforeCanonicalAssignSameJava14Preview.java | 9 ++++ ...CanonicalAssignViceVersaJava14Preview.java | 7 ++++ ...lNotAnnotatedParameterOkJava14Preview.java | 13 ++++++ .../daemon/LightRecordsHighlightingTest.java | 3 ++ ...dantRecordConstructorInspection14Test.java | 42 +++++++++++++++++++ ...undantRecordConstructorInspectionTest.java | 3 +- 15 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordCompactConstructorsJava15.java rename java/java-tests/testData/inspection/redundantRecordConstructor/{afterCanonicalAssignSame.java => java14/afterCanonicalAssignSameJava14Preview.java} (100%) rename java/java-tests/testData/inspection/redundantRecordConstructor/{afterCanonicalAssignViceVersa.java => java14/afterCanonicalAssignViceVersaJava14Preview.java} (100%) rename java/java-tests/testData/inspection/redundantRecordConstructor/{afterCanonicalNotAnnotatedParameterOk.java => java14/afterCanonicalNotAnnotatedParameterOkJava14Preview.java} (100%) create mode 100644 java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignSameJava14Preview.java create mode 100644 java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignViceVersaJava14Preview.java create mode 100644 java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalNotAnnotatedParameterOkJava14Preview.java create mode 100644 java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspection14Test.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java index 37cc2c90327b..35fd1dffd62a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java @@ -240,12 +240,14 @@ public class HighlightControlFlowUtil { if (identifier == null) return null; PsiMethod canonicalConstructor = JavaPsiRecordUtil.findCanonicalConstructor(aClass); if (canonicalConstructor == null || canonicalConstructor instanceof LightRecordCanonicalConstructor) return null; + boolean isCompact = JavaPsiRecordUtil.isCompactConstructor(canonicalConstructor); + if (isCompact && PsiUtil.getLanguageLevel(aClass) != LanguageLevel.JDK_14_PREVIEW) return null; PsiCodeBlock body = canonicalConstructor.getBody(); if (body == null) return null; PsiField field = JavaPsiRecordUtil.getFieldForComponent(component); if (field == null) return null; if (variableDefinitelyAssignedIn(field, body)) return null; - if (JavaPsiRecordUtil.isCompactConstructor(canonicalConstructor) && variableDefinitelyNotAssignedIn(field, body)) return null; + if (isCompact && variableDefinitelyNotAssignedIn(field, body)) return null; String description = JavaErrorBundle.message("record.component.not.initialized", field.getName()); return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(identifier).descriptionAndTooltip(description).create(); } @@ -646,7 +648,11 @@ public class HighlightControlFlowUtil { PsiField field = (PsiField)variable; if (innerClass != null && !containingFile.getManager().areElementsEquivalent(innerClass, field.getContainingClass())) return false; final PsiMember enclosingCtrOrInitializer = PsiUtil.findEnclosingConstructorOrInitializer(expression); - return enclosingCtrOrInitializer != null && isSameField(enclosingCtrOrInitializer, field, reference, containingFile); + return enclosingCtrOrInitializer != null && + !(enclosingCtrOrInitializer instanceof PsiMethod && + JavaPsiRecordUtil.isCompactConstructor((PsiMethod)enclosingCtrOrInitializer) && + PsiUtil.getLanguageLevel(enclosingCtrOrInitializer) != LanguageLevel.JDK_14_PREVIEW) && + isSameField(enclosingCtrOrInitializer, field, reference, containingFile); } if (variable instanceof PsiLocalVariable) { boolean isAccessedFromOtherClass = innerClass != null; diff --git a/java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java b/java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java index d1be159189fa..072a785d8006 100644 --- a/java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/RedundantRecordConstructorInspection.java @@ -7,6 +7,7 @@ import com.intellij.codeInsight.daemon.impl.analysis.HighlightingFeature; import com.intellij.codeInsight.daemon.impl.quickfix.DeleteElementFix; import com.intellij.java.JavaBundle; import com.intellij.openapi.project.Project; +import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.*; import com.intellij.psi.util.JavaPsiRecordUtil; import com.intellij.psi.util.PsiTreeUtil; @@ -65,6 +66,7 @@ public class RedundantRecordConstructorInspection extends AbstractBaseJavaLocalI return; } if (PsiUtil.findReturnStatements(body).length > 0) return; + if (PsiUtil.getLanguageLevel(ctor) != LanguageLevel.JDK_14_PREVIEW && assignedCount != components.length) return; holder.registerProblem(ctor.getParameterList(), JavaBundle.message("inspection.redundant.record.constructor.can.be.compact.message"), ProblemHighlightType.LIKE_UNUSED_SYMBOL, new ConvertToCompactConstructorFix()); } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordCompactConstructorsJava15.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordCompactConstructorsJava15.java new file mode 100644 index 000000000000..67df00d0bd37 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordCompactConstructorsJava15.java @@ -0,0 +1,8 @@ +record WrittenFields(int x, + int y, + int z) { + public WrittenFields { + this.x = 0; + if (Math.random() > 0.5) this.y = 1; + } +} diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java index 684769b47f31..bd2fbcd065c1 100644 --- a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignSame.java @@ -1,4 +1,4 @@ -// "Convert canonical constructor to compact form" "true" +// "Convert canonical constructor to compact form" "false" record Rec(int x, int y) { public Rec(int x, int y) { this.x = y; diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignViceVersa.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignViceVersa.java index b44d5d00a593..69a8e8f8e772 100644 --- a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignViceVersa.java +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalAssignViceVersa.java @@ -1,4 +1,4 @@ -// "Convert canonical constructor to compact form" "true" +// "Convert canonical constructor to compact form" "false" record Rec(int x, int y) { public Rec(int x, int y) { this.x = y; diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalNotAnnotatedParameterOk.java b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalNotAnnotatedParameterOk.java index 02ac94adef02..eee2f866cc92 100644 --- a/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalNotAnnotatedParameterOk.java +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/beforeCanonicalNotAnnotatedParameterOk.java @@ -1,4 +1,4 @@ -// "Convert canonical constructor to compact form" "true" +// "Convert canonical constructor to compact form" "false" import java.lang.annotation.ElementType; import java.lang.annotation.Target; diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignSame.java b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/afterCanonicalAssignSameJava14Preview.java similarity index 100% rename from java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignSame.java rename to java/java-tests/testData/inspection/redundantRecordConstructor/java14/afterCanonicalAssignSameJava14Preview.java diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignViceVersa.java b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/afterCanonicalAssignViceVersaJava14Preview.java similarity index 100% rename from java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalAssignViceVersa.java rename to java/java-tests/testData/inspection/redundantRecordConstructor/java14/afterCanonicalAssignViceVersaJava14Preview.java diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalNotAnnotatedParameterOk.java b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/afterCanonicalNotAnnotatedParameterOkJava14Preview.java similarity index 100% rename from java/java-tests/testData/inspection/redundantRecordConstructor/afterCanonicalNotAnnotatedParameterOk.java rename to java/java-tests/testData/inspection/redundantRecordConstructor/java14/afterCanonicalNotAnnotatedParameterOkJava14Preview.java diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignSameJava14Preview.java b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignSameJava14Preview.java new file mode 100644 index 000000000000..684769b47f31 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignSameJava14Preview.java @@ -0,0 +1,9 @@ +// "Convert canonical constructor to compact form" "true" +record Rec(int x, int y) { + public Rec(int x, int y) { + this.x = y; + // 1 + this.y /*2*/= y; //3 + //4 + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignViceVersaJava14Preview.java b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignViceVersaJava14Preview.java new file mode 100644 index 000000000000..b44d5d00a593 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalAssignViceVersaJava14Preview.java @@ -0,0 +1,7 @@ +// "Convert canonical constructor to compact form" "true" +record Rec(int x, int y) { + public Rec(int x, int y) { + this.x = y; + this.y = x; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalNotAnnotatedParameterOkJava14Preview.java b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalNotAnnotatedParameterOkJava14Preview.java new file mode 100644 index 000000000000..02ac94adef02 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantRecordConstructor/java14/beforeCanonicalNotAnnotatedParameterOkJava14Preview.java @@ -0,0 +1,13 @@ +// "Convert canonical constructor to compact form" "true" +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; + +record Rec(@Anno int x, int y) { + public Rec(int x, int y) { + this.x = y; + this.y = y; + } +} + +@Target(ElementType.FIELD) +@interface Anno {} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightRecordsHighlightingTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightRecordsHighlightingTest.java index 717cb2b63a90..f38e658967b5 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightRecordsHighlightingTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightRecordsHighlightingTest.java @@ -38,6 +38,9 @@ public class LightRecordsHighlightingTest extends LightJavaCodeInsightFixtureTes public void testRecordCompactConstructors() { doTest(); } + public void testRecordCompactConstructorsJava15() { + IdeaTestUtil.withLevel(getModule(), LanguageLevel.JDK_15_PREVIEW, this::doTest); + } public void testLocalRecords() { doTest(); } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspection14Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspection14Test.java new file mode 100644 index 000000000000..3843aaaf37ee --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspection14Test.java @@ -0,0 +1,42 @@ +/* + * 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.codeInspection; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.RedundantRecordConstructorInspection; +import com.intellij.testFramework.LightProjectDescriptor; +import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; +import org.jetbrains.annotations.NotNull; + +// Remove this test when support of 14-preview is dropped +public class RedundantRecordConstructorInspection14Test extends LightQuickFixParameterizedTestCase { + + @Override + protected LocalInspectionTool @NotNull [] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new RedundantRecordConstructorInspection()}; + } + + @Override + protected @NotNull LightProjectDescriptor getProjectDescriptor() { + return LightJavaCodeInsightFixtureTestCase.JAVA_14; + } + + @Override + protected String getBasePath() { + return "/inspection/redundantRecordConstructor/java14"; + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspectionTest.java index ad00f3c495ab..ad5999e7e8a9 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantRecordConstructorInspectionTest.java @@ -18,7 +18,6 @@ package com.intellij.java.codeInspection; import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; import com.intellij.codeInspection.LocalInspectionTool; import com.intellij.codeInspection.RedundantRecordConstructorInspection; -import com.intellij.codeInspection.RedundantStreamOptionalCallInspection; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; import org.jetbrains.annotations.NotNull; @@ -32,7 +31,7 @@ public class RedundantRecordConstructorInspectionTest extends LightQuickFixParam @Override protected @NotNull LightProjectDescriptor getProjectDescriptor() { - return LightJavaCodeInsightFixtureTestCase.JAVA_14; + return LightJavaCodeInsightFixtureTestCase.JAVA_15; } @Override