Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
stephan-herrmann opened this issue Aug 17, 2024 · 2 comments
Closed
Milestone

Comments

@stephan-herrmann
Copy link
Contributor

The tiny feature eclipse-jdt/eclipse.jdt.ui#1589 has a tiny follow-up eclipse-jdt/eclipse.jdt.ui#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 stephan-herrmann added this to the BETA_JAVA23 milestone Aug 17, 2024
@stephan-herrmann
Copy link
Contributor Author

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
Copy link
Member

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.

stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Aug 19, 2024
+ parser to record modifier start position of imports
+ avoid code duplication in SourceElementParser
+ new list ImportDeclaration.modifiers()
  + at 23 even 'static' is represented in that list
+ existing accessors may scan that list if present

fixes eclipse-jdt#2834
stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Aug 19, 2024
+ parser to record modifier start position of imports
+ avoid code duplication in SourceElementParser
+ new list ImportDeclaration.modifiers()
  + at 23 even 'static' is represented in that list
+ existing accessors may scan that list if present

fixes eclipse-jdt#2834
stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Aug 20, 2024
+ adaptations in other parts of DOM implementation
+ consistently use Modifier even for static at JLS23
+ enable DOM testing at JLS23

fixes eclipse-jdt#2834
@stephan-herrmann stephan-herrmann changed the title [23] DOM support for JEP 476: Module Import Declarations (Preview)? [23] DOM support for JEP 476: Module Import Declarations (Preview) Aug 20, 2024
stephan-herrmann added a commit that referenced this issue Aug 20, 2024
…2836)

+ parser to record modifier start position of imports
+ avoid code duplication in SourceElementParser
+ new list ImportDeclaration.modifiers()
  + at 23 even 'static' is represented in that list
+ existing accessors may scan that list if present
+ adaptations in other parts of DOM implementation
+ consistently use Modifier even for static at JLS23
+ enable DOM testing at JLS23

fixes #2834
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants