Merge pull request #7999 from asterite/warning-abstract-return-type

Turn abstract return type erros into warnings

spec/compiler/semantic/abstract_def_spec.cr

@@ -395,8 +395,8 @@ describe "Semantic: abstract def" do
       )
   end
 
-  it "errors if missing return type" do
-    assert_error %(
+  it "warning if missing return type" do
+    assert_warning %(
       abstract class Foo
         abstract def foo : Int32
       end
@@ -407,11 +407,11 @@ describe "Semantic: abstract def" do
         end
       end
       ),
-      "this method overrides Foo#foo() which has an explicit return type of Int32.\n\nPlease add an explicit return type (Int32 or a subtype of it) to this method as well."
+      "warning in line 8\nWarning: this method overrides Foo#foo() which has an explicit return type of Int32.\n\nPlease add an explicit return type (Int32 or a subtype of it) to this method as well."
   end
 
-  it "errors if different return type" do
-    assert_error %(
+  it "warning if different return type" do
+    assert_warning %(
       abstract class Foo
         abstract def foo : Int32
       end
@@ -425,7 +425,7 @@ describe "Semantic: abstract def" do
         end
       end
       ),
-      "this method must return Int32, which is the return type of the overridden method Foo#foo(), or a subtype of it, not Bar::Int32"
+      "warning in line 11\nWarning: this method must return Int32, which is the return type of the overridden method Foo#foo(), or a subtype of it, not Bar::Int32"
   end
 
   it "can return a more specific type" do
@@ -543,7 +543,7 @@ describe "Semantic: abstract def" do
   end
 
   it "is missing a return type in subclass of generic subclass" do
-    assert_error %(
+    assert_warning %(
         abstract class Foo(T)
           abstract def foo : T
         end
@@ -553,6 +553,34 @@ describe "Semantic: abstract def" do
           end
         end
       ),
-      "this method overrides Foo(T)#foo() which has an explicit return type of T.\n\nPlease add an explicit return type (Int32 or a subtype of it) to this method as well."
+      "warning in line 8\nWarning: this method overrides Foo(T)#foo() which has an explicit return type of T.\n\nPlease add an explicit return type (Int32 or a subtype of it) to this method as well."
+  end
+
+  it "can't find parent return type" do
+    assert_warning %(
+        abstract class Foo
+          abstract def foo : Unknown
+        end
+
+        class Bar < Foo
+          def foo
+          end
+        end
+      ),
+      "warning in line 4\nWarning: can't resolve return type Unknown"
+  end
+
+  it "can't find child return type" do
+    assert_warning %(
+        abstract class Foo
+          abstract def foo : Int32
+        end
+
+        class Bar < Foo
+          def foo : Unknown
+          end
+        end
+      ),
+      "warning in line 8\nWarning: can't resolve return type Unknown"
   end
 end

spec/spec_helper.cr

@@ -117,6 +117,7 @@ def warnings_result(code, inject_primitives = true)
   compiler.warnings = Warnings::All
   compiler.error_on_warnings = false
   compiler.prelude = "empty" # avoid issues in the current std lib
+  compiler.color = false
   apply_program_flags(compiler.flags)
   result = compiler.compile Compiler::Source.new("code.cr", code), output_filename
 

src/compiler/crystal/semantic/abstract_def_checker.cr

@@ -158,7 +158,11 @@ class Crystal::AbstractDefChecker
     base_return_type_node = base_method.return_type
     return unless base_return_type_node
 
-    original_base_return_type = base_type.lookup_type(base_return_type_node)
+    original_base_return_type = base_type.lookup_type?(base_return_type_node)
+    unless original_base_return_type
+      @program.warning_failures << base_return_type_node.warning "can't resolve return type #{base_return_type_node}\n#{this_warning_will_become_an_error}"
+      return
+    end
 
     # If the base type is a generic type, we find the generic instantiation of
     # t1 for it. This will have a mapping of type vars to types, for example
@@ -173,17 +177,27 @@ class Crystal::AbstractDefChecker
       base_return_type_node.accept(replacer)
     end
 
-    base_return_type = base_type.lookup_type(base_return_type_node)
+    base_return_type = base_type.lookup_type?(base_return_type_node)
+    unless base_return_type
+      @program.warning_failures << base_return_type_node.warning "can't resolve return type #{base_return_type_node}\n#{this_warning_will_become_an_error}"
+      return
+    end
 
     return_type_node = method.return_type
     unless return_type_node
-      method.raise "this method overrides #{Call.def_full_name(base_type, base_method)} which has an explicit return type of #{original_base_return_type}.\n#{@program.colorize("Please add an explicit return type (#{base_return_type} or a subtype of it) to this method as well.").yellow.bold}"
+      @program.warning_failures << method.warning "this method overrides #{Call.def_full_name(base_type, base_method)} which has an explicit return type of #{original_base_return_type}.\n#{@program.colorize("Please add an explicit return type (#{base_return_type} or a subtype of it) to this method as well.").yellow.bold}\n\n#{this_warning_will_become_an_error}"
+      return
     end
 
-    return_type = type.lookup_type(return_type_node)
+    return_type = type.lookup_type?(return_type_node)
+    unless return_type
+      @program.warning_failures << return_type_node.warning "can't resolve return type #{return_type_node}\n#{this_warning_will_become_an_error}"
+      return
+    end
 
     unless return_type.implements?(base_return_type)
-      return_type_node.raise "this method must return #{base_return_type}, which is the return type of the overridden method #{Call.def_full_name(base_type, base_method)}, or a subtype of it, not #{return_type}"
+      @program.warning_failures << return_type_node.warning "this method must return #{base_return_type}, which is the return type of the overridden method #{Call.def_full_name(base_type, base_method)}, or a subtype of it, not #{return_type}\n#{this_warning_will_become_an_error}"
+      return
     end
   end
 
@@ -203,6 +217,10 @@ class Crystal::AbstractDefChecker
     end.as(GenericInstanceType)
   end
 
+  private def this_warning_will_become_an_error
+    @program.colorize("The above warning will become an error in a future Crystal version.").yellow.bold
+  end
+
   class ReplacePathWithTypeVar < Visitor
     def initialize(@base_type : GenericType, @generic_type : GenericInstanceType)
     end