search

apache / netbeans

Apache NetBeans
https://netbeans.apache.org/
Apache License 2.0
2.63k stars 840 forks source link

ForwardingJavaPlatform is causing trouble #6568

Open mbien opened 10 months ago

mbien commented 10 months ago

As discovered in https://github.com/apache/netbeans/pull/6213#discussion_r1359320621, the proxy class delegates a subset of the methods to the wrapped JavaPlatform which creates a frankenstein platform.

https://github.com/apache/netbeans/blob/a9aeefafc04e2d79a4702be88d890324365989bd/java/java.platform/src/org/netbeans/spi/java/platform/support/ForwardingJavaPlatform.java#L36

https://github.com/apache/netbeans/blob/a9aeefafc04e2d79a4702be88d890324365989bd/java/java.platform/src/org/netbeans/api/java/platform/JavaPlatform.java#L41 has final methods and was not designed to be extended / delegated in that way.

It is possible that it was actually the intention of that class to leave some capabilities out, however it is passed to places which expect a working JavaPlatform object, with non-empty properties etc.

Lets try to figure out what ForwardingJavaPlatform was supposed to achieve (hide the event system?) and then fix it if possible.

neilcsmith-net commented 10 months ago

Uses in the IDE seem minimal. Most relevant to the Javadoc issue appears to be the line at https://github.com/apache/netbeans/blob/master/java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/platformdefinition/J2SEPlatformImpl.java#L462

That looks like the intention is just to have a version of the platform without Javadoc folders to pass to queries?

I'm curious why the later usage at https://github.com/apache/netbeans/blob/master/java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/platformdefinition/J2SEPlatformImpl.java#L505 does the same though - intuition would suggest it should return empty source folders?

mbien commented 10 months ago

this commit added the proxy: https://github.com/emilianbold/netbeans-releases/commit/8cfabf5c583a1c9cbb68bf176fe8b5054d724a1e

neilcsmith-net commented 10 months ago

Thanks, doesn't say much, but looking at it I assume it was added for use here to protect against the possibility of recursion in the queries. In which case, the sources one definitely appears to be a bug!

mbien commented 8 months ago

moving to NB 22