Skip to content

Commit

Permalink
Make abstract def return type warning an error
Browse files Browse the repository at this point in the history
  • Loading branch information
bcardiff committed Oct 8, 2020
1 parent 8c182b8 commit 7988800
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
30 changes: 15 additions & 15 deletions spec/compiler/semantic/abstract_def_spec.cr
Expand Up @@ -395,8 +395,8 @@ describe "Semantic: abstract def" do
)
end

it "warning if missing return type" do
assert_warning <<-CR,
it "errors if missing return type" do
assert_error <<-CR,
abstract class Foo
abstract def foo : Int32
end
Expand All @@ -407,11 +407,11 @@ describe "Semantic: abstract def" do
end
end
CR
"warning in line 6\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."
"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 "warning if different return type" do
assert_warning <<-CR,
it "errors if different return type" do
assert_error <<-CR,
abstract class Foo
abstract def foo : Int32
end
Expand All @@ -425,7 +425,7 @@ describe "Semantic: abstract def" do
end
end
CR
"warning in line 9\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"
"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
Expand Down Expand Up @@ -542,8 +542,8 @@ describe "Semantic: abstract def" do
))
end

it "is missing a return type in subclass of generic subclass" do
assert_warning <<-CR,
it "errors if missing a return type in subclass of generic subclass" do
assert_error <<-CR,
abstract class Foo(T)
abstract def foo : T
end
Expand All @@ -553,11 +553,11 @@ describe "Semantic: abstract def" do
end
end
CR
"warning in line 6\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."
"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 <<-CR,
it "errors if can't find parent return type" do
assert_error <<-CR,
abstract class Foo
abstract def foo : Unknown
end
Expand All @@ -567,11 +567,11 @@ describe "Semantic: abstract def" do
end
end
CR
"warning in line 2\nWarning: can't resolve return type Unknown"
"can't resolve return type Unknown"
end

it "can't find child return type" do
assert_warning <<-CR,
it "errors if can't find child return type" do
assert_error <<-CR,
abstract class Foo
abstract def foo : Int32
end
Expand All @@ -581,7 +581,7 @@ describe "Semantic: abstract def" do
end
end
CR
"warning in line 6\nWarning: can't resolve return type Unknown"
"can't resolve return type Unknown"
end

it "doesn't crash when abstract method is implemented by supertype (#8031)" do
Expand Down
14 changes: 7 additions & 7 deletions src/compiler/crystal/semantic/abstract_def_checker.cr
Expand Up @@ -261,7 +261,7 @@ class Crystal::AbstractDefChecker

original_base_return_type = base_type.lookup_type?(base_return_type_node)
unless original_base_return_type
report_warning(base_return_type_node, "can't resolve return type #{base_return_type_node}\n#{this_warning_will_become_an_error}")
report_error(base_return_type_node, "can't resolve return type #{base_return_type_node}")
return
end

Expand All @@ -279,24 +279,24 @@ class Crystal::AbstractDefChecker

base_return_type = base_type.lookup_type?(base_return_type_node)
unless base_return_type
report_warning(base_return_type_node, "can't resolve return type #{base_return_type_node}\n#{this_warning_will_become_an_error}")
report_error(base_return_type_node, "can't resolve return type #{base_return_type_node}")
return
end

return_type_node = method.return_type
unless return_type_node
report_warning(method, "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}")
report_error(method, "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")
return
end

return_type = type.lookup_type?(return_type_node)
unless return_type
report_warning(return_type_node, "can't resolve return type #{return_type_node}\n#{this_warning_will_become_an_error}")
report_error(return_type_node, "can't resolve return type #{return_type_node}")
return
end

unless return_type.implements?(base_return_type)
report_warning(return_type_node, "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}")
report_error(return_type_node, "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}")
return
end
end
Expand All @@ -321,8 +321,8 @@ class Crystal::AbstractDefChecker
@program.colorize("The above warning will become an error in a future Crystal version.").yellow.bold
end

private def report_warning(node, message)
@program.report_warning(node, message)
private def report_error(node, message)
node.raise(message, nil, Crystal::AbstractDefImplementationError)
end

class ReplacePathWithTypeVar < Visitor
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/crystal/semantic/exception.cr
Expand Up @@ -171,6 +171,9 @@ module Crystal
end
end

class AbstractDefImplementationError < TypeException
end

class MethodTraceException < Exception
def initialize(@owner : Type?, @trace : Array(ASTNode), @nil_reason : NilReason?, @show : Bool)
super(nil)
Expand Down

0 comments on commit 7988800

Please sign in to comment.