diff --git a/ArchUnitNET/Fluent/ArchRule.cs b/ArchUnitNET/Fluent/ArchRule.cs index 8aa046858..6235bf4e5 100644 --- a/ArchUnitNET/Fluent/ArchRule.cs +++ b/ArchUnitNET/Fluent/ArchRule.cs @@ -23,14 +23,7 @@ public ArchRule WithoutRequiringPositiveResults() public bool HasNoViolations(Architecture architecture) { - if (_ruleCreator.RequirePositiveResults) - { - return Evaluate(architecture).All(e => e.Passed); - } - else - { - return _ruleCreator.HasNoViolations(architecture); - } + return _ruleCreator.HasNoViolations(architecture); } public IEnumerable Evaluate(Architecture architecture) @@ -58,22 +51,22 @@ public IEnumerable Evaluate(Architecture architecture) public CombinedArchRuleDefinition And() { - return new CombinedArchRuleDefinition(_ruleCreator, LogicalConjunctionDefinition.And); + return new CombinedArchRuleDefinition(this, LogicalConjunctionDefinition.And); } public CombinedArchRuleDefinition Or() { - return new CombinedArchRuleDefinition(_ruleCreator, LogicalConjunctionDefinition.Or); + return new CombinedArchRuleDefinition(this, LogicalConjunctionDefinition.Or); } public IArchRule And(IArchRule archRule) { - return new CombinedArchRule(_ruleCreator, LogicalConjunctionDefinition.And, archRule); + return new CombinedArchRule(this, LogicalConjunctionDefinition.And, archRule); } public IArchRule Or(IArchRule archRule) { - return new CombinedArchRule(_ruleCreator, LogicalConjunctionDefinition.Or, archRule); + return new CombinedArchRule(this, LogicalConjunctionDefinition.Or, archRule); } } } diff --git a/ArchUnitNET/Fluent/ArchRuleCreator.cs b/ArchUnitNET/Fluent/ArchRuleCreator.cs index 68eae4f84..0e7cc170e 100644 --- a/ArchUnitNET/Fluent/ArchRuleCreator.cs +++ b/ArchUnitNET/Fluent/ArchRuleCreator.cs @@ -25,7 +25,10 @@ public ArchRuleCreator(BasicObjectProvider basicObjectProvider) public bool HasNoViolations(Architecture architecture) { - return HasNoViolations(GetAnalyzedObjects(architecture), architecture); + var objects = GetAnalyzedObjects(architecture).ToList(); + if (RequirePositiveResults && objects.Count == 0) + return false; + return HasNoViolations(objects, architecture); } public IEnumerable Evaluate(Architecture architecture) diff --git a/ArchUnitNET/Fluent/Slices/SliceRule.cs b/ArchUnitNET/Fluent/Slices/SliceRule.cs index 14d6d8727..1c364459e 100644 --- a/ArchUnitNET/Fluent/Slices/SliceRule.cs +++ b/ArchUnitNET/Fluent/Slices/SliceRule.cs @@ -26,22 +26,22 @@ public IEnumerable Evaluate(Architecture architecture) public CombinedArchRuleDefinition And() { - return new CombinedArchRuleDefinition(_ruleCreator, LogicalConjunctionDefinition.And); + return new CombinedArchRuleDefinition(this, LogicalConjunctionDefinition.And); } public CombinedArchRuleDefinition Or() { - return new CombinedArchRuleDefinition(_ruleCreator, LogicalConjunctionDefinition.Or); + return new CombinedArchRuleDefinition(this, LogicalConjunctionDefinition.Or); } public IArchRule And(IArchRule archRule) { - return new CombinedArchRule(_ruleCreator, LogicalConjunctionDefinition.And, archRule); + return new CombinedArchRule(this, LogicalConjunctionDefinition.And, archRule); } public IArchRule Or(IArchRule archRule) { - return new CombinedArchRule(_ruleCreator, LogicalConjunctionDefinition.Or, archRule); + return new CombinedArchRule(this, LogicalConjunctionDefinition.Or, archRule); } } } diff --git a/ArchUnitNETTests/Fluent/Syntax/Elements/LogicalConjunctionTests.cs b/ArchUnitNETTests/Fluent/Syntax/Elements/LogicalConjunctionTests.cs index bbc6093f6..f0d1c3d6f 100644 --- a/ArchUnitNETTests/Fluent/Syntax/Elements/LogicalConjunctionTests.cs +++ b/ArchUnitNETTests/Fluent/Syntax/Elements/LogicalConjunctionTests.cs @@ -680,6 +680,156 @@ public void OrTest() Assert.True(otherCondition2OrThisCondition2.HasNoViolations(Architecture)); Assert.True(falseThisCondition1OrFalseThisCondition2.HasNoViolations(Architecture)); } + + [Fact] + public void Or_WithArchRule_IsCommutative() + { + // TT + Assert.True(ThisClassExists.Or(OtherCondition1).HasNoViolations(Architecture)); + Assert.True(OtherCondition1.Or(ThisClassExists).HasNoViolations(Architecture)); + + // TF / FT + Assert.True(ThisClassExists.Or(ThisClassDoesNotExist).HasNoViolations(Architecture)); + Assert.True(ThisClassDoesNotExist.Or(ThisClassExists).HasNoViolations(Architecture)); + + // FF + Assert.False( + ThisClassDoesNotExist.Or(FalseThisShouldCondition1).HasNoViolations(Architecture) + ); + Assert.False( + FalseThisShouldCondition1.Or(ThisClassDoesNotExist).HasNoViolations(Architecture) + ); + + // Empty-predicate rule (no matching objects): counts as failing with RequirePositiveResults + var emptyRule = Classes() + .That() + .HaveName("NotTheNameOfAnyObject") + .Should() + .Be(ThisClass); + Assert.False(emptyRule.HasNoViolations(Architecture)); + Assert.False(emptyRule.Or(ThisClassDoesNotExist).HasNoViolations(Architecture)); + Assert.False(ThisClassDoesNotExist.Or(emptyRule).HasNoViolations(Architecture)); + } + + [Fact] + public void And_WithArchRule_IsCommutative() + { + // TT + Assert.True(ThisClassExists.And(OtherCondition1).HasNoViolations(Architecture)); + Assert.True(OtherCondition1.And(ThisClassExists).HasNoViolations(Architecture)); + + // TF / FT + Assert.False(ThisClassExists.And(ThisClassDoesNotExist).HasNoViolations(Architecture)); + Assert.False(ThisClassDoesNotExist.And(ThisClassExists).HasNoViolations(Architecture)); + + // FF + Assert.False( + ThisClassDoesNotExist.And(FalseThisShouldCondition1).HasNoViolations(Architecture) + ); + Assert.False( + FalseThisShouldCondition1.And(ThisClassDoesNotExist).HasNoViolations(Architecture) + ); + + // Empty-predicate rule (no matching objects): counts as failing with RequirePositiveResults + var emptyRule = Classes() + .That() + .HaveName("NotTheNameOfAnyObject") + .Should() + .Be(ThisClass); + Assert.False(emptyRule.HasNoViolations(Architecture)); + Assert.False(emptyRule.And(ThisClassExists).HasNoViolations(Architecture)); + Assert.False(ThisClassExists.And(emptyRule).HasNoViolations(Architecture)); + } + + // rule.Or(other) and rule.Or().other must give the same verdict + [Fact] + public void Or_FluentAndDirectForms_AreConsistent() + { + // FT: first fails, second passes + Assert.True(ThisClassDoesNotExist.Or(OtherCondition1).HasNoViolations(Architecture)); + Assert.True( + ThisClassDoesNotExist + .Or() + .Classes() + .That() + .Are(OtherClass) + .Should() + .Be(OtherClass) + .HasNoViolations(Architecture) + ); + + // TF: first passes, second fails + Assert.True(OtherCondition1.Or(ThisClassDoesNotExist).HasNoViolations(Architecture)); + Assert.True( + OtherCondition1 + .Or() + .Classes() + .That() + .Are(ThisClass) + .Should() + .NotBe(ThisClass) + .HasNoViolations(Architecture) + ); + + // FF + Assert.False( + ThisClassDoesNotExist.Or(FalseThisShouldCondition1).HasNoViolations(Architecture) + ); + Assert.False( + ThisClassDoesNotExist + .Or() + .Classes() + .That() + .Are(ThisClass) + .Should() + .NotBe(ThisClass) + .HasNoViolations(Architecture) + ); + } + + // rule.And(other) and rule.And().other must give the same verdict + [Fact] + public void And_FluentAndDirectForms_AreConsistent() + { + // TT + Assert.True(ThisClassExists.And(OtherCondition1).HasNoViolations(Architecture)); + Assert.True( + ThisClassExists + .And() + .Classes() + .That() + .Are(OtherClass) + .Should() + .Be(OtherClass) + .HasNoViolations(Architecture) + ); + + // TF + Assert.False(ThisClassExists.And(ThisClassDoesNotExist).HasNoViolations(Architecture)); + Assert.False( + ThisClassExists + .And() + .Classes() + .That() + .Are(ThisClass) + .Should() + .NotBe(ThisClass) + .HasNoViolations(Architecture) + ); + + // FT + Assert.False(ThisClassDoesNotExist.And(OtherCondition1).HasNoViolations(Architecture)); + Assert.False( + ThisClassDoesNotExist + .And() + .Classes() + .That() + .Are(OtherClass) + .Should() + .Be(OtherClass) + .HasNoViolations(Architecture) + ); + } } internal class OtherClassForLogicalConjunctionTest { }