Fix many small bugs with double usage of AST nodes in different subtrees, ... #645
Conversation
…ees, a couple of bugs in CtScanner and add checker which always checks AST correctness after model build
|
BTW I had really hard time figuring out that after altering CtScanner I also should change |
| @@ -573,10 +574,10 @@ public void visitCtIf(final CtIf ifElement) { | |||
| public <T> void visitCtNewClass(final CtNewClass<T> newClass) { | |||
| enter(newClass); | |||
| scan(newClass.getAnnotations()); | |||
| scan(newClass.getType()); | |||
There was a problem hiding this comment.
@monperrus because ctNewClass.getType() is not a "true" property of a ctNewClass, it delegates to getExecutable().getType(). In this case scanner shouldn't visit same node twice. Also see that CtNewClass extends CtConstructorCall, which have the same logic for getType(), but doesn't have scan(getType()) in it's visit method in CtScanner.
|
interesting PR |
|
@GerardPaligot what do you think? |
| @@ -139,7 +139,7 @@ public CodeFactory(Factory factory) { | |||
| fieldReference.setDeclaringType(type); | |||
|
|
|||
| CtFieldRead<Class<T>> fieldRead = factory.Core().createFieldRead(); | |||
| fieldRead.setType(classType); | |||
| fieldRead.setType(factory.Core().clone(classType)); | |||
There was a problem hiding this comment.
Do you have an use case where the type of the field read isn't equals of the field reference?
There was a problem hiding this comment.
@GerardPaligot it's not the matter of equality. Any object cannot appear in ast more than once. Otherwise very subtle bugs with element.replace() appear.
|
PR on your PR today. |
|
My mistake, PR not necessary. I merge it. |
…,
a couple of bugs in CtScanner and add checker which always checks AST correctness after model build.