Skip to content

Conversation

@mbien
Copy link
Member

@mbien mbien commented Oct 17, 2025

fixes #8928

@mbien mbien added this to the NB28 milestone Oct 17, 2025
@mbien mbien requested review from dbalek and lahodaj October 17, 2025 02:02
@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) hints ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Oct 17, 2025
@mbien mbien linked an issue Oct 17, 2025 that may be closed by this pull request
Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

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

Looks good. The only enhancement I could see is adding a test.

@mbien mbien force-pushed the missing-record-label_delivery branch from e6d7c57 to ccfbd96 Compare October 18, 2025 04:59
@mbien
Copy link
Member Author

mbien commented Oct 19, 2025

@jlahoda added tests for the hint.

There is one curiosity though: initially I wanted to add test cases to the UnusedDetectorTest, since it would be the most direct way to validate the functionality.

e.g:

    @Test
    public void testNoUnusedPackagePrivateClass() throws Exception {
        performTest(
                "test/Test.java",
                """
                package test;
                class Test {
                }
                """
        );
    }

However, it would fail since returns package private top level classes/interfaces/records.. etc as unused.

Testcase: testUnusedPackagePrivateClass(org.netbeans.modules.java.editor.base.semantic.UnusedDetectorTest):	FAILED
expected:<[]> but was:<[2:<init>:NOT_USED]>

I debugged through it and it correctly marks the class declaration as used, but later sees an synthetic constructor and marks that as unused. I believe what is happening afterwards is that since synthetic items have no line numbers, it maps the unused line to the class declaration so that it looks as if it would think that the class should be marked as unused. (constructor name == class name too so the error line becomes indistinguishable)

So why does the hint work?
the hint then drops the item at

if (ud.unusedElementPath().getLeaf() != ctx.getPath().getLeaf()) {

That is why the junit test works as expected for the hint, while the UnusedDetector produces technically wrong output in that case.

However, changing this is not required to resolve this issue and it would be unnecessarily risky for the release. We should probably fix this at some point though.

I think this can get in as is (+ the added hint tests)

@ebarboni ebarboni merged commit 67232c7 into apache:delivery Oct 23, 2025
69 of 70 checks passed
@lahodaj
Copy link
Contributor

lahodaj commented Oct 23, 2025

FWIW, retroactivelly (sorry), still looks good.
Re the synthetic problem - we might look into solving that (basically, no compute the unused status for synthetic declarations), but the tests ought to be OK for now, I think.

Thanks!

@mbien
Copy link
Member Author

mbien commented Oct 23, 2025

@lahodaj yep, would be good to properly fix UnusedDetector, but that would be something for NB 29 I think.

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) hints 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.

Package-private record class is shown as unused

3 participants