Skip to content

Commit

Permalink
add rule not to use ASM from (most) domain classes
Browse files Browse the repository at this point in the history
It was a conscious decision to allow the access from `JavaClassDescriptor`, because using the ASM infrastructure there is just very convenient and duplicating the logic into ArchUnit seems like more pain than gain for now (since this one isolated place would also be easy to fix in the future if need arises). However, there are also some conceptual dependencies from `JavaModifier` to ASM which were accidental, because it was just a convenient design to code those opcodes into the enum constants. Furthermore, we can't easily fix this anymore now, because there is public API in `JavaModifier` that allows creating modifiers from "ASM access" (which already shows the design flaw here). Likely the idea was that this would allow to create `JavaModifier`s from Reflection class objects, etc., but in hindsight I think this makes no sense. I would also suspect that actually no user ever uses this API. And it breaks the principle that users never create ArchUnit domain objects (which is a guideline for all other places). Thus, we deprecate these methods for now so we can remove them at some point in the future.

Signed-off-by: Peter Gafert <[email protected]>
  • Loading branch information
codecholeric committed Nov 21, 2022
1 parent 7d41549 commit 165dbbd
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
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
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,37 @@ 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);
}

/**
* @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

0 comments on commit 165dbbd

Please sign in to comment.