From 85a3eaf63ca9c5799034414537eefb4702a410ad Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 2 Aug 2019 16:50:18 -0700 Subject: [PATCH] Added new rule "reportCallInDefaultInitializer" that reports usage of function calls within default value initialization expressions. --- client/schemas/pyrightconfig.schema.json | 6 ++++ docs/configuration.md | 2 ++ server/src/analyzer/commentUtils.ts | 32 ++++++++++++++++--- server/src/analyzer/typeAnalyzer.ts | 22 +++++++++---- server/src/common/configOptions.ts | 20 +++++++++--- .../src/tests/samples/defaultInitializer1.py | 10 ++++++ server/src/tests/typeAnalyzer.test.ts | 13 ++++++++ 7 files changed, 91 insertions(+), 14 deletions(-) create mode 100644 server/src/tests/samples/defaultInitializer1.py diff --git a/client/schemas/pyrightconfig.schema.json b/client/schemas/pyrightconfig.schema.json index faac23561..02e72f6f0 100644 --- a/client/schemas/pyrightconfig.schema.json +++ b/client/schemas/pyrightconfig.schema.json @@ -234,6 +234,12 @@ "title": "Controls reporting class and instance variables whose types are unknown", "default": "none" }, + "reportCallInDefaultInitializer": { + "$id": "#/properties/reportCallInDefaultInitializer", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting usage of function calls within a default value initializer expression", + "default": "none" + }, "pythonVersion": { "$id": "#/properties/pythonVersion", "type": "string", diff --git a/docs/configuration.md b/docs/configuration.md index e9a7be1a5..cfb463b19 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -84,6 +84,8 @@ The following settings control pyright's diagnostic output (warnings or errors). **reportUnknownMemberType** [boolean or string, optional]: Generate or suppress diagnostics for class or instance variables that have an unknown type. The default value for this setting is 'none'. +**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'. + ## 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. diff --git a/server/src/analyzer/commentUtils.ts b/server/src/analyzer/commentUtils.ts index 8ec0574a4..132d1afc5 100644 --- a/server/src/analyzer/commentUtils.ts +++ b/server/src/analyzer/commentUtils.ts @@ -17,9 +17,11 @@ export class CommentUtils { static getFileLevelDirectives(tokens: TextRangeCollection, defaultSettings: DiagnosticSettings, useStrict: boolean): DiagnosticSettings { - let settings = useStrict ? - getStrictDiagnosticSettings() : - cloneDiagnosticSettings(defaultSettings); + let settings = cloneDiagnosticSettings(defaultSettings); + + if (useStrict) { + this._applyStrictSettings(settings); + } for (let i = 0; i < tokens.count; i++) { const token = tokens.getItemAt(i); @@ -35,6 +37,28 @@ export class CommentUtils { return settings; } + private static _applyStrictSettings(settings: DiagnosticSettings) { + const strictSettings = getStrictDiagnosticSettings(); + const boolSettingNames = getBooleanDiagnosticSettings(); + const diagSettingNames = getDiagLevelSettings(); + + // Enable the strict settings as appropriate. + for (const setting of boolSettingNames) { + if ((strictSettings as any)[setting]) { + (settings as any)[setting] = true; + } + } + + for (const setting of diagSettingNames) { + const strictValue: DiagnosticLevel = (strictSettings as any)[setting]; + const prevValue: DiagnosticLevel = (settings as any)[setting]; + + if (strictValue === 'error' || (strictValue === 'warning' && prevValue !== 'error')) { + (settings as any)[setting] = strictValue; + } + } + } + private static _parsePyrightComment(commentValue: string, settings: DiagnosticSettings) { // Is this a pyright-specific comment? const pyrightPrefix = 'pyright:'; @@ -45,7 +69,7 @@ export class CommentUtils { // If it contains a "strict" operand, replace the existing // diagnostic settings with their strict counterparts. if (operandList.some(s => s === 'strict')) { - settings = getStrictDiagnosticSettings(); + this._applyStrictSettings(settings); } for (let operand of operandList) { diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 0211d3c26..3faddf429 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -26,7 +26,7 @@ import { AssertNode, AssignmentNode, AugmentedAssignmentExpressionNode, SuiteNode, TernaryExpressionNode, TryNode, TupleExpressionNode, TypeAnnotationExpressionNode, UnaryExpressionNode, UnpackExpressionNode, WhileNode, WithNode, YieldExpressionNode, YieldFromExpressionNode } from '../parser/parseNodes'; -import { KeywordType, OperatorType } from '../parser/tokenizerTypes'; +import { KeywordType } from '../parser/tokenizerTypes'; import { ScopeUtils } from '../scopeUtils'; import { AnalyzerFileInfo } from './analyzerFileInfo'; import { AnalyzerNodeInfo } from './analyzerNodeInfo'; @@ -36,16 +36,15 @@ import { ImportResult, ImportType } from './importResult'; import { DefaultTypeSourceId, TypeSourceId } from './inferredType'; import { ParseTreeUtils } from './parseTreeUtils'; import { ParseTreeWalker } from './parseTreeWalker'; -import { Scope, ScopeType, SymbolWithScope } from './scope'; +import { Scope, ScopeType } from './scope'; import { Declaration, Symbol, SymbolCategory, SymbolTable } from './symbol'; import { SymbolUtils } from './symbolUtils'; import { TypeConstraintBuilder } from './typeConstraint'; import { TypeConstraintUtils } from './typeConstraintUtils'; import { AnyType, ClassType, ClassTypeFlags, FunctionParameter, FunctionType, - FunctionTypeFlags, ModuleType, NeverType, NoneType, ObjectType, - OverloadedFunctionType, PropertyType, Type, TypeCategory, TypeVarType, UnboundType, - UnionType, - UnknownType } from './types'; + FunctionTypeFlags, ModuleType, NoneType, ObjectType, + OverloadedFunctionType, PropertyType, Type, TypeCategory, TypeVarType, + UnboundType, UnionType, UnknownType } from './types'; import { ClassMemberLookupFlags, TypeUtils } from './typeUtils'; interface AliasMapEntry { @@ -73,6 +72,7 @@ export class TypeAnalyzer extends ParseTreeWalker { private readonly _moduleNode: ModuleNode; private readonly _fileInfo: AnalyzerFileInfo; private _currentScope: Scope; + private _defaultValueInitializerExpression = false; // Indicates where there was a change in the type analysis // the last time analyze() was called. Callers should repeatedly @@ -319,7 +319,10 @@ export class TypeAnalyzer extends ParseTreeWalker { if (param.defaultValue) { defaultValueType = this._getTypeOfExpression(param.defaultValue, EvaluatorFlags.ConvertEllipsisToAny); + + this._defaultValueInitializerExpression = true; this.walk(param.defaultValue); + this._defaultValueInitializerExpression = false; } if (param.typeAnnotation) { @@ -589,6 +592,13 @@ export class TypeAnalyzer extends ParseTreeWalker { this._currentScope.setAlwaysRaises(); } + if (this._defaultValueInitializerExpression && !this._fileInfo.isStubFile) { + this._addDiagnostic( + this._fileInfo.diagnosticSettings.reportCallInDefaultInitializer, + `Function calls within default value initializer are not permitted`, + node); + } + return true; } diff --git a/server/src/common/configOptions.ts b/server/src/common/configOptions.ts index 0496b6d2e..36fa79af1 100644 --- a/server/src/common/configOptions.ts +++ b/server/src/common/configOptions.ts @@ -123,6 +123,10 @@ export interface DiagnosticSettings { // Report usage of unknown input or return parameters? reportUnknownMemberType: DiagnosticLevel; + + // Report usage of function call within default value + // initialization expression? + reportCallInDefaultInitializer: DiagnosticLevel; } export function cloneDiagnosticSettings( @@ -165,7 +169,8 @@ export function getDiagLevelSettings() { 'reportInvalidStringEscapeSequence', 'reportUnknownParameterType', 'reportUnknownVariableType', - 'reportUnknownMemberType' + 'reportUnknownMemberType', + 'reportCallInDefaultInitializer' ]; } @@ -197,7 +202,8 @@ export function getStrictDiagnosticSettings(): DiagnosticSettings { reportInvalidStringEscapeSequence: 'error', reportUnknownParameterType: 'error', reportUnknownVariableType: 'error', - reportUnknownMemberType: 'error' + reportUnknownMemberType: 'error', + reportCallInDefaultInitializer: 'none' }; return diagSettings; @@ -231,7 +237,8 @@ export function getDefaultDiagnosticSettings(): DiagnosticSettings { reportInvalidStringEscapeSequence: 'warning', reportUnknownParameterType: 'none', reportUnknownVariableType: 'none', - reportUnknownMemberType: 'none' + reportUnknownMemberType: 'none', + reportCallInDefaultInitializer: 'none' }; return diagSettings; @@ -544,7 +551,12 @@ export class ConfigOptions { // Read the "reportUnknownMemberType" entry. reportUnknownMemberType: this._convertDiagnosticLevel( configObj.reportUnknownMemberType, 'reportUnknownMemberType', - defaultSettings.reportUnknownMemberType) + defaultSettings.reportUnknownMemberType), + + // Read the "reportCallInDefaultInitializer" entry. + reportCallInDefaultInitializer: this._convertDiagnosticLevel( + configObj.reportCallInDefaultInitializer, 'reportCallInDefaultInitializer', + defaultSettings.reportCallInDefaultInitializer) }; // Read the "venvPath". diff --git a/server/src/tests/samples/defaultInitializer1.py b/server/src/tests/samples/defaultInitializer1.py new file mode 100644 index 000000000..43c8164cb --- /dev/null +++ b/server/src/tests/samples/defaultInitializer1.py @@ -0,0 +1,10 @@ +# This sample tests the type analyzer's reporting of issues +# with parameter default initializer expressions. + +def foo( + a = None, + # This should generate an error + b = dict(), + # This should generate an error + c = max(3, 4)): + return 3 diff --git a/server/src/tests/typeAnalyzer.test.ts b/server/src/tests/typeAnalyzer.test.ts index 53934c995..e20ae8b7b 100644 --- a/server/src/tests/typeAnalyzer.test.ts +++ b/server/src/tests/typeAnalyzer.test.ts @@ -462,3 +462,16 @@ test('AugmentedAssignment1', () => { validateResults(analysisResults, 3); }); + +test('DefaultInitializer1', () => { + const configOptions = new ConfigOptions('.'); + + // By default, optional diagnostics are ignored. + let analysisResults = TestUtils.typeAnalyzeSampleFiles(['defaultInitializer1.py'], configOptions); + validateResults(analysisResults, 0); + + // Turn on errors. + configOptions.diagnosticSettings.reportCallInDefaultInitializer = 'error'; + analysisResults = TestUtils.typeAnalyzeSampleFiles(['defaultInitializer1.py'], configOptions); + validateResults(analysisResults, 2); +});