Improved consistency of unreachable code. Previously, unreachable code was not supported for if or else suites when the condition type was narrowed to Never. This addresses https://github.com/microsoft/pylance-release/issues/6028. (#8190)

This commit is contained in:
Eric Traut 2024-06-20 17:00:26 +02:00 committed by GitHub
parent 81ff21e21a
commit b841e110f3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 42 additions and 30 deletions

View File

@ -95,7 +95,7 @@ export interface CodeFlowAnalyzer {
} }
export interface CodeFlowEngine { export interface CodeFlowEngine {
createCodeFlowAnalyzer: (typeAtStart: TypeResult | undefined) => CodeFlowAnalyzer; createCodeFlowAnalyzer: () => CodeFlowAnalyzer;
isFlowNodeReachable: (flowNode: FlowNode, sourceFlowNode?: FlowNode, ignoreNoReturn?: boolean) => boolean; isFlowNodeReachable: (flowNode: FlowNode, sourceFlowNode?: FlowNode, ignoreNoReturn?: boolean) => boolean;
narrowConstrainedTypeVar: (flowNode: FlowNode, typeVar: TypeVarType) => Type | undefined; narrowConstrainedTypeVar: (flowNode: FlowNode, typeVar: TypeVarType) => Type | undefined;
printControlFlowGraph: ( printControlFlowGraph: (
@ -173,11 +173,7 @@ export function getCodeFlowEngine(
// Creates a new code flow analyzer that can be used to narrow the types // Creates a new code flow analyzer that can be used to narrow the types
// of the expressions within an execution context. Each code flow analyzer // of the expressions within an execution context. Each code flow analyzer
// instance maintains a cache of types it has already determined. // instance maintains a cache of types it has already determined.
// The caller should pass a typeAtStart value because the code flow function createCodeFlowAnalyzer(): CodeFlowAnalyzer {
// analyzer may cache types based on this value, but the typeAtStart
// may vary depending on the context in which the code flow analysis
// is performed.
function createCodeFlowAnalyzer(typeAtStart: TypeResult | undefined): CodeFlowAnalyzer {
const flowNodeTypeCacheSet = new Map<string, CodeFlowTypeCache>(); const flowNodeTypeCacheSet = new Map<string, CodeFlowTypeCache>();
function getFlowNodeTypeCacheForReference(referenceKey: string) { function getFlowNodeTypeCacheForReference(referenceKey: string) {
@ -513,10 +509,6 @@ export function getCodeFlowEngine(
} }
} }
if (flowTypeResult && !isFlowNodeReachable(flowNode)) {
flowTypeResult = undefined;
}
return setCacheEntry(curFlowNode, flowTypeResult?.type, !!flowTypeResult?.isIncomplete); return setCacheEntry(curFlowNode, flowTypeResult?.type, !!flowTypeResult?.isIncomplete);
} }
@ -1226,8 +1218,6 @@ export function getCodeFlowEngine(
curFlowNode.flags & curFlowNode.flags &
(FlowFlags.VariableAnnotation | (FlowFlags.VariableAnnotation |
FlowFlags.Assignment | FlowFlags.Assignment |
FlowFlags.TrueCondition |
FlowFlags.FalseCondition |
FlowFlags.WildcardImport | FlowFlags.WildcardImport |
FlowFlags.NarrowForPattern | FlowFlags.NarrowForPattern |
FlowFlags.ExhaustedMatch) FlowFlags.ExhaustedMatch)
@ -1235,15 +1225,19 @@ export function getCodeFlowEngine(
const typedFlowNode = curFlowNode as const typedFlowNode = curFlowNode as
| FlowVariableAnnotation | FlowVariableAnnotation
| FlowAssignment | FlowAssignment
| FlowCondition
| FlowWildcardImport | FlowWildcardImport
| FlowCondition
| FlowExhaustedMatch; | FlowExhaustedMatch;
curFlowNode = typedFlowNode.antecedent; curFlowNode = typedFlowNode.antecedent;
continue; continue;
} }
if (curFlowNode.flags & (FlowFlags.TrueNeverCondition | FlowFlags.FalseNeverCondition)) { if (
curFlowNode.flags &
(FlowFlags.TrueCondition |
FlowFlags.FalseCondition |
FlowFlags.TrueNeverCondition |
FlowFlags.FalseNeverCondition)
) {
const conditionalFlowNode = curFlowNode as FlowCondition; const conditionalFlowNode = curFlowNode as FlowCondition;
if (conditionalFlowNode.reference) { if (conditionalFlowNode.reference) {
// Make sure the reference type has a declared type. If not, // Make sure the reference type has a declared type. If not,
@ -1365,7 +1359,7 @@ export function getCodeFlowEngine(
// Protect against infinite recursion. // Protect against infinite recursion.
if (isReachableRecursionSet.has(flowNode.id)) { if (isReachableRecursionSet.has(flowNode.id)) {
return true; return false;
} }
isReachableRecursionSet.add(flowNode.id); isReachableRecursionSet.add(flowNode.id);

View File

@ -19876,7 +19876,7 @@ export function createTypeEvaluator(
} }
// Allocate a new code flow analyzer. // Allocate a new code flow analyzer.
const analyzer = codeFlowEngine.createCodeFlowAnalyzer(typeAtStart); const analyzer = codeFlowEngine.createCodeFlowAnalyzer();
if (entries) { if (entries) {
entries.push({ typeAtStart, codeFlowAnalyzer: analyzer }); entries.push({ typeAtStart, codeFlowAnalyzer: analyzer });
} else { } else {
@ -22181,7 +22181,7 @@ export function createTypeEvaluator(
const prevTypeCache = returnTypeInferenceTypeCache; const prevTypeCache = returnTypeInferenceTypeCache;
returnTypeInferenceContextStack.push({ returnTypeInferenceContextStack.push({
functionNode, functionNode,
codeFlowAnalyzer: codeFlowEngine.createCodeFlowAnalyzer(/* typeAtStart */ undefined), codeFlowAnalyzer: codeFlowEngine.createCodeFlowAnalyzer(),
}); });
try { try {

View File

@ -165,7 +165,7 @@ test('With2', () => {
test('With3', () => { test('With3', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['with3.py']); const analysisResults = TestUtils.typeAnalyzeSampleFiles(['with3.py']);
TestUtils.validateResults(analysisResults, 5); TestUtils.validateResults(analysisResults, 6);
}); });
test('With4', () => { test('With4', () => {

View File

@ -49,21 +49,21 @@ class Class1(Generic[_T1, _T2, _T3, _P, Unpack[_Ts]]):
# This should generate an error. # This should generate an error.
return [0] return [0]
if cond: if cond or 3 > 2:
if isinstance(val1, str): if isinstance(val1, str):
# This should generate an error. # This should generate an error.
return [0] return [0]
else: else:
return [0] return [0]
if cond: if cond or 3 > 2:
if isinstance(val3, B): if isinstance(val3, B):
return [B()] return [B()]
else: else:
# This should generate an error. # This should generate an error.
return [C()] return [C()]
if cond: if cond or 3 > 2:
if not isinstance(val3, B) and not isinstance(val3, C): if not isinstance(val3, B) and not isinstance(val3, C):
return [A()] return [A()]

View File

@ -1,7 +1,10 @@
# This sample validates that the exception type provided # This sample validates that the exception type provided
# within a raise statement is valid. # within a raise statement is valid.
a: bool = True from random import random
a: bool = True if random() > 0.5 else False
class CustomException1(BaseException): class CustomException1(BaseException):
@ -11,10 +14,10 @@ class CustomException1(BaseException):
# This should generate an error because CustomException1 # This should generate an error because CustomException1
# requires an argument to instantiate. # requires an argument to instantiate.
if a: if a or 2 > 1:
raise CustomException1 raise CustomException1
if a: if a or 2 > 1:
raise CustomException1(3) raise CustomException1(3)
@ -24,5 +27,5 @@ class CustomException2:
# This should generate an error because # This should generate an error because
# the exception doesn't derive from BaseException. # the exception doesn't derive from BaseException.
if a: if a or 2 > 1:
raise CustomException2 raise CustomException2

View File

@ -1,8 +1,8 @@
# This sample tests the detection and reporting of unreachable code. # This sample tests the detection and reporting of unreachable code.
from abc import abstractmethod
import os import os
import sys import sys
from abc import abstractmethod
from typing import NoReturn from typing import NoReturn
@ -109,3 +109,19 @@ def func10():
return return
# This should be marked unreachable. # This should be marked unreachable.
b = e.errno b = e.errno
def func11(obj: str) -> list:
if isinstance(obj, str):
return []
else:
# This should be marked as unreachable.
return obj
def func12(obj: str) -> list:
if isinstance(obj, str):
return []
# This should be marked as unreachable.
return obj

View File

@ -17,8 +17,7 @@ def test1() -> None:
raise RuntimeError() raise RuntimeError()
return return
# This should generate an error because # This should generate an error.
# the code is not unreachable.
c = "hi" + 3 c = "hi" + 3
with memoryview(x): with memoryview(x):

View File

@ -26,7 +26,7 @@ import * as TestUtils from './testUtils';
test('Unreachable1', () => { test('Unreachable1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable1.py']); const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable1.py']);
TestUtils.validateResults(analysisResults, 0, 0, 2, 1, 4); TestUtils.validateResults(analysisResults, 0, 0, 2, 1, 6);
}); });
test('Builtins1', () => { test('Builtins1', () => {