Repository: quora/pyanalyze
Pull request #445 (2022-01-27 02:47:09)

improve overload errors


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 6043345..f57c089 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Improve error messages for overloaded calls (#445)
 - Infer `NoReturn` instead of `Any` for unreachable code (#443)
 - Make `NoReturn` compatible with all other types (#442)
 - Fix treatment of walrus operator in `and`, `or`, and `if/else`
diff --git a/pyanalyze/signature.py b/pyanalyze/signature.py
index 53b7bec..ea16e44 100644
--- a/pyanalyze/signature.py
+++ b/pyanalyze/signature.py
@@ -1046,6 +1046,18 @@ class Signature:
         bound_args = self.bind_arguments(preprocessed, ctx)
         if bound_args is None:
             return self.get_default_return()
+        return self.check_call_with_bound_args(
+            preprocessed, bound_args, ctx, is_overload=is_overload
+        )
+
+    def check_call_with_bound_args(
+        self,
+        preprocessed: ActualArguments,
+        bound_args: BoundArgs,
+        ctx: CheckCallContext,
+        *,
+        is_overload: bool = False,
+    ) -> CallReturn:
         variables = {key: composite.value for key, (_, composite) in bound_args.items()}
 
         if self.callable is not None and ctx.visitor is not None:
@@ -1951,6 +1963,10 @@ class OverloadedSignature:
         Our behavior is closer to mypy. The general rule is to pick the first overload that matches
         and return an error otherwise, but there are two twists: ``Any`` and unions.
 
+        Before we do a full check, we first check only whether the argument names and
+        numbers match by calling :func:`Signature.bind_arguments` (a trick we picked up
+        from pyright). This makes for better error messages.
+
         If an overload matched only due to ``Any``, we continue looking for more overloads. If there
         are other matching overloads, we return ``Any`` (with
         ``AnySource.multiple_overload_matches``).
@@ -1984,13 +2000,40 @@ class OverloadedSignature:
         if actual_args is None:
             return AnyValue(AnySource.error)
 
+        # We first bind the arguments for each overload, to get the obvious errors
+        # out of the way first.
+        errors_per_overload = []
+        bound_args_per_overload = []
+        for sig in self.signatures:
+            with visitor.catch_errors() as caught_errors:
+                bound_args = sig.bind_arguments(actual_args, ctx)
+            bound_args_per_overload.append(bound_args)
+            errors_per_overload.append(caught_errors)
+
+        if not any(bound_args is not None for bound_args in bound_args_per_overload):
+            detail = self._make_detail(errors_per_overload)
+            visitor.show_error(
+                node,
+                "Cannot call overloaded function",
+                ErrorCode.incompatible_call,
+                detail=str(detail),
+            )
+            return AnyValue(AnySource.error)
+
         errors_per_overload = []
         any_rets: List[CallReturn] = []
         union_rets: List[CallReturn] = []
         union_and_any_rets: List[CallReturn] = []
-        last = len(self.signatures) - 1
-        for i, sig in enumerate(self.signatures):
+        sigs = [
+            sig
+            for sig, bound_args in zip(self.signatures, bound_args_per_overload)
+            if bound_args is not None
+        ]
+        last = len(sigs) - 1
+        for i, sig in enumerate(sigs):
             with visitor.catch_errors() as caught_errors:
+                # We can't use check_call_with_bound_args here because we may
+                # rebind the arguments.
                 ret = sig.check_call_preprocessed(
                     actual_args,
                     ctx,


Test patch:
diff --git a/pyanalyze/test_signature.py b/pyanalyze/test_signature.py
index 66e423a..f4be874 100644
--- a/pyanalyze/test_signature.py
+++ b/pyanalyze/test_signature.py
@@ -1050,7 +1050,7 @@ class TestOverload(TestNameCheckVisitorBase):
         def capybara():
             assert_is_value(overloaded(), TypedValue(int))
             assert_is_value(overloaded("x"), TypedValue(str))
-            overloaded(1)  # E: incompatible_call
+            overloaded(1)  # E: incompatible_argument
             overloaded("x", "y")  # E: incompatible_call
 
     @assert_passes()
@@ -1175,7 +1175,7 @@ class TestOverload(TestNameCheckVisitorBase):
             print("x", file=SupportsWrite(), flush=True)  # E: incompatible_argument
             print("x", file=SupportsWriteAndFlush(), flush=True)
             print("x", file=SupportsWriteAndFlush())
-            print("x", file="not a file")  # E: incompatible_call
+            print("x", file="not a file")  # E: incompatible_argument
 
         def pacarana(f: float):
             assert_is_value(f.__round__(), TypedValue(int))
@@ -1209,3 +1209,27 @@ class TestOverload(TestNameCheckVisitorBase):
         def capybara():
             assert_is_value(f(1), TypedValue(str))
             assert_is_value(f(""), TypedValue(int))
+
+    @assert_passes()
+    def test_bound_args_first(self):
+        from pyanalyze.extensions import overload
+
+        @overload
+        def f(x: int, y: str) -> None:
+            pass
+
+        @overload
+        def f(x: int) -> int:
+            pass
+
+        @overload
+        def f(x: str) -> str:
+            pass
+
+        def f(x: object, y: object = ...) -> object:
+            raise NotImplementedError
+
+        def capybara():
+            f(1.0)  # E: incompatible_argument
+            f(1, 1)  # E: incompatible_argument
+            f(1, 1, 1)  # E: incompatible_call