Changed logic for function return type inference so "unbound" type is never propagated to callers. This eliminates incorrect and confusing errors.

This commit is contained in:
Eric Traut 2020-10-14 11:04:16 -07:00
parent 3c6cfc2a45
commit 97f8483392
4 changed files with 52 additions and 8 deletions

View File

@ -147,7 +147,7 @@ import {
OverloadedFunctionType,
ParamSpecEntry,
removeNoneFromUnion,
removeUnboundFromUnion,
removeUnbound,
Type,
TypeBase,
TypeCategory,
@ -2826,7 +2826,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// If the type was partially unbound, an error will have already been logged.
// Remove the unbound before assigning to the target expression so the unbound
// error doesn't propagate.
type = removeUnboundFromUnion(type);
type = removeUnbound(type);
switch (target.nodeType) {
case ParseNodeType.Name: {
@ -6383,7 +6383,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}
return false;
} else if (!skipUnknownCheck) {
const simplifiedType = removeUnboundFromUnion(argType);
const simplifiedType = removeUnbound(argType);
const fileInfo = getFileInfo(argParam.errorNode);
const diagAddendum = new DiagnosticAddendum();
@ -8425,7 +8425,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// Sometimes variables contain an "unbound" type if they're
// assigned only within conditional statements. Remove this
// to avoid confusion.
const simplifiedType = removeUnboundFromUnion(type);
const simplifiedType = removeUnbound(type);
if (isUnknown(simplifiedType)) {
addDiagnostic(diagLevel, rule, Localizer.Diagnostic.typeUnknown().format({ name: nameValue }), errorNode);
@ -9420,7 +9420,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// on platform type). We'll assume that the conditional logic is correct
// and strip off the "unbound" union.
if (argType.category === TypeCategory.Union) {
argType = removeUnboundFromUnion(argType);
argType = removeUnbound(argType);
}
if (!isAnyOrUnknown(argType) && !isUnbound(argType)) {
@ -10891,7 +10891,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// Remove any unbound values since those would generate an exception
// before being returned.
inferredReturnType = removeUnboundFromUnion(inferredReturnType);
inferredReturnType = removeUnbound(inferredReturnType);
}
// Is it a generator?
@ -14149,6 +14149,8 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
});
if (contextualReturnType) {
contextualReturnType = removeUnbound(contextualReturnType);
// Do we need to wrap this in an awaitable?
if (FunctionType.isWrapReturnTypeInAwait(type) && !isNoReturnType(contextualReturnType)) {
contextualReturnType = createAwaitableReturnType(functionNode, contextualReturnType);

View File

@ -1641,8 +1641,16 @@ export function removeUnknownFromUnion(type: Type): Type {
// If the type is a union, remove an "unbound" type from the union,
// returning only the known types.
export function removeUnboundFromUnion(type: Type): Type {
return removeFromUnion(type, (t: Type) => isUnbound(t));
export function removeUnbound(type: Type): Type {
if (type.category === TypeCategory.Union) {
return removeFromUnion(type, (t: Type) => isUnbound(t));
}
if (isUnbound(type)) {
return UnknownType.create();
}
return type;
}
// If the type is a union, remove an "None" type from the union,

View File

@ -0,0 +1,28 @@
# This sample tests that an unbound variable that is generated in
# a function does not propagate beyond that function to callers.
from typing import Literal
def func1():
# This should generate an error
return a
# This should not.
b = func1()
tb1: Literal["Unknown"] = reveal_type(b)
def func2(val: int):
if val < 3:
return val
# This should generate an error
return a
# This should not.
c = func2(36)
tc1: Literal["int | Unknown"] = reveal_type(c)

View File

@ -144,6 +144,12 @@ test('Unbound3', () => {
TestUtils.validateResults(analysisResults, 1);
});
test('Unbound4', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unbound4.py']);
TestUtils.validateResults(analysisResults, 2);
});
test('Assert1', () => {
const configOptions = new ConfigOptions('.');