From eb85741b253a49f9c6de9eaa7d735ba2a39fc8ef Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 27 Sep 2023 11:36:08 +0200 Subject: [PATCH] [java-inspections] CreateConstructorParameterFromFieldFix: rewrite to ModCommand, using trackDeclaration() Also fixes IDEA-329769 'Add constructor parameter' hint and action discrepancy GitOrigin-RevId: f897347600bfdfd2ebf7a4de0209e5887323f091 --- .../UnqualifiedFieldAccessInspection.java | 6 + .../src/messages/QuickFixBundle.properties | 2 +- ...reateConstructorParameterFromFieldFix.java | 466 ++++++------------ .../impl/AssignFieldFromParameterAction.java | 41 +- .../impl/config/QuickFixFactoryImpl.java | 2 +- .../afterChainMultiFields.java | 24 + .../afterGenerateEnumPrivateConstructor.java | 2 +- .../beforeChainMultiFields.java | 21 + ...eateConstructorParameterFromFieldTest.java | 19 + 9 files changed, 264 insertions(+), 319 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainMultiFields.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/beforeChainMultiFields.java diff --git a/java/java-analysis-impl/src/com/siyeh/ig/style/UnqualifiedFieldAccessInspection.java b/java/java-analysis-impl/src/com/siyeh/ig/style/UnqualifiedFieldAccessInspection.java index 08503b56ec91..38400bb930f1 100644 --- a/java/java-analysis-impl/src/com/siyeh/ig/style/UnqualifiedFieldAccessInspection.java +++ b/java/java-analysis-impl/src/com/siyeh/ig/style/UnqualifiedFieldAccessInspection.java @@ -11,6 +11,7 @@ import com.siyeh.ig.fixes.AddThisQualifierFix; import org.jetbrains.annotations.NotNull; public class UnqualifiedFieldAccessInspection extends BaseInspection implements CleanupLocalInspectionTool { + public static final String SHORT_NAME = "UnqualifiedFieldAccess"; @Override public BaseInspectionVisitor buildVisitor() { @@ -23,6 +24,11 @@ public class UnqualifiedFieldAccessInspection extends BaseInspection implements return InspectionGadgetsBundle.message("unqualified.field.access.problem.descriptor"); } + @Override + public @NotNull String getShortName() { + return SHORT_NAME; + } + @Override public LocalQuickFix buildFix(Object... infos) { final PsiReferenceExpression expressionToQualify = (PsiReferenceExpression)infos[0]; diff --git a/java/java-analysis-impl/src/messages/QuickFixBundle.properties b/java/java-analysis-impl/src/messages/QuickFixBundle.properties index edbe2164b27c..fa7d9a4dba9e 100644 --- a/java/java-analysis-impl/src/messages/QuickFixBundle.properties +++ b/java/java-analysis-impl/src/messages/QuickFixBundle.properties @@ -398,7 +398,7 @@ add.0.to.classpath=Add ''{0}'' to classpath iterate.iterable=Iterate choose.fields.to.generate.constructor.parameters.for=Choose Fields to Generate Constructor Parameters for -choose.constructors.to.add.parameter.to=Choose Constructors to Add Parameter to +choose.constructors.to.add.parameter.to=Choose Constructors to Add Parameter To add.constructor.parameters=Add constructor parameters add.annotation.attribute.name.family.name=Add annotation attribute name diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java index cd17373ff9f0..665d57b35dd3 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java @@ -1,43 +1,27 @@ // 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.codeInsight.daemon.impl.quickfix; +import com.intellij.application.options.CodeStyle; import com.intellij.codeInsight.AnnotationTargetUtil; -import com.intellij.codeInsight.FileModificationService; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInsight.daemon.QuickFixBundle; -import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil; -import com.intellij.codeInsight.generation.PsiElementClassMember; +import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; import com.intellij.codeInsight.generation.PsiFieldMember; import com.intellij.codeInsight.generation.PsiMethodMember; -import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInsight.intention.impl.AssignFieldFromParameterAction; -import com.intellij.codeInsight.intention.impl.BaseIntentionAction; import com.intellij.codeInsight.intention.impl.FieldFromParameterUtils; -import com.intellij.codeInsight.intention.preview.IntentionPreviewInfo; -import com.intellij.codeInspection.ex.GlobalInspectionContextBase; -import com.intellij.ide.util.MemberChooser; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.editor.Editor; +import com.intellij.modcommand.*; import com.intellij.openapi.project.Project; -import com.intellij.openapi.ui.DialogWrapper; import com.intellij.openapi.util.Comparing; -import com.intellij.openapi.util.Computable; -import com.intellij.openapi.util.Key; -import com.intellij.openapi.util.UserDataHolderEx; -import com.intellij.openapi.util.text.StringUtil; +import com.intellij.openapi.util.Ref; import com.intellij.psi.*; -import com.intellij.psi.codeStyle.JavaCodeStyleManager; -import com.intellij.psi.codeStyle.JavaCodeStyleSettings; -import com.intellij.psi.codeStyle.SuggestedNameInfo; -import com.intellij.psi.codeStyle.VariableKind; +import com.intellij.psi.codeStyle.*; import com.intellij.psi.search.LocalSearchScope; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; -import com.intellij.refactoring.changeSignature.ParameterInfo; -import com.intellij.refactoring.changeSignature.ParameterInfoImpl; import com.intellij.util.CommonJavaRefactoringUtil; -import com.intellij.util.IncorrectOperationException; +import com.intellij.util.JavaPsiConstructorUtil; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; import org.jetbrains.annotations.NotNull; @@ -45,173 +29,62 @@ import org.jetbrains.annotations.Nullable; import java.util.*; -public class CreateConstructorParameterFromFieldFix implements IntentionAction { - private static final Key, Boolean>> FIELDS = Key.create("CONSTRUCTOR_PARAMS"); - - private final SmartPsiElementPointer myField; - private final PsiClass myClass; - private int myConstructorsLength; - +public class CreateConstructorParameterFromFieldFix extends PsiBasedModCommandAction { public CreateConstructorParameterFromFieldFix(@NotNull PsiField field) { - myClass = field.getContainingClass(); - myField = SmartPointerManager.getInstance(field.getProject()).createSmartPsiElementPointer(field); - if (myClass != null) { - getFieldsToFix().add(myField); - myConstructorsLength = myClass.getConstructors().length; - } + super(field); } @Override - @NotNull - public String getText() { - if (getFieldsToFix().size() > 1 && myConstructorsLength <= 1) return QuickFixBundle.message("add.constructor.parameters"); - return QuickFixBundle.message("add.constructor.parameter.name"); + protected @Nullable Presentation getPresentation(@NotNull ActionContext context, @NotNull PsiField field) { + if (field.hasModifierProperty(PsiModifier.STATIC)) return null; + PsiClass psiClass = field.getContainingClass(); + if (psiClass == null || psiClass instanceof PsiSyntheticClass || psiClass.isRecord() || psiClass.getName() == null) return null; + if (psiClass.getConstructors().length <= 1 && getFieldsToFix(psiClass, field).size() > 1) { + return Presentation.of(QuickFixBundle.message("add.constructor.parameters")); + } + return Presentation.of(QuickFixBundle.message("add.constructor.parameter.name")); } @Override @NotNull public String getFamilyName() { - return getText(); + return QuickFixBundle.message("add.constructor.parameters"); } @Override - public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) { - return (myClass == null || myClass.isValid()) && isAvailable(getField()); + protected @NotNull ModCommand perform(@NotNull ActionContext context, @NotNull PsiField field) { + PsiClass psiClass = Objects.requireNonNull(field.getContainingClass()); + PsiMethod[] constructors = getNonSyntheticConstructors(psiClass); + final List filtered = getFilteredConstructors(constructors, field); + if (filtered.size() <= 1) { + return performForConstructors(context, field, filtered); + } + List members = ContainerUtil.map(filtered, PsiMethodMember::new); + return new ModChooseMember(QuickFixBundle.message("choose.constructors.to.add.parameter.to"), + members, + members, + ModChooseMember.SelectionMode.MULTIPLE, + selected -> performForConstructors( + context, field, ContainerUtil.map(selected, member -> ((PsiMethodMember)member).getElement()))); } - private static boolean isAvailable(PsiField field) { - PsiClass containingClass = field == null ? null : field.getContainingClass(); - return field != null - && BaseIntentionAction.canModify(field) - && !field.hasModifierProperty(PsiModifier.STATIC) - && containingClass != null - && !(containingClass instanceof PsiSyntheticClass) - && !containingClass.isRecord() - && containingClass.getName() != null; - } - - @Override - public @NotNull IntentionPreviewInfo generatePreview(@NotNull Project project, @NotNull Editor editor, @NotNull PsiFile copyFile) { - PsiClass copyMyClass = PsiTreeUtil.findSameElementInCopy(myClass, copyFile); - PsiField myFieldElement = myField.getElement(); - if (myFieldElement == null) { - return IntentionPreviewInfo.EMPTY; - } - PsiField copyMyField = PsiTreeUtil.findSameElementInCopy(myFieldElement, copyFile); - PsiMethod[] constructors = getNonSyntheticConstructors(copyMyClass); - if (constructors.length == 0) { - AddDefaultConstructorFix.addDefaultConstructor(copyMyClass); - constructors = copyMyClass.getConstructors(); - } - List filteredConstructors = getFilteredConstructors(constructors, copyMyField); - if (filteredConstructors.isEmpty()) { - return IntentionPreviewInfo.EMPTY; - } - PsiMethod currentConstructor = filteredConstructors.get(0); - PsiParameterList parameterList = currentConstructor.getParameterList(); - final String uniqueParameterName = getUniqueParameterName(parameterList.getParameters(), copyMyField, Map.of()); - ParameterInfoImpl info = ParameterInfoImpl.createNew() - .withName(uniqueParameterName) - .withType(AnnotationTargetUtil.keepStrictlyTypeUseAnnotations(copyMyField.getModifierList(), copyMyField.getType())) - .withDefaultValue(uniqueParameterName); - int parametersCount = parameterList.getParametersCount(); - PsiParameter newParameter = - JavaPsiFacade.getElementFactory(project).createParameterFromText(info.getTypeText() + " " + info.getName(), currentConstructor); - PsiParameter afterInsert; - if (parametersCount == 0) { - afterInsert = (PsiParameter)parameterList.add(newParameter); - } - else { - PsiParameter lastParameter = parameterList.getParameter(parametersCount - 1); - if (lastParameter == null) { - return IntentionPreviewInfo.EMPTY; - } - if (lastParameter.isVarArgs()) { - afterInsert = (PsiParameter)parameterList.addBefore(newParameter, lastParameter); - } - else { - afterInsert = (PsiParameter)parameterList.addAfter(newParameter, lastParameter); - } - } - if (JavaHighlightUtil.getChainedConstructors(currentConstructor).isEmpty()) { - AssignFieldFromParameterAction.addFieldAssignmentStatement(project, copyMyField, afterInsert, editor); - } - return IntentionPreviewInfo.DIFF; - } - - @Override - public void invoke(@NotNull final Project project, final Editor editor, final PsiFile file) throws IncorrectOperationException { - if (!FileModificationService.getInstance().prepareFileForWrite(file)) return; - - PsiMethod[] constructors = getNonSyntheticConstructors(myClass); - if (constructors.length == 0) { - ApplicationManager.getApplication().runWriteAction((Computable)() -> AddDefaultConstructorFix.addDefaultConstructor(myClass)); - constructors = getNonSyntheticConstructors(myClass); - } - final List filtered = getFilteredConstructors(constructors, getField()); - final List> cleanupElements = new ArrayList<>(); - if (filtered.size() > 1) { - final PsiMethodMember[] members = new PsiMethodMember[filtered.size()]; - int i = 0; - for (PsiMethod constructor : filtered) { - members[i++] = new PsiMethodMember(constructor); - } - final List elements; - if (ApplicationManager.getApplication().isUnitTestMode()) { - elements = Arrays.asList(members); - } else { - final MemberChooser chooser = new MemberChooser<>(members, false, true, project); - chooser.setTitle(QuickFixBundle.message("choose.constructors.to.add.parameter.to")); - chooser.show(); - elements = chooser.getSelectedElements(); - if (elements == null) return; - } - - for (PsiMethodMember member : elements) { - if (!addParameterToConstructor(project, file, editor, member.getElement(), new PsiField[] {getField()}, cleanupElements)) { - break; - } - } - } - else if (!filtered.isEmpty()) { - final Collection> fieldsToFix = getFieldsToFix(); - try { - final PsiMethod constructor = filtered.get(0); - final LinkedHashSet fields = new LinkedHashSet<>(); - fieldsToFix.add(myField); - for (SmartPsiElementPointer elementPointer : fieldsToFix) { - final PsiField field = elementPointer.getElement(); - if (field != null && isAvailable(field) && filterConstructorsIfFieldAlreadyAssigned(new PsiMethod[]{constructor}, field).contains(constructor)) { - fields.add(field); - } - } - if (filtered.size() == constructors.length && fields.size() > 1 && !ApplicationManager.getApplication().isUnitTestMode()) { - PsiFieldMember[] members = new PsiFieldMember[fields.size()]; - int i = 0; - for (PsiField field : fields) { - members[i++] = new PsiFieldMember(field); - } - MemberChooser chooser = new MemberChooser<>(members, false, true, project); - chooser.setTitle(QuickFixBundle.message("choose.fields.to.generate.constructor.parameters.for")); - chooser.show(); - if (chooser.getExitCode() != DialogWrapper.OK_EXIT_CODE) return; - final List selectedElements = chooser.getSelectedElements(); - if (selectedElements == null) return; - fields.clear(); - for (PsiElementClassMember member : selectedElements) { - fields.add((PsiField)member.getElement()); - } - } - - addParameterToConstructor(project, file, editor, constructor, filtered.size() == constructors.length - ? fields.toArray(PsiField.EMPTY_ARRAY) - : new PsiField[]{getField()}, cleanupElements); - } - finally { - fieldsToFix.clear(); - } - } - GlobalInspectionContextBase.cleanupElements(project, null, cleanupElements); + @NotNull + private static ModCommand performForConstructors(@NotNull ActionContext context, + @NotNull PsiField field, + @NotNull List constructors) { + if (!field.isValid() || ContainerUtil.exists(constructors, c -> !c.isValid())) return ModCommand.nop(); + PsiClass psiClass = field.getContainingClass(); + if (psiClass == null) return ModCommand.nop(); + List allFields = getFieldsToFix(psiClass, field); + if (allFields.isEmpty()) return ModCommand.nop(); + if (allFields.size() == 1) return performForConstructorsAndFields(context, allFields, constructors); + List members = ContainerUtil.map(allFields, PsiFieldMember::new); + return new ModChooseMember(QuickFixBundle.message("choose.constructors.to.add.parameter.to"), + members, + members, + ModChooseMember.SelectionMode.MULTIPLE, + selected -> performForConstructorsAndFields( + context, ContainerUtil.map(selected, member -> ((PsiFieldMember)member).getElement()), constructors)); } private static PsiMethod[] getNonSyntheticConstructors(@NotNull PsiClass psiClass) { @@ -239,36 +112,17 @@ public class CreateConstructorParameterFromFieldFix implements IntentionAction { } @NotNull - private Collection> getFieldsToFix() { - Map, Boolean> fields = myClass.getUserData(FIELDS); - if (fields == null) { - fields = ((UserDataHolderEx)myClass).putUserDataIfAbsent(FIELDS, ContainerUtil.createConcurrentWeakMap()); + private static List getFieldsToFix(@NotNull PsiClass psiClass, @NotNull PsiField startField) { + List fields = new ArrayList<>(); + for (PsiField field : psiClass.getFields()) { + if (field == startField || + (!field.hasModifierProperty(PsiModifier.STATIC) && + field.hasModifierProperty(PsiModifier.FINAL) && + !HighlightControlFlowUtil.isFieldInitializedAfterObjectConstruction(field))) { + fields.add(field); + } } - final Map, Boolean> finalFields = fields; - return new AbstractCollection<>() { - @Override - public boolean add(SmartPsiElementPointer psiVariable) { - PsiField field = psiVariable.getElement(); - if (field == null || !isAvailable(field)) return false; - return finalFields.put(psiVariable, Boolean.TRUE) == null; - } - - @NotNull - @Override - public Iterator> iterator() { - return finalFields.keySet().iterator(); - } - - @Override - public int size() { - return finalFields.size(); - } - - @Override - public void clear() { - finalFields.clear(); - } - }; + return fields; } static List filterConstructorsIfFieldAlreadyAssigned(PsiMethod[] constructors, PsiField field) { @@ -282,68 +136,91 @@ public class CreateConstructorParameterFromFieldFix implements IntentionAction { return result; } - private static boolean addParameterToConstructor(final Project project, - final PsiFile file, - final Editor editor, - final PsiMethod constructor, - final PsiField[] fields, - final List> cleanupElements) throws IncorrectOperationException { - final PsiParameterList parameterList = constructor.getParameterList(); - final PsiParameter[] parameters = parameterList.getParameters(); - ParameterInfoImpl[] newParamInfos = new ParameterInfoImpl[parameters.length + fields.length]; - final List params = new ArrayList<>(Arrays.asList(parameters)); - Collections.addAll(params, fields); - params.sort(new FieldParameterComparator(parameterList)); + @NotNull + private static ModCommand performForConstructorsAndFields(ActionContext context, List fields, List constructors) { + return ModCommand.psiUpdate(context, updater -> { + List writableFields = ContainerUtil.map(fields, updater::getWritable); + PsiClass psiClass = writableFields.get(0).getContainingClass(); + if (psiClass == null) return; + List writableConstructors; + if (constructors.isEmpty()) { + writableConstructors = List.of(AddDefaultConstructorFix.addDefaultConstructor(psiClass)); + } + else { + writableConstructors = ContainerUtil.map(constructors, updater::getWritable); + } + for (PsiMethod constructor : writableConstructors) { + updater.trackDeclaration(constructor); + } + Map data = ChainedConstructorData.getChainedConstructorDataMap(writableFields, writableConstructors); + for (PsiMethod constructor : writableConstructors) { + addParameterToConstructor(context, constructor, writableFields, updater, data.get(constructor)); + } + }); + } - int i = 0; - final Map usedFields = new HashMap<>(); + private static void addParameterToConstructor(@NotNull ActionContext context, @NotNull PsiMethod constructor, + @NotNull List fields, @NotNull ModPsiUpdater updater, + @Nullable ChainedConstructorData chainedConstructorData) { + final PsiParameterList parameterList = constructor.getParameterList(); + final List params = fillVariables(fields, parameterList); + + final Map usedFields = new LinkedHashMap<>(); final MultiMap types = new MultiMap<>(); for (PsiVariable param : params) { types.putValue(param.getType(), param); } - final JavaCodeStyleSettings settings = JavaCodeStyleSettings.getInstance(file); - final boolean preferLongerNames = settings.PREFER_LONGER_NAMES; + Project project = context.project(); + PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); + CodeStyleSettings allSettings = CodeStyleSettingsManager.getInstance(project).cloneSettings(CodeStyle.getSettings(context.file())); + JavaCodeStyleSettings settings = allSettings.getCustomSettings(JavaCodeStyleSettings.class); + boolean preferLongerNames = settings.PREFER_LONGER_NAMES; + Ref prev = Ref.create(); for (PsiVariable param : params) { final PsiType paramType = param.getType(); - if (param instanceof PsiParameter) { - newParamInfos[i++] = ParameterInfoImpl.create(parameterList.getParameterIndex((PsiParameter)param)) - .withName(param.getName()) - .withType(paramType) - .withDefaultValue(param.getName()); + if (param instanceof PsiField field) { + settings.PREFER_LONGER_NAMES = preferLongerNames || types.get(paramType).size() > 1; + CodeStyle.runWithLocalSettings(project, allSettings, () -> { + final String uniqueParameterName = getUniqueParameterName(parameterList.getParameters(), param, usedFields); + usedFields.put(field, uniqueParameterName); + PsiParameter parameter = factory + .createParameter(uniqueParameterName, AnnotationTargetUtil.keepStrictlyTypeUseAnnotations(param.getModifierList(), paramType), + parameterList); + if (prev.isNull()) { + prev.set(parameterList.isEmpty() ? parameterList.add(parameter) : + parameterList.addBefore(parameter, parameterList.getParameter(0))); + } + else { + prev.set(parameterList.addAfter(parameter, prev.get())); + } + }); } else { - try { - settings.PREFER_LONGER_NAMES = preferLongerNames || types.get(paramType).size() > 1; - final String uniqueParameterName = getUniqueParameterName(parameters, param, usedFields); - usedFields.put((PsiField)param, uniqueParameterName); - newParamInfos[i++] = ParameterInfoImpl.createNew() - .withName(uniqueParameterName) - .withType(AnnotationTargetUtil.keepStrictlyTypeUseAnnotations(param.getModifierList(), paramType)) - .withDefaultValue(uniqueParameterName); - } - finally { - settings.PREFER_LONGER_NAMES = preferLongerNames; - } + prev.set(param); + } + } + if (chainedConstructorData != null) { + chainedConstructorData.updateChainedCall(constructor, usedFields); + } else { + PsiParameter[] newParameters = constructor.getParameterList().getParameters(); + // do not introduce assignment in chained constructor + for (Map.Entry entry : usedFields.entrySet()) { + PsiField field = entry.getKey(); + final String parameterName = entry.getValue(); + PsiParameter parameter = findParamByName(parameterName, newParameters); + if (parameter == null) continue; + NullableNotNullManager.getInstance(field.getProject()).copyNullableOrNotNullAnnotation(field, parameter); + AssignFieldFromParameterAction.addFieldAssignmentStatement(project, field, parameter, updater); } } - final SmartPointerManager manager = SmartPointerManager.getInstance(project); - final SmartPsiElementPointer constructorPointer = manager.createSmartPsiElementPointer(constructor); - - final PsiMethod fromText = JavaPsiFacade.getElementFactory(project).createMethodFromText(createDummyMethod(constructor, newParamInfos), - constructor); - final PsiClass containingClass = constructor.getContainingClass(); - if (containingClass == null) return false; - final int minUsagesNumber = containingClass.findMethodsBySignature(fromText, false).length > 0 ? 0 : 1; - final List parameterInfos = - ChangeMethodSignatureFromUsageFix.performChange(project, editor, file, constructor, minUsagesNumber, newParamInfos, true, true, (List p) -> { - final ParameterInfo[] resultParams = p.toArray(new ParameterInfo[0]); - doCreate(project, editor, parameters, constructorPointer, resultParams, usedFields, cleanupElements); - } ); - return parameterInfos != null; } - private static String createDummyMethod(PsiMethod constructor, ParameterInfoImpl[] newParamInfos) { - return constructor.getName() + "(" + StringUtil.join(newParamInfos, info -> info.getTypeText() + " " + info.getName(), ", ") + "){}"; + @NotNull + private static List fillVariables(@NotNull List fields, @NotNull PsiParameterList parameterList) { + final List params = new ArrayList<>(Arrays.asList(parameterList.getParameters())); + params.addAll(fields); + params.sort(new FieldParameterComparator(parameterList)); + return params; } private static String getUniqueParameterName(PsiParameter[] parameters, PsiVariable variable, Map usedNames) { @@ -372,68 +249,49 @@ public class CreateConstructorParameterFromFieldFix implements IntentionAction { return true; } - private static boolean doCreate(Project project, Editor editor, PsiParameter[] parameters, SmartPsiElementPointer constructorPointer, - ParameterInfo[] parameterInfos, Map fields, List> cleanupElements) { - PsiMethod constructor = (PsiMethod)constructorPointer.getElement(); - assert constructor != null; - PsiParameter[] newParameters = constructor.getParameterList().getParameters(); - if (newParameters == parameters) return false; //user must have canceled dialog - // do not introduce assignment in chained constructor - if (JavaHighlightUtil.getChainedConstructors(constructor).isEmpty()) { - final SmartPointerManager manager = SmartPointerManager.getInstance(project); - boolean created = false; - for (Map.Entry entry : fields.entrySet()) { - PsiField field = entry.getKey(); - final String defaultParamName = entry.getValue(); - PsiParameter parameter = findParamByName(defaultParamName, field.getType(), newParameters, parameterInfos); - if (parameter == null) { - continue; - } - NullableNotNullManager.getInstance(field.getProject()).copyNullableOrNotNullAnnotation(field, parameter); - cleanupElements.add(manager.createSmartPsiElementPointer(parameter)); - final PsiElement assignmentStatement = AssignFieldFromParameterAction.addFieldAssignmentStatement(project, field, parameter, editor); - if (assignmentStatement != null) { - cleanupElements.add(manager.createSmartPsiElementPointer(assignmentStatement)); - } - created = true; - } - return created; - } else { - return true; - } - } - @Nullable - private static PsiParameter findParamByName(String newName, - PsiType type, - PsiParameter[] newParameters, - ParameterInfo[] parameterInfos) { + private static PsiParameter findParamByName(@NotNull String newName, @NotNull PsiParameter @NotNull [] newParameters) { for (PsiParameter newParameter : newParameters) { if (Comparing.strEqual(newName, newParameter.getName())) { return newParameter; } } - for (int i = 0; i < newParameters.length; i++) { - if (parameterInfos[i].isNew()) { - final PsiParameter parameter = newParameters[i]; - final PsiType paramType = ((ParameterInfoImpl)parameterInfos[i]).getTypeWrapper().getType(parameter); - if (type.isAssignableFrom(paramType)){ - return parameter; - } - } - } return null; } - private PsiField getField() { - return myField.getElement(); - } + record ChainedConstructorData(@NotNull PsiMethodCallExpression methodCall, @NotNull List thisCallVariables) { + @NotNull + private static Map getChainedConstructorDataMap(List fields, List writableConstructors) { + Map data = new HashMap<>(); + for (PsiMethod constructor : writableConstructors) { + PsiMethodCallExpression methodCall = JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(constructor); + if (methodCall != null) { + PsiMethod target = methodCall.resolveMethod(); + if (target != null && writableConstructors.contains(target)) { + data.put(constructor, new ChainedConstructorData(methodCall, fillVariables(fields, target.getParameterList()))); + } + } + } + return data; + } - @Override - public boolean startInWriteAction() { - return false; + private void updateChainedCall(@NotNull PsiMethod constructor, @NotNull Map usedFields) { + int index = 0; + PsiExpressionList argumentList = methodCall.getArgumentList(); + PsiExpression[] args = argumentList.getExpressions(); + PsiElementFactory factory = JavaPsiFacade.getElementFactory(constructor.getProject()); + for (PsiVariable variable : thisCallVariables) { + if (variable instanceof PsiParameter) { + index++; + } else if (variable instanceof PsiField field) { + String parameterName = usedFields.get(field); + PsiExpression arg = factory.createExpressionFromText(parameterName, constructor); + argumentList.addAfter(arg, index == 0 ? null : args[index - 1]); + } + } + } } - + private static class FieldParameterComparator implements Comparator { private final PsiParameterList myParameterList; diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java index 645c268b797e..3499a6e5ee62 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java @@ -1,22 +1,23 @@ // 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.codeInsight.intention.impl; +import com.intellij.codeHighlighting.HighlightDisplayLevel; import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil; +import com.intellij.codeInspection.ex.ToolsImpl; import com.intellij.java.JavaBundle; import com.intellij.lang.java.JavaLanguage; import com.intellij.modcommand.*; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; +import com.intellij.profile.codeInspection.InspectionProfileManager; import com.intellij.psi.*; -import com.intellij.psi.codeStyle.CodeStyleManager; -import com.intellij.psi.codeStyle.JavaCodeStyleManager; -import com.intellij.psi.codeStyle.SuggestedNameInfo; -import com.intellij.psi.codeStyle.VariableKind; +import com.intellij.psi.codeStyle.*; import com.intellij.psi.controlFlow.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; import com.siyeh.ig.psiutils.VariableAccessUtils; +import com.siyeh.ig.style.UnqualifiedFieldAccessInspection; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -102,6 +103,7 @@ public class AssignFieldFromParameterAction extends PsiUpdateModCommandAction 0 ? statements[i - 1] : null); + inserted = (PsiStatement)methodBody.addAfter(assignmentStmt, i > 0 ? statements[i - 1] : null); } if (updater != null) { updater.moveTo(inserted.getTextRange().getEndOffset()); diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java index 932e646976dd..836587bf560f 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/config/QuickFixFactoryImpl.java @@ -606,7 +606,7 @@ public final class QuickFixFactoryImpl extends QuickFixFactory { @NotNull @Override public IntentionAction createCreateConstructorParameterFromFieldFix(@NotNull PsiField field) { - return new CreateConstructorParameterFromFieldFix(field); + return new CreateConstructorParameterFromFieldFix(field).asIntention(); } @NotNull diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainMultiFields.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainMultiFields.java new file mode 100644 index 000000000000..36b303713599 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainMultiFields.java @@ -0,0 +1,24 @@ +// "Add constructor parameter" "true" +class X { + final int a; + final int b; + final int c; + final int d; + final int e; + + X(int a, int b, int c, int d, int e, String... extra) { + this.a = a; + this.b = b; + this.c = c; + this.d = d; + this.e = e; + } + + X(int a, int c, int e) { + this(a, 1, c, 2, e); + } + + X(String s1, String s2, int a, int c, int e) { + this(a, 3, c, 4, e, s1, s2); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterGenerateEnumPrivateConstructor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterGenerateEnumPrivateConstructor.java index 2c8757516f39..7e2f3b498f75 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterGenerateEnumPrivateConstructor.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterGenerateEnumPrivateConstructor.java @@ -1,7 +1,7 @@ // "Add constructor parameter" "true" public enum MyEnum { - VALUE_ONE(myField); + VALUE_ONE(0); private final int myField; MyEnum(int myField) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/beforeChainMultiFields.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/beforeChainMultiFields.java new file mode 100644 index 000000000000..e638a52c2832 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/beforeChainMultiFields.java @@ -0,0 +1,21 @@ +// "Add constructor parameter" "true" +class X { + final int a; + final int b; + final int c; + final int d; + final int e; + + X(int b, int d, String... extra) { + this.b = b; + this.d = d; + } + + X() { + this(1, 2); + } + + X(String s1, String s2) { + this(3, 4, s1, s2); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldTest.java index 9fb89dd33977..f950d9d89530 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldTest.java @@ -3,9 +3,13 @@ package com.intellij.codeInsight.daemon.impl.quickfix; import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; import com.intellij.codeInspection.deadCode.UnusedDeclarationInspection; +import com.intellij.psi.JavaPsiFacade; import com.intellij.psi.codeStyle.JavaCodeStyleSettings; +import com.intellij.refactoring.suggested.PerformSuggestedRefactoringKt; +import com.intellij.refactoring.suggested.SuggestedRefactoringExecution; import com.siyeh.ig.style.MissortedModifiersInspection; import com.siyeh.ig.style.UnqualifiedFieldAccessInspection; +import kotlin.jvm.functions.Function1; public class CreateConstructorParameterFromFieldTest extends LightQuickFixParameterizedTestCase { @Override @@ -22,4 +26,19 @@ public class CreateConstructorParameterFromFieldTest extends LightQuickFixParame protected String getBasePath() { return "/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField"; } + + @Override + public void runSingle() throws Throwable { + Function1 defaultValue = + PerformSuggestedRefactoringKt.get_suggestedChangeSignatureNewParameterValuesForTests(); + try { + PerformSuggestedRefactoringKt.set_suggestedChangeSignatureNewParameterValuesForTests( + idx -> new SuggestedRefactoringExecution.NewParameterValue.Expression( + JavaPsiFacade.getElementFactory(getProject()).createExpressionFromText(String.valueOf(idx), null))); + super.runSingle(); + } + finally { + PerformSuggestedRefactoringKt.set_suggestedChangeSignatureNewParameterValuesForTests(defaultValue); + } + } }