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 type of constructor calls in noclasspathmode #4647

Merged
merged 11 commits into from
Jun 16, 2022
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
23 changes: 23 additions & 0 deletions src/main/java/spoon/NoClasspathWorkaround.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* SPDX-License-Identifier: (MIT OR CECILL-C)
*
* Copyright (C) 2006-2019 INRIA and contributors
*
* Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon.
*/
package spoon;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I hate to come with yet another thing after not saying anything for so long, but should we not perhaps put this in the internal package? It seems like something we don't want to share with the world, but only use internally in Spoon.

Copy link
Collaborator

@slarse slarse May 11, 2022

Choose a reason for hiding this comment

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

Actually, spoon.testing.utils is probably the place we want to put it, considering the naming of that package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using a testing package seems wrong for this. As this is not really related to testing or? An internal package, yes indeed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Using a testing package seems wrong for this. As this is not really related to testing or?

I have no idea of what last week me was thinking. It was close to midnight so I suspect: not much.

An internal package, yes indeed.

Yes. That.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about we do this in spoon.experimental and see if we have enough classes to create an internal package in the future?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MartinWitt Ugh, missed this comment.

But yes, sure. Essentially, if you put it anywhere where we can freely break it I'm satisfied.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ugh, missed this comment.

No worries, I also had a lengthy summer vacation and enjoyed the nice weather.
I moved the class to the experimental package and added Experimental and Internal. More evidence that we can and will break this annotation's package is not possible.


/**
* This annotation is used to mark a workaround for the lack of a correct classpath so called noclasspathmode.
* <p>
* Workarounds for missing informations are marked with this annotation. These methods are not part of the Spoon API and best effort.
* With any new jdt version the workaround can be removed or no longer working.
*/
public @interface NoClasspathWorkaround {
/**
* The reason why this workaround is needed. This is used for documentation purposes.
* A link to the originale issue is sufficient.
* @return the reason why this workaround is needed
*/
String reason();
}
28 changes: 28 additions & 0 deletions src/main/java/spoon/experimental/NoClasspathWorkaround.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* SPDX-License-Identifier: (MIT OR CECILL-C)
*
* Copyright (C) 2006-2019 INRIA and contributors
*
* Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon.
*/
package spoon.experimental;

import spoon.support.Experimental;
import spoon.support.Internal;

/**
* This annotation is used to mark a workaround for the lack of a correct classpath so called noclasspathmode.
* <p>
* Workarounds for missing informations are marked with this annotation. These methods are not part of the Spoon API and best effort.
* With any new jdt version the workaround can be removed or no longer working.
*/
@Experimental
@Internal
public @interface NoClasspathWorkaround {
/**
* The reason why this workaround is needed. This is used for documentation purposes.
* A link to the originale issue is sufficient.
* @return the reason why this workaround is needed
*/
String reason();
}
37 changes: 36 additions & 1 deletion src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import org.eclipse.jdt.internal.compiler.lookup.VariableBinding;
import org.eclipse.jdt.internal.compiler.lookup.VoidTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.WildcardBinding;
import spoon.NoClasspathWorkaround;
import spoon.reflect.code.CtLambda;
import spoon.reflect.declaration.CtModule;
import spoon.reflect.declaration.CtPackage;
Expand Down Expand Up @@ -449,12 +450,17 @@ <T> CtExecutableReference<T> getExecutableReference(AllocationExpression allocat
CtExecutableReference<T> ref;
if (allocationExpression.binding != null) {
ref = getExecutableReference(allocationExpression.binding);
// in some cases the binding is not null but points wrong to object type see #4643
if (isIncorrectlyBoundExecutableInNoClasspath(ref, allocationExpression)) {
adjustExecutableAccordingToResolvedType(ref, allocationExpression);
}
} else {
ref = jdtTreeBuilder.getFactory().Core().createExecutableReference();
ref.setSimpleName(CtExecutableReference.CONSTRUCTOR_NAME);
ref.setDeclaringType(getTypeReference(null, allocationExpression.type));

final List<CtTypeReference<?>> parameters = new ArrayList<>(allocationExpression.argumentTypes.length);
final List<CtTypeReference<?>> parameters =
new ArrayList<>(allocationExpression.argumentTypes.length);
for (TypeBinding b : allocationExpression.argumentTypes) {
parameters.add(getTypeReference(b, true));
}
Expand All @@ -466,6 +472,35 @@ <T> CtExecutableReference<T> getExecutableReference(AllocationExpression allocat
return ref;
}

/**
* Checks if the given executable reference is incorrectly bound to the Object type and noclasspath is set.
* @param ref the executable reference to check
* @param allocationExpression the allocation expression that contains the executable reference of jdt.
* @return true if the executable reference is incorrectly bound to the Object type and noclasspath is set.
*/
@NoClasspathWorkaround(reason = "https://github.com/INRIA/spoon/issues/4643")
private boolean isIncorrectlyBoundExecutableInNoClasspath(CtExecutableReference<?> ref,
AllocationExpression allocationExpression) {
boolean noClasspath = ref.getFactory().getEnvironment().getNoClasspath();
return noClasspath && ref.getType().equals(ref.getFactory().Type().objectType())
&& allocationExpression.resolvedType != null;
}

/**
* Adjusts the executable reference according to the resolved type. This is needed because the binding is not correct in no classpath.
* @param ref the executable reference to adjust
* @param allocationExpression the allocation expression that contains the executable reference of jdt.
*/
@NoClasspathWorkaround(reason = "https://github.com/INRIA/spoon/issues/4643")
@SuppressWarnings("unchecked")
private void adjustExecutableAccordingToResolvedType(CtExecutableReference ref,
AllocationExpression allocationExpression) {
CtTypeReference<?> resolvedTypeRef = getTypeReference(allocationExpression.resolvedType);
ref.setType(resolvedTypeRef);
ref.getExecutableDeclaration().setType(resolvedTypeRef);
ref.setDeclaringType(resolvedTypeRef);
}

<T> CtExecutableReference<T> getExecutableReference(MessageSend messageSend) {
if (messageSend.binding != null) {
return getExecutableReference(messageSend.binding);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.List;
import java.util.TreeSet;
import java.util.stream.Collectors;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import spoon.Launcher;
Expand All @@ -42,6 +41,7 @@


import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertSame;
Expand Down Expand Up @@ -221,4 +221,18 @@ private CtTypeReference<?> getConstructorCallTypeFrom(String simpleName, String
assert calls.size() == 1;
return calls.get(0).getExecutable().getType();
}

@Test
public void testConstructorCorrectTyped() {
// no constructorcall from the input has the simple object type in noclasspathmode
Launcher launcher = new Launcher();
launcher.getEnvironment().setNoClasspath(true);
launcher.addInputResource("./src/test/resources/constructorcall-type/ConstructorCallWithTypesNotOnClasspath.java");
CtModel model = launcher.buildModel();
for (CtConstructorCall<?> ctConstructorCall : model
.getElements(new TypeFilter<>(CtConstructorCall.class))) {
assertThat(ctConstructorCall.getExecutable().getType().getSimpleName(), not(equalTo("Object")));
assertThat(ctConstructorCall.getType(), not(ctConstructorCall.getFactory().Type().objectType()));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class ConstructorCallWithTypesNotOnClasspath {
public static void main(String[] args) {
type.not.on.Classpath instanceOfTypeNotOnClasspath = null;
new another.type.not.on.Classpath(instanceOfTypeNotOnClasspath);
}
}