Skip to content

fixed class modifier auto completion for sealed classes. - #3228

Merged
jlahoda merged 2 commits into
apache:masterfrom
mbien:contains
Dec 1, 2021
Merged

fixed class modifier auto completion for sealed classes.#3228
jlahoda merged 2 commits into
apache:masterfrom
mbien:contains

Conversation

@mbien

@mbien mbien commented Oct 9, 2021

Copy link
Copy Markdown
Member

minor bugfix:
contains(String) was called on a Set containing Modifier enums - which is always false.

Although we can't use the two sealed enums directly since they are JDK 17+, they can be still queried via a little utility method.

Added auto completion tests for sealed class declaration.

@neilcsmith-net neilcsmith-net added this to the 12.6 milestone Oct 13, 2021
@neilcsmith-net neilcsmith-net modified the milestones: 12.6, NB13 Oct 22, 2021
@mbien mbien changed the title fixed class modifier auto completion when sealed classes are involved. fixed class modifier auto completion for sealed classes. Nov 24, 2021
@mbien
mbien force-pushed the contains branch 2 times, most recently from 44d088d to 1cd9833 Compare November 28, 2021 01:13
@mbien

mbien commented Nov 28, 2021

Copy link
Copy Markdown
Member Author

@Akshay-Gupta-Oracle @matthiasblaesing @jlahoda anyone interested to review this?

Had time to add some tests now. i give this a 75% chance that I did put the golden files into the wrong folder (again).

@matthiasblaesing matthiasblaesing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a single inline comment - apart from that, this looks sane to me. It would be great if the other reviewers could also have a look. I'll restart the tests.

Comment thread harness/nbjunit/src/org/netbeans/junit/NbTestCase.java

@matthiasblaesing matthiasblaesing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran tests locally and worked as expected. @mbien if in one week no second review comes in, please ping me and I'll merge.

@mbien

mbien commented Nov 30, 2021

Copy link
Copy Markdown
Member Author

@matthiasblaesing thanks. will do. Would be nice if @jlahoda could take a look. Because i have the suspicion that the golden files still have to be in the 1.8 folder but i am not 100% certain. It depends on whether NB 13 having JDK 17 compatible nb-javac installed by default or not I guess.

@jlahoda

jlahoda commented Nov 30, 2021

Copy link
Copy Markdown
Contributor

@matthiasblaesing thanks. will do. Would be nice if @jlahoda could take a look. Because i have the suspicion that the golden files still have to be in the 1.8 folder but i am not 100% certain. It depends on whether NB 13 having JDK 17 compatible nb-javac installed by default or not I guess.

Yes, I think it would be preferrable to have the in the 1.8 folder. The folders represent the runtime platform, and are unrelated to the source level supported by the Java parser. (If the selected Java parser does not support the source level, the test will be skipped anyway, but I think we are heading to a place where this won't happen normally.) I realize this is fairly confusing.

Otherwise looks good!

Thanks!

@mbien

mbien commented Nov 30, 2021

Copy link
Copy Markdown
Member Author

@jlahoda thanks jan! Moved them to the 1.8 folder.

@jlahoda

jlahoda commented Dec 1, 2021

Copy link
Copy Markdown
Contributor

Looks good.

@jlahoda
jlahoda merged commit 3649573 into apache:master Dec 1, 2021
@junichi11 junichi11 added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Dec 1, 2021
mbien added a commit to mbien/netbeans that referenced this pull request Jan 2, 2022
mbien added a commit to mbien/netbeans that referenced this pull request Jan 3, 2022
mbien added a commit to mbien/netbeans that referenced this pull request Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants