From 20b13c9122c8705e643429ba3a856f15e692d334 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 26 Dec 2019 12:38:03 +0700 Subject: [PATCH] Records: report canonical constructor parameter name mismatch (IDEA-228460) GitOrigin-RevId: d2c5c75f2a9066bd949ad503ad6fcbd71c594d18 --- .../impl/analysis/HighlightMethodUtil.java | 44 +++++++++++++------ .../impl/analysis/HighlightVisitorImpl.java | 2 +- .../GenerateConstructorHandler.java | 16 ++++--- .../intellij/psi/util/JavaPsiRecordUtil.java | 2 +- .../src/messages/JavaErrorMessages.properties | 1 + .../RecordConstructors.java | 6 +++ 6 files changed, 50 insertions(+), 21 deletions(-) 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 bbaac5e919ee..c49a9e18cb1f 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 @@ -36,6 +36,7 @@ import com.intellij.util.ArrayUtil; import com.intellij.util.JavaPsiConstructorUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.VisibilityUtil; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MostlySingularMultiMap; import com.intellij.util.ui.StartupUiUtil; import com.intellij.util.ui.UIUtil; @@ -50,6 +51,7 @@ import java.awt.*; import java.text.MessageFormat; import java.util.List; import java.util.*; +import java.util.function.Predicate; import java.util.stream.Stream; public class HighlightMethodUtil { @@ -1935,24 +1937,26 @@ public class HighlightMethodUtil { return checkRecordSpecialMethodDeclaration(method, JavaErrorMessages.message("record.accessor")); } - public static HighlightInfo checkRecordConstructorDeclaration(PsiMethod method) { - if (!method.isConstructor()) return null; + @NotNull + public static List checkRecordConstructorDeclaration(PsiMethod method) { + if (!method.isConstructor()) return Collections.emptyList(); PsiClass aClass = method.getContainingClass(); - if (aClass == null) return null; + if (aClass == null) return Collections.emptyList(); PsiIdentifier identifier = method.getNameIdentifier(); - if (identifier == null) return null; + if (identifier == null) return Collections.emptyList(); if (!aClass.isRecord()) { if (JavaPsiRecordUtil.isCompactConstructor(method)) { HighlightInfo info = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range( identifier).descriptionAndTooltip(JavaErrorMessages.message("compact.constructor.in.regular.class")).create(); QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createAddParameterListFix(method)); - return info; + return Collections.singletonList(info); } - return null; + return Collections.emptyList(); } if (JavaPsiRecordUtil.isCanonicalConstructor(method)) { PsiParameter[] parameters = method.getParameterList().getParameters(); PsiRecordComponent[] components = aClass.getRecordComponents(); + List problems = new ArrayList<>(); assert parameters.length == components.length; for (int i = 0; i < parameters.length; i++) { PsiType componentType = components[i].getType(); @@ -1963,29 +1967,43 @@ public class HighlightMethodUtil { if (parameterType instanceof PsiEllipsisType) { parameterType = ((PsiEllipsisType)parameterType).toArrayType(); } + String componentName = components[i].getName(); + String parameterName = parameters[i].getName(); if (!parameterType.equals(componentType)) { String message = - JavaErrorMessages.message("record.canonical.constructor.wrong.parameter.type", components[i].getName(), + JavaErrorMessages.message("record.canonical.constructor.wrong.parameter.type", componentName, componentType.getPresentableText(), parameterType.getPresentableText()); HighlightInfo info = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range( Objects.requireNonNull(parameters[i].getTypeElement())).descriptionAndTooltip(message).create(); QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createMethodParameterTypeFix(method, i, componentType, false)); - return info; + problems.add(info); + } + if (componentName != null && !parameterName.equals(componentName)) { + String message = JavaErrorMessages.message("record.canonical.constructor.wrong.parameter.name", componentName, parameterName); + HighlightInfo info = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range( + Objects.requireNonNull(parameters[i].getNameIdentifier())).descriptionAndTooltip(message).create(); + if (Arrays.stream(parameters).map(PsiParameter::getName).noneMatch(Predicate.isEqual(componentName))) { + QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createRenameElementFix(parameters[i], componentName)); + } + problems.add(info); } } - return checkRecordSpecialMethodDeclaration(method, JavaErrorMessages.message("record.canonical.constructor")); + ContainerUtil + .addIfNotNull(problems, checkRecordSpecialMethodDeclaration(method, JavaErrorMessages.message("record.canonical.constructor"))); + return problems; } else if (JavaPsiRecordUtil.isCompactConstructor(method)) { - return checkRecordSpecialMethodDeclaration(method, JavaErrorMessages.message("record.compact.constructor")); + return Collections + .singletonList(checkRecordSpecialMethodDeclaration(method, JavaErrorMessages.message("record.compact.constructor"))); } else { // Non-canonical constructor PsiMethodCallExpression call = JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(method); if (call == null || JavaPsiConstructorUtil.isSuperConstructorCall(call)) { String message = JavaErrorMessages.message("record.no.constructor.call.in.non.canonical"); - return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(identifier) - .descriptionAndTooltip(message).create(); + return Collections.singletonList(HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(identifier) + .descriptionAndTooltip(message).create()); } - return null; + return Collections.emptyList(); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index a535a7511618..d01024a5b430 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -881,7 +881,7 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh if (!myHolder.hasErrorResults()) myHolder.add(HighlightMethodUtil.checkRecursiveConstructorInvocation(method)); if (!myHolder.hasErrorResults()) myHolder.add(GenericsHighlightUtil.checkSafeVarargsAnnotation(method, myLanguageLevel)); if (!myHolder.hasErrorResults()) myHolder.add(HighlightMethodUtil.checkRecordAccessorDeclaration(method)); - if (!myHolder.hasErrorResults()) myHolder.add(HighlightMethodUtil.checkRecordConstructorDeclaration(method)); + if (!myHolder.hasErrorResults()) myHolder.addAll(HighlightMethodUtil.checkRecordConstructorDeclaration(method)); PsiClass aClass = method.getContainingClass(); if (!myHolder.hasErrorResults() && method.isConstructor()) { diff --git a/java/java-impl/src/com/intellij/codeInsight/generation/GenerateConstructorHandler.java b/java/java-impl/src/com/intellij/codeInsight/generation/GenerateConstructorHandler.java index 7b065cdd14a6..06fd86250bd2 100644 --- a/java/java-impl/src/com/intellij/codeInsight/generation/GenerateConstructorHandler.java +++ b/java/java-impl/src/com/intellij/codeInsight/generation/GenerateConstructorHandler.java @@ -15,6 +15,7 @@ import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleSettings; import com.intellij.psi.codeStyle.VariableKind; +import com.intellij.psi.impl.light.LightRecordCanonicalConstructor; import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.util.JavaPsiRecordUtil; import com.intellij.psi.util.TypeConversionUtil; @@ -67,12 +68,15 @@ public class GenerateConstructorHandler extends GenerateMembersHandlerBase { return null; } - if (aClass.isRecord() && JavaPsiRecordUtil.findCanonicalConstructor(aClass) == null) { - RecordConstructorChooserDialog dialog = new RecordConstructorChooserDialog(aClass); - if (!dialog.showAndGet()) return null; - ClassMember member = dialog.getClassMember(); - if (member != null) { - return new ClassMember[]{member}; + if (aClass.isRecord()) { + PsiMethod constructor = JavaPsiRecordUtil.findCanonicalConstructor(aClass); + if (constructor instanceof LightRecordCanonicalConstructor) { + RecordConstructorChooserDialog dialog = new RecordConstructorChooserDialog(aClass); + if (!dialog.showAndGet()) return null; + ClassMember member = dialog.getClassMember(); + if (member != null) { + return new ClassMember[]{member}; + } } } diff --git a/java/java-psi-api/src/com/intellij/psi/util/JavaPsiRecordUtil.java b/java/java-psi-api/src/com/intellij/psi/util/JavaPsiRecordUtil.java index b7c3203d0dfd..f9f4caa66b38 100644 --- a/java/java-psi-api/src/com/intellij/psi/util/JavaPsiRecordUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/util/JavaPsiRecordUtil.java @@ -85,7 +85,7 @@ public class JavaPsiRecordUtil { /** * @param recordClass record class * @return first explicitly declared canonical or compact constructor; - * null if no canonical and compact constructor declared or the supplied class is not a record + * null if the supplied class is not a record. Returns a synthetic constructor if it's not explicitly defined. */ @Nullable public static PsiMethod findCanonicalConstructor(@NotNull PsiClass recordClass) { diff --git a/java/java-psi-impl/src/messages/JavaErrorMessages.properties b/java/java-psi-impl/src/messages/JavaErrorMessages.properties index a6169a1a4879..bc6144221f2e 100644 --- a/java/java-psi-impl/src/messages/JavaErrorMessages.properties +++ b/java/java-psi-impl/src/messages/JavaErrorMessages.properties @@ -476,6 +476,7 @@ record.instance.initializer=Instance initializer is not allowed in record record.instance.field=Instance field is not allowed in record record.accessor.wrong.return.type=Incorrect component accessor return type. Expected: ''{0}'', found: ''{1}'' record.canonical.constructor.wrong.parameter.type=Incorrect parameter type for record component ''{0}''. Expected: ''{1}'', found: ''{2}'' +record.canonical.constructor.wrong.parameter.name=Canonical constructor parameter names must match record component names. Expected: ''{0}'', found: ''{1}'' record.constructor.call.in.canonical=Canonical constructor cannot delegate to another constructor 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 diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordConstructors.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordConstructors.java index e101cb70e559..b3d93178c0ed 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordConstructors.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingRecords/RecordConstructors.java @@ -15,6 +15,12 @@ record TypeMismatch(T t) { this.t = null; } } +record NameMismatch(int x, int y) { + public NameMismatch(int _x, int _y) { + x = _x; + y = _y; + } +} // Current spec draft allows this record VarArgMismatch(int... x) { public VarArgMismatch(int[] x) {