Skip to content

Commit

Permalink
Changed the way pyright identifies an "unimplemented protocol method"…
Browse files Browse the repository at this point in the history
… within a stub file. It now looks at whether the method is decorated with `@abstractmethod`. Previously, it assumed all such methods were potentially implemented. This addresses #6946. (#6947)
  • Loading branch information
erictraut authored Jan 9, 2024
1 parent a7c8526 commit 2835c47
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 26 deletions.
65 changes: 50 additions & 15 deletions packages/pyright-internal/src/analyzer/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4876,21 +4876,14 @@ export class Checker extends ParseTreeWalker {
}
}
} else if (decls[0].type === DeclarationType.Function) {
if (
decls.every(
(decl) =>
decl.type !== DeclarationType.Function || ParseTreeUtils.isSuiteEmpty(decl.node.suite)
)
) {
if (!decls[0].uri.hasExtension('.pyi')) {
if (!isSymbolImplemented(name)) {
diagAddendum.addMessage(
Localizer.DiagnosticAddendum.missingProtocolMember().format({
name,
classType: member.classType.details.name,
})
);
}
if (this._isUnimplementedProtocolMethod(member.symbol)) {
if (!isSymbolImplemented(name)) {
diagAddendum.addMessage(
Localizer.DiagnosticAddendum.missingProtocolMember().format({
name,
classType: member.classType.details.name,
})
);
}
}
}
Expand Down Expand Up @@ -5020,6 +5013,48 @@ export class Checker extends ParseTreeWalker {
});
}

// Determine whether a method defined in a protocol should be considered
// "unimplemented". This is an under-specified part of the typing spec.
private _isUnimplementedProtocolMethod(symbol: Symbol): boolean {
const symbolType = this._evaluator.getEffectiveTypeOfSymbol(symbol);

// Behavior differs between stub files and source files.
const decls = symbol.getDeclarations();
const isDeclaredInStubFile = decls.length > 0 && isStubFile(decls[0].uri);

if (isFunction(symbolType)) {
const decl = symbolType.details.declaration;
if (decl) {
return isDeclaredInStubFile
? FunctionType.isAbstractMethod(symbolType)
: ParseTreeUtils.isSuiteEmpty(decl.node.suite);
}
} else if (isOverloadedFunction(symbolType)) {
// If an implementation is present and has an empty body, assume
// the function is unimplemented.
const impl = OverloadedFunctionType.getImplementation(symbolType);
if (impl) {
const decl = impl.details.declaration;
if (decl) {
return ParseTreeUtils.isSuiteEmpty(decl.node.suite);
}

return false;
}

if (isDeclaredInStubFile) {
// If no implementation was present, see if any of the overloads
// are marked as abstract.
const overloads = OverloadedFunctionType.getOverloads(symbolType);
return overloads.some((overload) => FunctionType.isAbstractMethod(overload));
}

return true;
}

return false;
}

// If a class is marked final, it must implement all abstract methods,
// otherwise it is of no use.
private _validateFinalClassNotAbstract(classType: ClassType, errorNode: ClassNode) {
Expand Down
19 changes: 15 additions & 4 deletions packages/pyright-internal/src/tests/samples/classes11.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
# This sample tests the detection of mutually-incompatible base classes
# in classes that use multiple inheritance.

from typing import Collection, Mapping, Sequence, TypeVar
from typing import Collection, Iterator, Mapping, Sequence, TypeVar


# This should generate an error.
class A(Mapping[str, int], Collection[int]):
...
def __len__(self) -> int:
...

def __iter__(self) -> Iterator[str]:
...


# This should generate an error.
Expand Down Expand Up @@ -37,9 +41,16 @@ class F(Mapping[int, int], Sequence[float]):


class G(Mapping[T, S], Collection[T]):
...
def __len__(self) -> int:
...

def __iter__(self) -> Iterator[T]:
...

# This should generate an error.
class H(Mapping[T, S], Collection[S]):
...
def __len__(self) -> int:
...

def __iter__(self) -> Iterator[T]:
...
2 changes: 2 additions & 0 deletions packages/pyright-internal/src/tests/samples/constructor10.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ class A(Iterator[_T_co]):
def __new__(cls, __iterable: Iterable[_T]) -> "A[tuple[_T, _T]]":
...

def __next__(self) -> _T_co: ...


def func1(iterable: Iterable[_T]) -> Iterator[tuple[_T, _T, _T]]:
for (a, _), (b, c) in A(A(iterable)):
Expand Down
5 changes: 3 additions & 2 deletions packages/pyright-internal/src/tests/samples/genericType24.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# This sample tests a case where a default argument in a parent class
# needs to be specialized in the context of a child class.

from typing import Generic, Iterable, TypeVar
from typing import Generic, Iterable, Iterator, TypeVar

T = TypeVar("T")


class IterableProxy(Iterable[T]):
...
def __iter__(self) -> Iterator[T]:
...


class Parent(Generic[T]):
Expand Down
5 changes: 3 additions & 2 deletions packages/pyright-internal/src/tests/samples/genericType44.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
# invariant type parameter. Literal types need to be handled
# carefully in this case.

from typing import Awaitable, Literal, TypeVar
from typing import Any, Awaitable, Generator, Literal, TypeVar

_T = TypeVar("_T")


class Future(Awaitable[_T]):
...
def __await__(self) -> Generator[Any, None, _T]:
...


def func1(future: Future[_T]) -> Future[_T]:
Expand Down
4 changes: 2 additions & 2 deletions packages/pyright-internal/src/tests/samples/solver24.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# a union of type variables.

from os import PathLike
from typing import AnyStr, Generic, Iterable, Iterator, TypeAlias, TypeVar
from typing import AnyStr, Generic, Iterable, Iterator, Protocol, TypeAlias, TypeVar

V = TypeVar("V")
V_co = TypeVar("V_co", covariant=True)
Expand Down Expand Up @@ -34,7 +34,7 @@ class ClassC(Generic[AnyStr]):
...


class ClassD(Iterator[ClassC[AnyStr]]):
class ClassD(Iterator[ClassC[AnyStr]], Protocol):
...


Expand Down
5 changes: 4 additions & 1 deletion packages/pyright-internal/src/tests/samples/solver30.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# This sample tests the case where a deeply nested set of calls requires
# the use of bidirectional type inference to evaluate the type of a lambda.

from typing import Any, Callable, Iterable, Iterator, TypeVar
from typing import Any, Callable, Iterable, Iterator, Protocol, TypeVar

X = TypeVar("X")
Y = TypeVar("Y")
Expand All @@ -27,6 +27,9 @@ class func3(Iterator[Z]):
def __init__(self, a: Callable[[Z], Any], b: Iterable[Z]) -> None:
...

def __next__(self) -> Z:
...


def func4(a: Callable[[Z], Any], b: Iterable[Z]) -> Iterator[Z]:
...
Expand Down

0 comments on commit 2835c47

Please sign in to comment.