Merge pull request microsoft#1333 from jakebailey/pull-pyright-141

packages/pyright/.gitrepo

@@ -6,7 +6,7 @@
 [subrepo]
 	remote = https://github.com/microsoft/pyright.git
 	branch = master
-	commit = 2918da1d0281ef0d8a1336512ccc81c4232960d6
-	parent = 294513752d5ea678af3550a9a0a546388c2fea8e
+	commit = 6410a8a250c753cc7a8104a2168da4b08c9c9668
+	parent = 2b7bd9eeef1ab98a45051f8d43009df6faa35108
 	method = merge
 	cmdver = 0.4.1

packages/pyright/README.md

@@ -93,8 +93,6 @@ To update to the latest version:
 ## Using Pyright with VS Code Python Extension
 Pyright’s type-checking functionality and language features are now incorporated into a VS Code extension called [Pylance](https://github.com/microsoft/pylance-release), the officially supported Python Language Server from Microsoft. Pylance is designed to work with the Python extension for VS Code. In addition to Pyright’s functionality, Pylance adds compatibility with several advanced features including IntelliCode for AI-assisted completions. If you are a VS Code user, we recommend that you uninstall Pyright and instead install Pylance. You will get all the benefits of Pyright and more! 
 
-Installing both Pyright and Pylance at the same time is not recommended. If both are installed and enabled, you will see duplicate errors, hover text, and completion suggestions. 
-
 
 ## Documentation
 * [Getting Started with Type Checking](/docs/getting-started.md)

packages/pyright/docs/command-line.md

@@ -50,7 +50,7 @@ If the “--outputjson” option is specified on the command line, diagnostics a
 {
     version: string,
     time: string,
-    diagnostics: Diagnostic[],
+    generalDiagnostics: Diagnostic[],
     summary: {
         filesAnalyzed: number,
         errorCount: number,

packages/pyright/docs/configuration.md

@@ -130,6 +130,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
 
 **reportUnnecessaryCast** [boolean or string, optional]: Generate or suppress diagnostics for 'cast' calls that are statically determined to be unnecessary. Such calls are sometimes indicative of a programming error. The default value for this setting is 'none'.
 
+**reportUnnecessaryComparison** [boolean or string, optional]: Generate or suppress diagnostics for '==' or '!=' comparisons that are statically determined to always evaluate to False or True. Such comparisons are sometimes indicative of a programming error. The default value for this setting is 'none'.
+
 **reportAssertAlwaysTrue** [boolean or string, optional]: Generate or suppress diagnostics for 'assert' statement that will provably always assert. This can be indicative of a programming error. The default value for this setting is 'warning'.
 
 **reportSelfClsParameterName** [boolean or string, optional]: Generate or suppress diagnostics for a missing or misnamed “self” parameter in instance methods and “cls” parameter in class methods. Instance methods in metaclasses (classes that derive from “type”) are allowed to use “cls” for instance methods. The default value for this setting is 'warning'.
@@ -274,6 +276,7 @@ The following table lists the default severity levels for each diagnostic rule w
 | reportCallInDefaultInitializer            | "none"     | "none"     | "none"     |
 | reportUnnecessaryIsInstance               | "none"     | "none"     | "error"    |
 | reportUnnecessaryCast                     | "none"     | "none"     | "error"    |
+| reportUnnecessaryComparison               | "none"     | "none"     | "error"    |
 | reportAssertAlwaysTrue                    | "none"     | "warning"  | "error"    |
 | reportSelfClsParameterName                | "none"     | "warning"  | "error"    |
 | reportImplicitStringConcatenation         | "none"     | "none"     | "none"     |

packages/pyright/lerna.json

@@ -2,7 +2,7 @@
     "packages": [
         "packages/*"
     ],
-    "version": "1.1.129",
+    "version": "1.1.130",
     "command": {
         "version": {
             "push": false,

packages/pyright/packages/pyright-internal/package.json

@@ -2,7 +2,7 @@
     "name": "pyright-internal",
     "displayName": "pyright",
     "description": "Type checker for the Python language",
-    "version": "1.1.129",
+    "version": "1.1.130",
     "license": "MIT",
     "private": true,
     "files": [

packages/pyright/packages/pyright-internal/src/analyzer/checker.ts

@@ -92,6 +92,7 @@ import {
     isAnyOrUnknown,
     isClass,
     isFunction,
+    isModule,
     isNever,
     isNone,
     isObject,
@@ -613,44 +614,6 @@ export class Checker extends ParseTreeWalker {
     }
 
     visitIf(node: IfNode): boolean {
-        // Check for expressions where a variable is being compared to
-        // a literal string or number. Look for a common bug where
-        // the comparison will always be False. Don't do this for
-        // expressions like 'sys.platform == "win32"' because those
-        // can change based on the execution environment and are therefore
-        // valid.
-        if (
-            node.testExpression.nodeType === ParseNodeType.BinaryOperation &&
-            node.testExpression.operator === OperatorType.Equals &&
-            evaluateStaticBoolExpression(node.testExpression, this._fileInfo.executionEnvironment) === undefined
-        ) {
-            const rightType = this._evaluator.getType(node.testExpression.rightExpression);
-            if (rightType && isLiteralTypeOrUnion(rightType)) {
-                const leftType = this._evaluator.getType(node.testExpression.leftExpression);
-                if (leftType && isLiteralTypeOrUnion(leftType)) {
-                    let isPossiblyTrue = false;
-
-                    doForEachSubtype(leftType, (leftSubtype) => {
-                        if (this._evaluator.canAssignType(rightType, leftSubtype, new DiagnosticAddendum())) {
-                            isPossiblyTrue = true;
-                        }
-                    });
-
-                    if (!isPossiblyTrue) {
-                        this._evaluator.addDiagnostic(
-                            this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
-                            DiagnosticRule.reportGeneralTypeIssues,
-                            Localizer.Diagnostic.comparisonAlwaysFalse().format({
-                                leftType: this._evaluator.printType(leftType, /* expandTypeAlias */ true),
-                                rightType: this._evaluator.printType(rightType, /* expandTypeAlias */ true),
-                            }),
-                            node.testExpression
-                        );
-                    }
-                }
-            }
-        }
-
         this._evaluator.getType(node.testExpression);
         return true;
     }
@@ -928,6 +891,13 @@ export class Checker extends ParseTreeWalker {
     }
 
     visitBinaryOperation(node: BinaryOperationNode): boolean {
+        if (node.operator === OperatorType.Equals || node.operator === OperatorType.NotEquals) {
+            // Don't apply this rule if it's within an assert.
+            if (!ParseTreeUtils.isWithinAssertExpression(node)) {
+                this._validateComparisonTypes(node);
+            }
+        }
+
         this._evaluator.getType(node);
         return true;
     }
@@ -1068,6 +1038,169 @@ export class Checker extends ParseTreeWalker {
         return false;
     }
 
+    // Determines whether the types of the two operands for an == or != operation
+    // have overlapping types.
+    private _validateComparisonTypes(node: BinaryOperationNode) {
+        const leftType = this._evaluator.getType(node.leftExpression);
+        const rightType = this._evaluator.getType(node.rightExpression);
+
+        if (!leftType || !rightType) {
+            return;
+        }
+
+        // Check for the special case where the LHS and RHS are both literals.
+        if (isLiteralTypeOrUnion(rightType) && isLiteralTypeOrUnion(leftType)) {
+            if (evaluateStaticBoolExpression(node, this._fileInfo.executionEnvironment) === undefined) {
+                let isPossiblyTrue = false;
+
+                doForEachSubtype(leftType, (leftSubtype) => {
+                    if (this._evaluator.canAssignType(rightType, leftSubtype, new DiagnosticAddendum())) {
+                        isPossiblyTrue = true;
+                    }
+                });
+
+                if (!isPossiblyTrue) {
+                    this._evaluator.addDiagnostic(
+                        this._fileInfo.diagnosticRuleSet.reportUnnecessaryComparison,
+                        DiagnosticRule.reportUnnecessaryComparison,
+                        Localizer.Diagnostic.comparisonAlwaysFalse().format({
+                            leftType: this._evaluator.printType(leftType, /* expandTypeAlias */ true),
+                            rightType: this._evaluator.printType(rightType, /* expandTypeAlias */ true),
+                        }),
+                        node
+                    );
+                }
+            }
+        } else {
+            let isComparable = false;
+
+            doForEachSubtype(leftType, (leftSubtype) => {
+                if (isComparable) {
+                    return;
+                }
+
+                leftSubtype = transformTypeObjectToClass(leftSubtype);
+                leftSubtype = this._evaluator.makeTopLevelTypeVarsConcrete(leftSubtype);
+                doForEachSubtype(rightType, (rightSubtype) => {
+                    if (isComparable) {
+                        return;
+                    }
+
+                    rightSubtype = transformTypeObjectToClass(rightSubtype);
+                    rightSubtype = this._evaluator.makeTopLevelTypeVarsConcrete(rightSubtype);
+
+                    if (this._isTypeComparable(leftSubtype, rightSubtype)) {
+                        isComparable = true;
+                    }
+                });
+            });
+
+            if (!isComparable) {
+                const leftTypeText = this._evaluator.printType(leftType, /* expandTypeAlias */ true);
+                const rightTypeText = this._evaluator.printType(rightType, /* expandTypeAlias */ true);
+
+                const message =
+                    node.operator === OperatorType.Equals
+                        ? Localizer.Diagnostic.comparisonAlwaysFalse()
+                        : Localizer.Diagnostic.comparisonAlwaysTrue();
+
+                this._evaluator.addDiagnostic(
+                    this._fileInfo.diagnosticRuleSet.reportUnnecessaryComparison,
+                    DiagnosticRule.reportUnnecessaryComparison,
+                    message.format({
+                        leftType: leftTypeText,
+                        rightType: rightTypeText,
+                    }),
+                    node
+                );
+            }
+        }
+    }
+
+    // Determines whether the two types are potentially comparable -- i.e.
+    // their types overlap in such a way that it makes sense for them to
+    // be compared with an == or != operator.
+    private _isTypeComparable(leftType: Type, rightType: Type) {
+        if (isAnyOrUnknown(leftType) || isAnyOrUnknown(rightType)) {
+            return true;
+        }
+
+        if (isNever(leftType) || isNever(rightType)) {
+            return false;
+        }
+
+        if (isModule(leftType) || isModule(rightType)) {
+            return !isTypeSame(leftType, rightType);
+        }
+
+        if (isNone(leftType) || isNone(rightType)) {
+            return !isTypeSame(leftType, rightType);
+        }
+
+        if (isClass(leftType)) {
+            if (isClass(rightType)) {
+                const genericLeftType = ClassType.cloneForSpecialization(
+                    leftType,
+                    /* typeArguments */ undefined,
+                    /* isTypeArgumentExplicit */ false
+                );
+                const genericRightType = ClassType.cloneForSpecialization(
+                    rightType,
+                    /* typeArguments */ undefined,
+                    /* isTypeArgumentExplicit */ false
+                );
+
+                if (
+                    this._evaluator.canAssignType(genericLeftType, genericRightType, new DiagnosticAddendum()) ||
+                    this._evaluator.canAssignType(genericRightType, genericLeftType, new DiagnosticAddendum())
+                ) {
+                    return true;
+                }
+            }
+
+            // Does the class have an operator overload for eq?
+            const metaclass = leftType.details.effectiveMetaclass;
+            if (metaclass && isClass(metaclass)) {
+                if (lookUpClassMember(metaclass, '__eq__', ClassMemberLookupFlags.SkipObjectBaseClass)) {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        if (isObject(leftType)) {
+            if (isObject(rightType)) {
+                const genericLeftType = ClassType.cloneForSpecialization(
+                    leftType.classType,
+                    /* typeArguments */ undefined,
+                    /* isTypeArgumentExplicit */ false
+                );
+                const genericRightType = ClassType.cloneForSpecialization(
+                    rightType.classType,
+                    /* typeArguments */ undefined,
+                    /* isTypeArgumentExplicit */ false
+                );
+
+                if (
+                    this._evaluator.canAssignType(genericLeftType, genericRightType, new DiagnosticAddendum()) ||
+                    this._evaluator.canAssignType(genericRightType, genericLeftType, new DiagnosticAddendum())
+                ) {
+                    return true;
+                }
+            }
+
+            // Does the class have an operator overload for eq?
+            if (lookUpClassMember(leftType.classType, '__eq__', ClassMemberLookupFlags.SkipObjectBaseClass)) {
+                return true;
+            }
+
+            return false;
+        }
+
+        return true;
+    }
+
     // Determines whether the specified type is one that should trigger
     // an "unused" value diagnostic.
     private _isTypeValidForUnusedValueTest(type: Type) {
@@ -2687,9 +2820,23 @@ export class Checker extends ParseTreeWalker {
                 }
 
                 if (overrideFunction) {
-                    // Don't check magic functions or private symbols.
-                    if (!SymbolNameUtils.isDunderName(name) && !SymbolNameUtils.isPrivateName(name)) {
-                        if (!this._evaluator.canOverrideMethod(baseClassSymbolType, overrideFunction, diagAddendum)) {
+                    const exemptMethods = ['__init__', '__new__', '__init_subclass__'];
+
+                    // Don't enforce parameter names for dundered methods. Many of them
+                    // are misnamed in typeshed stubs, so this would result in many
+                    // false positives.
+                    const enforceParamNameMatch = !SymbolNameUtils.isDunderName(name);
+
+                    // Don't check certain magic functions or private symbols.
+                    if (!exemptMethods.some((exempt) => exempt === name) && !SymbolNameUtils.isPrivateName(name)) {
+                        if (
+                            !this._evaluator.canOverrideMethod(
+                                baseClassSymbolType,
+                                overrideFunction,
+                                diagAddendum,
+                                enforceParamNameMatch
+                            )
+                        ) {
                             const decl = overrideFunction.details.declaration;
                             if (decl && decl.type === DeclarationType.Function) {
                                 const diag = this._evaluator.addDiagnostic(

packages/pyright/packages/pyright-internal/src/analyzer/parseTreeUtils.ts

@@ -1015,6 +1015,48 @@ export function isWithinLoop(node: ParseNode): boolean {
     return false;
 }
 
+export function isWithinTryBlock(node: ParseNode): boolean {
+    let curNode: ParseNode | undefined = node;
+    let prevNode: ParseNode | undefined;
+
+    while (curNode) {
+        switch (curNode.nodeType) {
+            case ParseNodeType.Try: {
+                return curNode.trySuite === prevNode;
+            }
+
+            case ParseNodeType.Function:
+            case ParseNodeType.Module:
+            case ParseNodeType.Class: {
+                break;
+            }
+        }
+
+        prevNode = curNode;
+        curNode = curNode.parent;
+    }
+
+    return false;
+}
+
+export function isWithinAssertExpression(node: ParseNode): boolean {
+    let curNode: ParseNode | undefined = node;
+    let prevNode: ParseNode | undefined;
+
+    while (curNode) {
+        switch (curNode.nodeType) {
+            case ParseNodeType.Assert: {
+                return curNode.testExpression === prevNode;
+            }
+        }
+
+        prevNode = curNode;
+        curNode = curNode.parent;
+    }
+
+    return false;
+}
+
 export function getDocString(statements: StatementNode[]): string | undefined {
     // See if the first statement in the suite is a triple-quote string.
     if (statements.length === 0) {