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

feat(CtUnresolvedImport): keep all imports in noClassPath mode #2936

Merged
merged 23 commits into from
May 16, 2019

Conversation

nharrand
Copy link
Collaborator

@nharrand nharrand commented Apr 8, 2019

Related to #2927 .

@nharrand nharrand changed the title feat(CtUnresolvedImport): add pure string import in noClassPath mode WIP: feat(CtUnresolvedImport): add pure string import in noClassPath mode Apr 8, 2019
@nharrand nharrand changed the title WIP: feat(CtUnresolvedImport): add pure string import in noClassPath mode feat(CtUnresolvedImport): add pure string import in noClassPath mode May 7, 2019
@nharrand
Copy link
Collaborator Author

nharrand commented May 7, 2019

Ready for reviews!

spoon-pom/pom.xml Outdated Show resolved Hide resolved
@monperrus
Copy link
Collaborator

cool stuff, thanks! see my comments.

@nharrand
Copy link
Collaborator Author

nharrand commented May 7, 2019

Ok Actually I need to add a prettyprinter test. Back to WiP (The test is easy to write but it fails...)

@nharrand nharrand changed the title feat(CtUnresolvedImport): add pure string import in noClassPath mode WiP: feat(CtUnresolvedImport): add pure string import in noClassPath mode May 7, 2019
@nharrand
Copy link
Collaborator Author

Related to #2933

@nharrand
Copy link
Collaborator Author

nharrand commented May 14, 2019

The current implementation should work BUT it might lead to overlapping import when pretty-printing.
An example is src/test/java/spoon/test/prettyprinter/testclasses/Rule.java.
When imports are computed,

import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

are added (as these types are indeed used) but the import overlap with the pre existing CtUnresolvedImport import java.util.*;...

Edit: This is now solved.

@nharrand nharrand changed the title WiP: feat(CtUnresolvedImport): add pure string import in noClassPath mode feat(CtUnresolvedImport): add pure string import in noClassPath mode May 14, 2019
@nharrand
Copy link
Collaborator Author

OK, so now (with this PR), in noClassPath mode:

  1. Unresolved import in the original sources will remain when the model is pretty printed.
  2. But that means that import computation will never remove them even if they are unused.

This mean, that if an import my.package.* is unresolved we don't get the nice clean up effect that we use to have.

@monperrus
Copy link
Collaborator

Equals is not tested https://coveralls.io/builds/23377168/source?filename=src%2Fmain%2Fjava%2Fspoon%2Fexperimental%2FCtUnresolvedImport.java#L73

This is dangerous, could you add one assertTrue and one assertFalse?

@monperrus monperrus changed the title feat(CtUnresolvedImport): add pure string import in noClassPath mode feat(CtUnresolvedImport): keep all imports in noClassPath mode May 16, 2019
@monperrus monperrus merged commit d0aebbb into INRIA:master May 16, 2019
@monperrus
Copy link
Collaborator

Great feature for the noClasspath mode. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants