Skip to content

NETBEANS-9798 Fix PullUp/Move refactoring when a not initialized field is moved to an interface #9802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jherkel wants to merge 1 commit into
base: master
Choose a base branch
from
+94 −7
Open

NETBEANS-9798 Fix PullUp/Move refactoring when a not initialized field is moved to an interface #9802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/Bundle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ ERR_PullUp_MemberAlreadyExists=Member "{0}" already exists in the target type.
ERR_CannotPullupDuplicateMembers=Cannot pull up two members with the same signature.
ERR_PullupNonPublicToInterface=Member "{0}" is being moved to interface, but is not public.
ERR_PullupStaticToInterface=Member "{0}" cannot be pulled up to interface, it is static.
ERR_PullupNonNotInitializedToInterface=Member "{0}" cannot be pulled up to interface, it is not initialized.

@wrprice wrprice Nov 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this buries the lede here. Yes, the resulting syntax error is that the pulled-up field is not initialized, but that's because the refactoring has moved a non-static member field into an interface, which makes it implicitly public static final.

Making the moved field static is a much more significant change. If the proposal is to block the refactor operation then this (non-final non-static to static final) is the more important reason to surface. Suggesting to add an initializer avoids the syntax error but the developer may miss the larger semantic difference it creates.


TXT_PullUp_AddMethod=Add abstract method\: <b>{0}</b>
# Move member to the target type (0 = member name)
Expand Down
19 changes: 16 additions & 3 deletions ....java/src/org/netbeans/modules/refactoring/java/plugins/MoveMembersRefactoringPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.netbeans.modules.refactoring.java.plugins;

import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import com.sun.source.util.TreePath;
import java.io.IOException;
import java.net.URL;
Expand All @@ -29,20 +30,17 @@
import javax.lang.model.util.ElementFilter;
import org.netbeans.api.java.classpath.ClassPath;
import org.netbeans.api.java.source.*;
import org.netbeans.api.java.source.ClasspathInfo.PathKind;
import org.netbeans.api.project.FileOwnerQuery;
import org.netbeans.api.project.Project;
import org.netbeans.api.project.ProjectUtils;
import org.netbeans.modules.refactoring.api.AbstractRefactoring;
import org.netbeans.modules.refactoring.api.MoveRefactoring;
import org.netbeans.modules.refactoring.api.Problem;
import org.netbeans.modules.refactoring.api.ProgressEvent;
import org.netbeans.modules.refactoring.java.RefactoringUtils;
import org.netbeans.modules.refactoring.java.api.JavaMoveMembersProperties;
import org.netbeans.modules.refactoring.java.api.JavaRefactoringUtils;
import org.netbeans.modules.refactoring.java.spi.JavaRefactoringPlugin;
import org.netbeans.modules.refactoring.spi.RefactoringElementsBag;
import org.netbeans.spi.java.classpath.support.ClassPathSupport;
import org.openide.filesystems.FileObject;
import org.openide.filesystems.URLMapper;
import org.openide.util.Lookup;
Expand All @@ -66,6 +64,8 @@
"ERR_MoveAbstractMember=Cannot move abstract method \"{0}\"",
"# {0} - Method name",
"ERR_MoveMethodPolymorphic=Cannot move polymorphic method \"{0}\"",
"# {0} - Field name",
"ERR_MoveNotInitializedToInterface=Member \"{0}\" cannot be moved to interface, it is not initialized.",
"WRN_InitNoAccess=Field initializer uses local accessors which will not be accessible",
"# {0} - File displayname : line number",
"WRN_NoAccessor=No accessor found to invoke the method from: {0}",
Expand Down Expand Up @@ -239,6 +239,19 @@ protected Problem fastCheckParameters(CompilationController javac) throws IOExce
if(targetPackage.isUnnamed() && !sourcePackage.isUnnamed()) {
return new Problem(true, NbBundle.getMessage(MoveMembersRefactoringPlugin.class, "ERR_MovingMemberToDefaultPackage")); //NOI18N
}
if (targetClass.getKind().isInterface()) {

for (TreePathHandle treePathHandle : source) {
Element element = treePathHandle.resolveElement(javac);
if (element != null && element.getKind() == ElementKind.FIELD) {
VariableElement variableElement = (VariableElement) element;
VariableTree variableTree = (VariableTree) javac.getTrees().getTree(variableElement);
if (variableTree != null && variableTree.getInitializer() == null) {
return createProblem(p, true, NbBundle.getMessage(PullUpRefactoringPlugin.class, "ERR_MoveNotInitializedToInterface", element.getSimpleName()));
}
}
}
}
for (TreePathHandle treePathHandle : source) {
Element element = treePathHandle.resolveElement(javac);
List<? extends Element> enclosedElements = targetElement.getEnclosedElements();
Expand Down
18 changes: 14 additions & 4 deletions ...oring.java/src/org/netbeans/modules/refactoring/java/plugins/PullUpRefactoringPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@
*/
package org.netbeans.modules.refactoring.java.plugins;

import com.sun.source.tree.VariableTree;
import java.io.IOException;
import java.util.*;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import org.netbeans.api.java.source.*;
import org.netbeans.modules.refactoring.api.AbstractRefactoring;
import org.netbeans.modules.refactoring.api.Problem;
import org.netbeans.modules.refactoring.api.ProgressEvent;
import org.netbeans.modules.refactoring.java.RefactoringUtils;
import org.netbeans.modules.refactoring.java.api.JavaRefactoringUtils;
import org.netbeans.modules.refactoring.java.api.MemberInfo;
Expand Down Expand Up @@ -120,7 +121,8 @@ protected Problem preCheck(CompilationController cc) throws IOException {


@Override
public Problem fastCheckParameters() {
public Problem fastCheckParameters(CompilationController cc) throws IOException {
cc.toPhase(JavaSource.Phase.RESOLVED);
MemberInfo<ElementHandle<? extends Element>>[] info = refactoring.getMembers();
// #1 - check whether there are any members to pull up
if (info.length == 0) {
Expand All @@ -144,15 +146,24 @@ public Problem fastCheckParameters() {

Problem p=null;
if (refactoring.getTargetType().getKind().isInterface()) {
for (MemberInfo i:info) {
for (MemberInfo<ElementHandle<? extends Element>> i :info) {
Element member = i.getElementHandle().resolve(cc);
if (!i.getModifiers().contains(Modifier.PUBLIC)) {
p = createProblem(p, false, NbBundle.getMessage(PullUpRefactoringPlugin.class,"ERR_PullupNonPublicToInterface" ,i.getName()));
}

if (i.getModifiers().contains(Modifier.STATIC)) {
p = createProblem(p, true, NbBundle.getMessage(PullUpRefactoringPlugin.class,"ERR_PullupStaticToInterface" ,i.getName()));
}
if (member.getKind() == ElementKind.FIELD) {
VariableElement variableElement = (VariableElement) member;
VariableTree variableTree = (VariableTree) cc.getTrees().getTree(variableElement);
if (variableTree != null && variableTree.getInitializer() == null) {
return createProblem(p, true, NbBundle.getMessage(PullUpRefactoringPlugin.class, "ERR_PullupNonNotInitializedToInterface", member.getSimpleName()));
}
}
}

}

for (MemberInfo<ElementHandle<? extends Element>> i : info) {
Expand Down Expand Up @@ -237,7 +248,6 @@ protected Problem checkParameters(CompilationController cc) throws IOException {
// !sourceResource.equals(targetResource),
// !sourceResource.getPackageName().equals(targetResource.getPackageName()));
// }

fireProgressListenerStep();
}

Expand Down
2 changes: 2 additions & 0 deletions ...g.java/test/unit/src/org/netbeans/modules/refactoring/java/plugins/Bundle_test.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ ERR_MoveToSameClass=ERR_MoveToSameClass
ERR_MoveToSuperClass=ERR_MoveToSuperClass
ERR_MoveToSubClass=ERR_MoveToSubClass
ERR_MoveGenericField=ERR_MoveGenericField
ERR_MoveNotInitializedToInterface=ERR_MoveNotInitializedToInterface
WRN_InitNoAccess=WRN_InitNoAccess
WRN_NoAccessor=WRN_NoAccessor
ERR_CannotMovePublicIntoSamePackage=ERR_CannotMovePublicIntoSamePackage
Expand All @@ -82,6 +83,7 @@ ERR_ClassToMoveClashesInner=ERR_ClassToMoveClashesInner
ERR_PullUp_MemberAlreadyExists=ERR_PullUp_MemberAlreadyExists
ERR_PullUp_MemberTargetType=ERR_PullUp_MemberTargetType
ERR_PullupStaticToInterface=ERR_PullupStaticToInterface
ERR_PullupNonNotInitializedToInterface=ERR_PullupNonNotInitializedToInterface

# Copy
ERR_ClassToMoveClashes=ERR_ClassToMoveClashes
Expand Down
44 changes: 44 additions & 0 deletions ...actoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/MoveFieldTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,50 @@ public void testMoveField() throws Exception {
+ "}\n"));
}

public void testMoveNotInitializedFieldToInterface() throws Exception {
writeFilesAndWaitForScan(src,
new File("t/A.java", "package t;\n"
+ "public class A {\n"
Comment thread
mbien marked this conversation as resolved.
Resolved
+ " public static int i;\n"
+ "}\n"),
new File("t/B.java", "package t;\n"
+ "public interface B {\n"
+ "}\n"));
performMove(src.getFileObject("t/A.java"), new int[]{1}, src.getFileObject("t/B.java"), Visibility.ESCALATE, false, new Problem(true, "ERR_MoveNotInitializedToInterface"));
}

public void testMoveInitializedFieldToInterface() throws Exception {
writeFilesAndWaitForScan(src,
new File("t/A.java",
"""
package t;
public class A {
public static int i = 1;
}
"""),
new File("t/B.java",
"""
package t;
public interface B {
}
"""));
performMove(src.getFileObject("t/A.java"), new int[]{1}, src.getFileObject("t/B.java"), Visibility.ESCALATE, false);
verifyContent(src,
new File("t/A.java",
"""
package t;
public class A {
}
"""),
new File("t/B.java",
"""
package t;
public interface B {
public static int i = 1;
}
"""));
}

public void test242909() throws Exception {
writeFilesAndWaitForScan(src,
new File("t/A.java", "package t;\n"
Expand Down
17 changes: 17 additions & 0 deletions ...refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/PullUpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,23 @@ public void test206683() throws Exception {
performPullUp(src.getFileObject("pullup/A.java"), 1, Boolean.FALSE, new Problem(true, "ERR_PullUp_MemberAlreadyExists"));
}

public void testPullUpNotInitializedFieldToInterface() throws Exception {
writeFilesAndWaitForScan(src,
new File("pullup/A.java", "package pullup; public interface A {}"),
new File("pullup/B.java", "package pullup; public class B implements A { public int i; }"));
performPullUpIface(src.getFileObject("pullup/B.java"), 1, 0, Boolean.FALSE, new Problem(true, "ERR_PullupNonNotInitializedToInterface"));
}

public void testPullUpInitializedFieldToInterface() throws Exception {
writeFilesAndWaitForScan(src,
new File("t/A.java", "package pullup; public interface A {}"),
new File("t/B.java", "package pullup; public class B implements A { public int i = 1; }"));
performPullUpIface(src.getFileObject("t/B.java"), 1,0, Boolean.FALSE);
verifyContent(src,
new File("t/A.java", "package pullup; public interface A { public int i = 1; }"),
new File("t/B.java", "package pullup; public class B implements A {}"));
}

public void testPullUpField() throws Exception {
writeFilesAndWaitForScan(src,
new File("pullup/A.java", "package pullup; public class A extends B { public int i; }"),
Expand Down
Loading