From a44e254c3297b7a4af136058d470f5f35cd723be Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 25 Nov 2020 16:07:59 -0800 Subject: [PATCH] Added support for new reportUnsupportedDunderAll diagnostic rule. It checks for unsupported manipulations of `__all__`. --- docs/configuration.md | 2 + .../pyright-internal/src/analyzer/binder.ts | 46 ++++++++++++++++++- .../src/common/configOptions.ts | 15 ++++++ .../src/common/diagnosticRules.ts | 1 + .../src/localization/localize.ts | 2 + .../src/localization/package.nls.en-us.json | 2 + .../src/tests/samples/dunderAll1.py | 33 +++++++++++++ .../src/tests/typeEvaluator2.test.ts | 18 ++++++++ packages/vscode-pyright/package.json | 11 +++++ .../schemas/pyrightconfig.schema.json | 6 +++ 10 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 packages/pyright-internal/src/tests/samples/dunderAll1.py diff --git a/docs/configuration.md b/docs/configuration.md index c45ed9901..240acaa47 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -132,6 +132,8 @@ The following settings control pyright’s diagnostic output (warnings or errors **reportInvalidStubStatement** [boolean or string, optional]: Generate or suppress diagnostics for statements that are syntactically correct but have no purpose within a type stub file. The default value for this setting is 'none'. +**reportUnsupportedDunderAll** [boolean or string, optional]: Generate or suppress diagnostics for statements that define or manipulate `__all__` in a way that is not allowed by a static type checker, thus rendering the contents of `__all__` to be unknown or incorrect. 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. diff --git a/packages/pyright-internal/src/analyzer/binder.ts b/packages/pyright-internal/src/analyzer/binder.ts index 60e0eb527..616bfe2b9 100644 --- a/packages/pyright-internal/src/analyzer/binder.ts +++ b/packages/pyright-internal/src/analyzer/binder.ts @@ -567,6 +567,8 @@ export class Binder extends ParseTreeWalker { node.leftExpression.leftExpression.nodeType === ParseNodeType.Name && node.leftExpression.leftExpression.value === '__all__' ) { + let emitDunderAllWarning = true; + // Is this a call to "__all__.extend()"? if (node.leftExpression.memberName.value === 'extend' && node.arguments.length === 1) { const argExpr = node.arguments[0].valueExpression; @@ -580,6 +582,7 @@ export class Binder extends ParseTreeWalker { listEntryNode.strings[0].nodeType === ParseNodeType.String ) { this._dunderAllNames?.push(listEntryNode.strings[0].value); + emitDunderAllWarning = false; } }); } else if ( @@ -589,10 +592,11 @@ export class Binder extends ParseTreeWalker { ) { // Is this a call to "__all__.extend(.__all__)"? const namesToAdd = this._getDunderAllNamesFromImport(argExpr.leftExpression.value); - if (namesToAdd) { + if (namesToAdd && namesToAdd.length > 0) { namesToAdd.forEach((name) => { this._dunderAllNames?.push(name); }); + emitDunderAllWarning = false; } } } else if (node.leftExpression.memberName.value === 'remove' && node.arguments.length === 1) { @@ -605,6 +609,7 @@ export class Binder extends ParseTreeWalker { this._dunderAllNames ) { this._dunderAllNames = this._dunderAllNames.filter((name) => name !== argExpr.strings[0].value); + emitDunderAllWarning = false; } } else if (node.leftExpression.memberName.value === 'append' && node.arguments.length === 1) { // Is this a call to "__all__.append()"? @@ -615,8 +620,18 @@ export class Binder extends ParseTreeWalker { argExpr.strings[0].nodeType === ParseNodeType.String ) { this._dunderAllNames?.push(argExpr.strings[0].value); + emitDunderAllWarning = false; } } + + if (emitDunderAllWarning) { + this._addDiagnostic( + this._fileInfo.diagnosticRuleSet.reportUnsupportedDunderAll, + DiagnosticRule.reportUnsupportedDunderAll, + Localizer.Diagnostic.unsupportedDunderAllOperation(), + node + ); + } } return false; @@ -661,6 +676,7 @@ export class Binder extends ParseTreeWalker { ) { const expr = node.rightExpression; this._dunderAllNames = []; + let emitDunderAllWarning = false; if (expr.nodeType === ParseNodeType.List) { expr.entries.forEach((listEntryNode) => { @@ -670,6 +686,8 @@ export class Binder extends ParseTreeWalker { listEntryNode.strings[0].nodeType === ParseNodeType.String ) { this._dunderAllNames!.push(listEntryNode.strings[0].value); + } else { + emitDunderAllWarning = true; } }); } else if (expr.nodeType === ParseNodeType.Tuple) { @@ -680,8 +698,21 @@ export class Binder extends ParseTreeWalker { tupleEntryNode.strings[0].nodeType === ParseNodeType.String ) { this._dunderAllNames!.push(tupleEntryNode.strings[0].value); + } else { + emitDunderAllWarning = true; } }); + } else { + emitDunderAllWarning = true; + } + + if (emitDunderAllWarning) { + this._addDiagnostic( + this._fileInfo.diagnosticRuleSet.reportUnsupportedDunderAll, + DiagnosticRule.reportUnsupportedDunderAll, + Localizer.Diagnostic.unsupportedDunderAllOperation(), + node + ); } } } @@ -751,6 +782,7 @@ export class Binder extends ParseTreeWalker { node.leftExpression.value === '__all__' ) { const expr = node.rightExpression; + let emitDunderAllWarning = true; if (expr.nodeType === ParseNodeType.List) { // Is this the form __all__ += ["a", "b"]? @@ -763,6 +795,7 @@ export class Binder extends ParseTreeWalker { this._dunderAllNames?.push(listEntryNode.strings[0].value); } }); + emitDunderAllWarning = false; } else if ( expr.nodeType === ParseNodeType.MemberAccess && expr.leftExpression.nodeType === ParseNodeType.Name && @@ -774,8 +807,19 @@ export class Binder extends ParseTreeWalker { namesToAdd.forEach((name) => { this._dunderAllNames?.push(name); }); + + emitDunderAllWarning = false; } } + + if (emitDunderAllWarning) { + this._addDiagnostic( + this._fileInfo.diagnosticRuleSet.reportUnsupportedDunderAll, + DiagnosticRule.reportUnsupportedDunderAll, + Localizer.Diagnostic.unsupportedDunderAllOperation(), + node + ); + } } return false; diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index 2a5717fad..a03ed455e 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -225,6 +225,10 @@ export interface DiagnosticRuleSet { // Report statements that are syntactically correct but // have no semantic meaning within a type stub file. reportInvalidStubStatement: DiagnosticLevel; + + // Report operations on __all__ symbol that are not supported + // by a static type checker. + reportUnsupportedDunderAll: DiagnosticLevel; } export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet { @@ -290,6 +294,7 @@ export function getDiagLevelDiagnosticRules() { DiagnosticRule.reportUndefinedVariable, DiagnosticRule.reportUnboundVariable, DiagnosticRule.reportInvalidStubStatement, + DiagnosticRule.reportUnsupportedDunderAll, ]; } @@ -352,6 +357,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { reportUnboundVariable: 'warning', reportUndefinedVariable: 'warning', reportInvalidStubStatement: 'none', + reportUnsupportedDunderAll: 'none', }; return diagSettings; @@ -410,6 +416,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { reportUnboundVariable: 'error', reportUndefinedVariable: 'error', reportInvalidStubStatement: 'none', + reportUnsupportedDunderAll: 'warning', }; return diagSettings; @@ -468,6 +475,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { reportUnboundVariable: 'error', reportUndefinedVariable: 'error', reportInvalidStubStatement: 'error', + reportUnsupportedDunderAll: 'error', }; return diagSettings; @@ -1096,6 +1104,13 @@ export class ConfigOptions { DiagnosticRule.reportInvalidStubStatement, defaultSettings.reportInvalidStubStatement ), + + // Read the "reportUnsupportedDunderAll" entry. + reportUnsupportedDunderAll: this._convertDiagnosticLevel( + configObj.reportUnsupportedDunderAll, + DiagnosticRule.reportUnsupportedDunderAll, + defaultSettings.reportUnsupportedDunderAll + ), }; // Read the "venvPath". diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index 5beb0beec..39a328d82 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -59,4 +59,5 @@ export enum DiagnosticRule { reportUndefinedVariable = 'reportUndefinedVariable', reportUnboundVariable = 'reportUnboundVariable', reportInvalidStubStatement = 'reportInvalidStubStatement', + reportUnsupportedDunderAll = 'reportUnsupportedDunderAll', } diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts index 40cfe87dd..2f6d116f6 100644 --- a/packages/pyright-internal/src/localization/localize.ts +++ b/packages/pyright-internal/src/localization/localize.ts @@ -655,6 +655,8 @@ export namespace Localizer { export const unpackInSet = () => getRawString('Diagnostic.unpackInSet'); export const unpackTuplesIllegal = () => getRawString('Diagnostic.unpackTuplesIllegal'); export const unreachableCode = () => getRawString('Diagnostic.unreachableCode'); + export const unsupportedDunderAllAssignment = () => getRawString('Diagnostic.unsupportedDunderAllAssignment'); + export const unsupportedDunderAllOperation = () => getRawString('Diagnostic.unsupportedDunderAllOperation'); export const varAnnotationIllegal = () => getRawString('Diagnostic.varAnnotationIllegal'); export const walrusIllegal = () => getRawString('Diagnostic.walrusIllegal'); export const walrusNotAllowed = () => getRawString('Diagnostic.walrusNotAllowed'); 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 796318068..3235f2962 100644 --- a/packages/pyright-internal/src/localization/package.nls.en-us.json +++ b/packages/pyright-internal/src/localization/package.nls.en-us.json @@ -326,6 +326,8 @@ "unpackInSet": "Unpack operator not allowed within a set", "unpackTuplesIllegal": "Unpack operation not allowed in tuples prior to Python 3.8", "unreachableCode": "Code is unreachable", + "unsupportedDunderAllAssignment": "Expression assigned to \"__all__\" is not supported, so exported symbol list may be incorrect; use list or tuple of string literal values in assignment", + "unsupportedDunderAllOperation": "Operation on \"__all__\" is not supported, so exported symbol list may not be incorrect", "varAnnotationIllegal": "Type annotations for variables requires Python 3.6 or newer; use type comment for compatibility with previous versions", "walrusIllegal": "Operator \":=\" requires Python 3.8 or newer", "walrusNotAllowed": "Operator \":=\" not allowed in this context", diff --git a/packages/pyright-internal/src/tests/samples/dunderAll1.py b/packages/pyright-internal/src/tests/samples/dunderAll1.py new file mode 100644 index 000000000..79294ffb9 --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/dunderAll1.py @@ -0,0 +1,33 @@ +# This sample tests the reportUnsupportedDunderAll diagnostic rule. + +# pyright: reportMissingModuleSource=false + +from typing import Any +import mock + +__all__: Any + +__all__ = ("test", "hello") +__all__ = ["test", "hello"] +__all__.append("foo") +__all__.extend(["foo"]) +__all__.remove("foo") +__all__ += ["bar"] +__all__ += mock.__all__ +__all__.extend(mock.__all__) + + +my_string = "foo" + +# The following should all generate diagnostics if reportUnsupportedDunderAll +# is enabled. +__all__ = ("test", my_string) +__all__ = ["test", my_string] +__all__ = "test" +__all__.append(my_string) +__all__.extend([my_string]) +__all__.remove(my_string) +__all__ += [my_string] +__all__ += mock.AsyncMock +__all__.extend(mock.AsyncMock) +__all__.something() diff --git a/packages/pyright-internal/src/tests/typeEvaluator2.test.ts b/packages/pyright-internal/src/tests/typeEvaluator2.test.ts index 24103df2e..86988b3a4 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator2.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator2.test.ts @@ -722,6 +722,24 @@ test('Import12', () => { TestUtils.validateResults(analysisResults, 0, 0); }); +test('DunderAll1', () => { + const configOptions = new ConfigOptions('.'); + + // By default, reportUnsupportedDunderAll is a warning. + let analysisResults = TestUtils.typeAnalyzeSampleFiles(['dunderAll1.py'], configOptions); + TestUtils.validateResults(analysisResults, 0, 9); + + // Turn on error. + configOptions.diagnosticRuleSet.reportUnsupportedDunderAll = 'error'; + analysisResults = TestUtils.typeAnalyzeSampleFiles(['dunderAll1.py'], configOptions); + TestUtils.validateResults(analysisResults, 9, 0); + + // Turn off diagnostic. + configOptions.diagnosticRuleSet.reportUnsupportedDunderAll = 'none'; + analysisResults = TestUtils.typeAnalyzeSampleFiles(['dunderAll1.py'], configOptions); + TestUtils.validateResults(analysisResults, 0, 0); +}); + test('Overload1', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['overload1.py']); TestUtils.validateResults(analysisResults, 2); diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index 08bf94521..40b375d6f 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -586,6 +586,17 @@ "warning", "error" ] + }, + "reportUnsupportedDunderAll": { + "type": "string", + "description": "Diagnostics for unsupported operations performed on __all__.", + "default": "warning", + "enum": [ + "none", + "information", + "warning", + "error" + ] } } }, diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index f46694d1b..8da300943 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -381,6 +381,12 @@ "title": "Controls reporting of type stub statements that do not conform to PEP 484", "default": "none" }, + "reportUnsupportedDunderAll": { + "$id": "#/properties/reportUnsupportedDunderAll", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of unsupported operations performed on __all__", + "default": "warning" + }, "pythonVersion": { "$id": "#/properties/pythonVersion", "type": "string",