Skip to content

Commit

Permalink
fix: Fix missing position in rare cases (#3056)
Browse files Browse the repository at this point in the history
  • Loading branch information
Egor18 authored and monperrus committed Jul 25, 2019
1 parent 2469d6c commit 49e395f
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 6 deletions.
3 changes: 1 addition & 2 deletions src/main/java/spoon/reflect/factory/ClassFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ public <T> CtClass<T> create(CtPackage owner, String simpleName) {
* @param <T>
* type of created class
* @param qualifiedName
* full name of class to create. Name can contain . or $ for
* inner types
* full name of class to create. Name can contain $ for inner types
*/
public <T> CtClass<T> create(String qualifiedName) {
if (hasInnerType(qualifiedName) > 0) {
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/spoon/reflect/factory/InterfaceFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ public <T> CtInterface<T> create(CtType<T> owner, String simpleName) {
}

/**
* Creates an interface.
* Creates an interface from its qualified name.
*
* @param <T>
* type of created interface
*
* @param qualifiedName
* full name of interface to create. Name can contain $ for inner types
*/
@SuppressWarnings("unchecked")
public <T> CtInterface<T> create(String qualifiedName) {
Expand Down
14 changes: 12 additions & 2 deletions src/main/java/spoon/support/compiler/jdt/ContextBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,15 @@ <T> CtVariable<T> getVariableDeclaration(final String name) {
return variable;
}

/**
* Returns qualified name with appropriate package separator and inner type separator
*/
private static String getNormalQualifiedName(ReferenceBinding referenceBinding) {
String pkg = new String(referenceBinding.getPackage().readableName()).replaceAll("\\.", "\\" + CtPackage.PACKAGE_SEPARATOR);
String name = new String(referenceBinding.qualifiedSourceName()).replaceAll("\\.", "\\" + CtType.INNERTTYPE_SEPARATOR);
return pkg.equals("") ? name : pkg + "." + name;
}

@SuppressWarnings("unchecked")
private <T, U extends CtVariable<T>> U getVariableDeclaration(
final String name, final Class<U> clazz) {
Expand Down Expand Up @@ -250,11 +259,12 @@ private <T, U extends CtVariable<T>> U getVariableDeclaration(
final ReferenceBinding referenceBinding = referenceBindings.pop();
for (final FieldBinding fieldBinding : referenceBinding.fields()) {
if (name.equals(new String(fieldBinding.readableName()))) {
final String qualifiedNameOfParent =
new String(referenceBinding.readableName());
final String qualifiedNameOfParent = getNormalQualifiedName(referenceBinding);

final CtType parentOfField = referenceBinding.isClass()
? classFactory.create(qualifiedNameOfParent)
: interfaceFactory.create(qualifiedNameOfParent);

U field = (U) fieldFactory.create(parentOfField,
EnumSet.noneOf(ModifierKind.class),
referenceBuilder.getTypeReference(fieldBinding.type),
Expand Down
64 changes: 63 additions & 1 deletion src/test/java/spoon/test/position/PositionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.commons.io.FileUtils;
import org.junit.Test;
import spoon.Launcher;
import spoon.reflect.CtModel;
import spoon.reflect.code.CtAssignment;
import spoon.reflect.code.CtBlock;
import spoon.reflect.code.CtCase;
Expand All @@ -31,6 +32,7 @@
import spoon.reflect.code.CtForEach;
import spoon.reflect.code.CtIf;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtLambda;
import spoon.reflect.code.CtLocalVariable;
import spoon.reflect.code.CtNewClass;
import spoon.reflect.code.CtReturn;
Expand All @@ -49,6 +51,7 @@
import spoon.reflect.declaration.CtEnum;
import spoon.reflect.declaration.CtField;
import spoon.reflect.declaration.CtImport;
import spoon.reflect.declaration.CtInterface;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.CtPackageDeclaration;
import spoon.reflect.declaration.CtParameter;
Expand Down Expand Up @@ -94,7 +97,6 @@
import java.io.IOException;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -1314,4 +1316,64 @@ public void testPositionBuilderFailureIsCaugth() {
fail("Error while parsing incomplete class declaration");
}
}

@Test
public void testNoClasspathVariableAccessInInnerClass1() {
// contract: creating variable access in no classpath should not break source position
// https://github.com/INRIA/spoon/issues/3052
Launcher launcher = new Launcher();
launcher.addInputResource("./src/test/resources/noclasspath/lambdas/InheritedClassesWithLambda1.java");
launcher.getEnvironment().setNoClasspath(true);
CtModel model = launcher.buildModel();
List<CtClass> allClasses = model.getElements(new TypeFilter<>(CtClass.class));
assertEquals(3, allClasses.size());
CtClass failing = allClasses.stream().filter(t -> t.getSimpleName().equals("Failing")).findFirst().get();
assertEquals("InheritedClassesWithLambda1.java", failing.getPosition().getFile().getName());
assertEquals(11, failing.getPosition().getLine());

// in addition check that the variable reference is correct
CtLambda lambda = model.getElements(new TypeFilter<>(CtLambda.class)).get(0);
CtFieldRead field = (CtFieldRead) (((CtInvocation) lambda.getExpression()).getTarget());
assertEquals("com.pkg.InheritedClassesWithLambda1.Failing", field.getVariable().getDeclaringType().toString());
}

@Test
public void testNoClasspathVariableAccessInInnerClass2() {
// contract: same as for testNoClasspathVariableAccessInInnerClass1,
// but here we have inner class inside another inner class
Launcher launcher = new Launcher();
launcher.addInputResource("./src/test/resources/noclasspath/lambdas/InheritedClassesWithLambda2.java");
launcher.getEnvironment().setNoClasspath(true);
CtModel model = launcher.buildModel();
List<CtClass> allClasses = model.getElements(new TypeFilter<>(CtClass.class));
assertEquals(4, allClasses.size());
CtClass failing = allClasses.stream().filter(t -> t.getSimpleName().equals("Failing")).findFirst().get();
assertEquals("InheritedClassesWithLambda2.java", failing.getPosition().getFile().getName());
assertEquals(11, failing.getPosition().getLine());

// in addition check that the variable reference is correct
CtLambda lambda = model.getElements(new TypeFilter<>(CtLambda.class)).get(0);
CtFieldRead field = (CtFieldRead) (((CtInvocation) lambda.getExpression()).getTarget());
assertEquals("InheritedClassesWithLambda2.OneMoreClass.Failing", field.getVariable().getDeclaringType().toString());
}

@Test
public void testNoClasspathVariableAccessInInnerInterface() {
// contract: same as for testNoClasspathVariableAccessInInnerClass1,
// but here we have interface instead of class
Launcher launcher = new Launcher();
launcher.addInputResource("./src/test/resources/noclasspath/lambdas/InheritedInterfacesWithLambda.java");
launcher.getEnvironment().setNoClasspath(true);
CtModel model = launcher.buildModel();
List<CtInterface> allInterfaces = model.getElements(new TypeFilter<>(CtInterface.class));
assertEquals(1, allInterfaces.size());
CtInterface failing = allInterfaces.stream().filter(t -> t.getSimpleName().equals("Failing")).findFirst().get();
assertEquals("InheritedInterfacesWithLambda.java", failing.getPosition().getFile().getName());
assertEquals(3, failing.getPosition().getLine());

// in addition check that the variable reference is correct
CtLambda lambda = model.getElements(new TypeFilter<>(CtLambda.class)).get(0);
CtFieldRead field = (CtFieldRead) (((CtInvocation) lambda.getExpression()).getTarget());
assertEquals("InheritedInterfacesWithLambda.Failing", field.getVariable().getDeclaringType().toString());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.pkg;

public class InheritedClassesWithLambda1 {

private static class ExtendedFailClass extends Failing {
public void test(View itemView) {
itemView.setOnClickListener(v -> listener.foo());
}
}

public static class Failing {
ClickListener listener;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
public class InheritedClassesWithLambda2 {

private static class ExtendedFailClass extends OneMoreClass.Failing {
public void test(View itemView) {
itemView.setOnClickListener(v -> listener.foo());
}
}

public static class OneMoreClass
{
public static class Failing {
ClickListener listener;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
public class InheritedInterfacesWithLambda {

public interface Failing {
ClickListener listener;
}

private static class ExtendedFailClass implements Failing {
public void test(View itemView) {
itemView.setOnClickListener(v -> listener.foo());
}
}
}

0 comments on commit 49e395f

Please sign in to comment.