From e71f64ac4aeb20e3da70e73837d624b6629219f1 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 23 May 2019 20:31:23 -0700 Subject: [PATCH] Added support for new config options: reportUnusedImport, reportUnusedClass, reportUnusedFunction, and reportUnusedVariable. --- client/schemas/pyrightconfig.schema.json | 24 ++++++++++ docs/configuration.md | 8 ++++ server/src/analyzer/typeAnalyzer.ts | 59 +++++++++++++++++++----- server/src/common/configOptions.ts | 44 ++++++++++++++++++ 4 files changed, 123 insertions(+), 12 deletions(-) diff --git a/client/schemas/pyrightconfig.schema.json b/client/schemas/pyrightconfig.schema.json index 372beb81e..5f2df1720 100644 --- a/client/schemas/pyrightconfig.schema.json +++ b/client/schemas/pyrightconfig.schema.json @@ -97,6 +97,30 @@ "title": "Controls reporting of module imports that create cycles in import graph", "default": "none" }, + "reportUnusedImport": { + "$id": "#/properties/reportUnusedImport", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of imported symbols that are not referenced within the source file", + "default": "none" + }, + "reportUnusedClass": { + "$id": "#/properties/reportUnusedClass", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of private classes that are not accessed", + "default": "none" + }, + "reportUnusedFunction": { + "$id": "#/properties/reportUnusedFunction", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of private functions or methods that are not accessed", + "default": "none" + }, + "reportUnusedVariable": { + "$id": "#/properties/reportUnusedVariable", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of local variables that are not accessed", + "default": "none" + }, "reportOptionalSubscript": { "$id": "#/properties/reportOptionalSubscript", "$ref": "#/definitions/diagnostic", diff --git a/docs/configuration.md b/docs/configuration.md index ac739f136..6f83a152a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -40,6 +40,14 @@ The following settings control pyright's diagnostic output (warnings or errors). **reportImportCycles** [boolean or string, optional]: Generate or suppress diagnostics for cyclical import chains. These are not errors in Python, but they do slow down type analysis and often hint at architectural layering issues. Generally, they should be avoided. The default value for this setting is 'none'. Note that there are import cycles in the typeshed stdlib typestub files that are ignored by this setting. +**reportUnusedImport** [boolean or string, optional]: Generate or suppress diagnostics for an imported symbol that is not referenced within that file. The default value for this setting is 'none'. + +**reportUnusedClass** [boolean or string, optional]: Generate or suppress diagnostics for a class with a private name (starting with an underscore) that is not accessed. The default value for this setting is 'none'. + +**reportUnusedFunction** [boolean or string, optional]: Generate or suppress diagnostics for a function or method with a private name (starting with an underscore) that is not accessed. The default value for this setting is 'none'. + +**reportUnusedVariable** [boolean or string, optional]: Generate or suppress diagnostics for a variable that is not accessed. The default value for this setting is 'none'. + **reportOptionalSubscript** [boolean or string, optional]: Generate or suppress diagnostics for an attempt to subscript (index) a variable with an Optional type. The default value for this setting is 'none'. **reportOptionalMemberAccess** [boolean or string, optional]: Generate or suppress diagnostics for an attempt to access a member of a variable with an Optional type. The default value for this setting is 'none'. diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index e0229480d..68761812c 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -250,7 +250,9 @@ export class TypeAnalyzer extends ParseTreeWalker { this.walkMultiple(node.decorators); this.walkMultiple(node.arguments); - this._conditionallyReportUnusedName(node.name, true); + this._conditionallyReportUnusedName(node.name, true, + this._fileInfo.diagnosticSettings.reportUnusedClass, + `Class '${ node.name.nameToken.value }' is not accessed`); return false; } @@ -519,7 +521,9 @@ export class TypeAnalyzer extends ParseTreeWalker { this.walkMultiple(node.decorators); - this._conditionallyReportUnusedName(node.name, true); + this._conditionallyReportUnusedName(node.name, true, + this._fileInfo.diagnosticSettings.reportUnusedFunction, + `Function '${ node.name.nameToken.value }' is not accessed`); return false; } @@ -1063,21 +1067,31 @@ export class TypeAnalyzer extends ParseTreeWalker { visitName(node: NameNode) { const nameValue = node.nameToken.value; const symbolInScope = this._currentScope.lookUpSymbolRecursive(nameValue); + let declarations: Declaration[] | undefined; // If there's no declaration assigned to this name node, assign one // for the hover provider. - if (!AnalyzerNodeInfo.getDeclarations(node)) { + declarations = AnalyzerNodeInfo.getDeclarations(node); + if (!declarations) { if (symbolInScope && symbolInScope.symbol.hasDeclarations()) { - AnalyzerNodeInfo.setDeclarations(node, - TypeUtils.getPrimaryDeclarationsForSymbol(symbolInScope.symbol)!); + declarations = TypeUtils.getPrimaryDeclarationsForSymbol(symbolInScope.symbol)!; + AnalyzerNodeInfo.setDeclarations(node, declarations); } } // Determine if we should log information about private usage. this._conditionallyReportPrivateUsage(node); - // Determine if we should log information about an unused name. - this._conditionallyReportUnusedName(node); + let unaccessedDiagLevel: DiagnosticLevel = 'none'; + if (symbolInScope && declarations) { + // Determine if we should log information about an unused name. + if (declarations[0].category === SymbolCategory.Variable) { + unaccessedDiagLevel = this._fileInfo.diagnosticSettings.reportUnusedVariable; + } + } + + this._conditionallyReportUnusedName(node, false, unaccessedDiagLevel, + `Variable '${ node.nameToken.value }' is not accessed`); return true; } @@ -1168,7 +1182,9 @@ export class TypeAnalyzer extends ParseTreeWalker { this._assignTypeToNameNode(node.alias, moduleType, moduleDeclaration); this._updateExpressionTypeForNode(node.alias, moduleType); - this._conditionallyReportUnusedName(node.alias); + this._conditionallyReportUnusedName(node.alias, false, + this._fileInfo.diagnosticSettings.reportUnusedImport, + `Import '${ node.alias.nameToken.value }' is not accessed`); } else { this._bindMultiPartModuleNameToType(node.module.nameParts, moduleType, moduleDeclaration); @@ -1188,7 +1204,9 @@ export class TypeAnalyzer extends ParseTreeWalker { this._assignTypeToNameNode(aliasNode, symbolType); this._updateExpressionTypeForNode(aliasNode, symbolType); - this._conditionallyReportUnusedName(aliasNode); + this._conditionallyReportUnusedName(aliasNode, false, + this._fileInfo.diagnosticSettings.reportUnusedImport, + `Import '${ aliasNode.nameToken.value }' is not accessed`); } } } @@ -1285,7 +1303,9 @@ export class TypeAnalyzer extends ParseTreeWalker { AnalyzerNodeInfo.setDeclarations(importAs.name, [declaration]); } - this._conditionallyReportUnusedName(aliasNode); + this._conditionallyReportUnusedName(aliasNode, false, + this._fileInfo.diagnosticSettings.reportUnusedImport, + `Import '${ aliasNode.nameToken.value }' is not accessed`); }); } } else { @@ -1302,7 +1322,9 @@ export class TypeAnalyzer extends ParseTreeWalker { } this._assignTypeToNameNode(aliasNode, symbolType); - this._conditionallyReportUnusedName(aliasNode); + this._conditionallyReportUnusedName(aliasNode, false, + this._fileInfo.diagnosticSettings.reportUnusedImport, + `Import '${ aliasNode.nameToken.value }' is not accessed`); }); } } @@ -1626,9 +1648,17 @@ export class TypeAnalyzer extends ParseTreeWalker { symbol.addDeclaration(declaration); } - private _conditionallyReportUnusedName(node: NameNode, reportPrivateOnly = false) { + private _conditionallyReportUnusedName(node: NameNode, reportPrivateOnly: boolean, + diagLevel: DiagnosticLevel, message: string) { + const nameValue = node.nameToken.value; + // A name of "_" means "I know this symbol isn't used", so + // don't report it as unused. + if (nameValue === '_') { + return; + } + if (reportPrivateOnly && !SymbolUtils.isPrivateName(nameValue)) { return; } @@ -1644,6 +1674,8 @@ export class TypeAnalyzer extends ParseTreeWalker { const symbolInScope = this._currentScope.lookUpSymbolRecursive(nameValue); if (symbolInScope && !symbolInScope.symbol.isAccessed()) { this._addUnusedName(node); + + this._addDiagnostic(diagLevel, message, node); } } @@ -2712,6 +2744,9 @@ export class TypeAnalyzer extends ParseTreeWalker { } this._fileInfo.diagnosticSink.addUnusedCodeWithTextRange( `'${ multipartName }' is not accessed`, textRange); + + this._addDiagnostic(this._fileInfo.diagnosticSettings.reportUnusedImport, + `Import '${ multipartName }' is not accessed`, textRange); } } diff --git a/server/src/common/configOptions.ts b/server/src/common/configOptions.ts index 1ba7f66ef..299088318 100644 --- a/server/src/common/configOptions.ts +++ b/server/src/common/configOptions.ts @@ -59,6 +59,18 @@ export interface DiagnosticSettings { // Report cycles in import graph? reportImportCycles: DiagnosticLevel; + // Report imported symbol that is not accessed? + reportUnusedImport: DiagnosticLevel; + + // Report private class that is not accessed? + reportUnusedClass: DiagnosticLevel; + + // Report private function or method that is not accessed? + reportUnusedFunction: DiagnosticLevel; + + // Report variable that is not accessed? + reportUnusedVariable: DiagnosticLevel; + // Report attempts to subscript (index) an Optional type? reportOptionalSubscript: DiagnosticLevel; @@ -133,6 +145,10 @@ export function getDiagLevelSettings() { 'reportMissingImports', 'reportMissingTypeStubs', 'reportImportCycles', + 'reportUnusedImport', + 'reportUnusedClass', + 'reportUnusedFunction', + 'reportUnusedVariable', 'reportOptionalSubscript', 'reportOptionalMemberAccess', 'reportOptionalCall', @@ -161,6 +177,10 @@ export function getStrictDiagnosticSettings(): DiagnosticSettings { reportMissingImports: 'error', reportMissingTypeStubs: 'error', reportImportCycles: 'error', + reportUnusedImport: 'error', + reportUnusedClass: 'error', + reportUnusedFunction: 'error', + reportUnusedVariable: 'error', reportOptionalSubscript: 'error', reportOptionalMemberAccess: 'error', reportOptionalCall: 'error', @@ -191,6 +211,10 @@ export function getDefaultDiagnosticSettings(): DiagnosticSettings { reportMissingImports: 'error', reportMissingTypeStubs: 'none', reportImportCycles: 'none', + reportUnusedImport: 'none', + reportUnusedClass: 'none', + reportUnusedFunction: 'none', + reportUnusedVariable: 'none', reportOptionalSubscript: 'none', reportOptionalMemberAccess: 'none', reportOptionalCall: 'none', @@ -385,6 +409,26 @@ export class ConfigOptions { configObj.reportMissingImports, 'reportMissingImports', defaultSettings.reportMissingImports), + // Read the "reportUnusedImport" entry. + reportUnusedImport: this._convertDiagnosticLevel( + configObj.reportUnusedImport, 'reportUnusedImport', + defaultSettings.reportUnusedImport), + + // Read the "reportUnusedClass" entry. + reportUnusedClass: this._convertDiagnosticLevel( + configObj.reportUnusedClass, 'reportUnusedClass', + defaultSettings.reportUnusedClass), + + // Read the "reportUnusedFunction" entry. + reportUnusedFunction: this._convertDiagnosticLevel( + configObj.reportUnusedFunction, 'reportUnusedFunction', + defaultSettings.reportUnusedFunction), + + // Read the "reportUnusedVariable" entry. + reportUnusedVariable: this._convertDiagnosticLevel( + configObj.reportUnusedVariable, 'reportUnusedVariable', + defaultSettings.reportUnusedVariable), + // Read the "reportMissingTypeStubs" entry. reportMissingTypeStubs: this._convertDiagnosticLevel( configObj.reportMissingTypeStubs, 'reportMissingTypeStubs',