INRIA / INRIA/spoon

[Bug] Cloning of CtUnresolvedImport does not work

Open
#4,626 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
2k
Forks
392
Avg merge
11h 24m
Merged PRs (30d)
36

Description

**Describe the bug**
When cloning a compilation unit `CtUnresolvedImport` is turned into a `CtImportImpl` with no reference set. The unresolved reference is lost in translation.

**To Reproduce**
I found that while investigating issue #4593. The issue contains reproduction steps.

**The problem**
When changing the class name, the compilation unit is cloned in https://github.com/INRIA/spoon/blob/8397c32d7aa3f5389a21148b506e1b2c6c5938c6/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java#L2149

This clone ends up in the clone method for `CtImport`:
```java
// auto-generated, see spoon.generating.CloneVisitorGenerator
@java.lang.Override
public void visitCtImport(final spoon.reflect.declaration.CtImport ctImport) {
spoon.reflect.declaration.CtImport aCtImport = ctImport.getFactory().Core().createImport();
this.builder.copy(ctImport, aCtImport);
aCtImport.setReference(this.cloneHelper.clone(ctImport.getReference()));
aCtImport.setAnnotations(this.cloneHelper.clone(ctImport.getAnnotations()));
aCtImport.setComments(this.cloneHelper.clone(ctImport.getComments()));
this.cloneHelper.tailor(ctImport, aCtImport);
this.other = aCtImport;
}
```
As you can see, this always creates a new `CtImportImpl`, but never a `CtUnresolvedImport`. It also never copies the `String unresolvedReference` of the unresolved import. This is bad.

**Possible solutions**
1. Make the CtUnresolvedImport a metamodel element. If we do this we need to rename the class and introduce an interface for it.
2. Special-case the visitCtImport method in the clone visitor generator and hard code an `instanceof` distinction in there. This doesn't really sound appealing.

The first option sounds cleaner but is breaking (and maybe complicates the existence of the CtImportVisitor a bit?). Imports are currently in the `experimental` package, so maybe this is a worthwhile change.

**Moving forward**
I could probably implement both solutions but I'd like some input from the integrators to decide which option to proceed with.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.