bnd icon indicating copy to clipboard operation
bnd copied to clipboard

[quickfix] Inconsistent hierarchy not generating suggestions

Open kriegfrj opened this issue 9 months ago • 5 comments

One of the reasons I created the quick fix processor was to help in figuring out what my missing transitive dependencies were, as this was a laborious process.

It used to work well, but I have noticed recently that it doesn't seem to be working. I suspect there must have been a change in JDT that we haven't caught up with which is causing it to fail now.

Testing and debugging this could be painful as it might require upgrading the version of Eclipse we have in the dev workspace. We do have some test cases that could help (though they are disabled in CI because of persistent flakiness), and perhaps (on a related note) we could even consider having two bndrun files for these tests (one for the baseline version of Eclipse and one for the latest version).

kriegfrj avatar Mar 04 '25 01:03 kriegfrj

Which quick fix processors exactly don't work anymore? And which Eclipse are you using?

pkriens avatar Mar 14 '25 16:03 pkriens

Which quick fix processors exactly don't work anymore?

The Java quickfix that produces "add bundle" suggestions.

It still works generally, but there are specific cases that no longer provide suggestions of the "inconsistent hierarchy" variety; eg:

  1. C (in bundle Bc) extends B (in bundle Bb) extends A (in bundle Ba);
  2. Ba and Bc are on the classpath but Bb is not;
  3. Code like this:
C c = new C();
A a = c; // fails with "hierarchy inconsistent" or something similar

This fails to compile as the compiler is unable to determine that C is a subclass of A because it doesn't have B on the classpath (and hence cannot determine that B is a subclass of A). The quick fix processor is supposed to suggest adding Bb to the bundle classpath as the suggested fix. I have seen more and more cases where this hasn't worked.

These types of errors were one of my strongest motivators for writing the quick fix in the first place, as they are sometimes difficult to figure out and laborious to fix.

There are several test cases that are meant to test for situations like these:

https://github.com/bndtools/bnd/blob/eab701184c3f07ff153668d93aa8ab878a9afd81/bndtools.core.test/src/bndtools/core/test/editors/quickfix/BuildpathQuickFixProcessor_WithSimpleOnBuildpath_Test.java#L81-L129

They all still seem to pass on HEAD, which runs them against 2022-09.

And which Eclipse are you using?

2024-03

kriegfrj avatar Mar 15 '25 13:03 kriegfrj

2024-03

Maybe it's to ask next door at the Eclipse JDT guys. They are dealing often with issues like this. Just searched for the word "hierarchy": https://github.com/eclipse-jdt/eclipse.jdt.core/pulls?q=is%3Apr+hierarchy+is%3Aclosed+

Especially @stephan-herrmann comes up often in this context.

Just my two cents.

chrisrueger avatar Apr 28 '25 19:04 chrisrueger

Maybe it's to ask next door at the Eclipse JDT guys.

Asking shouldn't hurt. :)

But then you need a question to which we (JDT) can relate. When asking about a behavioral change, you should first narrow it down the the exact release version that brought the change (and make sure the problem exists in latest, i.e., 2025-03). You should also mention which JDT API is used and how.

Just saying

stephan-herrmann avatar Apr 29 '25 16:04 stephan-herrmann

@kriegfrj I guess you are the only person with knowledge on this. Is it ok to close?

chrisrueger avatar Jun 06 '25 15:06 chrisrueger

Stumbled over https://github.com/eclipse-pde/eclipse.pde/pull/1926 and https://github.com/eclipse-jdt/eclipse.jdt.core/pull/4293 which are maybe of interest in this regard.

chrisrueger avatar Aug 10 '25 16:08 chrisrueger

Ok, this annoyed me one too many times so I looked into it. It is a very simple case with a very simple fix.

  • All JDT error codes live in IProblem.
  • The particular Eclipse error code that I was relying on was the code IsClassPathCorrect
  • As the comment against that older code suggests, there is a new error code IsClasspathCorrectWithReferencingType
  • I have confirmed that in the cases where the quickfixes are not suggested, it is because JDT is flagging them with this newer code rather than the older code.

The fix is very simple, and when I went to implement the test case I realised that the tests were already covering this case and failing as a result. Pleasingly, my two-line fix not only worked in manual testing but also caused the tests to pass.

PR to come.

kriegfrj avatar Sep 11 '25 02:09 kriegfrj

For posterity:

I realised that the tests were already covering this case and failing as a result.

I noticed when I looked through the issue history that the above was not the case when I first raised this issue. So I am guessing that the upgrade to Eclipse 2022-12 baseline is when the IsClassPathCorrectWithReferencingType was first introduced, which is why the tests were failing now (and also made the fix possible).

kriegfrj avatar Sep 11 '25 04:09 kriegfrj