Skip to content
Draft
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
129 changes: 113 additions & 16 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13503,7 +13503,52 @@ export function createTypeEvaluator(
const paramName = paramNameNode ? paramNameNode.d.value : undefined;

if (paramName) {
if (paramName === 'default') {
if (paramName === 'covariant') {
if (argList[i].valueExpression && getBooleanValue(argList[i].valueExpression!)) {
if (
typeVar.shared.declaredVariance === Variance.Contravariant ||
typeVar.shared.declaredVariance === Variance.Auto
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeVarVariance(),
argList[i].valueExpression!
);
} else {
typeVar.shared.declaredVariance = Variance.Covariant;
}
}
} else if (paramName === 'contravariant') {
if (argList[i].valueExpression && getBooleanValue(argList[i].valueExpression!)) {
if (
typeVar.shared.declaredVariance === Variance.Covariant ||
typeVar.shared.declaredVariance === Variance.Auto
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeVarVariance(),
argList[i].valueExpression!
);
} else {
typeVar.shared.declaredVariance = Variance.Contravariant;
}
}
} else if (paramName === 'infer_variance') {
if (argList[i].valueExpression && getBooleanValue(argList[i].valueExpression!)) {
if (
typeVar.shared.declaredVariance === Variance.Covariant ||
typeVar.shared.declaredVariance === Variance.Contravariant
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeVarVariance(),
argList[i].valueExpression!
);
} else {
typeVar.shared.declaredVariance = Variance.Auto;
}
}
} else if (paramName === 'default') {
const expr = argList[i].valueExpression;
if (expr) {
const defaultType = getTypeVarTupleDefaultType(expr, /* isPep695Syntax */ false);
Expand Down Expand Up @@ -13593,7 +13638,52 @@ export function createTypeEvaluator(
const paramName = paramNameNode ? paramNameNode.d.value : undefined;

if (paramName) {
if (paramName === 'default') {
if (paramName === 'covariant') {
if (argList[i].valueExpression && getBooleanValue(argList[i].valueExpression!)) {
if (
paramSpec.shared.declaredVariance === Variance.Contravariant ||
paramSpec.shared.declaredVariance === Variance.Auto
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeVarVariance(),
argList[i].valueExpression!
);
} else {
paramSpec.shared.declaredVariance = Variance.Covariant;
}
}
} else if (paramName === 'contravariant') {
if (argList[i].valueExpression && getBooleanValue(argList[i].valueExpression!)) {
if (
paramSpec.shared.declaredVariance === Variance.Covariant ||
paramSpec.shared.declaredVariance === Variance.Auto
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeVarVariance(),
argList[i].valueExpression!
);
} else {
paramSpec.shared.declaredVariance = Variance.Contravariant;
}
}
} else if (paramName === 'infer_variance') {
if (argList[i].valueExpression && getBooleanValue(argList[i].valueExpression!)) {
if (
paramSpec.shared.declaredVariance === Variance.Covariant ||
paramSpec.shared.declaredVariance === Variance.Contravariant
) {
addDiagnostic(
DiagnosticRule.reportGeneralTypeIssues,
LocMessage.typeVarVariance(),
argList[i].valueExpression!
);
} else {
paramSpec.shared.declaredVariance = Variance.Auto;
}
}
} else if (paramName === 'default') {
Comment on lines +13641 to +13686
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.

duplicated code

const expr = argList[i].valueExpression;
if (expr) {
const defaultType = getParamSpecDefaultType(expr, /* isPep695Syntax */ false);
Expand Down Expand Up @@ -18690,24 +18780,33 @@ export function createTypeEvaluator(
undefined
);

classType.shared.typeParams.forEach((param, paramIndex) => {
// Skip TypeVarTuples and ParamSpecs.
if (isTypeVarTuple(param) || isParamSpec(param)) {
return;
}
// A scopeless ParamSpec used as the "top type" substitute for ParamSpec parameters
// during variance inference. It has no scope ID so makeTypeVarsBound leaves it free,
// giving us the needed asymmetry:
// - as src in assignBoundTypeVar: fails (not the gradual `...` form) → covariant check fails
// - as dest in assignTypeVar: returns true immediately (no scopeId) → contravariant check passes
const dummyParamSpec = TypeVarType.createInstantiable('__varianceDummyP', TypeVarKind.ParamSpec);
const dummyTypeVarTuple = TypeVarType.createInstantiable('__varianceDummyTs', TypeVarKind.TypeVarTuple);

classType.shared.typeParams.forEach((param, paramIndex) => {
// Skip type variables without auto-variance.
if (param.shared.declaredVariance !== Variance.Auto) {
return;
}

// Replace all type arguments with a dummy type except for the
// TypeVar of interest, which is replaced with an object instance.
// TypeVar of interest. For regular TypeVars use an object instance;
// for ParamSpec/TypeVarTuple use a scopeless dummy (object would become
// the gradual `...` form, making both checks pass; TypeVarTuple at paramIndex
// was previously returned as itself, making srcTypeArgs === destTypeArgs).
const srcTypeArgs = classType.shared.typeParams.map((p, i) => {
if (isTypeVarTuple(p)) {
return p;
if (i === paramIndex) {
if (isParamSpec(p)) return dummyParamSpec;
if (isTypeVarTuple(p)) return dummyTypeVarTuple;
return getObjectType();
}
return i === paramIndex ? getObjectType() : dummyTypeObject;
if (isTypeVarTuple(p)) return p;
return dummyTypeObject;
});

// Replace all type arguments with a dummy type except for the
Expand Down Expand Up @@ -23062,16 +23161,14 @@ export function createTypeEvaluator(
if (scopeNode.nodeType === ParseNodeType.Class) {
scopeType = TypeVarScopeType.Class;

// Set the variance to "auto" for class-scoped TypeVars.
typeVar.shared.declaredVariance =
isParamSpec(typeVar) || isTypeVarTuple(typeVar) ? Variance.Invariant : Variance.Auto;
// Set the variance to "auto" for class-scoped type variables
typeVar.shared.declaredVariance = Variance.Auto;
} else if (scopeNode.nodeType === ParseNodeType.Function) {
scopeType = TypeVarScopeType.Function;
} else {
assert(scopeNode.nodeType === ParseNodeType.TypeAlias);
scopeType = TypeVarScopeType.TypeAlias;
typeVar.shared.declaredVariance =
isParamSpec(typeVar) || isTypeVarTuple(typeVar) ? Variance.Invariant : Variance.Auto;
typeVar.shared.declaredVariance = Variance.Auto;
}

typeVar = TypeVarType.cloneForScopeId(
Expand Down
6 changes: 1 addition & 5 deletions packages/pyright-internal/src/analyzer/typeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3212,7 +3212,7 @@ export function isVarianceOfTypeArgCompatible(type: Type, typeParamVariance: Var
return true;
}

if (isTypeVar(type) && !isParamSpec(type) && !isTypeVarTuple(type)) {
if (isTypeVar(type)) {
const typeArgVariance = type.shared.declaredVariance;

if (typeArgVariance === Variance.Contravariant || typeArgVariance === Variance.Covariant) {
Expand All @@ -3223,10 +3223,6 @@ export function isVarianceOfTypeArgCompatible(type: Type, typeParamVariance: Var
return type.shared.typeParams.every((typeParam, index) => {
let typeArgType: Type | undefined;

if (isParamSpec(typeParam) || isTypeVarTuple(typeParam)) {
return true;
}

if (type.priv.typeArgs && index < type.priv.typeArgs.length) {
typeArgType = type.priv.typeArgs[index];
}
Expand Down
31 changes: 31 additions & 0 deletions packages/pyright-internal/src/tests/samples/paramSpecVariance1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# This sample tests variance inference for TypeVarTuple type parameters.

from typing import Callable, ParamSpec

P_co = ParamSpec("P_co", covariant=True)
P_contra = ParamSpec("P_contra", contravariant=True)
P_infer = ParamSpec("P_infer", infer_variance=True)
Comment on lines +5 to +7
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.

how come these are unused

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.

is it just to make sure theres no error on the arguments? if so should add a comment



class ShouldBeContravariant1[**OutP]:
def f(self) -> Callable[OutP, None]: ...
Comment on lines +10 to +11
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.

should also have a test for the most common usage:

class ShouldBeContravariant2[**OutP]:
    def f(self, *args: P.args, **kwargs: P.kwargs): ...



vcontra1_1: ShouldBeContravariant1[object] = ShouldBeContravariant1[int]() # pyright: ignore[reportAssignmentType]
vcontra1_2: ShouldBeContravariant1[int] = ShouldBeContravariant1[object]() # OK


class ShouldBeCovariant1[**OutP]:
def f(self, fn: Callable[OutP, None]) -> None: ...


vco1_1: ShouldBeCovariant1[int] = ShouldBeCovariant1[object]() # pyright: ignore[reportAssignmentType]
vco1_2: ShouldBeCovariant1[object] = ShouldBeCovariant1[int]() # OK


class ShouldBeInvariant1[**OutP]:
def f(self, fn: Callable[OutP, None]) -> Callable[OutP, None]: ...


vinv1_1: ShouldBeInvariant1[object] = ShouldBeInvariant1[int]() # pyright: ignore[reportAssignmentType]
vinv1_2: ShouldBeInvariant1[int] = ShouldBeInvariant1[object]() # pyright: ignore[reportAssignmentType]
2 changes: 2 additions & 0 deletions packages/pyright-internal/src/tests/samples/typeVarTuple1.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ class ClassB(Generic[_Xs]): ...
Ts2 = TypeVarTuple("Ts2", int, str)

# This should generate TypeVarTuple cannot be covariant.
# based no error
Ts3 = TypeVarTuple("Ts3", covariant=True)

# This should generate TypeVarTuple cannot be contravariant.
# based no error
Ts4 = TypeVarTuple("Ts4", contravariant=True)

# This should generate TypeVarTuple does not accept other keyword arguments.
Expand Down
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.

honestly i have never encountered a single situation where a typevartuple has been useful. could you please provide a realistic example of when anyone would ever use one, and how it could benefit from supporting variance? otherwise i dont see why we should even bother to add variance support to them or touch them in any way.

it sounds like it was just scope creeped in your discourse thread about ParamSpecs for no reason

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# This sample tests variance inference for ParamSpec type parameters.

from typing import TypeVarTuple

Ts_co = TypeVarTuple("Ts_co", covariant=True)
Ts_contra = TypeVarTuple("Ts_contra", contravariant=True)
Ts_infer = TypeVarTuple("Ts_infer", infer_variance=True)
Comment on lines +5 to +7
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.

comment saying why these unused typevar variables are here



class ShouldBeContravariant1[*OutTs]:
def f(self, t: tuple[*OutTs]): ...


vcontra1_1: ShouldBeContravariant1[object] = ShouldBeContravariant1[int]() # pyright: ignore[reportAssignmentType]
vcontra1_2: ShouldBeContravariant1[int] = ShouldBeContravariant1[object]() # OK


class ShouldBeCovariant1[*OutTs]:
def f(self) -> tuple[*OutTs]: ...


vco1_1: ShouldBeCovariant1[int] = ShouldBeCovariant1[object]() # pyright: ignore[reportAssignmentType]
vco1_2: ShouldBeCovariant1[object] = ShouldBeCovariant1[int]() # OK


class ShouldBeInvariant1[*OutTs]:
def f(self, t: tuple[*OutTs]) -> tuple[*OutTs]: ...


vinv1_1: ShouldBeInvariant1[object] = ShouldBeInvariant1[int]() # pyright: ignore[reportAssignmentType]
vinv1_2: ShouldBeInvariant1[int] = ShouldBeInvariant1[object]() # pyright: ignore[reportAssignmentType]
18 changes: 18 additions & 0 deletions packages/pyright-internal/src/tests/typeEvaluator5.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,24 @@ test('AutoVariance5', () => {
TestUtils.validateResults(analysisResults, 0);
});

test('ParamSpecVariance1', () => {
const configOptions = new ConfigOptions(Uri.empty());
configOptions.diagnosticRuleSet.reportUnnecessaryTypeIgnoreComment = 'error';
configOptions.diagnosticRuleSet.reportUnusedParameter = 'none';

const analysisResults = TestUtils.typeAnalyzeSampleFiles(['paramSpecVariance1.py'], configOptions);
TestUtils.validateResultsButBased(analysisResults, {});
});

test('TypeVarTupleVariance1', () => {
const configOptions = new ConfigOptions(Uri.empty());
configOptions.diagnosticRuleSet.reportUnnecessaryTypeIgnoreComment = 'error';
configOptions.diagnosticRuleSet.reportUnusedParameter = 'none';

const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeVarTupleVariance1.py'], configOptions);
TestUtils.validateResultsButBased(analysisResults, {});
});

test('TypeAliasStatement1', () => {
const configOptions = new ConfigOptions(Uri.empty());
configOptions.defaultPythonVersion = pythonVersion3_12;
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/typeEvaluator6.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ test('TypeVarTuple1', () => {

configOptions.defaultPythonVersion = pythonVersion3_11;
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeVarTuple1.py'], configOptions);
TestUtils.validateResults(analysisResults, 18);
TestUtils.validateResults(analysisResults, 16);
});

test('TypeVarTuple2', () => {
Expand Down
Loading