SONARJAVA-5984 Support Module Import Declarations#5403
SONARJAVA-5984 Support Module Import Declarations#5403tomasz-tylenda-sonarsource merged 6 commits intomasterfrom
Conversation
tomasz-tylenda-sonarsource
force-pushed
the
tt/import-module
branch
from
0f36ce4 to
12dca07
Compare
tomasz-tylenda-sonarsource
force-pushed
the
tt/import-module
branch
from
c67641c to
ecd5c5c
Compare
tomasz-tylenda-sonarsource
requested review from
dorian-burihabwa-sonarsource and
rombirli
dorian-burihabwa-sonarsource
left a comment
dorian-burihabwa-sonarsource
left a comment
There was a problem hiding this comment.
Looking good, I just have a couple of suggestions
java-frontend/src/main/java/org/sonar/plugins/java/api/tree/ImportTree.java
Show resolved
Hide resolved
| } | ||
|
|
||
| @Override | ||
| public List<Tree> children() { |
There was a problem hiding this comment.
Should we add the module token to the list of children?
There was a problem hiding this comment.
Done.
| boolean isModuleImport = org.eclipse.jdt.core.dom.Modifier.isModule(e2.getModifiers()); | ||
|
|
||
| // "on demand" means `import pkg.*;` | ||
| if (e2.isOnDemand() && !isModuleImport) { |
There was a problem hiding this comment.
Is it possible to mix an on-demand import and a module import? I am not sure the JLS allows this.
There was a problem hiding this comment.
I believe this is not allowed: https://docs.oracle.com/javase/specs/jls/se25/html/jls-7.html#jls-7.5 On demend imports exist only in the context of type and static imports.
There was a problem hiding this comment.
Ok, because we have coverage, I will not question this one too much but it looks little weird to me
…portTree.java Co-authored-by: Dorian Burihabwa <75226315+dorian-burihabwa-sonarsource@users.noreply.github.com>
| ); | ||
| t.binding = e2.resolveBinding(); | ||
|
|
||
| if (!isModuleImport) { |
There was a problem hiding this comment.
Why do we want to skip resolution of IModuleBinding when it is a module ? Asking it because we don't skip resolution of IPackageBinding when isOnDemand && !isModuleImport
There was a problem hiding this comment.
My understanding is that it does not make sense for a module. In fact, when the condition is removed, we are getting type cast exception in jdt.
tomasz-tylenda-sonarsource
requested a review
from dorian-burihabwa-sonarsource
| boolean isModuleImport = org.eclipse.jdt.core.dom.Modifier.isModule(e2.getModifiers()); | ||
|
|
||
| // "on demand" means `import pkg.*;` | ||
| if (e2.isOnDemand() && !isModuleImport) { |
There was a problem hiding this comment.
Ok, because we have coverage, I will not question this one too much but it looks little weird to me
|
3 participants
No description provided.