[ty] Improve @override, @final and Liskov checks in cases where there are multiple reachable definitions (#21767)

Author: AlexWaygood

crates/ty_ide/src/importer.rs

@@ -327,9 +327,7 @@ impl<'ast> MembersInScope<'ast> {
             .members_in_scope_at(node)
             .into_iter()
             .map(|(name, memberdef)| {
-                let Some(def) = memberdef.definition else {
-                    return (name, MemberInScope::other(memberdef.ty));
-                };
+                let def = memberdef.first_reachable_definition;
                 let kind = match *def.kind(db) {
                     DefinitionKind::Import(ref kind) => {
                         MemberImportKind::Imported(AstImportKind::Import(kind.import(parsed)))
@@ -1891,13 +1889,13 @@ else:
         "#);
         assert_snapshot!(
             test.import_from("foo", "MAGIC"), @r#"
-        import foo
+        from foo import MAGIC
         if os.getenv("WHATEVER"):
             from foo import MAGIC
         else:
             from bar import MAGIC
 
-        (foo.MAGIC)
+        (MAGIC)
         "#);
     }
 
@@ -2108,13 +2106,13 @@ except ImportError:
         ");
         assert_snapshot!(
             test.import_from("foo", "MAGIC"), @r"
-        import foo
+        from foo import MAGIC
         try:
             from foo import MAGIC
         except ImportError:
             from bar import MAGIC
 
-        (foo.MAGIC)
+        (MAGIC)
         ");
     }
 

crates/ty_python_semantic/resources/mdtest/final.md

@@ -488,11 +488,110 @@ class C(A):
         pass
 
     if coinflip():
-        def method2(self) -> None: ...  # TODO: should emit [override-of-final-method]
+        def method2(self) -> None: ...  # error: [override-of-final-method]
     else:
-        def method2(self) -> None: ...  # TODO: should emit [override-of-final-method]
+        def method2(self) -> None: ...
 
     if coinflip():
         def method3(self) -> None: ...  # error: [override-of-final-method]
-        def method4(self) -> None: ...  # error: [override-of-final-method]
+
+    # TODO: we should emit Liskov violations here too:
+    if coinflip():
+        method4 = 42  # error: [override-of-final-method]
+    else:
+        method4 = 56
+```
+
+## Definitions in statically known branches
+
+```toml
+[environment]
+python-version = "3.10"
+```
+
+```py
+import sys
+from typing_extensions import final
+
+class Parent:
+    if sys.version_info >= (3, 10):
+        @final
+        def foo(self) -> None: ...
+        @final
+        def foooo(self) -> None: ...
+        @final
+        def baaaaar(self) -> None: ...
+    else:
+        @final
+        def bar(self) -> None: ...
+        @final
+        def baz(self) -> None: ...
+        @final
+        def spam(self) -> None: ...
+
+class Child(Parent):
+    def foo(self) -> None: ...  # error: [override-of-final-method]
+
+    # The declaration on `Parent` is not reachable,
+    # so this is fine
+    def bar(self) -> None: ...
+
+    if sys.version_info >= (3, 10):
+        def foooo(self) -> None: ...  # error: [override-of-final-method]
+        def baz(self) -> None: ...
+    else:
+        # Fine because this doesn't override any reachable definitions
+        def foooo(self) -> None: ...
+        # There are `@final` definitions being overridden here,
+        # but the definitions that override them are unreachable
+        def spam(self) -> None: ...
+        def baaaaar(self) -> None: ...
+```
+
+## Overloads in statically-known branches in stub files
+
+<!-- snapshot-diagnostics -->
+
+```toml
+[environment]
+python-version = "3.10"
+```
+
+```pyi
+import sys
+from typing_extensions import overload, final
+
+class Foo:
+    if sys.version_info >= (3, 10):
+        @overload
+        @final
+        def method(self, x: int) -> int: ...
+    else:
+        @overload
+        def method(self, x: int) -> int: ...
+    @overload
+    def method(self, x: str) -> str: ...
+
+    if sys.version_info >= (3, 10):
+        @overload
+        def method2(self, x: int) -> int: ...
+    else:
+        @overload
+        @final
+        def method2(self, x: int) -> int: ...
+    @overload
+    def method2(self, x: str) -> str: ...
+
+class Bar(Foo):
+    @overload
+    def method(self, x: int) -> int: ...
+    @overload
+    def method(self, x: str) -> str: ...  # error: [override-of-final-method]
+
+    # This is fine: the only overload that is marked `@final`
+    # is in a statically-unreachable branch
+    @overload
+    def method2(self, x: int) -> int: ...
+    @overload
+    def method2(self, x: str) -> str: ...
 ```

crates/ty_python_semantic/resources/mdtest/liskov.md

@@ -583,3 +583,17 @@ class GoodChild2(Parent):
     @staticmethod
     def static_method(x: object) -> bool: ...
 ```
+
+## Definitely bound members with no reachable definitions(!)
+
+We don't emit a Liskov-violation diagnostic here, but if you're writing code like this, you probably
+have bigger problems:
+
+```py
+from __future__ import annotations
+
+class MaybeEqWhile:
+    while ...:
+        def __eq__(self, other: MaybeEqWhile) -> bool:
+            return True
+```

crates/ty_python_semantic/resources/mdtest/named_tuple.md

@@ -610,3 +610,24 @@ class Child(Base):
     # This is fine - Child is not directly a NamedTuple
     _asdict = 42
 ```
+
+## Edge case: multiple reachable definitions with distinct issues
+
+<!-- snapshot-diagnostics -->
+
+```py
+from typing import NamedTuple
+
+def coinflip() -> bool:
+    return True
+
+class Foo(NamedTuple):
+    if coinflip():
+        _asdict: bool  # error: [invalid-named-tuple] "NamedTuple field `_asdict` cannot start with an underscore"
+    else:
+        # TODO: there should only be one diagnostic here...
+        #
+        # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`"
+        # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`"
+        _asdict = True
+```