From b02003c53e5fa62d9667c9916a6cff36f6f577d8 Mon Sep 17 00:00:00 2001 From: DetachHead Date: Mon, 22 Jan 2024 17:05:58 +1000 Subject: [PATCH] don't report unreachable in `not TYPE_CHECKING` blocks --- .../pyright-internal/src/analyzer/binder.ts | 30 ++++++++++++++++--- .../src/analyzer/codeFlowTypes.ts | 1 + .../src/analyzer/codeFlowUtils.ts | 1 + .../src/analyzer/typeEvaluator.ts | 14 ++++++++- .../src/tests/samples/unreachable2.py | 7 ++++- .../src/tests/typeEvaluator1.test.ts | 12 ++++---- 6 files changed, 53 insertions(+), 12 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/binder.ts b/packages/pyright-internal/src/analyzer/binder.ts index baff20d287..a84b9feea4 100644 --- a/packages/pyright-internal/src/analyzer/binder.ts +++ b/packages/pyright-internal/src/analyzer/binder.ts @@ -1325,6 +1325,14 @@ export class Binder extends ParseTreeWalker { const elseLabel = this._createBranchLabel(); const postIfLabel = this._createBranchLabel(preIfFlowNode); + const notTypeCheckingNode: FlowNode = { + flags: FlowFlags.NotTypeChecking | FlowFlags.Unreachable, + id: getUniqueFlowNodeId(), + }; + + const isTypeCheckingNode = (node: ExpressionNode): node is NameNode => + node.nodeType === ParseNodeType.Name && node.value === 'TYPE_CHECKING'; + postIfLabel.affectedExpressions = this._trackCodeFlowExpressions(() => { // Determine if the test condition is always true or always false. If so, // we can treat either the then or the else clause as unconditional. @@ -1339,16 +1347,30 @@ export class Binder extends ParseTreeWalker { this._bindConditional(node.testExpression, thenLabel, elseLabel); // Handle the if clause. - this._currentFlowNode = - constExprValue === false ? Binder._unreachableFlowNode : this._finishFlowLabel(thenLabel); + if (constExprValue === false) { + this._currentFlowNode = + node.testExpression.nodeType === ParseNodeType.UnaryOperation && + node.testExpression.operator === OperatorType.Not && + isTypeCheckingNode(node.testExpression.expression) && + constExprValue === false + ? notTypeCheckingNode + : Binder._unreachableFlowNode; + } else { + this._currentFlowNode = this._finishFlowLabel(thenLabel); + } this.walk(node.ifSuite); this._addAntecedent(postIfLabel, this._currentFlowNode); // Now handle the else clause if it's present. If there // are chained "else if" statements, they'll be handled // recursively here. - this._currentFlowNode = - constExprValue === true ? Binder._unreachableFlowNode : this._finishFlowLabel(elseLabel); + if (constExprValue === true) { + this._currentFlowNode = isTypeCheckingNode(node.testExpression) + ? notTypeCheckingNode + : Binder._unreachableFlowNode; + } else { + this._currentFlowNode = this._finishFlowLabel(elseLabel); + } if (node.elseSuite) { this.walk(node.elseSuite); } else { diff --git a/packages/pyright-internal/src/analyzer/codeFlowTypes.ts b/packages/pyright-internal/src/analyzer/codeFlowTypes.ts index 84dd234658..49a49296a6 100644 --- a/packages/pyright-internal/src/analyzer/codeFlowTypes.ts +++ b/packages/pyright-internal/src/analyzer/codeFlowTypes.ts @@ -50,6 +50,7 @@ export enum FlowFlags { FalseNeverCondition = 1 << 17, // Condition whose type evaluates to never when narrowed in negative test NarrowForPattern = 1 << 18, // Narrow the type of the subject expression within a case statement ExhaustedMatch = 1 << 19, // Control flow gate that is closed when match is provably exhaustive + NotTypeChecking = 1 << 20, // not TYPE_CHECKING block (ie. unreachable, but should not reportUnreachable) } let _nextFlowNodeId = 1; diff --git a/packages/pyright-internal/src/analyzer/codeFlowUtils.ts b/packages/pyright-internal/src/analyzer/codeFlowUtils.ts index a808c31303..e3d08b9e0d 100644 --- a/packages/pyright-internal/src/analyzer/codeFlowUtils.ts +++ b/packages/pyright-internal/src/analyzer/codeFlowUtils.ts @@ -262,6 +262,7 @@ export function formatControlFlowGraph(flowNode: FlowNode) { if (flags & FlowFlags.FalseNeverCondition) return 'FalseNever'; if (flags & FlowFlags.NarrowForPattern) return 'Pattern'; if (flags & FlowFlags.ExhaustedMatch) return 'Exhaust'; + if (flags & FlowFlags.NotTypeChecking) return 'NotTypeChecking'; throw new Error(); } diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index fda2e800b1..4fee081f1d 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -2997,6 +2997,18 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions return codeFlowEngine.isFlowNodeReachable(flowNode, sourceFlowNode); } + function isNotTypeCheckingBlock(node: ParseNode): boolean { + //TODO: abstract this logic, its used in isNodeReachable as well + const flowNode = AnalyzerNodeInfo.getFlowNode(node); + if (!flowNode) { + if (node.parent) { + return isNotTypeCheckingBlock(node.parent); + } + return false; + } + return !!(flowNode.flags & FlowFlags.NotTypeChecking); + } + function isAfterNodeReachable(node: ParseNode): boolean { const returnFlowNode = AnalyzerNodeInfo.getAfterFlowNode(node); if (!returnFlowNode) { @@ -3090,7 +3102,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions if (!isDiagnosticSuppressedForNode(node)) { const fileInfo = AnalyzerNodeInfo.getFileInfo(node); const message = LocMessage.unreachableCode(); - if (fileInfo.diagnosticRuleSet.reportUnreachable === 'none') { + if (fileInfo.diagnosticRuleSet.reportUnreachable === 'none' || isNotTypeCheckingBlock(node)) { fileInfo.diagnosticSink.addUnreachableCodeWithTextRange(message, textRange); } else { addDiagnostic(DiagnosticRule.reportUnreachable, message, node, textRange); diff --git a/packages/pyright-internal/src/tests/samples/unreachable2.py b/packages/pyright-internal/src/tests/samples/unreachable2.py index 07d2545e61..398226f946 100644 --- a/packages/pyright-internal/src/tests/samples/unreachable2.py +++ b/packages/pyright-internal/src/tests/samples/unreachable2.py @@ -1,4 +1,9 @@ -from typing import TYPE_CHECKING: +from typing import TYPE_CHECKING if not TYPE_CHECKING: + ... + +if TYPE_CHECKING: + ... +else: ... \ No newline at end of file diff --git a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts index e8751c154f..d97fbdaaee 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts @@ -31,13 +31,13 @@ test('Unreachable1 reportUnreachable', () => { TestUtils.validateResults(analysisResults, 4, 0, 2, 1, 0); }); -// test('Unreachable2 reportUnreachable (TYPE_CHECKING)', () => { -// const configOptions = new ConfigOptions(Uri.empty()); -// configOptions.diagnosticRuleSet.reportUnreachable = 'error'; -// const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable2.py'], configOptions); +test('Unreachable2 reportUnreachable TYPE_CHECKING', () => { + const configOptions = new ConfigOptions(Uri.empty()); + configOptions.diagnosticRuleSet.reportUnreachable = 'error'; + const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable2.py'], configOptions); -// TestUtils.validateResults(analysisResults, 0, 0, 0, 0, 0); -// }); + TestUtils.validateResults(analysisResults, 0, 0, 0, 0, 2); +}); test('Builtins1', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['builtins1.py']);