#407 Antlr changes from Ticket #403 break extension plugins

[Simon Geard]

Just stumbled upon the ticket 403 changes after running into difficulties updating an in-house Checkstyle extension to the latest versions. 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?

Essentially, my concern here is that the Antlr API is part of the Checkstyle extension API - it's impossible to do anything substantial with the Checkstyle-provided DetailAST class without calling methods in the Antlr-provided superclasses. Removing these classes from the declared API simply breaks existing code.

(I originally added this as a comment to #403, but realised that with it being closed, nobody was likely to see my comments).

[Simon Geard]

I don't know much about OSGI bundling, but perhaps a fix would be for Checkstyle itself to override these methods in DetailAST, exposing them as part of its own API instead of just being inherited from Antlr?

[Michael Vorburger]

FYI This has also been raised on https://github.com/sevntu-checkstyle/sevntu.checkstyle/issues/460

I'll try to spend some time on this hopefully next week, to see what a clean solution for this would have to look like (possibly combined with https://github.com/checkstyle/checkstyle/issues/3169). If I don't manage, or don't have time, or get run over by a bus, then perhaps https://sourceforge.net/p/eclipse-cs/bugs/403/ should be reverted .. but if you give me a like 1-2 weeks, I'll try to find a better solution.

[Simon Geard]

I think that if the problem is that exposing the Antlr API is harmful for the Eclipse plugin, then 403 may be a good fix - but the problem then is that Checkstyle exposes the Antlr API as part of its own.

Ideally, Checkstyle would wrap the Antlr stuff internally, instead of subclassing it - but as I suggested, it may be sufficient as a quick-fix if Checkstyle were to simply override the important methods from the superclass, so that the Antlr methods aren't called directly by extension classes.

[Lars Koedderitzsch]

Meanwhile I've reverted changes from #403 and released fix version 6.19.1.

If a more complete fix to the original problem becomes available I will happily apply it to one of the next releases.

[Simon Geard]

Ok, I've just updated to 6.19.1, and my extension plugin is building and working, so thanks for that... I can now roll Neon out to the rest of the development team.

Just as an example of the kind of thing I'm doing, btw - I have a Check which looks for unit tests with the @Ignore annotation, and if found, extracts the value of the annotation (i.e. the reason the test is disabled) and includes it in the warning message. I've got others that do things with naming conventions... all of which need to be able to call getText() to find the name of a method, or the value of a string, etc.

Using getType() is a little less common, but the most common case is stuff like running a check that looks at anything class-like (classes, interfaces, enums), but has small differences in how they're treated.