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

[sealed-classes] Incorrect unused import warning #1808

Open
snjeza opened this issue Jan 3, 2024 · 5 comments · May be fixed by #1809
Open

[sealed-classes] Incorrect unused import warning #1808

snjeza opened this issue Jan 3, 2024 · 5 comments · May be fixed by #1809
Milestone

Comments

@snjeza
Copy link
Contributor

snjeza commented Jan 3, 2024

The original issue: redhat-developer/vscode-java#3446
Steps to reproduce:

  • open the class
package foo;
import foo.X.B;
public sealed interface X permits B { 
    record B(int data) implements X {}
}

The JDT compiler reports

The import foo.X.B is never used
snjeza added a commit to snjeza/eclipse.jdt.core that referenced this issue Jan 3, 2024
Fixes eclipse-jdt#1808
The issue happens for imports used in the permits clause when that import is
- declared as a nested class
- is used only in a permits clause
The fix checks if member types are imported and marks the import as used.
snjeza added a commit to snjeza/eclipse.jdt.core that referenced this issue Jan 3, 2024
Fixes eclipse-jdt#1808
The issue happens for imports used in the permits clause when that import is
- declared as a nested class
- is used only in a permits clause
The fix checks if member types are imported and marks the import as used.

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
@srikanth-sankaran srikanth-sankaran changed the title Incorrect unused import warning [sealed-classes] Incorrect unused import warning Jan 31, 2024
@srikanth-sankaran
Copy link
Contributor

The unused import warning is a red-herring.

Real problem is this code should not compile but does:

public sealed interface X permits B { 
    record B(int data)  {}
}

@srikanth-sankaran srikanth-sankaran self-assigned this Jan 31, 2024
akurtakov pushed a commit to snjeza/eclipse.jdt.core that referenced this issue Feb 9, 2024
Fixes eclipse-jdt#1808
The issue happens for imports used in the permits clause when that import is
- declared as a nested class
- is used only in a permits clause
The fix checks if member types are imported and marks the import as used.

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
snjeza added a commit to snjeza/eclipse.jdt.core that referenced this issue Feb 15, 2024
Fixes eclipse-jdt#1808
The issue happens for imports used in the permits clause when that import is
- declared as a nested class
- is used only in a permits clause
The fix checks if member types are imported and marks the import as used.

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
@snjeza
Copy link
Contributor Author

snjeza commented Feb 15, 2024

@srikanth-sankaran @jukzi @akurtakov I have updated #1809

snjeza added a commit to snjeza/eclipse.jdt.core that referenced this issue Feb 15, 2024
Fixes eclipse-jdt#1808
The issue happens for imports used in the permits clause when that import is
- declared as a nested class
- is used only in a permits clause
The fix checks if member types are imported and marks the import as used.

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
snjeza added a commit to snjeza/eclipse.jdt.core that referenced this issue Feb 15, 2024
Fixes eclipse-jdt#1808
The issue happens for imports used in the permits clause when that import is
- declared as a nested class
- is used only in a permits clause
The fix checks if member types are imported and marks the import as used.

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
snjeza added a commit to snjeza/eclipse.jdt.core that referenced this issue Feb 15, 2024
Fixes eclipse-jdt#1808
The issue happens for imports used in the permits clause when that import is
- declared as a nested class
- is used only in a permits clause
The fix checks if member types are imported and marks the import as used.

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
@rgrunber rgrunber added this to the 4.32 milestone Mar 12, 2024
@rgrunber rgrunber assigned snjeza and unassigned srikanth-sankaran Mar 13, 2024
@srikanth-sankaran
Copy link
Contributor

Please check with the original owner before reassigning. The proposed PR looks too much of a point fix - if ( !(typeReference instanceof QualifiedTypeReference) && permittedType != null && permittedType.isMemberType() && permittedType.enclosingType() != null && permittedType.enclosingType().isInterface()) {

Is this fix required in a hurry ? I am planning a comprehensive review and rewrite of sealed classes support in the 4.32 M3 time frame. Basically the permitted classes clauses are resolved in the wrong phase.

My comment above repeated here:

The unused import warning is a red-herring.

Real problem is this code should not compile but does:

public sealed interface X permits B { 
    record B(int data)  {}
}

hold a clue to the solution.

@snjeza
Copy link
Contributor Author

snjeza commented Mar 22, 2024

Real problem is this code should not compile but does:

It doesn't compile with the latest PR

with the PR
sealed

without the PR
sealedwithout

I am planning a comprehensive review and rewrite of sealed classes support in the 4.32 M3 time frame. Basically the permitted classes clauses are resolved in the wrong phase.

+1

@srikanth-sankaran
Copy link
Contributor

Added several review comments to the PR - If this is not super urgent, please feel free to assign it back to me. Thanks!

@snjeza snjeza removed their assignment Mar 22, 2024
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 a pull request may close this issue.

3 participants