Stackoverflow fix in ImageDescriptor

[akurtakov]

Fixes getImageData(100) calling itself endlessly (introduced via c2fe144 ) by reintroducing the bigger getImageData(100) and getImageData() endlessly (introduced in 2017 via
e6d12ca ) that wasn't experienced thanks to the happy accident that it was asbtract method and all implementations before that had their own and every implementor since then most likely figured the endless loop while developing.
Enhanced documentation to mention that.

[iloveeclipse]

@akurtakov, do I miss something? PR description says about StackOverflow fix, but code change shows only javadoc comment changed?

[merks]

FYI, the installer was broken from stack overflow in org.eclipse.emf.edit.ui.provider.ImageWrapperImageDescriptor

[akurtakov]

@akurtakov, do I miss something? PR description says about StackOverflow fix, but code change shows only javadoc comment changed?

I forgot to push that part initially. Should be good now.

[merks]

FYI, this was the stack overflow

[iloveeclipse]

I forgot to push that part initially. Should be good now.

Good, but if that was automated cleanup that redirects from deprecated to non deprecated methods, wouldn't that hit again on next cleanup round?

[merks]

I forgot to push that part initially. Should be good now.

Good, but if that was automated cleanup that redirects from deprecated to non deprecated methods, wouldn't that hit again on next cleanup round?

Yes, I had the same question. But apparently the cleanup considers the Javadoc to determine the replacement call and @akurtakov tested that the changes to the Javadoc prevents the save action (automated by the cleanup bot) from making that change again. We should keep and eye on this nevertheless!

[akurtakov]

Save action runs that changes deprecated calls relies on deprecated javadoc tag content (https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/210f65acc02641c0e26a081aa88ca119c4c2b2ca/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessorUtil.java#L591 ) which explicitly kicks in only for "use" and "replace by" thus I changed the javadoc to "replace with" for the sake of differing.

[iloveeclipse]

thus I changed the javadoc to "replace with" for the sake of differing.

Hmm...

Next time someone will change javadoc & face same issue again.

@jjohnstn : would it be possible to exclude automated replacement of deprecated methods via some kind of "do_not_fix_me" tag on the code that uses deprecated method?

[github-actions]

Test Results

 2 904 files  ±0   2 904 suites  ±0   1h 59m 40s ⏱️ - 4m 9s
 8 019 tests ±0   7 774 ✅ +1  245 💤 ±0  0 ❌  - 1 
23 591 runs  ±0  22 809 ✅ +1  782 💤 ±0  0 ❌  - 1 

Results for commit 40924de. ± Comparison against base commit 4f687d6.

[laeubi]

@akurtakov javaodc of the class says:

Using this abstract class involves defining a concrete subclass
and re-implementing the {@link #getImageData(int)} method.
Legacy subclasses that are not HiDPI-aware used to override the
deprecated {@link #getImageData()} method. Subclasses must
re-implement exactly one of the {@code getImageData} methods
and they should not re-implement both.

So does this javadoc needs fixing?

[akurtakov]

IMO one could drop the note about reimplement exactly one but I haven't given it enough thought. I'm merging this one to prevent the regression and starting new I-build. Documentation enhancement can happen in follow up PRs.

[akurtakov]

Ongoing build is https://ci.eclipse.org/releng/job/Builds/job/I-build-4.38/40/ .

[merks]

Thank you. ❣️