Skip to content

Commit

Permalink
fix wrong modifiers on imported array type class objects
Browse files Browse the repository at this point in the history 
The Reflection API considers the visibility of an array type class object
to be equal to the visibility of the element type class object.
ArchUnit so far considered every array type class object as public.
Since we usually strive for being as similar to the Reflection API as possible,
we fix this now by deriving the visibility from the element type as well.

Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
  • Loading branch information
@codecholeric
codecholeric committed Apr 7, 2024
1 parent a7ff56f commit 3eaaeaf
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 7 deletions.
3 changes: 2 additions & 1 deletion archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Sets.immutableEnumSet;
import static com.google.common.collect.Sets.union;
import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
import static com.tngtech.archunit.base.ClassLoaders.getCurrentClassLoader;
Expand Down Expand Up @@ -138,7 +139,7 @@ public final class JavaClass
isRecord = builder.isRecord();
isAnonymousClass = builder.isAnonymousClass();
isMemberClass = builder.isMemberClass();
modifiers = checkNotNull(builder.getModifiers());
modifiers = immutableEnumSet(builder.getModifiers());
reflectSupplier = Suppliers.memoize(new ReflectClassSupplier());
sourceCodeLocation = SourceCodeLocation.of(this);
javaPackage = JavaPackage.simple(this);
Expand Down
36 changes: 30 additions & 6 deletions archunit/src/main/java/com/tngtech/archunit/core/importer/ImportedClasses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand All @@ -30,15 +31,20 @@
import com.tngtech.archunit.core.importer.DomainBuilders.JavaClassBuilder;
import com.tngtech.archunit.core.importer.resolvers.ClassResolver;

import static com.google.common.collect.Sets.immutableEnumSet;
import static com.tngtech.archunit.core.domain.JavaModifier.ABSTRACT;
import static com.tngtech.archunit.core.domain.JavaModifier.FINAL;
import static com.tngtech.archunit.core.domain.JavaModifier.PRIVATE;
import static com.tngtech.archunit.core.domain.JavaModifier.PROTECTED;
import static com.tngtech.archunit.core.domain.JavaModifier.PUBLIC;
import static com.tngtech.archunit.core.importer.ImportedClasses.ImportedClassState.HAD_TO_BE_IMPORTED;
import static com.tngtech.archunit.core.importer.ImportedClasses.ImportedClassState.WAS_ALREADY_PRESENT;

class ImportedClasses {
private static final ImmutableSet<JavaModifier> PRIMITIVE_AND_ARRAY_TYPE_MODIFIERS =
Sets.immutableEnumSet(PUBLIC, ABSTRACT, FINAL);
private static final ImmutableSet<JavaModifier> PRIMITIVE_TYPE_MODIFIERS =
immutableEnumSet(PUBLIC, ABSTRACT, FINAL);
private static final ImmutableSet<JavaModifier> VISIBILITY_MODIFIERS =
immutableEnumSet(PUBLIC, PROTECTED, PRIVATE);

private final ImmutableMap<String, JavaClass> directlyImported;
private final Map<String, JavaClass> allClasses = new HashMap<>();
Expand Down Expand Up @@ -92,19 +98,37 @@ Collection<JavaClass> getAllWithOuterClassesSortedBeforeInnerClasses() {
return ImmutableSortedMap.copyOf(allClasses).values();
}

private static JavaClass stubClassOf(String typeName) {
private JavaClass stubClassOf(String typeName) {
JavaClassDescriptor descriptor = JavaClassDescriptor.From.name(typeName);
JavaClassBuilder builder = JavaClassBuilder.forStub().withDescriptor(descriptor);
addModifiersIfPossible(builder, descriptor);
return builder.build();
}

private static void addModifiersIfPossible(JavaClassBuilder builder, JavaClassDescriptor descriptor) {
if (descriptor.isPrimitive() || descriptor.isArray()) {
builder.withModifiers(PRIMITIVE_AND_ARRAY_TYPE_MODIFIERS);
/**
* See {@link Class#getModifiers()}
*/
private void addModifiersIfPossible(JavaClassBuilder builder, JavaClassDescriptor descriptor) {
if (descriptor.isPrimitive()) {
builder.withModifiers(PRIMITIVE_TYPE_MODIFIERS);
} else if (descriptor.isArray()) {
JavaClass elementType = getOrResolve(getElementType(descriptor).getFullyQualifiedClassName());
Set<JavaModifier> modifiers = ImmutableSet.<JavaModifier>builder()
.addAll(getVisibility(elementType))
.add(ABSTRACT, FINAL)
.build();
builder.withModifiers(modifiers);
}
}

private JavaClassDescriptor getElementType(JavaClassDescriptor descriptor) {
return descriptor.tryGetComponentType().map(this::getElementType).orElse(descriptor);
}

private Set<JavaModifier> getVisibility(JavaClass javaClass) {
return Sets.intersection(VISIBILITY_MODIFIERS, javaClass.getModifiers());
}

public Optional<JavaClass> getMethodReturnType(String declaringClassName, String methodName) {
return getMethodReturnType.getReturnType(declaringClassName, methodName);
}
Expand Down
61 changes: 61 additions & 0 deletions archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,58 @@ public void finds_multidimensional_array_type() {
assertThatType(twoDimArray.tryGetComponentType().get().tryGetComponentType().get()).isEqualTo(type);
}

@DataProvider
public static Object[][] data_imported_array_types_match_Reflection_API() {
@SuppressWarnings("unused")
class PublicClassArray {
String[] array;
}
@SuppressWarnings("unused")
class PublicClassMultiDimArray {
String[][] array;
}
@SuppressWarnings("unused")
class PrimitiveClassArray {
int[] array;
}
class LocalClass {
}
@SuppressWarnings("unused")
class LocalClassArray {
LocalClass[] array;
}
@SuppressWarnings("unused")
class PrivateClassArray {
SomePrivateClass[] array;
}
@SuppressWarnings("unused")
class ProtectedClassArray {
SomeProtectedClass[] array;
}
@SuppressWarnings("unused")
class InnerPackagePrivateClassArray {
SomePackagePrivateClass[] array;
}
return $$(
$(PublicClassArray.class, String[].class),
$(PublicClassMultiDimArray.class, String[][].class),
$(PrimitiveClassArray.class, int[].class),
$(LocalClassArray.class, LocalClass[].class),
$(PrivateClassArray.class, SomePrivateClass[].class),
$(ProtectedClassArray.class, SomeProtectedClass[].class),
$(InnerPackagePrivateClassArray.class, SomePackagePrivateClass[].class)
);
}

@Test
@UseDataProvider
public void test_imported_array_types_match_Reflection_API(Class<?> classWithArrayField, Class<?> expectedReflectionType) {
JavaClass classWithArray = new ClassFileImporter().importClass(classWithArrayField);
JavaClass arrayClassObject = classWithArray.getField("array").getRawType();

assertThatType(arrayClassObject).matches(expectedReflectionType);
}

@Test
public void erased_type_of_class_is_the_class_itself() {
class SimpleClass {
Expand Down Expand Up @@ -2652,4 +2704,13 @@ void annotationSelfReference() {
void throwableSelfReference() throws ClassWithSelfReferences {
}
}

private class SomePrivateClass {
}

class SomePackagePrivateClass {
}

protected class SomeProtectedClass {
}
}

0 comments on commit 3eaaeaf

Please sign in to comment.