Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improved narrowing of generics #745

Merged
merged 17 commits into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions docs/benefits-over-pyright/improved-generic-narrowing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# improved type narrowing

when narrowing a type using an `isinstance` check, there's no way for the type checker to narrow its type variables, so pyright just narrows them to ["Unknown"](../usage/mypy-comparison.md#unknown-type-and-strict-mode)):

```py
def foo(value: object):
if isinstance(value, list):
reveal_type(value) # list[Unknown]
```

this makes sense in cases where the generic is invariant and there's no other way to represent any of its possibilities. for example if it were to be narrowed to `list[object]`, you wouldn't be able to assign `list[int]` to it. however in cases where the generic is covariant, contravariant, or uses constraints, it can be narrowed more accurately.

basedpyright introduces the new [`strictGenericNarrowing`](../configuration/config-files.md#strictGenericNarrowing) setting to address this. the following sections explain how this new behavior effects different types of generics.

## narrowing of covariant generics

when a type variable is covariant, its widest possible type is its bound, which defaults to `object`.

when `strictGenericNarrowing` is enabled, if a generic is covariant and does not have a bound, it gets narrowed to `object` instead of "Unknown":

```py
T_co = TypeVar("T_co", covariant=True)

class Foo(Generic[T_co]):
...

def foo(value: object):
if isinstance(value, Foo):
reveal_type(value) # Foo[object]
```

if the generic does have a bound, it gets narrowed to that bound instead:

```py
T_co = TypeVar("T_co", bound=int | str, covariant=True)

class Foo(Generic[T_co]):
...

def foo(value: object):
if isinstance(value, Foo):
reveal_type(value) # Foo[int | str]
```

## narrowing of contravariant generics

when a type variable is contravariant its widest possible type is `Never`, so when `strictGenericNarrowing` is enabled, contravariant generics get narrowed to `Never` instead of "Unknown":

```py
T_contra = TypeVar("T_contra", bound=int | str, covariant=True)

class Foo(Generic[T_contra]):
...

def foo(value: object):
if isinstance(value, Foo):
reveal_type(value) # Foo[Never]
```

## narrowing of constraints

when a type variable uses constraints, the rules of variance do not apply - see [this issue](https://github.com/DetachHead/basedpyright/issues/893) for more information. instead, a constraint declares that the generic must be resolved to be exactly one of the types specified.

when `strictGenericNarrowing` is enabled, constrained generics are narrowed to a union of all possibilities:

```py
class Foo[T: (int, str)]:
...

def foo(value: object):
if isinstance(value, Foo):
reveal_type(value) # Foo[int] | Foo[str]
```

this also works when there's more than one constrained type variable - it creates a union of all possible combinations:

```py

class Foo[T: (int, str), U: (float, bytes)]:
...

def foo(value: object):
if isinstance(value, Foo):
reveal_type(value) # Foo[int, float] | Foo[int, bytes] | Foo[str, float] | Foo[str, bytes]
```
34 changes: 19 additions & 15 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ The following settings control the *environment* in which basedpyright will chec

- **useLibraryCodeForTypes** [boolean]: Determines whether pyright reads, parses and analyzes library code to extract type information in the absence of type stub files. Type information will typically be incomplete. We recommend using type stubs where possible. The default value for this option is true.

### basedpyright exclusive settings

- <a name="failOnWarnings"></a> **failOnWarnings** [boolean]: Whether to exit with a non-zero exit code in the CLI if any `"warning"` diagnostics are reported. Has no effect on the language server. This is equivalent to the `--warnings` CLI argument.

## Type Evaluation Settings

The following settings determine how different types should be evaluated.
Expand All @@ -69,6 +73,10 @@ The following settings determine how different types should be evaluated.

- <a name="disableBytesTypePromotions"></a> **disableBytesTypePromotions** [boolean]: Disables legacy behavior where `bytearray` and `memoryview` are considered subtypes of `bytes`. [PEP 688](https://peps.python.org/pep-0688/#no-special-meaning-for-bytes) deprecates this behavior, but this switch is provided to restore the older behavior.

### basedpyright exclusive settings

- <a name="strictGenericNarrowing"></a> **strictGenericNarrowing** [boolean]: When a type is narrowed in such a way that its type parameters are not known (eg. using an `isinstance` check), basedpyright will resolve the type parameter to the generic's bound or constraint instead of `Any`. [more info](../benefits-over-pyright/improved-generic-narrowing.md)

## Diagnostic Categories

diagnostics can be configured to be reported as any of the following categories:
Expand Down Expand Up @@ -259,33 +267,29 @@ The following settings allow more fine grained control over the **typeCheckingMo

- <a name="reportShadowedImports"></a> **reportShadowedImports** [boolean or string, optional]: Generate or suppress diagnostics for files that are overriding a module in the stdlib.

## Based options

the following additional options are not available in regular pyright:

- <a name="failOnWarnings"></a> **failOnWarnings** [boolean]: Whether to exit with a non-zero exit code in the CLI if any `"warning"` diagnostics are reported. Has no effect on the language server. This is equivalent to the `--warnings` CLI argument.
### basedpyright exclusive settings

- <a name="reportUnreachable"></a> **reportUnreachable** [boolean or string, optional]: Generate or suppress diagnostics for unreachable code.
- <a name="reportUnreachable"></a> **reportUnreachable** [boolean or string, optional]: Generate or suppress diagnostics for unreachable code. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportunreachable)

- <a name="reportAny"></a> **reportAny** [boolean or string, optional]: Generate or suppress diagnostics for expressions that have the `Any` type. this accounts for all scenarios not covered by the `reportUnknown*` rules (since "Unknown" isn't a real type, but a distinction pyright makes to disallow the `Any` type only in certain circumstances).
- <a name="reportAny"></a> **reportAny** [boolean or string, optional]: Generate or suppress diagnostics for expressions that have the `Any` type. this accounts for all scenarios not covered by the `reportUnknown*` rules (since "Unknown" isn't a real type, but a distinction pyright makes to disallow the `Any` type only in certain circumstances). [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportany)

- <a name="reportExplicitAny"></a> **reportExplicitAny** [boolean or string, optional]: Ban all explicit usages of the `Any` type. While `reportAny` bans expressions typed as `Any`, this rule bans using the `Any` type directly eg. in a type annotation.
- <a name="reportExplicitAny"></a> **reportExplicitAny** [boolean or string, optional]: Ban all explicit usages of the `Any` type. While `reportAny` bans expressions typed as `Any`, this rule bans using the `Any` type directly eg. in a type annotation. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportexplicitany)

- <a name="reportIgnoreCommentWithoutRule"></a> **reportIgnoreCommentWithoutRule** [boolean or string, optional]: Enforce that all `# type:ignore`/`# pyright:ignore` comments specify a rule in brackets (eg. `# pyright:ignore[reportAny]`)
- <a name="reportIgnoreCommentWithoutRule"></a> **reportIgnoreCommentWithoutRule** [boolean or string, optional]: Enforce that all `# type:ignore`/`# pyright:ignore` comments specify a rule in brackets (eg. `# pyright:ignore[reportAny]`). [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportignorecommentwithoutrule)

- <a name="reportPrivateLocalImportUsage"></a> **reportPrivateLocalImportUsage** [boolean or string, optional]: Generate or suppress diagnostics for use of a symbol from a local module that is not meant to be exported from that module. Like `reportPrivateImportUsage` but also checks imports from your own code.
- <a name="reportPrivateLocalImportUsage"></a> **reportPrivateLocalImportUsage** [boolean or string, optional]: Generate or suppress diagnostics for use of a symbol from a local module that is not meant to be exported from that module. Like `reportPrivateImportUsage` but also checks imports from your own code. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportprivatelocalimportusage)

- <a name="reportImplicitRelativeImport"></a> **reportImplicitRelativeImport** [boolean or string, optional]: Generate or suppress diagnostics for non-relative imports that do not specify the full path to the module.
- <a name="reportImplicitRelativeImport"></a> **reportImplicitRelativeImport** [boolean or string, optional]: Generate or suppress diagnostics for non-relative imports that do not specify the full path to the module. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportimplicitrelativeimport)

- <a name="reportInvalidCast"></a> **reportInvalidCast** [boolean or string, optional]: Generate or suppress diagnostics for `cast`s to non-overlapping types.
- <a name="reportInvalidCast"></a> **reportInvalidCast** [boolean or string, optional]: Generate or suppress diagnostics for `cast`s to non-overlapping types. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportinvalidcast)

- <a name="reportUnsafeMultipleInheritance"></a> **reportUnsafeMultipleInheritance** [boolean or string, optional]: Generate or suppress diagnostics for classes that inherit from multiple base classes with an `__init__` or `__new__` method, which is unsafe because those additional constructors may either never get called or get called with invalid arguments.
- <a name="reportUnsafeMultipleInheritance"></a> **reportUnsafeMultipleInheritance** [boolean or string, optional]: Generate or suppress diagnostics for classes that inherit from multiple base classes with an `__init__` or `__new__` method, which is unsafe because those additional constructors may either never get called or get called with invalid arguments. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportunsafemultipleinheritance)

- <a name="reportUnusedParameter"></a> **reportUnusedParameter** [boolean or string, optional]: Generate or suppress diagnostics for unused function parameters.

- <a name="reportImplicitAbstractClass"></a> **reportImplicitAbstractClass** [boolean or string, optional]: Diagnostics for classes that extend abstract classes without also explicitly declaring themselves as abstract or implementing all of the required abstract methods.
- <a name="reportImplicitAbstractClass"></a> **reportImplicitAbstractClass** [boolean or string, optional]: Diagnostics for classes that extend abstract classes without also explicitly declaring themselves as abstract or implementing all of the required abstract methods. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportimplicitabstractclass)

- <a name="reportUnannotatedClassAttribute"></a> **reportUnannotatedClassAttribute** [boolean or string, optional]: Generate or suppress diagnostics for class attribute declarations that do not have a type annotation. These are unsafe because for performance reasons, subtypes are not validated to ensure that they are compatible with the supertype.
- <a name="reportUnannotatedClassAttribute"></a> **reportUnannotatedClassAttribute** [boolean or string, optional]: Generate or suppress diagnostics for class attribute declarations that do not have a type annotation. These are unsafe because for performance reasons, subtypes are not validated to ensure that they are compatible with the supertype. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportunannotatedclassattribute)

## Discouraged options

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"test-python": "uv run --no-sync pytest tests",
"generate-docstubs": "uv run --no-sync based_build/generate_docstubs.py",
"localization-helper": "uv run --no-sync based_build/localization_helper.py",
"docs": "uv run mkdocs serve"
"docs": "uv run --no-sync mkdocs serve"
},
"devDependencies": {
"@detachhead/ts-helpers": "^16.2.0",
Expand Down
7 changes: 6 additions & 1 deletion packages/pyright-internal/src/analyzer/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3793,7 +3793,12 @@ export class Checker extends ParseTreeWalker {
return;
}

const classTypeList = getIsInstanceClassTypes(this._evaluator, arg1Type);
const classTypeList = getIsInstanceClassTypes(
this._evaluator,
arg1Type,
arg0Type,
this._fileInfo.diagnosticRuleSet.strictGenericNarrowing
);
if (!classTypeList) {
return;
}
Expand Down
11 changes: 10 additions & 1 deletion packages/pyright-internal/src/analyzer/patternMatching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
PatternSequenceNode,
PatternValueNode,
} from '../parser/parseNodes';
import { getFileInfo } from './analyzerNodeInfo';
import { CodeFlowReferenceExpressionNode } from './codeFlowTypes';
import { addConstraintsForExpectedType } from './constraintSolver';
import { ConstraintTracker } from './constraintTracker';
Expand Down Expand Up @@ -84,6 +85,7 @@ import {
mapSubtypes,
partiallySpecializeType,
preserveUnknown,
shouldUseVarianceForSpecialization,
specializeTupleClass,
specializeWithUnknownTypeArgs,
transformPossibleRecursiveTypeAlias,
Expand Down Expand Up @@ -774,7 +776,14 @@ function narrowTypeBasedOnClassPattern(
// specialize it with Unknown type arguments.
if (isClass(exprType) && !exprType.props?.typeAliasInfo) {
exprType = ClassType.cloneRemoveTypePromotions(exprType);
exprType = specializeWithUnknownTypeArgs(exprType, evaluator.getTupleClassType());
evaluator.inferVarianceForClass(exprType);
exprType = specializeWithUnknownTypeArgs(
exprType,
evaluator.getTupleClassType(),
shouldUseVarianceForSpecialization(type, getFileInfo(pattern).diagnosticRuleSet.strictGenericNarrowing)
? evaluator.getObjectType()
: undefined
);
}

// Are there any positional arguments? If so, try to get the mappings for
Expand Down
84 changes: 54 additions & 30 deletions packages/pyright-internal/src/analyzer/typeGuards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ import {
makeTypeVarsFree,
mapSubtypes,
MemberAccessFlags,
shouldUseVarianceForSpecialization,
specializeTupleClass,
specializeWithUnknownTypeArgs,
stripTypeForm,
Expand Down Expand Up @@ -623,7 +624,12 @@ export function getTypeNarrowingCallback(
const arg1TypeResult = evaluator.getTypeOfExpression(arg1Expr, EvalFlags.IsInstanceArgDefaults);
const arg1Type = arg1TypeResult.type;

const classTypeList = getIsInstanceClassTypes(evaluator, arg1Type);
const classTypeList = getIsInstanceClassTypes(
evaluator,
arg1Type,
evaluator.getTypeOfExpression(arg0Expr).type,
getFileInfo(testExpression).diagnosticRuleSet.strictGenericNarrowing
);
const isIncomplete = !!callTypeResult.isIncomplete || !!arg1TypeResult.isIncomplete;

if (classTypeList) {
Expand Down Expand Up @@ -1125,45 +1131,63 @@ function narrowTypeForIsEllipsis(evaluator: TypeEvaluator, node: ExpressionNode,
// which form and returns a list of classes or undefined.
export function getIsInstanceClassTypes(
evaluator: TypeEvaluator,
argType: Type
argType: Type,
typeToNarrow: Type,
strictGenericNarrowing: boolean
): (ClassType | TypeVarType | FunctionType)[] | undefined {
let foundNonClassType = false;
const classTypeList: (ClassType | TypeVarType | FunctionType)[] = [];

// Create a helper function that returns a list of class types or
// undefined if any of the types are not valid.
const addClassTypesToList = (types: Type[]) => {
types.forEach((subtype) => {
if (isClass(subtype)) {
subtype = specializeWithUnknownTypeArgs(subtype, evaluator.getTupleClassType());

if (isInstantiableClass(subtype) && ClassType.isBuiltIn(subtype, 'Callable')) {
subtype = convertToInstantiable(getUnknownTypeForCallable());
types.forEach((type) => {
const subtypes: Type[] = [];
if (isClass(type)) {
const useVariance = shouldUseVarianceForSpecialization(typeToNarrow, strictGenericNarrowing);
if (useVariance) {
evaluator.inferVarianceForClass(type);
}
type = specializeWithUnknownTypeArgs(
type,
evaluator.getTupleClassType(),
useVariance ? evaluator.getObjectType() : undefined
);

doForEachSubtype(type, (subtype) => {
if (isInstantiableClass(subtype) && ClassType.isBuiltIn(subtype, 'Callable')) {
subtypes.push(convertToInstantiable(getUnknownTypeForCallable()));
} else {
subtypes.push(subtype);
}
});
} else {
subtypes.push(type);
}

if (isInstantiableClass(subtype)) {
// If this is a reference to a class that has type promotions (e.g.
// float or complex), remove the promotions for purposes of the
// isinstance check).
if (!subtype.priv.includeSubclasses && subtype.priv.includePromotions) {
subtype = ClassType.cloneRemoveTypePromotions(subtype);
for (let subtype of subtypes) {
if (isInstantiableClass(subtype)) {
// If this is a reference to a class that has type promotions (e.g.
// float or complex), remove the promotions for purposes of the
// isinstance check).
if (!subtype.priv.includeSubclasses && subtype.priv.includePromotions) {
subtype = ClassType.cloneRemoveTypePromotions(subtype);
}
classTypeList.push(subtype);
} else if (isTypeVar(subtype) && TypeBase.isInstantiable(subtype)) {
classTypeList.push(subtype);
} else if (isNoneTypeClass(subtype)) {
assert(isInstantiableClass(subtype));
classTypeList.push(subtype);
} else if (
isFunction(subtype) &&
subtype.shared.parameters.length === 2 &&
subtype.shared.parameters[0].category === ParamCategory.ArgsList &&
subtype.shared.parameters[1].category === ParamCategory.KwargsDict
) {
classTypeList.push(subtype);
} else {
foundNonClassType = true;
}
classTypeList.push(subtype);
} else if (isTypeVar(subtype) && TypeBase.isInstantiable(subtype)) {
classTypeList.push(subtype);
} else if (isNoneTypeClass(subtype)) {
assert(isInstantiableClass(subtype));
classTypeList.push(subtype);
} else if (
isFunction(subtype) &&
subtype.shared.parameters.length === 2 &&
subtype.shared.parameters[0].category === ParamCategory.ArgsList &&
subtype.shared.parameters[1].category === ParamCategory.KwargsDict
) {
classTypeList.push(subtype);
} else {
foundNonClassType = true;
}
});
};
Expand Down
Loading
Loading