-
-
Notifications
You must be signed in to change notification settings - Fork 375
review: feat: add support for module imports #6586
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
Changes from 8 commits
62413b2
c1fcde3
30de3f0
4a40527
d868da6
cde0f7c
2ea5a60
2d3e7d2
6f0cb06
c28fa3c
da794d3
c7a81eb
0b1d2df
f8f80ea
07ee4b2
66b3c5e
5051a29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,8 +264,11 @@ void onCompilationUnitProcessed(CtCompilationUnit compilationUnit) { | |
| ModelList<CtImport> existingImports = compilationUnit.getImports(); | ||
| Set<CtImport> computedImports = new HashSet<>(this.computedImports.values()); | ||
| topfor: for (CtImport oldImport : new ArrayList<>(existingImports)) { | ||
| if (oldImport.getImportKind() == CtImportKind.MODULE) { | ||
| //never remove module imports | ||
| continue; | ||
| } | ||
|
Comment on lines
267
to
270
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this is a conservative solution, i.e., removing such an import would be legal/might make sense in some cases (e.g., if we're in the same module?). In that case, I think the comment should reflect that a bit better, e.g., "be conservative: do not remove any module imports". This should help if anyone ever tries to do more here. |
||
| if (!removeImport(oldImport, computedImports)) { | ||
|
|
||
| // case: import is required in Javadoc | ||
| for (CtType type: compilationUnit.getDeclaredTypes()) { | ||
| for (CtJavaDoc element: type.getElements(new TypeFilter<>(CtJavaDoc.class))) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,15 +9,18 @@ | |
|
|
||
| import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration; | ||
|
||
| import org.eclipse.jdt.internal.compiler.ast.ImportReference; | ||
| import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; | ||
| import org.eclipse.jdt.internal.compiler.env.ICompilationUnit; | ||
|
|
||
| import spoon.reflect.cu.CompilationUnit; | ||
| import spoon.reflect.declaration.CtImport; | ||
| import spoon.reflect.declaration.CtMethod; | ||
| import spoon.reflect.declaration.CtModule; | ||
| import spoon.reflect.declaration.CtNamedElement; | ||
| import spoon.reflect.declaration.CtPackage; | ||
| import spoon.reflect.declaration.CtType; | ||
| import spoon.reflect.factory.Factory; | ||
| import spoon.reflect.reference.CtModuleReference; | ||
| import spoon.reflect.reference.CtReference; | ||
|
|
||
| import java.util.Arrays; | ||
|
|
@@ -73,6 +76,12 @@ void build() { | |
| } | ||
| } | ||
|
|
||
| } else if ((importRef.modifiers & ClassFileConstants.AccModule) != 0) { // this check is copied from JDT's ImportReference | ||
| // import of a module | ||
| String moduleName = JDTTreeBuilderHelper.createQualifiedTypeName(importRef.getImportName()); | ||
| CtModule ctModule = factory.Module().getOrCreate(moduleName); | ||
| CtModuleReference moduleRef = factory.Module().createReference(ctModule).setSimpleName(moduleName); | ||
| this.imports.add(createImportWithPosition(moduleRef, importRef)); | ||
| } else { | ||
| CtType klass = this.getOrLoadClass(importName); | ||
| if (klass != null) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.RetentionPolicy; | ||
| import java.lang.annotation.Target; | ||
|
|
||
| import org.assertj.core.api.Assertions; | ||
| import org.junit.jupiter.api.Nested; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtensionContext; | ||
|
|
@@ -39,6 +41,7 @@ | |
| import spoon.support.reflect.reference.CtArrayTypeReferenceImpl; | ||
| import spoon.test.SpoonTestHelpers; | ||
| import spoon.testing.assertions.SpoonAssertions; | ||
| import spoon.testing.utils.BySimpleName; | ||
| import spoon.testing.utils.GitHubIssue; | ||
| import spoon.testing.utils.ModelTest; | ||
|
|
||
|
|
@@ -474,4 +477,12 @@ void printAnnotationsInOrphanTypeReference(Factory factory) { | |
| private @interface TypeUseAnnotation { | ||
|
|
||
| } | ||
|
|
||
| @ModelTest(value = {"src/test/resources/imports/ModuleImport.java"}, autoImport = true, complianceLevel = 25) | ||
| void moduleImportsArePrinted(@BySimpleName("ModuleImport") CtType<?> type, Launcher launcher) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I already forgot about the |
||
| PrettyPrinter prettyPrinter = launcher.createPrettyPrinter(); | ||
| String output = prettyPrinter.prettyprint(type.getPosition().getCompilationUnit()); | ||
| Assertions.assertThat(output).contains("import module java.base"); | ||
| } | ||
|
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.