Skip to content

Commit

Permalink
fix: fix modeling bug (#5912)
Browse files Browse the repository at this point in the history
  • Loading branch information
monperrus authored Aug 10, 2024
1 parent d960701 commit 44e3b21
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ CtExpression<?> createTargetFieldAccess(QualifiedNameReference qualifiedNameRefe
} else if (ref.isStatic()) {
target = createTypeAccess(qualifiedNameReference, ref);
} else if (!ref.isStatic() && !ref.getDeclaringType().isAnonymous()) {
if (!JDTTreeBuilderQuery.isResolvedField(qualifiedNameReference)) {
if (!JDTTreeBuilderQuery.isFieldReference(qualifiedNameReference)) {
target = createTypeAccessNoClasspath(qualifiedNameReference);
} else {
target = jdtTreeBuilder.getFactory().Code().createThisAccess(jdtTreeBuilder.getReferencesBuilder().<Object>getTypeReference(qualifiedNameReference.actualReceiverType), true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,15 +217,14 @@ static boolean isValidProblemBindingField(QualifiedNameReference qualifiedNameRe
}

/**
* Check if the name reference is resolved in the JDT tree, i.e. that the declaration is available.
* Check if the name reference is resolved as a field ref in the JDT tree.
*
* @param qualifiedNameReference
* Reference which should contain a field binding.
* @return true if the field has been resolved by the jdt builder.
*/
static boolean isResolvedField(QualifiedNameReference qualifiedNameReference) {
return qualifiedNameReference.binding instanceof FieldBinding
&& ((FieldBinding) qualifiedNameReference.binding).original().sourceField() != null;
static boolean isFieldReference(QualifiedNameReference qualifiedNameReference) {
return qualifiedNameReference.binding instanceof FieldBinding;
}


Expand Down
47 changes: 47 additions & 0 deletions src/test/java/spoon/test/prettyprinter/QualifiedThisRefTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
import org.junit.jupiter.api.Test;
import spoon.Launcher;
import spoon.compiler.SpoonResourceHelper;
import spoon.reflect.code.CtBinaryOperator;
import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtFieldRead;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtLiteral;
import spoon.reflect.code.CtStatement;
Expand All @@ -39,7 +41,9 @@
import spoon.reflect.reference.CtTypeReference;
import spoon.reflect.visitor.DefaultJavaPrettyPrinter;
import spoon.reflect.visitor.filter.TypeFilter;
import spoon.support.compiler.VirtualFile;
import spoon.support.reflect.code.CtFieldAccessImpl;
import spoon.support.reflect.code.CtTypeAccessImpl;
import spoon.test.delete.testclasses.Adobada;
import spoon.test.prettyprinter.testclasses.QualifiedThisRef;

Expand Down Expand Up @@ -144,4 +148,47 @@ public void testPrintCtFieldAccessWorkEvenWhenParentNotInitialized() {
assertFalse(printer.getResult().isEmpty());

}

@Test
public void testGuavaBug() {
// contract: does not model buf as a typeaccess, does not write ByteArrayOutputStream.buf
// this bug was found by spooning Guava in Jenkins
final String code = "class ExposedByteArrayOutputStream extends java.io.ByteArrayOutputStream {\n" +
" ExposedByteArrayOutputStream(int expectedInputSize) {\n" +
" super(expectedInputSize);\n" +
" }\n" +
"\n" +
" void write(java.nio.ByteBuffer input) {\n" +
" int remaining = input.remaining();\n" +
" if ((count + remaining) > buf.length) {\n" +
" buf = java.util.Arrays.copyOf(buf, count + remaining);\n" +
" }\n" +
" input.get(buf, count, remaining);\n" +
" count += remaining;\n" +
" }\n" +
"\n" +
" byte[] byteArray() {\n" +
" return buf;\n" +
" }\n" +
"\n" +
" int length() {\n" +
" return count;\n" +
" }\n" +
" }";

Launcher launcher = new Launcher();
launcher.addInputResource(new VirtualFile(code));
launcher.getEnvironment().setNoClasspath(false);
launcher.getEnvironment().setAutoImports(true);

CtClass<?> c = (CtClass<?>) launcher.buildModel().getAllTypes().iterator().next();
assertEquals(c.getSimpleName().toString(), "ExposedByteArrayOutputStream");

final List<Object> list = c.filterChildren(new TypeFilter<>(CtBinaryOperator.class)).list();
CtBinaryOperator<?> binaryOperator = (CtBinaryOperator<?>) list.get(0);
assertTrue(CtFieldRead.class.isAssignableFrom(binaryOperator.getRightHandOperand().getClass()));
assertEquals("(count + remaining) > buf.length", binaryOperator.toString());
assertFalse(c.toString().contains("ByteArrayOutputStream.buf"), "that will not compile for sure");

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -296,12 +296,16 @@ public void testOnlyStaticTargetFieldReadNoClasspath() {
final Launcher launcher = new Launcher();
launcher.getEnvironment().setNoClasspath(true);
launcher.addInputResource("./src/test/resources/spoon/test/noclasspath/targeted/StaticFieldReadOnly.java");

CtModel model = launcher.buildModel();

List<CtInvocation<?>> invocations = model.getElements(e -> e.getExecutable().getSimpleName().equals("error"));
CtInvocation<?> inv = invocations.get(0);
CtFieldRead<?> fieldRead = (CtFieldRead<?>) inv.getTarget();
CtExpression<?> target = fieldRead.getTarget();
// we do have the right type access in noclasspath mode
// the slight behavior change is that PR 5812 adds one level of indirection in the model, hence the filterChildren call
// however correct behavior is full classpath mode is higher priority, see https://github.com/INRIA/spoon/pull/5912
CtTypeAccess<?> target = (CtTypeAccess<?>) fieldRead.filterChildren(new TypeFilter<>(CtTypeAccess.class)).list().get(0);

assertTrue(target instanceof CtTypeAccess);
assertEquals("Launcher", ((CtTypeAccess<?>) target).getAccessedType().getSimpleName());
Expand Down

0 comments on commit 44e3b21

Please sign in to comment.