Modified reportUnnecessaryIsInstance diagnostic to never report always-false conditions because the type checker no longer generates such conditions.

This commit is contained in:
Eric Traut 2021-07-03 10:03:49 -06:00
parent c78f6db84b
commit 845764c5ae
10 changed files with 7 additions and 38 deletions

View File

@ -128,7 +128,7 @@ The following settings control pyrights 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'.

View File

@ -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,

View File

@ -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

View File

@ -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');

View File

@ -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",

View File

@ -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', () => {

View File

@ -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))

View File

@ -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))

View File

@ -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",

View File

@ -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": {