You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "homberghp (via GitHub)" <gi...@apache.org> on 2025/01/26 18:35:03 UTC
[PR] solves #7044, refactor inner to outer for record [netbeans]
homberghp opened a new pull request, #8197:
URL: https://github.com/apache/netbeans/pull/8197
see issue #7044 for exact problem.
Start with adding tests.
Introduced RECORD and RECORD_COMPONENT case labels in multiple places.
Deal with method params for the record header in InnerOuterTransformer. The given way of printing the 'method parameters' for the record header include 'private final', producing illegal java code. The solution found might not be the moset elegant, but since the tree modifications will not persist into any real source code, it is acceptable. The solution strips `private` and `final` from the VARIABLE trees.
Delete file threw and unnecessary exception where a simple null test
suffices.
---
**^Add meaningful description above**
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
- are all your own work, and you have the right to contribute them.
- are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).
Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field.
If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#issuecomment-2702261662
Got it working for inner records for pull up and inner to outer refactoring.
Formatting is (still) broken for the case that the inner record has an implements clause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1934984604
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java:
##########
@@ -0,0 +1,635 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.refactoring.java.test;
+
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
+import java.io.IOException;
+import java.time.LocalDate;
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.List;
+import javax.lang.model.SourceVersion;
+import javax.lang.model.element.Name;
+import org.junit.FixMethodOrder;
+import org.junit.runners.MethodSorters;
+import org.netbeans.api.java.source.CompilationController;
+import org.netbeans.api.java.source.JavaSource;
+import org.netbeans.api.java.source.Task;
+import org.netbeans.api.java.source.TreePathHandle;
+import org.netbeans.modules.refactoring.api.Problem;
+import org.netbeans.modules.refactoring.api.RefactoringSession;
+import org.netbeans.modules.refactoring.java.api.InnerToOuterRefactoring;
+import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.addAllProblems;
+import org.openide.util.Exceptions;
+
+/**
+ * Test inner to outer refactoring for test.
+ *
+ * In the input files, and the expected outcomes, the indentation does not
+ * really matter as far as the tests are concerned because the indentation is
+ * stripped away before the remaining source lines are compared to the expected
+ * lines.
+ *
+ * @author homberghp {@code <pieter.van.den.hombergh@gmail.com)>}
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class InnerOuterRecordTest extends RefactoringTestBase {
+
+ public InnerOuterRecordTest(String name) {
+ super(name, "16");
+ //ensure we are running on at least 16.
+ try {
+ SourceVersion.valueOf("RELEASE_16"); //NOI18N
+ } catch (IllegalArgumentException ex) {
+ //OK, no RELEASE_16, skip test
+ throw new RuntimeException("need at least Java 16 for record");
+ }
+ }
+
+ // for reference
+ public void test0_259004() throws Exception {
+ String source =
+ """
+ package t;
+ import java.util.function.Consumer;
+ public class A {
+ public static void main(String[] args) {
+ Consumer<F> c = f -> {};
+ }
+ public static final class F {}
+ }""";
+ String newOuter =
+ """
+ package t;
+ import java.util.function.Consumer;
+ public class A {
+ public static void main(String[] args) {
+ Consumer<F> c = f -> {};
+ }
+ }""";
+ String newInner =
+ """
+ /*
+ * Refactoring License
+ */
+ package t;
+ /**
+ *
+ * @author junit
+ */
+ public final class F {
+ }
+ """;
+ innerOuterSetupAndTest(source, newOuter, newInner);
+ }
Review Comment:
you missed this section, please align the """ manually, example:
```java
"""
package t;
import java.util.function.Consumer;
public class A {
public static void main(String[] args) {
Consumer<F> c = f -> {};
}
public static final class F {}
}
""";
```
this ensures that the resulting text block string has the expected indentation when used in the test (and it looks less noisy).
##########
java/java.source.base/src/org/netbeans/modules/java/source/base/SourceLevelUtils.java:
##########
@@ -34,6 +34,13 @@ public class SourceLevelUtils {
public static final Source JDK1_9 = Source.lookup("9");
public static final Source JDK14 = Source.lookup("14");
public static final Source JDK15 = Source.lookup("15");
+ public static final Source JDK16 = Source.lookup("16");
+ public static final Source JDK17 = Source.lookup("17");
+ // for next release:
+// public static final Source JDK18 = Source.lookup("18");
+// public static final Source JDK19 = Source.lookup("19");
+// public static final Source JDK20 = Source.lookup("20");
+// public static final Source JDK21 = Source.lookup("21");
Review Comment:
what I meant was that the two new fields you added aren't used anywhere in your patch : https://github.com/apache/netbeans/pull/8197.diff
Lets avoid unnecessary changes beyond those needed to resolve the issue.
It is esp questionable here since NB ships with latest javac out of the box now (since ~NB 13), the source values can be directly accessed (up to `Source.JDK24`), so this utility might have lost a bit of its original purpose.
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
@@ -84,7 +86,38 @@ public RefactoringTestBase(String name, String sourcelevel) {
this.sourcelevel = sourcelevel;
}
+ static boolean debug = false;
+// static boolean skipIndexing = false;
Review Comment:
unused -> can be removed
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
Review Comment:
I suspected that this might have been unintended. But we really shouldn't do that when fixing bugs. This messes with git blame and makes it difficult to reason about repo history.
here is a commit which reverts some of the changes in unrelated code https://github.com/mbien/netbeans/commit/9bcdda116bc739fbbb3ebcaa1b45690f13d54fc1
its on a copy of your branch https://github.com/mbien/netbeans/tree/issue7044_copy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930574803
##########
java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java:
##########
@@ -640,9 +641,17 @@ public String getMethodHeader(MethodTree t, String s) {
print(tree.name);
s = replace(s, NAME);
}
- print('(');
- wrapTrees(tree.params, WrapStyle.WRAP_NEVER, out.col);
- print(')');
+ boolean isRecord = enclClass.getKind()== Kind.RECORD;
+ // return type null makes this method a constructor
+ boolean isCandidateCompactCtor = isRecord && null == t.getReturnType();
+ if (isCandidateCompactCtor) {
+ System.err.println("spotted compact "+ t.toString());
+ }
Review Comment:
removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1939470549
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java:
##########
@@ -0,0 +1,635 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.refactoring.java.test;
+
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
+import java.io.IOException;
+import java.time.LocalDate;
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.List;
+import javax.lang.model.SourceVersion;
+import javax.lang.model.element.Name;
+import org.junit.FixMethodOrder;
+import org.junit.runners.MethodSorters;
+import org.netbeans.api.java.source.CompilationController;
+import org.netbeans.api.java.source.JavaSource;
+import org.netbeans.api.java.source.Task;
+import org.netbeans.api.java.source.TreePathHandle;
+import org.netbeans.modules.refactoring.api.Problem;
+import org.netbeans.modules.refactoring.api.RefactoringSession;
+import org.netbeans.modules.refactoring.java.api.InnerToOuterRefactoring;
+import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.addAllProblems;
+import org.openide.util.Exceptions;
+
+/**
+ * Test inner to outer refactoring for test.
+ *
+ * In the input files, and the expected outcomes, the indentation does not
+ * really matter as far as the tests are concerned because the indentation is
+ * stripped away before the remaining source lines are compared to the expected
+ * lines.
+ *
+ * @author homberghp {@code <pieter.van.den.hombergh@gmail.com)>}
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class InnerOuterRecordTest extends RefactoringTestBase {
+
+ public InnerOuterRecordTest(String name) {
+ super(name, "16");
+ //ensure we are running on at least 16.
+ try {
+ SourceVersion.valueOf("RELEASE_16"); //NOI18N
+ } catch (IllegalArgumentException ex) {
+ //OK, no RELEASE_16, skip test
+ throw new RuntimeException("need at least Java 16 for record");
+ }
+ }
+
+ // for reference
+ public void test0_259004() throws Exception {
+ String source =
+ """
+ package t;
+ import java.util.function.Consumer;
+ public class A {
+ public static void main(String[] args) {
+ Consumer<F> c = f -> {};
+ }
+ public static final class F {}
+ }""";
+ String newOuter =
+ """
+ package t;
+ import java.util.function.Consumer;
+ public class A {
+ public static void main(String[] args) {
+ Consumer<F> c = f -> {};
+ }
+ }""";
+ String newInner =
+ """
+ /*
+ * Refactoring License
+ */
+ package t;
+ /**
+ *
+ * @author junit
+ */
+ public final class F {
+ }
+ """;
+ innerOuterSetupAndTest(source, newOuter, newInner);
+ }
Review Comment:
I agree with less noisy.
As per spec (JLS), the left margin (indentation) is taken from the least indented line in the text block, so it will not affect the tests.
In the tests I have written, I specifically discarded the indentation, to make the test independent of any formatting settings. This is on purpose.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#issuecomment-2668421507
Do not merge,. solution does not yet work for the case where the inner record has a varargs generic argument
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1939661789
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
Review Comment:
@homberghp may I add a commit to your branch? It appears that https://github.com/mbien/netbeans/commit/9bcdda116bc739fbbb3ebcaa1b45690f13d54fc1 wasn't applied, I still see large parts of `InnerToOuterTransformer` reformatted in the diff https://github.com/apache/netbeans/pull/8197/files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930578337
##########
java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java:
##########
@@ -977,6 +986,9 @@ private void printEnumConstants(java.util.List<? extends JCTree> defs, boolean f
@Override
public void visitMethodDef(JCMethodDecl tree) {
+ boolean methodIsConstructor = null==tree.getReturnType();
+ boolean enclosingIsRecord= enclClass.getKind() == Kind.RECORD;
+ boolean paramsIsComponents= paramsIsComponents(tree.getParameters());
Review Comment:
nitpick back: null==tree.getReturnType() comes from the practice that a compiler will not accept an assignment to a constant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1935009700
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
@@ -84,7 +86,38 @@ public RefactoringTestBase(String name, String sourcelevel) {
this.sourcelevel = sourcelevel;
}
+ static boolean debug = false;
+// static boolean skipIndexing = false;
Review Comment:
L90 unused -> can be removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930578337
##########
java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java:
##########
@@ -977,6 +986,9 @@ private void printEnumConstants(java.util.List<? extends JCTree> defs, boolean f
@Override
public void visitMethodDef(JCMethodDecl tree) {
+ boolean methodIsConstructor = null==tree.getReturnType();
+ boolean enclosingIsRecord= enclClass.getKind() == Kind.RECORD;
+ boolean paramsIsComponents= paramsIsComponents(tree.getParameters());
Review Comment:
nitpick back: null==tree.getReturnType() comes from the practice that a compiler will not accept an assignment to a constant. This is less of an issue in Java but is for truthty languages like C and python.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#issuecomment-2631195326
remo
https://github.com/apache/netbeans/pull/8197#discussion_r1934980774
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930573084
##########
java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java:
##########
@@ -582,44 +586,80 @@ private ClassTree refactorInnerClass(ClassTree innerClass) {
}
rewrite(modifiersTree, newModifiersTree);
+ // create constructor which refers with member to old outer.
if (referenceName != null) {
for (Tree member:newInnerClass.getMembers()) {
if (member.getKind() == Tree.Kind.METHOD) {
- MethodTree m = (MethodTree) member;
- if (m.getName().contentEquals("<init>") || m.getReturnType() == null) {
- VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), refactoring.getReferenceName(), make.Type(outerType), null);
- MethodTree newConstructor = hasVarArgs(m) ?
- make.insertMethodParameter(m, m.getParameters().size() - 1, parameter) :
- make.addMethodParameter(m, parameter);
-
+ MethodTree ctor = (MethodTree) member;
+ if (ctor.getName().contentEquals("<init>") || ctor.getReturnType() == null) {
+ VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), referenceName, make.Type(outerType), null);
+ MethodTree newConstructor = hasVarArgs(ctor) ?
+ make.insertMethodParameter(ctor, ctor.getParameters().size() - 1, parameter) :
+ make.addMethodParameter(ctor, parameter);
+
AssignmentTree assign = make.Assignment(make.Identifier("this."+referenceName), make.Identifier(referenceName)); // NOI18N
BlockTree block = make.insertBlockStatement(newConstructor.getBody(), 1, make.ExpressionStatement(assign));
Set<Modifier> modifiers = EnumSet.noneOf(Modifier.class);
modifiers.addAll(newConstructor.getModifiers().getFlags());
modifiers.remove(Modifier.PRIVATE);
newConstructor = make.Constructor(
make.Modifiers(modifiers,newConstructor.getModifiers().getAnnotations()),
- newConstructor.getTypeParameters(),
+ newConstructor.getTypeParameters(),
newConstructor.getParameters(),
newConstructor.getThrows(),
block);
- newInnerClass = make.removeClassMember(newInnerClass, m);
- genUtils.copyComments(m, newConstructor, true);
- genUtils.copyComments(m, newConstructor, false);
+ newInnerClass = make.removeClassMember(newInnerClass, ctor);
+ genUtils.copyComments(ctor, newConstructor, true);
+ genUtils.copyComments(ctor, newConstructor, false);
newInnerClass = genUtils.insertClassMember(newInnerClass, newConstructor);
}
}
}
}
-
+
if(innerClass != newInnerClass) {
genUtils.copyComments(innerClass, newInnerClass, true);
genUtils.copyComments(innerClass, newInnerClass, false);
}
return newInnerClass;
}
+ private ClassTree refactorInnerRecord(ClassTree innerRecord) {
+ refactoring.setInnerIsRecord(true);
+ ClassTree newInnerRecord = innerRecord;
+ List<? extends Tree> members = newInnerRecord.getMembers();
+ int insertAt=0;
+ for (Tree member : members) {
+ Tree.Kind kind = member.getKind();
+// System.out.println("member = " + kind + "" + member);
+
+ switch (kind) {
+ // The fields, except the static, should loose private final modifiers
+ // so that they appear without these modifiers in the record header
+ case VARIABLE:
+ VariableTree vt = (VariableTree) member;
+ ModifiersTree mt = vt.getModifiers();
+ if (mt.getFlags().contains(Modifier.STATIC)) {
+ continue;
+ }
+ var mtype = vt.getType();
+ var mName = vt.getName();
+ var mods = make.Modifiers(EnumSet.noneOf(Modifier.class));
+ var newMember = make.RecordComponent(mods, mName, mtype);
+ newInnerRecord = make.removeClassMember(newInnerRecord, member);
+ newInnerRecord = make.insertClassMember(newInnerRecord, insertAt, newMember);
Review Comment:
In general, I agree. Do not obfuscate code unnecessarily.
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930599133
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/RecordFormattingTest.java:
##########
Review Comment:
Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1935034677
##########
java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/DeleteFile.java:
##########
@@ -75,11 +75,10 @@ public String getDisplayText() {
public void performChange() {
try {
FileObject fo = URLMapper.findFileObject(res);
- if (fo == null) {
- throw new IOException(res.toString());
+ if (fo != null) {
+ id = BackupFacility.getDefault().backup(fo);
+ DataObject.find(fo).delete();
}
- id = BackupFacility.getDefault().backup(fo);
- DataObject.find(fo).delete();
} catch (DataObjectNotFoundException ex) {
Review Comment:
wondering why this change was necessary, this particular refactoring shouldn't delete anything, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930572549
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
@@ -109,33 +171,30 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except
while (!todo.isEmpty()) {
FileObject file = todo.remove(0);
- if (file.isData()) {
+ if (file.isData()) { // normal file
content.put(FileUtil.getRelativePath(sourceRoot, file), copyFileToString(FileUtil.toFile(file)));
- } else {
+ } else { //folder?
Review Comment:
From the `org.openide.filesystems.FileObject` it follows that `!isData == isFolder`, more or less
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
@@ -400,7 +463,8 @@ protected static boolean problemIsFatal(List<Problem> problems) {
return false;
}
- private static final int RETRIES = 3;
+// private static final int RETRIES = 3;
+ private static final int RETRIES = 1;
Review Comment:
This needs more investigation.
The stability of some tests may have to do with not using proper (new) temporary directories.
In the places where I looked, the temp files and directories were made inside the build tree. I would place them
outside the build tree, like in /tmp. The later could then be placed on a fast or even virtual device for speed up.
I will set it back to 3.
In a future version, I would advise to make it a non-static field, that individual subclasses or individual tests can set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930611905
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
Review Comment:
Sorry, muscle memory. See issue #8130.
I am trying to avoid that. but sometimes I type those keys and also do `remove trailing spaces`.
But now it is done, said file now complies with the default formatting rules. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#issuecomment-2615905517
mbien, thank you for your time.
I just push to branch issue7044 on my fork.
Is there anything I need to do?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1929843676
##########
java/java.source.base/src/org/netbeans/modules/java/source/base/SourceLevelUtils.java:
##########
@@ -34,6 +34,13 @@ public class SourceLevelUtils {
public static final Source JDK1_9 = Source.lookup("9");
public static final Source JDK14 = Source.lookup("14");
public static final Source JDK15 = Source.lookup("15");
+ public static final Source JDK16 = Source.lookup("16");
+ public static final Source JDK17 = Source.lookup("17");
+ // for next release:
+// public static final Source JDK18 = Source.lookup("18");
+// public static final Source JDK19 = Source.lookup("19");
+// public static final Source JDK20 = Source.lookup("20");
+// public static final Source JDK21 = Source.lookup("21");
Review Comment:
doesn't look like this change is needed JDK16/JDK17 isn't used.
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
@@ -109,33 +171,30 @@ protected void verifyContent(FileObject sourceRoot, File... files) throws Except
while (!todo.isEmpty()) {
FileObject file = todo.remove(0);
- if (file.isData()) {
+ if (file.isData()) { // normal file
content.put(FileUtil.getRelativePath(sourceRoot, file), copyFileToString(FileUtil.toFile(file)));
- } else {
+ } else { //folder?
Review Comment:
yes that is a folder, remove question mark :)
##########
java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/RecordFormattingTest.java:
##########
Review Comment:
this test can be removed since it is already in:
https://github.com/apache/netbeans/blob/445977fa502ef748c7773caa2334be1fe378b12d/java/java.source.base/test/unit/src/org/netbeans/modules/java/source/save/FormatingTest.java#L6688
##########
java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java:
##########
@@ -640,9 +641,17 @@ public String getMethodHeader(MethodTree t, String s) {
print(tree.name);
s = replace(s, NAME);
}
- print('(');
- wrapTrees(tree.params, WrapStyle.WRAP_NEVER, out.col);
- print(')');
+ boolean isRecord = enclClass.getKind()== Kind.RECORD;
+ // return type null makes this method a constructor
+ boolean isCandidateCompactCtor = isRecord && null == t.getReturnType();
+ if (isCandidateCompactCtor) {
+ System.err.println("spotted compact "+ t.toString());
+ }
Review Comment:
no err/out println in production code
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
@@ -400,7 +463,8 @@ protected static boolean problemIsFatal(List<Problem> problems) {
return false;
}
- private static final int RETRIES = 3;
+// private static final int RETRIES = 3;
+ private static final int RETRIES = 1;
Review Comment:
interesting. I didn't know that some tests had retry loops implemented.
please don't change the settings here. This would have to be looked at separately. We have have retry scripts in CI already for well known test suites.
IMO: problematic tests have to be isolated and run separately, only this makes it observable from the outside how bad something is without hiding a problem. But this is beyond the scope of this PR.
##########
java/java.source.base/src/org/netbeans/modules/java/source/pretty/VeryPretty.java:
##########
@@ -977,6 +986,9 @@ private void printEnumConstants(java.util.List<? extends JCTree> defs, boolean f
@Override
public void visitMethodDef(JCMethodDecl tree) {
+ boolean methodIsConstructor = null==tree.getReturnType();
+ boolean enclosingIsRecord= enclClass.getKind() == Kind.RECORD;
+ boolean paramsIsComponents= paramsIsComponents(tree.getParameters());
Review Comment:
nitpick:
```java
boolean methodIsConstructor = tree.getReturnType() == null;
boolean enclosingIsRecord = enclClass.getKind() == Kind.RECORD;
boolean paramsIsComponents = paramsIsComponents(tree.getParameters());
```
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
@@ -76,15 +78,46 @@ public class RefactoringTestBase extends NbTestCase {
public RefactoringTestBase(String name) {
super(name);
- sourcelevel = "1.6";
+ sourcelevel = "17";//"1.6";//??
Review Comment:
17 is probably not a good default. Lets not change this value in the base class.
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java:
##########
@@ -0,0 +1,713 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.refactoring.java.test;
+
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
+import java.io.IOException;
+import java.time.LocalDate;
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.List;
+import javax.lang.model.SourceVersion;
+import javax.lang.model.element.Name;
+import org.junit.FixMethodOrder;
+import org.junit.runners.MethodSorters;
+import org.netbeans.api.java.source.CompilationController;
+import org.netbeans.api.java.source.JavaSource;
+import org.netbeans.api.java.source.Task;
+import org.netbeans.api.java.source.TreePathHandle;
+import org.netbeans.modules.refactoring.api.Problem;
+import org.netbeans.modules.refactoring.api.RefactoringSession;
+import org.netbeans.modules.refactoring.java.api.InnerToOuterRefactoring;
+import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.addAllProblems;
+import org.openide.util.Exceptions;
+
+/**
+ * Test inner to outer refactoring for test.
+ *
+ * In the input files, and the expected outcomes, the indentation does not
+ * really matter as far as the tests are concerned because the indentation is
+ * stripped away before the remaining source lines are compared to the expected
+ * lines.
+ *
+ * @author homberghp {@code <pieter.van.den.hombergh@gmail.com)>}
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class InnerOuterRecordTest extends RefactoringTestBase {
+
+ public InnerOuterRecordTest(String name) {
+ super(name);
+ //ensure we are running on at least 16.
+ try {
+ SourceVersion.valueOf("RELEASE_16"); //NOI18N
+ } catch (IllegalArgumentException ex) {
+ //OK, no RELEASE_16, skip test
+ throw new RuntimeException("need at least Java 16 for record");
+ }
+ }
+
+ // for reference
+ public void test0_259004() throws Exception {
+ String source
+ = """
+ package t;
+
+ import java.util.function.Consumer;
+
+ public class A {
+
+ public static void main(String[] args) {
+ Consumer<F> c = f -> {};
+ }
+
+ public static final class F {}
+ }""";
Review Comment:
recommend to align the `"""` tokens since it influences indentation/whitespace prefixing, example:
```java
String source =
"""
package t;
import java.util.function.Consumer;
public class A {
public static void main(String[] args) {
Consumer<F> c = f -> {};
}
public static final class F {}
}
""";
```
i think it does also look nicer when its less noisy.
##########
java/java.source.base/src/org/netbeans/api/java/source/TreePathHandle.java:
##########
@@ -322,6 +322,7 @@ private static boolean isSupported(Element el) {
case ENUM_CONSTANT:
case RECORD:
//TODO: record component
+ case RECORD_COMPONENT:
Review Comment:
note to self: check if this todo can be removed
##########
java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java:
##########
@@ -582,44 +586,80 @@ private ClassTree refactorInnerClass(ClassTree innerClass) {
}
rewrite(modifiersTree, newModifiersTree);
+ // create constructor which refers with member to old outer.
if (referenceName != null) {
for (Tree member:newInnerClass.getMembers()) {
if (member.getKind() == Tree.Kind.METHOD) {
- MethodTree m = (MethodTree) member;
- if (m.getName().contentEquals("<init>") || m.getReturnType() == null) {
- VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), refactoring.getReferenceName(), make.Type(outerType), null);
- MethodTree newConstructor = hasVarArgs(m) ?
- make.insertMethodParameter(m, m.getParameters().size() - 1, parameter) :
- make.addMethodParameter(m, parameter);
-
+ MethodTree ctor = (MethodTree) member;
+ if (ctor.getName().contentEquals("<init>") || ctor.getReturnType() == null) {
+ VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), referenceName, make.Type(outerType), null);
+ MethodTree newConstructor = hasVarArgs(ctor) ?
+ make.insertMethodParameter(ctor, ctor.getParameters().size() - 1, parameter) :
+ make.addMethodParameter(ctor, parameter);
+
AssignmentTree assign = make.Assignment(make.Identifier("this."+referenceName), make.Identifier(referenceName)); // NOI18N
BlockTree block = make.insertBlockStatement(newConstructor.getBody(), 1, make.ExpressionStatement(assign));
Set<Modifier> modifiers = EnumSet.noneOf(Modifier.class);
modifiers.addAll(newConstructor.getModifiers().getFlags());
modifiers.remove(Modifier.PRIVATE);
newConstructor = make.Constructor(
make.Modifiers(modifiers,newConstructor.getModifiers().getAnnotations()),
- newConstructor.getTypeParameters(),
+ newConstructor.getTypeParameters(),
newConstructor.getParameters(),
newConstructor.getThrows(),
block);
- newInnerClass = make.removeClassMember(newInnerClass, m);
- genUtils.copyComments(m, newConstructor, true);
- genUtils.copyComments(m, newConstructor, false);
+ newInnerClass = make.removeClassMember(newInnerClass, ctor);
+ genUtils.copyComments(ctor, newConstructor, true);
+ genUtils.copyComments(ctor, newConstructor, false);
newInnerClass = genUtils.insertClassMember(newInnerClass, newConstructor);
}
}
}
}
-
+
if(innerClass != newInnerClass) {
genUtils.copyComments(innerClass, newInnerClass, true);
genUtils.copyComments(innerClass, newInnerClass, false);
}
return newInnerClass;
}
+ private ClassTree refactorInnerRecord(ClassTree innerRecord) {
+ refactoring.setInnerIsRecord(true);
+ ClassTree newInnerRecord = innerRecord;
+ List<? extends Tree> members = newInnerRecord.getMembers();
+ int insertAt=0;
+ for (Tree member : members) {
+ Tree.Kind kind = member.getKind();
+// System.out.println("member = " + kind + "" + member);
+
+ switch (kind) {
+ // The fields, except the static, should loose private final modifiers
+ // so that they appear without these modifiers in the record header
+ case VARIABLE:
+ VariableTree vt = (VariableTree) member;
+ ModifiersTree mt = vt.getModifiers();
+ if (mt.getFlags().contains(Modifier.STATIC)) {
+ continue;
+ }
+ var mtype = vt.getType();
+ var mName = vt.getName();
+ var mods = make.Modifiers(EnumSet.noneOf(Modifier.class));
+ var newMember = make.RecordComponent(mods, mName, mtype);
+ newInnerRecord = make.removeClassMember(newInnerRecord, member);
+ newInnerRecord = make.insertClassMember(newInnerRecord, insertAt, newMember);
Review Comment:
i am not a fan of `var` since it hides the type only so that the IDE can render it next to it again as inline hint (-> nothing is gained and it makes review more difficult). But I think i can figure out what the types are here so I let it slide ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1929858411
##########
java/refactoring.java/src/org/netbeans/modules/refactoring/java/api/InnerToOuterRefactoring.java:
##########
@@ -39,53 +39,104 @@ public final class InnerToOuterRefactoring extends AbstractRefactoring {
// parameters of the refactoring
private String className;
private String referenceName;
-
+
+ private boolean innerIsRecord;
+
/**
* Creates a new instance of InnerToOuterRefactoring.
- *
- * @param sourceType An inner class that should be converted to a top-level class.
+ *
+ * @param sourceType An inner class that should be converted to a top-level
+ * class.
*/
public InnerToOuterRefactoring(TreePathHandle sourceType) {
super(Lookups.singleton(sourceType));
}
-
- /** Returns the type the members of which should be pulled up
- * by this refactoring.
+
+ /**
+ * Returns the type the members of which should be pulled up by this
+ * refactoring.
+ *
* @return Source of the members to be pulled up.
*/
public TreePathHandle getSourceType() {
return getRefactoringSource().lookup(TreePathHandle.class);
}
// --- PARAMETERS ----------------------------------------------------------
-
- /** Returns the name for the top-level class to be created.
+ /**
+ * Returns the name for the top-level class to be created.
+ *
* @return Class name.
*/
public String getClassName() {
return className;
}
- /** Sets name for the top-level class to be created.
+ /**
+ * Sets name for the top-level class to be created.
+ *
* @param className Class name.
*/
public void setClassName(String className) {
this.className = className;
}
- /** Returns name of the field that should be generated as a reference to the original
- * outer class. If null, no field will be generated.
- * @return Name of the field to be generated or null if no field will be generated.
+ /**
+ * Returns name of the field that should be generated as a reference to the
+ * original outer class. If null, no field will be generated.
+ *
+ * @return Name of the field to be generated or null if no field will be
+ * generated.
*/
public String getReferenceName() {
return referenceName;
}
-
- /** Sets name of the field that should be generated as a reference to the original
- * outer class. Can be set to null which indicates that no field should be generated.
- * @param referenceName Name of the field or null if no field should be generated.
- */
+
+ /**
+ * Sets name of the field that should be generated as a reference to the
+ * original outer class. Can be set to null which indicates that no field
+ * should be generated.
+ *
+ * @param referenceName Name of the field or null if no field should be
+ * generated.
+ */
public void setReferenceName(String referenceName) {
this.referenceName = referenceName;
}
+
+ /**
+ * Inner records need special handling because of the RecordComponents which
+ * are declared before the first curly brace, which differs from class,
+ * interface and enum.
+ *
+ * Also, the compact constructor should be considered.
+ *
+ * A compact constructor consists of the name of the Record, no parameters
+ * (not even the parens) and a block that does NOT assign the fields.
+ *
+ * @return the current value for this refactoring
+ */
+ public boolean isInnerIsRecord() {
+
+ return innerIsRecord;
+ }
+
+ /**
+ * Inner records need special handling because of the RecordComponents which
+ * are declared before the first curly brace, which differs from class,
+ * interface and enum.
+ *
+ * Also, the compact constructor should be considered.
+ *
+ * A compact constructor consists of the name of the Record, no parameters
+ * (not even the parens) and a block that does NOT assign the fields.
+ *
+ * @param innerIsRecord use when inner class needs the special handling of
+ * an inner record.
+ */
+
Review Comment:
remove extra line
##########
java/java.source.base/src/org/netbeans/api/java/source/ElementHandle.java:
##########
@@ -213,6 +208,8 @@ private T resolveImpl (final ModuleElement module, final JavacTaskImpl jt) {
}
case FIELD:
case ENUM_CONSTANT:
+ case RECORD_COMPONENT:
+
Review Comment:
remove extra line
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/RefactoringTestBase.java:
##########
Review Comment:
this whole file was reformatted it seems. please don't do that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1930572170
##########
java/refactoring.java/test/unit/src/org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java:
##########
@@ -0,0 +1,713 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.refactoring.java.test;
+
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
+import java.io.IOException;
+import java.time.LocalDate;
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.List;
+import javax.lang.model.SourceVersion;
+import javax.lang.model.element.Name;
+import org.junit.FixMethodOrder;
+import org.junit.runners.MethodSorters;
+import org.netbeans.api.java.source.CompilationController;
+import org.netbeans.api.java.source.JavaSource;
+import org.netbeans.api.java.source.Task;
+import org.netbeans.api.java.source.TreePathHandle;
+import org.netbeans.modules.refactoring.api.Problem;
+import org.netbeans.modules.refactoring.api.RefactoringSession;
+import org.netbeans.modules.refactoring.java.api.InnerToOuterRefactoring;
+import static org.netbeans.modules.refactoring.java.test.RefactoringTestBase.addAllProblems;
+import org.openide.util.Exceptions;
+
+/**
+ * Test inner to outer refactoring for test.
+ *
+ * In the input files, and the expected outcomes, the indentation does not
+ * really matter as far as the tests are concerned because the indentation is
+ * stripped away before the remaining source lines are compared to the expected
+ * lines.
+ *
+ * @author homberghp {@code <pieter.van.den.hombergh@gmail.com)>}
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class InnerOuterRecordTest extends RefactoringTestBase {
+
+ public InnerOuterRecordTest(String name) {
+ super(name);
+ //ensure we are running on at least 16.
+ try {
+ SourceVersion.valueOf("RELEASE_16"); //NOI18N
+ } catch (IllegalArgumentException ex) {
+ //OK, no RELEASE_16, skip test
+ throw new RuntimeException("need at least Java 16 for record");
+ }
+ }
+
+ // for reference
+ public void test0_259004() throws Exception {
+ String source
+ = """
+ package t;
+
+ import java.util.function.Consumer;
+
+ public class A {
+
+ public static void main(String[] args) {
+ Consumer<F> c = f -> {};
+ }
+
+ public static final class F {}
+ }""";
Review Comment:
I agree, for a better look.
But: this is what the 'default' formatting makes of it. See issue issue #8130.
##########
java/java.source.base/src/org/netbeans/modules/java/source/base/SourceLevelUtils.java:
##########
@@ -34,6 +34,13 @@ public class SourceLevelUtils {
public static final Source JDK1_9 = Source.lookup("9");
public static final Source JDK14 = Source.lookup("14");
public static final Source JDK15 = Source.lookup("15");
+ public static final Source JDK16 = Source.lookup("16");
+ public static final Source JDK17 = Source.lookup("17");
+ // for next release:
+// public static final Source JDK18 = Source.lookup("18");
+// public static final Source JDK19 = Source.lookup("19");
+// public static final Source JDK20 = Source.lookup("20");
+// public static final Source JDK21 = Source.lookup("21");
Review Comment:
JDK17 should be. JDK16 is the version that made record a definitive feature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8197:
URL: https://github.com/apache/netbeans/pull/8197#discussion_r1935037771
##########
java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/InnerToOuterTransformer.java:
##########
@@ -582,44 +586,79 @@ private ClassTree refactorInnerClass(ClassTree innerClass) {
}
rewrite(modifiersTree, newModifiersTree);
+ // create constructor which refers with member to old outer.
if (referenceName != null) {
for (Tree member:newInnerClass.getMembers()) {
if (member.getKind() == Tree.Kind.METHOD) {
- MethodTree m = (MethodTree) member;
- if (m.getName().contentEquals("<init>") || m.getReturnType() == null) {
- VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), refactoring.getReferenceName(), make.Type(outerType), null);
- MethodTree newConstructor = hasVarArgs(m) ?
- make.insertMethodParameter(m, m.getParameters().size() - 1, parameter) :
- make.addMethodParameter(m, parameter);
-
+ MethodTree ctor = (MethodTree) member;
+ if (ctor.getName().contentEquals("<init>") || ctor.getReturnType() == null) {
+ VariableTree parameter = make.Variable(make.Modifiers(EnumSet.of(Modifier.FINAL)), referenceName, make.Type(outerType), null);
+ MethodTree newConstructor = hasVarArgs(ctor) ?
+ make.insertMethodParameter(ctor, ctor.getParameters().size() - 1, parameter) :
+ make.addMethodParameter(ctor, parameter);
+
AssignmentTree assign = make.Assignment(make.Identifier("this."+referenceName), make.Identifier(referenceName)); // NOI18N
BlockTree block = make.insertBlockStatement(newConstructor.getBody(), 1, make.ExpressionStatement(assign));
Set<Modifier> modifiers = EnumSet.noneOf(Modifier.class);
modifiers.addAll(newConstructor.getModifiers().getFlags());
modifiers.remove(Modifier.PRIVATE);
newConstructor = make.Constructor(
make.Modifiers(modifiers,newConstructor.getModifiers().getAnnotations()),
- newConstructor.getTypeParameters(),
+ newConstructor.getTypeParameters(),
newConstructor.getParameters(),
newConstructor.getThrows(),
block);
- newInnerClass = make.removeClassMember(newInnerClass, m);
- genUtils.copyComments(m, newConstructor, true);
- genUtils.copyComments(m, newConstructor, false);
+ newInnerClass = make.removeClassMember(newInnerClass, ctor);
+ genUtils.copyComments(ctor, newConstructor, true);
+ genUtils.copyComments(ctor, newConstructor, false);
newInnerClass = genUtils.insertClassMember(newInnerClass, newConstructor);
}
}
}
}
-
+
if(innerClass != newInnerClass) {
genUtils.copyComments(innerClass, newInnerClass, true);
genUtils.copyComments(innerClass, newInnerClass, false);
}
return newInnerClass;
}
+ private ClassTree refactorInnerRecord(ClassTree innerRecord) {
+ refactoring.setInnerIsRecord(true);
+ ClassTree newInnerRecord = innerRecord;
+ List<? extends Tree> members = newInnerRecord.getMembers();
+ int insertAt=0;
+ for (Tree member : members) {
+ Tree.Kind kind = member.getKind();
+// System.out.println("member = " + kind + "" + member);
+
+ switch (kind) {
+ // The fields, except the static, should loose private final modifiers
+ // so that they appear without these modifiers in the record header
+ case VARIABLE:
+ VariableTree vt = (VariableTree) member;
+ ModifiersTree mt = vt.getModifiers();
+ if (mt.getFlags().contains(Modifier.STATIC)) {
+ continue;
+ }
+ Tree mtype = vt.getType();
+ Name mName = vt.getName();
+ ModifiersTree mods = make.Modifiers(EnumSet.noneOf(Modifier.class));
+ VariableTree newMember = make.RecordComponent(mods, mName, mtype);
+ newInnerRecord = make.removeClassMember(newInnerRecord, member);
+ newInnerRecord = make.insertClassMember(newInnerRecord, insertAt, newMember);
+ insertAt++;
+ break;
+ default:
+ }
+
+// if (member.getKind())
Review Comment:
looks like this can be removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] solves #7044, refactor inner to outer for record [netbeans]
Posted by "homberghp (via GitHub)" <gi...@apache.org>.
homberghp closed pull request #8197: solves #7044, refactor inner to outer for record
URL: https://github.com/apache/netbeans/pull/8197
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists