Require right-hand side of one-to-many assignments to be Indexable (#…

…11545)

Makefile

@@ -28,7 +28,7 @@ static ?=       ## Enable static linking
 O := .build
 SOURCES := $(shell find src -name '*.cr')
 SPEC_SOURCES := $(shell find spec -name '*.cr')
-override FLAGS += -D preview_multi_assign $(if $(release),--release )$(if $(stats),--stats )$(if $(progress),--progress )$(if $(threads),--threads $(threads) )$(if $(debug),-d )$(if $(static),--static )$(if $(LDFLAGS),--link-flags="$(LDFLAGS)" )$(if $(target),--cross-compile --target $(target) )
+override FLAGS += -D strict_multi_assign $(if $(release),--release )$(if $(stats),--stats )$(if $(progress),--progress )$(if $(threads),--threads $(threads) )$(if $(debug),-d )$(if $(static),--static )$(if $(LDFLAGS),--link-flags="$(LDFLAGS)" )$(if $(target),--cross-compile --target $(target) )
 SPEC_WARNINGS_OFF := --exclude-warnings spec/std --exclude-warnings spec/compiler --exclude-warnings spec/primitives
 SPEC_FLAGS := $(if $(verbose),-v )$(if $(junit_output),--junit_output $(junit_output) )
 CRYSTAL_CONFIG_LIBRARY_PATH := '$$ORIGIN/../lib/crystal'

spec/compiler/codegen/array_literal_spec.cr

@@ -266,7 +266,7 @@ end
 
 private def enumerable_element_type
   %(
-    struct Enumerable(T)
+    module Enumerable(T)
       def self.element_type(x)
         x.each { |elem| return elem }
         ret = uninitialized NoReturn

spec/compiler/codegen/multi_assign_spec.cr

@@ -8,20 +8,20 @@ describe "Code gen: multi assign" do
       CR
   end
 
-  it "supports 1 to n assignment" do
-    run(<<-CR).to_i.should eq(123)
-      class Foo
-        def [](index)
-          index &+ 1
+  context "without strict_multi_assign" do
+    it "supports 1 to n assignment" do
+      run(<<-CR).to_i.should eq(123)
+        class Foo
+          def [](index)
+            index &+ 1
+          end
         end
-      end
 
-      a, b, c = Foo.new
-      a &* 100 &+ b &* 10 &+ c
-      CR
-  end
+        a, b, c = Foo.new
+        a &* 100 &+ b &* 10 &+ c
+        CR
+    end
 
-  context "without strict_multi_assign" do
     it "doesn't raise if value size in 1 to n assignment doesn't match target count" do
       run(<<-CR).to_i.should eq(4)
         require "prelude"
@@ -38,6 +38,27 @@ describe "Code gen: multi assign" do
   end
 
   context "strict_multi_assign" do
+    it "supports 1 to n assignment" do
+      run(<<-CR, flags: %w(strict_multi_assign)).to_i.should eq(123)
+        require "prelude"
+
+        class Foo
+          include Indexable(Int32)
+
+          def unsafe_fetch(index)
+            index &+ 1
+          end
+
+          def size
+            3
+          end
+        end
+
+        a, b, c = Foo.new
+        a &* 100 &+ b &* 10 &+ c
+        CR
+    end
+
     it "raises if value size in 1 to n assignment doesn't match target count" do
       run(<<-CR, flags: %w(strict_multi_assign)).to_i.should eq(5)
         require "prelude"
@@ -119,6 +140,7 @@ describe "Code gen: multi assign" do
   it "supports 1 to n assignment, with splat on left-hand side (2)" do
     run(<<-CR).to_i.should eq(12345)
       #{range_new}
+      #{include_indexable}
 
       *a, b, c = {1, 2, 3, 4, 5}
       a[0] &* 10000 &+ a[1] &* 1000 &+ a[2] &* 100 &+ b &* 10 &+ c
@@ -128,6 +150,7 @@ describe "Code gen: multi assign" do
   it "supports 1 to n assignment, with splat on left-hand side (3)" do
     run(<<-CR).to_i.should eq(12345)
       #{range_new}
+      #{include_indexable}
 
       a, b, *c = {1, 2, 3, 4, 5}
       a &* 10000 &+ b &* 1000 &+ c[0] &* 100 &+ c[1] &* 10 &+ c[2]
@@ -147,6 +170,7 @@ describe "Code gen: multi assign" do
     run(<<-CR).to_b.should be_true
       #{tuple_new}
       #{range_new}
+      #{include_indexable}
 
       *x, _, _ = {1, 2}
       x.is_a?(Tuple(*typeof(Tuple.new)))
@@ -157,6 +181,7 @@ describe "Code gen: multi assign" do
     run(<<-CR).to_b.should be_true
       #{tuple_new}
       #{range_new}
+      #{include_indexable}
 
       _, _, *x = {1, 2}
       x.is_a?(Tuple(*typeof(Tuple.new)))
@@ -210,7 +235,7 @@ private def tuple_new
         args
       end
     end
-  CR
+    CR
 end
 
 private def range_new
@@ -219,5 +244,13 @@ private def range_new
       def initialize(@begin : B, @end : E, @exclusive : Bool = false)
       end
     end
-  CR
+    CR
+end
+
+private def include_indexable
+  <<-CR
+    struct Tuple(*T)
+      include Indexable(Union(*T))
+    end
+    CR
 end

spec/compiler/semantic/multi_assign_spec.cr

@@ -11,6 +11,38 @@ describe "Semantic: multi assign" do
         {a, b}
         )) { tuple_of [int32, int32] }
     end
+
+    it "doesn't error if assigning non-Indexable (#11414)" do
+      assert_no_errors <<-CR
+        class Foo
+          def [](index)
+          end
+
+          def size
+            3
+          end
+        end
+
+        a, b, c = Foo.new
+        CR
+    end
+
+    it "errors if assigning non-Indexable to splat (#11414)" do
+      assert_error <<-CR, "right-hand side of one-to-many assignment must be an Indexable, not Foo"
+        require "prelude"
+
+        class Foo
+          def [](index)
+          end
+
+          def size
+            3
+          end
+        end
+
+        a, *b, c = Foo.new
+        CR
+    end
   end
 
   context "strict_multi_assign" do
@@ -60,5 +92,39 @@ describe "Semantic: multi assign" do
         {a, b}
         ), flags: "strict_multi_assign") { tuple_of [int32, union_of(int32, string)] }
     end
+
+    it "errors if assigning non-Indexable (#11414)" do
+      assert_error <<-CR, "right-hand side of one-to-many assignment must be an Indexable, not Foo", flags: "strict_multi_assign"
+        require "prelude"
+
+        class Foo
+          def [](index)
+          end
+
+          def size
+            3
+          end
+        end
+
+        a, b, c = Foo.new
+        CR
+    end
+
+    it "errors if assigning non-Indexable to splat (#11414)" do
+      assert_error <<-CR, "right-hand side of one-to-many assignment must be an Indexable, not Foo", flags: "strict_multi_assign"
+        require "prelude"
+
+        class Foo
+          def [](index)
+          end
+
+          def size
+            3
+          end
+        end
+
+        a, *b, c = Foo.new
+        CR
+    end
   end
 end

src/compiler/crystal/program.cr

@@ -191,6 +191,9 @@ module Crystal
       types["Struct"] = struct_t = @struct_t = NonGenericClassType.new self, self, "Struct", value
       abstract_value_type(struct_t)
 
+      types["Enumerable"] = @enumerable = GenericModuleType.new self, self, "Enumerable", ["T"]
+      types["Indexable"] = @indexable = GenericModuleType.new self, self, "Indexable", ["T"]
+
       types["Array"] = @array = GenericClassType.new self, self, "Array", reference, ["T"]
       types["Hash"] = @hash_type = GenericClassType.new self, self, "Hash", reference, ["K", "V"]
       types["Regex"] = @regex = NonGenericClassType.new self, self, "Regex", reference
@@ -450,8 +453,8 @@ module Crystal
     end
 
     {% for name in %w(object no_return value number reference void nil bool char int int8 int16 int32 int64 int128
-                     uint8 uint16 uint32 uint64 uint128 float float32 float64 string symbol pointer array static_array
-                     exception tuple named_tuple proc union enum range regex crystal
+                     uint8 uint16 uint32 uint64 uint128 float float32 float64 string symbol pointer enumerable indexable
+                     array static_array exception tuple named_tuple proc union enum range regex crystal
                      packed_annotation thread_local_annotation no_inline_annotation
                      always_inline_annotation naked_annotation returns_twice_annotation
                      raises_annotation primitive_annotation call_convention_annotation

src/compiler/crystal/semantic/cleanup_transformer.cr

@@ -307,15 +307,28 @@ module Crystal
     end
 
     def transform(node : MultiAssign)
-      if @program.has_flag?("strict_multi_assign")
-        if node.values.size == 1
-          # the expanded node always starts with `temp = {{ node.values[0] }}`;
-          # this is the whole Assign node and its deduced type is equal to the
-          # original RHS's type
-          temp_assign = node.expanded.as(Expressions).expressions.first
-          target_count = node.targets.size
-
-          case type = temp_assign.type
+      expanded = node.expanded
+
+      if node.values.size == 1 && expanded
+        # the expanded node always starts with `temp = {{ node.values[0] }}`;
+        # `temp_assign` is this whole Assign node and its deduced type is same
+        # as the original RHS's type
+        temp_assign = expanded.as(Expressions).expressions.first
+        type = temp_assign.type
+        target_count = node.targets.size
+        has_strict_multi_assign = @program.has_flag?("strict_multi_assign")
+
+        # disallows `a, *b, c = {0 => "x", 1 => "y", 2 => "z"}`, as `Hash` is
+        # not `Indexable`
+        # also disallows `a, b, c = ...` if the strict flag is set
+        if node.targets.any?(Splat) || has_strict_multi_assign
+          unless type.implements?(@program.indexable)
+            node.values.first.raise "right-hand side of one-to-many assignment must be an Indexable, not #{type}"
+          end
+        end
+
+        if has_strict_multi_assign
+          case type
           when UnionType
             sizes = type.union_types.map { |union_type| constant_size(union_type) }
             if sizes.none? &.in?(target_count, nil)
@@ -331,7 +344,7 @@ module Crystal
         end
       end
 
-      if expanded = node.expanded
+      if expanded
         return expanded.transform self
       end
 

src/compiler/crystal/semantic/type_guess_visitor.cr

@@ -555,7 +555,7 @@ module Crystal
     end
 
     def guess_type(node : RegexLiteral)
-      program.types["Regex"]
+      program.regex
     end
 
     def guess_type(node : TupleLiteral)