From 5f48e396712cb2ce607468738f750205906e4242 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 8 Dec 2022 20:05:32 -0800 Subject: [PATCH] Exposed new configuration setting `analyzeUnannotatedFunctions` which corresponds to the `--skipunannotated` command-line option. Added an information diagnostic for skipped functions. This addresses https://github.com/microsoft/pyright/issues/4303. --- docs/configuration.md | 3 ++ .../pyright-internal/src/analyzer/checker.ts | 9 ++++ .../pyright-internal/src/analyzer/program.ts | 1 - .../pyright-internal/src/analyzer/service.ts | 6 ++- .../src/analyzer/typeEvaluator.ts | 48 +++++++++++++------ .../src/common/commandLineOptions.ts | 2 +- .../src/common/configOptions.ts | 11 +++-- .../src/common/diagnosticRules.ts | 1 + .../src/localization/localize.ts | 2 + .../src/localization/package.nls.en-us.json | 1 + packages/pyright-internal/src/pyright.ts | 4 +- .../schemas/pyrightconfig.schema.json | 6 +++ 12 files changed, 71 insertions(+), 23 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index a7a1c2b4e..72728d2cf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -50,6 +50,8 @@ The following settings control pyright’s diagnostic output (warnings or errors **strictSetInference** [boolean]: When inferring the type of a set, use strict type assumptions. For example, the expression `{1, 'a', 3.4}` could be inferred to be of type `set[Any]` or `set[int | str | float]`. If this setting is true, it will use the latter (stricter) type. The default value for this setting is 'false'. + **analyzeUnannotatedFunctions** [boolean]: Analyze and report errors for functions and methods that have no type annotations for input parameters or return types. The default value for this setting is 'true'. + **strictParameterNoneValue** [boolean]: PEP 484 indicates that when a function parameter is assigned a default value of None, its type should implicitly be Optional even if the explicit type is not. When enabled, this rule requires that parameter type annotations use Optional explicitly in this case. The default value for this setting is 'true'. **enableTypeIgnoreComments** [boolean]: PEP 484 defines support for "# type: ignore" comments. This switch enables or disables support for these comments. The default value for this setting is 'true'. This does not affect "# pyright: ignore" comments. @@ -293,6 +295,7 @@ The following table lists the default severity levels for each diagnostic rule w | strictListInference | false | false | true | | strictDictionaryInference | false | false | true | | strictSetInference | false | false | true | +| analyzeUnannotatedFunctions | true | true | true | | strictParameterNoneValue | true | true | true | | enableTypeIgnoreComments | true | true | true | | reportMissingModuleSource | "warning" | "warning" | "warning" | diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts index 41813b25d..c56b8fdb2 100644 --- a/packages/pyright-internal/src/analyzer/checker.ts +++ b/packages/pyright-internal/src/analyzer/checker.ts @@ -414,6 +414,15 @@ export class Checker extends ParseTreeWalker { this.walk(node.typeParameters); } + if (!this._fileInfo.diagnosticRuleSet.analyzeUnannotatedFunctions && !this._fileInfo.isStubFile) { + if (ParseTreeUtils.isUnannotatedFunction(node)) { + this._evaluator.addInformation( + Localizer.Diagnostic.unannotatedFunctionSkipped().format({ name: node.name.value }), + node.name + ); + } + } + const functionTypeResult = this._evaluator.getTypeOfFunction(node); const containingClassNode = ParseTreeUtils.getEnclosingClass(node, /* stopAtFunction */ true); diff --git a/packages/pyright-internal/src/analyzer/program.ts b/packages/pyright-internal/src/analyzer/program.ts index 361b51da8..9e8cdf35c 100644 --- a/packages/pyright-internal/src/analyzer/program.ts +++ b/packages/pyright-internal/src/analyzer/program.ts @@ -886,7 +886,6 @@ export class Program { printTypeFlags: Program._getPrintTypeFlags(this._configOptions), logCalls: this._configOptions.logTypeEvaluationTime, minimumLoggingThreshold: this._configOptions.typeEvaluationTimeThreshold, - analyzeUnannotatedFunctions: this._configOptions.analyzeUnannotatedFunctions, evaluateUnknownImportsAsAny: !!this._configOptions.evaluateUnknownImportsAsAny, verifyTypeCacheEvaluatorFlags: !!this._configOptions.internalTestMode, }, diff --git a/packages/pyright-internal/src/analyzer/service.ts b/packages/pyright-internal/src/analyzer/service.ts index 8f534c411..c6beb20bb 100644 --- a/packages/pyright-internal/src/analyzer/service.ts +++ b/packages/pyright-internal/src/analyzer/service.ts @@ -761,7 +761,11 @@ export class AnalyzerService { configOptions.applyDiagnosticOverrides(commandLineOptions.diagnosticSeverityOverrides); } - configOptions.analyzeUnannotatedFunctions = commandLineOptions.analyzeUnannotatedFunctions ?? true; + // Override the analyzeUnannotatedFunctions setting based on the command-line setting. + if (commandLineOptions.analyzeUnannotatedFunctions !== undefined) { + configOptions.diagnosticRuleSet.analyzeUnannotatedFunctions = + commandLineOptions.analyzeUnannotatedFunctions; + } const reportDuplicateSetting = (settingName: string, configValue: number | string | boolean) => { const settingSource = commandLineOptions.fromVsCodeExtension diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index 616d66c5b..519918739 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -567,7 +567,6 @@ export interface EvaluatorOptions { printTypeFlags: TypePrinter.PrintTypeFlags; logCalls: boolean; minimumLoggingThreshold: number; - analyzeUnannotatedFunctions: boolean; evaluateUnknownImportsAsAny: boolean; verifyTypeCacheEvaluatorFlags: boolean; } @@ -2935,6 +2934,21 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions return undefined; } + // Should we suppress this diagnostic because it's within an unannotated function? + const fileInfo = AnalyzerNodeInfo.getFileInfo(node); + if (!fileInfo.diagnosticRuleSet.analyzeUnannotatedFunctions) { + const containingFunction = ParseTreeUtils.getEnclosingFunction(node); + + // Is the target node within the body of the function? If so, suppress the diagnostic. + if ( + containingFunction && + ParseTreeUtils.isUnannotatedFunction(containingFunction) && + ParseTreeUtils.isNodeContainedWithin(node, containingFunction.suite) + ) { + return undefined; + } + } + const diagnostic = addDiagnosticWithSuppressionCheck(diagLevel, message, node, range); if (diagnostic) { diagnostic.setRule(rule); @@ -4059,16 +4073,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions let isIncomplete = false; const allowForwardReferences = (flags & EvaluatorFlags.AllowForwardReferences) !== 0 || fileInfo.isStubFile; - if (!evaluatorOptions.analyzeUnannotatedFunctions) { - const containingFunction = ParseTreeUtils.getEnclosingFunction(node); - if (containingFunction && ParseTreeUtils.isUnannotatedFunction(containingFunction)) { - return { - type: AnyType.create(), - isIncomplete: false, - }; - } - } - // Does this name refer to a PEP 695-style type parameter? const typeParamSymbol = AnalyzerNodeInfo.getTypeParameterSymbol(node); if (typeParamSymbol) { @@ -4109,6 +4113,19 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } symbol = symbolWithScope.symbol; + setSymbolAccessed(fileInfo, symbol, node); + + // If we're not supposed to be analyzing this function, skip the remaining work + // to determine the name's type. Simply evaluate its type as Any. + if (!fileInfo.diagnosticRuleSet.analyzeUnannotatedFunctions) { + const containingFunction = ParseTreeUtils.getEnclosingFunction(node); + if (containingFunction && ParseTreeUtils.isUnannotatedFunction(containingFunction)) { + return { + type: AnyType.create(), + isIncomplete: false, + }; + } + } // Get the effective type (either the declared type or the inferred type). // If we're using code flow analysis, pass the usage node so we consider @@ -4196,8 +4213,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions // Detect, report, and fill in missing type arguments if appropriate. type = reportMissingTypeArguments(node, type, flags); - setSymbolAccessed(fileInfo, symbol, node); - if ((flags & EvaluatorFlags.ExpectingTypeAnnotation) !== 0) { // Verify that the name does not refer to a (non type alias) variable. if (effectiveTypeInfo.includesVariableDecl && !type.typeAliasInfo) { @@ -20260,6 +20275,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions function getFunctionInferredReturnType(type: FunctionType, args?: ValidateArgTypeParams[]) { let returnType: Type | undefined; let isIncomplete = false; + let analyzeUnannotatedFunctions = true; // Don't attempt to infer the return type for a stub file. if (FunctionType.isStubDefinition(type)) { @@ -20277,9 +20293,11 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions returnType = NoneType.createInstance(); } else if (type.details.declaration) { const functionNode = type.details.declaration.node; + analyzeUnannotatedFunctions = + AnalyzerNodeInfo.getFileInfo(functionNode).diagnosticRuleSet.analyzeUnannotatedFunctions; // Skip return type inference if we are in "skip unannotated function" mode. - if (evaluatorOptions.analyzeUnannotatedFunctions && !checkCodeFlowTooComplex(functionNode.suite)) { + if (analyzeUnannotatedFunctions && !checkCodeFlowTooComplex(functionNode.suite)) { const codeFlowComplexity = AnalyzerNodeInfo.getCodeFlowComplexity(functionNode); // For very complex functions that have no annotated parameter types, @@ -20332,7 +20350,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions // attempt to do a better job at inference. if ( !isIncomplete && - evaluatorOptions.analyzeUnannotatedFunctions && + analyzeUnannotatedFunctions && isPartlyUnknown(returnType) && FunctionType.hasUnannotatedParams(type) && !FunctionType.isStubDefinition(type) && diff --git a/packages/pyright-internal/src/common/commandLineOptions.ts b/packages/pyright-internal/src/common/commandLineOptions.ts index e81ecd91f..14bf953ab 100644 --- a/packages/pyright-internal/src/common/commandLineOptions.ts +++ b/packages/pyright-internal/src/common/commandLineOptions.ts @@ -137,5 +137,5 @@ export class CommandLineOptions { enableAmbientAnalysis = true; // Analyze functions and methods that have no type annotations? - analyzeUnannotatedFunctions = true; + analyzeUnannotatedFunctions?: boolean; } diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index bc5223ffe..eec6bd8fb 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -92,6 +92,9 @@ export interface DiagnosticRuleSet { // Use strict inference rules for dictionary expressions? strictDictionaryInference: boolean; + // Analyze functions and methods that have no annotations? + analyzeUnannotatedFunctions: boolean; + // Use strict type rules for parameters assigned default of None? strictParameterNoneValue: boolean; @@ -312,6 +315,7 @@ export function getBooleanDiagnosticRules(includeNonOverridable = false) { DiagnosticRule.strictListInference, DiagnosticRule.strictSetInference, DiagnosticRule.strictDictionaryInference, + DiagnosticRule.analyzeUnannotatedFunctions, DiagnosticRule.strictParameterNoneValue, ]; @@ -410,6 +414,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { strictListInference: false, strictSetInference: false, strictDictionaryInference: false, + analyzeUnannotatedFunctions: true, strictParameterNoneValue: true, enableTypeIgnoreComments: true, reportGeneralTypeIssues: 'none', @@ -489,6 +494,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { strictListInference: false, strictSetInference: false, strictDictionaryInference: false, + analyzeUnannotatedFunctions: true, strictParameterNoneValue: true, enableTypeIgnoreComments: true, reportGeneralTypeIssues: 'error', @@ -568,6 +574,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { strictListInference: true, strictSetInference: true, strictDictionaryInference: true, + analyzeUnannotatedFunctions: true, strictParameterNoneValue: true, enableTypeIgnoreComments: true, // Not overridden by strict mode reportGeneralTypeIssues: 'error', @@ -716,10 +723,6 @@ export class ConfigOptions { // Was this config initialized from JSON (pyrightconfig/pyproject)? initializedFromJson = false; - // Should we skip analysis of all functions and methods that have - // no parameter ore return type annotations? - analyzeUnannotatedFunctions = true; - //--------------------------------------------------------------- // Diagnostics Rule Set diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index 77522a4ef..5dd04ac01 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -14,6 +14,7 @@ export enum DiagnosticRule { strictListInference = 'strictListInference', strictSetInference = 'strictSetInference', strictDictionaryInference = 'strictDictionaryInference', + analyzeUnannotatedFunctions = 'analyzeUnannotatedFunctions', strictParameterNoneValue = 'strictParameterNoneValue', enableTypeIgnoreComments = 'enableTypeIgnoreComments', diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts index ac88a8140..14b88ff16 100644 --- a/packages/pyright-internal/src/localization/localize.ts +++ b/packages/pyright-internal/src/localization/localize.ts @@ -930,6 +930,8 @@ export namespace Localizer { new ParameterizedString<{ name: string }>(getRawString('Diagnostic.unaccessedSymbol')); export const unaccessedVariable = () => new ParameterizedString<{ name: string }>(getRawString('Diagnostic.unaccessedVariable')); + export const unannotatedFunctionSkipped = () => + new ParameterizedString<{ name: string }>(getRawString('Diagnostic.unannotatedFunctionSkipped')); export const unexpectedAsyncToken = () => getRawString('Diagnostic.unexpectedAsyncToken'); export const unexpectedExprToken = () => getRawString('Diagnostic.unexpectedExprToken'); export const unexpectedIndent = () => getRawString('Diagnostic.unexpectedIndent'); 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 9b14a1e6c..1b7c83f99 100644 --- a/packages/pyright-internal/src/localization/package.nls.en-us.json +++ b/packages/pyright-internal/src/localization/package.nls.en-us.json @@ -470,6 +470,7 @@ "unaccessedImport": "Import \"{name}\" is not accessed", "unaccessedSymbol": "\"{name}\" is not accessed", "unaccessedVariable": "Variable \"{name}\" is not accessed", + "unannotatedFunctionSkipped": "Analysis of function \"{name}\" is skipped because it unannotated", "unexpectedAsyncToken": "Expected \"def\", \"with\" or \"for\" to follow \"async\"", "unexpectedExprToken": "Unexpected token at end of expression", "unexpectedIndent": "Unexpected indentation", diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 071c15174..e3e4958ab 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -255,7 +255,9 @@ async function processArgs(): Promise { options.typeStubTargetImportName = args.createstub; } - options.analyzeUnannotatedFunctions = !args.skipunannotated; + if (args.skipunannotated) { + options.analyzeUnannotatedFunctions = false; + } if (args.verbose) { options.verboseOutput = true; diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index fbdba5f39..25bfd0693 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -130,6 +130,12 @@ "title": "Infer strict types for dictionary expressions", "default": false }, + "analyzeUnannotatedFunctions": { + "$id": "#/properties/analyzeUnannotatedFunctions", + "type": "boolean", + "title": "Analyze and report diagnostics for functions that have no annotations", + "default": true + }, "strictParameterNoneValue": { "$id": "#/properties/strictParameterNoneValue", "type": "boolean",