Fixed bug in type evaluator relating to type var matching for function calls. The code now "locks" the type var map after the first pass. In the second pass, it verifies that the arguments are compatible with the types associated with the (now-locked) type vars.

This commit is contained in:
Eric Traut 2020-02-15 00:16:59 -07:00
parent e6396e35ef
commit 46e792c67d
5 changed files with 80 additions and 14 deletions

View File

@ -3824,7 +3824,8 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
// Run through all args and validate them against their matched parameter.
// We'll do two passes. The first one will match any type arguments. The second
// will perform the actual validation.
// will perform the actual validation. We can skip the first pass if there
// are no type vars to match.
if (validateArgTypeParams.some(arg => arg.requiresTypeVarMatching)) {
useSpeculativeMode(() => {
validateArgTypeParams.forEach(argParam => {
@ -3833,6 +3834,10 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
}
});
});
// Lock the type var map so it cannot be modified and revalidate the
// arguments in a second pass.
typeVarMap.lock();
}
validateArgTypeParams.forEach(argParam => {
@ -7473,6 +7478,7 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
}
const partialType = cachedEntry as PartialType;
if (partialType.speculativeModeId !== invalidSpeculativeModeId) {
if (!isSpeculativeMode()) {
return undefined;
@ -9241,7 +9247,9 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
// Assigns the source type to the dest type var in the type map. If an existing type is
// already associated with that type var name, it attempts to either widen or narrow
// the type (depending on the value of the canNarrowType parameter). The goal is to
// produce the narrowest type that meets all of the requirements.
// produce the narrowest type that meets all of the requirements. If the type var map
// has been "locked", it simply validates that the srcType is compatible (with no attempt
// to widen or narrow).
function assignTypeToTypeVar(destType: TypeVarType, srcType: Type, canNarrowType: boolean,
diag: DiagnosticAddendum, typeVarMap: TypeVarMap, flags = CanAssignFlags.Default,
recursionCount = 0): boolean {
@ -9277,7 +9285,7 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
// Handle the unconstrained (but possibly bound) case.
let updatedType = srcType;
const curTypeIsNarrowable = typeVarMap.isNarrowable(destType.name);
const curTypeIsNarrowable = typeVarMap.isNarrowable(destType.name) && !typeVarMap.isLocked();
const updatedTypeIsNarrowable = canNarrowType && curTypeIsNarrowable;
if (curTypeVarMapping) {
@ -9315,7 +9323,14 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
updatedType = curTypeVarMapping;
}
}
} else if (!canAssignType(srcType, curTypeVarMapping, new DiagnosticAddendum(),
} else {
if (typeVarMap.isLocked()) {
diag.addMessage(`Type '${ printType(curTypeVarMapping) }' cannot be assigned to ` +
`type '${ printType(srcType) }'`);
return false;
}
if (!canAssignType(srcType, curTypeVarMapping, new DiagnosticAddendum(),
typeVarMap, flags, recursionCount + 1)) {
// Create a union, widening the type.
@ -9323,6 +9338,7 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
}
}
}
}
// If there's a bound type, make sure the source is derived from it.
if (destType.boundType) {
@ -9335,7 +9351,9 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
}
}
if (!typeVarMap.isLocked()) {
typeVarMap.set(destType.name, updatedType, updatedTypeIsNarrowable);
}
return true;
}
@ -9601,8 +9619,10 @@ export function createTypeEvaluator(importLookup: ImportLookup): TypeEvaluator {
// of the way it's defined in enum.pyi. The type var _T must be
// manually set to the corresponding enum object type.
if (typeVarMap && ClassType.isBuiltIn(metaclass, 'EnumMeta')) {
if (!typeVarMap.isLocked()) {
typeVarMap.set('_T', ObjectType.create(srcType), false);
}
}
return canAssignClass(destClassType, metaclass, diag,
typeVarMap, flags, recursionCount + 1, false);

View File

@ -652,10 +652,15 @@ export function stripFirstParameter(type: FunctionType): FunctionType {
return type;
}
// Recursively finds all of the type arguments to the specified srcType.
// Recursively finds all of the type arguments and sets them
// to the specified srcType.
export function setTypeArgumentsRecursive(destType: Type, srcType: Type,
typeVarMap: TypeVarMap, recursionCount = 0) {
if (typeVarMap.isLocked()) {
return;
}
switch (destType.category) {
case TypeCategory.Union:
destType.subtypes.forEach(subtype => {
@ -1183,11 +1188,9 @@ export function requiresSpecialization(type: Type, recursionCount = 0): boolean
) !== undefined;
}
if (ClassType.getTypeParameters(type).length === 0) {
return false;
}
return true;
// If there are any type parameters, we need to specialize
// since there are no corresponding type arguments.
return ClassType.getTypeParameters(type).length > 0;
}
case TypeCategory.Object: {

View File

@ -10,10 +10,12 @@
*/
import { Type } from "./types";
import { assert } from "../common/debug";
export class TypeVarMap {
private _typeMap: Map<string, Type>;
private _isNarrowableMap: Map<string, boolean>;
private _isLocked = false;
constructor() {
this._typeMap = new Map<string, Type>();
@ -27,6 +29,8 @@ export class TypeVarMap {
newTypeVarMap.set(name, value, this.isNarrowable(name));
});
newTypeVarMap._isLocked = this._isLocked;
return newTypeVarMap;
}
@ -38,7 +42,12 @@ export class TypeVarMap {
return this._typeMap.get(name);
}
forEach(callback: (value: Type, key: string) => void) {
return this._typeMap.forEach(callback);
}
set(name: string, type: Type, isNarrowable: boolean) {
assert(!this._isLocked);
this._typeMap.set(name, type);
this._isNarrowableMap.set(name, isNarrowable);
}
@ -53,4 +62,14 @@ export class TypeVarMap {
// Unless told otherwise, assume type is narrowable.
return isNarrowable !== undefined ? isNarrowable : true;
}
lock() {
// Locks the type var map, preventing any further changes.
assert(!this._isLocked);
this._isLocked = true;
}
isLocked(): boolean {
return this._isLocked;
}
}

View File

@ -864,6 +864,12 @@ test('GenericTypes16', () => {
validateResults(analysisResults, 0);
});
test('GenericTypes17', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['genericTypes17.py']);
validateResults(analysisResults, 1);
});
test('Protocol1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['protocol1.py']);

View File

@ -0,0 +1,18 @@
# This sample tests the type evaluator's handling of
# type vars that span multiple parameters - especially when
# the first parameter is a callable (which has parameters
# that are contravariant).
from typing import Any, Callable, Iterable, Iterator, TypeVar
def is_one(x: int) -> bool:
return x == 1
nums = ["a", "b", "c"]
_T = TypeVar('_T')
def filterX(__function: Callable[[_T], Any], __iterable: Iterable[_T]) -> Iterator[_T]: ...
# This should be flagged as an error because nums is
# not an int array.
ones = filterX(is_one, nums)