eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
164 stars 130 forks source link

[23] DOM support for JEP 476: Module Import Declarations (Preview) #2834

Closed stephan-herrmann closed 2 months ago

stephan-herrmann commented 2 months ago

The tiny feature https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1589 has a tiny follow-up https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1589 where all the sudden it seems we need to extend the DOM AST:

In a regular compilation unit module is not a keyword and hence it would be suitable to apply semantic highlighting (category restricted identifiers). But then we'd need an ASTNode that informs us about the position of the modifier.

It's a real pity that introduction of import static was answered just with a boolean flag in ImportDeclaration. At this point we'd be better off with a list of Modifier.

I have a quick-n-dirty draft of such addition, but I'd like to discuss this approach first, because now it looks a bit odd to have isStatic() and modifiers(). Should the latter answer a Modifier also for static?? Should we migrate the internal storage to list of Modifier and only map isStatic() and setStatic() to corresponding acrobatics? This looks similar to how Modifier lists were introduced for JLS3 in the first place.

@jarthana @mpalat wdyt?

stephan-herrmann commented 2 months ago

If module imports were a strongly debated feature I'd say let's wait until it leaves preview state, but specifically with its connection to implicitly declared classes I guess it's safe to assume that this feature will stay.

jarthana commented 2 months ago

I have a quick-n-dirty draft of such addition, but I'd like to discuss this approach first, because now it looks a bit odd to have isStatic() and modifiers(). Should the latter answer a Modifier also for static?? Should we migrate the internal storage to list of Modifier and only map isStatic() and setStatic() to corresponding acrobatics? This looks similar to how Modifier lists were introduced for JLS3 in the first place.

Yes, we can may be have a general modifiers() and perhaps a retirement plan for the isStatic() and family. It will be redundant for a while, but definitely future-proof.