mirror of
https://github.com/microsoft/pyright.git
synced 2024-10-26 10:55:06 +03:00
Modified type inference logic so methods that raise an exception whose type derives from NotImplementedError
is not inferred to return NoReturn
. Previously, only NotImplementedError
was exempted, not subclasses. This addresses https://github.com/microsoft/pyright/issues/5608. (#5609)
Co-authored-by: Eric Traut <erictr@microsoft.com>
This commit is contained in:
parent
a5b5b706fe
commit
501681295b
@ -142,7 +142,7 @@ def func1(val: int):
|
|||||||
|
|
||||||
#### NoReturn return type
|
#### NoReturn return type
|
||||||
|
|
||||||
If there is no code path that returns from a function (e.g. all code paths raise an exception), Pyright infers a return type of `NoReturn`. As an exception to this rule, if the function is decorated with `@abstractmethod`, the return type is not inferred as `NoReturn` even if there is no return. This accommodates a common practice where an abstract method is implemented with a `raise NotImplementedError()` statement.
|
If there is no code path that returns from a function (e.g. all code paths raise an exception), Pyright infers a return type of `NoReturn`. As an exception to this rule, if the function is decorated with `@abstractmethod`, the return type is not inferred as `NoReturn` even if there is no return. This accommodates a common practice where an abstract method is implemented with a `raise` statement that raises an exception of type `NotImplementedError`.
|
||||||
|
|
||||||
```python
|
```python
|
||||||
class Foo:
|
class Foo:
|
||||||
|
@ -311,7 +311,7 @@ StrOrInt: TypeAlias = str | int
|
|||||||
```
|
```
|
||||||
|
|
||||||
#### Abstract Classes and Methods
|
#### Abstract Classes and Methods
|
||||||
Classes that must be subclassed should derive from `ABC`, and methods or properties that must be overridden should be decorated with the `@abstractmethod` decorator. This allows type checkers to validate that the required methods have been overridden and provide developers with useful error messages when they are not. It is customary to implement an abstract method by raising a `NotImplementedError` exception.
|
Classes that must be subclassed should derive from `ABC`, and methods or properties that must be overridden should be decorated with the `@abstractmethod` decorator. This allows type checkers to validate that the required methods have been overridden and provide developers with useful error messages when they are not. It is customary to implement an abstract method by raising a `NotImplementedError` exception or subclass thereof.
|
||||||
|
|
||||||
```python
|
```python
|
||||||
from abc import ABC, abstractmethod
|
from abc import ABC, abstractmethod
|
||||||
|
@ -65,6 +65,7 @@ import {
|
|||||||
} from './types';
|
} from './types';
|
||||||
import {
|
import {
|
||||||
ClassMemberLookupFlags,
|
ClassMemberLookupFlags,
|
||||||
|
derivesFromStdlibClass,
|
||||||
doForEachSubtype,
|
doForEachSubtype,
|
||||||
isIncompleteUnknown,
|
isIncompleteUnknown,
|
||||||
isTypeAliasPlaceholder,
|
isTypeAliasPlaceholder,
|
||||||
@ -1620,16 +1621,13 @@ export function getCodeFlowEngine(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (simpleStatement.nodeType === ParseNodeType.Raise && simpleStatement.typeExpression) {
|
if (simpleStatement.nodeType === ParseNodeType.Raise && simpleStatement.typeExpression) {
|
||||||
// Check for "raise NotImplementedError" or "raise NotImplementedError()"
|
// Check for a raising about 'NotImplementedError' or a subtype thereof.
|
||||||
const isNotImplementedName = (node: ParseNode) => {
|
const exceptionType = evaluator.getType(simpleStatement.typeExpression);
|
||||||
return node?.nodeType === ParseNodeType.Name && node.value === 'NotImplementedError';
|
|
||||||
};
|
|
||||||
|
|
||||||
if (isNotImplementedName(simpleStatement.typeExpression)) {
|
if (
|
||||||
foundRaiseNotImplemented = true;
|
exceptionType &&
|
||||||
} else if (
|
isClass(exceptionType) &&
|
||||||
simpleStatement.typeExpression.nodeType === ParseNodeType.Call &&
|
derivesFromStdlibClass(exceptionType, 'NotImplementedError')
|
||||||
isNotImplementedName(simpleStatement.typeExpression.leftExpression)
|
|
||||||
) {
|
) {
|
||||||
foundRaiseNotImplemented = true;
|
foundRaiseNotImplemented = true;
|
||||||
}
|
}
|
||||||
|
@ -200,6 +200,7 @@ import {
|
|||||||
convertToInstantiable,
|
convertToInstantiable,
|
||||||
convertTypeToParamSpecValue,
|
convertTypeToParamSpecValue,
|
||||||
derivesFromClassRecursive,
|
derivesFromClassRecursive,
|
||||||
|
derivesFromStdlibClass,
|
||||||
doForEachSubtype,
|
doForEachSubtype,
|
||||||
ensureFunctionSignaturesAreUnique,
|
ensureFunctionSignaturesAreUnique,
|
||||||
explodeGenericClass,
|
explodeGenericClass,
|
||||||
@ -17540,9 +17541,9 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
|
|||||||
return inferredReturnType ? { type: inferredReturnType, isIncomplete } : undefined;
|
return inferredReturnType ? { type: inferredReturnType, isIncomplete } : undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Determines whether the function consists only of a "raise" statement
|
// Determines whether the method consists only of a "raise" statement
|
||||||
// and the exception type raised is a NotImplementedError. This is commonly
|
// and the exception type raised is a NotImplementedError or a subclass
|
||||||
// used for abstract methods that
|
// thereof. This is commonly used for abstract methods.
|
||||||
function methodAlwaysRaisesNotImplemented(functionDecl?: FunctionDeclaration): boolean {
|
function methodAlwaysRaisesNotImplemented(functionDecl?: FunctionDeclaration): boolean {
|
||||||
if (
|
if (
|
||||||
!functionDecl ||
|
!functionDecl ||
|
||||||
@ -17564,7 +17565,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
|
|||||||
: isClassInstance(raiseType)
|
: isClassInstance(raiseType)
|
||||||
? raiseType
|
? raiseType
|
||||||
: undefined;
|
: undefined;
|
||||||
if (!classType || !ClassType.isBuiltIn(classType, 'NotImplementedError')) {
|
if (!classType || !derivesFromStdlibClass(classType, 'NotImplementedError')) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1856,6 +1856,10 @@ export function specializeForBaseClass(srcType: ClassType, baseClass: ClassType)
|
|||||||
return specializedType as ClassType;
|
return specializedType as ClassType;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function derivesFromStdlibClass(classType: ClassType, className: string) {
|
||||||
|
return classType.details.mro.some((mroClass) => isClass(mroClass) && ClassType.isBuiltIn(mroClass, className));
|
||||||
|
}
|
||||||
|
|
||||||
// If ignoreUnknown is true, an unknown base class is ignored when
|
// If ignoreUnknown is true, an unknown base class is ignored when
|
||||||
// checking for derivation. If ignoreUnknown is false, a return value
|
// checking for derivation. If ignoreUnknown is false, a return value
|
||||||
// of true is assumed.
|
// of true is assumed.
|
||||||
|
@ -12,7 +12,7 @@ def make_api_request(auth: str) -> str:
|
|||||||
return "meow"
|
return "meow"
|
||||||
|
|
||||||
|
|
||||||
def testfunc() -> None:
|
def func1() -> None:
|
||||||
resp = open("test")
|
resp = open("test")
|
||||||
auth = resp.read()
|
auth = resp.read()
|
||||||
|
|
||||||
|
@ -0,0 +1,33 @@
|
|||||||
|
# This sample tests return type annotations for functions that
|
||||||
|
# do not return.
|
||||||
|
|
||||||
|
from abc import ABC, abstractmethod
|
||||||
|
|
||||||
|
|
||||||
|
class OtherError(NotImplementedError):
|
||||||
|
...
|
||||||
|
|
||||||
|
|
||||||
|
class A(ABC):
|
||||||
|
def func1(self):
|
||||||
|
raise Exception("test")
|
||||||
|
|
||||||
|
def func2(self):
|
||||||
|
raise NotImplementedError()
|
||||||
|
|
||||||
|
def func3(self):
|
||||||
|
raise OtherError
|
||||||
|
|
||||||
|
@abstractmethod
|
||||||
|
def func4(self):
|
||||||
|
raise Exception()
|
||||||
|
|
||||||
|
|
||||||
|
def func1(a: A):
|
||||||
|
reveal_type(a.func1(), expected_text="NoReturn")
|
||||||
|
|
||||||
|
reveal_type(a.func2(), expected_text="Unknown")
|
||||||
|
|
||||||
|
reveal_type(a.func3(), expected_text="Unknown")
|
||||||
|
|
||||||
|
reveal_type(a.func4(), expected_text="Unknown")
|
@ -374,6 +374,11 @@ test('InferredTypes2', () => {
|
|||||||
TestUtils.validateResults(analysisResults, 0);
|
TestUtils.validateResults(analysisResults, 0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('InferredTypes3', () => {
|
||||||
|
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['inferredTypes3.py']);
|
||||||
|
TestUtils.validateResults(analysisResults, 0);
|
||||||
|
});
|
||||||
|
|
||||||
test('CallSite2', () => {
|
test('CallSite2', () => {
|
||||||
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['callSite2.py']);
|
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['callSite2.py']);
|
||||||
TestUtils.validateResults(analysisResults, 0);
|
TestUtils.validateResults(analysisResults, 0);
|
||||||
|
Loading…
Reference in New Issue
Block a user