Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Solves #7044 #8156

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft

Solves #7044 #8156

wants to merge 27 commits into from

Conversation

homberghp
Copy link
Contributor

This pull request solves #7044.

The work helped me to improve my understanding of the refactoring API.

homberghp and others added 27 commits December 30, 2024 11:12
…ition

adds tests to show the error
adds inner and outer record in ui template text for setting java format
spaces
solves the error by moving spaces(cs.spaceWithinMethodDeclParens() ? 1 : 0, true);  to the correct place.

solves apache#7043 formatter to handle record closing braces correctly

squashed commits of test and solution.
Add expected values for passing test in InnerOuterRecordTest.
The existing javaformatting did not conform to any standard.
To make it possible to work with that file, default netbeans formatting
has been applied. That leads to some white space changes only, so be it.

Further added method assertLinesEqual to get better readable output when
comparing expected and acyla multi line string values, as in comparing
or diffing files.

Part of effort to find the problem described in apache#7044
…tion

the original comparison of expected and actual by collapsing all 
white space chars into one space is kept to keep original tests intact.
Refactoring of records is still broken as described in apache#7044.
These tests should help to find the error. The error might be something small 
as forgetting to consider a record as a class in the same way as enum.
Case labels for RECORD were missing in two places in WorkingCopy.java
Faster without scan and wait
New tests to separate out more issues.
Current recognized issues : Compact construtor in inner is not properly
moved. Record header of inner is not properly moved.

Current understanding is that code refactoring to source for
inner record is incomplete (Not considering RECORD and or RECORD_COMPONENT).
Inner record has compact constructor that does a meaningful validation.
Test runs without issues.
Rationale: Trying to find who mishandles the inner record as described
in apache#7044.
since Netbeans requires 17 to build and run, it is reasonable to expect
that in source levelutils too.

In RefactoringTestBase the repo is always reindexed, even when not
required.
As per documentation this is an expensive operation and taxes your
patience.

remove modifiers in variable member, so the do not show up

Modifiers (private final in particular) show up in record header,
making the resulting source code illegal and different from expected.
The removal is done by rewriting the member tree where variables occur.

make sure static fields of inner records survive field removal.

ensure static fields remain in inner record.

normal fields are modified by removing private final, but this should
not happen
to static fields.
This is part of making inner records work.

avoid use of raw types.

pass declared type further down.

Original code always passes down CLASS, now RECORD is passed down as
such. It should not make a difference for class, interface, or enum.

spelling

RECORD should be equivalent to class, interface and enum.

Add RECORD_COMPONENT as allowed element types

add isInnerRecord boolean field. Some reformatting.

For the reformatting: sorry. Stick to the formatting rules.

For the field: Record does not fit the standard class definition mold
because of the record header which has parenthesis before the first curly brace
 and the compact constructor that needs special attention.
Somehow the information that ithe inner tis a inner record must be forwardable to
the processors.

The author comment is modified by some part.

The @author line is substituted by some yet unknown processor,
so exclude it from the result comparison to make the tests less britle
because of dependence of the user property.

generateOuter sounds like a boolean, but it is the name of the new inner

match expected to actual after formatting.

This test passes, showing that the Compact constructor in the outer
survives the refactoring.

formatting in expected outer

cleanup. Permanently remove deprecated boolean cancelRequest

cancelRequest is not only deprecated but has no use at all and was
already replaced (function wide) by and atomicBoolean cancelRequested

This commit cleans up the tests, may add a print that needs removing.
For now: I am able to refactor inner record to outer but are still
lacking the understandig of how to modify a MethodTree or the equivalent
source code, and how to write a compact constructor instead.

remove deprecated boolean

no need to handle method special here.

make test helper static to make it usable in more places.
The refactoring of inner record to outer now works as expected.

The implementation tries to detect candidate 'compact constructor' even
if the original constructor declaration was not. I consider that
acceptable.
@mbien
Copy link
Member

mbien commented Jan 15, 2025

@homberghp hi! I forgot to mention it on the last PR you created #8106. It is much better to create a branch for each PR. This decouples master and any modifications to it.

@mbien mbien marked this pull request as draft January 15, 2025 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants