-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Fix many small bugs with double usage of AST nodes in different subtrees, ... #645
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/** | ||
* Copyright (C) 2006-2015 INRIA and contributors | ||
* Spoon - http://spoon.gforge.inria.fr/ | ||
* | ||
* This software is governed by the CeCILL-C License under French law and | ||
* abiding by the rules of distribution of free software. You can use, modify | ||
* and/or redistribute the software under the terms of the CeCILL-C license as | ||
* circulated by CEA, CNRS and INRIA at http://www.cecill.info. | ||
* | ||
* This program is distributed in the hope that it will be useful, but WITHOUT | ||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
* FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. | ||
* | ||
* The fact that you are presently reading this means that you have had | ||
* knowledge of the CeCILL-C license and that you accept its terms. | ||
*/ | ||
package spoon.reflect.visitor; | ||
|
||
import spoon.reflect.declaration.CtElement; | ||
|
||
public class AstParentConsistencyChecker extends CtScanner { | ||
|
||
private CtElement parent; | ||
@Override | ||
public void scan(CtElement element) { | ||
if (element == null) { | ||
return; | ||
} | ||
if (parent != null && element.getParent() != parent) { | ||
throw new IllegalStateException(toDebugString(element) | ||
+ "is set as child of\n" + toDebugString(element.getParent()) | ||
+ "however it is visited as a child of\n" + toDebugString(parent)); | ||
} | ||
CtElement parent = this.parent; | ||
this.parent = element; | ||
super.scan(element); | ||
this.parent = parent; | ||
} | ||
|
||
private static String toDebugString(CtElement e) { | ||
return "Element: " + e + "\nSignature: " + e.getSignature() + "\nClass: " + e.getClass() + "\n"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -565,6 +565,7 @@ public <T> void visitCtConstructorCall(final CtConstructorCall<T> ctConstructorC | |
scan(ctConstructorCall.getTypeCasts()); | ||
scan(ctConstructorCall.getExecutable()); | ||
scan(ctConstructorCall.getTarget()); | ||
scan(ctConstructorCall.getActualTypeArguments()); | ||
scan(ctConstructorCall.getArguments()); | ||
scan(ctConstructorCall.getComments()); | ||
exit(ctConstructorCall); | ||
|
@@ -573,10 +574,10 @@ public <T> void visitCtConstructorCall(final CtConstructorCall<T> ctConstructorC | |
public <T> void visitCtNewClass(final CtNewClass<T> newClass) { | ||
enter(newClass); | ||
scan(newClass.getAnnotations()); | ||
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. why this? 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. @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. |
||
scan(newClass.getType()); | ||
scan(newClass.getTypeCasts()); | ||
scan(newClass.getExecutable()); | ||
scan(newClass.getTarget()); | ||
scan(newClass.getActualTypeArguments()); | ||
scan(newClass.getArguments()); | ||
scan(newClass.getAnonymousClass()); | ||
scan(newClass.getComments()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1486,16 +1486,17 @@ public <T> void visitCtConstructorCall(final spoon.reflect.code.CtConstructorCal | |
replaceInListIfExist(ctConstructorCall.getTypeCasts(), new spoon.support.visitor.replace.ReplacementVisitor.CtExpressionTypeCastsReplaceListener(ctConstructorCall)); | ||
replaceElementIfExist(ctConstructorCall.getExecutable(), new spoon.support.visitor.replace.ReplacementVisitor.CtAbstractInvocationExecutableReplaceListener(ctConstructorCall)); | ||
replaceElementIfExist(ctConstructorCall.getTarget(), new spoon.support.visitor.replace.ReplacementVisitor.CtTargetedExpressionTargetReplaceListener(ctConstructorCall)); | ||
replaceInListIfExist(ctConstructorCall.getActualTypeArguments(), new spoon.support.visitor.replace.ReplacementVisitor.CtGenericElementReferenceActualTypeArgumentsReplaceListener(ctConstructorCall)); | ||
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.
|
||
replaceInListIfExist(ctConstructorCall.getArguments(), new spoon.support.visitor.replace.ReplacementVisitor.CtAbstractInvocationArgumentsReplaceListener(ctConstructorCall)); | ||
replaceInListIfExist(ctConstructorCall.getComments(), new spoon.support.visitor.replace.ReplacementVisitor.CtElementCommentsReplaceListener(ctConstructorCall)); | ||
} | ||
|
||
public <T> void visitCtNewClass(final spoon.reflect.code.CtNewClass<T> newClass) { | ||
replaceInListIfExist(newClass.getAnnotations(), new spoon.support.visitor.replace.ReplacementVisitor.CtElementAnnotationsReplaceListener(newClass)); | ||
replaceElementIfExist(newClass.getType(), new spoon.support.visitor.replace.ReplacementVisitor.CtTypedElementTypeReplaceListener(newClass)); | ||
replaceInListIfExist(newClass.getTypeCasts(), new spoon.support.visitor.replace.ReplacementVisitor.CtExpressionTypeCastsReplaceListener(newClass)); | ||
replaceElementIfExist(newClass.getExecutable(), new spoon.support.visitor.replace.ReplacementVisitor.CtAbstractInvocationExecutableReplaceListener(newClass)); | ||
replaceElementIfExist(newClass.getTarget(), new spoon.support.visitor.replace.ReplacementVisitor.CtTargetedExpressionTargetReplaceListener(newClass)); | ||
replaceInListIfExist(newClass.getActualTypeArguments(), new spoon.support.visitor.replace.ReplacementVisitor.CtGenericElementReferenceActualTypeArgumentsReplaceListener(newClass)); | ||
replaceInListIfExist(newClass.getArguments(), new spoon.support.visitor.replace.ReplacementVisitor.CtAbstractInvocationArgumentsReplaceListener(newClass)); | ||
replaceElementIfExist(newClass.getAnonymousClass(), new spoon.support.visitor.replace.ReplacementVisitor.CtNewClassAnonymousClassReplaceListener(newClass)); | ||
replaceInListIfExist(newClass.getComments(), new spoon.support.visitor.replace.ReplacementVisitor.CtElementCommentsReplaceListener(newClass)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.