diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightMethodUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightMethodUtil.java index 34d44ba16991..8c3a5fb4b71d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightMethodUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightMethodUtil.java @@ -1366,7 +1366,7 @@ public class HighlightMethodUtil { * instance method overrides static. see JLS 8.4.6.1, 8.4.6.2 */ static HighlightInfo checkStaticMethodOverride(@NotNull PsiMethod method, @NotNull PsiFile containingFile) { - // constructors are not members and therefor don't override class methods + // constructors are not members and therefore don't override class methods if (method.isConstructor()) { return null; } @@ -2038,7 +2038,21 @@ public class HighlightMethodUtil { QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createDeleteFix(typeParameterList)); return info; } - if (!method.hasModifierProperty(PsiModifier.PUBLIC)) { + if (method.isConstructor() && PsiUtil.getLanguageLevel(method) != LanguageLevel.JDK_14_PREVIEW) { + AccessModifier modifier = AccessModifier.fromModifierList(method.getModifierList()); + PsiModifierList classModifierList = Objects.requireNonNull(method.getContainingClass()).getModifierList(); + if (classModifierList != null) { + AccessModifier classModifier = AccessModifier.fromModifierList(classModifierList); + if (classModifier.isWeaker(modifier)) { + HighlightInfo info = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(identifier) + .descriptionAndTooltip(JavaErrorBundle.message("record.special.method.stronger.access", methodTitle, classModifier)) + .create(); + QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createModifierListFix( + method, classModifier.toPsiModifier(), true, false)); + return info; + } + } + } else if (!method.hasModifierProperty(PsiModifier.PUBLIC)) { HighlightInfo info = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(identifier) .descriptionAndTooltip(JavaErrorBundle.message("record.special.method.non.public", methodTitle)) .create(); diff --git a/java/java-impl/src/com/intellij/codeInsight/generation/RecordConstructorMember.java b/java/java-impl/src/com/intellij/codeInsight/generation/RecordConstructorMember.java index d0b9acae755a..ae1c7bad14b0 100644 --- a/java/java-impl/src/com/intellij/codeInsight/generation/RecordConstructorMember.java +++ b/java/java-impl/src/com/intellij/codeInsight/generation/RecordConstructorMember.java @@ -3,11 +3,14 @@ package com.intellij.codeInsight.generation; import com.intellij.codeInsight.AnnotationTargetUtil; import com.intellij.openapi.project.Project; +import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleSettings; +import com.intellij.psi.util.AccessModifier; import com.intellij.psi.util.PsiFormatUtil; import com.intellij.psi.util.PsiFormatUtilBase; +import com.intellij.psi.util.PsiUtil; import com.intellij.ui.SimpleColoredComponent; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; @@ -47,18 +50,26 @@ public class RecordConstructorMember implements ClassMember { @NotNull public PsiMethod generateRecordConstructor() { String constructor; + AccessModifier accessModifier = AccessModifier.PUBLIC; + if (PsiUtil.getLanguageLevel(myRecord) != LanguageLevel.JDK_14_PREVIEW) { + PsiModifierList list = myRecord.getModifierList(); + if (list != null) { + accessModifier = AccessModifier.fromModifierList(list); + } + } if (myCompact) { - constructor = "public " + myRecord.getName() + "{\n}"; + constructor = myRecord.getName() + "{\n}"; } else { PsiRecordComponent[] components = myRecord.getRecordComponents(); String parameters = StreamEx.of(components).map(PsiRecordComponent::getText).joining(",", "(", ")"); String body = StreamEx.of(components).map(PsiRecordComponent::getName).map(name -> "this." + name + "=" + name + ";\n").joining("", "{", "}"); - constructor = "public " + myRecord.getName() + parameters + body; + constructor = myRecord.getName() + parameters + body; } Project project = myRecord.getProject(); PsiMethod ctor = JavaPsiFacade.getElementFactory(project).createMethodFromText(constructor, myRecord); + ctor.getModifierList().setModifierProperty(accessModifier.toPsiModifier(), true); if (!myCompact) { JavaCodeStyleSettings settings = JavaCodeStyleSettings.getInstance(myRecord.getContainingFile()); boolean finalParameters = settings.isGenerateFinalParameters(); diff --git a/java/java-psi-impl/src/com/intellij/psi/util/AccessModifier.java b/java/java-psi-impl/src/com/intellij/psi/util/AccessModifier.java index b4e0ce43e294..1dd1127d1551 100644 --- a/java/java-psi-impl/src/com/intellij/psi/util/AccessModifier.java +++ b/java/java-psi-impl/src/com/intellij/psi/util/AccessModifier.java @@ -1,6 +1,7 @@ // Copyright 2000-2020 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.psi.util; +import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.*; import com.intellij.psi.impl.light.LightRecordCanonicalConstructor; import com.intellij.util.containers.ContainerUtil; @@ -115,10 +116,19 @@ public enum AccessModifier { if (member instanceof PsiMethod) { PsiMethod method = (PsiMethod)member; if (containingClass == null || containingClass.isEnum() && method.isConstructor()) return Collections.emptyList(); - if (JavaPsiRecordUtil.getRecordComponentForAccessor(method) != null || - JavaPsiRecordUtil.isCompactConstructor(method) || + if (JavaPsiRecordUtil.getRecordComponentForAccessor(method) != null) { + return Collections.singletonList(PUBLIC); + } + if (JavaPsiRecordUtil.isCompactConstructor(method) || JavaPsiRecordUtil.isExplicitCanonicalConstructor(method) || method instanceof LightRecordCanonicalConstructor) { + if (PsiUtil.getLanguageLevel(member) != LanguageLevel.JDK_14_PREVIEW) { + PsiModifierList list = containingClass.getModifierList(); + if (list != null) { + AccessModifier classModifier = fromModifierList(list); + return ContainerUtil.filter(ALL_MODIFIERS, m -> !classModifier.isWeaker(m)); + } + } return Collections.singletonList(PUBLIC); } if (containingClass.isInterface()) { diff --git a/java/java-psi-impl/src/messages/JavaErrorBundle.properties b/java/java-psi-impl/src/messages/JavaErrorBundle.properties index 2018a2fa1d3b..b9cfadf5eae1 100644 --- a/java/java-psi-impl/src/messages/JavaErrorBundle.properties +++ b/java/java-psi-impl/src/messages/JavaErrorBundle.properties @@ -423,6 +423,7 @@ record.constructor.call.in.canonical=Canonical constructor cannot delegate to an record.no.constructor.call.in.non.canonical=Non-canonical record constructor must delegate to another constructor record.special.method.type.parameters={0} cannot have type parameters record.special.method.non.public={0} must be ''public'' +record.special.method.stronger.access={0} access level cannot be stronger than the record access level (''{1}'') record.special.method.throws={0} cannot declare thrown exceptions record.canonical.constructor=Canonical constructor record.compact.constructor=Compact constructor diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordConstructorAccessJava15.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordConstructorAccessJava15.java new file mode 100644 index 000000000000..d692dec35400 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordConstructorAccessJava15.java @@ -0,0 +1,65 @@ +class Outer { + public record PublicRecord1() { + public PublicRecord1 {} + } + public record PublicRecord2() { + protected PublicRecord2() {} + } + public record PublicRecord3() { + PublicRecord3() {} + } + public record PublicRecord4() { + private PublicRecord4 {} + } + protected record ProtectedRecord1() { + public ProtectedRecord1 {} + } + protected record ProtectedRecord2() { + protected ProtectedRecord2() {} + } + protected record ProtectedRecord3() { + ProtectedRecord3() {} + } + protected record ProtectedRecord4() { + private ProtectedRecord4 {} + } + record PackageRecord1() { + public PackageRecord1 {} + } + record PackageRecord2() { + protected PackageRecord2() {} + } + record PackageRecord3() { + PackageRecord3() {} + } + record PackageRecord4() { + private PackageRecord4 {} + } + private record PrivateRecord1() { + public PrivateRecord1 {} + } + private record PrivateRecord2() { + protected PrivateRecord2() {} + } + private record PrivateRecord3() { + PrivateRecord3() {} + } + private record PrivateRecord4() { + private PrivateRecord4 {} + } + + void test() { + record LocalRecord1() { + public LocalRecord1 {} + } + record LocalRecord2() { + protected LocalRecord2() {} + } + record LocalRecord3() { + LocalRecord3() {} + } + record LocalRecord4() { + private LocalRecord4 {} + } + } +} \ 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 33a0f31313ab..717cb2b63a90 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 @@ -32,6 +32,9 @@ public class LightRecordsHighlightingTest extends LightJavaCodeInsightFixtureTes public void testRecordConstructors() { doTest(); } + public void testRecordConstructorAccessJava15() { + IdeaTestUtil.withLevel(getModule(), LanguageLevel.JDK_15_PREVIEW, this::doTest); + } public void testRecordCompactConstructors() { doTest(); } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspection.java index 4c362fba13c2..9f77d0e8af75 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspection.java @@ -17,8 +17,10 @@ package com.siyeh.ig.classlayout; import com.intellij.codeInspection.ProblemDescriptor; 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.PsiUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; @@ -104,9 +106,10 @@ public class PublicConstructorInNonPublicClassInspection extends BaseInspection if (containingClass == null) { return; } - if (containingClass.isRecord() && + if (containingClass.isRecord() && PsiUtil.getLanguageLevel(containingClass) == LanguageLevel.JDK_14_PREVIEW && (JavaPsiRecordUtil.isCompactConstructor(method) || JavaPsiRecordUtil.isExplicitCanonicalConstructor(method))) { - // compact and canonical constructors in record must be public, according to spec + // compact and canonical constructors in record must be public, according to Java 14-preview spec + // this restriction is relaxed in Java 15-preview, so the inspection makes sense again return; } if (containingClass.hasModifierProperty(PsiModifier.PUBLIC) || diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/public_constructor_in_non_public_class/RecordsJava15.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/public_constructor_in_non_public_class/RecordsJava15.java new file mode 100644 index 000000000000..dfcc513b1ec4 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/public_constructor_in_non_public_class/RecordsJava15.java @@ -0,0 +1,18 @@ +package com.siyeh.igtest.classlayout.public_constructor_in_non_public_class; + +record Rec() { + public Rec {} + public Rec(int x) { + this(); + System.out.println(x); + } +} +record Rec2(int x) { + public Rec2() { + this(0); + } + public Rec2(int x) { + System.out.println(x); + this.x = x; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspectionTest.java index 37b7b83390fb..8e687266d0e3 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/classlayout/PublicConstructorInNonPublicClassInspectionTest.java @@ -2,6 +2,8 @@ package com.siyeh.ig.classlayout; import com.intellij.codeInsight.intention.IntentionAction; +import com.intellij.pom.java.LanguageLevel; +import com.intellij.testFramework.IdeaTestUtil; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; import com.siyeh.ig.LightJavaInspectionTestCase; @@ -27,6 +29,9 @@ public class PublicConstructorInNonPublicClassInspectionTest extends LightJavaCo public void testPublicConstructorInNonPublicClass() { doTest(); } + public void testRecordsJava15() { + IdeaTestUtil.withLevel(myFixture.getModule(), LanguageLevel.JDK_15_PREVIEW, this::doTest); + } public void testQuickfix() { doTest(); diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1.java new file mode 100644 index 000000000000..a9e63ca6e39b --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1.java @@ -0,0 +1,5 @@ +class Outer { + protected record Foo() { + public Foo {} + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1Java14.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1Java14.java new file mode 100644 index 000000000000..a9e63ca6e39b --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1Java14.java @@ -0,0 +1,5 @@ +class Outer { + protected record Foo() { + public Foo {} + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1_after.java new file mode 100644 index 000000000000..07a0e7b35d02 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor1_after.java @@ -0,0 +1,5 @@ +class Outer { + protected record Foo() { + protected Foo {} + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor2.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor2.java new file mode 100644 index 000000000000..c336cc7fa5af --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor2.java @@ -0,0 +1,5 @@ +class Outer { + private record Foo() { + protected Foo {} + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor2_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor2_after.java new file mode 100644 index 000000000000..e788d8c10b17 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/modifiers/change_modifier/RecordConstructor2_after.java @@ -0,0 +1,5 @@ +class Outer { + private record Foo() { + public Foo {} + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/modifiers/ChangeModifierIntentionTest.java b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/modifiers/ChangeModifierIntentionTest.java index f6470c8ae55b..fc4f899d8c8f 100644 --- a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/modifiers/ChangeModifierIntentionTest.java +++ b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/modifiers/ChangeModifierIntentionTest.java @@ -81,6 +81,18 @@ public class ChangeModifierIntentionTest extends IPPTestCase { BaseRefactoringProcessor.ConflictsInTestsException.withIgnoredConflicts(() -> doTestWithChooser("protected")); } + public void testRecordConstructor1() { + IdeaTestUtil.withLevel(myFixture.getModule(), LanguageLevel.JDK_15_PREVIEW, () -> doTest("Make 'Foo' protected")); + } + + public void testRecordConstructor1Java14() { + IdeaTestUtil.withLevel(myFixture.getModule(), LanguageLevel.JDK_14_PREVIEW, () -> assertIntentionNotAvailable("Make 'Foo' protected")); + } + + public void testRecordConstructor2() { + IdeaTestUtil.withLevel(myFixture.getModule(), LanguageLevel.JDK_15_PREVIEW, () -> doTestWithChooser("public")); + } + void doTestWithChooser(String wanted) { UiInterceptors .register(new ChooserInterceptor(Arrays.asList("public", "protected", "package-private", "private"), Pattern.quote(wanted)));