You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@netbeans.apache.org by Pieter van den Hombergh <pi...@gmail.com> on 2025/04/13 09:28:42 UTC
need help resolving issue 7044
Dear developers,
@Jan.Lahoda I have noticed that you are the last person working on
CasualDiff in relation with records.
I am trying to resolve issue7044 but am dumbfounded by the interaction
between the refactoring transformers and
classes in java.source.base, in particular CasualDiff and friends.
The code breaks on two, maybe three issues:
1. Varargs are not properly transferred to the new (new outer) record.
2. `implements` clauses are put in the wrong place, after the first
brace '{' of the record.
3. How to detect that a constructor is a compact constructor and omit
the parameter list.
I have added some tests (and a test helper class)
The failing test show the following: (the '|' shows where expected and
actual differ)
test test8Varargs PASSED with compare mode = IGNORE_WHITESPACE_DIFF
t/A.java expected
+ actual
t/A.java [ 0] package t;
package t;
t/A.java [ 1] record A(F f, String... params) {
record A(F f, String... params) {
t/A.java [ 2] public A {
public A {
t/A.java [ 3] assert f != null;
assert f != null;
t/A.java [ 4] }
}
t/A.java [ 5] }
}
test test8Varargs FAILED with compare mode = IGNORE_WHITESPACE_DIFF
t/F.java expected
+ actual
t/F.java [ 0] /*
/*
t/F.java [ 1] * Refactoring License
* Refactoring License
t/F.java [ 2] */
*/
t/F.java [ 3] package t;
package t;
t/F.java [ 4] /**
/**
t/F.java [ 5] *
*
t/F.java [ 6] * @author junit
* @author junit
t/F.java [ 7] */
*/
t/F.java [ 8] record F<P>(P first, String... second) {
| record F<P>(P first, String[] second) {
t/F.java [ 9] public F {
| public F(P first, String... second) {
t/F.java [ 10] assert null != first;
assert null != first;
t/F.java [ 11] assert null != second && second.length > 0;
assert null != second && second.length > 0;
t/F.java [ 12] }
}
t/F.java [ 13] }
}
Some issues might be resolved by detecting the actual record parameters.
A suitable constructor is always in the AST, either generated by the
compiler with name '<init>' of by the source code. This constructor shows
the varargs as intended, and is
in my opinion the correct candidate to pick up the varargs. I have given it
a try with a class called RecordUtils, but the resulting parameter list
somehow interferes with what CasualDiff expects
Anyway, it messes things up.
The implements clause causes another issue:
test test8RecordImplements1 PASSED with compare mode =
IGNORE_WHITESPACE_DIFF
t/A.java expected
+ actual
t/A.java [ 0] package t;
package t;
t/A.java [ 1] import java.time.LocalDate;
import java.time.LocalDate;
t/A.java [ 2] import java.io.Serializable;
import java.io.Serializable;
t/A.java [ 3] public class A implements Cloneable, Serializable {
public class A implements Cloneable, Serializable {
t/A.java [ 4] static F f;
static F f;
t/A.java [ 5] }
}
test test8RecordImplements1 FAILED with compare mode =
IGNORE_WHITESPACE_DIFF
t/F.java expected
+ actual
t/F.java [ 0] /*
/*
t/F.java [ 1] * Refactoring License
* Refactoring License
t/F.java [ 2] */
*/
t/F.java [ 3] package t;
package t;
t/F.java [ 4] import java.io.Serializable;
import java.io.Serializable;
t/F.java [ 5] /**
/**
t/F.java [ 6] *
*
t/F.java [ 7] * @author hom
* @author hom
t/F.java [ 8] */
*/
t/F.java [ 9] public record F(int x, int y) implements Cloneable,
Serializable { | public record F(int x, int y) { implements
Cloneable, Serializable
t/F.java [ 10] /** I should be back. */
/** I should be back. */
t/F.java [ 11] static String code = "nix";
static String code = "nix";
t/F.java [ 12] }
}
It is particularly annoying that the 'implements' in the original text for
the class A is transferred properly. I assume that this
is realised by the text copying part in CasualDif. However, the newly
generated class 'F' is wrong, and it is not clear to me who is responsible
for
the generation here. I was not able to fix this in CasualDiff without
breaking the formatting for either A of F in this case, and such changes
in CasualDiff might affect the expectations of other transformers (and did,
by my experience with the PullUp transformer).
I have seen recent changes in CasualDiff, in this case about permits
clauses (which are not relevant for records, as the can't be extended).
So could someone please shed some light on the interaction of CasualDiff
and the other collaborators.
Anyone that is willing to help, I have a fork with a branch at
https://github.com/homberghp/netbeans/tree/issue7044b
This is synced with the main branch on sunday April 13.
Thanks in advance.
Met vriendelijke groet,
Pieter van den Hombergh.
Kerboschstraat 12
5913 WH Venlo
Re: need help resolving issue 7044
Posted by Pieter van den Hombergh <pi...@gmail.com>.
Hi Jan,
A small addition.
From my observations and looking into JLS and the internals of
*com.sun.tools.javac.code.Flags*, I learned
1. The canonical constructor can easily be identified by a flag.
2. A canonical constructor is always generated by javac, in case the record
code does not provide one.
This canonical constructor has exactly the parameters in the format and
style (including generics and varargs) you would
need.
The code for that is equally simple to what you proposed:
List<JCTree.JCVariableDecl> result = record.getMembers().stream()
.filter(m -> m.getKind()== Kind.METHOD)
.map(JCTree.JCMethodDecl.class::cast)
.filter(met -> met.getReturnType() == null &&
(met.mods.flags & com.sun.tools.javac.code.Flags.RECORD)!=0 )
.flatMap(mt -> ((JCTree.JCMethodDecl)
mt).getParameters().stream())
.toList();
Met vriendelijke groet,
Pieter van den Hombergh.
Kerboschstraat 12
5913 WH Venlo
On Mon, Apr 14, 2025 at 10:21 PM Pieter van den Hombergh <
pieter.van.den.hombergh@gmail.com> wrote:
> Hi Jan,
>
> I understand your modification suggestion of VerryPretty to print the
> record parameters.
> I had that in an earlier attempt, which made it even to PR status, but I
> have closed that PR before acceptance, because of the
> 'implements after the {' so I started anew.
>
> Running my test on the code with your changes applied shows that the tests
> do not hit them. (I inserted an extra System.err.println())
>
> I have added record-specific test with file.
> *.../org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java*
> To make the output more intuitive, I have added a test helper class in
> harness/nbunit in file
> *.../org/netbeans/junit/AssertLinesEqualHelpers.java*
> and modified
> *.../org/netbeans/modules/refactoring/java/test/RefactoringTestbase.java*
>
> All can be found in https://github.com/homberghp/netbeans/tree/issue7044b.
> I have added the named files in a zip for your convenience.
> Of course, these files would have gone/will go into a successful PR.
>
> Inspect, and unpack (unzip -l) in the root of the NetBeans project/branch.
>
>
> Met vriendelijke groet,
> Pieter van den Hombergh.
> Kerboschstraat 12
> 5913 WH Venlo
>
>
> On Mon, Apr 14, 2025 at 7:59 PM Jan Lahoda <la...@gmail.com> wrote:
>
>> While it is true that some refactorings like inner to outter is mostly
>> copying stuff, the point of the code generator is to abstract from that.
>> So
>> that at the feature level (for each IDE feature) we don't need to think
>> about all the aspects of each language feature.
>>
>> When creating a tree that does not correspond to anything in the original
>> source, CasualDiff will, of course, use VeryPretty. And VeryPretty has
>> some
>> capability to simply copy the original text to the new place (see
>> VeryPretty.handlePossibleOldTrees), although I don't think it currently
>> works across files.
>>
>> I've sketched some very simply changes to make at least some simple
>> inner-to-outter refactoring partially work:
>> https://github.com/lahodaj/netbeans/tree/NETBEANS-7044-attempt
>>
>> Although there's much more to do. (Among other things, the tests should
>> primarily be in java.source.base.)
>>
>> Jan
>>
>>
>> On Mon, Apr 14, 2025 at 5:24 PM Pieter van den Hombergh <
>> pieter.van.den.hombergh@gmail.com> wrote:
>>
>> > Jan,
>> > thank you for your time to read and extensively answers to my questions.
>> >
>> > In the case of inner-to-outer the "old" AST (oldT in the code)
>> > is rather useless imho, because it has no relation to the original code
>> but
>> > instead is generated from a template, with an empty parameter list and
>> > empty class body. So diffing is actually rebuilding the code from the
>> AST.
>> > Which is what the VerryPretty code printer does or should do. Also, in
>> case
>> > of enum and record, which have no link to the host class, the "new"
>> code is
>> > a verbatim copy of the outed inner class, with the small exception of
>> what
>> > the code template brings for license and class javadoc code, and of
>> course
>> > out-denting.
>> >
>> > Where could we benefit from those facts?
>> > met vriendelijke groet
>> > Pieter van den Hombergh
>> >
>> > Op ma 14 apr 2025, 15:10 schreef Jan Lahoda <jl...@apache.org>:
>> >
>> > > Hi Pieter,
>> > >
>> > > In principle, the goal of CasualDiff is simple: having an original
>> AST,
>> > > original source text and an updated AST, and produce (textual) diffs
>> that
>> > > transform the original source text into a source text that (when
>> parsed)
>> > > would result in the updated AST.
>> > >
>> > > And the other code then can only take the original AST, and do
>> re-writes
>> > to
>> > > it such that the new re-written/updated AST is what they want, and
>> > > CasualDiff should make sure the textual representation is (or can be)
>> > > updated.
>> > >
>> > > In reality, this may get very tricky, esp. for AST shapes that don't
>> very
>> > > well match what's in the source code (like enums, multi-variables, and
>> > now
>> > > records). But, ideally, as long as the updated AST corresponds to an
>> AST
>> > > that would be produced for some source code, CasualDiff should be
>> able to
>> > > handle it.
>> > >
>> > > Also, sometimes, there need to be special methods or handling in
>> > TreeMaker
>> > > to create special AST nodes - like if we wanted clients to create
>> compact
>> > > constructors from scratch, we probably need a new method there. But,
>> that
>> > > may not be necessary for the case where the compact constructor is
>> just
>> > an
>> > > existing AST node.
>> > >
>> > > Let's see if I can look into this.
>> > >
>> > > Jan
>> > >
>> > >
>> > > On Sun, Apr 13, 2025 at 11:29 AM Pieter van den Hombergh <
>> > > pieter.van.den.hombergh@gmail.com> wrote:
>> > >
>> > > > Dear developers,
>> > > > @Jan.Lahoda I have noticed that you are the last person working on
>> > > > CasualDiff in relation with records.
>> > > >
>> > > > I am trying to resolve issue7044 but am dumbfounded by the
>> interaction
>> > > > between the refactoring transformers and
>> > > > classes in java.source.base, in particular CasualDiff and friends.
>> > > >
>> > > > The code breaks on two, maybe three issues:
>> > > >
>> > > > 1. Varargs are not properly transferred to the new (new outer)
>> > record.
>> > > > 2. `implements` clauses are put in the wrong place, after the
>> first
>> > > > brace '{' of the record.
>> > > > 3. How to detect that a constructor is a compact constructor and
>> > omit
>> > > > the parameter list.
>> > > >
>> > > > I have added some tests (and a test helper class)
>> > > > The failing test show the following: (the '|' shows where expected
>> and
>> > > > actual differ)
>> > > > test test8Varargs PASSED with compare mode = IGNORE_WHITESPACE_DIFF
>> > > > t/A.java expected
>> > > > + actual
>> > > >
>> > > > t/A.java [ 0] package t;
>> > > > package t;
>> > > >
>> > > > t/A.java [ 1] record A(F f, String... params) {
>> > > > record A(F f, String... params) {
>> > > >
>> > > > t/A.java [ 2] public A {
>> > > > public A {
>> > > >
>> > > > t/A.java [ 3] assert f != null;
>> > > > assert f != null;
>> > > >
>> > > > t/A.java [ 4] }
>> > > > }
>> > > >
>> > > > t/A.java [ 5] }
>> > > > }
>> > > >
>> > > >
>> > > > test test8Varargs FAILED with compare mode = IGNORE_WHITESPACE_DIFF
>> > > > t/F.java expected
>> > > > + actual
>> > > >
>> > > > t/F.java [ 0] /*
>> > > > /*
>> > > >
>> > > > t/F.java [ 1] * Refactoring License
>> > > > * Refactoring License
>> > > >
>> > > > t/F.java [ 2] */
>> > > > */
>> > > >
>> > > > t/F.java [ 3] package t;
>> > > > package t;
>> > > >
>> > > > t/F.java [ 4] /**
>> > > > /**
>> > > >
>> > > > t/F.java [ 5] *
>> > > > *
>> > > >
>> > > > t/F.java [ 6] * @author junit
>> > > > * @author junit
>> > > >
>> > > > t/F.java [ 7] */
>> > > > */
>> > > >
>> > > > t/F.java [ 8] record F<P>(P first, String... second) {
>> > > > | record F<P>(P first, String[] second) {
>> > > >
>> > > > t/F.java [ 9] public F {
>> > > > | public F(P first, String... second) {
>> > > >
>> > > > t/F.java [ 10] assert null != first;
>> > > > assert null != first;
>> > > >
>> > > > t/F.java [ 11] assert null != second && second.length > 0;
>> > > > assert null != second &&
>> second.length >
>> > > 0;
>> > > >
>> > > > t/F.java [ 12] }
>> > > > }
>> > > >
>> > > > t/F.java [ 13] }
>> > > > }
>> > > >
>> > > >
>> > > > Some issues might be resolved by detecting the actual record
>> > parameters.
>> > > > A suitable constructor is always in the AST, either generated by the
>> > > > compiler with name '<init>' of by the source code. This constructor
>> > shows
>> > > > the varargs as intended, and is
>> > > > in my opinion the correct candidate to pick up the varargs. I have
>> > given
>> > > it
>> > > > a try with a class called RecordUtils, but the resulting parameter
>> list
>> > > > somehow interferes with what CasualDiff expects
>> > > > Anyway, it messes things up.
>> > > >
>> > > > The implements clause causes another issue:
>> > > > test test8RecordImplements1 PASSED with compare mode =
>> > > > IGNORE_WHITESPACE_DIFF
>> > > > t/A.java expected
>> > > > + actual
>> > > >
>> > > > t/A.java [ 0] package t;
>> > > > package t;
>> > > >
>> > > > t/A.java [ 1] import java.time.LocalDate;
>> > > > import java.time.LocalDate;
>> > > >
>> > > > t/A.java [ 2] import java.io.Serializable;
>> > > > import java.io.Serializable;
>> > > >
>> > > > t/A.java [ 3] public class A implements Cloneable, Serializable {
>> > > > public class A implements Cloneable,
>> > Serializable
>> > > {
>> > > >
>> > > > t/A.java [ 4] static F f;
>> > > > static F f;
>> > > >
>> > > > t/A.java [ 5] }
>> > > > }
>> > > >
>> > > >
>> > > > test test8RecordImplements1 FAILED with compare mode =
>> > > > IGNORE_WHITESPACE_DIFF
>> > > > t/F.java expected
>> > > > + actual
>> > > >
>> > > > t/F.java [ 0] /*
>> > > > /*
>> > > >
>> > > > t/F.java [ 1] * Refactoring License
>> > > > * Refactoring License
>> > > >
>> > > > t/F.java [ 2] */
>> > > > */
>> > > >
>> > > > t/F.java [ 3] package t;
>> > > > package t;
>> > > >
>> > > > t/F.java [ 4] import java.io.Serializable;
>> > > > import java.io.Serializable;
>> > > >
>> > > > t/F.java [ 5] /**
>> > > > /**
>> > > >
>> > > > t/F.java [ 6] *
>> > > > *
>> > > >
>> > > > t/F.java [ 7] * @author hom
>> > > > * @author hom
>> > > >
>> > > > t/F.java [ 8] */
>> > > > */
>> > > >
>> > > > t/F.java [ 9] public record F(int x, int y) implements Cloneable,
>> > > > Serializable { | public record F(int x, int y) {
>> > implements
>> > > > Cloneable, Serializable
>> > > > t/F.java [ 10] /** I should be back. */
>> > > > /** I should be back. */
>> > > >
>> > > > t/F.java [ 11] static String code = "nix";
>> > > > static String code = "nix";
>> > > >
>> > > > t/F.java [ 12] }
>> > > > }
>> > > >
>> > > >
>> > > > It is particularly annoying that the 'implements' in the original
>> text
>> > > for
>> > > > the class A is transferred properly. I assume that this
>> > > > is realised by the text copying part in CasualDif. However, the
>> newly
>> > > > generated class 'F' is wrong, and it is not clear to me who is
>> > > responsible
>> > > > for
>> > > > the generation here. I was not able to fix this in CasualDiff
>> without
>> > > > breaking the formatting for either A of F in this case, and such
>> > changes
>> > > > in CasualDiff might affect the expectations of other transformers
>> (and
>> > > did,
>> > > > by my experience with the PullUp transformer).
>> > > >
>> > > > I have seen recent changes in CasualDiff, in this case about permits
>> > > > clauses (which are not relevant for records, as the can't be
>> extended).
>> > > > So could someone please shed some light on the interaction of
>> > CasualDiff
>> > > > and the other collaborators.
>> > > >
>> > > > Anyone that is willing to help, I have a fork with a branch at
>> > > > https://github.com/homberghp/netbeans/tree/issue7044b
>> > > > This is synced with the main branch on sunday April 13.
>> > > >
>> > > > Thanks in advance.
>> > > >
>> > > > Met vriendelijke groet,
>> > > > Pieter van den Hombergh.
>> > > > Kerboschstraat 12
>> > > > 5913 WH Venlo
>> > > >
>> > >
>> >
>>
>
Re: need help resolving issue 7044
Posted by Pieter van den Hombergh <pi...@gmail.com>.
Hi Jan,
I understand your modification suggestion of VerryPretty to print the
record parameters.
I had that in an earlier attempt, which made it even to PR status, but I
have closed that PR before acceptance, because of the
'implements after the {' so I started anew.
Running my test on the code with your changes applied shows that the tests
do not hit them. (I inserted an extra System.err.println())
I have added record-specific test with file.
*.../org/netbeans/modules/refactoring/java/test/InnerOuterRecordTest.java*
To make the output more intuitive, I have added a test helper class in
harness/nbunit in file
*.../org/netbeans/junit/AssertLinesEqualHelpers.java*
and modified
*.../org/netbeans/modules/refactoring/java/test/RefactoringTestbase.java*
All can be found in https://github.com/homberghp/netbeans/tree/issue7044b.
I have added the named files in a zip for your convenience.
Of course, these files would have gone/will go into a successful PR.
Inspect, and unpack (unzip -l) in the root of the NetBeans project/branch.
Met vriendelijke groet,
Pieter van den Hombergh.
Kerboschstraat 12
5913 WH Venlo
On Mon, Apr 14, 2025 at 7:59 PM Jan Lahoda <la...@gmail.com> wrote:
> While it is true that some refactorings like inner to outter is mostly
> copying stuff, the point of the code generator is to abstract from that. So
> that at the feature level (for each IDE feature) we don't need to think
> about all the aspects of each language feature.
>
> When creating a tree that does not correspond to anything in the original
> source, CasualDiff will, of course, use VeryPretty. And VeryPretty has some
> capability to simply copy the original text to the new place (see
> VeryPretty.handlePossibleOldTrees), although I don't think it currently
> works across files.
>
> I've sketched some very simply changes to make at least some simple
> inner-to-outter refactoring partially work:
> https://github.com/lahodaj/netbeans/tree/NETBEANS-7044-attempt
>
> Although there's much more to do. (Among other things, the tests should
> primarily be in java.source.base.)
>
> Jan
>
>
> On Mon, Apr 14, 2025 at 5:24 PM Pieter van den Hombergh <
> pieter.van.den.hombergh@gmail.com> wrote:
>
> > Jan,
> > thank you for your time to read and extensively answers to my questions.
> >
> > In the case of inner-to-outer the "old" AST (oldT in the code)
> > is rather useless imho, because it has no relation to the original code
> but
> > instead is generated from a template, with an empty parameter list and
> > empty class body. So diffing is actually rebuilding the code from the
> AST.
> > Which is what the VerryPretty code printer does or should do. Also, in
> case
> > of enum and record, which have no link to the host class, the "new" code
> is
> > a verbatim copy of the outed inner class, with the small exception of
> what
> > the code template brings for license and class javadoc code, and of
> course
> > out-denting.
> >
> > Where could we benefit from those facts?
> > met vriendelijke groet
> > Pieter van den Hombergh
> >
> > Op ma 14 apr 2025, 15:10 schreef Jan Lahoda <jl...@apache.org>:
> >
> > > Hi Pieter,
> > >
> > > In principle, the goal of CasualDiff is simple: having an original AST,
> > > original source text and an updated AST, and produce (textual) diffs
> that
> > > transform the original source text into a source text that (when
> parsed)
> > > would result in the updated AST.
> > >
> > > And the other code then can only take the original AST, and do
> re-writes
> > to
> > > it such that the new re-written/updated AST is what they want, and
> > > CasualDiff should make sure the textual representation is (or can be)
> > > updated.
> > >
> > > In reality, this may get very tricky, esp. for AST shapes that don't
> very
> > > well match what's in the source code (like enums, multi-variables, and
> > now
> > > records). But, ideally, as long as the updated AST corresponds to an
> AST
> > > that would be produced for some source code, CasualDiff should be able
> to
> > > handle it.
> > >
> > > Also, sometimes, there need to be special methods or handling in
> > TreeMaker
> > > to create special AST nodes - like if we wanted clients to create
> compact
> > > constructors from scratch, we probably need a new method there. But,
> that
> > > may not be necessary for the case where the compact constructor is just
> > an
> > > existing AST node.
> > >
> > > Let's see if I can look into this.
> > >
> > > Jan
> > >
> > >
> > > On Sun, Apr 13, 2025 at 11:29 AM Pieter van den Hombergh <
> > > pieter.van.den.hombergh@gmail.com> wrote:
> > >
> > > > Dear developers,
> > > > @Jan.Lahoda I have noticed that you are the last person working on
> > > > CasualDiff in relation with records.
> > > >
> > > > I am trying to resolve issue7044 but am dumbfounded by the
> interaction
> > > > between the refactoring transformers and
> > > > classes in java.source.base, in particular CasualDiff and friends.
> > > >
> > > > The code breaks on two, maybe three issues:
> > > >
> > > > 1. Varargs are not properly transferred to the new (new outer)
> > record.
> > > > 2. `implements` clauses are put in the wrong place, after the
> first
> > > > brace '{' of the record.
> > > > 3. How to detect that a constructor is a compact constructor and
> > omit
> > > > the parameter list.
> > > >
> > > > I have added some tests (and a test helper class)
> > > > The failing test show the following: (the '|' shows where expected
> and
> > > > actual differ)
> > > > test test8Varargs PASSED with compare mode = IGNORE_WHITESPACE_DIFF
> > > > t/A.java expected
> > > > + actual
> > > >
> > > > t/A.java [ 0] package t;
> > > > package t;
> > > >
> > > > t/A.java [ 1] record A(F f, String... params) {
> > > > record A(F f, String... params) {
> > > >
> > > > t/A.java [ 2] public A {
> > > > public A {
> > > >
> > > > t/A.java [ 3] assert f != null;
> > > > assert f != null;
> > > >
> > > > t/A.java [ 4] }
> > > > }
> > > >
> > > > t/A.java [ 5] }
> > > > }
> > > >
> > > >
> > > > test test8Varargs FAILED with compare mode = IGNORE_WHITESPACE_DIFF
> > > > t/F.java expected
> > > > + actual
> > > >
> > > > t/F.java [ 0] /*
> > > > /*
> > > >
> > > > t/F.java [ 1] * Refactoring License
> > > > * Refactoring License
> > > >
> > > > t/F.java [ 2] */
> > > > */
> > > >
> > > > t/F.java [ 3] package t;
> > > > package t;
> > > >
> > > > t/F.java [ 4] /**
> > > > /**
> > > >
> > > > t/F.java [ 5] *
> > > > *
> > > >
> > > > t/F.java [ 6] * @author junit
> > > > * @author junit
> > > >
> > > > t/F.java [ 7] */
> > > > */
> > > >
> > > > t/F.java [ 8] record F<P>(P first, String... second) {
> > > > | record F<P>(P first, String[] second) {
> > > >
> > > > t/F.java [ 9] public F {
> > > > | public F(P first, String... second) {
> > > >
> > > > t/F.java [ 10] assert null != first;
> > > > assert null != first;
> > > >
> > > > t/F.java [ 11] assert null != second && second.length > 0;
> > > > assert null != second &&
> second.length >
> > > 0;
> > > >
> > > > t/F.java [ 12] }
> > > > }
> > > >
> > > > t/F.java [ 13] }
> > > > }
> > > >
> > > >
> > > > Some issues might be resolved by detecting the actual record
> > parameters.
> > > > A suitable constructor is always in the AST, either generated by the
> > > > compiler with name '<init>' of by the source code. This constructor
> > shows
> > > > the varargs as intended, and is
> > > > in my opinion the correct candidate to pick up the varargs. I have
> > given
> > > it
> > > > a try with a class called RecordUtils, but the resulting parameter
> list
> > > > somehow interferes with what CasualDiff expects
> > > > Anyway, it messes things up.
> > > >
> > > > The implements clause causes another issue:
> > > > test test8RecordImplements1 PASSED with compare mode =
> > > > IGNORE_WHITESPACE_DIFF
> > > > t/A.java expected
> > > > + actual
> > > >
> > > > t/A.java [ 0] package t;
> > > > package t;
> > > >
> > > > t/A.java [ 1] import java.time.LocalDate;
> > > > import java.time.LocalDate;
> > > >
> > > > t/A.java [ 2] import java.io.Serializable;
> > > > import java.io.Serializable;
> > > >
> > > > t/A.java [ 3] public class A implements Cloneable, Serializable {
> > > > public class A implements Cloneable,
> > Serializable
> > > {
> > > >
> > > > t/A.java [ 4] static F f;
> > > > static F f;
> > > >
> > > > t/A.java [ 5] }
> > > > }
> > > >
> > > >
> > > > test test8RecordImplements1 FAILED with compare mode =
> > > > IGNORE_WHITESPACE_DIFF
> > > > t/F.java expected
> > > > + actual
> > > >
> > > > t/F.java [ 0] /*
> > > > /*
> > > >
> > > > t/F.java [ 1] * Refactoring License
> > > > * Refactoring License
> > > >
> > > > t/F.java [ 2] */
> > > > */
> > > >
> > > > t/F.java [ 3] package t;
> > > > package t;
> > > >
> > > > t/F.java [ 4] import java.io.Serializable;
> > > > import java.io.Serializable;
> > > >
> > > > t/F.java [ 5] /**
> > > > /**
> > > >
> > > > t/F.java [ 6] *
> > > > *
> > > >
> > > > t/F.java [ 7] * @author hom
> > > > * @author hom
> > > >
> > > > t/F.java [ 8] */
> > > > */
> > > >
> > > > t/F.java [ 9] public record F(int x, int y) implements Cloneable,
> > > > Serializable { | public record F(int x, int y) {
> > implements
> > > > Cloneable, Serializable
> > > > t/F.java [ 10] /** I should be back. */
> > > > /** I should be back. */
> > > >
> > > > t/F.java [ 11] static String code = "nix";
> > > > static String code = "nix";
> > > >
> > > > t/F.java [ 12] }
> > > > }
> > > >
> > > >
> > > > It is particularly annoying that the 'implements' in the original
> text
> > > for
> > > > the class A is transferred properly. I assume that this
> > > > is realised by the text copying part in CasualDif. However, the newly
> > > > generated class 'F' is wrong, and it is not clear to me who is
> > > responsible
> > > > for
> > > > the generation here. I was not able to fix this in CasualDiff without
> > > > breaking the formatting for either A of F in this case, and such
> > changes
> > > > in CasualDiff might affect the expectations of other transformers
> (and
> > > did,
> > > > by my experience with the PullUp transformer).
> > > >
> > > > I have seen recent changes in CasualDiff, in this case about permits
> > > > clauses (which are not relevant for records, as the can't be
> extended).
> > > > So could someone please shed some light on the interaction of
> > CasualDiff
> > > > and the other collaborators.
> > > >
> > > > Anyone that is willing to help, I have a fork with a branch at
> > > > https://github.com/homberghp/netbeans/tree/issue7044b
> > > > This is synced with the main branch on sunday April 13.
> > > >
> > > > Thanks in advance.
> > > >
> > > > Met vriendelijke groet,
> > > > Pieter van den Hombergh.
> > > > Kerboschstraat 12
> > > > 5913 WH Venlo
> > > >
> > >
> >
>
Re: need help resolving issue 7044
Posted by Jan Lahoda <la...@gmail.com>.
While it is true that some refactorings like inner to outter is mostly
copying stuff, the point of the code generator is to abstract from that. So
that at the feature level (for each IDE feature) we don't need to think
about all the aspects of each language feature.
When creating a tree that does not correspond to anything in the original
source, CasualDiff will, of course, use VeryPretty. And VeryPretty has some
capability to simply copy the original text to the new place (see
VeryPretty.handlePossibleOldTrees), although I don't think it currently
works across files.
I've sketched some very simply changes to make at least some simple
inner-to-outter refactoring partially work:
https://github.com/lahodaj/netbeans/tree/NETBEANS-7044-attempt
Although there's much more to do. (Among other things, the tests should
primarily be in java.source.base.)
Jan
On Mon, Apr 14, 2025 at 5:24 PM Pieter van den Hombergh <
pieter.van.den.hombergh@gmail.com> wrote:
> Jan,
> thank you for your time to read and extensively answers to my questions.
>
> In the case of inner-to-outer the "old" AST (oldT in the code)
> is rather useless imho, because it has no relation to the original code but
> instead is generated from a template, with an empty parameter list and
> empty class body. So diffing is actually rebuilding the code from the AST.
> Which is what the VerryPretty code printer does or should do. Also, in case
> of enum and record, which have no link to the host class, the "new" code is
> a verbatim copy of the outed inner class, with the small exception of what
> the code template brings for license and class javadoc code, and of course
> out-denting.
>
> Where could we benefit from those facts?
> met vriendelijke groet
> Pieter van den Hombergh
>
> Op ma 14 apr 2025, 15:10 schreef Jan Lahoda <jl...@apache.org>:
>
> > Hi Pieter,
> >
> > In principle, the goal of CasualDiff is simple: having an original AST,
> > original source text and an updated AST, and produce (textual) diffs that
> > transform the original source text into a source text that (when parsed)
> > would result in the updated AST.
> >
> > And the other code then can only take the original AST, and do re-writes
> to
> > it such that the new re-written/updated AST is what they want, and
> > CasualDiff should make sure the textual representation is (or can be)
> > updated.
> >
> > In reality, this may get very tricky, esp. for AST shapes that don't very
> > well match what's in the source code (like enums, multi-variables, and
> now
> > records). But, ideally, as long as the updated AST corresponds to an AST
> > that would be produced for some source code, CasualDiff should be able to
> > handle it.
> >
> > Also, sometimes, there need to be special methods or handling in
> TreeMaker
> > to create special AST nodes - like if we wanted clients to create compact
> > constructors from scratch, we probably need a new method there. But, that
> > may not be necessary for the case where the compact constructor is just
> an
> > existing AST node.
> >
> > Let's see if I can look into this.
> >
> > Jan
> >
> >
> > On Sun, Apr 13, 2025 at 11:29 AM Pieter van den Hombergh <
> > pieter.van.den.hombergh@gmail.com> wrote:
> >
> > > Dear developers,
> > > @Jan.Lahoda I have noticed that you are the last person working on
> > > CasualDiff in relation with records.
> > >
> > > I am trying to resolve issue7044 but am dumbfounded by the interaction
> > > between the refactoring transformers and
> > > classes in java.source.base, in particular CasualDiff and friends.
> > >
> > > The code breaks on two, maybe three issues:
> > >
> > > 1. Varargs are not properly transferred to the new (new outer)
> record.
> > > 2. `implements` clauses are put in the wrong place, after the first
> > > brace '{' of the record.
> > > 3. How to detect that a constructor is a compact constructor and
> omit
> > > the parameter list.
> > >
> > > I have added some tests (and a test helper class)
> > > The failing test show the following: (the '|' shows where expected and
> > > actual differ)
> > > test test8Varargs PASSED with compare mode = IGNORE_WHITESPACE_DIFF
> > > t/A.java expected
> > > + actual
> > >
> > > t/A.java [ 0] package t;
> > > package t;
> > >
> > > t/A.java [ 1] record A(F f, String... params) {
> > > record A(F f, String... params) {
> > >
> > > t/A.java [ 2] public A {
> > > public A {
> > >
> > > t/A.java [ 3] assert f != null;
> > > assert f != null;
> > >
> > > t/A.java [ 4] }
> > > }
> > >
> > > t/A.java [ 5] }
> > > }
> > >
> > >
> > > test test8Varargs FAILED with compare mode = IGNORE_WHITESPACE_DIFF
> > > t/F.java expected
> > > + actual
> > >
> > > t/F.java [ 0] /*
> > > /*
> > >
> > > t/F.java [ 1] * Refactoring License
> > > * Refactoring License
> > >
> > > t/F.java [ 2] */
> > > */
> > >
> > > t/F.java [ 3] package t;
> > > package t;
> > >
> > > t/F.java [ 4] /**
> > > /**
> > >
> > > t/F.java [ 5] *
> > > *
> > >
> > > t/F.java [ 6] * @author junit
> > > * @author junit
> > >
> > > t/F.java [ 7] */
> > > */
> > >
> > > t/F.java [ 8] record F<P>(P first, String... second) {
> > > | record F<P>(P first, String[] second) {
> > >
> > > t/F.java [ 9] public F {
> > > | public F(P first, String... second) {
> > >
> > > t/F.java [ 10] assert null != first;
> > > assert null != first;
> > >
> > > t/F.java [ 11] assert null != second && second.length > 0;
> > > assert null != second && second.length >
> > 0;
> > >
> > > t/F.java [ 12] }
> > > }
> > >
> > > t/F.java [ 13] }
> > > }
> > >
> > >
> > > Some issues might be resolved by detecting the actual record
> parameters.
> > > A suitable constructor is always in the AST, either generated by the
> > > compiler with name '<init>' of by the source code. This constructor
> shows
> > > the varargs as intended, and is
> > > in my opinion the correct candidate to pick up the varargs. I have
> given
> > it
> > > a try with a class called RecordUtils, but the resulting parameter list
> > > somehow interferes with what CasualDiff expects
> > > Anyway, it messes things up.
> > >
> > > The implements clause causes another issue:
> > > test test8RecordImplements1 PASSED with compare mode =
> > > IGNORE_WHITESPACE_DIFF
> > > t/A.java expected
> > > + actual
> > >
> > > t/A.java [ 0] package t;
> > > package t;
> > >
> > > t/A.java [ 1] import java.time.LocalDate;
> > > import java.time.LocalDate;
> > >
> > > t/A.java [ 2] import java.io.Serializable;
> > > import java.io.Serializable;
> > >
> > > t/A.java [ 3] public class A implements Cloneable, Serializable {
> > > public class A implements Cloneable,
> Serializable
> > {
> > >
> > > t/A.java [ 4] static F f;
> > > static F f;
> > >
> > > t/A.java [ 5] }
> > > }
> > >
> > >
> > > test test8RecordImplements1 FAILED with compare mode =
> > > IGNORE_WHITESPACE_DIFF
> > > t/F.java expected
> > > + actual
> > >
> > > t/F.java [ 0] /*
> > > /*
> > >
> > > t/F.java [ 1] * Refactoring License
> > > * Refactoring License
> > >
> > > t/F.java [ 2] */
> > > */
> > >
> > > t/F.java [ 3] package t;
> > > package t;
> > >
> > > t/F.java [ 4] import java.io.Serializable;
> > > import java.io.Serializable;
> > >
> > > t/F.java [ 5] /**
> > > /**
> > >
> > > t/F.java [ 6] *
> > > *
> > >
> > > t/F.java [ 7] * @author hom
> > > * @author hom
> > >
> > > t/F.java [ 8] */
> > > */
> > >
> > > t/F.java [ 9] public record F(int x, int y) implements Cloneable,
> > > Serializable { | public record F(int x, int y) {
> implements
> > > Cloneable, Serializable
> > > t/F.java [ 10] /** I should be back. */
> > > /** I should be back. */
> > >
> > > t/F.java [ 11] static String code = "nix";
> > > static String code = "nix";
> > >
> > > t/F.java [ 12] }
> > > }
> > >
> > >
> > > It is particularly annoying that the 'implements' in the original text
> > for
> > > the class A is transferred properly. I assume that this
> > > is realised by the text copying part in CasualDif. However, the newly
> > > generated class 'F' is wrong, and it is not clear to me who is
> > responsible
> > > for
> > > the generation here. I was not able to fix this in CasualDiff without
> > > breaking the formatting for either A of F in this case, and such
> changes
> > > in CasualDiff might affect the expectations of other transformers (and
> > did,
> > > by my experience with the PullUp transformer).
> > >
> > > I have seen recent changes in CasualDiff, in this case about permits
> > > clauses (which are not relevant for records, as the can't be extended).
> > > So could someone please shed some light on the interaction of
> CasualDiff
> > > and the other collaborators.
> > >
> > > Anyone that is willing to help, I have a fork with a branch at
> > > https://github.com/homberghp/netbeans/tree/issue7044b
> > > This is synced with the main branch on sunday April 13.
> > >
> > > Thanks in advance.
> > >
> > > Met vriendelijke groet,
> > > Pieter van den Hombergh.
> > > Kerboschstraat 12
> > > 5913 WH Venlo
> > >
> >
>
Re: need help resolving issue 7044
Posted by Pieter van den Hombergh <pi...@gmail.com>.
Jan,
thank you for your time to read and extensively answers to my questions.
In the case of inner-to-outer the "old" AST (oldT in the code)
is rather useless imho, because it has no relation to the original code but
instead is generated from a template, with an empty parameter list and
empty class body. So diffing is actually rebuilding the code from the AST.
Which is what the VerryPretty code printer does or should do. Also, in case
of enum and record, which have no link to the host class, the "new" code is
a verbatim copy of the outed inner class, with the small exception of what
the code template brings for license and class javadoc code, and of course
out-denting.
Where could we benefit from those facts?
met vriendelijke groet
Pieter van den Hombergh
Op ma 14 apr 2025, 15:10 schreef Jan Lahoda <jl...@apache.org>:
> Hi Pieter,
>
> In principle, the goal of CasualDiff is simple: having an original AST,
> original source text and an updated AST, and produce (textual) diffs that
> transform the original source text into a source text that (when parsed)
> would result in the updated AST.
>
> And the other code then can only take the original AST, and do re-writes to
> it such that the new re-written/updated AST is what they want, and
> CasualDiff should make sure the textual representation is (or can be)
> updated.
>
> In reality, this may get very tricky, esp. for AST shapes that don't very
> well match what's in the source code (like enums, multi-variables, and now
> records). But, ideally, as long as the updated AST corresponds to an AST
> that would be produced for some source code, CasualDiff should be able to
> handle it.
>
> Also, sometimes, there need to be special methods or handling in TreeMaker
> to create special AST nodes - like if we wanted clients to create compact
> constructors from scratch, we probably need a new method there. But, that
> may not be necessary for the case where the compact constructor is just an
> existing AST node.
>
> Let's see if I can look into this.
>
> Jan
>
>
> On Sun, Apr 13, 2025 at 11:29 AM Pieter van den Hombergh <
> pieter.van.den.hombergh@gmail.com> wrote:
>
> > Dear developers,
> > @Jan.Lahoda I have noticed that you are the last person working on
> > CasualDiff in relation with records.
> >
> > I am trying to resolve issue7044 but am dumbfounded by the interaction
> > between the refactoring transformers and
> > classes in java.source.base, in particular CasualDiff and friends.
> >
> > The code breaks on two, maybe three issues:
> >
> > 1. Varargs are not properly transferred to the new (new outer) record.
> > 2. `implements` clauses are put in the wrong place, after the first
> > brace '{' of the record.
> > 3. How to detect that a constructor is a compact constructor and omit
> > the parameter list.
> >
> > I have added some tests (and a test helper class)
> > The failing test show the following: (the '|' shows where expected and
> > actual differ)
> > test test8Varargs PASSED with compare mode = IGNORE_WHITESPACE_DIFF
> > t/A.java expected
> > + actual
> >
> > t/A.java [ 0] package t;
> > package t;
> >
> > t/A.java [ 1] record A(F f, String... params) {
> > record A(F f, String... params) {
> >
> > t/A.java [ 2] public A {
> > public A {
> >
> > t/A.java [ 3] assert f != null;
> > assert f != null;
> >
> > t/A.java [ 4] }
> > }
> >
> > t/A.java [ 5] }
> > }
> >
> >
> > test test8Varargs FAILED with compare mode = IGNORE_WHITESPACE_DIFF
> > t/F.java expected
> > + actual
> >
> > t/F.java [ 0] /*
> > /*
> >
> > t/F.java [ 1] * Refactoring License
> > * Refactoring License
> >
> > t/F.java [ 2] */
> > */
> >
> > t/F.java [ 3] package t;
> > package t;
> >
> > t/F.java [ 4] /**
> > /**
> >
> > t/F.java [ 5] *
> > *
> >
> > t/F.java [ 6] * @author junit
> > * @author junit
> >
> > t/F.java [ 7] */
> > */
> >
> > t/F.java [ 8] record F<P>(P first, String... second) {
> > | record F<P>(P first, String[] second) {
> >
> > t/F.java [ 9] public F {
> > | public F(P first, String... second) {
> >
> > t/F.java [ 10] assert null != first;
> > assert null != first;
> >
> > t/F.java [ 11] assert null != second && second.length > 0;
> > assert null != second && second.length >
> 0;
> >
> > t/F.java [ 12] }
> > }
> >
> > t/F.java [ 13] }
> > }
> >
> >
> > Some issues might be resolved by detecting the actual record parameters.
> > A suitable constructor is always in the AST, either generated by the
> > compiler with name '<init>' of by the source code. This constructor shows
> > the varargs as intended, and is
> > in my opinion the correct candidate to pick up the varargs. I have given
> it
> > a try with a class called RecordUtils, but the resulting parameter list
> > somehow interferes with what CasualDiff expects
> > Anyway, it messes things up.
> >
> > The implements clause causes another issue:
> > test test8RecordImplements1 PASSED with compare mode =
> > IGNORE_WHITESPACE_DIFF
> > t/A.java expected
> > + actual
> >
> > t/A.java [ 0] package t;
> > package t;
> >
> > t/A.java [ 1] import java.time.LocalDate;
> > import java.time.LocalDate;
> >
> > t/A.java [ 2] import java.io.Serializable;
> > import java.io.Serializable;
> >
> > t/A.java [ 3] public class A implements Cloneable, Serializable {
> > public class A implements Cloneable, Serializable
> {
> >
> > t/A.java [ 4] static F f;
> > static F f;
> >
> > t/A.java [ 5] }
> > }
> >
> >
> > test test8RecordImplements1 FAILED with compare mode =
> > IGNORE_WHITESPACE_DIFF
> > t/F.java expected
> > + actual
> >
> > t/F.java [ 0] /*
> > /*
> >
> > t/F.java [ 1] * Refactoring License
> > * Refactoring License
> >
> > t/F.java [ 2] */
> > */
> >
> > t/F.java [ 3] package t;
> > package t;
> >
> > t/F.java [ 4] import java.io.Serializable;
> > import java.io.Serializable;
> >
> > t/F.java [ 5] /**
> > /**
> >
> > t/F.java [ 6] *
> > *
> >
> > t/F.java [ 7] * @author hom
> > * @author hom
> >
> > t/F.java [ 8] */
> > */
> >
> > t/F.java [ 9] public record F(int x, int y) implements Cloneable,
> > Serializable { | public record F(int x, int y) { implements
> > Cloneable, Serializable
> > t/F.java [ 10] /** I should be back. */
> > /** I should be back. */
> >
> > t/F.java [ 11] static String code = "nix";
> > static String code = "nix";
> >
> > t/F.java [ 12] }
> > }
> >
> >
> > It is particularly annoying that the 'implements' in the original text
> for
> > the class A is transferred properly. I assume that this
> > is realised by the text copying part in CasualDif. However, the newly
> > generated class 'F' is wrong, and it is not clear to me who is
> responsible
> > for
> > the generation here. I was not able to fix this in CasualDiff without
> > breaking the formatting for either A of F in this case, and such changes
> > in CasualDiff might affect the expectations of other transformers (and
> did,
> > by my experience with the PullUp transformer).
> >
> > I have seen recent changes in CasualDiff, in this case about permits
> > clauses (which are not relevant for records, as the can't be extended).
> > So could someone please shed some light on the interaction of CasualDiff
> > and the other collaborators.
> >
> > Anyone that is willing to help, I have a fork with a branch at
> > https://github.com/homberghp/netbeans/tree/issue7044b
> > This is synced with the main branch on sunday April 13.
> >
> > Thanks in advance.
> >
> > Met vriendelijke groet,
> > Pieter van den Hombergh.
> > Kerboschstraat 12
> > 5913 WH Venlo
> >
>
Re: need help resolving issue 7044
Posted by Jan Lahoda <jl...@apache.org>.
Hi Pieter,
In principle, the goal of CasualDiff is simple: having an original AST,
original source text and an updated AST, and produce (textual) diffs that
transform the original source text into a source text that (when parsed)
would result in the updated AST.
And the other code then can only take the original AST, and do re-writes to
it such that the new re-written/updated AST is what they want, and
CasualDiff should make sure the textual representation is (or can be)
updated.
In reality, this may get very tricky, esp. for AST shapes that don't very
well match what's in the source code (like enums, multi-variables, and now
records). But, ideally, as long as the updated AST corresponds to an AST
that would be produced for some source code, CasualDiff should be able to
handle it.
Also, sometimes, there need to be special methods or handling in TreeMaker
to create special AST nodes - like if we wanted clients to create compact
constructors from scratch, we probably need a new method there. But, that
may not be necessary for the case where the compact constructor is just an
existing AST node.
Let's see if I can look into this.
Jan
On Sun, Apr 13, 2025 at 11:29 AM Pieter van den Hombergh <
pieter.van.den.hombergh@gmail.com> wrote:
> Dear developers,
> @Jan.Lahoda I have noticed that you are the last person working on
> CasualDiff in relation with records.
>
> I am trying to resolve issue7044 but am dumbfounded by the interaction
> between the refactoring transformers and
> classes in java.source.base, in particular CasualDiff and friends.
>
> The code breaks on two, maybe three issues:
>
> 1. Varargs are not properly transferred to the new (new outer) record.
> 2. `implements` clauses are put in the wrong place, after the first
> brace '{' of the record.
> 3. How to detect that a constructor is a compact constructor and omit
> the parameter list.
>
> I have added some tests (and a test helper class)
> The failing test show the following: (the '|' shows where expected and
> actual differ)
> test test8Varargs PASSED with compare mode = IGNORE_WHITESPACE_DIFF
> t/A.java expected
> + actual
>
> t/A.java [ 0] package t;
> package t;
>
> t/A.java [ 1] record A(F f, String... params) {
> record A(F f, String... params) {
>
> t/A.java [ 2] public A {
> public A {
>
> t/A.java [ 3] assert f != null;
> assert f != null;
>
> t/A.java [ 4] }
> }
>
> t/A.java [ 5] }
> }
>
>
> test test8Varargs FAILED with compare mode = IGNORE_WHITESPACE_DIFF
> t/F.java expected
> + actual
>
> t/F.java [ 0] /*
> /*
>
> t/F.java [ 1] * Refactoring License
> * Refactoring License
>
> t/F.java [ 2] */
> */
>
> t/F.java [ 3] package t;
> package t;
>
> t/F.java [ 4] /**
> /**
>
> t/F.java [ 5] *
> *
>
> t/F.java [ 6] * @author junit
> * @author junit
>
> t/F.java [ 7] */
> */
>
> t/F.java [ 8] record F<P>(P first, String... second) {
> | record F<P>(P first, String[] second) {
>
> t/F.java [ 9] public F {
> | public F(P first, String... second) {
>
> t/F.java [ 10] assert null != first;
> assert null != first;
>
> t/F.java [ 11] assert null != second && second.length > 0;
> assert null != second && second.length > 0;
>
> t/F.java [ 12] }
> }
>
> t/F.java [ 13] }
> }
>
>
> Some issues might be resolved by detecting the actual record parameters.
> A suitable constructor is always in the AST, either generated by the
> compiler with name '<init>' of by the source code. This constructor shows
> the varargs as intended, and is
> in my opinion the correct candidate to pick up the varargs. I have given it
> a try with a class called RecordUtils, but the resulting parameter list
> somehow interferes with what CasualDiff expects
> Anyway, it messes things up.
>
> The implements clause causes another issue:
> test test8RecordImplements1 PASSED with compare mode =
> IGNORE_WHITESPACE_DIFF
> t/A.java expected
> + actual
>
> t/A.java [ 0] package t;
> package t;
>
> t/A.java [ 1] import java.time.LocalDate;
> import java.time.LocalDate;
>
> t/A.java [ 2] import java.io.Serializable;
> import java.io.Serializable;
>
> t/A.java [ 3] public class A implements Cloneable, Serializable {
> public class A implements Cloneable, Serializable {
>
> t/A.java [ 4] static F f;
> static F f;
>
> t/A.java [ 5] }
> }
>
>
> test test8RecordImplements1 FAILED with compare mode =
> IGNORE_WHITESPACE_DIFF
> t/F.java expected
> + actual
>
> t/F.java [ 0] /*
> /*
>
> t/F.java [ 1] * Refactoring License
> * Refactoring License
>
> t/F.java [ 2] */
> */
>
> t/F.java [ 3] package t;
> package t;
>
> t/F.java [ 4] import java.io.Serializable;
> import java.io.Serializable;
>
> t/F.java [ 5] /**
> /**
>
> t/F.java [ 6] *
> *
>
> t/F.java [ 7] * @author hom
> * @author hom
>
> t/F.java [ 8] */
> */
>
> t/F.java [ 9] public record F(int x, int y) implements Cloneable,
> Serializable { | public record F(int x, int y) { implements
> Cloneable, Serializable
> t/F.java [ 10] /** I should be back. */
> /** I should be back. */
>
> t/F.java [ 11] static String code = "nix";
> static String code = "nix";
>
> t/F.java [ 12] }
> }
>
>
> It is particularly annoying that the 'implements' in the original text for
> the class A is transferred properly. I assume that this
> is realised by the text copying part in CasualDif. However, the newly
> generated class 'F' is wrong, and it is not clear to me who is responsible
> for
> the generation here. I was not able to fix this in CasualDiff without
> breaking the formatting for either A of F in this case, and such changes
> in CasualDiff might affect the expectations of other transformers (and did,
> by my experience with the PullUp transformer).
>
> I have seen recent changes in CasualDiff, in this case about permits
> clauses (which are not relevant for records, as the can't be extended).
> So could someone please shed some light on the interaction of CasualDiff
> and the other collaborators.
>
> Anyone that is willing to help, I have a fork with a branch at
> https://github.com/homberghp/netbeans/tree/issue7044b
> This is synced with the main branch on sunday April 13.
>
> Thanks in advance.
>
> Met vriendelijke groet,
> Pieter van den Hombergh.
> Kerboschstraat 12
> 5913 WH Venlo
>