From 76cb0af9adf70b36792bf3d2fb2e7cc23f119fe3 Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Mon, 17 Jun 2024 10:05:50 -0400 Subject: [PATCH] Fix move of record to new file refactoring when component accessed (#1453) - add check in MemberVisibilityAdjustor.rewriteVisibility() to ignore record fields - add new test to MoveInnerToNewTests16 - fixes #1419 --- .../structure/MemberVisibilityAdjustor.java | 4 +-- .../moveInnerRecord2/in/p1/Foo.java | 9 +++++ .../moveInnerRecord2/out/p1/Bar.java | 6 ++++ .../moveInnerRecord2/out/p1/Foo.java | 7 ++++ .../refactoring/MoveInnerToNewTests16.java | 35 ++++++++++++++++++- 5 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/in/p1/Foo.java create mode 100644 org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Bar.java create mode 100644 org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Foo.java diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/structure/MemberVisibilityAdjustor.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/structure/MemberVisibilityAdjustor.java index 444a8903f34..889d482e23e 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/structure/MemberVisibilityAdjustor.java +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/structure/MemberVisibilityAdjustor.java @@ -166,14 +166,14 @@ protected final void rewriteVisibility(final MemberVisibilityAdjustor adjustor, Assert.isNotNull(rewrite); Assert.isNotNull(root); final int visibility= fKeyword != null ? fKeyword.toFlagValue() : Modifier.NONE; - if (fMember instanceof IField && !Flags.isEnum(fMember.getFlags())) { + if (fMember instanceof IField && !Flags.isEnum(fMember.getFlags()) && !Flags.isRecord(fMember.getFlags())) { final VariableDeclarationFragment fragment= ASTNodeSearchUtil.getFieldDeclarationFragmentNode((IField) fMember, root); final FieldDeclaration declaration= (FieldDeclaration) fragment.getParent(); VariableDeclarationFragment[] fragmentsToChange= new VariableDeclarationFragment[] { fragment }; VariableDeclarationRewrite.rewriteModifiers(declaration, fragmentsToChange, visibility, ModifierRewrite.VISIBILITY_MODIFIERS, rewrite, group); if (status != null) adjustor.fStatus.merge(status); - } else if (fMember != null) { + } else if (fMember != null && !Flags.isRecord(fMember.getFlags())) { final BodyDeclaration declaration= ASTNodeSearchUtil.getBodyDeclarationNode(fMember, root); if (declaration != null) { ModifierRewrite.create(rewrite, declaration).setVisibility(visibility, group); diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/in/p1/Foo.java b/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/in/p1/Foo.java new file mode 100644 index 00000000000..61e4bd56e11 --- /dev/null +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/in/p1/Foo.java @@ -0,0 +1,9 @@ +package p1; + +public record Foo(Foo.Bar bar) { + static record Bar (T left, T right) {} + + public String foo(Bar in) { + return in.left(); + } +} \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Bar.java b/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Bar.java new file mode 100644 index 00000000000..ba31b8d5829 --- /dev/null +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Bar.java @@ -0,0 +1,6 @@ +/** + * + */ +package p1; + +record Bar (T left, T right) {} \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Foo.java b/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Foo.java new file mode 100644 index 00000000000..7481924b754 --- /dev/null +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/MoveInnerToNew14/moveInnerRecord2/out/p1/Foo.java @@ -0,0 +1,7 @@ +package p1; + +public record Foo(Bar bar) { + public String foo(Bar in) { + return in.left(); + } +} \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/MoveInnerToNewTests16.java b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/MoveInnerToNewTests16.java index 9c873fae928..d2476349b01 100644 --- a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/MoveInnerToNewTests16.java +++ b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/MoveInnerToNewTests16.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019, 2021 IBM Corporation and others. + * Copyright (c) 2019, 2024 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -82,6 +82,39 @@ public void moveInnerRecord() throws Exception{ assertTrue("activation was supposed to be successful" + preconditionResult.toString(), preconditionResult.isOK()); + RefactoringStatus checkInputResult= ref.checkFinalConditions(new NullProgressMonitor()); + assertFalse("precondition was supposed to pass", checkInputResult.hasError()); + performChange(ref, false); + + assertEquals("p1 files", 2, packP1.getChildren().length); + + String expectedSource= getFileContents(getRefactoringPath() + getName() + outDir + p1Name + "/Foo.java"); + assertEqualLines("incorrect update of Foo", expectedSource, packP1.getCompilationUnit("Foo.java").getSource()); + + expectedSource= getFileContents(getRefactoringPath() + getName() + outDir + p1Name + "/Bar.java"); + assertEqualLines("incorrect creation of Bar", expectedSource, packP1.getCompilationUnit("Bar.java").getSource()); + + } + + @Test + public void moveInnerRecord2() throws Exception{ + ParticipantTesting.reset(); + final String p1Name= "p1"; + final String inDir= "/in/"; + final String outDir= "/out/"; + + IPackageFragment packP1= createPackage(p1Name); + ICompilationUnit p1Foo= createCu(packP1, getName() + inDir + p1Name + "/Foo.java", "Foo.java"); + IType fooType= p1Foo.getTypes()[0]; + IType barType= fooType.getTypes()[0]; + + assertTrue("should be enabled", RefactoringAvailabilityTester.isMoveInnerAvailable(barType)); + MoveInnerToTopRefactoring ref= ((RefactoringAvailabilityTester.isMoveInnerAvailable(barType)) ? new MoveInnerToTopRefactoring(barType, JavaPreferencesSettings.getCodeGenerationSettings(barType.getJavaProject())) : null); + assertNotNull("MoveInnerToTopRefactoring should not be null", ref); + RefactoringStatus preconditionResult= ref.checkInitialConditions(new NullProgressMonitor()); + assertTrue("activation was supposed to be successful" + preconditionResult.toString(), preconditionResult.isOK()); + + RefactoringStatus checkInputResult= ref.checkFinalConditions(new NullProgressMonitor()); assertFalse("precondition was supposed to pass", checkInputResult.hasError()); performChange(ref, false);