From 91de1fef2ab1f85128683400e16edc6543514c35 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 26 Apr 2022 20:02:28 -0700 Subject: [PATCH] Improved type evaluation for expressions that are determined to be unreachable. Such code should evaluate to the type `Never` and never generate errors or warnings. --- .../pyright-internal/src/analyzer/binder.ts | 7 +++++- .../src/analyzer/codeFlowEngine.ts | 4 +-- .../src/analyzer/patternMatching.ts | 16 ++++++------ .../src/analyzer/typeEvaluator.ts | 25 ++++++++++++------- .../src/tests/samples/unreachable1.py | 25 +++++++++++++++++++ .../src/tests/typeEvaluator1.test.ts | 12 ++++----- 6 files changed, 64 insertions(+), 25 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/binder.ts b/packages/pyright-internal/src/analyzer/binder.ts index c7eb2a80517a..aede5c7375d6 100644 --- a/packages/pyright-internal/src/analyzer/binder.ts +++ b/packages/pyright-internal/src/analyzer/binder.ts @@ -591,7 +591,12 @@ export class Binder extends ParseTreeWalker { override visitCall(node: CallNode): boolean { this._disableTrueFalseTargets(() => { this.walk(node.leftExpression); - this.walkMultiple(node.arguments); + node.arguments.forEach((argNode) => { + if (this._currentFlowNode) { + AnalyzerNodeInfo.setFlowNode(argNode, this._currentFlowNode); + } + this.walk(argNode); + }); }); this._createCallFlowNode(node); diff --git a/packages/pyright-internal/src/analyzer/codeFlowEngine.ts b/packages/pyright-internal/src/analyzer/codeFlowEngine.ts index 8e2f40672ed5..0c7320dbc684 100644 --- a/packages/pyright-internal/src/analyzer/codeFlowEngine.ts +++ b/packages/pyright-internal/src/analyzer/codeFlowEngine.ts @@ -62,6 +62,7 @@ import { isTypeSame, isTypeVar, ModuleType, + NeverType, removeIncompleteUnknownFromUnion, Type, TypeVarType, @@ -316,8 +317,7 @@ export function getCodeFlowEngine( if (curFlowNode.flags & FlowFlags.Unreachable) { // We can get here if there are nodes in a compound logical expression // (e.g. "False and x") that are never executed but are evaluated. - // The type doesn't matter in this case. - return setCacheEntry(curFlowNode, /* type */ undefined, /* isIncomplete */ false); + return setCacheEntry(curFlowNode, NeverType.createNever(), /* isIncomplete */ false); } if (curFlowNode.flags & FlowFlags.VariableAnnotation) { diff --git a/packages/pyright-internal/src/analyzer/patternMatching.ts b/packages/pyright-internal/src/analyzer/patternMatching.ts index cb8232f891da..5c8551ff0df3 100644 --- a/packages/pyright-internal/src/analyzer/patternMatching.ts +++ b/packages/pyright-internal/src/analyzer/patternMatching.ts @@ -543,7 +543,7 @@ function narrowTypeBasedOnClassPattern( ); } - if (!TypeBase.isInstantiable(exprType)) { + if (!TypeBase.isInstantiable(exprType) && !isNever(exprType)) { evaluator.addDiagnostic( getFileInfo(pattern).diagnosticRuleSet.reportGeneralTypeIssues, DiagnosticRule.reportGeneralTypeIssues, @@ -1321,12 +1321,14 @@ export function validateClassPattern(evaluator: TypeEvaluator, pattern: PatternC pattern.className ); } else if (!isInstantiableClass(exprType) || exprType.includeSubclasses) { - evaluator.addDiagnostic( - getFileInfo(pattern).diagnosticRuleSet.reportGeneralTypeIssues, - DiagnosticRule.reportGeneralTypeIssues, - Localizer.DiagnosticAddendum.typeNotClass().format({ type: evaluator.printType(exprType) }), - pattern.className - ); + if (!isNever(exprType)) { + evaluator.addDiagnostic( + getFileInfo(pattern).diagnosticRuleSet.reportGeneralTypeIssues, + DiagnosticRule.reportGeneralTypeIssues, + Localizer.DiagnosticAddendum.typeNotClass().format({ type: evaluator.printType(exprType) }), + pattern.className + ); + } } else { const isBuiltIn = classPatternSpecialCases.some((className) => exprType.details.fullName === className); diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index f1de0a8f5ab1..c72495de0e02 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -2630,7 +2630,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions node: ParseNode, range?: TextRange ) { - if (!isDiagnosticSuppressedForNode(node)) { + if (!isDiagnosticSuppressedForNode(node) && isNodeReachable(node)) { const fileInfo = AnalyzerNodeInfo.getFileInfo(node); return fileInfo.diagnosticSink.addDiagnosticWithTextRange(diagLevel, message, range || node); } @@ -3906,15 +3906,18 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } } else { // Handle the special case of "reveal_type" and "reveal_locals". - if (name !== 'reveal_type' && name !== 'reveal_locals') { + if (name === 'reveal_type' || name === 'reveal_locals') { + type = AnyType.create(); + } else { addDiagnostic( fileInfo.diagnosticRuleSet.reportUndefinedVariable, DiagnosticRule.reportUndefinedVariable, Localizer.Diagnostic.symbolIsUndefined().format({ name }), node ); + + type = UnknownType.create(); } - type = UnknownType.create(); } if (isParamSpec(type)) { @@ -4472,13 +4475,9 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions switch (baseType.category) { case TypeCategory.Any: - case TypeCategory.Unknown: { - type = baseType; - break; - } - + case TypeCategory.Unknown: case TypeCategory.Never: { - type = UnknownType.create(); + type = baseType; break; } @@ -11370,6 +11369,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } concreteLeftType = removeTruthinessFromType(concreteLeftType); + + if (isNever(rightType)) { + return concreteLeftType; + } } else if (operator === OperatorType.Or) { // If the LHS evaluates to truthy, the Or expression will // always return the type of the left-hand side. @@ -11384,6 +11387,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } concreteLeftType = removeFalsinessFromType(concreteLeftType); + + if (isNever(rightType)) { + return concreteLeftType; + } } if (isNever(leftType) || isNever(rightType)) { diff --git a/packages/pyright-internal/src/tests/samples/unreachable1.py b/packages/pyright-internal/src/tests/samples/unreachable1.py index 5e54698f33e0..de4b21798555 100644 --- a/packages/pyright-internal/src/tests/samples/unreachable1.py +++ b/packages/pyright-internal/src/tests/samples/unreachable1.py @@ -1,6 +1,8 @@ # This sample tests the detection and reporting of unreachable code. from abc import abstractmethod +import os +import sys def func1(): @@ -85,3 +87,26 @@ def func9(): # This should be marked unreachable. return 3 + + +def func10(): + e = OSError() + a1 = os.name == "nt" and None == e.errno + reveal_type(a1, expected_text="bool") + + a2 = True and os.name == "nt" + reveal_type(a2, expected_text="bool") + + if os.name == "nt": + # This should be marked unreachable. + b = e.errno + + if sys.version_info >= (4, 0): + # This should be marked unreachable. + b = e.errno + + return + # This should be marked unreachable. + b = e.errno + + \ 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 0728d412d5c1..7bb2c1b31c8c 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator1.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator1.test.ts @@ -16,6 +16,12 @@ import { ConfigOptions } from '../common/configOptions'; import { PythonVersion } from '../common/pythonVersion'; import * as TestUtils from './testUtils'; +test('Unreachable1', () => { + const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable1.py']); + + TestUtils.validateResults(analysisResults, 0, 0, 2, 5); +}); + test('Builtins1', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['builtins1.py']); @@ -698,12 +704,6 @@ test('KwargsUnpack1', () => { TestUtils.validateResults(analysisResults, 11); }); -test('Unreachable1', () => { - const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unreachable1.py']); - - TestUtils.validateResults(analysisResults, 0, 0, 0, 2); -}); - test('FunctionMember1', () => { // Analyze with reportFunctionMemberAccess disabled. const analysisResult1 = TestUtils.typeAnalyzeSampleFiles(['functionMember1.py']);