Skip to content

Commit

Permalink
hopefully fix unused detection for constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
@Luro02
Luro02 committed Apr 13, 2024
1 parent 86c1b11 commit ae751ea
Show file tree
Hide file tree
Showing 3 changed files with 291 additions and 5 deletions.
79 changes: 77 additions & 2 deletions autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import spoon.reflect.code.CtConstructorCall;
import spoon.reflect.code.CtExecutableReferenceExpression;
import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtFieldWrite;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtJavaDoc;
import spoon.reflect.code.CtLambda;
Expand All @@ -38,15 +37,16 @@
import spoon.reflect.code.UnaryOperatorKind;
import spoon.reflect.cu.SourcePosition;
import spoon.reflect.cu.position.CompoundSourcePosition;
import spoon.reflect.declaration.CtClass;
import spoon.reflect.declaration.CtCompilationUnit;
import spoon.reflect.declaration.CtConstructor;
import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtExecutable;
import spoon.reflect.declaration.CtImport;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.CtNamedElement;
import spoon.reflect.declaration.CtParameter;
import spoon.reflect.declaration.CtType;
import spoon.reflect.declaration.CtTypeInformation;
import spoon.reflect.declaration.CtTypeMember;
import spoon.reflect.declaration.CtTypeParameter;
import spoon.reflect.declaration.CtTypedElement;
Expand Down Expand Up @@ -1339,10 +1339,85 @@ private static Filter<CtElement> buildExecutableFilter(CtExecutable<?> ctExecuta
);
}

// constructors can be called implicitly
if (ctExecutable instanceof CtConstructor<?> ctConstructor && ctConstructor.getParameters().isEmpty()) {
// constructors of subclasses must use the no-arg constructor of the super class
// (this is not true, only if the subclass constructor calls the super class constructor explicitly or does not)
filter = new CompositeFilter<>(
FilteringOperator.UNION,
filter,
new FilterAdapter<>(
otherConstructor -> otherConstructor.getParameters().isEmpty()
&& otherConstructor != ctConstructor
&& SpoonUtil.callsParentConstructor(otherConstructor, ctConstructor),
(Class<CtConstructor<?>>) (Object) CtConstructor.class
)
);

// if the subclass does not have an explicit constructor, it still uses the super class constructor
if (ctConstructor.getDeclaringType() instanceof CtClass<?> ctClass) {
for (CtType<?> ctType : SpoonUtil.getDirectSubtypes(ctClass)) {
if (ctType instanceof CtClass<?> subclass) {
var ctConstructors = subclass.getConstructors();
if (ctConstructors.size() == 1 && ctConstructors.iterator().next().isImplicit()) {
filter = new CompositeFilter<>(
FilteringOperator.UNION,
filter,
new FilterAdapter<>(
element -> element == subclass,
(Class<CtType<?>>) (Object) CtType.class
)
);
}

}
}
}
}

return filter;
}
}

private static List<CtType<?>> getDirectSubtypes(CtType<?> ctType) {
return ctType.getFactory().getModel().getElements(new TypeFilter<CtType<?>>(CtType.class))
.stream()
.filter(type -> SpoonUtil.isDirectSubtypeOf(type, ctType))
.toList();

}

private static boolean callsParentConstructor(CtConstructor<?> child, CtConstructor<?> parent) {
CtType<?> childType = child.getDeclaringType();
CtType<?> parentType = parent.getDeclaringType();

if (!SpoonUtil.isDirectSubtypeOf(childType, parentType)) {
return false;
}

for (CtStatement ctStatement : SpoonUtil.getEffectiveStatements(child.getBody())) {
if (!(ctStatement instanceof CtInvocation<?> ctInvocation) || ctInvocation.getTarget() != null) {
continue;
}

if (ctInvocation.getExecutable().equals(parent.getReference())) {
return true;
}
}

return false;
}

private static boolean isDirectSubtypeOf(CtTypeInformation ctType, CtType<?> superType) {
if (ctType.getSuperclass() == null) {
return SpoonUtil.isTypeEqualTo(superType.getReference(), java.lang.Object.class);
}

return ctType.getSuperclass().equals(superType.getReference())
|| superType.equals(SpoonUtil.getReferenceDeclaration(ctType.getSuperclass()))
|| ctType.getSuperInterfaces().contains(superType.getReference());
}

// Supported CtElement subtypes:
// - CtVariable<?>
// - CtExecutable<?>
Expand Down
87 changes: 87 additions & 0 deletions ...e/src/test/java/de/firemage/autograder/core/check/general/TestUseDifferentVisibility.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,4 +571,91 @@ public MyException() {}

problems.assertExhausted();
}

@Test
void testConstructorImplicitSuper() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
JavaVersion.JAVA_17,
Map.ofEntries(
Map.entry(
"Main",
"""
public class Main {
public static void main(String[] args) {
Parent parent = new Child();
System.out.println(parent);
}
}
"""
),
Map.entry(
"Parent",
"""
public abstract class Parent {
protected Parent() {
}
}
"""
),
Map.entry(
"Child",
"""
public class Child extends Parent {
public Child() {
}
}
"""
)
)
), PROBLEM_TYPES);

problems.assertExhausted();
}

@Test
void testConstructorImplicitSuperTwice() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
JavaVersion.JAVA_17,
Map.ofEntries(
Map.entry(
"Main",
"""
public class Main {
public static void main(String[] args) {
Parent parent = new SubChild();
System.out.println(parent);
}
}
"""
),
Map.entry(
"Parent",
"""
public abstract class Parent {
protected Parent() {
}
}
"""
),
Map.entry(
"Child",
"""
public class Child extends Parent {
}
"""
),
Map.entry(
"SubChild",
"""
public class SubChild extends Child {
public SubChild() {
}
}
"""
)
)
), PROBLEM_TYPES);

problems.assertExhausted();
}
}
130 changes: 127 additions & 3 deletions ...c/test/java/de/firemage/autograder/core/check/unnecessary/TestUnusedCodeElementCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public enum MyEnum { // not ok

@Test
void testUnusedConstructor() throws LinterException, IOException {
List<Problem> problems = this.check(StringSourceInfo.fromSourceStrings(
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
JavaVersion.JAVA_17,
Map.ofEntries(
Map.entry(
Expand All @@ -373,8 +373,9 @@ public static void main(String[] args) {}
)
), PROBLEM_TYPES);

assertProblemSize(1, problems);
assertEqualsUnused("Main()", problems.get(0));
assertEqualsUnused("Main()", problems.next());

problems.assertExhausted();
}

@Test
Expand Down Expand Up @@ -768,4 +769,127 @@ public static void main(String[] args) {

problems.assertExhausted();
}

@Test
void testImplicitConstructorCall() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
JavaVersion.JAVA_17,
Map.ofEntries(
Map.entry(
"Main",
"""
public class Main {
public static void main(String[] args) {
Parent parent = new Child();
System.out.println(parent);
}
}
"""
),
Map.entry(
"Parent",
"""
public class Parent {
protected Parent() {
System.out.println("Called Parent Constructor");
}
}
"""
),
Map.entry(
"Child",
"""
public class Child extends Parent {
public Child() {
// implicit super() call
}
}
"""
)
)
), PROBLEM_TYPES);

problems.assertExhausted();
}

@Test
void testExplicitSuperConstructorCall() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
JavaVersion.JAVA_17,
Map.ofEntries(
Map.entry(
"Main",
"""
public class Main {
public static void main(String[] args) {
Parent parent = new Child();
System.out.println(parent);
}
}
"""
),
Map.entry(
"Parent",
"""
public class Parent {
protected Parent() {
System.out.println("Called Parent Constructor");
}
}
"""
),
Map.entry(
"Child",
"""
public class Child extends Parent {
public Child() {
super();
}
}
"""
)
)
), PROBLEM_TYPES);

problems.assertExhausted();
}

@Test
void testImplicitConstructorCallWithoutConstructor() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceStrings(
JavaVersion.JAVA_17,
Map.ofEntries(
Map.entry(
"Main",
"""
public class Main {
public static void main(String[] args) {
Parent parent = new Child();
System.out.println(parent);
}
}
"""
),
Map.entry(
"Parent",
"""
public class Parent {
protected Parent() {
System.out.println("Called Parent Constructor");
}
}
"""
),
Map.entry(
"Child",
"""
public class Child extends Parent {
}
"""
)
)
), PROBLEM_TYPES);

problems.assertExhausted();
}
}

0 comments on commit ae751ea

Please sign in to comment.