Correct LHS evaluation in SetDelayed assignment (#603)

* improving clarity in Builtin.contribute

* adding comments. adding tests

* fix test for SetDelayed. Move special case for Set with LHS a list away from the conditional.

* more on modularize assignment. assign_elementary->assign

* fixing set_eval

* adding tests for OneIdentity

* fix OneIdentity

* remove comment

* fix pytest

* fix a bug that makes that URLSave always fails

* fix WriteString standard output

* catch not known attributes in ClearAttributes and SetAttributes

* Update 'attributes' for current standards

`mathics.builtin.attributes`:

* apply -> eval
* Add WMA links add
* Class names ordered alphabetically
* Hard breaks in docstring removed
* Some incorrect references to "leaves" changed to "attributes"

`mathics.core.attributes`:

* Add short comments above flag value

* adding comments. adding tests

* fix test for SetDelayed. Move special case for Set with LHS a list away from the conditional.

* more on modularize assignment. assign_elementary->assign

* Handle optional with a first element that is not a Pattern[]

* Update examples in OneIdentity

* More pervasive use of Symbols

symbols.py:
   system_symbols() -> symbol_set(); "systems_symbols" name is too close
Symbol( System`  to module systemsymbols. Also, we now require symbols as parameters),
   not strings.

systemsymbols.py: more system symbols

* Pattern_create -> Pattern.create

It appears this was originally Pattern.create. I suspect due to bad
modularity and a lack of understandig Python that an import could be added inside the
routine, this static method got moved outside of the class.

Later on, the modularity was fixed, but the hack persisted. These kinds
of code smells side effects of poor communication.

* Add function signature; straighten import issue

* Put test_rules_patterns tests where they belong

* Add note to add skipped example as a doctest ...

When it gets fixed.

* Changes suggested in PR review

Move core-like assignment interals out of builtins and into core.
(We may want to split up core more in the future though)

Better sort long list of assignment methods alphabetically

Some small RstT tagging in a docstring

Remove nonexistent word "evaluable"

Add yet another type annotation to a signature

Make test assert failure messages unique

* Move ASSIGNMENT_FUNCTION_MAP into core

Co-authored-by: R. Bernstein <[email protected]>
Co-authored-by: rocky <[email protected]>

mathics/builtin/assignments/assignment.py

@@ -4,8 +4,14 @@
 """
 
 
-from mathics.builtin.assignments.internals import _SetOperator
 from mathics.builtin.base import BinaryOperator, Builtin
+from mathics.core.assignment import (
+    ASSIGNMENT_FUNCTION_MAP,
+    AssignmentException,
+    assign_store_rules_by_tag,
+    normalize_lhs,
+)
+
 from mathics.core.attributes import (
     A_HOLD_ALL,
     A_HOLD_FIRST,
@@ -18,6 +24,39 @@
 from mathics.core.systemsymbols import SymbolFailed
 
 
+class _SetOperator:
+    """
+    This is the base class for assignment Builtin operators.
+
+    Special cases are determined by the head of the expression. Then
+    they are processed by specific routines, which are poke from
+    the ``ASSIGNMENT_FUNCTION_MAP`` dict.
+    """
+
+    # FIXME:
+    # Assigment is determined by the LHS.
+    # Are there a larger patterns or natural groupings that we are missing?
+    # For example, it might be that it
+    # we can key off of some attributes or other properties of the
+    # LHS of a builtin, instead of listing all of the builtins in that class
+    # (which may miss some).
+    # Below, we key on a string, but Symbol is more correct.
+
+    def assign(self, lhs, rhs, evaluation, tags=None, upset=False):
+        lhs, lookup_name = normalize_lhs(lhs, evaluation)
+        try:
+            # Using a builtin name, find which assignment procedure to perform,
+            # and then call that function.
+            assignment_func = ASSIGNMENT_FUNCTION_MAP.get(lookup_name, None)
+            if assignment_func:
+                return assignment_func(self, lhs, rhs, evaluation, tags, upset)
+
+            return assign_store_rules_by_tag(self, lhs, rhs, evaluation, tags, upset)
+        except AssignmentException:
+
+            return False
+
+
 class Set(BinaryOperator, _SetOperator):
     """
     <dl>

mathics/builtin/assignments/clear.py

@@ -38,10 +38,9 @@
     SymbolUpValues,
 )
 
+from mathics.core.assignment import is_protected
 from mathics.core.atoms import String
 
-from mathics.builtin.assignments.internals import is_protected
-
 
 class Clear(Builtin):
     """

mathics/builtin/assignments/types.py

@@ -8,8 +8,7 @@
 
 from mathics.builtin.base import Builtin
 
-from mathics.builtin.assignments.internals import get_symbol_values
-
+from mathics.core.assignment import get_symbol_values
 from mathics.core.attributes import A_HOLD_ALL, A_PROTECTED
 
 

mathics/builtin/assignments/upvalues.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 
 
-from mathics.builtin.assignments.internals import _SetOperator
+from mathics.builtin.assignments.assignment import _SetOperator
 from mathics.builtin.base import BinaryOperator
 from mathics.core.attributes import (
     A_HOLD_ALL,

mathics/builtin/atomic/symbols.py

@@ -7,7 +7,6 @@
 
 import re
 
-from mathics.builtin.assignments.internals import get_symbol_values
 from mathics.builtin.base import (
     Builtin,
     PrefixOperator,
@@ -16,6 +15,7 @@
 
 from mathics.builtin.atomic.strings import to_regex
 
+from mathics.core.assignment import get_symbol_values
 from mathics.core.atoms import String
 
 from mathics.core.attributes import (

mathics/builtin/attributes.py

@@ -15,7 +15,7 @@
 
 
 from mathics.builtin.base import Predefined, Builtin
-from mathics.builtin.assignments.internals import get_symbol_list
+from mathics.core.assignment import get_symbol_list
 from mathics.core.atoms import String
 from mathics.core.attributes import (
     attributes_bitset_to_list,

mathics/builtin/scoping.py

@@ -4,13 +4,13 @@
 """
 
 
-from mathics.builtin.assignments.internals import get_symbol_list
 from mathics.core.attributes import (
     A_HOLD_ALL,
     A_PROTECTED,
     attribute_string_to_number,
 )
 from mathics.builtin.base import Builtin, Predefined
+from mathics.core.assignment import get_symbol_list
 from mathics.core.atoms import (
     String,
     Integer,