From baa844938cb928abe21790503a41923134a211b6 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 18:16:54 -0700 Subject: [PATCH] Continued moving alias declaration creation from type analyzer to binder. --- server/src/analyzer/binder.ts | 27 ++-- server/src/analyzer/declaration.ts | 20 ++- server/src/analyzer/typeAnalyzer.ts | 220 ++++++++++------------------ server/src/analyzer/types.ts | 28 ++-- 4 files changed, 125 insertions(+), 170 deletions(-) diff --git a/server/src/analyzer/binder.ts b/server/src/analyzer/binder.ts index 557adbeed..b024f7947 100644 --- a/server/src/analyzer/binder.ts +++ b/server/src/analyzer/binder.ts @@ -19,7 +19,7 @@ import * as assert from 'assert'; import { DiagnosticLevel } from '../common/configOptions'; -import { CreateTypeStubFileAction, getEmptyPosition, getEmptyRange } from '../common/diagnostic'; +import { CreateTypeStubFileAction, getEmptyRange } from '../common/diagnostic'; import { DiagnosticRule } from '../common/diagnosticRules'; import { convertOffsetsToRange } from '../common/positionUtils'; import { PythonVersion } from '../common/pythonVersion'; @@ -594,15 +594,16 @@ export abstract class Binder extends ParseTreeWalker { visitImportAs(node: ImportAsNode): boolean { if (node.module.nameParts.length > 0) { - let symbolName: string | undefined; + const firstNamePartValue = node.module.nameParts[0].nameToken.value; + let symbolName: string | undefined; if (node.alias) { // The symbol name is defined by the alias. symbolName = node.alias.nameToken.value; } else { // There was no alias, so we need to use the first element of // the name parts as the symbol. - symbolName = node.module.nameParts[0].nameToken.value; + symbolName = firstNamePartValue; } if (symbolName) { @@ -612,21 +613,29 @@ export abstract class Binder extends ParseTreeWalker { assert(importInfo !== undefined); if (importInfo && importInfo.isImportFound && importInfo.resolvedPaths.length > 0 && symbol) { - const resolvedPath = importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1]; + // See if there's already a matching alias delaration for this import. + // if so, we'll update it rather than creating a new one. This is required + // to handle cases where multiple import statements target the same + // starting symbol such as "import a.b.c" and "import a.d". In this case, + // we'll build a single declaration that describes the combined actions + // of both import statements, thus reflecting the behavior of the + // python module loader. const existingDecl = symbol.getDeclarations().find( - decl => decl.type === DeclarationType.Alias && decl.path === resolvedPath); + decl => decl.type === DeclarationType.Alias && + decl.firstNamePart === firstNamePartValue); const newDecl: AliasDeclaration = existingDecl as AliasDeclaration || { type: DeclarationType.Alias, path: '', range: getEmptyRange(), + firstNamePart: firstNamePartValue, implicitImports: new Map() }; // Add the implicit imports for this module if it's the last // name part we're resolving. - if (node.alias || node.module.nameParts.length === 0) { - newDecl.path = importInfo.resolvedPaths[0]; + if (node.alias || node.module.nameParts.length === 1) { + newDecl.path = importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1]; this._addImplicitImportsToLoaderActions(importInfo, newDecl); } else { // Fill in the remaining name parts. @@ -1047,8 +1056,8 @@ export class ModuleScopeBinder extends Binder { this.walkMultiple(moduleNode.statements); // Associate the module's scope with the module type. - const moduleType = ModuleType.create(this._currentScope.getSymbolTable(), - this._getDocString((this._scopedNode as ModuleNode).statements)); + const moduleType = ModuleType.create(this._currentScope.getSymbolTable()); + moduleType.docString = this._getDocString((this._scopedNode as ModuleNode).statements); AnalyzerNodeInfo.setExpressionType(this._scopedNode, moduleType); } diff --git a/server/src/analyzer/declaration.ts b/server/src/analyzer/declaration.ts index f1bbc951d..a31a003e2 100644 --- a/server/src/analyzer/declaration.ts +++ b/server/src/analyzer/declaration.ts @@ -71,7 +71,7 @@ export interface VariableDeclaration extends DeclarationBase { // Alias declarations are used for imports. They are resolved // after the binding phase. -export interface AliasDeclaration extends DeclarationBase, ModuleLoaderActions { +export interface AliasDeclaration extends DeclarationBase { type: DeclarationType.Alias; // If a symbolName is defined, the alias refers to a symbol @@ -79,6 +79,18 @@ export interface AliasDeclaration extends DeclarationBase, ModuleLoaderActions { // 'path' field). If symbolName is missing, the alias refers // to the module itself. symbolName?: string; + + // The first part of the multi-part name used in the import + // statement (e.g. for "import a.b.c", firstNamePart would + // be "a"). + firstNamePart?: string; + + // If the alias is targeting a module, multiple other modules + // may also need to be resolved and inserted implicitly into + // the module's namespace to emulate the behavior of the python + // module loader. This can be recursive (e.g. in the case of + // an "import a.b.c.d" statement). + implicitImports: Map; } // This interface represents a set of actions that the python loader @@ -89,11 +101,7 @@ export interface ModuleLoaderActions { // a directory). path: string; - // If the alias is targeting a module, multiple other modules - // may also need to be resolved and inserted implicitly into - // the module's namespace to emulate the behavior of the python - // module loader. This can be recursive (e.g. in the case of - // an "import a.b.c.d" statement). + // See comment for "implicitImports" field in AliasDeclaration. implicitImports: Map; } diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 8e639f465..d04364b72 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -1296,75 +1296,61 @@ export class TypeAnalyzer extends ParseTreeWalker { } visitImportAs(node: ImportAsNode): boolean { + if (node.module.nameParts.length === 0) { + return false; + } + const importInfo = AnalyzerNodeInfo.getImportInfo(node.module); assert(importInfo !== undefined); - if (importInfo && importInfo.isImportFound && importInfo.resolvedPaths.length > 0) { - const resolvedPath = importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1]; + let symbolNameNode: NameNode; + if (node.alias) { + // The symbol name is defined by the alias. + symbolNameNode = node.alias; + } else { + // There was no alias, so we need to use the first element of + // the name parts as the symbol. + symbolNameNode = node.module.nameParts[0]; + } - let moduleType = this._getModuleTypeForImportPath(importInfo, resolvedPath); + const symbolWithScope = this._currentScope.lookUpSymbolRecursive( + symbolNameNode.nameToken.value); + assert(symbolWithScope !== undefined); + const aliasDecl = symbolWithScope!.symbol.getDeclarations().find( + decl => decl.type === DeclarationType.Alias); - // Is there a cached module type associated with this node? - const cachedModuleType = AnalyzerNodeInfo.getExpressionType(node) as ModuleType; - if (cachedModuleType && cachedModuleType.category === TypeCategory.Module && moduleType) { - // If the fields match, use the cached version instead of the - // newly-created version so we preserve the work done to - // populate the loader fields. - if (moduleType.fields === cachedModuleType.fields) { - moduleType = cachedModuleType; - } + let symbolType: Type | undefined; + if (aliasDecl && aliasDecl.type === DeclarationType.Alias) { + symbolType = this._getSymbolTypeForAliasDeclaration(aliasDecl); + } + + if (!symbolType) { + symbolType = UnknownType.create(); + } + + // Is there a cached module type associated with this node? If so, use + // it instead of the type we just created. This will preserve the + // symbol accessed flags. + const cachedModuleType = AnalyzerNodeInfo.getExpressionType(node) as ModuleType; + if (cachedModuleType && cachedModuleType.category === TypeCategory.Module && symbolType) { + if (isTypeSame(symbolType, cachedModuleType)) { + symbolType = cachedModuleType; } + } - if (moduleType) { - // Import the implicit imports in the module's namespace. - importInfo.implicitImports.forEach(implicitImport => { - const implicitModuleType = this._getModuleTypeForImportPath( - importInfo, implicitImport.path); - if (implicitModuleType) { - if (!ModuleType.getField(moduleType!, implicitImport.name)) { - const newSymbol = Symbol.createWithType( - SymbolFlags.ClassMember, implicitModuleType, defaultTypeSourceId); - setSymbolPreservingAccess(moduleType!.loaderFields!, implicitImport.name, - newSymbol); - } - } - }); + // Cache the module type for subsequent passes. + AnalyzerNodeInfo.setExpressionType(node, symbolType); - if (node.alias) { - this._assignTypeToNameNode(node.alias, moduleType); - this._updateExpressionTypeForNode(node.alias, moduleType); + this._assignTypeToNameNode(symbolNameNode, symbolType); + this._updateExpressionTypeForNode(symbolNameNode, symbolType); - this._conditionallyReportUnusedName(node.alias, false, - this._fileInfo.diagnosticSettings.reportUnusedImport, - DiagnosticRule.reportUnusedImport, - `Import '${ node.alias.nameToken.value }' is not accessed`); - } else { - this._bindMultiPartModuleNameToType(node.module.nameParts, moduleType); - } - - // Cache the module type for subsequent passes. - AnalyzerNodeInfo.setExpressionType(node, moduleType); - } else { - // We were unable to resolve the import. Bind the names (or alias) - // to an unknown type. - const symbolType = UnknownType.create(); - const nameNode = node.module.nameParts.length > 0 ? node.module.nameParts[0] : undefined; - const aliasNode = node.alias || nameNode; - - if (node.alias && nameNode) { - this._updateExpressionTypeForNode(nameNode, symbolType); - } - - if (aliasNode) { - this._assignTypeToNameNode(aliasNode, symbolType); - this._updateExpressionTypeForNode(aliasNode, symbolType); - - this._conditionallyReportUnusedName(aliasNode, false, - this._fileInfo.diagnosticSettings.reportUnusedImport, - DiagnosticRule.reportUnusedImport, - `Import '${ aliasNode.nameToken.value }' is not accessed`); - } - } + if (node.alias) { + this._conditionallyReportUnusedName(symbolNameNode, false, + this._fileInfo.diagnosticSettings.reportUnusedImport, + DiagnosticRule.reportUnusedImport, + `Import '${ node.alias.nameToken.value }' is not accessed`); + } else { + // TODO - need to add unused import logic } return false; @@ -1559,6 +1545,36 @@ export class TypeAnalyzer extends ParseTreeWalker { return false; } + private _getSymbolTypeForAliasDeclaration(declaration: AliasDeclaration): ModuleType | undefined { + // This call doesn't apply to alias declarations with a symbol name. + assert(declaration.symbolName === undefined); + + // Build a module type that corresponds to the declaration and + // its associated loader actions. + const moduleType = ModuleType.create(); + this._applyLoaderActionsToModuleType(moduleType, declaration); + return moduleType; + } + + private _applyLoaderActionsToModuleType(moduleType: ModuleType, loaderActions: ModuleLoaderActions) { + if (loaderActions.path) { + const moduleSymbolTable = this._fileInfo.importLookup(loaderActions.path); + if (moduleSymbolTable) { + moduleType.fields = moduleSymbolTable; + } + } + + loaderActions.implicitImports.forEach((implicitImport, name) => { + const importedModuleType = ModuleType.create(); + const importedModuleSymbol = Symbol.createWithType( + SymbolFlags.None, importedModuleType, defaultTypeSourceId); + moduleType.loaderFields.set(name, importedModuleSymbol); + + // Recursively apply loader actions. + this._applyLoaderActionsToModuleType(importedModuleType, implicitImport); + }); + } + // Validates that a call to isinstance is necessary. This is a // common source of programming errors. private _validateIsInstanceCallNecessary(node: CallExpressionNode) { @@ -3022,21 +3038,20 @@ export class TypeAnalyzer extends ParseTreeWalker { const symbolTable = this._fileInfo.importLookup(path); if (symbolTable) { - return ModuleType.cloneForLoadedModule(ModuleType.create(symbolTable)); + return ModuleType.create(symbolTable); } else if (importResult) { // There was no module even though the import was resolved. This // happens in the case of namespace packages, where an __init__.py // is not necessarily present. We'll synthesize a module type in // this case. - const moduleType = ModuleType.cloneForLoadedModule( - ModuleType.create(new SymbolTable())); + const moduleType = ModuleType.create(); // Add the implicit imports. importResult.implicitImports.forEach(implicitImport => { const implicitModuleType = this._getModuleTypeForImportPath( undefined, implicitImport.path); if (implicitModuleType) { - setSymbolPreservingAccess(moduleType.loaderFields!, implicitImport.name, + setSymbolPreservingAccess(moduleType.loaderFields, implicitImport.name, Symbol.createWithType( SymbolFlags.ClassMember, implicitModuleType, defaultTypeSourceId)); } @@ -3329,83 +3344,6 @@ export class TypeAnalyzer extends ParseTreeWalker { this._evaluateExpressionForAssignment(target, srcType, srcExpr); } - private _bindMultiPartModuleNameToType(nameParts: NameNode[], type: ModuleType, - declaration?: Declaration): void { - - // The target symbol table will change as we progress through - // the multi-part name. Start with the current scope's symbol - // table, which should include the first part of the name. - const permanentScope = ScopeUtils.getPermanentScope(this._currentScope); - let targetSymbolTable = permanentScope.getSymbolTable(); - - for (let i = 0; i < nameParts.length; i++) { - const name = nameParts[i].nameToken.value; - - // Does this symbol already exist within this scope? - let targetSymbol = targetSymbolTable.get(name); - let symbolType = targetSymbol ? - TypeUtils.getEffectiveTypeOfSymbol(targetSymbol) : undefined; - - if (!symbolType || symbolType.category !== TypeCategory.Module) { - symbolType = ModuleType.create(new SymbolTable()); - symbolType = ModuleType.cloneForLoadedModule(symbolType); - } - - // If the symbol didn't exist, create a new one. - if (!targetSymbol) { - targetSymbol = Symbol.createWithType(SymbolFlags.ClassMember, - symbolType, defaultTypeSourceId); - } - - if (i === 0) { - // Assign the first part of the multi-part name to the current scope. - this._assignTypeToNameNode(nameParts[0], symbolType); - } - - if (i === nameParts.length - 1) { - const moduleType = symbolType; - moduleType.fields = type.fields; - moduleType.docString = type.docString; - - if (type.loaderFields) { - assert(moduleType.loaderFields !== undefined); - - // Copy the loader fields, which may include implicit - // imports for the module. - type.loaderFields.forEach((symbol, name) => { - setSymbolPreservingAccess(moduleType.loaderFields!, name, symbol); - }); - } - - if (declaration) { - targetSymbol.addDeclaration(declaration); - } - } - - setSymbolPreservingAccess(targetSymbolTable, name, targetSymbol); - assert(symbolType.loaderFields !== undefined); - targetSymbolTable = symbolType.loaderFields!; - - // If this is the last element, determine if it's accessed. - if (i === nameParts.length - 1) { - // Is this module ever accessed? - if (targetSymbol && !targetSymbol.isAccessed()) { - const multipartName = nameParts.map(np => np.nameToken.value).join('.'); - const textRange = { start: nameParts[0].start, length: nameParts[0].length }; - if (nameParts.length > 1) { - TextRange.extend(textRange, nameParts[nameParts.length - 1]); - } - this._fileInfo.diagnosticSink.addUnusedCodeWithTextRange( - `'${ multipartName }' is not accessed`, textRange); - - this._addDiagnostic(this._fileInfo.diagnosticSettings.reportUnusedImport, - DiagnosticRule.reportUnusedImport, - `Import '${ multipartName }' is not accessed`, textRange); - } - } - } - } - private _assignTypeToNameNode(nameNode: NameNode, srcType: Type, declaration?: Declaration, srcExpressionNode?: ParseNode) { diff --git a/server/src/analyzer/types.ts b/server/src/analyzer/types.ts index cbd3e7606..b0139b8a5 100644 --- a/server/src/analyzer/types.ts +++ b/server/src/analyzer/types.ts @@ -115,24 +115,14 @@ export interface ModuleType extends TypeBase { // A "loader" module includes symbols that were injected by // the module loader. We keep these separate so we don't // pollute the symbols exported by the module itself. - loaderFields?: SymbolTable; + loaderFields: SymbolTable; } export namespace ModuleType { - export function create(fields: SymbolTable, docString?: string) { + export function create(symbolTable?: SymbolTable) { const newModuleType: ModuleType = { category: TypeCategory.Module, - fields, - docString - }; - return newModuleType; - } - - export function cloneForLoadedModule(moduleType: ModuleType) { - const newModuleType: ModuleType = { - category: TypeCategory.Module, - fields: moduleType.fields, - docString: moduleType.docString, + fields: symbolTable || new SymbolTable(), loaderFields: new SymbolTable() }; return newModuleType; @@ -1223,7 +1213,17 @@ export function isTypeSame(type1: Type, type2: Type, recursionCount = 0): boolea // Module types are the same if they share the same // module symbol table. - return (type1.fields === type2Module.fields); + if (type1.fields === type2Module.fields) { + return true; + } + + // If both symbol tables are empty, we can also assume + // they're equal. + if (type1.fields.isEmpty() && type2Module.fields.isEmpty()) { + return true; + } + + return false; } }