fix[lang]: forbid assignment to values

tests/functional/codegen/features/test_assignment.py

@@ -1,7 +1,13 @@
 import pytest
 
 from vyper.evm.opcodes import version_check
-from vyper.exceptions import CodegenPanic, ImmutableViolation, InvalidType, TypeMismatch
+from vyper.exceptions import (
+    CodegenPanic,
+    ImmutableViolation,
+    InvalidType,
+    StructureException,
+    TypeMismatch,
+)
 
 
 def test_augassign(get_contract):
@@ -384,6 +390,91 @@ def foo(x: int128):
     assert_compile_failed(lambda: get_contract(code), ImmutableViolation)
 
 
+def test_invalid_assign_to_call_return(assert_compile_failed, get_contract):
+    code = """
+@internal
+def g() -> uint256[3]:
+    return [1, 2, 3]
+
+@external
+def f():
+    self.g()[0] = 5
+"""
+    with pytest.raises(StructureException) as e:
+        get_contract(code)
+
+    assert e.value.message == "`self.g()[0]` is not a valid assignment target"
+
+
+def test_invalid_assign_to_self_balance(assert_compile_failed, get_contract):
+    code = """
+@external
+def f():
+    self.balance = 100
+"""
+    with pytest.raises(StructureException) as e:
+        get_contract(code)
+
+    assert e.value.message == "`self.balance` is not a valid assignment target"
+
+
+def test_invalid_assign_to_storage_addr_balance(assert_compile_failed, get_contract):
+    code = """
+addr: address
+
+@external
+def f():
+    self.addr.balance = 1
+"""
+    with pytest.raises(StructureException) as e:
+        get_contract(code)
+
+    assert e.value.message == "`self.addr.balance` is not a valid assignment target"
+
+
+def test_invalid_assign_to_storage_addr_codehash(assert_compile_failed, get_contract):
+    code = """
+addr: address
+
+@external
+def f():
+    self.addr.codehash = empty(bytes32)
+"""
+    with pytest.raises(StructureException) as e:
+        get_contract(code)
+
+    assert e.value.message == "`self.addr.codehash` is not a valid assignment target"
+
+
+def test_invalid_assign_to_iface_address(assert_compile_failed, get_contract):
+    code = """
+interface IFoo:
+    def foo() -> uint256: nonpayable
+
+f: IFoo
+
+@external
+def g():
+    self.f.address = empty(address)
+"""
+    with pytest.raises(StructureException) as e:
+        get_contract(code)
+
+    assert e.value.message == "`self.f.address` is not a valid assignment target"
+
+
+def test_read_addr_balance_still_works(get_contract):
+    code = """
+addr: address
+
+@external
+def f() -> uint256:
+    return self.addr.balance
+"""
+    c = get_contract(code)
+    assert c.f() == 0
+
+
 def test_valid_literal_increment(get_contract):
     code = """
 storx: uint256

tests/functional/codegen/types/test_dynamic_array.py

@@ -11,10 +11,10 @@
 from vyper.exceptions import (
     ArgumentException,
     ArrayIndexException,
-    CompilerPanic,
     ImmutableViolation,
     OverflowException,
     StateAccessViolation,
+    StructureException,
     TypeMismatch,
 )
 
@@ -1892,8 +1892,7 @@ def boo() -> uint256:
     assert c.foo() == [1, 2, 3, 4]
 
 
-@pytest.mark.xfail(raises=CompilerPanic)
-def test_dangling_reference(get_contract, tx_failed):
+def test_dangling_reference(get_contract, assert_compile_failed):
     code = """
 a: DynArray[DynArray[uint256, 5], 5]
 
@@ -1902,9 +1901,10 @@ def foo():
     self.a = [[1]]
     self.a.pop().append(2)
     """
-    c = get_contract(code)
-    with tx_failed():
-        c.foo()
+    with pytest.raises(StructureException) as e:
+        get_contract(code)
+
+    assert e.value.message == "`self.a.pop()` is not a valid assignment target"
 
 
 def test_dynarray_append_single_field_struct_storage(get_contract):

tests/functional/syntax/test_flag.py

@@ -4,7 +4,6 @@
 from vyper.exceptions import (
     FlagDeclarationException,
     InvalidOperation,
-    InvalidReference,
     NamespaceCollision,
     StructureException,
     TypeMismatch,
@@ -134,7 +133,7 @@ def foo():
 def test_assign_to_flag():
   Status.ACTIVE = 2
         """,
-        InvalidReference,
+        StructureException,
     ),
 ]
 

tests/unit/semantics/analysis/test_for_loop.py

@@ -210,6 +210,44 @@ def foo():
     analyze_module_single(vyper_module)
 
 
+def test_modify_iterator_sibling_named_self():
+    # a struct field named `self` is a disjoint sibling of the loop iterator
+    code = """
+struct Foo:
+    self: uint256
+    arr: uint256[3]
+
+f: Foo
+
+@external
+def foo():
+    for i: uint256 in self.f.arr:
+        self.f.self = i
+    """
+    vyper_module = parse_to_ast(code)
+    analyze_module_single(vyper_module)
+
+
+def test_modify_iterator_self_field_self():
+    # writing the same field named `self` used as the iterator is a real conflict
+    code = """
+struct Foo:
+    self: uint256[3]
+
+f: Foo
+
+@external
+def foo():
+    for i: uint256 in self.f.self:
+        self.f.self[0] = i
+    """
+    vyper_module = parse_to_ast(code)
+    with pytest.raises(ImmutableViolation) as e:
+        analyze_module_single(vyper_module)
+
+    assert e.value._message == "Cannot modify loop variable `f`"
+
+
 def test_modify_subscript_barrier():
     # test that Subscript nodes are a barrier for analysis
     code = """

vyper/codegen/expr.py

@@ -35,7 +35,6 @@
     CodegenPanic,
     CompilerPanic,
     EvmVersionException,
-    StructureException,
     TypeCheckFailure,
     TypeMismatch,
     UnimplementedException,
@@ -825,6 +824,6 @@ def parse_value_expr(cls, expr, context):
     @classmethod
     def parse_pointer_expr(cls, expr, context):
         o = cls(expr, context).ir_node
-        if not o.location:
-            raise StructureException("Looking for a variable location, instead got a value", expr)
+        # Looking for a variable location, instead got a value
+        assert o.location
         return o

vyper/semantics/analysis/local.py

@@ -187,9 +187,9 @@ def _validate_pure_access(node: vy_ast.Attribute | vy_ast.Name, typ: VyperType)
 
 
 # analyse the variable access for the attribute chain for a node
-# e.x. `x` will return varinfo for `x`
-# `module.foo` will return VarAccess for `module.foo`
-# `self.my_struct.x.y` will return VarAccess for `self.my_struct.x.y`
+# e.x. `x` will return `VarAccess(<x>, ())`
+# `module.foo` will return `VarAccess(<module.foo>, ())`
+# `self.my_struct.x.y` will return `VarAccess(<self.my_struct>, ('x', 'y'))`
 def _get_variable_access(node: vy_ast.ExprNode) -> Optional[VarAccess]:
     path: list[str | object] = []
     info = get_expr_info(node)
@@ -207,13 +207,9 @@ def _get_variable_access(node: vy_ast.ExprNode) -> Optional[VarAccess]:
         if (attr := info.attr) is not None:
             path.append(attr)
 
-        assert isinstance(node, (vy_ast.Subscript, vy_ast.Attribute))  # help mypy
         node = node.value
         info = get_expr_info(node)
 
-    # ignore `self.` as it interferes with VarAccess comparison across modules
-    if len(path) > 0 and path[-1] == "self":
-        path.pop()
     path.reverse()
 
     return VarAccess(info.var_info, tuple(path))
@@ -571,6 +567,11 @@ def _handle_modification(self, target: vy_ast.ExprNode):
         if info.modifiability == Modifiability.CONSTANT:
             raise ImmutableViolation("Constant value cannot be written to.")
 
+        if info.location == DataLocation.UNSET:
+            raise StructureException(
+                f"`{target.node_source_code}` is not a valid assignment target", target
+            )
+
         var_access = _get_variable_access(target)
         assert var_access is not None
 

vyper/semantics/analysis/utils.py

@@ -23,6 +23,7 @@
 from vyper.semantics import types
 from vyper.semantics.analysis.base import ExprInfo, Modifiability, ModuleInfo, VarAccess, VarInfo
 from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions
+from vyper.semantics.data_locations import DataLocation
 from vyper.semantics.namespace import get_namespace
 from vyper.semantics.types.base import TYPE_T, VyperType
 from vyper.semantics.types.bytestrings import BytesT, StringT
@@ -110,6 +111,13 @@ def get_expr_info(self, node: vy_ast.VyperNode, is_callable: bool = False) -> Ex
             if isinstance(t, ModuleInfo):
                 return ExprInfo.from_moduleinfo(t, attr=attr)
 
+            if info.typ._type_members and attr in info.typ._type_members:
+                # things like `addr.balance` should not inherit the location of `addr`
+                # since `addr` can be assignable, while `addr.balance` never is
+                return ExprInfo(
+                    t, attr=attr, location=DataLocation.UNSET, modifiability=info.modifiability
+                )
+
             return info.copy_with_type(t, attr=attr)
 
         # If it's a Subscript, propagate the subscriptable varinfo