Expression comparison for node sharing does not take into account parameter positions

[larrybehaviorlanguage]

The code below was tested with the latest version of the NRules develop branch (b1d9074 of December 26, 2020 at 3:29:35 PM PST).

Looking at ThreeFactOrBetaTestRule, this specific ordering and reference to the matched myNameFact and otherNameFact for some reason causes this rule not to fire.

using System;
using System.Linq;
using NRules.Fluent.Dsl;
using NRules.IntegrationTests.TestAssets;
using NRules.RuleModel;
using Xunit;

namespace NRules.IntegrationTests
{
    public class ThreeFactOrBetaTest : BaseRuleTestFixture
    {

        [Fact]
        public void ThreeFactOrBeta()
        {
            //Arrange
            var myNameFact = new NameFact { Name = "myName" };
            var otherNameFact = new NameFact { Name = "otherName" };
            var referenceFact = new ReferenceFact { ReferenceA = myNameFact, ReferenceB = otherNameFact };

            Session.Insert(myNameFact);
            Session.Insert(otherNameFact);
            Session.Insert(referenceFact);

            //Act
            Session.Fire();

            //Assert
            AssertFiredOnce();
        }


        protected override void SetUpRules()
        {
            SetUpRule<ThreeFactOrBetaTestRule>();
        }
        
        public class NameFact
        {
            public string Name { get; set; }
        }

        public class ReferenceFact
        {
            public NameFact ReferenceA { get; set; }
            public NameFact ReferenceB { get; set; }
        }   

        public class ThreeFactOrBetaTestRule : Rule
        {
            public Action<IContext> Action = ctx => { };

            public override void Define()
            {
                NameFact myNameFact = null;
                NameFact otherNameFact = null;
                ReferenceFact refFact = null;

            When()
                .Match<NameFact>(() => myNameFact, f => f.Name == "myName")
                .Match<NameFact>(() => otherNameFact, f => f.Name == "otherName")

                .Or(x => x
                    .Match<ReferenceFact>(() => refFact, rf => rf.ReferenceA == myNameFact, rf => rf.ReferenceB == myNameFact)
                    .Match<ReferenceFact>(() => refFact, rf => rf.ReferenceA == myNameFact, rf => rf.ReferenceB == otherNameFact)
                );
                ;

                Then()
                    .Do(ctx => Action(ctx));
            }
        }
    }
}

In testing, the following all alter or correct the issue:

• Reversing the order of the two clauses in the or group causes the rule fire, but it incorrectly fires twice
• Changing the last test in the first or clause from 'rf => rf.ReferenceB == myNameFact' to 'rf => rf.ReferenceB == null' causes the rule to run properly
• Adding a call to a boolean-returning method at the end of the second clause's final test causes the rule to properly fire once, as shown below:

public class ThreeFactOrBetaTestRule : Rule
        {
            public Action<IContext> Action = ctx => { };

            public bool ReturnsTrue() { return true; }

            public override void Define()
            {
                NameFact myNameFact = null;
                NameFact otherNameFact = null;
                ReferenceFact refFact = null;

            When()
                .Match<NameFact>(() => myNameFact, f => f.Name == "myName")
                .Match<NameFact>(() => otherNameFact, f => f.Name == "otherName")

                .Or(x => x
                    .Match<ReferenceFact>(() => refFact, rf => rf.ReferenceA == myNameFact, rf => rf.ReferenceB == null)
                    .Match<ReferenceFact>(() => refFact, rf => rf.ReferenceA == myNameFact, rf => rf.ReferenceB == otherNameFact && ReturnsTrue())
                );
                ;

                Then()
                    .Do(ctx => Action(ctx));
            }
        }

[snikolayev]

A rule with an OR clause is actually transformed into two rules. So, this issue is not just when using an OR clause, but can happen with any set of rules that share conditions and participate in expression comparison for node sharing.
The below two rules expose the issue:

    public class NodeSharingNonEquivalentTest : BaseRuleTestFixture
    {
        [Fact]
        public void Fire_FirstRuleMatchesFacts_OnlyFirstRuleFires()
        {
            //Arrange
            var myNameFact = new NameFact { Name = "myName" };
            var otherNameFact = new NameFact { Name = "otherName" };
            var referenceFact = new ReferenceFact { Reference = myNameFact };

            Session.Insert(myNameFact);
            Session.Insert(otherNameFact);
            Session.Insert(referenceFact);

            //Act
            Session.Fire();

            //Assert
            AssertFiredOnce<TestRule1>();
            AssertDidNotFire<TestRule2>();
        }
        
        [Fact]
        public void Fire_SecondRuleMatchesFacts_OnlySecondRuleFires()
        {
            //Arrange
            var myNameFact = new NameFact { Name = "myName" };
            var otherNameFact = new NameFact { Name = "otherName" };
            var referenceFact = new ReferenceFact { Reference = otherNameFact };

            Session.Insert(myNameFact);
            Session.Insert(otherNameFact);
            Session.Insert(referenceFact);

            //Act
            Session.Fire();

            //Assert
            AssertDidNotFire<TestRule1>();
            AssertFiredOnce<TestRule2>();
        }

        protected override void SetUpRules()
        {
            SetUpRule<TestRule1>();
            SetUpRule<TestRule2>();
        }
        
        public class NameFact
        {
            public string Name { get; set; }
        }

        public class ReferenceFact
        {
            public NameFact Reference { get; set; }
        }   

        public class TestRule1 : Rule
        {
            public override void Define()
            {
                NameFact myNameFact1 = null;
                NameFact otherNameFact1 = null;
                ReferenceFact refFact1 = null;

                When()
                    .Match(() => myNameFact1, f => f.Name == "myName")
                    .Match(() => otherNameFact1, f => f.Name == "otherName")
                    .Match(() => refFact1, rf => rf.Reference == myNameFact1);

                Then()
                    .Do(ctx => ctx.NoOp());
            }
        }

        public class TestRule2 : Rule
        {
            public override void Define()
            {
                NameFact myNameFact2 = null;
                NameFact otherNameFact2 = null;
                ReferenceFact refFact2 = null;

                When()
                    .Match(() => myNameFact2, f => f.Name == "myName")
                    .Match(() => otherNameFact2, f => f.Name == "otherName")
                    .Match(() => refFact2, rf => rf.Reference == otherNameFact2);

                Then()
                    .Do(ctx => ctx.NoOp());
            }
        }
    }

This results in this rete graph where all nodes for these two rules got fully shared, even though the reference expressions are referencing different instances of the NameFact.

This is what the resulting graph should be:

In this case the first two matches are fully shared, but only the alpha network is shared for the third match, and beta sub-graph is not shared as join expressions are different.

The root issue here is that expression comparison does not account for parameter positions in the match tuple, when deciding which nodes to share. In this case, given there are multiple instances of the NameFact matched by the rule, the comparison fails to distinguish between these different instances.