From fcced9f318afb056f1fc6df9aa36d3b18980e9b2 Mon Sep 17 00:00:00 2001 From: detachhead Date: Wed, 27 Nov 2024 00:07:55 +1000 Subject: [PATCH 1/4] fix `strictGenericNarrowing` causing false positive `reportUnnecessaryIsInstance` errors when narrowing `Callable` --- .../pyright-internal/src/analyzer/typeUtils.ts | 5 ++--- .../tests/samples/typeNarrowingUsingBounds.py | 18 ++++++++++++++++-- .../typeNarrowingUsingBoundsDisabled.py | 18 ++++++++++++++++-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/typeUtils.ts b/packages/pyright-internal/src/analyzer/typeUtils.ts index c61d434fc..677f9283b 100644 --- a/packages/pyright-internal/src/analyzer/typeUtils.ts +++ b/packages/pyright-internal/src/analyzer/typeUtils.ts @@ -1090,9 +1090,8 @@ export const shouldUseVarianceForSpecialization = (typeToNarrow: Type, strictGen return allSubtypes( typeToNarrow, (subtype) => - subtype.category !== TypeCategory.Class || - // !ClassType.isSameGenericClass(subtype, narrowToType) || - subtype.shared.typeParams.length === 0 + (subtype.category !== TypeCategory.Class || subtype.shared.typeParams.length === 0) && + (subtype.category !== TypeCategory.Function || !subtype.priv.isCallableWithTypeArgs) ); }; diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py index d1f07adb6..4a06c52f4 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py @@ -1,4 +1,4 @@ -from typing import Any, Never, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible +from typing import Any, Never, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable class Covariant[T]: @@ -115,4 +115,18 @@ def _( value: str | Foo[str], ): if isinstance(value, Foo): - assert_type(value, Foo[str]) \ No newline at end of file + assert_type(value, Foo[str]) + +def _(f: Callable[[], None]): + if isinstance(f, staticmethod): + # narrowing Callable this way doesn't work so we use the old method. + # see https://github.com/DetachHead/basedpyright/issues/905 + assert_type(f, staticmethod[..., Any]) + +class CallableProtocol[**P, T](Protocol): + def __call__(self, *args: P.args, **kwargs: P.kwargs) -> T: ... + + +def _(f: CallableProtocol[[], None]): + if isinstance(f, staticmethod): + assert_type(f, staticmethod[[], None]) diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py index 7f5a658f4..11d0be78d 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py @@ -1,4 +1,4 @@ -from typing import Any, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible +from typing import Any, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable class Covariant[T]: @@ -115,4 +115,18 @@ def _( value: str | Foo[str], ): if isinstance(value, Foo): - assert_type(value, Foo[str]) \ No newline at end of file + assert_type(value, Foo[str]) + +def _(f: Callable[[], None]): + if isinstance(f, staticmethod): + # narrowing Callable this way doesn't work so we use the old method. + # see https://github.com/DetachHead/basedpyright/issues/905 + assert_type(f, staticmethod[..., Any]) + +class CallableProtocol[**P, T](Protocol): + def __call__(self, *args: P.args, **kwargs: P.kwargs) -> T: ... + + +def _(f: CallableProtocol[[], None]): + if isinstance(f, staticmethod): + assert_type(f, staticmethod[[], None]) From 7163a9b07368fca53270a17a9c817d21d339500c Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 1 Dec 2024 15:33:05 +1000 Subject: [PATCH 2/4] add a hacky method to narrow `Callable` types while keeping the generics --- .../src/analyzer/typeGuards.ts | 49 ++++++++++++++++++- .../src/analyzer/typeUtils.ts | 4 +- .../tests/samples/typeNarrowingUsingBounds.py | 9 ++-- .../typeNarrowingUsingBoundsDisabled.py | 2 - .../src/tests/typeEvaluatorBased.test.ts | 1 + 5 files changed, 55 insertions(+), 10 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/typeGuards.ts b/packages/pyright-internal/src/analyzer/typeGuards.ts index 3ebde159a..9290afe8e 100644 --- a/packages/pyright-internal/src/analyzer/typeGuards.ts +++ b/packages/pyright-internal/src/analyzer/typeGuards.ts @@ -10,6 +10,7 @@ */ import { assert } from '../common/debug'; +import { Uri } from '../common/uri/uri'; import { ArgCategory, AssignmentExpressionNode, @@ -95,6 +96,7 @@ import { specializeTupleClass, specializeWithUnknownTypeArgs, stripTypeForm, + synthesizeTypeVarForSelfCls, transformPossibleRecursiveTypeAlias, } from './typeUtils'; @@ -1814,7 +1816,13 @@ function narrowTypeForInstance( } } - if (isClass(subtype)) { + if ( + isClass(subtype) || + (getFileInfo(errorNode).diagnosticRuleSet.strictGenericNarrowing && isFunction(subtype)) + ) { + if (isFunction(subtype)) { + subtype = synthesizeCallableProtocolFromFunctionType(evaluator, subtype, errorNode); + } return combineTypes( filterClassType( unexpandedSubtype, @@ -1850,6 +1858,45 @@ function narrowTypeForInstance( return filteredType; } +/** + * the logic for narrowing `typing.Callable` (`FunctionType`) is completely different to the logic + * for narrowing callable protocols. when `typing.Callable`s are narrowed, it does not retain the + * generics from the supertype, so we create a fake callable protocol from a `FunctionType` so it + * can be narrowed using the same logic that's used to narrow `ClassType`s. + * + * this is not ideal and probably super hacky, but i couldnt figure out how to update the narrowing + * logic for `FunctionType` so this solution was easier. + */ +const synthesizeCallableProtocolFromFunctionType = ( + evaluator: TypeEvaluator, + callable: FunctionType, + errorNode: ParseNode +): ClassType => { + //TODO: fix hover text. currently this causes narrowed `FunctionType`s to display like this: + // "" + const callableType = ClassType.createInstantiable('Callable', '', '', Uri.empty(), 0, 0, undefined, undefined); + callableType.shared.baseClasses.push(evaluator.getBuiltInType(errorNode, 'Protocol')); + computeMroLinearization(callableType); + const fields = ClassType.getSymbolTable(callableType); + const callMethod = FunctionType.createSynthesizedInstance(callable.shared.name); + FunctionType.addParam( + callMethod, + FunctionParam.create( + ParamCategory.Simple, + synthesizeTypeVarForSelfCls(callableType, false), + FunctionParamFlags.TypeDeclared, + 'self' + ) + ); + for (const parameter of callable.shared.parameters) { + FunctionType.addParam(callMethod, parameter); + } + callMethod.shared.declaredReturnType = FunctionType.getEffectiveReturnType(callable); + const callSymbol = Symbol.createWithType(SymbolFlags.ClassMember, callMethod); + fields.set('__call__', callSymbol); + return ClassType.cloneAsInstance(callableType); +}; + // This function assumes that the caller has already verified that the two // types are the same class and are not literals. It also assumes that the // caller has verified that type1 is not assignable to type2 or vice versa. diff --git a/packages/pyright-internal/src/analyzer/typeUtils.ts b/packages/pyright-internal/src/analyzer/typeUtils.ts index 677f9283b..2b889e441 100644 --- a/packages/pyright-internal/src/analyzer/typeUtils.ts +++ b/packages/pyright-internal/src/analyzer/typeUtils.ts @@ -1089,9 +1089,7 @@ export const shouldUseVarianceForSpecialization = (typeToNarrow: Type, strictGen } return allSubtypes( typeToNarrow, - (subtype) => - (subtype.category !== TypeCategory.Class || subtype.shared.typeParams.length === 0) && - (subtype.category !== TypeCategory.Function || !subtype.priv.isCallableWithTypeArgs) + (subtype) => subtype.category !== TypeCategory.Class || subtype.shared.typeParams.length === 0 ); }; diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py index 4a06c52f4..da1ee2467 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py @@ -117,11 +117,12 @@ def _( if isinstance(value, Foo): assert_type(value, Foo[str]) -def _(f: Callable[[], None]): +def _(f: Callable[[int], str]): if isinstance(f, staticmethod): - # narrowing Callable this way doesn't work so we use the old method. - # see https://github.com/DetachHead/basedpyright/issues/905 - assert_type(f, staticmethod[..., Any]) + # can't use assert_type on the function itself, see TODO in synthesizeCallableProtocolFromFunctionType + assert_type(f(1), str) + assert_type(f.__call__, Callable[[int], str]) + reveal_type(f) class CallableProtocol[**P, T](Protocol): def __call__(self, *args: P.args, **kwargs: P.kwargs) -> T: ... diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py index 11d0be78d..b9fe3c0da 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py @@ -119,8 +119,6 @@ def _( def _(f: Callable[[], None]): if isinstance(f, staticmethod): - # narrowing Callable this way doesn't work so we use the old method. - # see https://github.com/DetachHead/basedpyright/issues/905 assert_type(f, staticmethod[..., Any]) class CallableProtocol[**P, T](Protocol): diff --git a/packages/pyright-internal/src/tests/typeEvaluatorBased.test.ts b/packages/pyright-internal/src/tests/typeEvaluatorBased.test.ts index c6e9efc1b..74396c1df 100644 --- a/packages/pyright-internal/src/tests/typeEvaluatorBased.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluatorBased.test.ts @@ -126,6 +126,7 @@ describe('narrowing type vars using their bounds', () => { const analysisResults = typeAnalyzeSampleFiles(['typeNarrowingUsingBounds.py'], configOptions); validateResultsButBased(analysisResults, { errors: [], + infos: [{ line: 124, message: 'Type of "f" is ""' }], }); }); test('disabled', () => { From 84a509b905221b53cd310cfd20ba10f33ddfaa10 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 1 Dec 2024 18:42:51 +1000 Subject: [PATCH 3/4] fix type being narrowed to `Never` when the type in the `isinstance` check is decorated with `@final` --- packages/pyright-internal/src/analyzer/typeGuards.ts | 11 ++++++++++- .../src/tests/samples/typeNarrowingUsingBounds.py | 6 +++++- .../tests/samples/typeNarrowingUsingBoundsDisabled.py | 5 +++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/typeGuards.ts b/packages/pyright-internal/src/analyzer/typeGuards.ts index 9290afe8e..02ca177f2 100644 --- a/packages/pyright-internal/src/analyzer/typeGuards.ts +++ b/packages/pyright-internal/src/analyzer/typeGuards.ts @@ -1874,7 +1874,16 @@ const synthesizeCallableProtocolFromFunctionType = ( ): ClassType => { //TODO: fix hover text. currently this causes narrowed `FunctionType`s to display like this: // "" - const callableType = ClassType.createInstantiable('Callable', '', '', Uri.empty(), 0, 0, undefined, undefined); + const callableType = ClassType.createInstantiable( + 'Callable', + '', + '', + Uri.empty(), + ClassTypeFlags.ProtocolClass, + 0, + undefined, + undefined + ); callableType.shared.baseClasses.push(evaluator.getBuiltInType(errorNode, 'Protocol')); computeMroLinearization(callableType); const fields = ClassType.getSymbolTable(callableType); diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py index da1ee2467..b6df0e847 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py @@ -1,5 +1,5 @@ from typing import Any, Never, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable - +from types import FunctionType class Covariant[T]: def foo(self, other: object): @@ -131,3 +131,7 @@ def __call__(self, *args: P.args, **kwargs: P.kwargs) -> T: ... def _(f: CallableProtocol[[], None]): if isinstance(f, staticmethod): assert_type(f, staticmethod[[], None]) + +def _(f: Callable[[int], str]): + if isinstance(f, FunctionType): + assert_type(f, FunctionType) \ No newline at end of file diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py index b9fe3c0da..4ffd3bb05 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py @@ -1,4 +1,5 @@ from typing import Any, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable +from types import FunctionType class Covariant[T]: @@ -128,3 +129,7 @@ def __call__(self, *args: P.args, **kwargs: P.kwargs) -> T: ... def _(f: CallableProtocol[[], None]): if isinstance(f, staticmethod): assert_type(f, staticmethod[[], None]) + +def _(f: Callable[[int], str]): + if isinstance(f, FunctionType): + assert_type(f, FunctionType) \ No newline at end of file From 848af1e4410945d87364dbb59f62cb8f7dbe391c Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 1 Dec 2024 20:02:00 +1000 Subject: [PATCH 4/4] fix `Callable` narrowing hack incorrectly being used on `TypeIs` --- packages/pyright-internal/src/analyzer/typeGuards.ts | 7 ++++++- .../src/tests/samples/typeNarrowingUsingBounds.py | 10 ++++++++-- .../tests/samples/typeNarrowingUsingBoundsDisabled.py | 10 ++++++++-- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/typeGuards.ts b/packages/pyright-internal/src/analyzer/typeGuards.ts index 02ca177f2..6a4e489f5 100644 --- a/packages/pyright-internal/src/analyzer/typeGuards.ts +++ b/packages/pyright-internal/src/analyzer/typeGuards.ts @@ -1818,7 +1818,12 @@ function narrowTypeForInstance( if ( isClass(subtype) || - (getFileInfo(errorNode).diagnosticRuleSet.strictGenericNarrowing && isFunction(subtype)) + // when strictGenericNarrowing is enabled, we need to convert the Callable type to a Callable + // protocol to make sure it keeps the generics when narrowing, but this is only needed for + // isinstance checks because you can't specify generics to it + (!isTypeIsCheck && + getFileInfo(errorNode).diagnosticRuleSet.strictGenericNarrowing && + isFunction(subtype)) ) { if (isFunction(subtype)) { subtype = synthesizeCallableProtocolFromFunctionType(evaluator, subtype, errorNode); diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py index b6df0e847..c5abf9b6a 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBounds.py @@ -1,4 +1,4 @@ -from typing import Any, Never, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable +from typing import Any, Never, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable, TypeIs from types import FunctionType class Covariant[T]: @@ -134,4 +134,10 @@ def _(f: CallableProtocol[[], None]): def _(f: Callable[[int], str]): if isinstance(f, FunctionType): - assert_type(f, FunctionType) \ No newline at end of file + assert_type(f, FunctionType) + +def takes_arg(value: object) -> TypeIs[Callable[[int], None]]: ... + +def _(value: Callable[[], None] | Callable[[int], None]): + if takes_arg(value): + assert_type(value, Callable[[int], None]) \ No newline at end of file diff --git a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py index 4ffd3bb05..9860d4358 100644 --- a/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py +++ b/packages/pyright-internal/src/tests/samples/typeNarrowingUsingBoundsDisabled.py @@ -1,4 +1,4 @@ -from typing import Any, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable +from typing import Any, assert_type, runtime_checkable, Protocol, Iterable, Iterator, MutableMapping, Reversible, Callable, TypeIs from types import FunctionType @@ -132,4 +132,10 @@ def _(f: CallableProtocol[[], None]): def _(f: Callable[[int], str]): if isinstance(f, FunctionType): - assert_type(f, FunctionType) \ No newline at end of file + assert_type(f, FunctionType) + +def takes_arg(value: object) -> TypeIs[Callable[[int], None]]: ... + +def _(value: Callable[[], None] | Callable[[int], None]): + if takes_arg(value): + assert_type(value, Callable[[int], None]) \ No newline at end of file