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

Fix many small bugs with double usage of AST nodes in different subtrees, ... #645

Merged
merged 1 commit into from
May 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/main/java/spoon/reflect/factory/CodeFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public <T> CtFieldAccess<Class<T>> createClassAccess(CtTypeReference<T> type) {
fieldReference.setDeclaringType(type);

CtFieldRead<Class<T>> fieldRead = factory.Core().createFieldRead();
fieldRead.setType(classType);
fieldRead.setType(factory.Core().clone(classType));
Copy link
Contributor

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?

Copy link
Contributor Author

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.

fieldRead.setVariable(fieldReference);
fieldRead.setTarget(typeAccess);
return fieldRead;
Expand All @@ -159,7 +159,7 @@ public <T> CtConstructorCall<T> createConstructorCall(CtTypeReference<T> type, C
CtExecutableReference<T> executableReference = factory.Core()
.createExecutableReference();
executableReference.setType(type);
executableReference.setDeclaringType(type);
executableReference.setDeclaringType(factory.Core().clone(type));
executableReference.setSimpleName(CtExecutableReference.CONSTRUCTOR_NAME);
List<CtTypeReference<?>> typeReferences = new ArrayList<CtTypeReference<?>>();
for (int i = 0; i < parameters.length; i++) {
Expand Down Expand Up @@ -285,7 +285,7 @@ public <T> CtLocalVariable<T> createLocalVariable(CtTypeReference<T> type, Strin
*/
public <T> CtLocalVariableReference<T> createLocalVariableReference(CtLocalVariable<T> localVariable) {
CtLocalVariableReference<T> ref = factory.Core().createLocalVariableReference();
ref.setType(localVariable.getType());
ref.setType(factory.Core().clone(localVariable.getType()));
ref.setSimpleName(localVariable.getSimpleName());
ref.setDeclaration(localVariable);
return ref;
Expand Down Expand Up @@ -368,7 +368,7 @@ public <T> CtVariableAccess<T> createVariableRead(CtVariableReference<T> variabl
} else {
va = factory.Core().createVariableRead();
}
return va.setVariable(variable).setType(variable.getType());
return va.setVariable(variable).setType(factory.Core().clone(variable.getType()));
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/spoon/reflect/factory/ConstructorFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public <T> CtExecutableReference<T> createReference(CtConstructor<T> c) {
*/
public <T> CtExecutableReference<T> createReference(Constructor<T> constructor) {
CtTypeReference<T> type = factory.Type().createReference(constructor.getDeclaringClass());
return createReference(type, type, CtExecutableReference.CONSTRUCTOR_NAME,
return createReference(type, factory.Core().clone(type), CtExecutableReference.CONSTRUCTOR_NAME,
factory.Type().createReferences(Arrays.asList(constructor.getParameterTypes())));
}

Expand All @@ -166,11 +166,11 @@ public <T> CtExecutableReference<T> createReference(Constructor<T> constructor)
public <T> CtExecutableReference<T> createReference(CtTypeReference<T> type, CtExpression<?>...parameters) {
final CtExecutableReference<T> executableReference = factory.Core().createExecutableReference();
executableReference.setType(type);
executableReference.setDeclaringType(type);
executableReference.setDeclaringType(factory.Core().clone(type));
executableReference.setSimpleName(CtExecutableReference.CONSTRUCTOR_NAME);
List<CtTypeReference<?>> typeReferences = new ArrayList<CtTypeReference<?>>();
for (CtExpression<?> parameter : parameters) {
typeReferences.add(parameter.getType());
typeReferences.add(factory.Core().clone(parameter.getType()));
}
executableReference.setParameters(typeReferences);
return executableReference;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/spoon/reflect/factory/ExecutableFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ public <T> CtExecutableReference<T> createReference(CtExecutable<T> e) {
CtTypeReference<?> refs[] = new CtTypeReference[e.getParameters().size()];
int i = 0;
for (CtParameter<?> param : e.getParameters()) {
refs[i++] = param.getType();
refs[i++] = factory.Core().clone(param.getType());
}
if (e instanceof CtMethod) {
return createReference(((CtMethod<T>) e).getDeclaringType().getReference(), ((CtMethod<T>) e).getType(), e.getSimpleName(), refs);
return createReference(((CtMethod<T>) e).getDeclaringType().getReference(), factory.Core().clone(((CtMethod<T>) e).getType()), e.getSimpleName(), refs);
}
return createReference(((CtConstructor<T>) e).getDeclaringType().getReference(), ((CtConstructor<T>) e).getType(), CtExecutableReference.CONSTRUCTOR_NAME, refs);
return createReference(((CtConstructor<T>) e).getDeclaringType().getReference(), factory.Core().clone(((CtConstructor<T>) e).getType()), CtExecutableReference.CONSTRUCTOR_NAME, refs);
}

/**
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/spoon/reflect/factory/TypeFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,11 @@ public CtTypeParameterReference createTypeParameterReference(String name, List<C
*/
public <T> CtIntersectionTypeReference<T> createIntersectionTypeReference(List<CtTypeReference<?>> bounds) {
final CtIntersectionTypeReference<T> intersectionRef = factory.Core().createIntersectionTypeReference();
intersectionRef.setSimpleName(bounds.get(0).getSimpleName());
intersectionRef.setDeclaringType(bounds.get(0).getDeclaringType());
intersectionRef.setPackage(bounds.get(0).getPackage());
intersectionRef.setActualTypeArguments(bounds.get(0).getActualTypeArguments());
CtTypeReference<?> firstBound = factory.Core().clone(bounds.get(0));
intersectionRef.setSimpleName(firstBound.getSimpleName());
intersectionRef.setDeclaringType(firstBound.getDeclaringType());
intersectionRef.setPackage(firstBound.getPackage());
intersectionRef.setActualTypeArguments(firstBound.getActualTypeArguments());
intersectionRef.setBounds(bounds);
return intersectionRef;
}
Expand Down
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";
}
}
3 changes: 2 additions & 1 deletion src/main/java/spoon/reflect/visitor/CtScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import spoon.reflect.factory.Factory;
import spoon.reflect.visitor.DefaultJavaPrettyPrinter;
import spoon.reflect.visitor.Filter;
import spoon.reflect.visitor.AstParentConsistencyChecker;
import spoon.reflect.visitor.PrettyPrinter;
import spoon.reflect.visitor.Query;
import spoon.support.QueueProcessingManager;
Expand Down Expand Up @@ -130,9 +131,14 @@ public boolean build(JDTBuilder builder) {
t = System.currentTimeMillis();
templateSuccess = buildTemplates(builder);
factory.getEnvironment().debugMessage("built in " + (System.currentTimeMillis() - t) + " ms");
checkModel();
return srcSuccess && templateSuccess;
}

private void checkModel() {
factory.getModel().getRootPackage().accept(new AstParentConsistencyChecker());
}

@Override
public boolean compile() {
initInputClassLoader();
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -2095,7 +2095,7 @@ private <T, E extends CtExpression<?>> CtExecutableReferenceExpression<T, E> cre
executableReference.setSimpleName(new String(referenceExpression.selector));
executableReference.setDeclaringType(references.getTypeReference(referenceExpression.lhs.resolvedType));
}
executableReference.setType((CtTypeReference<T>) executableReference.getDeclaringType());
executableReference.setType(factory.Core().clone((CtTypeReference<T>) executableReference.getDeclaringType()));
executableRef.setExecutable(executableReference);
return executableRef;
}
Expand Down Expand Up @@ -3272,7 +3272,7 @@ public boolean onAccess(char[][] tokens, int index) {
va = factory.Core().createVariableRead();
}
va.setVariable(references.getVariableReference((VariableBinding) qualifiedNameReference.binding));
va.setType(va.getVariable().getType());
va.setType(factory.Core().clone(va.getVariable().getType()));
if (qualifiedNameReference.otherBindings != null) {
int i = 0; //positions index;
int sourceStart = (int) (positions[0] >>> 32);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/spoon/support/compiler/jdt/ParentExiter.java
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,9 @@ public <T> void visitCtNewClass(CtNewClass<T> newClass) {
final QualifiedAllocationExpression node = (QualifiedAllocationExpression) jdtTreeBuilder.context.stack.peek().node;
final ReferenceBinding[] referenceBindings = node.resolvedType == null ? null : node.resolvedType.superInterfaces();
if (referenceBindings != null && referenceBindings.length > 0) {
((CtClass<?>) child).addSuperInterface(newClass.getType());
((CtClass<?>) child).addSuperInterface(child.getFactory().Core().clone(newClass.getType()));
} else if (newClass.getType() != null) {
((CtClass<?>) child).setSuperclass(newClass.getType());
((CtClass<?>) child).setSuperclass(child.getFactory().Core().clone(newClass.getType()));
}
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReplacementVisitor is generated by a processor in spoon.generating. A test case should notify you about that. I'll check that.

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));
Expand Down