Fixed inconsistency in type narrowing for isinstance and isclass calls. Previously, the narrowing logic used the target class(es) if the source expression was of type Any but did not do the same when the source expression was a union type that included Any but all other subtypes were eliminated.

This commit is contained in:
Eric Traut 2020-11-03 15:46:32 -08:00
parent 2efdd9b2df
commit 4634309d2a
3 changed files with 69 additions and 30 deletions

View File

@ -12996,42 +12996,48 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
return filteredTypes.map((t) => ObjectType.create(t));
};
if (isInstanceCheck && isObject(effectiveType)) {
const filteredType = filterType(effectiveType.classType);
return combineTypes(filteredType);
} else if (!isInstanceCheck && isClass(effectiveType)) {
const filteredType = filterType(effectiveType);
return combineTypes(filteredType);
} else if (effectiveType.category === TypeCategory.Union) {
let remainingTypes: Type[] = [];
const anyOrUnknownSubstitutions: Type[] = [];
const anyOrUnknown: Type[] = [];
effectiveType.subtypes.forEach((t) => {
if (isAnyOrUnknown(t)) {
// Any types always remain for both positive and negative
// checks because we can't say anything about them.
remainingTypes.push(t);
} else if (isInstanceCheck && isObject(t)) {
remainingTypes = remainingTypes.concat(filterType(t.classType));
} else if (!isInstanceCheck && isClass(t)) {
remainingTypes = remainingTypes.concat(filterType(t));
const filteredType = doForSubtypes(effectiveType, (subtype) => {
if (isInstanceCheck && isObject(subtype)) {
return combineTypes(filterType(subtype.classType));
} else if (!isInstanceCheck && isClass(subtype)) {
return combineTypes(filterType(subtype));
} else if (isPositiveTest && isAnyOrUnknown(subtype)) {
// If this is a positive test and the effective type is Any or
// Unknown, we can assume that the type matches one of the
// specified types.
if (isInstanceCheck) {
anyOrUnknownSubstitutions.push(
combineTypes(classTypeList.map((classType) => ObjectType.create(classType)))
);
} else {
// All other types are never instances of a class.
if (!isPositiveTest) {
remainingTypes.push(t);
}
anyOrUnknownSubstitutions.push(combineTypes(classTypeList));
}
});
return combineTypes(remainingTypes);
} else if (isInstanceCheck && isPositiveTest && isAnyOrUnknown(effectiveType)) {
// If this is a positive test for isinstance and the effective
// type is Any or Unknown, we can assume that the type matches
// one of the specified types.
type = combineTypes(classTypeList.map((classType) => ObjectType.create(classType)));
anyOrUnknown.push(subtype);
return undefined;
}
return isPositiveTest ? undefined : subtype;
});
// If the result is Any/Unknown and contains no other subtypes and
// we have substitutions for Any/Unknown, use those instead. We don't
// want to apply this if the filtering produced something other than
// Any/Unknown. For example, if the statement is "isinstance(x, list)"
// and the type of x is "List[str] | int | Any", the result should be
// "List[str]", not "List[str] | List[Unknown]".
if (isNever(filteredType) && anyOrUnknownSubstitutions.length > 0) {
return combineTypes(anyOrUnknownSubstitutions);
}
// Return the original type.
return type;
if (anyOrUnknown.length > 0) {
return combineTypes([filteredType, ...anyOrUnknown]);
}
return filteredType;
}
// Attempts to narrow a type (make it more constrained) based on an "in" or

View File

@ -0,0 +1,27 @@
# This sample tests basic type narrowing behavior for
# the isinstance call.
from typing import Any, List, Literal, Union
def func1(x: Union[List[str], int]):
if isinstance(x, list):
t1: Literal["List[str]"] = reveal_type(x)
else:
t2: Literal["int"] = reveal_type(x)
def func2(x: Any):
if isinstance(x, list):
t1: Literal["list[Unknown]"] = reveal_type(x)
else:
t2: Literal["Any"] = reveal_type(x)
def func3(x):
if isinstance(x, list):
t1: Literal["list[Unknown]"] = reveal_type(x)
else:
t2: Literal["Unknown"] = reveal_type(x)

View File

@ -102,6 +102,12 @@ test('NewType3', () => {
TestUtils.validateResults(analysisResults, 4);
});
test('isInstance1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['isinstance1.py']);
TestUtils.validateResults(analysisResults, 0);
});
test('isInstance2', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['isinstance2.py']);