eclipse-jdtls / eclipse-jdt-core-incubator

Eclipse Public License 2.0
8 stars 1 forks source link

Dubious 'unused' diagnostics disappear when opening files #763

Closed fbricon closed 6 days ago

fbricon commented 2 weeks ago

There are a couple issues with the 'unused' code detection:

https://github.com/user-attachments/assets/373ff4ba-c3e7-41a8-ae31-3b86b5cd23d2

Tested against https://github.com/eclipse/lemminx

fbricon commented 2 weeks ago

Also private constructors should not be flagged as unused

testforstephen commented 2 weeks ago

Initially you see the unused diagnostics in the problems view, they're reported by the build job. When you open the Java file, it disappears because the DOM parser does not enable the unused checker and cleanup the diagnostics of current file.

https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/blob/343016820349fce63f9ff5095c905ded7d646500/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacCompilationUnitResolver.java#L498-L505

I hooked the unused checker into the DOM parser by listening at the ANALYZE task. It looks like the ANALYZE task is never invoked by DOM parser. This should work before, no idea why it's not called in latest code.

@mickaelistria any thoughts?

mickaelistria commented 2 weeks ago

At the moment, the analyze is called while resolving bindings. So when bindings are disabled, analyze() is never run. We should probably also consider FORCE_PROBLEM_DETECTION flag to run that analyze code.

mickaelistria commented 2 weeks ago

https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/pull/771 might help. @testforstephen Can you please check?

testforstephen commented 2 weeks ago

After debugging the code, it's because the analyze in DOM parser stops on error and return earlier. In the build job, we have overwritten the compiler to force it to proceed on error. But for the DOM parser, we don't do that and it will return earlier if the errorCount() of JavaCompiler > 0.

fbricon commented 2 weeks ago

Also private static final long serialVersionUID fields on Serializable classes should be ignored

Screenshot 2024-08-29 at 11 31 43
testforstephen commented 6 days ago

this should be fixed by recent commits https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/commit/cb3c54ff805b938febe42cabb52e760df1614b29, https://github.com/eclipse-jdtls/eclipse-jdt-core-incubator/commit/5cadbcc0c5a3cfcf628141f29e686873990b52d2