1 / Fix issues with pattern matching for Rubi (#1176)

# Overview

In this PR, fixes for pattern matching, esp. those that concern ``Rubi`
``, are collected.

# Related issues
- #1172 
- #1173 

# Failing patterns
- Pattern `a_. + b_. x_` does not match `1 + 2 x`
- Likely fixed by
aravindh-krishnamoorthy@85ad0f5
- Pattern `x_.^m_.` does not match `x`
- Likely fixed by
aravindh-krishnamoorthy@c1999ce
+
aravindh-krishnamoorthy@e1dee54
- Pattern `a_. + b_.` does not match `x`
- Crash: `mathics/core/pattern.py` -> `assert new_pattern is not None`
- Likely fixed by
aravindh-krishnamoorthy@0a89945
- ~~Pattern `x_Integer + y_Integer z_.` for `1 + 2 x`~~ (Not directly
relevant for Rubi)
- ~See
#1176 (comment)
- ~Pattern with conditional within `With`: `g[x_] := With[{}, x /;
False] /; True` matches for `g[2]`~
    - ~Must not match.~

(Fixes for items struck out above are moved to the next PR.)

# Final review

# Tests
- [X] Tests for
aravindh-krishnamoorthy@85ad0f5
- [X] Tests for
aravindh-krishnamoorthy@c1999ce
+
aravindh-krishnamoorthy@e1dee54
- [X] Tests for
aravindh-krishnamoorthy@0a89945

# Documentation

N/A

Author: aravindh-krishnamoorthy

mathics/core/pattern.py

@@ -701,7 +701,12 @@ def match_expression_with_one_identity(
             isinstance(pat_elem, PatternObject)
             and pat_elem.get_head() == SymbolOptional
         ):
-            if len(pat_elem.elements) == 2:
+            if optionals:
+                # A default pattern already exists
+                # Do not use the second one
+                if new_pattern is None:
+                    new_pattern = pat_elem
+            elif len(pat_elem.elements) == 2:
                 pat, value = pat_elem.elements
                 if isinstance(pat, Pattern):
                     key = pat.elements[0].atom.name  # type: ignore[attr-defined]
@@ -724,8 +729,12 @@ def match_expression_with_one_identity(
                 result = defaultvalue_expr.evaluate(evaluation)
                 assert result is not None
                 if result.sameQ(defaultvalue_expr):
-                    return
-                optionals[key] = result
+                    if new_pattern is None:
+                        # The optional pattern has no default value
+                        # for the given position
+                        new_pattern = pat_elem
+                else:
+                    optionals[key] = result
             else:
                 return
         elif new_pattern is not None:
@@ -757,6 +766,8 @@ def match_expression_with_one_identity(
     del parms["attributes"]
     assert new_pattern is not None
     new_pattern.match(expression=expression, pattern_context=parms)
+    for optional in optionals:
+        vars_dict.pop(optional)
 
 
 def basic_match_expression(

test/builtin/test_testing_expressions.py

@@ -135,3 +135,45 @@ def test_private_doctests_numerical_properties(str_expr, msgs, str_expected, fai
         failure_message=fail_msg,
         expected_messages=msgs,
     )
+
+
+@pytest.mark.parametrize(
+    ("str_expr", "msgs", "str_expected", "fail_msg"),
+    [
+        # Two default arguments (linear)
+        ("MatchQ[1, a_.+b_.*x_]", None, "True", None),
+        ("MatchQ[x, a_.+b_.*x_]", None, "True", None),
+        ("MatchQ[2*x, a_.+b_.*x_]", None, "True", None),
+        ("MatchQ[1+x, a_.+b_.*x_]", None, "True", None),
+        ("MatchQ[1+2*x, a_.+b_.*x_]", None, "True", None),
+        # Default argument (power)
+        ("MatchQ[1, x_^m_.]", None, "True", None),
+        ("MatchQ[x, x_^m_.]", None, "True", None),
+        ("MatchQ[x^1, x_^m_.]", None, "True", None),
+        ("MatchQ[x^2, x_^m_.]", None, "True", None),
+        # Two default arguments (power)
+        ("MatchQ[1, x_.^m_.]", None, "True", None),
+        ("MatchQ[x, x_.^m_.]", None, "True", None),
+        ("MatchQ[x^1, x_.^m_.]", None, "True", None),
+        ("MatchQ[x^2, x_.^m_.]", None, "True", None),
+        # Two default arguments (no non-head)
+        ("MatchQ[1, a_.+b_.]", None, "True", None),
+        ("MatchQ[x, a_.+b_.]", None, "True", None),
+        ("MatchQ[1+x, a_.+b_.]", None, "True", None),
+        ("MatchQ[1+2*x, a_.+b_.]", None, "True", None),
+        ("MatchQ[1, a_.*b_.]", None, "True", None),
+        ("MatchQ[x, a_.*b_.]", None, "True", None),
+        ("MatchQ[2*x, a_.*b_.]", None, "True", None),
+    ],
+)
+def test_matchq(str_expr, msgs, str_expected, fail_msg):
+    """text_expressions.matchq"""
+    check_evaluation(
+        str_expr,
+        str_expected,
+        to_string_expr=True,
+        to_string_expected=True,
+        hold_expected=True,
+        failure_message=fail_msg,
+        expected_messages=msgs,
+    )

test/core/test_rules.py

@@ -184,3 +184,48 @@ def test_flat_on_rules(str_expr, str_expected, msg):
 @pytest.mark.xfail
 def test_default_optional_on_rules(str_expr, str_expected, msg):
     check_evaluation(str_expr, str_expected, failure_message=msg)
+
+
+@pytest.mark.parametrize(
+    ("str_expr", "msgs", "str_expected", "fail_msg"),
+    [
+        # Two default arguments (linear)
+        ("rule=A[a_.+B[b_.*x_]]->{a,b,x};", None, "Null", None),
+        ("A[B[1]] /. rule", None, "{0, 1, 1}", None),
+        ("A[B[x]] /. rule", None, "{0, 1, x}", None),
+        ("A[B[2*x]] /. rule", None, "{0, x, 2}", None),
+        ("A[1+B[x]] /. rule", None, "{1, 1, x}", None),
+        ("A[1+B[2*x]] /. rule", None, "{1, x, 2}", None),
+        # Default argument (power)
+        ("rule=A[x_^n_.]->{x,n};", None, "Null", None),
+        ("A[1] /. rule", None, "{1, 1}", None),
+        ("A[x] /. rule", None, "{x, 1}", None),
+        ("A[x^1] /. rule", None, "{x, 1}", None),
+        ("A[x^2] /. rule", None, "{x, 2}", None),
+        # Two default arguments (power)
+        ("rule=A[x_.^n_.]->{x,n};", None, "Null", None),
+        ("A[] /. rule", None, "A[]", None),
+        ("A[1] /. rule", None, "{1, 1}", None),
+        ("A[x] /. rule", None, "{x, 1}", None),
+        ("A[x^1] /. rule", None, "{x, 1}", None),
+        ("A[x^2] /. rule", None, "{x, 2}", None),
+        # Two default arguments (no non-head)
+        ("rule=A[a_. + B[b_.*x_.]]->{a,b,x};", None, "Null", None),
+        ("A[B[]] /. rule", None, "A[B[]]", None),
+        ("A[B[1]] /. rule", None, "{0, 1, 1}", None),
+        ("A[B[x]] /. rule", None, "{0, 1, x}", None),
+        ("A[1 + B[x]] /. rule", None, "{1, 1, x}", None),
+        ("A[1 + B[2*x]] /. rule", None, "{1, 2, x}", None),
+    ],
+)
+def test_pattern_rules(str_expr, msgs, str_expected, fail_msg):
+    """pattern_rules"""
+    check_evaluation(
+        str_expr,
+        str_expected,
+        to_string_expr=True,
+        to_string_expected=True,
+        hold_expected=True,
+        failure_message=fail_msg,
+        expected_messages=msgs,
+    )

test/eval/test_patterns.py

@@ -0,0 +1,57 @@
+# -*- coding: utf-8 -*-
+"""
+Unit tests for mathics.eval.patterns
+"""
+
+from test.helper import session
+
+import pytest
+
+from mathics.core.definitions import Definitions
+from mathics.core.parser import MathicsSingleLineFeeder, parse
+from mathics.core.pattern import ExpressionPattern
+from mathics.eval.patterns import Matcher
+
+# Preload the Mathics definitions
+defintions = Definitions(True)
+
+
+def check_pattern(str_expr, str_pattern):
+    expr = parse(defintions, MathicsSingleLineFeeder(str_expr))
+    pattern = ExpressionPattern(parse(defintions, MathicsSingleLineFeeder(str_pattern)))
+    ret = Matcher(pattern, session.evaluation).match(expr, session.evaluation)
+    assert ret == True
+
+
+@pytest.mark.parametrize(
+    ("str_expr", "str_pattern"),
+    [
+        # Two default arguments (linear)
+        ("1", "a_.+b_.*x_"),
+        ("x", "a_.+b_.*x_"),
+        ("2*x", "a_.+b_.*x_"),
+        ("1+x", "a_.+b_.*x_"),
+        ("1+2*x", "a_.+b_.*x_"),
+        # Default argument (power)
+        ("1", "x_^m_."),
+        ("x", "x_^m_."),
+        ("x^1", "x_^m_."),
+        ("x^2", "x_^m_."),
+        # Two default arguments (power)
+        ("1", "x_.^m_."),
+        ("x", "x_.^m_."),
+        ("x^1", "x_.^m_."),
+        ("x^2", "x_.^m_."),
+        # Two default arguments (no non-head)
+        ("1", "a_.+b_."),
+        ("x", "a_.+b_."),
+        ("1+x", "a_.+b_."),
+        ("1+2*x", "a_.+b_."),
+        ("1", "a_.*b_."),
+        ("x", "a_.*b_."),
+        ("2*x", "a_.*b_."),
+    ],
+)
+def test_eval_patterns(str_expr, str_pattern):
+    """eval_patterns"""
+    check_pattern(str_expr, str_pattern)