Fixed bug that resulted in false positive when a variable was modified in a loop that employed conditional type narrowing and was also used as a member access expression.

This commit is contained in:
Eric Traut 2021-12-08 12:12:25 -08:00
parent 9e96b5858d
commit c938d54efa
4 changed files with 58 additions and 24 deletions

View File

@ -699,10 +699,12 @@ export function getCodeFlowEngine(
// If there is an "Unknown" type within a union type, remove // If there is an "Unknown" type within a union type, remove
// it. Otherwise we might end up resolving the cycle with a type // it. Otherwise we might end up resolving the cycle with a type
// that includes an undesirable unknown. // that includes an undesirable unknown.
// Note that we return isIncomplete = false here but do not
// save the cached entry for next time. This is because the
return { return {
type: cacheEntry?.type ? removeUnknownFromUnion(cacheEntry.type) : undefined, type: cacheEntry?.type ? removeUnknownFromUnion(cacheEntry.type) : undefined,
usedOuterScopeAlias: loopUsedOuterScopeAlias, usedOuterScopeAlias: loopUsedOuterScopeAlias,
isIncomplete: true, isIncomplete: false,
}; };
} }

View File

@ -4309,6 +4309,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
{ {
type: subtype, type: subtype,
node, node,
isIncomplete: baseTypeResult.isIncomplete,
}, },
usage, usage,
EvaluatorFlags.None EvaluatorFlags.None
@ -4365,34 +4366,37 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
} }
if (!type) { if (!type) {
let diagMessage = Localizer.Diagnostic.memberAccess();
if (usage.method === 'set') {
diagMessage = Localizer.Diagnostic.memberSet();
} else if (usage.method === 'del') {
diagMessage = Localizer.Diagnostic.memberDelete();
}
// If there is an expected type diagnostic addendum (used for assignments),
// use that rather than the local diagnostic addendum because it will be
// more informative.
if (usage.setExpectedTypeDiag) {
diag = usage.setExpectedTypeDiag;
}
const isFunctionRule = const isFunctionRule =
isFunction(baseType) || isFunction(baseType) ||
isOverloadedFunction(baseType) || isOverloadedFunction(baseType) ||
(isClassInstance(baseType) && ClassType.isBuiltIn(baseType, 'function')); (isClassInstance(baseType) && ClassType.isBuiltIn(baseType, 'function'));
const [ruleSet, rule] = isFunctionRule
? [fileInfo.diagnosticRuleSet.reportFunctionMemberAccess, DiagnosticRule.reportFunctionMemberAccess]
: [fileInfo.diagnosticRuleSet.reportGeneralTypeIssues, DiagnosticRule.reportGeneralTypeIssues];
addDiagnostic( if (!baseTypeResult.isIncomplete) {
ruleSet, let diagMessage = Localizer.Diagnostic.memberAccess();
rule, if (usage.method === 'set') {
diagMessage.format({ name: memberName, type: printType(baseType) }) + diag.getString(), diagMessage = Localizer.Diagnostic.memberSet();
node.memberName } else if (usage.method === 'del') {
); diagMessage = Localizer.Diagnostic.memberDelete();
}
// If there is an expected type diagnostic addendum (used for assignments),
// use that rather than the local diagnostic addendum because it will be
// more informative.
if (usage.setExpectedTypeDiag) {
diag = usage.setExpectedTypeDiag;
}
const [ruleSet, rule] = isFunctionRule
? [fileInfo.diagnosticRuleSet.reportFunctionMemberAccess, DiagnosticRule.reportFunctionMemberAccess]
: [fileInfo.diagnosticRuleSet.reportGeneralTypeIssues, DiagnosticRule.reportGeneralTypeIssues];
addDiagnostic(
ruleSet,
rule,
diagMessage.format({ name: memberName, type: printType(baseType) }) + diag.getString(),
node.memberName
);
}
// If this is member access on a function, use "Any" so if the // If this is member access on a function, use "Any" so if the
// reportFunctionMemberAccess rule is disabled, we don't trigger // reportFunctionMemberAccess rule is disabled, we don't trigger

View File

@ -0,0 +1,22 @@
# This sample tests a loop that modifies a variable through type narrowing.
from typing import Literal, Union
class State:
def confirm_dialog(self) -> Union["State", bool]:
return False
state = State()
t0: Literal["State"] = reveal_type(state)
for _ in range(1):
result = state.confirm_dialog()
if isinstance(result, State):
t1: Literal["State"] = reveal_type(state)
t2: Literal["State"] = reveal_type(result)
state = result
else:
t3: Literal["State"] = reveal_type(state)
t4: Literal["bool"] = reveal_type(result)

View File

@ -204,6 +204,12 @@ test('Loops13', () => {
TestUtils.validateResults(analysisResults, 0); TestUtils.validateResults(analysisResults, 0);
}); });
test('Loops14', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['loops14.py']);
TestUtils.validateResults(analysisResults, 0);
});
test('ForLoop1', () => { test('ForLoop1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['forLoop1.py']); const analysisResults = TestUtils.typeAnalyzeSampleFiles(['forLoop1.py']);