Skip to content

Commit

Permalink
Return type error reporting inverted
Browse files Browse the repository at this point in the history
Summary:
D35858872 fixed and issue with reporting incorrect return types by removing the Exact[?] specifier when it is not required in the message. But, in so doing it accidentally (I guess) reversed the order of the error message. Most of the time this was not confusing enough for folk to notice. But for the case in the task it made it look like the compiler was wrong then actually the compiler was fine and the code wrong.

This change flipps stuff back the way it should be to be clear and fixes up the impacted tests.

Reviewed By: alexmalyshev

Differential Revision: D56023775

fbshipit-source-id: f3aab1330a8d7a1e14a1f0d7e71164c972cbbd52
  • Loading branch information
SonicField authored and facebook-github-bot committed Apr 11, 2024
1 parent 29c6237 commit 252a419
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 9 deletions.
2 changes: 1 addition & 1 deletion cinderx/PythonLib/cinderx/compiler/static/type_binder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1719,7 +1719,7 @@ def visitReturn(self, node: Return) -> None:
and not expected.klass.can_assign_from(returned)
):
reason = resolve_assign_error_msg(
expected.klass, returned, "return type must be {1}, not {0}"
expected.klass, returned, "return type must be {0}, not {1}"
)
self.syntax_error(reason, node)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def foo() -> Annotated[str, "aaaaa"]:
"""
with self.assertRaisesRegex(
TypedSyntaxError,
r"return type must be str, not Literal\[0\]",
r"return type must be Literal\[0\], not str",
):
self.compile(codestr)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def type_mismatch(from_type: str, to_type: str) -> str:


def bad_ret_type(from_type: str, to_type: str) -> str:
return re.escape(f"return type must be {to_type}, not {from_type}")
return re.escape(f"return type must be {from_type}, not {to_type}")


def disable_hir_inliner(f):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3374,6 +3374,19 @@ def f(x: Optional[int]) -> None:
):
self.compile(codestr, modname="foo")

def test_and_in_return(self):
codestr = """
from typing import Optional
def f(x: Optional[int]) -> None:
return x and None
"""
with self.assertRaisesRegex(
TypedSyntaxError,
bad_ret_type("Optional[int]", "None"),
):
self.compile(codestr, modname="foo")

def test_none_compare(self):
codestr = """
def f(x: int | None):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ def test_literal_bool_annotation_error(self) -> None:
def f(x: bool) -> Literal[False]:
return x
"""
self.type_error(codestr, r"return type must be Literal\[False\]", "return x")
self.type_error(
codestr, r"return type must be bool, not Literal\[False\]", "return x"
)

def test_literal_bool_annotation_runtime_cast(self) -> None:
codestr = """
Expand Down Expand Up @@ -72,7 +74,9 @@ def test_literal_int_annotation_error(self) -> None:
def f(x: int) -> Literal[1]:
return x
"""
self.type_error(codestr, r"return type must be Literal\[1\]", "return x")
self.type_error(
codestr, r"return type must be int, not Literal\[1\]", "return x"
)

def test_literal_int_annotation_runtime_cast(self) -> None:
codestr = """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ def f(x: int) -> str:
return C(x)
"""
with self.assertRaisesRegex(
TypedSyntaxError, "return type must be str, not int"
TypedSyntaxError, "return type must be int, not str"
):
self.compile(codestr)

Expand Down Expand Up @@ -335,7 +335,7 @@ def f() -> C:
"""
with self.assertRaisesRegex(
TypedSyntaxError,
"return type must be <module>.C, not object",
"return type must be object, not <module>.C",
):
self.compile(codestr)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2301,7 +2301,7 @@ def foo(x: Optional[int]) -> str:
prod_assert(x)
return x
"""
self.type_error(codestr, "return type must be str, not int")
self.type_error(codestr, "return type must be int, not str")

def test_prod_assert_raises(self):
codestr = """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ def test_walrus_operator_post_type(self):
def fn() -> None:
return (b:= 2)
"""
with self.assertRaisesRegex(TypedSyntaxError, r"return type must be None, not"):
with self.assertRaisesRegex(
TypedSyntaxError, r"return type must be Literal\[2\], not None"
):
self.compile(codestr)

def test_walrus_operator_post_type_primitive(self):
Expand Down

0 comments on commit 252a419

Please sign in to comment.