Added config feature "reportAssertAlwaysTrue". It flags errant assert statements.

This commit is contained in:
Eric Traut 2019-11-17 18:02:46 -08:00
parent 49fec491da
commit 4e559d327a
7 changed files with 61 additions and 6 deletions

View File

@ -258,6 +258,12 @@
"title": "Controls reporting calls to 'cast' that are unnecessary",
"default": "none"
},
"reportAssertAlwaysTrue": {
"$id": "#/properties/reportAssertAlwaysTrue",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting assert expressions that will always evaluate to true",
"default": "none"
},
"pythonVersion": {
"$id": "#/properties/pythonVersion",
"type": "string",

View File

@ -96,6 +96,8 @@ The following settings control pyrights 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'.
**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'.
## Execution Environment Options
Pyright allows multiple “execution environments” to be defined for different portions of your source tree. For example, a subtree may be designed to run with different import search paths or a different version of the python interpreter than the rest of the source base.

View File

@ -386,7 +386,18 @@ export class Checker extends ParseTreeWalker {
this._getTypeOfExpression(node.exceptionExpression);
}
this._getTypeOfExpression(node.testExpression);
const type = this._getTypeOfExpression(node.testExpression);
if (type.category === TypeCategory.Object) {
if (ClassType.isBuiltIn(type.classType, 'Tuple') && type.classType.typeArguments) {
if (type.classType.typeArguments.length > 0) {
this._evaluator.addDiagnosticForTextRange(this._fileInfo,
this._fileInfo.diagnosticSettings.reportAssertAlwaysTrue,
DiagnosticRule.reportAssertAlwaysTrue,
`Assert expression always evaluates to true`, node.testExpression);
}
}
}
return true;
}

View File

@ -146,6 +146,9 @@ export interface DiagnosticSettings {
// Report calls to cast that are statically determined
// to always unnecessary.
reportUnnecessaryCast: DiagnosticLevel;
// Report assert expressions that will always evaluate to true.
reportAssertAlwaysTrue: DiagnosticLevel;
}
export function cloneDiagnosticSettings(
@ -198,7 +201,8 @@ export function getDiagLevelSettings() {
DiagnosticRule.reportUnknownMemberType,
DiagnosticRule.reportCallInDefaultInitializer,
DiagnosticRule.reportUnnecessaryIsInstance,
DiagnosticRule.reportUnnecessaryCast
DiagnosticRule.reportUnnecessaryCast,
DiagnosticRule.reportAssertAlwaysTrue
];
}
@ -236,7 +240,8 @@ export function getStrictDiagnosticSettings(): DiagnosticSettings {
reportUnknownMemberType: 'error',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'error',
reportUnnecessaryCast: 'error'
reportUnnecessaryCast: 'error',
reportAssertAlwaysTrue: 'error'
};
return diagSettings;
@ -276,7 +281,8 @@ export function getDefaultDiagnosticSettings(): DiagnosticSettings {
reportUnknownMemberType: 'none',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'none',
reportUnnecessaryCast: 'none'
reportUnnecessaryCast: 'none',
reportAssertAlwaysTrue: 'warning'
};
return diagSettings;
@ -623,7 +629,12 @@ export class ConfigOptions {
// Read the "reportUnnecessaryCast" entry.
reportUnnecessaryCast: this._convertDiagnosticLevel(
configObj.reportUnnecessaryCast, DiagnosticRule.reportUnnecessaryCast,
defaultSettings.reportUnnecessaryCast)
defaultSettings.reportUnnecessaryCast),
// Read the "reportAssertAlwaysTrue" entry.
reportAssertAlwaysTrue: this._convertDiagnosticLevel(
configObj.reportAssertAlwaysTrue, DiagnosticRule.reportAssertAlwaysTrue,
defaultSettings.reportAssertAlwaysTrue)
};
// Read the "venvPath".

View File

@ -42,5 +42,6 @@ export const enum DiagnosticRule {
reportUnknownMemberType = 'reportUnknownMemberType',
reportCallInDefaultInitializer = 'reportCallInDefaultInitializer',
reportUnnecessaryIsInstance = 'reportUnnecessaryIsInstance',
reportUnnecessaryCast = 'reportUnnecessaryCast'
reportUnnecessaryCast = 'reportUnnecessaryCast',
reportAssertAlwaysTrue = 'reportAssertAlwaysTrue'
}

View File

@ -673,6 +673,24 @@ test('UnnecessaryCast', () => {
validateResults(analysisResults, 1);
});
test('AssertAlwaysTrue', () => {
const configOptions = new ConfigOptions('.');
// By default, this is reported as a warning.
let analysisResults = TestUtils.typeAnalyzeSampleFiles(['assert1.py'], configOptions);
validateResults(analysisResults, 0, 1);
// Enable it as an error.
configOptions.diagnosticSettings.reportAssertAlwaysTrue = 'error';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['assert1.py'], configOptions);
validateResults(analysisResults, 1, 0);
// Turn off the diagnostic.
configOptions.diagnosticSettings.reportAssertAlwaysTrue = 'none';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['assert1.py'], configOptions);
validateResults(analysisResults, 0, 0);
});
test('RevealedType1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['revealedType1.py']);

View File

@ -0,0 +1,6 @@
# This sample tests the ability to detect errant assert calls
# that are always true - the "reportAssertAlwaysTrue" option.
# This should generate a warning
assert (1 != 2, "Error message")