Skip to content

Commit

Permalink
Revert "Improved consistency of unreachable code. Previously, unreach…
Browse files Browse the repository at this point in the history
…able code was not supported for `if` or `else` suites when the condition type was narrowed to `Never`. This addresses microsoft/pylance-release#6028. (#8190)"

This reverts commit b841e11.
  • Loading branch information
DetachHead committed Jul 6, 2024
1 parent 0e19ee3 commit eadeb56
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 42 deletions.
26 changes: 16 additions & 10 deletions packages/pyright-internal/src/analyzer/codeFlowEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: (
Expand Down Expand Up @@ -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<string, CodeFlowTypeCache>();

function getFlowNodeTypeCacheForReference(referenceKey: string) {
Expand Down Expand Up @@ -509,6 +513,10 @@ export function getCodeFlowEngine(
}
}

if (flowTypeResult && !isFlowNodeReachable(flowNode)) {
flowTypeResult = undefined;
}

return setCacheEntry(curFlowNode, flowTypeResult?.type, !!flowTypeResult?.isIncomplete);
}

Expand Down Expand Up @@ -1218,26 +1226,24 @@ export function getCodeFlowEngine(
curFlowNode.flags &
(FlowFlags.VariableAnnotation |
FlowFlags.Assignment |
FlowFlags.TrueCondition |
FlowFlags.FalseCondition |
FlowFlags.WildcardImport |
FlowFlags.NarrowForPattern |
FlowFlags.ExhaustedMatch)
) {
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,
Expand Down Expand Up @@ -1359,7 +1365,7 @@ export function getCodeFlowEngine(

// Protect against infinite recursion.
if (isReachableRecursionSet.has(flowNode.id)) {
return false;
return true;
}
isReachableRecursionSet.add(flowNode.id);

Expand Down
4 changes: 2 additions & 2 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -22507,7 +22507,7 @@ export function createTypeEvaluator(
const prevTypeCache = returnTypeInferenceTypeCache;
returnTypeInferenceContextStack.push({
functionNode,
codeFlowAnalyzer: codeFlowEngine.createCodeFlowAnalyzer(),
codeFlowAnalyzer: codeFlowEngine.createCodeFlowAnalyzer(/* typeAtStart */ undefined),
});

try {
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/checker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ test('With2', () => {
test('With3', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['with3.py']);

TestUtils.validateResults(analysisResults, 6);
TestUtils.validateResults(analysisResults, 5);
});

test('With4', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()]

Expand Down
11 changes: 4 additions & 7 deletions packages/pyright-internal/src/tests/samples/tryExcept4.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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)


Expand All @@ -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
18 changes: 1 addition & 17 deletions packages/pyright-internal/src/tests/samples/unreachable1.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion packages/pyright-internal/src/tests/samples/with3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/tests/typeEvaluator1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit eadeb56

Please sign in to comment.