Skip to content

Commit

Permalink
recognize empty records (#999)
Browse files Browse the repository at this point in the history 
As `ClassVisitor#visitRecordComponent` is not invoked for empty records
(without components), we better rely on the access flags provided to
`ClassVisitor#visit`.

resolves #998
  • Loading branch information
@codecholeric
codecholeric authored Nov 21, 2022
2 parents 7d41549 + 2cad383 commit 055f5c2
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 26 deletions.
19 changes: 18 additions & 1 deletion archunit-integration-test/src/test/java/com/tngtech/archunit/ImporterRules.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,33 @@
import com.tngtech.archunit.lang.ArchRule;

import static com.tngtech.archunit.ArchUnitArchitectureTest.THIRDPARTY_PACKAGE_IDENTIFIER;
import static com.tngtech.archunit.base.DescribedPredicate.doNot;
import static com.tngtech.archunit.base.DescribedPredicate.not;
import static com.tngtech.archunit.core.domain.JavaClass.Predicates.belongToAnyOf;
import static com.tngtech.archunit.core.domain.JavaClass.Predicates.resideOutsideOfPackage;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;

public class ImporterRules {

@ArchTest
public static final ArchRule domain_does_not_access_importer =
noClasses().that().resideInAPackage("..core.domain..")
.should().accessClassesThat(belong_to_the_import_context());
.should().dependOnClassesThat(belong_to_the_import_context());

@ArchTest
public static final ArchRule asm_is_only_used_in_importer_or_JavaClassDescriptor =
noClasses()
.that(
resideOutsideOfPackage("..core.importer..")
.and(doNot(belongToAnyOf(JavaClassDescriptor.class
// Conceptually there are also dependencies from JavaModifier to ASM.
// However, at the moment all those are inlined by the compiler (primitives).
// Whenever we get the chance to break the public API
// we should remove the dependencies from JavaModifier to ASM.
// Those dependencies crept in by accident at some point because the design was convenient.
)))
)
.should().dependOnClassesThat().resideInAPackage("org.objectweb..");

@ArchTest
public static final ArchRule ASM_type_is_only_accessed_within_JavaClassDescriptor_or_JavaClassDescriptorImporter =
Expand Down
24 changes: 18 additions & 6 deletions ...t/src/jdk16test/java/com/tngtech/archunit/core/importer/ClassFileImporterRecordsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,33 @@
import com.tngtech.archunit.core.domain.JavaConstructor;
import com.tngtech.archunit.core.domain.JavaField;
import com.tngtech.archunit.core.domain.JavaMethod;
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
import org.junit.Test;
import org.junit.runner.RunWith;

import static com.tngtech.archunit.testutil.Assertions.assertThat;
import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;

@RunWith(DataProviderRunner.class)
public class ClassFileImporterRecordsTest {

@Test
public void imports_simple_record() {
record RecordToImport(String component1, int component2) {
@DataProvider
public static Object[][] imports_record() {
record SimpleRecord(String component1, int component2) {
}
record EmptyRecord() {
}
return testForEach(SimpleRecord.class, EmptyRecord.class);
}

JavaClass javaClass = new ClassFileImporter().importClasses(RecordToImport.class, Record.class).get(RecordToImport.class);
@Test
@UseDataProvider
public void imports_record(Class<?> recordToImport) {
JavaClass javaClass = new ClassFileImporter().importClasses(recordToImport, Record.class).get(recordToImport);

assertThat(javaClass)
.matches(RecordToImport.class)
.matches(recordToImport)
.hasRawSuperclassMatching(Record.class)
.hasNoInterfaces()
.isInterface(false)
Expand Down
34 changes: 33 additions & 1 deletion archunit/src/main/java/com/tngtech/archunit/core/domain/JavaModifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.EnumSet;
import java.util.Set;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.tngtech.archunit.PublicAPI;
import org.objectweb.asm.Opcodes;
Expand Down Expand Up @@ -64,16 +65,47 @@ public enum JavaModifier {
this.asmAccessFlag = asmAccessFlag;
}

/**
* @deprecated This seems like an unnecessary API for users of ArchUnit, but limits us to do internal refactorings.
* If you think you need this API, please reach out to us on GitHub by creating an issue at
* <a href="https://github.com/TNG/ArchUnit/issues">https://github.com/TNG/ArchUnit/issues</a>.
* Otherwise, at some point in the future we will remove this API without any replacement.
*/
@Deprecated
@PublicAPI(usage = ACCESS)
public static Set<JavaModifier> getModifiersForClass(int asmAccess) {
return getModifiersFor(ApplicableType.CLASS, asmAccess);
Set<JavaModifier> modifiers = getModifiersFor(ApplicableType.CLASS, asmAccess);
boolean opCodeForRecordIsPresent = (asmAccess & Opcodes.ACC_RECORD) != 0;
if (opCodeForRecordIsPresent) {
// As records are implicitly static and final (compare JLS 8.10 Record Declarations),
// we ensure that those modifiers are always present. (asmAccess does not contain STATIC.)
return ImmutableSet.<JavaModifier>builder()
.addAll(modifiers)
.add(JavaModifier.STATIC, JavaModifier.FINAL)
.build();
}
return modifiers;
}

/**
* @deprecated This seems like an unnecessary API for users of ArchUnit, but limits us to do internal refactorings.
* If you think you need this API, please reach out to us on GitHub by creating an issue at
* <a href="https://github.com/TNG/ArchUnit/issues">https://github.com/TNG/ArchUnit/issues</a>.
* Otherwise, at some point in the future we will remove this API without any replacement.
*/
@Deprecated
@PublicAPI(usage = ACCESS)
public static Set<JavaModifier> getModifiersForField(int asmAccess) {
return getModifiersFor(ApplicableType.FIELD, asmAccess);
}

/**
* @deprecated This seems like an unnecessary API for users of ArchUnit, but limits us to do internal refactorings.
* If you think you need this API, please reach out to us on GitHub by creating an issue at
* <a href="https://github.com/TNG/ArchUnit/issues">https://github.com/TNG/ArchUnit/issues</a>.
* Otherwise, at some point in the future we will remove this API without any replacement.
*/
@Deprecated
@PublicAPI(usage = ACCESS)
public static Set<JavaModifier> getModifiersForMethod(int asmAccess) {
return getModifiersFor(ApplicableType.METHOD, asmAccess);
Expand Down
21 changes: 3 additions & 18 deletions archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.SetMultimap;
import com.google.common.primitives.Booleans;
import com.google.common.primitives.Bytes;
Expand Down Expand Up @@ -58,7 +57,6 @@
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.RecordComponentVisitor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -114,6 +112,7 @@ public void visit(int version, int access, String name, String signature, String
boolean opCodeForInterfaceIsPresent = (access & Opcodes.ACC_INTERFACE) != 0;
boolean opCodeForEnumIsPresent = (access & Opcodes.ACC_ENUM) != 0;
boolean opCodeForAnnotationIsPresent = (access & Opcodes.ACC_ANNOTATION) != 0;
boolean opCodeForRecordIsPresent = (access & Opcodes.ACC_RECORD) != 0;
Optional<String> superclassName = getSuperclassName(superName, opCodeForInterfaceIsPresent);
LOG.trace("Found superclass {} on class '{}'", superclassName.orElse(null), name);

Expand All @@ -123,7 +122,8 @@ public void visit(int version, int access, String name, String signature, String
.withInterface(opCodeForInterfaceIsPresent)
.withEnum(opCodeForEnumIsPresent)
.withAnnotation(opCodeForAnnotationIsPresent)
.withModifiers(JavaModifier.getModifiersForClass(access));
.withModifiers(JavaModifier.getModifiersForClass(access))
.withRecord(opCodeForRecordIsPresent);

className = descriptor.getFullyQualifiedClassName();
declarationHandler.onNewClass(className, superclassName, interfaceNames);
Expand Down Expand Up @@ -153,21 +153,6 @@ public void visitSource(String source, String debug) {
}
}

@Override
public RecordComponentVisitor visitRecordComponent(String name, String descriptor, String signature) {
javaClassBuilder.withRecord(true);

// Records are implicitly static and final (compare JLS 8.10 Record Declarations)
// Thus we ensure that those modifiers are always present (the access flag in visit(..) does not contain STATIC)
ImmutableSet<JavaModifier> recordModifiers = ImmutableSet.<JavaModifier>builder()
.addAll(javaClassBuilder.getModifiers())
.add(JavaModifier.STATIC, JavaModifier.FINAL)
.build();
javaClassBuilder.withModifiers(recordModifiers);

return super.visitRecordComponent(name, descriptor, signature);
}

@Override
public void visitInnerClass(String name, String outerName, String innerName, int access) {
if (importAborted()) {
Expand Down

0 comments on commit 055f5c2

Please sign in to comment.