diff --git a/README.md b/README.md index bc50a98ca..cecbbfe38 100644 --- a/README.md +++ b/README.md @@ -79,10 +79,7 @@ To update to the latest version: Pyright is a work in progress. The following functionality is not yet finished. If you would like to contribute to any of these areas, contact the maintainers of the repo. * Provide switch that treats instance variables and methods that begin with underscore as private -* Validate that overridden methods in subclass have same signature as base class methods * Add support for f-strings -* Add numeric codes to diagnostics and a configuration mechanism for disabling errors by code -* Synthesize TypeVar param and return types for lambdas where possible ## Contributing diff --git a/client/schemas/pyrightconfig.schema.json b/client/schemas/pyrightconfig.schema.json index 7f32bbaf7..65b9de218 100644 --- a/client/schemas/pyrightconfig.schema.json +++ b/client/schemas/pyrightconfig.schema.json @@ -121,6 +121,12 @@ "title": "Controls reporting of class decorators without type annotations, which obscure class types", "default": "none" }, + "reportPrivateUsage": { + "$id": "#/properties/reportPrivateUsage", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of private variables and functions used outside of the owning class or module", + "default": "none" + }, "pythonVersion": { "$id": "#/properties/pythonVersion", "type": "string", diff --git a/docs/configuration.md b/docs/configuration.md index 7d22db96d..a368a9c26 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -48,6 +48,8 @@ The following settings control pyright's diagnostic output (warnings or errors). **reportUntypedClassDecorator** [boolean or string, optional]: Generate or suppress diagnostics for class decorators that have no type annotations. These obscure the class type, defeating many type analysis features. +**reportPrivateUsage** [boolean or string, optional]: Generate or suppress diagnostics for uses of private variables or functions outside of the class or module that declares them. Private variables and functions, by convention, are named starting with a single underscoe (“_”) character. + ## 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/parseTreeUtils.ts b/server/src/analyzer/parseTreeUtils.ts index a38f64089..e065b0ec6 100644 --- a/server/src/analyzer/parseTreeUtils.ts +++ b/server/src/analyzer/parseTreeUtils.ts @@ -258,12 +258,30 @@ export class ParseTreeUtils { return undefined; } + static getEnclosingClassOrModule(node: ParseNode): ClassNode | ModuleNode | undefined { + let curNode = node.parent; + while (curNode) { + if (curNode instanceof ClassNode) { + return curNode; + } + + if (curNode instanceof ModuleNode) { + return curNode; + } + + curNode = curNode.parent; + } + + return undefined; + } + static getEnclosingFunction(node: ParseNode): FunctionNode | undefined { let curNode = node.parent; while (curNode) { if (curNode instanceof FunctionNode) { return curNode; } + if (curNode instanceof ClassNode) { return undefined; } @@ -273,4 +291,17 @@ export class ParseTreeUtils { return undefined; } + + static isNodeContainedWithin(node: ParseNode, potentialContainer: ParseNode): boolean { + let curNode = node.parent; + while (curNode) { + if (curNode === potentialContainer) { + return true; + } + + curNode = curNode.parent; + } + + return false; + } } diff --git a/server/src/analyzer/postParseWalker.ts b/server/src/analyzer/postParseWalker.ts index 109401f85..bf3c2acdf 100644 --- a/server/src/analyzer/postParseWalker.ts +++ b/server/src/analyzer/postParseWalker.ts @@ -116,7 +116,7 @@ export class PostParseWalker extends ParseTreeWalker { visitWith(node: WithNode): boolean { node.withItems.forEach(item => { - this.walk(item.expression); + this.walk(item); }); node.withItems.forEach(item => { diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index d9e3ab72c..eed8c9988 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -1019,7 +1019,8 @@ export class TypeAnalyzer extends ParseTreeWalker { } visitName(node: NameNode) { - let symbolInScope = this._currentScope.lookUpSymbolRecursive(node.nameToken.value); + const nameValue = node.nameToken.value; + const symbolInScope = this._currentScope.lookUpSymbolRecursive(nameValue); if (symbolInScope && symbolInScope.symbol.declarations) { // For now, always assume it's the first declaration @@ -1033,6 +1034,10 @@ export class TypeAnalyzer extends ParseTreeWalker { // node, allowing it to be accessed for hover and definition // information. this._getTypeOfExpression(node); + + // Determine if we should log information about private usage. + this._conditionallyReportPrivateUsage(node); + return true; } @@ -1229,6 +1234,44 @@ export class TypeAnalyzer extends ParseTreeWalker { return false; } + private _conditionallyReportPrivateUsage(node: NameNode) { + if (this._fileInfo.configOptions.reportPrivateUsage === 'none') { + return; + } + + const nameValue = node.nameToken.value; + + // Is it a private name? + if (!nameValue.startsWith('_') || nameValue.startsWith('__')) { + return; + } + + const declaration = AnalyzerNodeInfo.getDeclaration(node); + if (!declaration || node === declaration.node) { + return; + } + + let classOrModuleNode = ParseTreeUtils.getEnclosingClassOrModule(declaration.node); + + // If this is the name of a class, find the module or class that contains it rather + // than using constraining the use of the class name within the class itself. + if (declaration.node.parent && + declaration.node.parent === classOrModuleNode && + classOrModuleNode instanceof ClassNode) { + + classOrModuleNode = ParseTreeUtils.getEnclosingClassOrModule(classOrModuleNode); + } + + if (classOrModuleNode && !ParseTreeUtils.isNodeContainedWithin(node, classOrModuleNode)) { + const scopeName = classOrModuleNode instanceof ClassNode ? + 'class' : 'module'; + + this._addDiagnostic(this._fileInfo.configOptions.reportPrivateUsage, + `'${ nameValue }' is private and used outside of its owning ${ scopeName }`, + node); + } + } + private _createAwaitableFunction(functionType: FunctionType): FunctionType { const returnType = functionType.getEffectiveReturnType(); diff --git a/server/src/common/configOptions.ts b/server/src/common/configOptions.ts index 34c4efe3c..8b8e917d2 100644 --- a/server/src/common/configOptions.ts +++ b/server/src/common/configOptions.ts @@ -106,6 +106,10 @@ export class ConfigOptions { // Report untyped class decorators that obscure the class type? reportUntypedClassDecorator: DiagnosticLevel = 'none'; + // Report usage of private variables and functions outside of + // the owning class or module? + reportPrivateUsage: DiagnosticLevel = 'none'; + //--------------------------------------------------------------- // Parsing and Import Resolution Settings @@ -240,6 +244,10 @@ export class ConfigOptions { this.reportUntypedClassDecorator = this._convertDiagnosticLevel( configObj.reportUntypedClassDecorator, 'reportUntypedClassDecorator', 'none'); + // Read the "reportPrivateUsage" entry. + this.reportPrivateUsage = this._convertDiagnosticLevel( + configObj.reportPrivateUsage, 'reportPrivateUsage', 'none'); + // Read the "venvPath". this.venvPath = undefined; if (configObj.venvPath !== undefined) { diff --git a/server/src/tests/typeAnalyzer.test.ts b/server/src/tests/typeAnalyzer.test.ts index d96d4e221..f6a190310 100644 --- a/server/src/tests/typeAnalyzer.test.ts +++ b/server/src/tests/typeAnalyzer.test.ts @@ -211,6 +211,19 @@ test('Optional1', () => { validateResults(analysisResults, 4); }); +test('Private1', () => { + const configOptions = new ConfigOptions('.'); + + // By default, optional diagnostics are ignored. + let analysisResults = TestUtils.typeAnalyzeSampleFiles(['private1.py'], configOptions); + validateResults(analysisResults, 0); + + // Turn on errors. + configOptions.reportPrivateUsage = 'error'; + analysisResults = TestUtils.typeAnalyzeSampleFiles(['private1.py'], configOptions); + validateResults(analysisResults, 4); +}); + test('Tuples1', () => { let analysisResults = TestUtils.typeAnalyzeSampleFiles(['tuples1.py']);