Fixed bug in ParamSpec type evaluation that caused a false positive error when assigning a callable with a Concatenate to another ParamSpec.

This commit is contained in:
Eric Traut 2021-11-15 22:56:15 -08:00
parent fec0ac356c
commit 33dbcabe0b
5 changed files with 123 additions and 99 deletions

View File

@ -8491,16 +8491,11 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
return false;
}
if (!paramSpecValue.concrete) {
// If the paramSpec is bound to another paramSpec, it's valid.
return true;
}
let reportedArgError = false;
// Build a map of all named parameters.
const paramMap = new Map<string, ParamSpecEntry>();
const paramSpecParams = paramSpecValue.concrete.parameters;
const paramSpecParams = paramSpecValue.parameters;
paramSpecParams.forEach((param) => {
if (param.name) {
paramMap.set(param.name, param);
@ -8586,7 +8581,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
return paramInfo.category === ParameterCategory.Simple && !paramInfo.hasDefault;
});
if (unassignedParams.length > 0) {
if (unassignedParams.length > 0 && !paramSpecValue.paramSpec) {
const missingParamNames = unassignedParams.map((p) => `"${p}"`).join(', ');
addDiagnostic(
AnalyzerNodeInfo.getFileInfo(errorNode).diagnosticRuleSet.reportGeneralTypeIssues,
@ -17815,7 +17810,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
if (isTypeVar(srcType) && srcType.details.isParamSpec) {
const existingEntry = typeVarMap.getParamSpec(destType);
if (existingEntry) {
if (!existingEntry.concrete && existingEntry.paramSpec) {
if (existingEntry.parameters.length === 0 && existingEntry.paramSpec) {
// If there's an existing entry that matches, that's fine.
if (
isTypeSame(
@ -17831,7 +17826,11 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}
} else {
if (!typeVarMap.isLocked() && isTypeVarInScope) {
typeVarMap.setParamSpec(destType, { paramSpec: srcType });
typeVarMap.setParamSpec(destType, {
flags: FunctionTypeFlags.None,
parameters: [],
paramSpec: srcType,
});
}
return true;
}
@ -17852,9 +17851,9 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
if (existingEntry) {
// Verify that the existing entry matches the new entry.
if (
existingEntry.concrete &&
existingEntry.concrete.parameters.length === parameters.length &&
!existingEntry.concrete.parameters.some((existingParam, index) => {
!existingEntry.paramSpec &&
existingEntry.parameters.length === parameters.length &&
!existingEntry.parameters.some((existingParam, index) => {
const newParam = parameters[index];
return (
existingParam.category !== newParam.category ||
@ -17874,7 +17873,11 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}
} else {
if (!typeVarMap.isLocked() && isTypeVarInScope) {
typeVarMap.setParamSpec(destType, { concrete: { parameters, flags: srcType.details.flags } });
typeVarMap.setParamSpec(destType, {
parameters,
flags: srcType.details.flags,
paramSpec: undefined,
});
}
return true;
}
@ -20277,33 +20280,26 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
const effectiveSrcType = (flags & CanAssignFlags.ReverseTypeVarMatching) === 0 ? srcType : destType;
if (effectiveDestType.details.paramSpec) {
if (effectiveSrcType.details.paramSpec) {
typeVarMap.setParamSpec(effectiveDestType.details.paramSpec, {
paramSpec: convertToInstance(effectiveSrcType.details.paramSpec) as TypeVarType,
});
} else {
typeVarMap.setParamSpec(effectiveDestType.details.paramSpec, {
concrete: {
parameters: effectiveSrcType.details.parameters
.map((p, index) => {
const paramSpecEntry: ParamSpecEntry = {
category: p.category,
name: p.name,
isNameSynthesized: p.isNameSynthesized,
hasDefault: !!p.hasDefault,
type: FunctionType.getEffectiveParameterType(effectiveSrcType, index),
};
return paramSpecEntry;
})
.slice(
// Skip position-only and keyword-only separators.
effectiveDestType.details.parameters.filter((p) => p.name).length,
effectiveSrcType.details.parameters.length
),
flags: effectiveSrcType.details.flags,
},
});
}
typeVarMap.setParamSpec(effectiveDestType.details.paramSpec, {
parameters: effectiveSrcType.details.parameters
.map((p, index) => {
const paramSpecEntry: ParamSpecEntry = {
category: p.category,
name: p.name,
isNameSynthesized: p.isNameSynthesized,
hasDefault: !!p.hasDefault,
type: FunctionType.getEffectiveParameterType(effectiveSrcType, index),
};
return paramSpecEntry;
})
.slice(
// Skip position-only and keyword-only separators.
effectiveDestType.details.parameters.filter((p) => p.name).length,
effectiveSrcType.details.parameters.length
),
flags: effectiveSrcType.details.flags,
paramSpec: effectiveSrcType.details.paramSpec ? convertToInstance(effectiveSrcType.details.paramSpec) as TypeVarType : undefined
});
}
}
}

View File

@ -1353,10 +1353,16 @@ export function buildTypeVarMap(
});
});
typeVarMap.setParamSpec(typeParam, {
concrete: { parameters: paramSpecEntries, flags: typeArgType.details.flags },
parameters: paramSpecEntries,
flags: typeArgType.details.flags,
paramSpec: typeArgType.details.paramSpec,
});
} else if (isParamSpec(typeArgType)) {
typeVarMap.setParamSpec(typeParam, { paramSpec: typeArgType });
typeVarMap.setParamSpec(typeParam, {
flags: FunctionTypeFlags.None,
parameters: [],
paramSpec: typeArgType,
});
}
}
} else {
@ -2100,11 +2106,11 @@ function _transformTypeVarsInClassType(
if (paramSpecEntries) {
specializationNeeded = true;
if (paramSpecEntries.concrete) {
if (paramSpecEntries.parameters.length > 0) {
// Create a function type from the param spec entries.
const functionType = FunctionType.createInstance('', '', '', FunctionTypeFlags.ParamSpecValue);
paramSpecEntries.concrete.parameters.forEach((entry) => {
paramSpecEntries.parameters.forEach((entry) => {
FunctionType.addParameter(functionType, {
category: entry.category,
name: entry.name,
@ -2115,6 +2121,8 @@ function _transformTypeVarsInClassType(
});
});
functionType.details.paramSpec = paramSpecEntries.paramSpec;
return functionType;
}

View File

@ -1015,14 +1015,9 @@ export interface ParamSpecEntry {
}
export interface ParamSpecValue {
concrete?: {
flags: FunctionTypeFlags;
parameters: ParamSpecEntry[];
};
// If the param spec is assigned to another param spec,
// this will contain that type, and concrete will be undefined.
paramSpec?: TypeVarType | undefined;
flags: FunctionTypeFlags;
parameters: ParamSpecEntry[];
paramSpec: TypeVarType | undefined;
}
export namespace FunctionType {
@ -1195,38 +1190,36 @@ export namespace FunctionType {
delete newFunction.details.paramSpec;
if (paramTypes) {
if (paramTypes.concrete) {
newFunction.details.parameters = [
...type.details.parameters,
...paramTypes.concrete.parameters.map((specEntry) => {
return {
category: specEntry.category,
name: specEntry.name,
hasDefault: specEntry.hasDefault,
isNameSynthesized: specEntry.isNameSynthesized,
hasDeclaredType: true,
type: specEntry.type,
};
}),
];
newFunction.details.parameters = [
...type.details.parameters,
...paramTypes.parameters.map((specEntry) => {
return {
category: specEntry.category,
name: specEntry.name,
hasDefault: specEntry.hasDefault,
isNameSynthesized: specEntry.isNameSynthesized,
hasDeclaredType: true,
type: specEntry.type,
};
}),
];
newFunction.details.flags =
(paramTypes.concrete.flags &
(FunctionTypeFlags.ClassMethod |
FunctionTypeFlags.StaticMethod |
FunctionTypeFlags.ConstructorMethod |
FunctionTypeFlags.ParamSpecValue)) |
FunctionTypeFlags.SynthesizedMethod;
newFunction.details.flags =
(paramTypes.flags &
(FunctionTypeFlags.ClassMethod |
FunctionTypeFlags.StaticMethod |
FunctionTypeFlags.ConstructorMethod |
FunctionTypeFlags.ParamSpecValue)) |
FunctionTypeFlags.SynthesizedMethod;
// Update the specialized parameter types as well.
if (newFunction.specializedTypes) {
paramTypes.concrete.parameters.forEach((paramInfo) => {
newFunction.specializedTypes!.parameterTypes.push(paramInfo.type);
});
}
} else if (paramTypes.paramSpec) {
newFunction.details.paramSpec = paramTypes.paramSpec;
// Update the specialized parameter types as well.
if (newFunction.specializedTypes) {
paramTypes.parameters.forEach((paramInfo) => {
newFunction.specializedTypes!.parameterTypes.push(paramInfo.type);
});
}
newFunction.details.paramSpec = paramTypes.paramSpec;
}
return newFunction;
@ -1245,26 +1238,24 @@ export namespace FunctionType {
// Make a shallow clone of the details.
newFunction.details = { ...type.details };
if (paramTypes.concrete) {
// Remove the last two parameters, which are the *args and **kwargs.
newFunction.details.parameters = newFunction.details.parameters.slice(
0,
newFunction.details.parameters.length - 2
);
// Remove the last two parameters, which are the *args and **kwargs.
newFunction.details.parameters = newFunction.details.parameters.slice(
0,
newFunction.details.parameters.length - 2
);
paramTypes.concrete.parameters.forEach((specEntry) => {
newFunction.details.parameters.push({
category: specEntry.category,
name: specEntry.name,
hasDefault: specEntry.hasDefault,
isNameSynthesized: specEntry.isNameSynthesized,
hasDeclaredType: true,
type: specEntry.type,
});
paramTypes.parameters.forEach((specEntry) => {
newFunction.details.parameters.push({
category: specEntry.category,
name: specEntry.name,
hasDefault: specEntry.hasDefault,
isNameSynthesized: specEntry.isNameSynthesized,
hasDeclaredType: true,
type: specEntry.type,
});
} else if (paramTypes.paramSpec) {
newFunction.details.paramSpec = paramTypes.paramSpec;
}
});
newFunction.details.paramSpec = paramTypes.paramSpec;
return newFunction;
}

View File

@ -0,0 +1,21 @@
# This sample tests the case where a Callable that includes a Concatenate
# is assigned to a ParamSpec that doesn't include a Concatenate.
from typing import Callable, Literal, TypeVar
from typing_extensions import Concatenate, ParamSpec
Pi = ParamSpec("Pi")
def is_inty(f: Callable[Pi, object]) -> Callable[Pi, int]:
...
Po = ParamSpec("Po")
T = TypeVar("T")
def outer(f: Callable[Concatenate[str, Po], object]):
x = is_inty(f)
t_x: Literal["Callable[Concatenate[str, Po@outer], int]"] = reveal_type(x)

View File

@ -787,6 +787,14 @@ test('ParamSpec22', () => {
TestUtils.validateResults(results, 0);
});
test('ParamSpec23', () => {
const configOptions = new ConfigOptions('.');
configOptions.defaultPythonVersion = PythonVersion.V3_10;
const results = TestUtils.typeAnalyzeSampleFiles(['paramSpec23.py'], configOptions);
TestUtils.validateResults(results, 0);
});
test('ClassVar1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['classVar1.py']);