[java-refactoring] IDEA-360690 Inline method: replace inaccessible field references with getters/setters when available

GitOrigin-RevId: 287af8db31ae356162add06e3564c4052025a026
This commit is contained in:
Tagir Valeev
2025-04-15 07:43:30 +00:00
committed by intellij-monorepo-bot
parent c94b90dfe8
commit 87fcb4eece
16 changed files with 386 additions and 117 deletions
@@ -171,10 +171,6 @@ public abstract class QuickFixFactory {
public abstract @NotNull IntentionAction createChangeParameterClassFix(@NotNull PsiClass aClass, @NotNull PsiClassType type);
public abstract @NotNull IntentionAction createReplaceInaccessibleFieldWithGetterSetterFix(@NotNull PsiReferenceExpression element,
@NotNull PsiMethod getter,
boolean isSetter);
public abstract @NotNull IntentionAction createSurroundWithArrayFix(@Nullable PsiCall methodCall, @Nullable PsiExpression expression);
public abstract @NotNull IntentionAction createImplementAbstractClassMethodsFix(@NotNull PsiElement elementToHighlight);
@@ -96,7 +96,12 @@ public final class HighlightFixUtil {
if (qualifier instanceof PsiExpression) {
accessObjectClass = (PsiClass)PsiUtil.getAccessObjectClass((PsiExpression)qualifier).getElement();
}
registerReplaceInaccessibleFieldWithGetterSetterFix(info, refElement, place, accessObjectClass);
if (place instanceof PsiReferenceExpression ref) {
FieldAccessFixer fixer = FieldAccessFixer.create(ref, refElement, place);
if (fixer != null) {
info.accept(new ReplaceInaccessibleFieldWithGetterSetterFix(ref, fixer));
}
}
if (refElement instanceof PsiCompiledElement) return;
PsiModifierList modifierList = refElement.getModifierList();
@@ -257,37 +262,6 @@ public final class HighlightFixUtil {
return QuickFixFactory.getInstance().createChangeParameterClassFix(rClass, (PsiClassType)lType);
}
private static void registerReplaceInaccessibleFieldWithGetterSetterFix(@NotNull Consumer<? super CommonIntentionAction> info,
@NotNull PsiMember refElement,
@NotNull PsiJavaCodeReferenceElement place,
@Nullable PsiClass accessObjectClass) {
if (refElement instanceof PsiField psiField && place instanceof PsiReferenceExpression ref) {
if (PsiTypes.nullType().equals(psiField.getType())) return;
PsiClass containingClass = psiField.getContainingClass();
if (containingClass != null) {
if (PsiUtil.isOnAssignmentLeftHand(ref)) {
PsiMethod setterPrototype = PropertyUtilBase.generateSetterPrototype(psiField);
PsiMethod setter = containingClass.findMethodBySignature(setterPrototype, true);
if (setter != null && PsiUtil.isAccessible(setter, ref, accessObjectClass)) {
PsiElement element = PsiTreeUtil.skipParentsOfType(ref, PsiParenthesizedExpression.class);
if (element instanceof PsiAssignmentExpression && ((PsiAssignmentExpression)element).getOperationTokenType() == JavaTokenType.EQ) {
IntentionAction action = QuickFixFactory.getInstance().createReplaceInaccessibleFieldWithGetterSetterFix(ref, setter, true);
info.accept(action);
}
}
}
else if (PsiUtil.isAccessedForReading(ref)) {
PsiMethod getterPrototype = PropertyUtilBase.generateGetterPrototype(psiField);
PsiMethod getter = containingClass.findMethodBySignature(getterPrototype, true);
if (getter != null && PsiUtil.isAccessible(getter, ref, accessObjectClass)) {
IntentionAction action = QuickFixFactory.getInstance().createReplaceInaccessibleFieldWithGetterSetterFix(ref, getter, false);
info.accept(action);
}
}
}
}
}
static void registerLambdaReturnTypeFixes(@NotNull Consumer<? super CommonIntentionAction> info, PsiLambdaExpression lambda, PsiExpression expression) {
PsiType type = LambdaUtil.getFunctionalInterfaceReturnType(lambda);
if (type != null) {
@@ -0,0 +1,37 @@
// Copyright 2000-2025 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.codeInsight.daemon.QuickFixBundle;
import com.intellij.modcommand.ActionContext;
import com.intellij.modcommand.ModPsiUpdater;
import com.intellij.modcommand.Presentation;
import com.intellij.modcommand.PsiUpdateModCommandAction;
import com.intellij.psi.PsiReferenceExpression;
import com.siyeh.ig.psiutils.FieldAccessFixer;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class ReplaceInaccessibleFieldWithGetterSetterFix extends PsiUpdateModCommandAction<PsiReferenceExpression> {
@NotNull private final FieldAccessFixer myFixer;
public ReplaceInaccessibleFieldWithGetterSetterFix(@NotNull PsiReferenceExpression ref, @NotNull FieldAccessFixer fixer) {
super(ref);
myFixer = fixer;
}
@Override
protected void invoke(@NotNull ActionContext context, @NotNull PsiReferenceExpression place, @NotNull ModPsiUpdater updater) {
myFixer.apply(place);
}
@Override
protected @Nullable Presentation getPresentation(@NotNull ActionContext context, @NotNull PsiReferenceExpression element) {
String message = myFixer.setter() ? QuickFixBundle.message("replace.with.setter") : QuickFixBundle.message("replace.with.getter");
return Presentation.of(message).withFixAllOption(this);
}
@Override
public @NotNull String getFamilyName() {
return QuickFixBundle.message("replace.with.getter.setter");
}
}
@@ -0,0 +1,125 @@
// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.siyeh.ig.psiutils;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.util.PropertyUtil;
import com.intellij.psi.util.PropertyUtilBase;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.function.Predicate;
/**
* A utility that can replace a direct field access with an accessor method call.
*
* @param accessor accessor method
* @param kind accessor kind (exact, overridable, name-based)
* @param setter if true, the accessor is a setter
*/
public record FieldAccessFixer(@NotNull String accessorName, @NotNull AccessorKind kind, boolean setter) {
/**
* A kind of accessor method, namely how good the method replaces a direct field access.
*/
public enum AccessorKind {
/**
* Accessor accesses the field and cannot be overridden (static, final, or in final class):
* replacing a field reference with an accessor is safe.
*/
EXACT,
/**
* Accessor accesses the field but can be overridden. Replacing a field reference with an accessor
* may be not completely safe if it's overridden in a subclass.
*/
OVERRIDABLE,
/**
* Accessor method is selected based on its name (e.g., {@code getSomething} for a field named {@code something}).
* No guarantee that it does the same.
*/
NAME_BASED
}
public void apply(@NotNull PsiReferenceExpression ref) {
String qualifier = null;
final PsiExpression qualifierExpression = ref.getQualifierExpression();
if (qualifierExpression != null) {
qualifier = qualifierExpression.getText();
}
Project project = ref.getProject();
PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(project);
PsiMethodCallExpression callExpression;
final String call = (qualifier != null ? qualifier + "." : "") + accessorName;
if (!setter) {
callExpression = (PsiMethodCallExpression)elementFactory.createExpressionFromText(call + "()", null);
callExpression = (PsiMethodCallExpression)CodeStyleManager.getInstance(project).reformat(callExpression);
ref.replace(callExpression);
} else {
PsiElement parent = PsiTreeUtil.skipParentsOfType(ref, PsiParenthesizedExpression.class);
if (parent instanceof PsiAssignmentExpression assignmentExpression) {
final PsiExpression rExpression = assignmentExpression.getRExpression();
final String argList = rExpression != null ? rExpression.getText() : "";
callExpression = (PsiMethodCallExpression)elementFactory.createExpressionFromText(call + "(" + argList + ")", null);
callExpression = (PsiMethodCallExpression)CodeStyleManager.getInstance(project).reformat(callExpression);
parent.replace(callExpression);
}
}
}
/**
* @param ref reference to a potentially inaccessible field
* @param target the symbol the ref resolves to at a current place
* @param place place where reference occurs. Can be the ref itself, or probably a place where the ref is about to be inlined
* @return the fixer which will replace the direct field access with an accessor method call;
* null if there's no suitable accessor method or the field is already accessible.
*/
public static @Nullable FieldAccessFixer create(@NotNull PsiReferenceExpression ref, @Nullable PsiElement target,
@NotNull PsiElement place) {
if (!(target instanceof PsiField field)) return null;
PsiElement qualifier = ref.getQualifier();
PsiClass accessObjectClass =
qualifier instanceof PsiExpression expression ? (PsiClass)PsiUtil.getAccessObjectClass(expression).getElement() : null;
if (PsiUtil.isAccessible(field, place, accessObjectClass)) return null;
Predicate<PsiMethod> accessTest = m -> PsiUtil.isAccessible(m, place, accessObjectClass);
if (PsiTypes.nullType().equals(field.getType())) return null;
PsiClass containingClass = field.getContainingClass();
if (containingClass == null) return null;
PsiElement element = PsiTreeUtil.skipParentsOfType(ref, PsiParenthesizedExpression.class);
boolean setter;
PsiMethod accessor;
AccessorKind kind;
PsiMethod prototype = null;
if (element instanceof PsiAssignmentExpression assignment && assignment.getOperationTokenType() == JavaTokenType.EQ &&
PsiTreeUtil.isAncestor(assignment.getLExpression(), ref, false)) {
setter = true;
accessor = ContainerUtil.find(containingClass.getMethods(),
method -> PropertyUtil.getFieldOfSetter(method) == field && accessTest.test(method));
if (accessor == null) {
prototype = PropertyUtilBase.generateSetterPrototype(field);
}
}
else if (PsiUtil.isAccessedForReading(ref)) {
setter = false;
accessor = ContainerUtil.find(containingClass.getMethods(),
method -> PropertyUtil.getFieldOfGetter(method) == field && accessTest.test(method));
if (accessor == null) {
prototype = PropertyUtilBase.generateGetterPrototype(field);
}
}
else {
// Increment/decrement, compound update is not supported
return null;
}
if (prototype != null) {
accessor = containingClass.findMethodBySignature(prototype, true);
if (accessor == null || !accessTest.test(accessor)) return null;
kind = AccessorKind.NAME_BASED;
} else {
kind = PsiUtil.canBeOverridden(accessor) ? AccessorKind.OVERRIDABLE : AccessorKind.EXACT;
}
return new FieldAccessFixer(accessor.getName(), kind, setter);
}
}
@@ -49,10 +49,7 @@ import com.intellij.util.IncorrectOperationException;
import com.intellij.util.JavaPsiConstructorUtil;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.MultiMap;
import com.siyeh.ig.psiutils.CodeBlockSurrounder;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.SideEffectChecker;
import com.siyeh.ig.psiutils.VariableNameGenerator;
import com.siyeh.ig.psiutils.*;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -647,9 +644,11 @@ public class InlineMethodProcessor extends BaseRefactoringProcessor {
InlineMethodHelper helper = new InlineMethodHelper(myProject, myMethod, myMethodCopy, methodCall);
BlockData blockData = prepareBlock(ref, helper);
ChangeContextUtil.encodeContextInfo(blockData.block, false);
PsiCodeBlock block = blockData.block;
replaceWithAccessors(ref, block);
ChangeContextUtil.encodeContextInfo(block, false);
helper.substituteTypes(blockData.parmVars);
InlineUtil.solveLocalNameConflicts(blockData.block, ref, myMethodCopy.getBody());
InlineUtil.solveLocalNameConflicts(block, ref, myMethodCopy.getBody());
helper.initializeParameters(blockData.parmVars);
addThisInitializer(methodCall, blockData.thisVar);
@@ -661,15 +660,15 @@ public class InlineMethodProcessor extends BaseRefactoringProcessor {
PsiLocalVariable thisVar = null;
PsiLocalVariable[] parmVars = new PsiLocalVariable[blockData.parmVars.length];
PsiLocalVariable resultVar = null;
PsiStatement[] statements = blockData.block.getStatements();
PsiElement firstBodyElement = blockData.block.getFirstBodyElement();
PsiStatement[] statements = block.getStatements();
PsiElement firstBodyElement = block.getFirstBodyElement();
if (firstBodyElement instanceof PsiWhiteSpace) firstBodyElement = PsiTreeUtil.skipWhitespacesForward(firstBodyElement);
PsiElement firstAdded = null;
if (firstBodyElement != null && firstBodyElement != blockData.block.getRBrace()) {
if (firstBodyElement != null && firstBodyElement != block.getRBrace()) {
int last = statements.length - 1;
final PsiElement rBraceOrReturnStatement =
last >= 0 ? PsiTreeUtil.skipWhitespacesAndCommentsForward(statements[last]) : blockData.block.getLastBodyElement();
last >= 0 ? PsiTreeUtil.skipWhitespacesAndCommentsForward(statements[last]) : block.getLastBodyElement();
LOG.assertTrue(rBraceOrReturnStatement != null);
final PsiElement beforeRBraceStatement = rBraceOrReturnStatement.getPrevSibling();
LOG.assertTrue(beforeRBraceStatement != null);
@@ -703,7 +702,6 @@ public class InlineMethodProcessor extends BaseRefactoringProcessor {
}
}
PsiClass thisClass = myMethod.getContainingClass();
PsiExpression thisAccessExpr;
if (thisVar != null) {
@@ -732,6 +730,20 @@ public class InlineMethodProcessor extends BaseRefactoringProcessor {
ChangeContextUtil.clearContextInfo(anchorParent);
}
private static void replaceWithAccessors(PsiReferenceExpression ref, PsiCodeBlock block) {
List<PsiReferenceExpression> list = SyntaxTraverser.psiTraverser(block).filter(PsiReferenceExpression.class).toList();
// Iterate in opposite order, so in case of nested accessors, we first replace method arguments, then methods itself
for (int i = list.size() - 1; i >= 0; i--) {
PsiReferenceExpression r = list.get(i);
if (!r.isValid()) continue;
FieldAccessFixer fixer = FieldAccessFixer.create(r, r.resolve(), ref);
// Name-based is too risky for inline
if (fixer != null && fixer.kind() != FieldAccessFixer.AccessorKind.NAME_BASED) {
fixer.apply(r);
}
}
}
static @Nullable PsiReferenceExpression replaceCall(@NotNull PsiElementFactory factory,
@NotNull PsiMethodCallExpression methodCall,
@Nullable PsiElement firstAdded,
@@ -1,63 +0,0 @@
// Copyright 2000-2024 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.codeInsight.daemon.QuickFixBundle;
import com.intellij.modcommand.ActionContext;
import com.intellij.modcommand.ModPsiUpdater;
import com.intellij.modcommand.Presentation;
import com.intellij.modcommand.PsiUpdateModCommandAction;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.util.PsiTreeUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class ReplaceInaccessibleFieldWithGetterSetterFix extends PsiUpdateModCommandAction<PsiReferenceExpression> {
private final String myMethodName;
private final boolean myIsSetter;
public ReplaceInaccessibleFieldWithGetterSetterFix(@NotNull PsiReferenceExpression element, @NotNull PsiMethod getter, boolean isSetter) {
super(element);
myMethodName = getter.getName();
myIsSetter = isSetter;
}
@Override
protected void invoke(@NotNull ActionContext context, @NotNull PsiReferenceExpression place, @NotNull ModPsiUpdater updater) {
String qualifier = null;
final PsiExpression qualifierExpression = place.getQualifierExpression();
if (qualifierExpression != null) {
qualifier = qualifierExpression.getText();
}
Project project = context.project();
PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(project);
PsiMethodCallExpression callExpression;
final String call = (qualifier != null ? qualifier + "." : "") + myMethodName;
if (!myIsSetter) {
callExpression = (PsiMethodCallExpression)elementFactory.createExpressionFromText(call + "()", null);
callExpression = (PsiMethodCallExpression)CodeStyleManager.getInstance(project).reformat(callExpression);
place.replace(callExpression);
} else {
PsiElement parent = PsiTreeUtil.skipParentsOfType(place, PsiParenthesizedExpression.class);
if (parent instanceof PsiAssignmentExpression) {
final PsiExpression rExpression = ((PsiAssignmentExpression)parent).getRExpression();
final String argList = rExpression != null ? rExpression.getText() : "";
callExpression = (PsiMethodCallExpression)elementFactory.createExpressionFromText(call + "(" + argList + ")", null);
callExpression = (PsiMethodCallExpression)CodeStyleManager.getInstance(project).reformat(callExpression);
parent.replace(callExpression);
}
}
}
@Override
protected @Nullable Presentation getPresentation(@NotNull ActionContext context, @NotNull PsiReferenceExpression element) {
String message = myIsSetter ? QuickFixBundle.message("replace.with.setter") : QuickFixBundle.message("replace.with.getter");
return Presentation.of(message).withFixAllOption(this);
}
@Override
public @NotNull String getFamilyName() {
return QuickFixBundle.message("replace.with.getter.setter");
}
}
@@ -337,13 +337,6 @@ public final class QuickFixFactoryImpl extends QuickFixFactory {
return new ChangeParameterClassFix(aClass, type);
}
@Override
public @NotNull IntentionAction createReplaceInaccessibleFieldWithGetterSetterFix(@NotNull PsiReferenceExpression element,
@NotNull PsiMethod getter,
boolean isSetter) {
return new ReplaceInaccessibleFieldWithGetterSetterFix(element, getter, isSetter).asIntention();
}
@Override
public @NotNull IntentionAction createSurroundWithArrayFix(@Nullable PsiCall methodCall, @Nullable PsiExpression expression) {
return new SurroundWithArrayFix(methodCall, expression).asIntention();
@@ -0,0 +1,20 @@
class User {
private String name;
String name() {
return name;
}
void name(String name) {
this.name = name;
}
void abbreviateName() {
name = name.substring(0, 100);
}
}
class Use {
void test(User user) {
user.<caret>abbreviateName();
}
}
@@ -0,0 +1,17 @@
class User {
private String name;
String name() {
return name;
}
void name(String name) {
this.name = name;
}
}
class Use {
void test(User user) {
user.name(user.name().substring(0, 100));
}
}
@@ -0,0 +1,25 @@
class Complex {
private final double x, y;
Complex(double x, double y) {
this.x = x;
this.y = y;
}
double getModulus() {
return Math.hypot(x, y);
}
double getX() {
return x;
}
double getY() {
return y;
}
}
class Use {
void test(Complex c) {
System.out.println(c.getM<caret>odulus());
}
}
@@ -0,0 +1,21 @@
class Complex {
private final double x, y;
Complex(double x, double y) {
this.x = x;
this.y = y;
}
double getX() {
return x;
}
double getY() {
return y;
}
}
class Use {
void test(Complex c) {
System.out.println(Math.hypot(c.getX(), c.getY()));
}
}
@@ -0,0 +1,25 @@
class Complex {
final double x, y;
Complex(double x, double y) {
this.x = x;
this.y = y;
}
double getModulus() {
return Math.hypot(x, y);
}
double getX() {
return x;
}
double getY() {
return y;
}
}
class Use {
void test(Complex c) {
System.out.println(c.getM<caret>odulus());
}
}
@@ -0,0 +1,21 @@
class Complex {
final double x, y;
Complex(double x, double y) {
this.x = x;
this.y = y;
}
double getX() {
return x;
}
double getY() {
return y;
}
}
class Use {
void test(Complex c) {
System.out.println(Math.hypot(c.x, c.y));
}
}
@@ -0,0 +1,25 @@
class Complex {
private final double x, y;
Complex(double x, double y) {
this.x = x;
this.y = y;
}
double getModulus() {
return Math.hypot(x, y);
}
double getX() {
return x+1;
}
double getY() {
return y;
}
}
class Use {
void test(Complex c) {
System.out.println(c.getM<caret>odulus());
}
}
@@ -0,0 +1,21 @@
class Complex {
private final double x, y;
Complex(double x, double y) {
this.x = x;
this.y = y;
}
double getX() {
return x+1;
}
double getY() {
return y;
}
}
class Use {
void test(Complex c) {
System.out.println(Math.hypot(c.x, c.getY()));
}
}
@@ -626,6 +626,26 @@ public class InlineMethodTest extends LightRefactoringTestCase {
TestDialogManager.setTestDialog(TestDialog.YES, getTestRootDisposable());
doTest();
}
public void testAutomaticGetterUse() {
TestDialogManager.setTestDialog(TestDialog.YES, getTestRootDisposable());
BaseRefactoringProcessor.ConflictsInTestsException.withIgnoredConflicts(() -> doTest());
}
public void testNoAutomaticGetterUseGetterDoesDifferentThing() {
TestDialogManager.setTestDialog(TestDialog.YES, getTestRootDisposable());
BaseRefactoringProcessor.ConflictsInTestsException.withIgnoredConflicts(() -> doTest());
}
public void testNoAutomaticGetterUseAccessibleField() {
TestDialogManager.setTestDialog(TestDialog.YES, getTestRootDisposable());
doTest();
}
public void testAutomaticGetterSetterUse() {
TestDialogManager.setTestDialog(TestDialog.YES, getTestRootDisposable());
BaseRefactoringProcessor.ConflictsInTestsException.withIgnoredConflicts(() -> doTest());
}
@Override
protected Sdk getProjectJDK() {