Skip to content

Commit

Permalink
Improved type evaluation for expressions that are determined to be un…
Browse files Browse the repository at this point in the history
…reachable. Such code should evaluate to the type `Never` and never generate errors or warnings.
  • Loading branch information
msfterictraut committed Apr 27, 2022
1 parent d22d889 commit 91de1fe
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 25 deletions.
7 changes: 6 additions & 1 deletion packages/pyright-internal/src/analyzer/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions packages/pyright-internal/src/analyzer/codeFlowEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import {
isTypeSame,
isTypeVar,
ModuleType,
NeverType,
removeIncompleteUnknownFromUnion,
Type,
TypeVarType,
Expand Down Expand Up @@ -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) {
Expand Down
16 changes: 9 additions & 7 deletions packages/pyright-internal/src/analyzer/patternMatching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ function narrowTypeBasedOnClassPattern(
);
}

if (!TypeBase.isInstantiable(exprType)) {
if (!TypeBase.isInstantiable(exprType) && !isNever(exprType)) {
evaluator.addDiagnostic(
getFileInfo(pattern).diagnosticRuleSet.reportGeneralTypeIssues,
DiagnosticRule.reportGeneralTypeIssues,
Expand Down Expand Up @@ -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);

Expand Down
25 changes: 16 additions & 9 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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.
Expand All @@ -11384,6 +11387,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}

concreteLeftType = removeFalsinessFromType(concreteLeftType);

if (isNever(rightType)) {
return concreteLeftType;
}
}

if (isNever(leftType) || isNever(rightType)) {
Expand Down
25 changes: 25 additions & 0 deletions packages/pyright-internal/src/tests/samples/unreachable1.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# This sample tests the detection and reporting of unreachable code.

from abc import abstractmethod
import os
import sys


def func1():
Expand Down Expand Up @@ -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


12 changes: 6 additions & 6 deletions packages/pyright-internal/src/tests/typeEvaluator1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);

Expand Down Expand Up @@ -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']);
Expand Down

0 comments on commit 91de1fe

Please sign in to comment.