diff --git a/docs/configuration.md b/docs/configuration.md index 84abfc016..23e0dea69 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -128,7 +128,7 @@ The following settings control pyright’s diagnostic output (warnings or errors **reportCallInDefaultInitializer** [boolean or string, optional]: Generate or suppress diagnostics for function calls within a default value initialization expression. Such calls can mask expensive operations that are performed at module initialization time. The default value for this setting is 'none'. -**reportUnnecessaryIsInstance** [boolean or string, optional]: Generate or suppress diagnostics for 'isinstance' or 'issubclass' calls where the result is statically determined to be always true or always false. Such calls are often indicative of a programming error. The default value for this setting is 'none'. +**reportUnnecessaryIsInstance** [boolean or string, optional]: Generate or suppress diagnostics for 'isinstance' or 'issubclass' calls where the result is statically determined to be always true. Such calls are often indicative of a programming error. The default value for this setting is 'none'. **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'. diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts index 9f32dd836..6d9f33b2a 100644 --- a/packages/pyright-internal/src/analyzer/checker.ts +++ b/packages/pyright-internal/src/analyzer/checker.ts @@ -2380,10 +2380,6 @@ export class Checker extends ParseTreeWalker { // If the variable type is a superclass of the isinstance // filter, we can narrow the type to the subclass. filteredTypes.push(filterType); - } else if (ClassType.isProtocolClass(filterType) && ClassType.isProtocolClass(varType)) { - // If the both the filter and variable types are protocol classes, - // assume the narrowed type is the filter type. - filteredTypes.push(filterType); } } @@ -2433,22 +2429,7 @@ export class Checker extends ParseTreeWalker { return combineTypes(objTypeList); }; - if (isNever(filteredType)) { - this._evaluator.addDiagnostic( - this._fileInfo.diagnosticRuleSet.reportUnnecessaryIsInstance, - DiagnosticRule.reportUnnecessaryIsInstance, - isInstanceCheck - ? Localizer.Diagnostic.unnecessaryIsInstanceNever().format({ - testType: this._evaluator.printType(arg0Type, /* expandTypeAlias */ false), - classType: this._evaluator.printType(getTestType(), /* expandTypeAlias */ false), - }) - : Localizer.Diagnostic.unnecessaryIsSubclassNever().format({ - testType: this._evaluator.printType(arg0Type, /* expandTypeAlias */ false), - classType: this._evaluator.printType(getTestType(), /* expandTypeAlias */ false), - }), - node - ); - } else if (isTypeSame(filteredType, arg0Type, /* ignorePseudoGeneric */ true)) { + if (isTypeSame(filteredType, arg0Type, /* ignorePseudoGeneric */ true)) { this._evaluator.addDiagnostic( this._fileInfo.diagnosticRuleSet.reportUnnecessaryIsInstance, DiagnosticRule.reportUnnecessaryIsInstance, diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index b720ca79b..ec948f748 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -220,7 +220,7 @@ export interface DiagnosticRuleSet { reportCallInDefaultInitializer: DiagnosticLevel; // Report calls to isinstance or issubclass that are statically determined - // to always be true or false. + // to always be true. reportUnnecessaryIsInstance: DiagnosticLevel; // Report calls to cast that are statically determined diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts index 1cde71abc..b1288416c 100644 --- a/packages/pyright-internal/src/localization/localize.ts +++ b/packages/pyright-internal/src/localization/localize.ts @@ -757,10 +757,6 @@ export namespace Localizer { export const unionSyntaxIllegal = () => getRawString('Diagnostic.unionSyntaxIllegal'); export const unnecessaryCast = () => new ParameterizedString<{ type: string }>(getRawString('Diagnostic.unnecessaryCast')); - export const unnecessaryIsInstanceNever = () => - new ParameterizedString<{ testType: string; classType: string }>( - getRawString('Diagnostic.unnecessaryIsInstanceNever') - ); export const unnecessaryIsInstanceAlways = () => new ParameterizedString<{ testType: string; classType: string }>( getRawString('Diagnostic.unnecessaryIsInstanceAlways') @@ -769,10 +765,6 @@ export namespace Localizer { new ParameterizedString<{ testType: string; classType: string }>( getRawString('Diagnostic.unnecessaryIsSubclassAlways') ); - export const unnecessaryIsSubclassNever = () => - new ParameterizedString<{ testType: string; classType: string }>( - getRawString('Diagnostic.unnecessaryIsSubclassNever') - ); export const unpackArgCount = () => getRawString('Diagnostic.unpackArgCount'); export const unpackedArgInTypeArgument = () => getRawString('Diagnostic.unpackedArgInTypeArgument'); export const unpackedArgWithVariadicParam = () => getRawString('Diagnostic.unpackedArgWithVariadicParam'); diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json index d063dc0fc..f399b73ac 100644 --- a/packages/pyright-internal/src/localization/package.nls.en-us.json +++ b/packages/pyright-internal/src/localization/package.nls.en-us.json @@ -393,9 +393,7 @@ "unionSyntaxIllegal": "Alternative syntax for unions requires Python 3.10 or newer", "unnecessaryCast": "Unnecessary \"cast\" call; type is already \"{type}\"", "unnecessaryIsInstanceAlways": "Unnecessary isinstance call; \"{testType}\" is always an instance of \"{classType}\"", - "unnecessaryIsInstanceNever": "Unnecessary isinstance call; \"{testType}\" is never an instance of \"{classType}\"", "unnecessaryIsSubclassAlways": "Unnecessary issubclass call; \"{testType}\" is always a subclass of \"{classType}\"", - "unnecessaryIsSubclassNever": "Unnecessary issubclass call; \"{testType}\" is never a subclass of \"{classType}\"", "unpackArgCount": "Expected a single type argument after \"Unpack\"", "unpackedArgInTypeArgument": "Unpacked arguments cannot be used in type argument lists", "unpackedArgWithVariadicParam": "Unpacked argument cannot be used for TupleTypeVar parameter", diff --git a/packages/pyright-internal/src/tests/checker.test.ts b/packages/pyright-internal/src/tests/checker.test.ts index 09d4240b5..07a14783b 100644 --- a/packages/pyright-internal/src/tests/checker.test.ts +++ b/packages/pyright-internal/src/tests/checker.test.ts @@ -189,7 +189,7 @@ test('UnnecessaryIsInstance1', () => { // Turn on errors. configOptions.diagnosticRuleSet.reportUnnecessaryIsInstance = 'error'; analysisResults = TestUtils.typeAnalyzeSampleFiles(['unnecessaryIsInstance1.py'], configOptions); - TestUtils.validateResults(analysisResults, 5); + TestUtils.validateResults(analysisResults, 4); }); test('UnnecessaryIsSubclass1', () => { @@ -201,7 +201,7 @@ test('UnnecessaryIsSubclass1', () => { // Turn on errors. configOptions.diagnosticRuleSet.reportUnnecessaryIsInstance = 'error'; analysisResults = TestUtils.typeAnalyzeSampleFiles(['unnecessaryIsSubclass1.py'], configOptions); - TestUtils.validateResults(analysisResults, 3); + TestUtils.validateResults(analysisResults, 2); }); test('UnnecessaryCast', () => { diff --git a/packages/pyright-internal/src/tests/samples/unnecessaryIsInstance1.py b/packages/pyright-internal/src/tests/samples/unnecessaryIsInstance1.py index b1d050bd3..78aad531d 100644 --- a/packages/pyright-internal/src/tests/samples/unnecessaryIsInstance1.py +++ b/packages/pyright-internal/src/tests/samples/unnecessaryIsInstance1.py @@ -17,7 +17,6 @@ def func1(p1: int, p2: Union[int, str]): # This should generate an error because this is always true. c = isinstance(p2, (float, dict, int, str)) - # This should generate an error because this is always false. d = isinstance(p1, float) e = isinstance(p2, (float, dict, int)) diff --git a/packages/pyright-internal/src/tests/samples/unnecessaryIsSubclass1.py b/packages/pyright-internal/src/tests/samples/unnecessaryIsSubclass1.py index da32923c6..e82064cc6 100644 --- a/packages/pyright-internal/src/tests/samples/unnecessaryIsSubclass1.py +++ b/packages/pyright-internal/src/tests/samples/unnecessaryIsSubclass1.py @@ -11,7 +11,6 @@ def foo(p1: Type[int], p2: Union[Type[int], Type[str]]): # This should generate an error because this is always true. c = issubclass(p2, (float, dict, int, str)) - # This should generate an error because this is always false. d = issubclass(p1, float) e = issubclass(p2, (float, dict, int)) diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index 4efe50244..844754198 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -533,7 +533,7 @@ }, "reportUnnecessaryIsInstance": { "type": "string", - "description": "Diagnostics for 'isinstance' or 'issubclass' calls where the result is statically determined to be always true or always false. Such calls are often indicative of a programming error.", + "description": "Diagnostics for 'isinstance' or 'issubclass' calls where the result is statically determined to be always true. Such calls are often indicative of a programming error.", "default": "none", "enum": [ "none", diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index 7794d1d70..56d241391 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -362,7 +362,7 @@ "reportUnnecessaryIsInstance": { "$id": "#/properties/reportUnnecessaryIsInstance", "$ref": "#/definitions/diagnostic", - "title": "Controls reporting calls to 'isinstance' or 'issubclass' where the result is statically determined to be always true or false", + "title": "Controls reporting calls to 'isinstance' or 'issubclass' where the result is statically determined to be always true", "default": "none" }, "reportUnnecessaryCast": {