From de0e95a46b5a69f32af8731d151c55827747a670 Mon Sep 17 00:00:00 2001 From: Mikhail Pyltsin Date: Thu, 16 Nov 2023 15:28:15 +0100 Subject: [PATCH] Revert "[java-inspection] IDEA-264141 Warn if compact constructor calls methods that access fields" This reverts commit 30612508e6ae8a2bf3f7951880430144a2a7481a. GitOrigin-RevId: 543b8b881b1a973000121d6f749a62fe2138a905 --- ...cordConstructorAccessFieldsInspection.java | 131 ------------------ java/java-impl/src/META-INF/JavaPlugin.xml | 4 - ...cordConstructorAccessFieldsInspection.html | 22 --- .../NestedCalls.java | 34 ----- ...ConstructorAccessFieldsInspectionTest.java | 26 ---- ...yStreamApiCallChainsInspectionFixTest.java | 13 -- .../resources/messages/JavaBundle.properties | 4 - 7 files changed, 234 deletions(-) delete mode 100644 java/java-impl-inspections/src/com/intellij/codeInspection/CompactRecordConstructorAccessFieldsInspection.java delete mode 100644 java/java-impl/src/inspectionDescriptions/CompactRecordConstructorAccessFieldsInspection.html delete mode 100644 java/java-tests/testData/inspection/compactRecordConstructorAccessFields/NestedCalls.java delete mode 100644 java/java-tests/testSrc/com/intellij/java/codeInspection/CompactRecordConstructorAccessFieldsInspectionTest.java diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/CompactRecordConstructorAccessFieldsInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/CompactRecordConstructorAccessFieldsInspection.java deleted file mode 100644 index 9f1d4656aab8..000000000000 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/CompactRecordConstructorAccessFieldsInspection.java +++ /dev/null @@ -1,131 +0,0 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -package com.intellij.codeInspection; - -import com.intellij.codeInsight.daemon.impl.analysis.HighlightingFeature; -import com.intellij.java.JavaBundle; -import com.intellij.modcommand.ActionContext; -import com.intellij.modcommand.ModCommand; -import com.intellij.modcommand.Presentation; -import com.intellij.modcommand.PsiBasedModCommandAction; -import com.intellij.openapi.util.Ref; -import com.intellij.psi.*; -import com.intellij.psi.impl.light.LightRecordMember; -import com.intellij.psi.util.JavaPsiRecordUtil; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.util.containers.MultiMap; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -import java.util.ArrayList; -import java.util.List; - -public class CompactRecordConstructorAccessFieldsInspection extends AbstractBaseJavaLocalInspectionTool { - - @Override - public boolean runForWholeFile() { - return true; - } - - @Override - public @NotNull PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { - if (!HighlightingFeature.RECORDS.isAvailable(holder.getFile())) { - return PsiElementVisitor.EMPTY_VISITOR; - } - return new JavaElementVisitor() { - @Override - public void visitClass(@NotNull PsiClass aClass) { - if (!aClass.isRecord()) { - return; - } - if (aClass.getRecordComponents().length == 0) { - return; - } - - PsiMethod[] methods = aClass.getMethods(); - for (PsiMethod method : methods) { - if (!JavaPsiRecordUtil.isCompactConstructor(method)) { - continue; - } - List problemInfos = processCompactConstructor(method); - for (ProblemInfo problemInfo : problemInfos) { - if (isOnTheFly) { - LocalQuickFix fix = LocalQuickFix.from(new NavigateToUsageFix(problemInfo.reference)); - holder.registerProblem(problemInfo.callExpression, - JavaBundle.message("inspection.record.compact.constructor.access.fields.display.name"), - fix); - } - else { - holder.registerProblem(problemInfo.callExpression, - JavaBundle.message("inspection.record.compact.constructor.access.fields.display.name")); - } - } - } - } - - - record ProblemInfo(@NotNull PsiCallExpression callExpression, @NotNull PsiReferenceExpression reference) { - } - - private static List processCompactConstructor(PsiMethod compactConstructor) { - List result = new ArrayList<>(); - PsiManager psiManager = compactConstructor.getManager(); - PsiClass containingClass = compactConstructor.getContainingClass(); - MultiMap nestedMethods = new MultiMap<>(); - final JavaRecursiveElementWalkingVisitor visitor = new JavaRecursiveElementWalkingVisitor() { - @Override - public void visitCallExpression(@NotNull PsiCallExpression callExpression) { - PsiMethod resolvedMethod = callExpression.resolveMethod(); - if (resolvedMethod != null && - !resolvedMethod.hasModifierProperty(PsiModifier.STATIC) && - psiManager.areElementsEquivalent(resolvedMethod.getContainingClass(), containingClass)) { - nestedMethods.putValue(resolvedMethod, callExpression); - } - } - }; - compactConstructor.accept(visitor); - for (PsiMethod method : nestedMethods.keySet()) { - Ref found = new Ref<>(); - PsiTreeUtil.processElements(method.getBody(), e -> { - if (e instanceof PsiReferenceExpression referenceExpression) { - PsiElement resolved = referenceExpression.resolve(); - if (resolved instanceof LightRecordMember) { - found.set(referenceExpression); - return false; - } - } - return true; - }); - if (!found.isNull()) { - nestedMethods.get(method).forEach(expr -> { - result.add(new ProblemInfo(expr, found.get())); - }); - } - } - return result; - } - }; - } - - private static class NavigateToUsageFix extends PsiBasedModCommandAction { - private NavigateToUsageFix(@NotNull PsiReferenceExpression reference) { - super(reference); - } - - @Override - @NotNull - public String getFamilyName() { - return JavaBundle.message("inspection.record.compact.constructor.access.fields.navigate.usages.family"); - } - - @Override - protected @Nullable Presentation getPresentation(@NotNull ActionContext context, @NotNull PsiReferenceExpression reference) { - return Presentation.of(JavaBundle.message("inspection.record.compact.constructor.access.fields.navigate.usages.declaration.text", - reference.getCanonicalText())); - } - - @Override - protected @NotNull ModCommand perform(@NotNull ActionContext context, @NotNull PsiReferenceExpression reference) { - return ModCommand.select(reference); - } - } -} diff --git a/java/java-impl/src/META-INF/JavaPlugin.xml b/java/java-impl/src/META-INF/JavaPlugin.xml index 5b3ce55def8b..e6b4ea102eb1 100644 --- a/java/java-impl/src/META-INF/JavaPlugin.xml +++ b/java/java-impl/src/META-INF/JavaPlugin.xml @@ -1607,10 +1607,6 @@ groupBundle="messages.InspectionsBundle" groupKey="group.names.probable.bugs" bundle="messages.JavaBundle" key="inspection.meaningless.record.annotation.description" implementationClass="com.intellij.codeInspection.MeaninglessRecordAnnotationInspection"/> - - -Reports method calls within record compact constructors if these methods access fields. -

These calls look suspicious because record fields may not be initialized at the time of these calls.

-

Example:

-

-public record MyRecord(String name, int id) {
-    public MyRecord {
-        validateMyFields(); // it looks suspicious
-    }
-
-    private void validateMyFields() {
-        if (this.name.isEmpty()) {
-            throw new IllegalArgumentException();
-        }
-    }
-}
-
- -

New in 2024.1

- - \ No newline at end of file diff --git a/java/java-tests/testData/inspection/compactRecordConstructorAccessFields/NestedCalls.java b/java/java-tests/testData/inspection/compactRecordConstructorAccessFields/NestedCalls.java deleted file mode 100644 index e8aab7f71e91..000000000000 --- a/java/java-tests/testData/inspection/compactRecordConstructorAccessFields/NestedCalls.java +++ /dev/null @@ -1,34 +0,0 @@ -class RecordMain { - public record MyRecord(String name, int id) { - - public MyRecord { - validateMyFields(); - validateMyFieldsWithGetter(); - nothing(); - } - - private void nothing() { - - } - - private static void t() { - - } - private void validateMyFields() { - if (this.name.isEmpty()) { - throw new IllegalArgumentException(); - } - } - private void validateMyFieldsWithGetter() { - if (this.name().isEmpty()) { - throw new IllegalArgumentException(); - } - } - } - - - public static void main(String[] args) { - MyRecord myRecord = new MyRecord("s", 2); - System.out.println(myRecord); - } -} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/CompactRecordConstructorAccessFieldsInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/CompactRecordConstructorAccessFieldsInspectionTest.java deleted file mode 100644 index d9cee63a84a6..000000000000 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/CompactRecordConstructorAccessFieldsInspectionTest.java +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -package com.intellij.java.codeInspection; - -import com.intellij.JavaTestUtil; -import com.intellij.codeInspection.CompactRecordConstructorAccessFieldsInspection; -import com.intellij.pom.java.LanguageLevel; -import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; - - -public class CompactRecordConstructorAccessFieldsInspectionTest extends LightJavaCodeInsightFixtureTestCase { - public void testNestedCalls() { doTest(); } - - private void doTest() { - myFixture.enableInspections(new CompactRecordConstructorAccessFieldsInspection()); - myFixture.testHighlighting(getTestName(false) + ".java"); - } - - protected LanguageLevel getLanguageLevel() { - return LanguageLevel.JDK_21; - } - - @Override - protected String getBasePath() { - return JavaTestUtil.getRelativeJavaTestDataPath()+"/inspection/compactRecordConstructorAccessFields"; - } -} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/SimplifyStreamApiCallChainsInspectionFixTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/SimplifyStreamApiCallChainsInspectionFixTest.java index 98ff281ce215..8571609869bf 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/SimplifyStreamApiCallChainsInspectionFixTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/SimplifyStreamApiCallChainsInspectionFixTest.java @@ -19,10 +19,7 @@ import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCa import com.intellij.codeInspection.LocalInspectionTool; import com.intellij.codeInspection.SimplifyStreamApiCallChainsInspection; import com.intellij.openapi.projectRoots.Sdk; -import com.intellij.pom.java.LanguageLevel; import com.intellij.testFramework.IdeaTestUtil; -import com.intellij.testFramework.LightProjectDescriptor; -import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; import org.jetbrains.annotations.NotNull; public class SimplifyStreamApiCallChainsInspectionFixTest extends LightQuickFixParameterizedTestCase { @@ -37,16 +34,6 @@ public class SimplifyStreamApiCallChainsInspectionFixTest extends LightQuickFixP return IdeaTestUtil.getMockJdk11(); } - @Override - protected LanguageLevel getLanguageLevel() { - return LanguageLevel.JDK_21; - } - - @Override - protected @NotNull LightProjectDescriptor getProjectDescriptor() { - return LightJavaCodeInsightFixtureTestCase.JAVA_21; - } - @Override protected String getBasePath() { return "/inspection/streamApiCallChains"; diff --git a/java/openapi/resources/messages/JavaBundle.properties b/java/openapi/resources/messages/JavaBundle.properties index 9676284921fb..8952137e2ec8 100644 --- a/java/openapi/resources/messages/JavaBundle.properties +++ b/java/openapi/resources/messages/JavaBundle.properties @@ -1705,10 +1705,6 @@ inspection.redundant.unmodifiable.call.description=Redundant usage of unmodifiab inspection.redundant.unmodifiable.call.unwrap.argument.quickfix=Unwrap argument completion.override.implement.methods=Override/Implement methods... lambda.tree.node.presentation=Lambda -inspection.record.compact.constructor.access.fields.description=Compact constructor access components -inspection.record.compact.constructor.access.fields.display.name=Calling method can access non-initialized fields -inspection.record.compact.constructor.access.fields.navigate.usages.family=Navigate to a component usage -inspection.record.compact.constructor.access.fields.navigate.usages.declaration.text=Navigate to a component usage ''{0}'' inspection.meaningless.record.annotation.description=Meaningless record annotation inspection.meaningless.record.annotation.message.method.and.parameter=Annotation has no effect: its targets are METHOD and PARAMETER but both accessor and canonical constructor are explicitly declared inspection.meaningless.record.annotation.message.method=Annotation has no effect: its target is METHOD but the corresponding accessor is explicitly declared