Fix And/Or not being commutative when combining rules

ArchUnitNET/Fluent/ArchRule.cs

@@ -23,14 +23,7 @@ public ArchRule<TRuleType> 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<EvaluationResult> Evaluate(Architecture architecture)
@@ -58,22 +51,22 @@ public IEnumerable<EvaluationResult> 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);
         }
     }
 }

ArchUnitNET/Fluent/ArchRuleCreator.cs

@@ -25,7 +25,10 @@ public ArchRuleCreator(BasicObjectProvider<TRuleType> 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<EvaluationResult> Evaluate(Architecture architecture)

ArchUnitNET/Fluent/Slices/SliceRule.cs

@@ -26,22 +26,22 @@ public IEnumerable<EvaluationResult> 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);
         }
     }
 }

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 { }