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

CodeGenerator should be able to generate basic code for records #7581

Merged

Conversation

mbien
Copy link
Member

@mbien mbien commented Jul 15, 2024

generates a final class as workaround instead of throwing an exception

fixes #7570 (see for reproducer)

So i tried to implement this properly at first, but there is some magic behind the scenes in the javac impl which kept generating final classes even though everything seemed to be correct. I had this in TreeFactory which works analog to Enum:

    public ClassTree Record(ModifiersTree modifiers, 
             CharSequence simpleName,
             List<? extends Tree> implementsClauses,
             List<? extends Tree> memberDecls) {
        long flags = getBitFlags(modifiers.getFlags()) | Flags.RECORD;
        return Class(flags, (com.sun.tools.javac.util.List<JCAnnotation>) modifiers.getAnnotations(), simpleName, Collections.<TypeParameterTree>emptyList(), null, implementsClauses, memberDecls);
    }

since this had no effect, I decided to change this back to a minimal impact quickfix which simply calls make.Class right away, without introducing public API changes. This would also have to be updated to extend AbstractElementVisitor14 as the todo indicates.

Edit: I might give this a second attempt to implement it properly

@mbien mbien added kind:bug Bug report or fix Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Jul 15, 2024
@mbien mbien added this to the NB23 milestone Jul 15, 2024
@mbien mbien requested review from dbalek, sdedic and lahodaj July 15, 2024 15:31
@ebarboni ebarboni modified the milestones: NB23, NB24 Jul 26, 2024
@dbalek
Copy link
Contributor

dbalek commented Jul 30, 2024

In addition to your changes to the TreeFactory, you have to IMHO change also the org.netbeans.modules.java.source.pretty.VeryPretty.visitClassDef(...) to generate records properly.

@mbien
Copy link
Member Author

mbien commented Jul 30, 2024

In addition to your changes to the TreeFactory, you have to IMHO change also the org.netbeans.modules.java.source.pretty.VeryPretty.visitClassDef(...) to generate records properly.

@dbalek thanks for the hint! I believe this was the missing piece why I could not figure out why it kept generating final classes. I will give this another attempt.

@mbien mbien marked this pull request as ready for review August 2, 2024 22:19
@mbien mbien modified the milestones: NB24, NB23 Aug 2, 2024
@mbien
Copy link
Member Author

mbien commented Aug 2, 2024

this would be a bit more work, due to the fact that VeryPretty would need access to the members while writing the record header, those are visited later however.

I think we could get this into NB 23 so that is generates the record as final class instead of throwing an exception. It can be still properly implemented later.

generates a final class as workaround instead of throwing an exception
@mbien mbien changed the base branch from master to delivery August 2, 2024 22:33
@mbien mbien force-pushed the quickfix-basic-codegen-record-support branch 2 times, most recently from 88d1ea4 to 3c0f522 Compare August 2, 2024 22:36
@ebarboni ebarboni merged commit aba81a5 into apache:delivery Aug 6, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:bug Bug report or fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

go-to action to records does not work for dependencies which don't have sources attached
3 participants