Added support for new reportUnnecessaryContains diagnostic rule to catch potential bugs in in and not in containment checks.

docs/configuration.md

@@ -148,6 +148,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
 
 **reportUnnecessaryComparison** [boolean or string, optional]: Generate or suppress diagnostics for '==' or '!=' comparisons or other conditional expressions 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'.
 
+**reportUnnecessaryContains** [boolean or string, optional]: Generate or suppress diagnostics for 'in' operations that are statically determined to always evaluate to False or True. Such operations 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'.

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

@@ -107,7 +107,12 @@ import * as SymbolNameUtils from './symbolNameUtils';
 import { getLastTypedDeclaredForSymbol, isFinalVariable } from './symbolUtils';
 import { maxCodeComplexity } from './typeEvaluator';
 import { TypeEvaluator } from './typeEvaluatorTypes';
-import { isIsinstanceFilterSubclass, isIsinstanceFilterSuperclass } from './typeGuards';
+import {
+    getElementTypeForContainerNarrowing,
+    isIsinstanceFilterSubclass,
+    isIsinstanceFilterSuperclass,
+    narrowTypeForContainerElementType,
+} from './typeGuards';
 import {
     ClassType,
     combineTypes,
@@ -1188,6 +1193,11 @@ export class Checker extends ParseTreeWalker {
             if (!ParseTreeUtils.isWithinAssertExpression(node)) {
                 this._validateComparisonTypes(node);
             }
+        } else if (node.operator === OperatorType.In || node.operator === OperatorType.NotIn) {
+            // Don't apply this rule if it's within an assert.
+            if (!ParseTreeUtils.isWithinAssertExpression(node)) {
+                this._validateContainmentTypes(node);
+            }
         }
 
         this._evaluator.getType(node);
@@ -1550,6 +1560,44 @@ export class Checker extends ParseTreeWalker {
         }
     }
 
+    private _validateContainmentTypes(node: BinaryOperationNode) {
+        const leftType = this._evaluator.getType(node.leftExpression);
+        const containerType = this._evaluator.getType(node.rightExpression);
+
+        if (!leftType || !containerType) {
+            return;
+        }
+
+        if (isNever(leftType) || isNever(containerType)) {
+            return;
+        }
+
+        // Use the common narrowing logic for containment.
+        const elementType = getElementTypeForContainerNarrowing(containerType);
+        if (!elementType) {
+            return;
+        }
+        const narrowedType = narrowTypeForContainerElementType(this._evaluator, leftType, elementType);
+
+        if (isNever(narrowedType)) {
+            const getMessage = () => {
+                return node.operator === OperatorType.In
+                    ? Localizer.Diagnostic.containmentAlwaysFalse()
+                    : Localizer.Diagnostic.containmentAlwaysTrue();
+            };
+
+            this._evaluator.addDiagnostic(
+                this._fileInfo.diagnosticRuleSet.reportUnnecessaryContains,
+                DiagnosticRule.reportUnnecessaryContains,
+                getMessage().format({
+                    leftType: this._evaluator.printType(leftType, /* expandTypeAlias */ true),
+                    rightType: this._evaluator.printType(elementType, /* expandTypeAlias */ true),
+                }),
+                node
+            );
+        }
+    }
+
     // Determines whether the types of the two operands for an == or != operation
     // have overlapping types.
     private _validateComparisonTypes(node: BinaryOperationNode) {

packages/pyright-internal/src/common/configOptions.ts

@@ -248,6 +248,9 @@ export interface DiagnosticRuleSet {
     // Report == or != operators that always evaluate to True or False.
     reportUnnecessaryComparison: DiagnosticLevel;
 
+    // Report 'in' operations that always evaluate to True or False.
+    reportUnnecessaryContains: DiagnosticLevel;
+
     // Report assert expressions that will always evaluate to true.
     reportAssertAlwaysTrue: DiagnosticLevel;
 
@@ -370,6 +373,7 @@ export function getDiagLevelDiagnosticRules() {
         DiagnosticRule.reportUnnecessaryIsInstance,
         DiagnosticRule.reportUnnecessaryCast,
         DiagnosticRule.reportUnnecessaryComparison,
+        DiagnosticRule.reportUnnecessaryContains,
         DiagnosticRule.reportAssertAlwaysTrue,
         DiagnosticRule.reportSelfClsParameterName,
         DiagnosticRule.reportImplicitStringConcatenation,
@@ -451,6 +455,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
         reportUnnecessaryIsInstance: 'none',
         reportUnnecessaryCast: 'none',
         reportUnnecessaryComparison: 'none',
+        reportUnnecessaryContains: 'none',
         reportAssertAlwaysTrue: 'none',
         reportSelfClsParameterName: 'none',
         reportImplicitStringConcatenation: 'none',
@@ -528,6 +533,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
         reportUnnecessaryIsInstance: 'none',
         reportUnnecessaryCast: 'none',
         reportUnnecessaryComparison: 'none',
+        reportUnnecessaryContains: 'none',
         reportAssertAlwaysTrue: 'warning',
         reportSelfClsParameterName: 'warning',
         reportImplicitStringConcatenation: 'none',
@@ -605,6 +611,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
         reportUnnecessaryIsInstance: 'error',
         reportUnnecessaryCast: 'error',
         reportUnnecessaryComparison: 'error',
+        reportUnnecessaryContains: 'error',
         reportAssertAlwaysTrue: 'error',
         reportSelfClsParameterName: 'error',
         reportImplicitStringConcatenation: 'none',

packages/pyright-internal/src/common/diagnosticRules.ts

@@ -64,6 +64,7 @@ export enum DiagnosticRule {
     reportUnnecessaryIsInstance = 'reportUnnecessaryIsInstance',
     reportUnnecessaryCast = 'reportUnnecessaryCast',
     reportUnnecessaryComparison = 'reportUnnecessaryComparison',
+    reportUnnecessaryContains = 'reportUnnecessaryContains',
     reportAssertAlwaysTrue = 'reportAssertAlwaysTrue',
     reportSelfClsParameterName = 'reportSelfClsParameterName',
     reportImplicitStringConcatenation = 'reportImplicitStringConcatenation',

packages/pyright-internal/src/localization/localize.ts

@@ -291,6 +291,14 @@ export namespace Localizer {
             new ParameterizedString<{ type: string }>(getRawString('Diagnostic.constructorNoArgs'));
         export const constructorParametersMismatch = () =>
             new ParameterizedString<{ classType: string }>(getRawString('Diagnostic.constructorParametersMismatch'));
+        export const containmentAlwaysFalse = () =>
+            new ParameterizedString<{ leftType: string; rightType: string }>(
+                getRawString('Diagnostic.containmentAlwaysFalse')
+            );
+        export const containmentAlwaysTrue = () =>
+            new ParameterizedString<{ leftType: string; rightType: string }>(
+                getRawString('Diagnostic.containmentAlwaysTrue')
+            );
         export const continueInFinally = () => getRawString('Diagnostic.continueInFinally');
         export const continueOutsideLoop = () => getRawString('Diagnostic.continueOutsideLoop');
         export const dataClassBaseClassNotFrozen = () => getRawString('Diagnostic.dataClassBaseClassNotFrozen');

packages/pyright-internal/src/localization/package.nls.en-us.json

@@ -68,6 +68,8 @@
         "concatenateTypeArgsMissing": "\"Concatenate\" requires at least two type arguments",
         "constantRedefinition": "\"{name}\" is constant (because it is uppercase) and cannot be redefined",
         "constructorParametersMismatch": "Mismatch between signature of __new__ and __init__ in class \"{classType}\"",
+        "containmentAlwaysFalse": "Expression will always evaluate to False since the types \"{leftType}\" and \"{rightType}\" have no overlap",
+        "containmentAlwaysTrue": "Expression will always evaluate to True since the types \"{leftType}\" and \"{rightType}\" have no overlap",
         "continueInFinally": "\"continue\" cannot be used within a finally clause",
         "continueOutsideLoop": "\"continue\" can be used only within a loop",
         "constructorNoArgs": "Expected no arguments to \"{type}\" constructor",

packages/pyright-internal/src/tests/checker.test.ts

@@ -252,6 +252,18 @@ test('UnnecessaryCast1', () => {
     TestUtils.validateResults(analysisResults, 1);
 });
 
+test('UnnecessaryContains1', () => {
+    const configOptions = new ConfigOptions('.');
+
+    let analysisResults = TestUtils.typeAnalyzeSampleFiles(['unnecessaryContains1.py'], configOptions);
+    TestUtils.validateResults(analysisResults, 0);
+
+    // Turn on errors.
+    configOptions.diagnosticRuleSet.reportUnnecessaryContains = 'error';
+    analysisResults = TestUtils.typeAnalyzeSampleFiles(['unnecessaryContains1.py'], configOptions);
+    TestUtils.validateResults(analysisResults, 4);
+});
+
 test('TypeIgnore1', () => {
     const configOptions = new ConfigOptions('.');
 

packages/pyright-internal/src/tests/samples/unnecessaryContains1.py

@@ -0,0 +1,34 @@
+# This sample tests the "reportUnnecessaryContains" diagnostic rule.
+
+from typing import Literal
+
+
+def func1(x: str | int):
+    if x in ("a",):
+        return
+
+    # This should generate an error if "reportUnnecessaryContains" is enabled.
+    if x in (b"a",):
+        return
+
+
+def func2(x: Literal[1, 2, 3]):
+    if x in ("4", 1):
+        return
+
+    # This should generate an error if "reportUnnecessaryContains" is enabled.
+    if x not in ("4", "1"):
+        pass
+
+    # This should generate an error if "reportUnnecessaryContains" is enabled.
+    if x in (4, 5):
+        return
+
+
+def func3(x: list[str]):
+    if x in (["hi"], [2, 3]):
+        return
+
+    # This should generate an error if "reportUnnecessaryContains" is enabled.
+    if x not in ([1, 2], [3]):
+        pass

packages/vscode-pyright/package.json

@@ -630,6 +630,17 @@
                                 "error"
                             ]
                         },
+                        "reportUnnecessaryContains": {
+                            "type": "string",
+                            "description": "Diagnostics for 'in' operation that is statically determined to be unnecessary. Such operations are sometimes indicative of a programming error.",
+                            "default": "none",
+                            "enum": [
+                                "none",
+                                "information",
+                                "warning",
+                                "error"
+                            ]
+                        },
                         "reportAssertAlwaysTrue": {
                             "type": "string",
                             "description": "Diagnostics for 'assert' statement that will provably always assert. This can be indicative of a programming error.",

packages/vscode-pyright/schemas/pyrightconfig.schema.json

@@ -424,6 +424,12 @@
       "title": "Controls reporting the use of '==' or '!=' comparisons that are unnecessary",
       "default": "none"
     },
+    "reportUnnecessaryContains": {
+      "$id": "#/properties/reportUnnecessaryContains",
+      "$ref": "#/definitions/diagnostic",
+      "title": "Controls reporting the use of 'in' operations that are unnecessary",
+      "default": "none"
+    },
     "reportAssertAlwaysTrue": {
       "$id": "#/properties/reportAssertAlwaysTrue",
       "$ref": "#/definitions/diagnostic",