#403 ANTLR should not be exported (leak) but kept internal

[Michael Vorburger]

Hi ho,

I've tracked down an issue with have had with ANOTHER (not Eclipse Checkstyle) Eclipse plugin that manifest itself as a "java.lang.LinkageError: org/antlr/v4/runtime/TokenStream" to have something to probably have something to do with the Eclipse Checkstyle, and while I've managed to solve the problem in the code of the other Eclipse plugin, so am not blocked by this, I just wanted to let you know that I do think something is perhaps not done quite right in Eclipse Checkstyle with regards to how external JAR "leak" out:

See https://bugs.opendaylight.org/show_bug.cgi?id=5798 for full background. While I have NOT looked into the source code of your eclipse-cs plugin, I notice that e.g. /home/vorburger/.p2/pool/plugins/net.sf.eclipsecs.checkstyle_6.16.0.201603042321/checkstyle-6.16.1-all.jar includes a /org/antlr/v4/runtime/ in "fat JAR" style. I'm assuming, without having checked this out in details, that eclipse-cs exposes the packages in net.sf.eclipsecs.checkstyle? That would then cause other Eclipse plugins which use Import-Package: org.antlr.v4.runtime to have version linkage errors related to JAR hell version mismatch.

Unless it's absolutely neccessary and completely impossible to avoid exposing ANTLR packages, it would thus be "safer" if eclipse-cs could treat that as a private internal implementation detail dependency, not export it.

I do agree that other Eclipse plugins should not do Import-Package: org.antlr.v4.runtime themselves, and have rewritten the other 3rd party plugin where this problem occured to not do that anymore, but perhaps you would still like to clean up eclipse-cs as well.

Best,
M. http://www.vorburger.ch

[Michael Vorburger]

Proposed fix in https://sourceforge.net/p/eclipse-cs/git/merge-requests/3/, to avoid exposing ANTLR. As far as I can see, e.g. the net.sf.eclipsecs.ui never needs to directly access ANTLR. I cannot image any custom Checkstyle checks Eclipse plugins direclty requiring ANTLR access either.

FYI I've actually hit this kind of problem before, a few years ago, at the time it was in connection with a (non open source, in-house) IDE which used Xtext plugins which also uses ANTLR - something similar got mixed up there as well at the time, thus my motivation to propose a root cause fix for this.

PS, for a possible longer term future, FYI https://github.com/checkstyle/checkstyle/issues/3169.

[Lars Koedderitzsch]

Thanks Michael, PR is merged and will ship with 6.18.0.

[Simon Geard]

Just stumbled upon this change after running into difficulties updating an in-house Checkstyle extension to the latest versions. Essentially, my problem is that Eclipse reports the following error whenever I call the DetailAST.getText() or getType() methods:

Access restriction: The method 'CommonAST.getText()' is not API (restriction on required library '/opt/local/eclipse-neon/plugins/net.sf.eclipsecs.checkstyle_6.19.0.201606092149/checkstyle-6.19-all.jar')

And this makes sense, because while the DetailAST class itself is provided by Checkstyle, that class extends antlr.CommonAST, which provides several useful methods. In particular, getText() is used to get the names of variables and methods, while getType() tells me what kind of node I'm looking at (e.g a CLASS_DEF vs INTERFACE_DEF).

So, how do you suggest dealing with this? Is there an alternative to using these Antlr-provided functions?

[Simon Geard]

Essentially, my concern here is that Antlr forms part of the Checkstyle public API, because the primary extension point in that API - the "visitToken (DetailAST)" - takes an Antlr-derived class as a parameter. That API is now broken...

[Michael Vorburger]

Sorry about this! Follow up in https://sourceforge.net/p/eclipse-cs/bugs/407/ ...