Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions .basedpyright/baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,14 @@
"lineCount": 1
}
},
{
"code": "reportUnsolvedTypeVar",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I like this name.

"Solving" a type variable is a perspective of the type-checker implementer, that the type-checker user shouldn't need to think about.

The user doesn't need to think about "solving" things, or "solutions".

This kind of seems like something for the family of reportUnknown... rules.
reportUnknownTypeVar?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while the term is possibly confusing since type checkers seem to have different words for it (pyright uses the word "specialize" but idk if that term is unique to pyright), i don't consider the actual resolving of a type var to be an implementation detail. the user should be able to clearly tell when using a function or class with a generic whether that generic was able to be inferred or not, regardless of what word is used to describe that process.

i agree reportUnknownTypeVar sounds like a better name, and is more consistent with other similar rules

"range": {
"startColumn": 4,
"endColumn": 5,
"lineCount": 7
}
},
{
"code": "reportUnusedCallResult",
"range": {
Expand All @@ -305,6 +313,14 @@
"lineCount": 7
}
},
{
"code": "reportUnsolvedTypeVar",
"range": {
"startColumn": 4,
"endColumn": 5,
"lineCount": 6
}
},
{
"code": "reportUnusedCallResult",
"range": {
Expand Down
42 changes: 40 additions & 2 deletions packages/pyright-internal/src/analyzer/constructors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import { appendArray } from '../common/collectionUtils';
import { DiagnosticAddendum } from '../common/diagnostic';
import { DiagnosticRule } from '../common/diagnosticRules';
import { LocMessage } from '../localization/localize';
import { ExpressionNode, ParamCategory } from '../parser/parseNodes';
import { ConstraintSolution } from './constraintSolution';
import { addConstraintsForExpectedType } from './constraintSolver';
Expand All @@ -40,7 +41,9 @@ import {
isInstantiableClass,
isNever,
isOverloaded,
isParamSpec,
isTypeVar,
isTypeVarTuple,
isUnknown,
} from './types';
import {
Expand Down Expand Up @@ -478,6 +481,10 @@ function validateNewMethod(
newReturnType = applyExpectedTypeForConstructor(evaluator, type, inferenceContext, constraints);
}

if (!argumentErrors && !isTypeIncomplete && !useSpeculativeModeForArgs) {
reportUnsolvedClassTypeParams(evaluator, type, constraints, errorNode);
}

return { argumentErrors, returnType: newReturnType, isTypeIncomplete, overloadsUsedForCall };
}

Expand Down Expand Up @@ -531,8 +538,11 @@ function validateInitMethod(

if (callResult.argumentErrors) {
argumentErrors = true;
} else if (callResult.overloadsUsedForCall) {
overloadsUsedForCall.push(...callResult.overloadsUsedForCall);
} else {
if (callResult.overloadsUsedForCall) {
overloadsUsedForCall.push(...callResult.overloadsUsedForCall);
}
reportUnsolvedClassTypeParams(evaluator, adjustedClassType, constraints, errorNode);
}

return { argumentErrors, returnType, isTypeIncomplete, overloadsUsedForCall };
Expand Down Expand Up @@ -640,6 +650,34 @@ function applyExpectedSubtypeForConstructor(
return specializedType;
}

// Reports errors for class-level type parameters that have no solution in the constraints.
function reportUnsolvedClassTypeParams(
evaluator: TypeEvaluator,
classType: ClassType,
constraints: ConstraintTracker,
errorNode: ExpressionNode
): void {
if (classType.shared.typeParams.length === 0 || classType.priv.typeArgs) {
return;
}
const constraintSet = constraints.getMainConstraintSet();
for (const typeParam of classType.shared.typeParams) {
if (typeParam.shared.isDefaultExplicit) {
continue;
}
if (isParamSpec(typeParam) || isTypeVarTuple(typeParam)) {
continue;
}
if (!constraintSet.getTypeVar(typeParam)?.lowerBound) {
evaluator.addDiagnostic(
DiagnosticRule.reportUnsolvedTypeVar,
LocMessage.typeVarUnsolved().format({ name: typeParam.shared.name }),
errorNode
);
}
}
}

// Handles the case where a constructor is a generic type and the type
// arguments are not specified but can be provided by the expected type.
function applyExpectedTypeForConstructor(
Expand Down
55 changes: 55 additions & 0 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the primer has raised some false positives, or at least scenarios that need investigating

Original file line number Diff line number Diff line change
Expand Up @@ -12617,6 +12617,50 @@ export function createTypeEvaluator(
specializedInitSelfType = solveAndApplyConstraints(specializedInitSelfType, constraints);
}

// check for unsolved type variables and report errors
if (!argumentErrors && !isTypeIncomplete && type.shared.typeParams.length > 0) {
const solution = solveConstraints(evaluatorInterface, constraints);

const solutionSet = solution.getMainSolutionSet();
const scopeIds = getTypeVarScopeIds(type);
const unsolvedExemptTypeVars = getUnknownExemptTypeVarsForReturnType(type, returnType);

for (const typeParam of type.shared.typeParams) {
// skip type parameters that have explicit default values
if (typeParam.shared.isDefaultExplicit) {
continue;
}

// skip ParamSpecs and TypeVarTuples for now - they have different semantics
if (isParamSpec(typeParam) || isTypeVarTuple(typeParam)) {
continue;
}

// only check type parameters that are in scope for this function
if (typeParam.priv.scopeId && scopeIds && !scopeIds.includes(typeParam.priv.scopeId)) {
continue;
}

// skip type parameters that are exempt from being solved (e.g., they appear
// only in the return type as part of a returned callable)
if (unsolvedExemptTypeVars.some((exemptVar) => isTypeSame(exemptVar, typeParam))) {
continue;
}

const solvedType = solutionSet.getType(typeParam);

if (!solvedType) {
addDiagnostic(
DiagnosticRule.reportUnsolvedTypeVar,
LocMessage.typeVarUnsolved().format({
name: typeParam.shared.name,
}),
errorNode
);
}
}
}

matchResults.argumentMatchScore = argumentMatchScore;

return {
Expand Down Expand Up @@ -15077,6 +15121,17 @@ export function createTypeEvaluator(
}
} else {
isEmptyContainer = true;

// Report unsolved type variable for empty containers without expected type
if (!hasExpectedType) {
const diag = new DiagnosticAddendum();
diag.addMessage(LocAddendum.typeVarUnsolvedCollectionHint().format({ className: builtInClassName }));
addDiagnostic(
DiagnosticRule.reportUnsolvedTypeVar,
LocMessage.typeVarUnsolved().format({ name: 'T' }) + diag.getString(),
node
);
}
}

const listOrSetClass = getBuiltInType(node, builtInClassName);
Expand Down
8 changes: 8 additions & 0 deletions packages/pyright-internal/src/common/configOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ export interface DiagnosticRuleSet {
reportIncompatibleUnannotatedOverride: DiagnosticLevel;
reportInvalidAbstractMethod: DiagnosticLevel;
reportSelfClsDefault: DiagnosticLevel;
reportUnsolvedTypeVar: DiagnosticLevel;
allowedUntypedLibraries: string[];
}

Expand Down Expand Up @@ -564,6 +565,7 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportIncompatibleUnannotatedOverride,
DiagnosticRule.reportInvalidAbstractMethod,
DiagnosticRule.reportSelfClsDefault,
DiagnosticRule.reportUnsolvedTypeVar,
];
}

Expand Down Expand Up @@ -703,6 +705,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportSelfClsDefault: 'none',
reportUnsolvedTypeVar: 'none',
allowedUntypedLibraries: [],
};

Expand Down Expand Up @@ -824,6 +827,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportSelfClsDefault: 'none',
reportUnsolvedTypeVar: 'none',
allowedUntypedLibraries: [],
};

Expand Down Expand Up @@ -945,6 +949,7 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet {
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportSelfClsDefault: 'none',
reportUnsolvedTypeVar: 'none',
allowedUntypedLibraries: [],
};

Expand Down Expand Up @@ -1065,6 +1070,7 @@ export const getRecommendedDiagnosticRuleSet = (): DiagnosticRuleSet => ({
reportIncompatibleUnannotatedOverride: 'none', // TODO: change to error when we're confident there's no performance issues with this rule
reportInvalidAbstractMethod: 'warning',
reportSelfClsDefault: 'warning',
reportUnsolvedTypeVar: 'error',
allowedUntypedLibraries: [],
});

Expand Down Expand Up @@ -1182,6 +1188,7 @@ export const getAllDiagnosticRuleSet = (): DiagnosticRuleSet => ({
reportIncompatibleUnannotatedOverride: 'error',
reportInvalidAbstractMethod: 'error',
reportSelfClsDefault: 'error',
reportUnsolvedTypeVar: 'error',
allowedUntypedLibraries: [],
});

Expand Down Expand Up @@ -1300,6 +1307,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportSelfClsDefault: 'none',
reportUnsolvedTypeVar: 'none',
allowedUntypedLibraries: [],
};

Expand Down
1 change: 1 addition & 0 deletions packages/pyright-internal/src/common/diagnosticRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,5 @@ export enum DiagnosticRule {
reportIncompatibleUnannotatedOverride = 'reportIncompatibleUnannotatedOverride',
reportInvalidAbstractMethod = 'reportInvalidAbstractMethod',
reportSelfClsDefault = 'reportSelfClsDefault',
reportUnsolvedTypeVar = 'reportUnsolvedTypeVar',
}
6 changes: 6 additions & 0 deletions packages/pyright-internal/src/localization/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,8 @@ export namespace Localizer {
new ParameterizedString<{ type: string; name: string }>(
getRawString('Diagnostic.typeVarAssignmentMismatch')
);
export const typeVarUnsolved = () =>
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.typeVarUnsolved'));
export const typeVarBoundAndConstrained = () => getRawString('Diagnostic.typeVarBoundAndConstrained');
export const typeVarBoundGeneric = () => getRawString('Diagnostic.typeVarBoundGeneric');
export const typeVarConstraintGeneric = () => getRawString('Diagnostic.typeVarConstraintGeneric');
Expand Down Expand Up @@ -1670,6 +1672,10 @@ export namespace Localizer {
new ParameterizedString<{ exceptionType: string; parentType: string }>(
getRawString('DiagnosticAddendum.unreachableExcept')
);
export const typeVarUnsolvedCollectionHint = () =>
new ParameterizedString<{ className: string }>(
getRawString('DiagnosticAddendum.typeVarUnsolvedCollectionHint')
);
export const useDictInstead = () => getRawString('DiagnosticAddendum.useDictInstead');
export const useListInstead = () => getRawString('DiagnosticAddendum.useListInstead');
export const useTupleInstead = () => getRawString('DiagnosticAddendum.useTupleInstead');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,9 @@
"comment": "{Locked='TypeVar'}"
},
"typeVarAssignmentMismatch": "Type \"{type}\" cannot be assigned to type variable \"{name}\"",
"typeVarUnsolved": {
"message": "Type variable \"{name}\" has no solution; consider providing it explicitly"
},
"typeVarBoundAndConstrained": {
"message": "TypeVar cannot be both bound and constrained",
"comment": "{Locked='TypeVar'}"
Expand Down Expand Up @@ -2185,6 +2188,10 @@
"message": "Use tuple[T1, ..., Tn] to indicate a tuple type or T1 | T2 to indicate a union type",
"comment": "{Locked='tuple[T1, ..., Tn]','tuple','T1 | T2','union'}"
},
"typeVarUnsolvedCollectionHint": {
"message": "e.g. \"{className}[<type>]()\"",
"comment": "{Locked='{className}[<type>]()'}"
},
"useTypeInstead": {
"message": "Use type[T] instead",
"comment": "{Locked='type[T]'}"
Expand Down
72 changes: 72 additions & 0 deletions packages/pyright-internal/src/tests/samples/typeVarUnsolved1.py

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the example from that issue still doesn't report an error:

a = []

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the error message is pretty confusing in this case:

Type variable "T" has no solution; consider providing it explicitly

maybe for list/set/dict/etc. literals it should give an example of how to fix it, eg:

Type variable "T" has no solution; consider providing it explicitly (eg. list[int]())

maybe that can be a diagnostic addendum

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I like suggesting
a = list[int]()
when
a: list[int] = []
would have better performance.

Comment thread
KotlinIsland marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# this sample tests the detection of unsolved type variables in function calls

from typing import Any, Callable

def func1[T]() -> T: ...


def func2[T](x: T) -> T:
return x


def func3[T, U](x: T) -> U: ...


def func4[T = int]() -> T: ...


def func5[T, U = str](x: T) -> U: ...


# Error: T cannot be inferred
a = func1()

# OK: T is inferred from argument
b = func2(1)

# Error: U cannot be inferred (T is fine)
c = func3(1) # pyright: ignore[reportUnsolvedTypeVar]

# OK: T has a default value
d = func4()

# OK: T is inferred, U has a default value
e = func5(1)


# test with explicit type arguments
f: int = func1()

class MyClass[T]: ...

# ok: T is explicit
i = MyClass[int]()

# error: can't infer class type from constructor
i = MyClass() # pyright: ignore[reportUnsolvedTypeVar]

class Default[T=Any]: ...

# ok: T is explicit
i = Default[int]()

# ok: uses default
i = Default()

def deco[T]() -> Callable[[Callable[[], T]], Callable[[], T]]: ...

# ok: T is None
@deco()
def func6():
pass

deco()(lambda: None)

# OK: T is inferred from argument
j: int = func2(func1())

# error
k = []

# no error
l: list[int] = []
Loading
Loading