From eadeb56cc9e4be6b0d817dc6d7d8ea2e62b943b0 Mon Sep 17 00:00:00 2001 From: detachhead Date: Fri, 5 Jul 2024 21:40:48 +1000 Subject: [PATCH] Revert "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 reverts commit b841e110f31dd552d1da9eaabb58c100881f0bd5. --- .../src/analyzer/codeFlowEngine.ts | 26 ++++++++++++------- .../src/analyzer/typeEvaluator.ts | 4 +-- .../src/tests/checker.test.ts | 2 +- .../src/tests/samples/constrainedTypeVar13.py | 6 ++--- .../src/tests/samples/tryExcept4.py | 11 +++----- .../src/tests/samples/unreachable1.py | 18 +------------ .../src/tests/samples/with3.py | 3 ++- .../src/tests/typeEvaluator1.test.ts | 2 +- 8 files changed, 30 insertions(+), 42 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/codeFlowEngine.ts b/packages/pyright-internal/src/analyzer/codeFlowEngine.ts index 05456ceb25..11a9cd9f03 100644 --- a/packages/pyright-internal/src/analyzer/codeFlowEngine.ts +++ b/packages/pyright-internal/src/analyzer/codeFlowEngine.ts @@ -95,7 +95,7 @@ export interface CodeFlowAnalyzer { } export interface CodeFlowEngine { - createCodeFlowAnalyzer: () => CodeFlowAnalyzer; + createCodeFlowAnalyzer: (typeAtStart: TypeResult | undefined) => CodeFlowAnalyzer; isFlowNodeReachable: (flowNode: FlowNode, sourceFlowNode?: FlowNode, ignoreNoReturn?: boolean) => boolean; narrowConstrainedTypeVar: (flowNode: FlowNode, typeVar: TypeVarType) => Type | undefined; printControlFlowGraph: ( @@ -173,7 +173,11 @@ export function getCodeFlowEngine( // 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 // instance maintains a cache of types it has already determined. - function createCodeFlowAnalyzer(): CodeFlowAnalyzer { + // The caller should pass a typeAtStart value because the code flow + // 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(); function getFlowNodeTypeCacheForReference(referenceKey: string) { @@ -509,6 +513,10 @@ export function getCodeFlowEngine( } } + if (flowTypeResult && !isFlowNodeReachable(flowNode)) { + flowTypeResult = undefined; + } + return setCacheEntry(curFlowNode, flowTypeResult?.type, !!flowTypeResult?.isIncomplete); } @@ -1218,6 +1226,8 @@ export function getCodeFlowEngine( curFlowNode.flags & (FlowFlags.VariableAnnotation | FlowFlags.Assignment | + FlowFlags.TrueCondition | + FlowFlags.FalseCondition | FlowFlags.WildcardImport | FlowFlags.NarrowForPattern | FlowFlags.ExhaustedMatch) @@ -1225,19 +1235,15 @@ export function getCodeFlowEngine( const typedFlowNode = curFlowNode as | FlowVariableAnnotation | FlowAssignment + | FlowCondition | FlowWildcardImport + | FlowCondition | FlowExhaustedMatch; curFlowNode = typedFlowNode.antecedent; continue; } - if ( - curFlowNode.flags & - (FlowFlags.TrueCondition | - FlowFlags.FalseCondition | - FlowFlags.TrueNeverCondition | - FlowFlags.FalseNeverCondition) - ) { + if (curFlowNode.flags & (FlowFlags.TrueNeverCondition | FlowFlags.FalseNeverCondition)) { const conditionalFlowNode = curFlowNode as FlowCondition; if (conditionalFlowNode.reference) { // Make sure the reference type has a declared type. If not, @@ -1359,7 +1365,7 @@ export function getCodeFlowEngine( // Protect against infinite recursion. if (isReachableRecursionSet.has(flowNode.id)) { - return false; + return true; } isReachableRecursionSet.add(flowNode.id); diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index 8f70a53dd5..263bc7dedc 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -20187,7 +20187,7 @@ export function createTypeEvaluator( } // Allocate a new code flow analyzer. - const analyzer = codeFlowEngine.createCodeFlowAnalyzer(); + const analyzer = codeFlowEngine.createCodeFlowAnalyzer(typeAtStart); if (entries) { entries.push({ typeAtStart, codeFlowAnalyzer: analyzer }); } else { @@ -22507,7 +22507,7 @@ export function createTypeEvaluator( const prevTypeCache = returnTypeInferenceTypeCache; returnTypeInferenceContextStack.push({ functionNode, - codeFlowAnalyzer: codeFlowEngine.createCodeFlowAnalyzer(), + codeFlowAnalyzer: codeFlowEngine.createCodeFlowAnalyzer(/* typeAtStart */ undefined), }); try { diff --git a/packages/pyright-internal/src/tests/checker.test.ts b/packages/pyright-internal/src/tests/checker.test.ts index cd37f0d3b2..a5d3b7dbf0 100644 --- a/packages/pyright-internal/src/tests/checker.test.ts +++ b/packages/pyright-internal/src/tests/checker.test.ts @@ -167,7 +167,7 @@ test('With2', () => { test('With3', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['with3.py']); - TestUtils.validateResults(analysisResults, 6); + TestUtils.validateResults(analysisResults, 5); }); test('With4', () => { diff --git a/packages/pyright-internal/src/tests/samples/constrainedTypeVar13.py b/packages/pyright-internal/src/tests/samples/constrainedTypeVar13.py index e7362b9e33..18f64591f7 100644 --- a/packages/pyright-internal/src/tests/samples/constrainedTypeVar13.py +++ b/packages/pyright-internal/src/tests/samples/constrainedTypeVar13.py @@ -49,21 +49,21 @@ def meth1( # This should generate an error. return [0] - if cond or 3 > 2: + if cond: if isinstance(val1, str): # This should generate an error. return [0] else: return [0] - if cond or 3 > 2: + if cond: if isinstance(val3, B): return [B()] else: # This should generate an error. return [C()] - if cond or 3 > 2: + if cond: if not isinstance(val3, B) and not isinstance(val3, C): return [A()] diff --git a/packages/pyright-internal/src/tests/samples/tryExcept4.py b/packages/pyright-internal/src/tests/samples/tryExcept4.py index 6a0cbdab32..74cba7b332 100644 --- a/packages/pyright-internal/src/tests/samples/tryExcept4.py +++ b/packages/pyright-internal/src/tests/samples/tryExcept4.py @@ -1,10 +1,7 @@ # This sample validates that the exception type provided # within a raise statement is valid. -from random import random - - -a: bool = True if random() > 0.5 else False +a: bool = True class CustomException1(BaseException): @@ -14,10 +11,10 @@ def __init__(self, code: int): # This should generate an error because CustomException1 # requires an argument to instantiate. -if a or 2 > 1: +if a: raise CustomException1 -if a or 2 > 1: +if a: raise CustomException1(3) @@ -27,5 +24,5 @@ class CustomException2: # This should generate an error because # the exception doesn't derive from BaseException. -if a or 2 > 1: +if a: raise CustomException2 diff --git a/packages/pyright-internal/src/tests/samples/unreachable1.py b/packages/pyright-internal/src/tests/samples/unreachable1.py index d67513bec0..d3a38067a0 100644 --- a/packages/pyright-internal/src/tests/samples/unreachable1.py +++ b/packages/pyright-internal/src/tests/samples/unreachable1.py @@ -1,8 +1,8 @@ # This sample tests the detection and reporting of unreachable code. +from abc import abstractmethod import os import sys -from abc import abstractmethod from typing import NoReturn @@ -109,19 +109,3 @@ def func10(): return # This should be marked unreachable. 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 diff --git a/packages/pyright-internal/src/tests/samples/with3.py b/packages/pyright-internal/src/tests/samples/with3.py index ff466089a1..60a6fa535c 100644 --- a/packages/pyright-internal/src/tests/samples/with3.py +++ b/packages/pyright-internal/src/tests/samples/with3.py @@ -17,7 +17,8 @@ class A: raise RuntimeError() return - # This should generate an error. + # This should generate an error because + # the code is not unreachable. c = "hi" + 3 with memoryview(x): diff --git a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts index 38b3bee7ff..9f6332bc5a 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts @@ -26,7 +26,7 @@ import * as TestUtils from './testUtils'; test('Unreachable1', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable1.py']); - TestUtils.validateResults(analysisResults, 0, 0, 2, 1, 6); + TestUtils.validateResults(analysisResults, 0, 0, 2, 1, 4); }); test('Builtins1', () => {