Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ uses semantic versioning for public releases.

### Fixed

- preserve ordinary member-owned dependencies in type and package cycle checks when synthetic
edges are ignored (the default), and exclude synthetic member evidence correctly; checks may
now report real cycles that `0.1.0` silently missed;
- accept documented dotted CLI rule IDs while still rejecting unknown rule fields;
- retain input and parser failures in CLI/direct `CliAnalyzer` results so corrupt archives or
malformed classes cannot silently produce passing partial checks; and
Expand Down
7 changes: 7 additions & 0 deletions docs/USER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ These entry points are all under the
The generated Javadocs list the overloads and option values. Prefer a narrow selector and an
explicit external-dependency policy over broad patterns that happen to pass today.

Type and package cycle rules ignore evidence from synthetic types, synthetic members, and bridge
methods by default. Ordinary member-owned dependencies remain part of the graph, including an edge
that has both ordinary and synthetic evidence. Evidence without an owning member is retained unless
the edge itself originates from a synthetic type or member. Use
`CycleRuleOptions.withSyntheticEdges(SyntheticEdgePolicy.INCLUDE)` to include generated evidence;
type-use dependencies such as field types are controlled separately and are included by default.

## Layers, slices, modules, and PlantUML

Layers and slices are projections over the imported dependency graph. Define membership first,
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/dev/archunitjava/rules/CycleRules.java
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,9 @@ private static DependencyGraph withoutSyntheticEvidence(
&& syntheticMembers.contains(member));
edge.evidence().stream()
.filter(evidence -> !syntheticOrigin
&& evidence.ownerMember().map(syntheticMembers::contains).orElse(true))
&& evidence.ownerMember()
.map(member -> !syntheticMembers.contains(member))
.orElse(true))
.forEach(evidence -> result.addDependency(
edge.origin(), edge.target(), edge.kind(), evidence));
}
Expand Down
173 changes: 173 additions & 0 deletions src/test/java/dev/archunitjava/rules/CycleRulesBytecodeTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
package dev.archunitjava.rules;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import dev.archunitjava.cli.CliAnalysisResult;
import dev.archunitjava.cli.CliAnalyzer;
import dev.archunitjava.cli.CliConfigurationLoader;
import dev.archunitjava.graph.DependencyEvidence;
import dev.archunitjava.graph.DependencyKind;
import dev.archunitjava.graph.MemberId;
import dev.archunitjava.graph.TypeId;
import dev.archunitjava.model.JavaMemberModifier;
import dev.archunitjava.pattern.JavaPattern;
import dev.archunitjava.pattern.PatternDomain;
import dev.archunitjava.result.RuleStatus;
import dev.archunitjava.selector.PackageSelector;
import dev.archunitjava.selector.TypeSelector;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

class CycleRulesBytecodeTest {
private static final String PREFIX = "dev.archunitjava.rules.fixtures.cycles.";
private static final TypeId LEFT = TypeId.ofBinaryName(PREFIX + "left.CycleLeft");
private static final TypeId RIGHT = TypeId.ofBinaryName(PREFIX + "right.CycleRight");

@TempDir Path temporaryDirectory;
private CliAnalysisResult analysis;

@BeforeEach
void importJavacFixturesWithoutLoadingTargetClasses() throws IOException {
for (String name : List.of(LEFT.binaryName(), RIGHT.binaryName(),
LEFT.binaryName() + "$OneWay", RIGHT.binaryName() + "$Leaf")) {
String resource = name.replace('.', '/') + ".class";
Path target = temporaryDirectory.resolve("classes").resolve(resource);
Files.createDirectories(target.getParent());
try (var input = CycleRulesBytecodeTest.class.getResourceAsStream("/" + resource)) {
assertNotNull(input, resource);
Files.copy(input, target);
}
}
Path configuration = temporaryDirectory.resolve("archunitjava.properties");
Files.writeString(configuration, """
schema=archunitjava.cli.v1
inputs=classes
rules=fixture
rule.fixture.domain=types
rule.fixture.mode=only
rule.fixture.origins=glob:dev.archunitjava.rules.fixtures.cycles.**
rule.fixture.targets=glob:dev.archunitjava.rules.fixtures.cycles.**
rule.fixture.external=ignore
""");
analysis = new CliAnalyzer().analyze(
CliConfigurationLoader.load(configuration, temporaryDirectory));
assertTrue(analysis.imports().model().classFileDiagnostics().isEmpty());
assertEquals(4, analysis.imports().model().types().size());
}

@Test
void defaultRulesDetectJavacTypeAndPackageCycles() {
TypeSelector selected = types(LEFT.binaryName(), RIGHT.binaryName());
assertEquals(RuleStatus.FAILED, CycleRules.typesAreAcyclic(
analysis.imports().model(), analysis.graph(), selected).check().status());
assertEquals(RuleStatus.FAILED, CycleRules.packagesAreAcyclic(
analysis.imports().model(), analysis.graph(), PackageSelector.all()).check().status());
}

@Test
void javacMethodCyclesAreDetectedUnderEverySyntheticPolicy() {
assertCyclesUnderEverySyntheticPolicy(DependencyKind.METHOD_CALL);
}

@Test
void javacFieldCyclesRespectTheIndependentTypeUsePolicy() {
assertCyclesUnderEverySyntheticPolicy(DependencyKind.FIELD_TYPE);
var noTypeUses = CycleRuleOptions.defaults().includingOnly(Set.of(DependencyKind.FIELD_TYPE))
.withTypeUseDependencies(TypeUseDependencyPolicy.IGNORE);
assertEquals(RuleStatus.PASSED, CycleRules.typesAreAcyclic(
analysis.imports().model(), analysis.graph(), types(LEFT.binaryName(), RIGHT.binaryName()),
noTypeUses).check().status());
}

@Test
void javacLambdaEvidenceIsFilteredWithoutRemovingOrdinaryCallsOnTheSameEdge() {
Set<MemberId> syntheticMembers = analysis.imports().model().types().stream()
.flatMap(type -> type.declaredMembers().stream()
.filter(member -> member.modifiers().contains(JavaMemberModifier.SYNTHETIC))
.map(member -> MemberId.of(TypeId.ofBinaryName(type.binaryName()),
member.name(), member.descriptor())))
.collect(Collectors.toSet());
assertFalse(syntheticMembers.isEmpty(), "javac must generate a lambda implementation member");
var calls = analysis.graph().edges().stream()
.filter(edge -> edge.origin().equals(LEFT) && edge.target().equals(RIGHT)
&& edge.kind() == DependencyKind.METHOD_CALL)
.flatMap(edge -> edge.evidence().stream()).toList();
assertTrue(calls.stream().anyMatch(evidence -> syntheticMembers.contains(
evidence.ownerMember().orElseThrow())));
assertTrue(calls.stream().anyMatch(evidence -> !syntheticMembers.contains(
evidence.ownerMember().orElseThrow())));

for (SyntheticEdgePolicy policy : SyntheticEdgePolicy.values()) {
var result = CycleRules.typesAreAcyclic(analysis.imports().model(), analysis.graph(),
types(LEFT.binaryName(), RIGHT.binaryName()), CycleRuleOptions.defaults()
.includingOnly(Set.of(DependencyKind.METHOD_CALL))
.withSyntheticEdges(policy)).check();
assertEquals(RuleStatus.FAILED, result.status());
Set<MemberId> owners = result.violations().getFirst().evidence().stream()
.map(DependencyEvidence::ownerMember).map(owner -> owner.orElseThrow())
.collect(Collectors.toSet());
assertTrue(owners.contains(MemberId.of(LEFT, "work", "()V")));
assertTrue(owners.contains(MemberId.of(RIGHT, "work", "()V")));
assertEquals(policy == SyntheticEdgePolicy.INCLUDE,
owners.stream().anyMatch(syntheticMembers::contains));
}
}

@Test
void javacOneWayDependenciesRemainAcyclicUnderEverySyntheticPolicy() {
TypeSelector selected = types(LEFT.binaryName() + "$OneWay", RIGHT.binaryName() + "$Leaf");
assertEquals(2, selected.selectFrom(analysis.imports().model()).selected().size());
Set<DependencyKind> importedKinds = analysis.graph().edges().stream()
.filter(edge -> edge.origin().equals(TypeId.ofBinaryName(LEFT.binaryName() + "$OneWay"))
&& edge.target().equals(TypeId.ofBinaryName(RIGHT.binaryName() + "$Leaf")))
.map(edge -> edge.kind()).collect(Collectors.toSet());
assertTrue(importedKinds.containsAll(Set.of(DependencyKind.METHOD_CALL, DependencyKind.FIELD_TYPE)));
assertEquals(RuleStatus.PASSED, CycleRules.typesAreAcyclic(
analysis.imports().model(), analysis.graph(), selected).check().status());
for (SyntheticEdgePolicy policy : SyntheticEdgePolicy.values()) {
var result = CycleRules.typesAreAcyclic(analysis.imports().model(), analysis.graph(),
selected, CycleRuleOptions.defaults().withSyntheticEdges(policy)).check();
assertEquals(RuleStatus.PASSED, result.status(), policy.name());
assertTrue(result.violations().isEmpty());
}
}

private void assertCyclesUnderEverySyntheticPolicy(DependencyKind kind) {
var edges = analysis.graph().edges().stream()
.filter(edge -> edge.kind() == kind)
.filter(edge -> Set.of(LEFT, RIGHT).contains(edge.origin())
&& Set.of(LEFT, RIGHT).contains(edge.target()))
.toList();
assertEquals(2, edges.size(), kind.name());
assertTrue(edges.stream().flatMap(edge -> edge.evidence().stream())
.allMatch(evidence -> evidence.ownerMember().isPresent()));
for (SyntheticEdgePolicy policy : SyntheticEdgePolicy.values()) {
var options = CycleRuleOptions.defaults().includingOnly(Set.of(kind)).withSyntheticEdges(policy);
var result = CycleRules.typesAreAcyclic(analysis.imports().model(), analysis.graph(),
types(LEFT.binaryName(), RIGHT.binaryName()), options).check();
assertEquals(RuleStatus.FAILED, result.status(), kind + "/" + policy);
assertEquals(1, result.violations().size());
assertFalse(result.violations().getFirst().evidence().isEmpty());
var packages = CycleRules.packagesAreAcyclic(analysis.imports().model(),
analysis.graph(), PackageSelector.all(), options).check();
assertEquals(RuleStatus.FAILED, packages.status(), kind + "/" + policy);
assertEquals(1, packages.violations().size());
}
}

private static TypeSelector types(String first, String second) {
return TypeSelector.anyOf(
TypeSelector.binaryName(JavaPattern.exact(PatternDomain.QUALIFIED_NAME, first)),
TypeSelector.binaryName(JavaPattern.exact(PatternDomain.QUALIFIED_NAME, second)));
}
}
92 changes: 90 additions & 2 deletions src/test/java/dev/archunitjava/rules/CycleRulesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ class CycleRulesTest {
private static final TypeId C = TypeId.ofBinaryName("app.C");
private static final TypeId D = TypeId.ofBinaryName("other.D");
private static final TypeId GENERATED = TypeId.ofBinaryName("app.Generated");
private static final MemberId A_WORK = MemberId.of(A, "work", "()V");
private static final MemberId A_BRIDGE = MemberId.of(A, "bridge", "()V");
private static final MemberId A_SYNTHETIC = MemberId.of(A, "generated", "()V");
private static final MemberId B_WORK = MemberId.of(B, "work", "()V");
private static final MemberId D_WORK = MemberId.of(D, "work", "()V");
private static final MemberId GENERATED_WORK = MemberId.of(GENERATED, "work", "()V");

@TempDir Path temporaryDirectory;
Expand Down Expand Up @@ -132,10 +135,91 @@ void typeUseAndSyntheticEdgesAreIndependentExplicitPolicies() {
.check().status());
}

@Test
void ordinaryMemberEvidenceIsRetainedUnderEverySyntheticPolicy() {
DependencyEvidence outward = evidence(A_WORK, 10);
DependencyEvidence inward = evidence(B_WORK, 20);
DependencyGraph graph = DependencyGraph.builder()
.addNode(A).addNode(B)
.addDependency(A, B, DependencyKind.METHOD_CALL, outward)
.addDependency(B, A, DependencyKind.METHOD_CALL, inward)
.build();

assertEquals(RuleStatus.FAILED,
CycleRules.typesAreAcyclic(model, graph, packages("app")).check().status());
for (SyntheticEdgePolicy policy : SyntheticEdgePolicy.values()) {
var result = CycleRules.typesAreAcyclic(model, graph, packages("app"),
CycleRuleOptions.defaults().withSyntheticEdges(policy)).check();
assertEquals(RuleStatus.FAILED, result.status(), policy.name());
assertEquals(1, result.violations().size());
assertEquals(List.of(outward, inward), result.violations().getFirst().evidence());
}
}

@Test
void syntheticMemberEvidenceCannotCreateAnOrdinaryTypeCycle() {
for (MemberId owner : List.of(A_BRIDGE, A_SYNTHETIC)) {
DependencyGraph graph = DependencyGraph.builder()
.addNode(A).addNode(B)
.addDependency(A, B, DependencyKind.METHOD_CALL, evidence(owner, 10))
.addDependency(B, A, DependencyKind.METHOD_CALL, evidence(null, 20))
.build();

assertEquals(RuleStatus.PASSED,
CycleRules.typesAreAcyclic(model, graph, packages("app")).check().status(),
owner.stableKey());
var included = CycleRules.typesAreAcyclic(model, graph, packages("app"),
CycleRuleOptions.defaults().withSyntheticEdges(SyntheticEdgePolicy.INCLUDE))
.check();
assertEquals(RuleStatus.FAILED, included.status());
assertEquals(1, included.violations().size());
}
}

@Test
void mixedEvidenceKeepsTheOrdinaryCycleAndDropsOnlySyntheticOwners() {
DependencyEvidence ordinary = evidence(A_WORK, 10);
DependencyEvidence synthetic = evidence(A_BRIDGE, 20);
DependencyEvidence reverse = evidence(B_WORK, 30);
DependencyGraph graph = DependencyGraph.builder()
.addNode(A).addNode(B)
.addDependency(A, B, DependencyKind.METHOD_CALL, ordinary)
.addDependency(A, B, DependencyKind.METHOD_CALL, synthetic)
.addDependency(B, A, DependencyKind.METHOD_CALL, reverse)
.build();

var ignored = CycleRules.typesAreAcyclic(model, graph, packages("app")).check();
assertEquals(RuleStatus.FAILED, ignored.status());
assertEquals(List.of(ordinary, reverse), ignored.violations().getFirst().evidence());
var included = CycleRules.typesAreAcyclic(model, graph, packages("app"),
CycleRuleOptions.defaults().withSyntheticEdges(SyntheticEdgePolicy.INCLUDE)).check();
assertEquals(RuleStatus.FAILED, included.status());
assertEquals(List.of(ordinary, synthetic, reverse).stream().sorted().toList(),
included.violations().getFirst().evidence());
}

@Test
void syntheticOriginsAreIgnoredEvenWithoutEvidenceOwners() {
for (var origin : List.of(GENERATED, A_BRIDGE)) {
DependencyGraph graph = DependencyGraph.builder()
.addNode(origin).addNode(B)
.addDependency(origin, B, DependencyKind.METHOD_CALL, evidence(null, 10))
.addDependency(B, origin, DependencyKind.METHOD_CALL, evidence(null, 20))
.build();

assertEquals(RuleStatus.PASSED,
CycleRules.typesAreAcyclic(model, graph, packages("app")).check().status());
assertEquals(RuleStatus.FAILED,
CycleRules.typesAreAcyclic(model, graph, packages("app"),
CycleRuleOptions.defaults().withSyntheticEdges(SyntheticEdgePolicy.INCLUDE))
.check().status());
}
}

@Test
void packageProjectionIgnoresIntraPackageEdgesAndRetainsUnderlyingEvidence() {
DependencyEvidence outward = evidence(null, 50);
DependencyEvidence inward = evidence(null, 60);
DependencyEvidence outward = evidence(A_WORK, 50);
DependencyEvidence inward = evidence(D_WORK, 60);
DependencyGraph graph = DependencyGraph.builder()
.addNode(A).addNode(B).addNode(D)
.addDependency(A, B, DependencyKind.METHOD_CALL, evidence(null, 40))
Expand Down Expand Up @@ -182,6 +266,10 @@ private static byte[] type(String binaryName, boolean syntheticBridge) {
"bridge", MethodTypeDesc.of(ClassDesc.ofDescriptor("V")),
ClassFile.ACC_PUBLIC | ClassFile.ACC_SYNTHETIC | ClassFile.ACC_BRIDGE,
code -> code.return_());
builder.withMethodBody(
"generated", MethodTypeDesc.of(ClassDesc.ofDescriptor("V")),
ClassFile.ACC_PRIVATE | ClassFile.ACC_SYNTHETIC,
code -> code.return_());
}
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package dev.archunitjava.rules.fixtures.cycles.left;

import dev.archunitjava.rules.fixtures.cycles.right.CycleRight;

/** Compiled by the test build; imported as bytes, never loaded or executed by the analyzer. */
public final class CycleLeft {
private CycleRight right;

public void work() {
right.work();
}

public Runnable deferred() {
return () -> right.work();
}

public static final class OneWay {
private CycleRight.Leaf leaf;

public void work() {
leaf.work();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package dev.archunitjava.rules.fixtures.cycles.right;

import dev.archunitjava.rules.fixtures.cycles.left.CycleLeft;

/** Compiled by the test build; imported as bytes, never loaded or executed by the analyzer. */
public final class CycleRight {
private CycleLeft left;

public void work() {
left.work();
}

public static final class Leaf {
public void work() {}
}
}
Loading