WiP feat: computeImports at the level of CU (which were before in DJPP) #2100
Conversation
monperrus
changed the title
| @@ -33,6 +36,7 @@ | |||
| @Override | |||
| public void onObjectUpdate(CtElement currentElement, CtRole role, | |||
| CtElement newValue, CtElement oldValue) { | |||
| this.changeCu(currentElement); | |||
There was a problem hiding this comment.
Let's rename this type from EmptyModelChangeListener to DefaultModelChangeListener ... because it is no more empty
WDYT?
There was a problem hiding this comment.
Yes I did not plan to keep it like that ;)
| private void changeCu(CtElement element) { | ||
| SourcePosition position = element.getPosition(); | ||
| if (position != null && !(position instanceof NoSourcePosition)) { | ||
| position.getCompilationUnit().setIsChanged(true); |
There was a problem hiding this comment.
If spoon needs that call to keep internal consistency, then it should be part of some listener, which is always used - cannot be replaced by a different custom listener. I guess spoon should allow to register more then one listener.
WDYT?
There was a problem hiding this comment.
Not necessarily.
It should be part of a standard listener that the user should be able to extend but it also could create another listener and never call this method: he would have to manage manually the flags to compute back the new imports then.
|
I m afraid of the negative performance impact of this change |
Actually it should not impact much the users: the imports are only computed back in case of pretty-printing after a change in the model... and this was already the case before |
My issue is not this one. You are calling getParent(CtType.class) a lot of time and this method is not a light one. This will be call frequently during the creation of the model... |
Actually I do this call only if the element has no attached source position to give me the CU. |
Some ast node does not have a position, and during the creation of the model, the elements do not have a position yet. And I m pretty sure that all the compilation unit will be marked as changed with the current change. |
Then I should only activate this listener when the model has been built. Is there a property somewhere that is set when the model has been built for the first time? Do you think it might be interesting to add this feature if it does not exist? |
yes! I need that for Sniper mode too ... |
|
API changes: 6 (Detected by Revapi) Old API: fr.inria.gforge.spoon:spoon-core:jar:6.3.0-20180629.101005-191 / New API: fr.inria.gforge.spoon:spoon-core:jar:6.3.0-SNAPSHOT
|
|
close? |
|
The sniper mode still needs improvements in area of computation of imports, so this PR is probably (I haven't tried to understood it yet) still relevant. Simon has to decide.
this sounds like a very good way 👍 |
|
@surli ok to close? |
|
@surli do not delete the branch of this PR. It is some chance that I fail with refactoring of import scanner and that we have to continue here. |
wasn't intending to: as I said I might restart the work here :) |
This PR follows the work started in #2083.
The idea is to be able to compute imports in CU, to allow the user to call that method properly after their transformations: then I'm trying to extract everything regarding the imports from the DJPP to the CU.