From 4292280e11cb0ddd09c94ef26bdc51919312a16e Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 08:59:54 -0700 Subject: [PATCH 01/23] Eliminated use of ModuleType from sourceFile and import maps. --- server/src/analyzer/analyzerFileInfo.ts | 4 +- server/src/analyzer/expressionEvaluator.ts | 10 ++--- server/src/analyzer/program.ts | 16 +++---- server/src/analyzer/sourceFile.ts | 17 +++---- server/src/analyzer/typeAnalyzer.ts | 14 ++---- .../src/languageService/completionProvider.ts | 6 +-- server/src/languageService/hoverProvider.ts | 44 +------------------ server/src/tests/testUtils.ts | 4 +- 8 files changed, 29 insertions(+), 86 deletions(-) diff --git a/server/src/analyzer/analyzerFileInfo.ts b/server/src/analyzer/analyzerFileInfo.ts index fa77c8f70..ca5a3bd4d 100644 --- a/server/src/analyzer/analyzerFileInfo.ts +++ b/server/src/analyzer/analyzerFileInfo.ts @@ -13,10 +13,10 @@ import StringMap from '../common/stringMap'; import { TextRange } from '../common/textRange'; import { TextRangeCollection } from '../common/textRangeCollection'; import { Scope } from './scope'; -import { ModuleType } from './types'; +import { SymbolTable } from './symbol'; // Maps import paths to the parse tree for the imported module. -export type ImportMap = Map; +export type ImportMap = Map; export interface AnalyzerFileInfo { importMap: ImportMap; diff --git a/server/src/analyzer/expressionEvaluator.ts b/server/src/analyzer/expressionEvaluator.ts index e70a04326..eb32ab818 100644 --- a/server/src/analyzer/expressionEvaluator.ts +++ b/server/src/analyzer/expressionEvaluator.ts @@ -608,16 +608,12 @@ export class ExpressionEvaluator { return undefined; } - const moduleType = this._fileInfo.importMap.get(typingImportPath); - if (!moduleType) { + const symbolTable = this._fileInfo.importMap.get(typingImportPath); + if (!symbolTable) { return undefined; } - if (moduleType.category !== TypeCategory.Module) { - return undefined; - } - - const symbol = ModuleType.getField(moduleType, symbolName); + const symbol = symbolTable.get(symbolName); if (!symbol) { return undefined; } diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index 9b4c55649..afca8d8ed 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -30,7 +30,7 @@ import { ImportResolver } from './importResolver'; import { ImportResult, ImportType } from './importResult'; import { Scope } from './scope'; import { SourceFile } from './sourceFile'; -import { ModuleType } from './types'; +import { SymbolTable } from './symbol'; import { TypeStubWriter } from './typeStubWriter'; const _maxImportDepth = 256; @@ -531,12 +531,12 @@ export class Program { } private _buildImportMap(sourceFileInfo: SourceFileInfo): ImportMap { - const importMap: ImportMap = new Map(); + const importMap: ImportMap = new Map(); for (const importedFileInfo of sourceFileInfo.imports) { - const moduleType = importedFileInfo.sourceFile.getModuleType(); - if (moduleType) { - importMap.set(importedFileInfo.sourceFile.getFilePath(), moduleType); + const symbolTable = importedFileInfo.sourceFile.getModuleSymbolTable(); + if (symbolTable) { + importMap.set(importedFileInfo.sourceFile.getFilePath(), symbolTable); } } @@ -550,9 +550,9 @@ export class Program { this._sourceFileList.forEach(fileInfo => { if (fileInfo !== sourceFileToExclude) { - const moduleType = fileInfo.sourceFile.getModuleType(); - if (moduleType) { - moduleSymbolMap[fileInfo.sourceFile.getFilePath()] = moduleType.fields; + const symbolTable = fileInfo.sourceFile.getModuleSymbolTable(); + if (symbolTable) { + moduleSymbolMap[fileInfo.sourceFile.getFilePath()] = symbolTable; } } }); diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index b99ff9770..0f5a74840 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -43,9 +43,9 @@ import { ImportResolver } from './importResolver'; import { ImportResult } from './importResult'; import { ParseTreeCleanerWalker } from './parseTreeCleaner'; import { Scope } from './scope'; +import { SymbolTable } from './symbol'; import { TestWalker } from './testWalker'; import { TypeAnalyzer } from './typeAnalyzer'; -import { ModuleType, TypeCategory } from './types'; const _maxImportCyclesPerFile = 4; @@ -107,7 +107,7 @@ export class SourceFile { private _parseTreeNeedsCleaning = false; private _parseResults?: ParseResults; - private _moduleType?: ModuleType; + private _moduleSymbolTable?: SymbolTable; // Diagnostics generated during different phases of analysis. private _parseDiagnostics: Diagnostic[] = []; @@ -280,8 +280,8 @@ export class SourceFile { return this._builtinsImport; } - getModuleType(): ModuleType | undefined { - return this._moduleType; + getModuleSymbolTable(): SymbolTable | undefined { + return this._moduleSymbolTable; } // Indicates whether the contents of the file have changed since @@ -318,7 +318,7 @@ export class SourceFile { this._fileContentsVersion++; this._isTypeAnalysisFinalized = false; this._isTypeAnalysisPassNeeded = true; - this._moduleType = undefined; + this._moduleSymbolTable = undefined; } markReanalysisRequired(): void { @@ -647,10 +647,7 @@ export class SourceFile { this._bindDiagnostics = fileInfo.diagnosticSink.diagnostics; const moduleScope = AnalyzerNodeInfo.getScope(this._parseResults!.parseTree); assert(moduleScope !== undefined); - const moduleType = AnalyzerNodeInfo.getExpressionType( - this._parseResults!.parseTree); - assert(moduleType && moduleType.category === TypeCategory.Module); - this._moduleType = moduleType as ModuleType; + this._moduleSymbolTable = moduleScope!.getSymbolTable(); }); } catch (e) { const message: string = (e.stack ? e.stack.toString() : undefined) || @@ -741,7 +738,7 @@ export class SourceFile { const analysisDiagnostics = new TextRangeDiagnosticSink(this._parseResults!.tokenizerOutput.lines); const fileInfo: AnalyzerFileInfo = { - importMap: importMap || new Map(), + importMap: importMap || new Map(), futureImports: this._parseResults!.futureImports, builtinsScope, typingModulePath: this._typingModulePath, diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 74fd32106..f7877b693 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -2683,18 +2683,12 @@ export class TypeAnalyzer extends ParseTreeWalker { } private _findCollectionsImportSymbolTable(): SymbolTable | undefined { - let moduleType: ModuleType | undefined; for (const key of this._fileInfo.importMap.keys()) { if (key.endsWith('collections/__init__.pyi')) { - moduleType = this._fileInfo.importMap.get(key); - break; + return this._fileInfo.importMap.get(key); } } - if (moduleType) { - return moduleType.fields; - } - return undefined; } @@ -3055,9 +3049,9 @@ export class TypeAnalyzer extends ParseTreeWalker { } } - const moduleType = this._fileInfo.importMap.get(path); - if (moduleType) { - return ModuleType.cloneForLoadedModule(moduleType); + const symbolTable = this._fileInfo.importMap.get(path); + if (symbolTable) { + return ModuleType.cloneForLoadedModule(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 diff --git a/server/src/languageService/completionProvider.ts b/server/src/languageService/completionProvider.ts index c9df7a9b5..b4dd0e8f1 100644 --- a/server/src/languageService/completionProvider.ts +++ b/server/src/languageService/completionProvider.ts @@ -566,10 +566,8 @@ export class CompletionProvider { const importMap = this._importMapCallback(); - const moduleType = importMap.get(resolvedPath); - if (moduleType) { - const symbolTable = new SymbolTable(); - TypeUtils.getMembersForModule(moduleType, symbolTable); + const symbolTable = importMap.get(resolvedPath); + if (symbolTable) { this._addSymbolsForSymbolTable(symbolTable, name => { // Don't suggest symbols that have already been imported. diff --git a/server/src/languageService/hoverProvider.ts b/server/src/languageService/hoverProvider.ts index edeb66e11..e380ab756 100644 --- a/server/src/languageService/hoverProvider.ts +++ b/server/src/languageService/hoverProvider.ts @@ -54,9 +54,7 @@ export class HoverProvider { } }; - if (node.nodeType === ParseNodeType.ModuleName) { - this._addResultsForModuleNameNode(results.parts, node, offset, importMap); - } else if (node.nodeType === ParseNodeType.Name) { + if (node.nodeType === ParseNodeType.Name) { const declarations = DeclarationUtils.getDeclarationsForNameNode(node); if (declarations && declarations.length > 0) { this._addResultsForDeclaration(results.parts, declarations[0], node); @@ -150,46 +148,6 @@ export class HoverProvider { } } - private static _addResultsForModuleNameNode(parts: HoverTextPart[], node: ModuleNameNode, - offset: number, importMap: ImportMap) { - - // If this is an imported module name, try to map the position - // to the resolved import path. - const importInfo = AnalyzerNodeInfo.getImportInfo(node); - if (!importInfo) { - return; - } - - let pathOffset = node.nameParts.findIndex(range => { - return offset >= range.start && offset < TextRange.getEnd(range); - }); - - if (pathOffset < 0) { - return; - } - - if (pathOffset >= importInfo.resolvedPaths.length) { - pathOffset = importInfo.resolvedPaths.length - 1; - } - - if (importInfo.resolvedPaths[pathOffset]) { - const resolvedPath = importInfo.resolvedPaths[pathOffset]; - this._addResultsPart(parts, '(module) "' + resolvedPath + '"', true); - - if (importInfo.importType === ImportType.ThirdParty && !importInfo.isStubFile) { - this._addResultsPart(parts, - 'No type stub found for this module. Imported symbol types are unknown.'); - } - - // If the module has been resolved and already analyzed, - // we can add the docString for it as well. - const moduleType = importMap.get(resolvedPath); - if (moduleType) { - this._addDocumentationPartForType(parts, moduleType); - } - } - } - private static _getTypeFromNode(node: ParseNode): Type | undefined { return AnalyzerNodeInfo.getExpressionType(node); } diff --git a/server/src/tests/testUtils.ts b/server/src/tests/testUtils.ts index 8b922f1f5..1786412e9 100644 --- a/server/src/tests/testUtils.ts +++ b/server/src/tests/testUtils.ts @@ -15,8 +15,8 @@ import { AnalyzerFileInfo } from '../analyzer/analyzerFileInfo'; import { ModuleScopeBinder } from '../analyzer/binder'; import { ImportResolver } from '../analyzer/importResolver'; import { Program } from '../analyzer/program'; +import { SymbolTable } from '../analyzer/symbol'; import { TestWalker } from '../analyzer/testWalker'; -import { ModuleType } from '../analyzer/types'; import { cloneDiagnosticSettings, ConfigOptions, ExecutionEnvironment } from '../common/configOptions'; import { Diagnostic, DiagnosticCategory } from '../common/diagnostic'; import { DiagnosticSink, TextRangeDiagnosticSink } from '../common/diagnosticSink'; @@ -77,7 +77,7 @@ export function buildAnalyzerFileInfo(filePath: string, parseResults: ParseResul const analysisDiagnostics = new TextRangeDiagnosticSink(parseResults.tokenizerOutput.lines); const fileInfo: AnalyzerFileInfo = { - importMap: new Map(), + importMap: new Map(), futureImports: new StringMap(), builtinsScope: undefined, diagnosticSink: analysisDiagnostics, From 4cbc0fc11b7eb8e92af92e11b66e14e3f8f7794a Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 09:52:23 -0700 Subject: [PATCH 02/23] Fixed bug in parseTreeWalker where the named parts of a module name node weren't returned as children. --- server/src/analyzer/parseTreeWalker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/analyzer/parseTreeWalker.ts b/server/src/analyzer/parseTreeWalker.ts index 1b3b41f91..ac1945718 100644 --- a/server/src/analyzer/parseTreeWalker.ts +++ b/server/src/analyzer/parseTreeWalker.ts @@ -285,7 +285,7 @@ export class ParseTreeWalker { case ParseNodeType.ModuleName: if (this.visitModuleName(node)) { - return []; + return node.nameParts; } break; From 095683f004104ce200df357851f9858c1a925684 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 09:52:53 -0700 Subject: [PATCH 03/23] Added code to declarationUtils to return a synthesized alias declaration for a named node that's part of a module name. --- server/src/analyzer/declarationUtils.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/server/src/analyzer/declarationUtils.ts b/server/src/analyzer/declarationUtils.ts index 1bc49150a..fb83d4541 100644 --- a/server/src/analyzer/declarationUtils.ts +++ b/server/src/analyzer/declarationUtils.ts @@ -9,12 +9,13 @@ import * as assert from 'assert'; +import { getEmptyRange } from '../common/diagnostic'; import { NameNode, ParseNode, ParseNodeType } from '../parser/parseNodes'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; import { AliasDeclaration, Declaration, DeclarationType } from './declaration'; import * as ParseTreeUtils from './parseTreeUtils'; import { Symbol } from './symbol'; -import { ClassType, ModuleType, ObjectType, Type, TypeCategory, UnknownType } from './types'; +import { ClassType, ModuleType, ObjectType, Type, TypeCategory } from './types'; import * as TypeUtils from './typeUtils'; export function getDeclarationsForNameNode(node: NameNode): Declaration[] | undefined { @@ -51,6 +52,21 @@ export function getDeclarationsForNameNode(node: NameNode): Declaration[] | unde return subtype; }); } + } else if (node.parent && node.parent.nodeType === ParseNodeType.ModuleName) { + const namePartIndex = node.parent.nameParts.findIndex(part => part === node); + const importInfo = AnalyzerNodeInfo.getImportInfo(node.parent); + if (namePartIndex >= 0 && importInfo && namePartIndex < importInfo.resolvedPaths.length) { + if (importInfo.resolvedPaths[namePartIndex]) { + // Synthesize an alias declaration for this name part. The only + // time this case is used is for the hover provider. + const aliasDeclaration: AliasDeclaration = { + type: DeclarationType.Alias, + path: importInfo.resolvedPaths[namePartIndex], + range: getEmptyRange() + }; + declarations = [aliasDeclaration]; + } + } } else { const scopeNode = ParseTreeUtils.getScopeNodeForNode(node); if (scopeNode) { From ed7505c52fd9e99dc53ccee216a001f466338f2d Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 09:54:43 -0700 Subject: [PATCH 04/23] Replaced import maps with import lookup callback. This also required a change in the way the collections module is resolved. --- server/src/analyzer/analyzerFileInfo.ts | 7 +-- server/src/analyzer/expressionEvaluator.ts | 2 +- server/src/analyzer/program.ts | 26 ++++------- server/src/analyzer/sourceFile.ts | 46 +++++++++++++------ server/src/analyzer/typeAnalyzer.ts | 10 ++-- .../src/languageService/completionProvider.ts | 8 ++-- server/src/languageService/hoverProvider.ts | 7 ++- server/src/tests/testUtils.ts | 2 +- 8 files changed, 57 insertions(+), 51 deletions(-) diff --git a/server/src/analyzer/analyzerFileInfo.ts b/server/src/analyzer/analyzerFileInfo.ts index ca5a3bd4d..42868e630 100644 --- a/server/src/analyzer/analyzerFileInfo.ts +++ b/server/src/analyzer/analyzerFileInfo.ts @@ -15,14 +15,15 @@ import { TextRangeCollection } from '../common/textRangeCollection'; import { Scope } from './scope'; import { SymbolTable } from './symbol'; -// Maps import paths to the parse tree for the imported module. -export type ImportMap = Map; +// Maps import paths to the symbol table for the imported module. +export type ImportLookup = (filePath: string) => SymbolTable | undefined; export interface AnalyzerFileInfo { - importMap: ImportMap; + importLookup: ImportLookup; futureImports: StringMap; builtinsScope?: Scope; typingModulePath?: string; + collectionsModulePath?: string; diagnosticSink: TextRangeDiagnosticSink; executionEnvironment: ExecutionEnvironment; diagnosticSettings: DiagnosticSettings; diff --git a/server/src/analyzer/expressionEvaluator.ts b/server/src/analyzer/expressionEvaluator.ts index eb32ab818..61eddb823 100644 --- a/server/src/analyzer/expressionEvaluator.ts +++ b/server/src/analyzer/expressionEvaluator.ts @@ -608,7 +608,7 @@ export class ExpressionEvaluator { return undefined; } - const symbolTable = this._fileInfo.importMap.get(typingImportPath); + const symbolTable = this._fileInfo.importLookup(typingImportPath); if (!symbolTable) { return undefined; } diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index afca8d8ed..0a00e2a5d 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -23,7 +23,6 @@ import { Duration, timingStats } from '../common/timing'; import { ModuleSymbolMap } from '../languageService/completionProvider'; import { HoverResults } from '../languageService/hoverProvider'; import { SignatureHelpResults } from '../languageService/signatureHelpProvider'; -import { ImportMap } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; import { CircularDependency } from './circularDependency'; import { ImportResolver } from './importResolver'; @@ -527,20 +526,16 @@ export class Program { } } - fileToAnalyze.sourceFile.bind(options, builtinsScope); + fileToAnalyze.sourceFile.bind(options, this._lookUpImport, builtinsScope); } - private _buildImportMap(sourceFileInfo: SourceFileInfo): ImportMap { - const importMap: ImportMap = new Map(); - - for (const importedFileInfo of sourceFileInfo.imports) { - const symbolTable = importedFileInfo.sourceFile.getModuleSymbolTable(); - if (symbolTable) { - importMap.set(importedFileInfo.sourceFile.getFilePath(), symbolTable); - } + private _lookUpImport = (filePath: string): SymbolTable | undefined => { + const sourceFileInfo = this._sourceFileMap[filePath]; + if (!sourceFileInfo) { + return undefined; } - return importMap; + return sourceFileInfo.sourceFile.getModuleSymbolTable(); } // Build a map of all modules within this program and the module- @@ -590,15 +585,12 @@ export class Program { closureMap.set(fileToAnalyze.sourceFile.getFilePath(), false); if (fileToAnalyze.sourceFile.isTypeAnalysisRequired()) { - // Build the import map for the file. - const importMap = this._buildImportMap(fileToAnalyze); - // Do a type analysis pass and determine if any internal changes occurred // during the pass. If so, continue to analyze until it stops changing and // mark all of its dependencies as needing to be reanalyzed. let didAnalysisChange = false; while (true) { - fileToAnalyze.sourceFile.doTypeAnalysis(options, importMap); + fileToAnalyze.sourceFile.doTypeAnalysis(options, this._lookUpImport); if (!fileToAnalyze.sourceFile.isTypeAnalysisRequired()) { break; @@ -914,7 +906,7 @@ export class Program { } return sourceFileInfo.sourceFile.getHoverForPosition(position, - this._buildImportMap(sourceFileInfo)); + this._lookUpImport); } getSignatureHelpForPosition(filePath: string, position: DiagnosticTextPosition, @@ -955,7 +947,7 @@ export class Program { return sourceFileInfo.sourceFile.getCompletionsForPosition( position, options, importResolver, - () => this._buildImportMap(sourceFileInfo), + this._lookUpImport, () => this._buildModuleSymbolsMap(sourceFileInfo)); } diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index 0f5a74840..79e7d29f6 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -34,7 +34,7 @@ import { SignatureHelpProvider, SignatureHelpResults } from '../languageService/ import { ModuleNode } from '../parser/parseNodes'; import { ModuleImport, ParseOptions, Parser, ParseResults } from '../parser/parser'; import { Token } from '../parser/tokenizerTypes'; -import { AnalyzerFileInfo, ImportMap } from './analyzerFileInfo'; +import { AnalyzerFileInfo, ImportLookup } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; import { ModuleScopeBinder } from './binder'; import { CircularDependency } from './circularDependency'; @@ -144,6 +144,7 @@ export class SourceFile { private _imports?: ImportResult[]; private _builtinsImport?: ImportResult; private _typingModulePath?: string; + private _collectionsModulePath?: string; constructor(filePath: string, isTypeshedStubFile: boolean, isThirdPartyImport: boolean, console?: ConsoleInterface) { @@ -449,7 +450,7 @@ export class SourceFile { // Resolve imports. timingStats.resolveImportsTime.timeOperation(() => { - [this._imports, this._builtinsImport, this._typingModulePath] = + [this._imports, this._builtinsImport, this._typingModulePath, this._collectionsModulePath] = this._resolveImports(importResolver, parseResults.importedModules, execEnvironment); this._parseDiagnostics = diagSink.diagnostics; }); @@ -543,14 +544,16 @@ export class SourceFile { this._filePath, this._parseResults); } - getHoverForPosition(position: DiagnosticTextPosition, importMap: ImportMap): HoverResults | undefined { + getHoverForPosition(position: DiagnosticTextPosition, + importLookup: ImportLookup): HoverResults | undefined { + // If we have no completed analysis job, there's nothing to do. if (!this._parseResults) { return undefined; } return HoverProvider.getHoverForPosition( - this._parseResults, position, importMap); + this._parseResults, position, importLookup); } getSignatureHelpForPosition(position: DiagnosticTextPosition): SignatureHelpResults | undefined { @@ -571,7 +574,7 @@ export class SourceFile { getCompletionsForPosition(position: DiagnosticTextPosition, configOptions: ConfigOptions, importResolver: ImportResolver, - importMapCallback: () => ImportMap, + importLookup: ImportLookup, moduleSymbolsCallback: () => ModuleSymbolMap): CompletionList | undefined { // If we have no completed analysis job, there's nothing to do. @@ -588,7 +591,7 @@ export class SourceFile { const completionProvider = new CompletionProvider( this._parseResults, this._fileContents, importResolver, position, - this._filePath, configOptions, importMapCallback, + this._filePath, configOptions, importLookup, moduleSymbolsCallback); return completionProvider.getCompletionsForPosition(); @@ -622,7 +625,7 @@ export class SourceFile { this._isTypeAnalysisFinalized = false; } - bind(configOptions: ConfigOptions, builtinsScope?: Scope) { + bind(configOptions: ConfigOptions, importLookup: ImportLookup, builtinsScope?: Scope) { assert(!this.isParseRequired()); assert(this.isBindingRequired()); assert(this._parseResults); @@ -630,7 +633,7 @@ export class SourceFile { try { // Perform name binding. timingStats.bindTime.timeOperation(() => { - const fileInfo = this._buildFileInfo(configOptions, undefined, builtinsScope); + const fileInfo = this._buildFileInfo(configOptions, importLookup, builtinsScope); this._cleanParseTreeIfRequired(); const binder = new ModuleScopeBinder( @@ -672,7 +675,7 @@ export class SourceFile { this._isBindingNeeded = false; } - doTypeAnalysis(configOptions: ConfigOptions, importMap: ImportMap) { + doTypeAnalysis(configOptions: ConfigOptions, importLookup: ImportLookup) { assert(!this.isParseRequired()); assert(!this.isBindingRequired()); assert(this.isTypeAnalysisRequired()); @@ -680,7 +683,7 @@ export class SourceFile { try { timingStats.typeAnalyzerTime.timeOperation(() => { - const fileInfo = this._buildFileInfo(configOptions, importMap, undefined); + const fileInfo = this._buildFileInfo(configOptions, importLookup, undefined); // Perform static type analysis. const typeAnalyzer = new TypeAnalyzer(this._parseResults!.parseTree, @@ -733,15 +736,18 @@ export class SourceFile { this._diagnosticVersion++; } - private _buildFileInfo(configOptions: ConfigOptions, importMap?: ImportMap, builtinsScope?: Scope) { + private _buildFileInfo(configOptions: ConfigOptions, importLookup: ImportLookup, + builtinsScope?: Scope) { + assert(this._parseResults !== undefined); const analysisDiagnostics = new TextRangeDiagnosticSink(this._parseResults!.tokenizerOutput.lines); const fileInfo: AnalyzerFileInfo = { - importMap: importMap || new Map(), + importLookup, futureImports: this._parseResults!.futureImports, builtinsScope, typingModulePath: this._typingModulePath, + collectionsModulePath: this._collectionsModulePath, diagnosticSink: analysisDiagnostics, executionEnvironment: configOptions.findExecEnvironment(this._filePath), diagnosticSettings: this._diagnosticSettings, @@ -768,7 +774,7 @@ export class SourceFile { private _resolveImports(importResolver: ImportResolver, moduleImports: ModuleImport[], execEnv: ExecutionEnvironment): - [ImportResult[], ImportResult?, string?] { + [ImportResult[], ImportResult?, string?, string?] { const imports: ImportResult[] = []; @@ -810,6 +816,8 @@ export class SourceFile { typingModulePath = typingImportResult.resolvedPaths[0]; } + let collectionsModulePath: string | undefined; + for (const moduleImport of moduleImports) { const importResult = importResolver.resolveImport( this._filePath, @@ -820,6 +828,16 @@ export class SourceFile { importedSymbols: moduleImport.importedSymbols } ); + + // If the file imports the stdlib 'collections' module, stash + // away its file path. The type analyzer may need this to + // access types defined in the collections module. + if (importResult.isImportFound && importResult.isTypeshedFile) { + if (moduleImport.nameParts.length >= 1 && moduleImport.nameParts[0] === 'collections') { + collectionsModulePath = importResult.resolvedPaths[importResult.resolvedPaths.length - 1]; + } + } + imports.push(importResult); // Associate the import results with the module import @@ -828,6 +846,6 @@ export class SourceFile { AnalyzerNodeInfo.setImportInfo(moduleImport.nameNode, importResult); } - return [imports, builtinsImportResult, typingModulePath]; + return [imports, builtinsImportResult, typingModulePath, collectionsModulePath]; } } diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index f7877b693..8f5aa13e6 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -1432,7 +1432,7 @@ export class TypeAnalyzer extends ParseTreeWalker { const implicitImport = importInfo.implicitImports.find(impImport => impImport.name === name); if (implicitImport) { const moduleType = this._getModuleTypeForImportPath(importInfo, implicitImport.path); - if (moduleType && this._fileInfo.importMap.has(implicitImport.path)) { + if (moduleType && this._fileInfo.importLookup(implicitImport.path)) { symbolType = moduleType; declarations = [{ type: DeclarationType.Module, @@ -2683,10 +2683,8 @@ export class TypeAnalyzer extends ParseTreeWalker { } private _findCollectionsImportSymbolTable(): SymbolTable | undefined { - for (const key of this._fileInfo.importMap.keys()) { - if (key.endsWith('collections/__init__.pyi')) { - return this._fileInfo.importMap.get(key); - } + if (this._fileInfo.collectionsModulePath) { + return this._fileInfo.importLookup(this._fileInfo.collectionsModulePath); } return undefined; @@ -3049,7 +3047,7 @@ export class TypeAnalyzer extends ParseTreeWalker { } } - const symbolTable = this._fileInfo.importMap.get(path); + const symbolTable = this._fileInfo.importLookup(path); if (symbolTable) { return ModuleType.cloneForLoadedModule(ModuleType.create(symbolTable)); } else if (importResult) { diff --git a/server/src/languageService/completionProvider.ts b/server/src/languageService/completionProvider.ts index b4dd0e8f1..2eef928e4 100644 --- a/server/src/languageService/completionProvider.ts +++ b/server/src/languageService/completionProvider.ts @@ -11,7 +11,7 @@ import { CompletionItem, CompletionItemKind, CompletionList, MarkupKind, TextEdit } from 'vscode-languageserver'; -import { ImportMap } from '../analyzer/analyzerFileInfo'; +import { ImportLookup } from '../analyzer/analyzerFileInfo'; import * as AnalyzerNodeInfo from '../analyzer/analyzerNodeInfo'; import { Declaration, DeclarationType } from '../analyzer/declaration'; import { getTypeForDeclaration } from '../analyzer/declarationUtils'; @@ -142,7 +142,7 @@ export class CompletionProvider { private _position: DiagnosticTextPosition, private _filePath: string, private _configOptions: ConfigOptions, - private _importMapCallback: () => ImportMap, + private _importLookup: ImportLookup, private _moduleSymbolsCallback: () => ModuleSymbolMap) { } @@ -564,9 +564,7 @@ export class CompletionProvider { const resolvedPath = importInfo.resolvedPaths.length > 0 ? importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1] : ''; - const importMap = this._importMapCallback(); - - const symbolTable = importMap.get(resolvedPath); + const symbolTable = this._importLookup(resolvedPath); if (symbolTable) { this._addSymbolsForSymbolTable(symbolTable, name => { diff --git a/server/src/languageService/hoverProvider.ts b/server/src/languageService/hoverProvider.ts index e380ab756..36bc1e601 100644 --- a/server/src/languageService/hoverProvider.ts +++ b/server/src/languageService/hoverProvider.ts @@ -9,17 +9,16 @@ * position within a smart editor. */ -import { ImportMap } from '../analyzer/analyzerFileInfo'; +import { ImportLookup } from '../analyzer/analyzerFileInfo'; import * as AnalyzerNodeInfo from '../analyzer/analyzerNodeInfo'; import { Declaration, DeclarationType } from '../analyzer/declaration'; import * as DeclarationUtils from '../analyzer/declarationUtils'; -import { ImportType } from '../analyzer/importResult'; import * as ParseTreeUtils from '../analyzer/parseTreeUtils'; import { ClassType, FunctionType, printType, Type, TypeCategory, UnknownType } from '../analyzer/types'; import { DiagnosticTextPosition, DiagnosticTextRange } from '../common/diagnostic'; import { convertOffsetToPosition, convertPositionToOffset } from '../common/positionUtils'; import { TextRange } from '../common/textRange'; -import { ModuleNameNode, ParseNode, ParseNodeType } from '../parser/parseNodes'; +import { ParseNode, ParseNodeType } from '../parser/parseNodes'; import { ParseResults } from '../parser/parser'; export interface HoverTextPart { @@ -34,7 +33,7 @@ export interface HoverResults { export class HoverProvider { static getHoverForPosition(parseResults: ParseResults, position: DiagnosticTextPosition, - importMap: ImportMap): HoverResults | undefined { + importLookup: ImportLookup): HoverResults | undefined { const offset = convertPositionToOffset(position, parseResults.tokenizerOutput.lines); if (offset === undefined) { diff --git a/server/src/tests/testUtils.ts b/server/src/tests/testUtils.ts index 1786412e9..2e407635f 100644 --- a/server/src/tests/testUtils.ts +++ b/server/src/tests/testUtils.ts @@ -77,7 +77,7 @@ export function buildAnalyzerFileInfo(filePath: string, parseResults: ParseResul const analysisDiagnostics = new TextRangeDiagnosticSink(parseResults.tokenizerOutput.lines); const fileInfo: AnalyzerFileInfo = { - importMap: new Map(), + importLookup: _ => undefined, futureImports: new StringMap(), builtinsScope: undefined, diagnosticSink: analysisDiagnostics, From e247e9d1ef018793d5a7c4594146f2a666764455 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 10:50:22 -0700 Subject: [PATCH 05/23] Eliminated ModuleDeclaration and resolvedDeclarations field of AliasDeclaration. --- server/src/analyzer/declaration.ts | 14 ++--- server/src/analyzer/declarationUtils.ts | 60 +++++-------------- server/src/analyzer/symbol.ts | 2 +- server/src/analyzer/typeAnalyzer.ts | 60 +++++++------------ server/src/common/pathUtils.ts | 2 +- .../src/languageService/completionProvider.ts | 12 ++-- .../src/languageService/definitionProvider.ts | 4 +- .../languageService/documentSymbolProvider.ts | 5 +- server/src/languageService/hoverProvider.ts | 24 +++----- 9 files changed, 62 insertions(+), 121 deletions(-) diff --git a/server/src/analyzer/declaration.ts b/server/src/analyzer/declaration.ts index d48448c32..0338b02d7 100644 --- a/server/src/analyzer/declaration.ts +++ b/server/src/analyzer/declaration.ts @@ -21,7 +21,6 @@ export const enum DeclarationType { Function, Method, Class, - Module, Alias } @@ -70,11 +69,6 @@ export interface VariableDeclaration extends DeclarationBase { isConstant?: boolean; } -export interface ModuleDeclaration extends DeclarationBase { - type: DeclarationType.Module; - moduleType: ModuleType; -} - // Alias declarations are used for imports. They are resolved // after the binding phase. export interface AliasDeclaration extends DeclarationBase { @@ -86,10 +80,12 @@ export interface AliasDeclaration extends DeclarationBase { // the module itself. symbolName?: string; - // The resolved declaration. - resolvedDeclarations?: Declaration[]; + // If there is no symbol specified and the entire module + // is referenced, should the module type include the + // implicit imports within its namespace? + includeImplicitImports?: boolean; } export type Declaration = BuiltInDeclaration | ClassDeclaration | FunctionDeclaration | ParameterDeclaration | VariableDeclaration | - ModuleDeclaration | AliasDeclaration; + AliasDeclaration; diff --git a/server/src/analyzer/declarationUtils.ts b/server/src/analyzer/declarationUtils.ts index fb83d4541..71d9b6503 100644 --- a/server/src/analyzer/declarationUtils.ts +++ b/server/src/analyzer/declarationUtils.ts @@ -11,6 +11,7 @@ import * as assert from 'assert'; import { getEmptyRange } from '../common/diagnostic'; import { NameNode, ParseNode, ParseNodeType } from '../parser/parseNodes'; +import { ImportLookup } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; import { AliasDeclaration, Declaration, DeclarationType } from './declaration'; import * as ParseTreeUtils from './parseTreeUtils'; @@ -89,44 +90,20 @@ export function isFunctionOrMethodDeclaration(declaration: Declaration) { return declaration.type === DeclarationType.Method || declaration.type === DeclarationType.Function; } -export function resolveDeclarationAliases(declaration: Declaration) { - let resolvedDeclaration: Declaration | undefined = declaration; - const resolvedDeclarations: AliasDeclaration[] = []; - - while (resolvedDeclaration && resolvedDeclaration.type === DeclarationType.Alias) { - // Detect circular dependencies. - if (resolvedDeclarations.find(decl => decl === resolvedDeclaration)) { - return undefined; - } - - resolvedDeclarations.push(resolvedDeclaration); - resolvedDeclaration = resolvedDeclaration.resolvedDeclarations ? - resolvedDeclaration.resolvedDeclarations[0] : undefined; - } - - return resolvedDeclaration; -} - export function getTypeForDeclaration(declaration: Declaration): Type | undefined { - const resolvedDeclaration = resolveDeclarationAliases(declaration); - - if (!resolvedDeclaration) { - return undefined; - } - - switch (resolvedDeclaration.type) { + switch (declaration.type) { case DeclarationType.BuiltIn: - return resolvedDeclaration.declaredType; + return declaration.declaredType; case DeclarationType.Class: - return AnalyzerNodeInfo.getExpressionType(resolvedDeclaration.node.name); + return AnalyzerNodeInfo.getExpressionType(declaration.node.name); case DeclarationType.Function: case DeclarationType.Method: - return AnalyzerNodeInfo.getExpressionType(resolvedDeclaration.node.name); + return AnalyzerNodeInfo.getExpressionType(declaration.node.name); case DeclarationType.Parameter: { - let typeAnnotationNode = resolvedDeclaration.node.typeAnnotation; + let typeAnnotationNode = declaration.node.typeAnnotation; if (typeAnnotationNode && typeAnnotationNode.nodeType === ParseNodeType.StringList) { typeAnnotationNode = typeAnnotationNode.typeAnnotation; } @@ -141,7 +118,7 @@ export function getTypeForDeclaration(declaration: Declaration): Type | undefine } case DeclarationType.Variable: { - let typeAnnotationNode = resolvedDeclaration.typeAnnotationNode; + let typeAnnotationNode = declaration.typeAnnotationNode; if (typeAnnotationNode && typeAnnotationNode.nodeType === ParseNodeType.StringList) { typeAnnotationNode = typeAnnotationNode.typeAnnotation; } @@ -156,20 +133,14 @@ export function getTypeForDeclaration(declaration: Declaration): Type | undefine return undefined; } - case DeclarationType.Module: - return resolvedDeclaration.moduleType; + case DeclarationType.Alias: { + return undefined; + } } } -export function hasTypeForDeclaration(declaration: Declaration, resolveAliases = true): boolean { - const resolvedDeclaration = resolveAliases ? - resolveDeclarationAliases(declaration) : declaration; - - if (!resolvedDeclaration) { - return false; - } - - switch (resolvedDeclaration.type) { +export function hasTypeForDeclaration(declaration: Declaration): boolean { + switch (declaration.type) { case DeclarationType.BuiltIn: case DeclarationType.Class: case DeclarationType.Function: @@ -177,13 +148,10 @@ export function hasTypeForDeclaration(declaration: Declaration, resolveAliases = return true; case DeclarationType.Parameter: - return !!resolvedDeclaration.node.typeAnnotation; + return !!declaration.node.typeAnnotation; case DeclarationType.Variable: - return !!resolvedDeclaration.typeAnnotationNode; - - case DeclarationType.Module: - return true; + return !!declaration.typeAnnotationNode; case DeclarationType.Alias: return false; diff --git a/server/src/analyzer/symbol.ts b/server/src/analyzer/symbol.ts index 20f5f981e..b75d6dadc 100644 --- a/server/src/analyzer/symbol.ts +++ b/server/src/analyzer/symbol.ts @@ -160,7 +160,7 @@ export class Symbol { getTypedDeclarations() { return this.getDeclarations().filter( - decl => hasTypeForDeclaration(decl, false)); + decl => hasTypeForDeclaration(decl)); } } diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 8f5aa13e6..20ab15164 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -30,8 +30,7 @@ import { AssertNode, AssignmentExpressionNode, AssignmentNode, AugmentedAssignme import { KeywordType } from '../parser/tokenizerTypes'; import { AnalyzerFileInfo } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; -import { AliasDeclaration, Declaration, DeclarationType, ModuleDeclaration, - VariableDeclaration } from './declaration'; +import { AliasDeclaration, Declaration, DeclarationType, VariableDeclaration } from './declaration'; import * as DeclarationUtils from './declarationUtils'; import { EvaluatorFlags, ExpressionEvaluator } from './expressionEvaluator'; import { ImportResult, ImportType } from './importResult'; @@ -1323,16 +1322,9 @@ export class TypeAnalyzer extends ParseTreeWalker { importInfo, implicitImport.path); if (implicitModuleType) { if (!ModuleType.getField(moduleType!, implicitImport.name)) { - const moduleDeclaration: ModuleDeclaration = { - type: DeclarationType.Module, - moduleType: implicitModuleType, - path: implicitImport.path, - range: getEmptyRange() - }; const aliasDeclaration: AliasDeclaration = { type: DeclarationType.Alias, - resolvedDeclarations: [moduleDeclaration], - path: '', + path: implicitImport.path, range: getEmptyRange() }; @@ -1345,15 +1337,14 @@ export class TypeAnalyzer extends ParseTreeWalker { } }); - const moduleDeclaration: ModuleDeclaration = { - type: DeclarationType.Module, - moduleType, + const aliasDeclaration: AliasDeclaration = { + type: DeclarationType.Alias, path: resolvedPath, range: getEmptyRange() }; if (node.alias) { - this._assignTypeToNameNode(node.alias, moduleType, moduleDeclaration); + this._assignTypeToNameNode(node.alias, moduleType, aliasDeclaration); this._updateExpressionTypeForNode(node.alias, moduleType); this._conditionallyReportUnusedName(node.alias, false, @@ -1362,7 +1353,7 @@ export class TypeAnalyzer extends ParseTreeWalker { `Import '${ node.alias.nameToken.value }' is not accessed`); } else { this._bindMultiPartModuleNameToType(node.module.nameParts, - moduleType, moduleDeclaration); + moduleType, aliasDeclaration); } // Cache the module type for subsequent passes. @@ -1426,7 +1417,7 @@ export class TypeAnalyzer extends ParseTreeWalker { const name = importAs.name.nameToken.value; const aliasNode = importAs.alias || importAs.name; let symbolType: Type | undefined; - let declarations: Declaration[] | undefined; + let declaration: AliasDeclaration | undefined; // Is the name referring to an implicit import? const implicitImport = importInfo.implicitImports.find(impImport => impImport.name === name); @@ -1434,12 +1425,11 @@ export class TypeAnalyzer extends ParseTreeWalker { const moduleType = this._getModuleTypeForImportPath(importInfo, implicitImport.path); if (moduleType && this._fileInfo.importLookup(implicitImport.path)) { symbolType = moduleType; - declarations = [{ - type: DeclarationType.Module, - moduleType, + declaration = { + type: DeclarationType.Alias, path: implicitImport.path, range: getEmptyRange() - }]; + }; } } else { const moduleType = this._getModuleTypeForImportPath(importInfo, resolvedPath); @@ -1450,7 +1440,12 @@ export class TypeAnalyzer extends ParseTreeWalker { // will have no declarations. if (symbol && symbol.hasDeclarations()) { symbolType = TypeUtils.getEffectiveTypeOfSymbol(symbol); - declarations = symbol.getDeclarations(); + declaration = { + type: DeclarationType.Alias, + path: importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1], + symbolName: importAs.name.nameToken.value, + range: getEmptyRange() + }; } else { this._addError( `'${ importAs.name.nameToken.value }' is unknown import symbol`, @@ -1469,18 +1464,7 @@ export class TypeAnalyzer extends ParseTreeWalker { this._updateExpressionTypeForNode(importAs.alias, symbolType); } - let aliasDeclaration: AliasDeclaration | undefined; - if (declarations) { - aliasDeclaration = { - type: DeclarationType.Alias, - resolvedDeclarations: declarations, - symbolName: name, - path: '', - range: getEmptyRange() - }; - } - - this._assignTypeToNameNode(aliasNode, symbolType, aliasDeclaration); + this._assignTypeToNameNode(aliasNode, symbolType, declaration); // Python files generated by protoc ("_pb2.py" files) contain // unused imports. Don't report these because they're in generated @@ -2076,16 +2060,16 @@ export class TypeAnalyzer extends ParseTreeWalker { const declarations = DeclarationUtils.getDeclarationsForNameNode(node); - let primaryDeclaration = declarations && declarations.length > 0 ? + const primaryDeclaration = declarations && declarations.length > 0 ? declarations[0] : undefined; if (!primaryDeclaration || primaryDeclaration.node === node) { return; } - primaryDeclaration = DeclarationUtils.resolveDeclarationAliases(primaryDeclaration); - if (!primaryDeclaration || primaryDeclaration.node === node) { - return; - } + // primaryDeclaration = DeclarationUtils.resolveDeclarationAliases(primaryDeclaration); + // if (!primaryDeclaration || primaryDeclaration.node === node) { + // return; + // } let classOrModuleNode: ClassNode | ModuleNode | undefined; if (primaryDeclaration.node) { diff --git a/server/src/common/pathUtils.ts b/server/src/common/pathUtils.ts index 4a62eb903..4c673784f 100644 --- a/server/src/common/pathUtils.ts +++ b/server/src/common/pathUtils.ts @@ -339,7 +339,7 @@ export function getWildcardRoot(rootPath: string, fileSpec: string): string { export function getFileSpec(rootPath: string, fileSpec: string): FileSpec { let regExPattern = getWildcardRegexPattern(rootPath, fileSpec); - let escapedSeparator = path.sep === '/' ? '/' : '\\\\'; + const escapedSeparator = path.sep === '/' ? '/' : '\\\\'; regExPattern = `^(${ regExPattern })($|${ escapedSeparator })`; const regExp = new RegExp(regExPattern); diff --git a/server/src/languageService/completionProvider.ts b/server/src/languageService/completionProvider.ts index 2eef928e4..3bab8216f 100644 --- a/server/src/languageService/completionProvider.ts +++ b/server/src/languageService/completionProvider.ts @@ -666,7 +666,7 @@ export class CompletionProvider { typeDetail = 'class ' + name + '()'; break; - case DeclarationType.Module: + case DeclarationType.Alias: default: typeDetail = name; break; @@ -859,15 +859,11 @@ export class CompletionProvider { case DeclarationType.Class: return CompletionItemKind.Class; - case DeclarationType.Module: - return CompletionItemKind.Module; - case DeclarationType.Alias: - if (declaration.resolvedDeclarations) { - return this._convertDeclarationTypeToItemKind( - declaration.resolvedDeclarations[0], type); + if (declaration.symbolName) { + // TODO - need to resolve the alias } - return CompletionItemKind.Variable; + return CompletionItemKind.Module; } } diff --git a/server/src/languageService/definitionProvider.ts b/server/src/languageService/definitionProvider.ts index 010050223..8de28204c 100644 --- a/server/src/languageService/definitionProvider.ts +++ b/server/src/languageService/definitionProvider.ts @@ -12,7 +12,6 @@ import * as AnalyzerNodeInfo from '../analyzer/analyzerNodeInfo'; import { Declaration } from '../analyzer/declaration'; -import { resolveDeclarationAliases } from '../analyzer/declarationUtils'; import * as ParseTreeUtils from '../analyzer/parseTreeUtils'; import { Symbol } from '../analyzer/symbol'; import { ModuleType, TypeCategory } from '../analyzer/types'; @@ -121,7 +120,8 @@ export class DefinitionProvider { declarations: Declaration[]) { declarations.forEach(decl => { - const resolvedDecl = resolveDeclarationAliases(decl); + // TODO - need to resolve alias declarations + const resolvedDecl = decl; //resolveDeclarationAliases(decl); if (resolvedDecl && resolvedDecl.path) { definitions.push({ path: resolvedDecl.path, diff --git a/server/src/languageService/documentSymbolProvider.ts b/server/src/languageService/documentSymbolProvider.ts index bbdfe3085..5d7c92c95 100644 --- a/server/src/languageService/documentSymbolProvider.ts +++ b/server/src/languageService/documentSymbolProvider.ts @@ -118,7 +118,10 @@ class FindSymbolTreeWalker extends ParseTreeWalker { } break; - case DeclarationType.Module: + case DeclarationType.Alias: + if (declaration.symbolName) { + // TODO - need to look up symbol type + } symbolKind = SymbolKind.Module; break; diff --git a/server/src/languageService/hoverProvider.ts b/server/src/languageService/hoverProvider.ts index 36bc1e601..7d68fb29b 100644 --- a/server/src/languageService/hoverProvider.ts +++ b/server/src/languageService/hoverProvider.ts @@ -72,19 +72,9 @@ export class HoverProvider { private static _addResultsForDeclaration(parts: HoverTextPart[], declaration: Declaration, node: ParseNode): void { - let resolvedDecl: Declaration | undefined = declaration; - while (resolvedDecl && resolvedDecl.type === DeclarationType.Alias) { - resolvedDecl = resolvedDecl.resolvedDeclarations ? - resolvedDecl.resolvedDeclarations[0] : undefined; - } - - if (!resolvedDecl) { - return undefined; - } - - switch (resolvedDecl.type) { + switch (declaration.type) { case DeclarationType.Variable: { - const label = resolvedDecl.isConstant ? 'constant' : 'variable'; + const label = declaration.isConstant ? 'constant' : 'variable'; if (node.nodeType === ParseNodeType.Name) { this._addResultsPart(parts, `(${ label }) ` + node.nameToken.value + this._getTypeText(node), true); @@ -124,7 +114,7 @@ export class HoverProvider { } case DeclarationType.Method: { - const declaredType = DeclarationUtils.getTypeForDeclaration(resolvedDecl); + const declaredType = DeclarationUtils.getTypeForDeclaration(declaration); const label = declaredType && declaredType.category === TypeCategory.Property ? 'property' : 'method'; if (node.nodeType === ParseNodeType.Name) { @@ -136,9 +126,13 @@ export class HoverProvider { break; } - case DeclarationType.Module: { + case DeclarationType.Alias: { + if (declaration.symbolName) { + // TODO - need to resolve alias and present the right type + } if (node.nodeType === ParseNodeType.Name) { - this._addResultsPart(parts, '(module) ' + node.nameToken.value, true); + this._addResultsPart(parts, '(module) ' + node.nameToken.value + + this._getTypeText(node), true); this._addDocumentationPart(parts, node); return; } From 6981dc09a61594beef6ad750e8c365ff3ae92f52 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 11:32:00 -0700 Subject: [PATCH 06/23] Added back alias declaration resolution. --- server/src/analyzer/declarationUtils.ts | 45 +++++++++++++++++++ server/src/analyzer/program.ts | 8 ++-- server/src/analyzer/sourceFile.ts | 12 +++-- server/src/analyzer/typeAnalyzer.ts | 11 ++--- .../src/languageService/completionProvider.ts | 14 +++--- .../src/languageService/definitionProvider.ts | 30 +++++++------ .../languageService/documentSymbolProvider.ts | 20 ++++++--- server/src/languageService/hoverProvider.ts | 38 +++++++++------- 8 files changed, 123 insertions(+), 55 deletions(-) diff --git a/server/src/analyzer/declarationUtils.ts b/server/src/analyzer/declarationUtils.ts index 71d9b6503..b346d51e0 100644 --- a/server/src/analyzer/declarationUtils.ts +++ b/server/src/analyzer/declarationUtils.ts @@ -90,6 +90,51 @@ export function isFunctionOrMethodDeclaration(declaration: Declaration) { return declaration.type === DeclarationType.Method || declaration.type === DeclarationType.Function; } +// If the specified declaration is an alias declaration that points +// to a symbol, it resolves the alias and looks up the symbol, then +// returns the first declaration associated with that symbol. It does +// this recursively if necessary. If a symbol lookup fails, undefined +// is returned. +export function resolveAliasDeclaration(declaration: Declaration, importLookup: ImportLookup): + Declaration | undefined { + + let curDeclaration: Declaration | undefined = declaration; + const alreadyVisited: Declaration[] = []; + + while (true) { + if (curDeclaration.type !== DeclarationType.Alias) { + return curDeclaration; + } + + if (!curDeclaration.symbolName) { + return curDeclaration; + } + + const symbolTable = importLookup(declaration.path); + if (!symbolTable) { + return undefined; + } + + const symbol = symbolTable.get(curDeclaration.symbolName); + if (!symbol) { + return undefined; + } + + const declarations = symbol.getDeclarations(); + if (declarations.length === 0) { + return undefined; + } + + curDeclaration = declarations[0]; + + // Make sure we don't follow a circular list indefinitely. + if (alreadyVisited.find(decl => decl === curDeclaration)) { + return declaration; + } + alreadyVisited.push(curDeclaration); + } +} + export function getTypeForDeclaration(declaration: Declaration): Type | undefined { switch (declaration.type) { case DeclarationType.BuiltIn: diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index 0a00e2a5d..e210c7b2c 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -832,7 +832,7 @@ export class Program { return undefined; } - return sourceFile.getDefinitionsForPosition(position); + return sourceFile.getDefinitionsForPosition(position, this._lookUpImport); } getReferencesForPosition(filePath: string, position: DiagnosticTextPosition, @@ -881,7 +881,8 @@ export class Program { addSymbolsForDocument(filePath: string, symbolList: SymbolInformation[]) { const sourceFileInfo = this._sourceFileMap[filePath]; if (sourceFileInfo) { - sourceFileInfo.sourceFile.addSymbolsForDocument(symbolList); + sourceFileInfo.sourceFile.addSymbolsForDocument( + symbolList, this._lookUpImport); } } @@ -893,7 +894,8 @@ export class Program { } for (const sourceFileInfo of this._sourceFileList) { - sourceFileInfo.sourceFile.addSymbolsForDocument(symbolList, query); + sourceFileInfo.sourceFile.addSymbolsForDocument( + symbolList, this._lookUpImport, query); } } diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index 79e7d29f6..7d340f694 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -502,14 +502,16 @@ export class SourceFile { return true; } - getDefinitionsForPosition(position: DiagnosticTextPosition): DocumentTextRange[] | undefined { + getDefinitionsForPosition(position: DiagnosticTextPosition, importLookup: ImportLookup): + DocumentTextRange[] | undefined { + // If we have no completed analysis job, there's nothing to do. if (!this._parseResults) { return undefined; } return DefinitionProvider.getDefinitionsForPosition( - this._parseResults, position); + this._parseResults, position, importLookup); } getReferencesForPosition(position: DiagnosticTextPosition, includeDeclaration: boolean): @@ -534,14 +536,16 @@ export class SourceFile { this._parseResults, this._filePath, referencesResult, includeDeclaration); } - addSymbolsForDocument(symbolList: SymbolInformation[], query?: string) { + addSymbolsForDocument(symbolList: SymbolInformation[], importLookup: ImportLookup, + query?: string) { + // If we have no completed analysis job, there's nothing to do. if (!this._parseResults) { return; } DocumentSymbolProvider.addSymbolsForDocument(symbolList, query, - this._filePath, this._parseResults); + this._filePath, this._parseResults, importLookup); } getHoverForPosition(position: DiagnosticTextPosition, diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 20ab15164..408114f77 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -2060,16 +2060,17 @@ export class TypeAnalyzer extends ParseTreeWalker { const declarations = DeclarationUtils.getDeclarationsForNameNode(node); - const primaryDeclaration = declarations && declarations.length > 0 ? + let primaryDeclaration = declarations && declarations.length > 0 ? declarations[0] : undefined; if (!primaryDeclaration || primaryDeclaration.node === node) { return; } - // primaryDeclaration = DeclarationUtils.resolveDeclarationAliases(primaryDeclaration); - // if (!primaryDeclaration || primaryDeclaration.node === node) { - // return; - // } + primaryDeclaration = DeclarationUtils.resolveAliasDeclaration(primaryDeclaration, + this._fileInfo.importLookup); + if (!primaryDeclaration || primaryDeclaration.node === node) { + return; + } let classOrModuleNode: ClassNode | ModuleNode | undefined; if (primaryDeclaration.node) { diff --git a/server/src/languageService/completionProvider.ts b/server/src/languageService/completionProvider.ts index 3bab8216f..019cf4dd8 100644 --- a/server/src/languageService/completionProvider.ts +++ b/server/src/languageService/completionProvider.ts @@ -14,7 +14,7 @@ import { CompletionItem, CompletionItemKind, CompletionList, import { ImportLookup } from '../analyzer/analyzerFileInfo'; import * as AnalyzerNodeInfo from '../analyzer/analyzerNodeInfo'; import { Declaration, DeclarationType } from '../analyzer/declaration'; -import { getTypeForDeclaration } from '../analyzer/declarationUtils'; +import { getTypeForDeclaration, resolveAliasDeclaration } from '../analyzer/declarationUtils'; import { ImportedModuleDescriptor, ImportResolver, ModuleNameAndType } from '../analyzer/importResolver'; import { ImportType } from '../analyzer/importResult'; import * as ImportStatementUtils from '../analyzer/importStatementUtils'; @@ -828,7 +828,12 @@ export class CompletionProvider { private _convertDeclarationTypeToItemKind(declaration: Declaration, type?: Type): CompletionItemKind { - switch (declaration.type) { + const resolvedDeclaration = resolveAliasDeclaration(declaration, this._importLookup); + if (!resolvedDeclaration) { + return CompletionItemKind.Variable; + } + + switch (resolvedDeclaration.type) { case DeclarationType.BuiltIn: if (type) { if (type.category === TypeCategory.Class) { @@ -843,7 +848,7 @@ export class CompletionProvider { return CompletionItemKind.Variable; case DeclarationType.Variable: - return declaration.isConstant ? + return resolvedDeclaration.isConstant ? CompletionItemKind.Constant : CompletionItemKind.Variable; @@ -860,9 +865,6 @@ export class CompletionProvider { return CompletionItemKind.Class; case DeclarationType.Alias: - if (declaration.symbolName) { - // TODO - need to resolve the alias - } return CompletionItemKind.Module; } } diff --git a/server/src/languageService/definitionProvider.ts b/server/src/languageService/definitionProvider.ts index 8de28204c..d0f6aa28a 100644 --- a/server/src/languageService/definitionProvider.ts +++ b/server/src/languageService/definitionProvider.ts @@ -10,8 +10,10 @@ * definition is the top of the resolved import file. */ +import { ImportLookup } from '../analyzer/analyzerFileInfo'; import * as AnalyzerNodeInfo from '../analyzer/analyzerNodeInfo'; import { Declaration } from '../analyzer/declaration'; +import { resolveAliasDeclaration } from '../analyzer/declarationUtils'; import * as ParseTreeUtils from '../analyzer/parseTreeUtils'; import { Symbol } from '../analyzer/symbol'; import { ModuleType, TypeCategory } from '../analyzer/types'; @@ -28,7 +30,8 @@ const _startOfFileRange: DiagnosticTextRange = { start: _startOfFilePosition, en export class DefinitionProvider { static getDefinitionsForPosition(parseResults: ParseResults, - position: DiagnosticTextPosition): DocumentTextRange[] | undefined { + position: DiagnosticTextPosition, importLookup: ImportLookup): + DocumentTextRange[] | undefined { const offset = convertPositionToOffset(position, parseResults.tokenizerOutput.lines); if (offset === undefined) { @@ -42,17 +45,17 @@ export class DefinitionProvider { const definitions: DocumentTextRange[] = []; - if (node.nodeType === ParseNodeType.ModuleName) { - this._addDefinitionsForModuleNameNode(definitions, node, offset); - } else if (node.nodeType === ParseNodeType.Name) { + if (node.nodeType === ParseNodeType.Name) { // Is the user hovering over a member name? If so, we need to search // in the scope of that type rather than the current node's scope. if (node.parent && node.parent.nodeType === ParseNodeType.MemberAccess && node === node.parent.memberName) { - this._addDefinitionsForMemberAccessNode(definitions, node.parent); + this._addDefinitionsForMemberAccessNode(definitions, node.parent, importLookup); + } else if (node.parent && node.parent.nodeType === ParseNodeType.ModuleName) { + this._addDefinitionsForModuleNameNode(definitions, node.parent, offset); } else { - this._addDefinitionsForNameNode(definitions, node); + this._addDefinitionsForNameNode(definitions, node, importLookup); } } @@ -60,7 +63,7 @@ export class DefinitionProvider { } private static _addDefinitionsForMemberAccessNode(definitions: DocumentTextRange[], - node: MemberAccessExpressionNode) { + node: MemberAccessExpressionNode, importLookup: ImportLookup) { const baseType = AnalyzerNodeInfo.getExpressionType(node.leftExpression); if (!baseType) { @@ -87,14 +90,16 @@ export class DefinitionProvider { if (symbol) { const declarations = symbol.getDeclarations(); - this._addResultsForDeclarations(definitions, declarations); + this._addResultsForDeclarations(definitions, declarations, importLookup); } return subtype; }); } - private static _addDefinitionsForNameNode(definitions: DocumentTextRange[], node: NameNode) { + private static _addDefinitionsForNameNode(definitions: DocumentTextRange[], + node: NameNode, importLookup: ImportLookup) { + const scopeNode = ParseTreeUtils.getScopeNodeForNode(node); if (!scopeNode) { return; @@ -112,16 +117,15 @@ export class DefinitionProvider { const declarations = symbolWithScope.symbol.getDeclarations(); if (declarations) { - this._addResultsForDeclarations(definitions, declarations); + this._addResultsForDeclarations(definitions, declarations, importLookup); } } private static _addResultsForDeclarations(definitions: DocumentTextRange[], - declarations: Declaration[]) { + declarations: Declaration[], importLookup: ImportLookup) { declarations.forEach(decl => { - // TODO - need to resolve alias declarations - const resolvedDecl = decl; //resolveDeclarationAliases(decl); + const resolvedDecl = resolveAliasDeclaration(decl, importLookup); if (resolvedDecl && resolvedDecl.path) { definitions.push({ path: resolvedDecl.path, diff --git a/server/src/languageService/documentSymbolProvider.ts b/server/src/languageService/documentSymbolProvider.ts index 5d7c92c95..4222678a0 100644 --- a/server/src/languageService/documentSymbolProvider.ts +++ b/server/src/languageService/documentSymbolProvider.ts @@ -11,9 +11,10 @@ import { Location, Position, Range, SymbolInformation, SymbolKind } from 'vscode-languageserver'; import VSCodeUri from 'vscode-uri'; +import { ImportLookup } from '../analyzer/analyzerFileInfo'; import * as AnalyzerNodeInfo from '../analyzer/analyzerNodeInfo'; import { Declaration, DeclarationType } from '../analyzer/declaration'; -import { getTypeForDeclaration } from '../analyzer/declarationUtils'; +import { getTypeForDeclaration, resolveAliasDeclaration } from '../analyzer/declarationUtils'; import * as ParseTreeUtils from '../analyzer/parseTreeUtils'; import { ParseTreeWalker } from '../analyzer/parseTreeWalker'; import { TypeCategory } from '../analyzer/types'; @@ -31,15 +32,18 @@ class FindSymbolTreeWalker extends ParseTreeWalker { private _parseResults: ParseResults; private _symbolResults: SymbolInformation[]; private _query: string | undefined; + private _importLookup: ImportLookup; constructor(filePath: string, parseResults: ParseResults, - results: SymbolInformation[], query: string | undefined) { + results: SymbolInformation[], query: string | undefined, + importLookup: ImportLookup) { super(); this._filePath = filePath; this._parseResults = parseResults; this._symbolResults = results; this._query = query; + this._importLookup = importLookup; } findSymbols() { @@ -99,6 +103,11 @@ class FindSymbolTreeWalker extends ParseTreeWalker { } } + const resolvedSymbol = resolveAliasDeclaration(declaration, this._importLookup); + if (!resolvedSymbol) { + return; + } + let symbolKind: SymbolKind; switch (declaration.type) { case DeclarationType.Class: @@ -119,9 +128,6 @@ class FindSymbolTreeWalker extends ParseTreeWalker { break; case DeclarationType.Alias: - if (declaration.symbolName) { - // TODO - need to look up symbol type - } symbolKind = SymbolKind.Module; break; @@ -176,10 +182,10 @@ class FindSymbolTreeWalker extends ParseTreeWalker { export class DocumentSymbolProvider { static addSymbolsForDocument(symbolList: SymbolInformation[], query: string | undefined, - filePath: string, parseResults: ParseResults) { + filePath: string, parseResults: ParseResults, importLookup: ImportLookup) { const symbolTreeWalker = new FindSymbolTreeWalker(filePath, parseResults, - symbolList, query); + symbolList, query, importLookup); symbolTreeWalker.findSymbols(); } } diff --git a/server/src/languageService/hoverProvider.ts b/server/src/languageService/hoverProvider.ts index 7d68fb29b..fd8f2440b 100644 --- a/server/src/languageService/hoverProvider.ts +++ b/server/src/languageService/hoverProvider.ts @@ -56,13 +56,16 @@ export class HoverProvider { if (node.nodeType === ParseNodeType.Name) { const declarations = DeclarationUtils.getDeclarationsForNameNode(node); if (declarations && declarations.length > 0) { - this._addResultsForDeclaration(results.parts, declarations[0], node); - } - - // If we had no declaration, see if we can provide a minimal tooltip. - if (results.parts.length === 0) { - this._addResultsPart(results.parts, node.nameToken.value + this._getTypeText(node), true); - this._addDocumentationPart(results.parts, node); + this._addResultsForDeclaration(results.parts, declarations[0], node, importLookup); + } else if (!node.parent || node.parent.nodeType !== ParseNodeType.ModuleName) { + // If we had no declaration, see if we can provide a minimal tooltip. We'll skip + // this if it's part of a module name, since a module name part with no declaration + // is a directory (a namespace package), and we don't want to provide any hover + // information in that case. + if (results.parts.length === 0) { + this._addResultsPart(results.parts, node.nameToken.value + this._getTypeText(node), true); + this._addDocumentationPart(results.parts, node); + } } } @@ -70,11 +73,16 @@ export class HoverProvider { } private static _addResultsForDeclaration(parts: HoverTextPart[], - declaration: Declaration, node: ParseNode): void { + declaration: Declaration, node: ParseNode, importLookup: ImportLookup): void { - switch (declaration.type) { + const resolvedDecl = DeclarationUtils.resolveAliasDeclaration(declaration, importLookup); + if (!resolvedDecl) { + return; + } + + switch (resolvedDecl.type) { case DeclarationType.Variable: { - const label = declaration.isConstant ? 'constant' : 'variable'; + const label = resolvedDecl.isConstant ? 'constant' : 'variable'; if (node.nodeType === ParseNodeType.Name) { this._addResultsPart(parts, `(${ label }) ` + node.nameToken.value + this._getTypeText(node), true); @@ -96,7 +104,7 @@ export class HoverProvider { case DeclarationType.Class: { if (node.nodeType === ParseNodeType.Name) { - this._addResultsPart(parts, '(class) ' + this._getTypeText(node), true); + this._addResultsPart(parts, '(class) ' + node.nameToken.value, true); this._addDocumentationPart(parts, node); return; } @@ -114,7 +122,7 @@ export class HoverProvider { } case DeclarationType.Method: { - const declaredType = DeclarationUtils.getTypeForDeclaration(declaration); + const declaredType = DeclarationUtils.getTypeForDeclaration(resolvedDecl); const label = declaredType && declaredType.category === TypeCategory.Property ? 'property' : 'method'; if (node.nodeType === ParseNodeType.Name) { @@ -127,12 +135,8 @@ export class HoverProvider { } case DeclarationType.Alias: { - if (declaration.symbolName) { - // TODO - need to resolve alias and present the right type - } if (node.nodeType === ParseNodeType.Name) { - this._addResultsPart(parts, '(module) ' + node.nameToken.value + - this._getTypeText(node), true); + this._addResultsPart(parts, '(module) ' + node.nameToken.value, true); this._addDocumentationPart(parts, node); return; } From 2dd7ec9e239db0e9fc83cb28869109b1fa7a534e Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 11:38:52 -0700 Subject: [PATCH 07/23] Fixed bug that caused cyclical imports to be sticky. --- server/src/analyzer/sourceFile.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index 7d340f694..66b426fb4 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -673,7 +673,6 @@ export class SourceFile { this._diagnosticVersion++; this._typeAnalysisPassNumber = 1; this._lastReanalysisReason = ''; - this._circularDependencies = []; this._isTypeAnalysisPassNeeded = true; this._isTypeAnalysisFinalized = false; this._isBindingNeeded = false; @@ -722,6 +721,11 @@ export class SourceFile { this._isTypeAnalysisPassNeeded = false; this._typeAnalysisLastPassDiagnostics = diagSink.diagnostics; } + + // Clear any circular dependencies associated with this file. + // These will be detected by the program module and associated + // with the source file right before it is finalized. + this._circularDependencies = []; } // This method should be called once type analysis has completed for From f2daef1c0a7ba7112f7ace230930692d535af62f Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 12:01:30 -0700 Subject: [PATCH 08/23] Removed unnecessary hack in declarationUtils. --- server/src/analyzer/declarationUtils.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/server/src/analyzer/declarationUtils.ts b/server/src/analyzer/declarationUtils.ts index b346d51e0..2546e3e2f 100644 --- a/server/src/analyzer/declarationUtils.ts +++ b/server/src/analyzer/declarationUtils.ts @@ -53,21 +53,6 @@ export function getDeclarationsForNameNode(node: NameNode): Declaration[] | unde return subtype; }); } - } else if (node.parent && node.parent.nodeType === ParseNodeType.ModuleName) { - const namePartIndex = node.parent.nameParts.findIndex(part => part === node); - const importInfo = AnalyzerNodeInfo.getImportInfo(node.parent); - if (namePartIndex >= 0 && importInfo && namePartIndex < importInfo.resolvedPaths.length) { - if (importInfo.resolvedPaths[namePartIndex]) { - // Synthesize an alias declaration for this name part. The only - // time this case is used is for the hover provider. - const aliasDeclaration: AliasDeclaration = { - type: DeclarationType.Alias, - path: importInfo.resolvedPaths[namePartIndex], - range: getEmptyRange() - }; - declarations = [aliasDeclaration]; - } - } } else { const scopeNode = ParseTreeUtils.getScopeNodeForNode(node); if (scopeNode) { From e8925a2c260ed18739c23adcabde8af84243b945 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 12:01:58 -0700 Subject: [PATCH 09/23] Added code to ensure that dependent files are parsed before binding. --- server/src/analyzer/program.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index e210c7b2c..b0d1177ef 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -513,6 +513,11 @@ export class Program { this._parseFile(fileToAnalyze, options, importResolver); + // Parse any other files that this file depends upon. + fileToAnalyze.imports.forEach(importedFile => { + this._parseFile(importedFile, options, importResolver); + }); + // We need to parse and bind the builtins import first. let builtinsScope: Scope | undefined; if (fileToAnalyze.builtinsImport) { From 800e0604037412cb9b585367a170812e319bbf9c Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 12:02:22 -0700 Subject: [PATCH 10/23] Added constainsWildcardImport flag as output of parser. --- server/src/analyzer/sourceFile.ts | 10 +++++++++- server/src/parser/parser.ts | 6 +++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index 66b426fb4..9b010e6ec 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -327,6 +327,13 @@ export class SourceFile { this._parseTreeNeedsCleaning = true; this._isTypeAnalysisFinalized = false; this._isTypeAnalysisPassNeeded = true; + + // If the file continas a wildcard import, we need to rebind + // also because the dependent import may have changed. + if (this._parseResults && this._parseResults.containsWildcardImport) { + this._isBindingNeeded = true; + this._moduleSymbolTable = undefined; + } } setClientVersion(version: number | null, contents: string): void { @@ -481,7 +488,8 @@ export class SourceFile { typeIgnoreLines: {}, predominantEndOfLineSequence: '\n', predominantTabSequence: ' ' - } + }, + containsWildcardImport: false }; this._imports = undefined; this._builtinsImport = undefined; diff --git a/server/src/parser/parser.ts b/server/src/parser/parser.ts index bbc796ff6..176d84945 100644 --- a/server/src/parser/parser.ts +++ b/server/src/parser/parser.ts @@ -62,6 +62,7 @@ export interface ParseResults { importedModules: ModuleImport[]; futureImports: StringMap; tokenizerOutput: TokenizerOutput; + containsWildcardImport: boolean; } export interface ParseExpressionTextResults { @@ -92,6 +93,7 @@ export class Parser { private _isParsingTypeAnnotation = false; private _futureImportMap = new StringMap(); private _importedModules: ModuleImport[] = []; + private _containsWildcardImport = false; parseSourceFile(fileContents: string, parseOptions: ParseOptions, diagSink: DiagnosticSink, cancelToken?: CancelToken): ParseResults { @@ -132,7 +134,8 @@ export class Parser { parseTree: moduleNode, importedModules: this._importedModules, futureImports: this._futureImportMap, - tokenizerOutput: this._tokenizerOutput! + tokenizerOutput: this._tokenizerOutput!, + containsWildcardImport: this._containsWildcardImport }; } @@ -979,6 +982,7 @@ export class Parser { if (this._consumeTokenIfOperator(OperatorType.Multiply)) { extendRange(importFromNode, possibleStarToken); importFromNode.isWildcardImport = true; + this._containsWildcardImport = true; } else { const inParen = this._consumeTokenIfType(TokenType.OpenParenthesis); From 964f8a211a3066337696f4518733b4bd7e968d29 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 12:43:39 -0700 Subject: [PATCH 11/23] Revert "Removed unnecessary hack in declarationUtils." This reverts commit f2daef1c0a7ba7112f7ace230930692d535af62f. --- server/src/analyzer/declarationUtils.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/server/src/analyzer/declarationUtils.ts b/server/src/analyzer/declarationUtils.ts index 2546e3e2f..b346d51e0 100644 --- a/server/src/analyzer/declarationUtils.ts +++ b/server/src/analyzer/declarationUtils.ts @@ -53,6 +53,21 @@ export function getDeclarationsForNameNode(node: NameNode): Declaration[] | unde return subtype; }); } + } else if (node.parent && node.parent.nodeType === ParseNodeType.ModuleName) { + const namePartIndex = node.parent.nameParts.findIndex(part => part === node); + const importInfo = AnalyzerNodeInfo.getImportInfo(node.parent); + if (namePartIndex >= 0 && importInfo && namePartIndex < importInfo.resolvedPaths.length) { + if (importInfo.resolvedPaths[namePartIndex]) { + // Synthesize an alias declaration for this name part. The only + // time this case is used is for the hover provider. + const aliasDeclaration: AliasDeclaration = { + type: DeclarationType.Alias, + path: importInfo.resolvedPaths[namePartIndex], + range: getEmptyRange() + }; + declarations = [aliasDeclaration]; + } + } } else { const scopeNode = ParseTreeUtils.getScopeNodeForNode(node); if (scopeNode) { From 41b8053703c1b328f6994e7e5cbba076ba6343f9 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 16:35:30 -0700 Subject: [PATCH 12/23] Started to move alias declaration creation from type analyzer to binder. --- server/src/analyzer/binder.ts | 97 +++++++++++++++++++++++-- server/src/analyzer/declaration.ts | 31 +++++--- server/src/analyzer/declarationUtils.ts | 5 +- server/src/analyzer/typeAnalyzer.ts | 26 ++----- server/src/analyzer/types.ts | 8 ++ 5 files changed, 129 insertions(+), 38 deletions(-) diff --git a/server/src/analyzer/binder.ts b/server/src/analyzer/binder.ts index 91b921331..557adbeed 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, getEmptyRange } from '../common/diagnostic'; +import { CreateTypeStubFileAction, getEmptyPosition, getEmptyRange } from '../common/diagnostic'; import { DiagnosticRule } from '../common/diagnosticRules'; import { convertOffsetsToRange } from '../common/positionUtils'; import { PythonVersion } from '../common/pythonVersion'; @@ -36,9 +36,9 @@ import * as StringTokenUtils from '../parser/stringTokenUtils'; import { StringTokenFlags } from '../parser/tokenizerTypes'; import { AnalyzerFileInfo } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; -import { DeclarationType } from './declaration'; +import { AliasDeclaration, DeclarationType, ModuleLoaderActions } from './declaration'; import * as DocStringUtils from './docStringUtils'; -import { ImportType } from './importResult'; +import { ImportResult, ImportType } from './importResult'; import { defaultTypeSourceId, TypeSourceId } from './inferredType'; import * as ParseTreeUtils from './parseTreeUtils'; import { ParseTreeWalker } from './parseTreeWalker'; @@ -593,10 +593,79 @@ export abstract class Binder extends ParseTreeWalker { } visitImportAs(node: ImportAsNode): boolean { - if (node.alias) { - this._bindNameToScope(this._currentScope, node.alias.nameToken.value); - } else if (node.module.nameParts.length > 0) { - this._bindNameToScope(this._currentScope, node.module.nameParts[0].nameToken.value); + if (node.module.nameParts.length > 0) { + 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; + } + + if (symbolName) { + const symbol = this._bindNameToScope(this._currentScope, symbolName); + + const importInfo = AnalyzerNodeInfo.getImportInfo(node.module); + assert(importInfo !== undefined); + + if (importInfo && importInfo.isImportFound && importInfo.resolvedPaths.length > 0 && symbol) { + const resolvedPath = importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1]; + const existingDecl = symbol.getDeclarations().find( + decl => decl.type === DeclarationType.Alias && decl.path === resolvedPath); + + const newDecl: AliasDeclaration = existingDecl as AliasDeclaration || { + type: DeclarationType.Alias, + path: '', + range: getEmptyRange(), + 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]; + this._addImplicitImportsToLoaderActions(importInfo, newDecl); + } else { + // Fill in the remaining name parts. + let curLoaderActions: ModuleLoaderActions = newDecl; + + for (let i = 1; i < node.module.nameParts.length; i++) { + if (i >= importInfo.resolvedPaths.length) { + break; + } + + const namePartValue = node.module.nameParts[i].nameToken.value; + + // Is there an existing loader action for this name? + let loaderActions = curLoaderActions.implicitImports.get(namePartValue); + if (!loaderActions) { + // Allocate a new loader action. + loaderActions = { + path: '', + implicitImports: new Map() + }; + curLoaderActions.implicitImports.set(namePartValue, loaderActions); + } + + // If this is the last name part we're resolving, add in the + // implicit imports as well. + if (i === node.module.nameParts.length - 1) { + loaderActions.path = importInfo.resolvedPaths[i]; + this._addImplicitImportsToLoaderActions(importInfo, loaderActions); + } + + curLoaderActions = loaderActions; + } + } + + if (!existingDecl) { + symbol.addDeclaration(newDecl); + } + } + } } return true; @@ -775,6 +844,20 @@ export abstract class Binder extends ParseTreeWalker { }); } + private _addImplicitImportsToLoaderActions(importResult: ImportResult, loaderActions: ModuleLoaderActions) { + importResult.implicitImports.forEach(implicitImport => { + const existingLoaderAction = loaderActions.implicitImports.get(implicitImport.name); + if (existingLoaderAction) { + existingLoaderAction.path = implicitImport.path; + } else { + loaderActions.implicitImports.set(implicitImport.name, { + path: implicitImport.path, + implicitImports: new Map() + }); + } + }); + } + // Handles some special-case assignment statements that are found // within the typings.pyi file. private _handleTypingStubAssignment(node: AssignmentNode) { diff --git a/server/src/analyzer/declaration.ts b/server/src/analyzer/declaration.ts index 0338b02d7..f1bbc951d 100644 --- a/server/src/analyzer/declaration.ts +++ b/server/src/analyzer/declaration.ts @@ -12,7 +12,7 @@ import { DiagnosticTextRange } from '../common/diagnostic'; import { ClassNode, ExpressionNode, FunctionNode, NameNode, ParameterNode, ParseNode, StringListNode } from '../parser/parseNodes'; -import { ModuleType, Type } from './types'; +import { Type } from './types'; export const enum DeclarationType { BuiltIn, @@ -71,19 +71,30 @@ export interface VariableDeclaration extends DeclarationBase { // Alias declarations are used for imports. They are resolved // after the binding phase. -export interface AliasDeclaration extends DeclarationBase { +export interface AliasDeclaration extends DeclarationBase, ModuleLoaderActions { type: DeclarationType.Alias; - // If a symbol is present, the alias refers to the symbol - // within a module (whose path is defined in the 'path' - // field). If symbolName is missing, the alias refers to - // the module itself. + // If a symbolName is defined, the alias refers to a symbol + // within a resolved module (whose path is defined in the + // 'path' field). If symbolName is missing, the alias refers + // to the module itself. symbolName?: string; +} - // If there is no symbol specified and the entire module - // is referenced, should the module type include the - // implicit imports within its namespace? - includeImplicitImports?: boolean; +// This interface represents a set of actions that the python loader +// performs when a module import is encountered. +export interface ModuleLoaderActions { + // The resolved path of the implicit import. This can be empty + // if the resolved path doesn't reference a module (e.g. it's + // 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). + implicitImports: Map; } export type Declaration = BuiltInDeclaration | ClassDeclaration | diff --git a/server/src/analyzer/declarationUtils.ts b/server/src/analyzer/declarationUtils.ts index b346d51e0..c27eb8fb7 100644 --- a/server/src/analyzer/declarationUtils.ts +++ b/server/src/analyzer/declarationUtils.ts @@ -13,7 +13,7 @@ import { getEmptyRange } from '../common/diagnostic'; import { NameNode, ParseNode, ParseNodeType } from '../parser/parseNodes'; import { ImportLookup } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; -import { AliasDeclaration, Declaration, DeclarationType } from './declaration'; +import { AliasDeclaration, Declaration, DeclarationType, ModuleLoaderActions } from './declaration'; import * as ParseTreeUtils from './parseTreeUtils'; import { Symbol } from './symbol'; import { ClassType, ModuleType, ObjectType, Type, TypeCategory } from './types'; @@ -63,7 +63,8 @@ export function getDeclarationsForNameNode(node: NameNode): Declaration[] | unde const aliasDeclaration: AliasDeclaration = { type: DeclarationType.Alias, path: importInfo.resolvedPaths[namePartIndex], - range: getEmptyRange() + range: getEmptyRange(), + implicitImports: new Map() }; declarations = [aliasDeclaration]; } diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 408114f77..8e639f465 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -30,7 +30,7 @@ import { AssertNode, AssignmentExpressionNode, AssignmentNode, AugmentedAssignme import { KeywordType } from '../parser/tokenizerTypes'; import { AnalyzerFileInfo } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; -import { AliasDeclaration, Declaration, DeclarationType, VariableDeclaration } from './declaration'; +import { AliasDeclaration, Declaration, DeclarationType, ModuleLoaderActions, VariableDeclaration } from './declaration'; import * as DeclarationUtils from './declarationUtils'; import { EvaluatorFlags, ExpressionEvaluator } from './expressionEvaluator'; import { ImportResult, ImportType } from './importResult'; @@ -1322,29 +1322,16 @@ export class TypeAnalyzer extends ParseTreeWalker { importInfo, implicitImport.path); if (implicitModuleType) { if (!ModuleType.getField(moduleType!, implicitImport.name)) { - const aliasDeclaration: AliasDeclaration = { - type: DeclarationType.Alias, - path: implicitImport.path, - range: getEmptyRange() - }; - const newSymbol = Symbol.createWithType( SymbolFlags.ClassMember, implicitModuleType, defaultTypeSourceId); - newSymbol.addDeclaration(aliasDeclaration); setSymbolPreservingAccess(moduleType!.loaderFields!, implicitImport.name, newSymbol); } } }); - const aliasDeclaration: AliasDeclaration = { - type: DeclarationType.Alias, - path: resolvedPath, - range: getEmptyRange() - }; - if (node.alias) { - this._assignTypeToNameNode(node.alias, moduleType, aliasDeclaration); + this._assignTypeToNameNode(node.alias, moduleType); this._updateExpressionTypeForNode(node.alias, moduleType); this._conditionallyReportUnusedName(node.alias, false, @@ -1352,8 +1339,7 @@ export class TypeAnalyzer extends ParseTreeWalker { DiagnosticRule.reportUnusedImport, `Import '${ node.alias.nameToken.value }' is not accessed`); } else { - this._bindMultiPartModuleNameToType(node.module.nameParts, - moduleType, aliasDeclaration); + this._bindMultiPartModuleNameToType(node.module.nameParts, moduleType); } // Cache the module type for subsequent passes. @@ -1428,7 +1414,8 @@ export class TypeAnalyzer extends ParseTreeWalker { declaration = { type: DeclarationType.Alias, path: implicitImport.path, - range: getEmptyRange() + range: getEmptyRange(), + implicitImports: new Map() }; } } else { @@ -1444,7 +1431,8 @@ export class TypeAnalyzer extends ParseTreeWalker { type: DeclarationType.Alias, path: importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1], symbolName: importAs.name.nameToken.value, - range: getEmptyRange() + range: getEmptyRange(), + implicitImports: new Map() }; } else { this._addError( diff --git a/server/src/analyzer/types.ts b/server/src/analyzer/types.ts index 94299942c..cbd3e7606 100644 --- a/server/src/analyzer/types.ts +++ b/server/src/analyzer/types.ts @@ -1217,6 +1217,14 @@ export function isTypeSame(type1: Type, type2: Type, recursionCount = 0): boolea return true; } + + case TypeCategory.Module: { + const type2Module = type2 as ModuleType; + + // Module types are the same if they share the same + // module symbol table. + return (type1.fields === type2Module.fields); + } } return true; From 1d107048d513b14c5a31c06b817e5515e5ede687 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 17:42:24 -0700 Subject: [PATCH 13/23] Fixed bug in completion provider that resulted in docStrings not to be shown. --- .../src/languageService/completionProvider.ts | 70 ++++++++++--------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/server/src/languageService/completionProvider.ts b/server/src/languageService/completionProvider.ts index 019cf4dd8..fe0a6a2d9 100644 --- a/server/src/languageService/completionProvider.ts +++ b/server/src/languageService/completionProvider.ts @@ -640,46 +640,48 @@ export class CompletionProvider { let typeDetail: string | undefined; let documentation: string | undefined; - const declaration = declarations[0]; - const type = getTypeForDeclaration(declaration); - itemKind = this._convertDeclarationTypeToItemKind(declaration, type); + const declaration = resolveAliasDeclaration(declarations[0], this._importLookup); + if (declaration) { + const type = getTypeForDeclaration(declaration); + itemKind = this._convertDeclarationTypeToItemKind(declaration, type); - if (type) { - switch (declaration.type) { - case DeclarationType.BuiltIn: - case DeclarationType.Variable: - case DeclarationType.Parameter: - typeDetail = name + ': ' + printType(type); - break; + if (type) { + switch (declaration.type) { + case DeclarationType.BuiltIn: + case DeclarationType.Variable: + case DeclarationType.Parameter: + typeDetail = name + ': ' + printType(type); + break; - case DeclarationType.Function: - case DeclarationType.Method: - if (type.category === TypeCategory.OverloadedFunction) { - typeDetail = type.overloads.map(overload => - name + printType(overload.type)).join('\n'); - } else { - typeDetail = name + printType(type); - } - break; + case DeclarationType.Function: + case DeclarationType.Method: + if (type.category === TypeCategory.OverloadedFunction) { + typeDetail = type.overloads.map(overload => + name + printType(overload.type)).join('\n'); + } else { + typeDetail = name + printType(type); + } + break; - case DeclarationType.Class: - typeDetail = 'class ' + name + '()'; - break; + case DeclarationType.Class: + typeDetail = 'class ' + name + '()'; + break; - case DeclarationType.Alias: - default: - typeDetail = name; - break; + case DeclarationType.Alias: + default: + typeDetail = name; + break; + } } - } - if (type) { - if (type.category === TypeCategory.Module) { - documentation = type.docString; - } else if (type.category === TypeCategory.Class) { - documentation = ClassType.getDocString(type); - } else if (type.category === TypeCategory.Function) { - documentation = FunctionType.getDocString(type); + if (type) { + if (type.category === TypeCategory.Module) { + documentation = type.docString; + } else if (type.category === TypeCategory.Class) { + documentation = ClassType.getDocString(type); + } else if (type.category === TypeCategory.Function) { + documentation = FunctionType.getDocString(type); + } } } From 0c0dd705d0f5e8da24aff3e75174cfb83286df65 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 18:16:06 -0700 Subject: [PATCH 14/23] Fixed recent regression in completion provider that broke completions within import statements. --- server/src/languageService/completionProvider.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/languageService/completionProvider.ts b/server/src/languageService/completionProvider.ts index fe0a6a2d9..811c57bf2 100644 --- a/server/src/languageService/completionProvider.ts +++ b/server/src/languageService/completionProvider.ts @@ -265,7 +265,9 @@ export class CompletionProvider { if (curNode.nodeType === ParseNodeType.Name) { // Are we within a "from X import Y as Z" statement and // more specifically within the "Y"? - if (curNode.parent && curNode.parent.nodeType === ParseNodeType.ImportFromAs) { + if (curNode.parent && curNode.parent.nodeType === ParseNodeType.ModuleName) { + return this._getImportModuleCompletions(curNode.parent); + } else if (curNode.parent && curNode.parent.nodeType === ParseNodeType.ImportFromAs) { const parentNode = curNode.parent.parent; if (parentNode && parentNode.nodeType === ParseNodeType.ImportFrom) { From baa844938cb928abe21790503a41923134a211b6 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 18:16:54 -0700 Subject: [PATCH 15/23] 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; } } From ab7449163c2811b1bcd2c7119670f0262da9e338 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 18:49:15 -0700 Subject: [PATCH 16/23] Added back unaccessed import warnings. --- server/src/analyzer/typeAnalyzer.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index d04364b72..05b59666a 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -1350,7 +1350,19 @@ export class TypeAnalyzer extends ParseTreeWalker { DiagnosticRule.reportUnusedImport, `Import '${ node.alias.nameToken.value }' is not accessed`); } else { - // TODO - need to add unused import logic + if (symbolWithScope && !symbolWithScope.symbol.isAccessed()) { + const nameParts = node.module.nameParts; + if (nameParts.length > 0) { + const multipartName = nameParts.map(np => np.nameToken.value).join('.'); + const textRange = { start: nameParts[0].start, length: nameParts[0].length }; + this._fileInfo.diagnosticSink.addUnusedCodeWithTextRange( + `'${ multipartName }' is not accessed`, textRange); + + this._addDiagnostic(this._fileInfo.diagnosticSettings.reportUnusedImport, + DiagnosticRule.reportUnusedImport, + `Import '${ multipartName }' is not accessed`, textRange); + } + } } return false; From c721a96bbed4e06470344b1399e63666dbce1865 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 19:23:40 -0700 Subject: [PATCH 17/23] Added back support for module doc strings. --- server/src/analyzer/analyzerFileInfo.ts | 7 ++++- server/src/analyzer/binder.ts | 9 +++++- server/src/analyzer/declarationUtils.ts | 6 ++-- server/src/analyzer/expressionEvaluator.ts | 6 ++-- server/src/analyzer/program.ts | 15 ++++++++-- server/src/analyzer/sourceFile.ts | 8 +++++ server/src/analyzer/typeAnalyzer.ts | 29 +++++++++++-------- .../src/languageService/completionProvider.ts | 24 +++++++++++---- 8 files changed, 76 insertions(+), 28 deletions(-) diff --git a/server/src/analyzer/analyzerFileInfo.ts b/server/src/analyzer/analyzerFileInfo.ts index 42868e630..ecad9cb1e 100644 --- a/server/src/analyzer/analyzerFileInfo.ts +++ b/server/src/analyzer/analyzerFileInfo.ts @@ -16,7 +16,12 @@ import { Scope } from './scope'; import { SymbolTable } from './symbol'; // Maps import paths to the symbol table for the imported module. -export type ImportLookup = (filePath: string) => SymbolTable | undefined; +export type ImportLookup = (filePath: string) => ImportLookupResult | undefined; + +export interface ImportLookupResult { + symbolTable: SymbolTable; + docString?: string; +} export interface AnalyzerFileInfo { importLookup: ImportLookup; diff --git a/server/src/analyzer/binder.ts b/server/src/analyzer/binder.ts index b024f7947..87511d4fc 100644 --- a/server/src/analyzer/binder.ts +++ b/server/src/analyzer/binder.ts @@ -1035,6 +1035,8 @@ export abstract class Binder extends ParseTreeWalker { } export class ModuleScopeBinder extends Binder { + private _moduleDocString?: string; + constructor(node: ModuleNode, fileInfo: AnalyzerFileInfo) { super(node, fileInfo.builtinsScope ? ScopeType.Module : ScopeType.Builtin, fileInfo.builtinsScope, fileInfo); @@ -1057,13 +1059,18 @@ export class ModuleScopeBinder extends Binder { // Associate the module's scope with the module type. const moduleType = ModuleType.create(this._currentScope.getSymbolTable()); - moduleType.docString = this._getDocString((this._scopedNode as ModuleNode).statements); + this._moduleDocString = this._getDocString((this._scopedNode as ModuleNode).statements); + moduleType.docString = this._moduleDocString; AnalyzerNodeInfo.setExpressionType(this._scopedNode, moduleType); } bind() { this.bindDeferred(); } + + getModuleDocString() { + return this._moduleDocString; + } } export class ClassScopeBinder extends Binder { diff --git a/server/src/analyzer/declarationUtils.ts b/server/src/analyzer/declarationUtils.ts index c27eb8fb7..48d1397da 100644 --- a/server/src/analyzer/declarationUtils.ts +++ b/server/src/analyzer/declarationUtils.ts @@ -111,12 +111,12 @@ export function resolveAliasDeclaration(declaration: Declaration, importLookup: return curDeclaration; } - const symbolTable = importLookup(declaration.path); - if (!symbolTable) { + const lookupResult = importLookup(declaration.path); + if (!lookupResult) { return undefined; } - const symbol = symbolTable.get(curDeclaration.symbolName); + const symbol = lookupResult.symbolTable.get(curDeclaration.symbolName); if (!symbol) { return undefined; } diff --git a/server/src/analyzer/expressionEvaluator.ts b/server/src/analyzer/expressionEvaluator.ts index 61eddb823..6afa88969 100644 --- a/server/src/analyzer/expressionEvaluator.ts +++ b/server/src/analyzer/expressionEvaluator.ts @@ -608,12 +608,12 @@ export class ExpressionEvaluator { return undefined; } - const symbolTable = this._fileInfo.importLookup(typingImportPath); - if (!symbolTable) { + const lookupResult = this._fileInfo.importLookup(typingImportPath); + if (!lookupResult) { return undefined; } - const symbol = symbolTable.get(symbolName); + const symbol = lookupResult.symbolTable.get(symbolName); if (!symbol) { return undefined; } diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index b0d1177ef..733feca2f 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -23,6 +23,7 @@ import { Duration, timingStats } from '../common/timing'; import { ModuleSymbolMap } from '../languageService/completionProvider'; import { HoverResults } from '../languageService/hoverProvider'; import { SignatureHelpResults } from '../languageService/signatureHelpProvider'; +import { ImportLookupResult } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; import { CircularDependency } from './circularDependency'; import { ImportResolver } from './importResolver'; @@ -534,13 +535,23 @@ export class Program { fileToAnalyze.sourceFile.bind(options, this._lookUpImport, builtinsScope); } - private _lookUpImport = (filePath: string): SymbolTable | undefined => { + private _lookUpImport = (filePath: string): ImportLookupResult | undefined => { const sourceFileInfo = this._sourceFileMap[filePath]; if (!sourceFileInfo) { return undefined; } - return sourceFileInfo.sourceFile.getModuleSymbolTable(); + const symbolTable = sourceFileInfo.sourceFile.getModuleSymbolTable(); + if (!symbolTable) { + return undefined; + } + + const docString = sourceFileInfo.sourceFile.getModuleDocString(); + + return { + symbolTable, + docString + }; } // Build a map of all modules within this program and the module- diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index 9b010e6ec..3505ba720 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -108,6 +108,7 @@ export class SourceFile { private _parseResults?: ParseResults; private _moduleSymbolTable?: SymbolTable; + private _moduleDocString?: string; // Diagnostics generated during different phases of analysis. private _parseDiagnostics: Diagnostic[] = []; @@ -285,6 +286,10 @@ export class SourceFile { return this._moduleSymbolTable; } + getModuleDocString(): string | undefined { + return this._moduleDocString; + } + // Indicates whether the contents of the file have changed since // the last analysis was performed. didContentsChangeOnDisk(): boolean { @@ -320,6 +325,7 @@ export class SourceFile { this._isTypeAnalysisFinalized = false; this._isTypeAnalysisPassNeeded = true; this._moduleSymbolTable = undefined; + this._moduleDocString = undefined; } markReanalysisRequired(): void { @@ -333,6 +339,7 @@ export class SourceFile { if (this._parseResults && this._parseResults.containsWildcardImport) { this._isBindingNeeded = true; this._moduleSymbolTable = undefined; + this._moduleDocString = undefined; } } @@ -663,6 +670,7 @@ export class SourceFile { const moduleScope = AnalyzerNodeInfo.getScope(this._parseResults!.parseTree); assert(moduleScope !== undefined); this._moduleSymbolTable = moduleScope!.getSymbolTable(); + this._moduleDocString = binder.getModuleDocString(); }); } catch (e) { const message: string = (e.stack ? e.stack.toString() : undefined) || diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 05b59666a..437736f39 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -13,7 +13,7 @@ import * as assert from 'assert'; import { DiagnosticLevel } from '../common/configOptions'; import { AddMissingOptionalToParamAction, Diagnostic, - DiagnosticAddendum, getEmptyRange } from '../common/diagnostic'; + DiagnosticAddendum, DiagnosticTextRange, getEmptyRange } from '../common/diagnostic'; import { DiagnosticRule } from '../common/diagnosticRules'; import { convertOffsetsToRange } from '../common/positionUtils'; import { PythonVersion } from '../common/pythonVersion'; @@ -1300,9 +1300,6 @@ export class TypeAnalyzer extends ParseTreeWalker { return false; } - const importInfo = AnalyzerNodeInfo.getImportInfo(node.module); - assert(importInfo !== undefined); - let symbolNameNode: NameNode; if (node.alias) { // The symbol name is defined by the alias. @@ -1313,6 +1310,7 @@ export class TypeAnalyzer extends ParseTreeWalker { symbolNameNode = node.module.nameParts[0]; } + // Look up the symbol to find the alias declaration. const symbolWithScope = this._currentScope.lookUpSymbolRecursive( symbolNameNode.nameToken.value); assert(symbolWithScope !== undefined); @@ -1354,7 +1352,8 @@ export class TypeAnalyzer extends ParseTreeWalker { const nameParts = node.module.nameParts; if (nameParts.length > 0) { const multipartName = nameParts.map(np => np.nameToken.value).join('.'); - const textRange = { start: nameParts[0].start, length: nameParts[0].length }; + const textRange: TextRange = { start: nameParts[0].start, length: nameParts[0].length }; + TextRange.extend(textRange, nameParts[nameParts.length - 1]); this._fileInfo.diagnosticSink.addUnusedCodeWithTextRange( `'${ multipartName }' is not accessed`, textRange); @@ -1570,9 +1569,10 @@ export class TypeAnalyzer extends ParseTreeWalker { private _applyLoaderActionsToModuleType(moduleType: ModuleType, loaderActions: ModuleLoaderActions) { if (loaderActions.path) { - const moduleSymbolTable = this._fileInfo.importLookup(loaderActions.path); - if (moduleSymbolTable) { - moduleType.fields = moduleSymbolTable; + const lookupResults = this._fileInfo.importLookup(loaderActions.path); + if (lookupResults) { + moduleType.fields = lookupResults.symbolTable; + moduleType.docString = lookupResults.docString; } } @@ -2685,7 +2685,10 @@ export class TypeAnalyzer extends ParseTreeWalker { private _findCollectionsImportSymbolTable(): SymbolTable | undefined { if (this._fileInfo.collectionsModulePath) { - return this._fileInfo.importLookup(this._fileInfo.collectionsModulePath); + const lookupResult = this._fileInfo.importLookup(this._fileInfo.collectionsModulePath); + if (lookupResult) { + return lookupResult.symbolTable; + } } return undefined; @@ -3048,9 +3051,11 @@ export class TypeAnalyzer extends ParseTreeWalker { } } - const symbolTable = this._fileInfo.importLookup(path); - if (symbolTable) { - return ModuleType.create(symbolTable); + const lookupResults = this._fileInfo.importLookup(path); + if (lookupResults) { + const moduleType = ModuleType.create(lookupResults.symbolTable); + moduleType.docString = lookupResults.docString; + return moduleType; } 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 diff --git a/server/src/languageService/completionProvider.ts b/server/src/languageService/completionProvider.ts index 811c57bf2..99894b1af 100644 --- a/server/src/languageService/completionProvider.ts +++ b/server/src/languageService/completionProvider.ts @@ -566,9 +566,9 @@ export class CompletionProvider { const resolvedPath = importInfo.resolvedPaths.length > 0 ? importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1] : ''; - const symbolTable = this._importLookup(resolvedPath); - if (symbolTable) { - this._addSymbolsForSymbolTable(symbolTable, + const lookupResults = this._importLookup(resolvedPath); + if (lookupResults) { + this._addSymbolsForSymbolTable(lookupResults.symbolTable, name => { // Don't suggest symbols that have already been imported. return !importFromNode.imports.find( @@ -665,14 +665,26 @@ export class CompletionProvider { } break; - case DeclarationType.Class: + case DeclarationType.Class: { typeDetail = 'class ' + name + '()'; break; + } - case DeclarationType.Alias: - default: + case DeclarationType.Alias: { + typeDetail = name; + if (declaration.path) { + const lookupResults = this._importLookup(declaration.path); + if (lookupResults) { + documentation = lookupResults.docString; + } + } + break; + } + + default: { typeDetail = name; break; + } } } From e7d4749077da911bdd9ef964503c6d63b441eae8 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 19:47:53 -0700 Subject: [PATCH 18/23] Fixed regression in find references functionality. --- server/src/analyzer/program.ts | 10 ++--- server/src/analyzer/sourceFile.ts | 12 ++--- .../src/languageService/referencesProvider.ts | 44 ++++++++++++++----- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index 733feca2f..1bf7b5455 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -30,7 +30,6 @@ import { ImportResolver } from './importResolver'; import { ImportResult, ImportType } from './importResult'; import { Scope } from './scope'; import { SourceFile } from './sourceFile'; -import { SymbolTable } from './symbol'; import { TypeStubWriter } from './typeStubWriter'; const _maxImportDepth = 256; @@ -868,7 +867,7 @@ export class Program { } const referencesResult = sourceFileInfo.sourceFile.getReferencesForPosition( - position, includeDeclaration); + position, includeDeclaration, this._lookUpImport); if (!referencesResult) { return undefined; @@ -886,7 +885,7 @@ export class Program { } curSourceFileInfo.sourceFile.addReferences(referencesResult, - includeDeclaration); + includeDeclaration, this._lookUpImport); } } } @@ -999,7 +998,7 @@ export class Program { } const referencesResult = sourceFileInfo.sourceFile.getReferencesForPosition( - position, true); + position, true, this._lookUpImport); if (!referencesResult) { return undefined; @@ -1016,7 +1015,8 @@ export class Program { }); } - curSourceFileInfo.sourceFile.addReferences(referencesResult, true); + curSourceFileInfo.sourceFile.addReferences(referencesResult, + true, this._lookUpImport); } } } diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index 3505ba720..cac3cbcc6 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -529,8 +529,8 @@ export class SourceFile { this._parseResults, position, importLookup); } - getReferencesForPosition(position: DiagnosticTextPosition, includeDeclaration: boolean): - ReferencesResult | undefined { + getReferencesForPosition(position: DiagnosticTextPosition, includeDeclaration: boolean, + importLookup: ImportLookup): ReferencesResult | undefined { // If we have no completed analysis job, there's nothing to do. if (!this._parseResults) { @@ -538,17 +538,19 @@ export class SourceFile { } return ReferencesProvider.getReferencesForPosition( - this._parseResults, this._filePath, position, includeDeclaration); + this._parseResults, this._filePath, position, includeDeclaration, importLookup); } - addReferences(referencesResult: ReferencesResult, includeDeclaration: boolean): void { + addReferences(referencesResult: ReferencesResult, includeDeclaration: boolean, + importLookup: ImportLookup): void { + // If we have no completed analysis job, there's nothing to do. if (!this._parseResults) { return; } ReferencesProvider.addReferences( - this._parseResults, this._filePath, referencesResult, includeDeclaration); + this._parseResults, this._filePath, referencesResult, includeDeclaration, importLookup); } addSymbolsForDocument(symbolList: SymbolInformation[], importLookup: ImportLookup, diff --git a/server/src/languageService/referencesProvider.ts b/server/src/languageService/referencesProvider.ts index 37f0f69ad..cc48c5aa4 100644 --- a/server/src/languageService/referencesProvider.ts +++ b/server/src/languageService/referencesProvider.ts @@ -8,11 +8,11 @@ * by a location within a file. */ +import { ImportLookup } from '../analyzer/analyzerFileInfo'; import { Declaration, DeclarationType } from '../analyzer/declaration'; import * as DeclarationUtils from '../analyzer/declarationUtils'; import * as ParseTreeUtils from '../analyzer/parseTreeUtils'; import { ParseTreeWalker } from '../analyzer/parseTreeWalker'; -import { Symbol } from '../analyzer/symbol'; import { DiagnosticTextPosition, DocumentTextRange } from '../common/diagnostic'; import { convertOffsetToPosition, convertPositionToOffset } from '../common/positionUtils'; import { TextRange } from '../common/textRange'; @@ -31,15 +31,18 @@ class FindReferencesTreeWalker extends ParseTreeWalker { private _filePath: string; private _referencesResult: ReferencesResult; private _includeDeclaration: boolean; + private _importLookup: ImportLookup; constructor(parseResults: ParseResults, filePath: string, - referencesResult: ReferencesResult, includeDeclaration: boolean) { + referencesResult: ReferencesResult, includeDeclaration: boolean, + importLookup: ImportLookup) { super(); this._parseResults = parseResults; this._filePath = filePath; this._referencesResult = referencesResult; this._includeDeclaration = includeDeclaration; + this._importLookup = importLookup; } findReferences() { @@ -69,14 +72,22 @@ class FindReferencesTreeWalker extends ParseTreeWalker { } private _resultsContainsDeclaration(declaration: Declaration) { + const resolvedDecl = DeclarationUtils.resolveAliasDeclaration(declaration, this._importLookup); + if (!resolvedDecl) { + return false; + } + + // The reference results declarations are already resolved, so we don't + // need to call resolveAliasDeclaration on them. return this._referencesResult.declarations.some(decl => - DeclarationUtils.areDeclarationsSame(decl, declaration)); + DeclarationUtils.areDeclarationsSame(decl, resolvedDecl)); } } export class ReferencesProvider { static getReferencesForPosition(parseResults: ParseResults, filePath: string, - position: DiagnosticTextPosition, includeDeclaration: boolean): + position: DiagnosticTextPosition, includeDeclaration: boolean, + importLookup: ImportLookup): ReferencesResult | undefined { const offset = convertPositionToOffset(position, parseResults.tokenizerOutput.lines); @@ -94,12 +105,24 @@ export class ReferencesProvider { } const declarations = DeclarationUtils.getDeclarationsForNameNode(node); - if (!declarations || declarations.length === 0) { + if (!declarations) { + return undefined; + } + + const resolvedDeclarations: Declaration[] = []; + declarations.forEach(decl => { + const resovledDecl = DeclarationUtils.resolveAliasDeclaration(decl, importLookup); + if (resovledDecl) { + resolvedDeclarations.push(resovledDecl); + } + }); + + if (resolvedDeclarations.length === 0) { return undefined; } // Is this a type that potentially requires a global search? - const symbolDeclType = declarations[0].type; + const symbolDeclType = resolvedDeclarations[0].type; // Parameters are local to a scope, so they don't require a global search. const requiresGlobalSearch = symbolDeclType !== DeclarationType.Parameter; @@ -107,22 +130,23 @@ export class ReferencesProvider { const results: ReferencesResult = { requiresGlobalSearch, nodeAtOffset: node, - declarations, + declarations: resolvedDeclarations, locations: [] }; const refTreeWalker = new FindReferencesTreeWalker(parseResults, - filePath, results, includeDeclaration); + filePath, results, includeDeclaration, importLookup); refTreeWalker.findReferences(); return results; } static addReferences(parseResults: ParseResults, filePath: string, - referencesResult: ReferencesResult, includeDeclaration: boolean): void { + referencesResult: ReferencesResult, includeDeclaration: boolean, + importLookup: ImportLookup): void { const refTreeWalker = new FindReferencesTreeWalker(parseResults, - filePath, referencesResult, includeDeclaration); + filePath, referencesResult, includeDeclaration, importLookup); refTreeWalker.findReferences(); } } From 11c2c4978f43a87c0763832d299d17f4862c562f Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 20:38:25 -0700 Subject: [PATCH 19/23] Updated dependencies to latest version. --- client/package-lock.json | 24 +++++++++++------------ client/package.json | 4 ++-- package-lock.json | 12 ++++++------ package.json | 4 ++-- server/package-lock.json | 42 ++++++++++++++++++++-------------------- server/package.json | 8 ++++---- 6 files changed, 47 insertions(+), 47 deletions(-) diff --git a/client/package-lock.json b/client/package-lock.json index 67fdc6563..fe125098d 100644 --- a/client/package-lock.json +++ b/client/package-lock.json @@ -5,9 +5,9 @@ "requires": true, "dependencies": { "@types/node": { - "version": "12.7.8", - "resolved": "https://registry.npmjs.org/@types/node/-/node-12.7.8.tgz", - "integrity": "sha512-FMdVn84tJJdV+xe+53sYiZS4R5yn1mAIxfj+DVoNiQjTYz1+OYmjwEZr1ev9nU0axXwda0QDbYl06QHanRVH3A==", + "version": "12.11.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-12.11.1.tgz", + "integrity": "sha512-TJtwsqZ39pqcljJpajeoofYRfeZ7/I/OMUQ5pR4q5wOKf2ocrUvBAZUMhWsOvKx3dVc/aaV5GluBivt0sWqA5A==", "dev": true }, "agent-base": { @@ -199,9 +199,9 @@ } }, "commander": { - "version": "2.20.1", - "resolved": "https://registry.npmjs.org/commander/-/commander-2.20.1.tgz", - "integrity": "sha512-cCuLsMhJeWQ/ZpsFTbE765kvVfoeSddc4nU3up4fV+fDBcfUXnbITJ+JzhkdjzOqhURjZgujxaioam4RM9yGUg==", + "version": "2.20.3", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.20.3.tgz", + "integrity": "sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ==", "dev": true }, "concat-map": { @@ -1016,9 +1016,9 @@ } }, "typescript": { - "version": "3.6.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.6.3.tgz", - "integrity": "sha512-N7bceJL1CtRQ2RiG0AQME13ksR7DiuQh/QehubYcghzv20tnh+MQnQIuJddTmsbqYj+dztchykemz0zFzlvdQw==", + "version": "3.6.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.6.4.tgz", + "integrity": "sha512-unoCll1+l+YK4i4F8f22TaNVPRHcD9PA3yCuZ8g5e0qGqlVlJ/8FSateOLLSagn+Yg5+ZwuPkL8LFUc0Jcvksg==", "dev": true }, "uc.micro": { @@ -1082,9 +1082,9 @@ } }, "vsce": { - "version": "1.66.0", - "resolved": "https://registry.npmjs.org/vsce/-/vsce-1.66.0.tgz", - "integrity": "sha512-Zf4+WD4PhEcOr7jkU08SI9lwFqDhmhk73YOCGQ/tNLaBy+PnnX4eSdqj9LdzDLuI2dsyomJLXzDSNgxuaInxCQ==", + "version": "1.68.0", + "resolved": "https://registry.npmjs.org/vsce/-/vsce-1.68.0.tgz", + "integrity": "sha512-yFbRYu4x4GbdQzZdEQQeRJBxgPdummgcUOFHUtnclW8XQl3MTuKgXL3TtI09gb5oq7jE6kdyvBmpBcmDGsmhcQ==", "dev": true, "requires": { "azure-devops-node-api": "^7.2.0", diff --git a/client/package.json b/client/package.json index ce8332216..f94c1ae1f 100644 --- a/client/package.json +++ b/client/package.json @@ -104,8 +104,8 @@ "postinstall": "node ./node_modules/vscode/bin/install" }, "devDependencies": { - "typescript": "^3.6.3", - "vsce": "^1.66.0", + "typescript": "^3.6.4", + "vsce": "^1.68.0", "vscode": "^1.1.36" }, "dependencies": { diff --git a/package-lock.json b/package-lock.json index daa380e9a..162b7cf5a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,15 +11,15 @@ "dev": true }, "@types/node": { - "version": "11.13.21", - "resolved": "https://registry.npmjs.org/@types/node/-/node-11.13.21.tgz", - "integrity": "sha512-fLwcSjMmDnjfk4FP7/QDiNzXSCEOGNvEe9eA6vaITmC784+Gm70wF7woaFQxUb2CpMjgLBhSPyhH0oIe1JS2uw==", + "version": "11.13.22", + "resolved": "https://registry.npmjs.org/@types/node/-/node-11.13.22.tgz", + "integrity": "sha512-rOsaPRUGTOXbRBOKToy4cgZXY4Y+QSVhxcLwdEveozbk7yuudhWMpxxcaXqYizLMP3VY7OcWCFtx9lGFh5j5kg==", "dev": true }, "typescript": { - "version": "3.6.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.6.3.tgz", - "integrity": "sha512-N7bceJL1CtRQ2RiG0AQME13ksR7DiuQh/QehubYcghzv20tnh+MQnQIuJddTmsbqYj+dztchykemz0zFzlvdQw==", + "version": "3.6.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.6.4.tgz", + "integrity": "sha512-unoCll1+l+YK4i4F8f22TaNVPRHcD9PA3yCuZ8g5e0qGqlVlJ/8FSateOLLSagn+Yg5+ZwuPkL8LFUc0Jcvksg==", "dev": true } } diff --git a/package.json b/package.json index e6c78aa0c..ca56a4569 100644 --- a/package.json +++ b/package.json @@ -24,8 +24,8 @@ }, "devDependencies": { "@types/mocha": "^5.2.7", - "@types/node": "^11.13.21", - "typescript": "^3.6.3" + "@types/node": "^11.13.22", + "typescript": "^3.6.4" }, "main": "index.js", "bin": { diff --git a/server/package-lock.json b/server/package-lock.json index fe50380d8..74802dd74 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -484,9 +484,9 @@ } }, "@types/jest": { - "version": "24.0.18", - "resolved": "https://registry.npmjs.org/@types/jest/-/jest-24.0.18.tgz", - "integrity": "sha512-jcDDXdjTcrQzdN06+TSVsPPqxvsZA/5QkYfIZlq1JMw7FdP5AZylbOc+6B/cuDurctRe+MziUMtQ3xQdrbjqyQ==", + "version": "24.0.19", + "resolved": "https://registry.npmjs.org/@types/jest/-/jest-24.0.19.tgz", + "integrity": "sha512-YYiqfSjocv7lk5H/T+v5MjATYjaTMsUkbDnjGqSMoO88jWdtJXJV4ST/7DKZcoMHMBvB2SeSfyOzZfkxXHR5xg==", "dev": true, "requires": { "@types/jest-diff": "*" @@ -499,9 +499,9 @@ "dev": true }, "@types/node": { - "version": "11.13.21", - "resolved": "https://registry.npmjs.org/@types/node/-/node-11.13.21.tgz", - "integrity": "sha512-fLwcSjMmDnjfk4FP7/QDiNzXSCEOGNvEe9eA6vaITmC784+Gm70wF7woaFQxUb2CpMjgLBhSPyhH0oIe1JS2uw==", + "version": "11.13.22", + "resolved": "https://registry.npmjs.org/@types/node/-/node-11.13.22.tgz", + "integrity": "sha512-rOsaPRUGTOXbRBOKToy4cgZXY4Y+QSVhxcLwdEveozbk7yuudhWMpxxcaXqYizLMP3VY7OcWCFtx9lGFh5j5kg==", "dev": true }, "@types/stack-utils": { @@ -1070,9 +1070,9 @@ "integrity": "sha512-Phlt0plgpIIBOGTT/ehfFnbNlfsDEiqmzE2KRXoX1bLIlir4X/MR+zSyBEkL05ffWgnRSf/DXv+WrUAVr93/ow==" }, "bluebird": { - "version": "3.5.5", - "resolved": "https://registry.npmjs.org/bluebird/-/bluebird-3.5.5.tgz", - "integrity": "sha512-5am6HnnfN+urzt4yfg7IgTbotDjIT/u8AJpEt0sIU9FtXfVeezXAPKswrG+xKUCOYAINpSdgZVDU6QFh+cuH3w==", + "version": "3.7.1", + "resolved": "https://registry.npmjs.org/bluebird/-/bluebird-3.7.1.tgz", + "integrity": "sha512-DdmyoGCleJnkbp3nkbxTLJ18rjDsE4yCggEwKNXkeV123sPNfOCYeDoeuOY+F2FrSjO1YXcTU+dsy96KMy+gcg==", "dev": true }, "bn.js": { @@ -6131,9 +6131,9 @@ "dev": true }, "terser": { - "version": "4.3.4", - "resolved": "https://registry.npmjs.org/terser/-/terser-4.3.4.tgz", - "integrity": "sha512-Kcrn3RiW8NtHBP0ssOAzwa2MsIRQ8lJWiBG/K7JgqPlomA3mtb2DEmp4/hrUA+Jujx+WZ02zqd7GYD+QRBB/2Q==", + "version": "4.3.9", + "resolved": "https://registry.npmjs.org/terser/-/terser-4.3.9.tgz", + "integrity": "sha512-NFGMpHjlzmyOtPL+fDw3G7+6Ueh/sz4mkaUYa4lJCxOPTNzd0Uj0aZJOmsDYoSQyfuVoWDMSWTPU3huyOm2zdA==", "dev": true, "requires": { "commander": "^2.20.0", @@ -6415,9 +6415,9 @@ "dev": true }, "typescript": { - "version": "3.6.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.6.3.tgz", - "integrity": "sha512-N7bceJL1CtRQ2RiG0AQME13ksR7DiuQh/QehubYcghzv20tnh+MQnQIuJddTmsbqYj+dztchykemz0zFzlvdQw==", + "version": "3.6.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.6.4.tgz", + "integrity": "sha512-unoCll1+l+YK4i4F8f22TaNVPRHcD9PA3yCuZ8g5e0qGqlVlJ/8FSateOLLSagn+Yg5+ZwuPkL8LFUc0Jcvksg==", "dev": true }, "typescript-char": { @@ -6780,9 +6780,9 @@ "dev": true }, "webpack": { - "version": "4.41.0", - "resolved": "https://registry.npmjs.org/webpack/-/webpack-4.41.0.tgz", - "integrity": "sha512-yNV98U4r7wX1VJAj5kyMsu36T8RPPQntcb5fJLOsMz/pt/WrKC0Vp1bAlqPLkA1LegSwQwf6P+kAbyhRKVQ72g==", + "version": "4.41.2", + "resolved": "https://registry.npmjs.org/webpack/-/webpack-4.41.2.tgz", + "integrity": "sha512-Zhw69edTGfbz9/8JJoyRQ/pq8FYUoY0diOXqW0T6yhgdhCv6wr0hra5DwwWexNRns2Z2+gsnrNcbe9hbGBgk/A==", "dev": true, "requires": { "@webassemblyjs/ast": "1.8.5", @@ -6989,9 +6989,9 @@ "dev": true }, "yallist": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/yallist/-/yallist-3.1.0.tgz", - "integrity": "sha512-6gpP93MR+VOOehKbCPchro3wFZNSNmek8A2kbkOAZLIZAYx1KP/zAqwO0sOHi3xJEb+UBz8NaYt/17UNit1Q9w==", + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/yallist/-/yallist-3.1.1.tgz", + "integrity": "sha512-a4UGQaWPH59mOXUYnAG2ewncQS4i4F43Tv3JoAM+s2VDAmS9NsK8GpDMLrCHPksFT7h3K6TOoUNn2pb7RoXx4g==", "dev": true }, "yargs": { diff --git a/server/package.json b/server/package.json index 4546ab805..89bb17eb0 100644 --- a/server/package.json +++ b/server/package.json @@ -32,8 +32,8 @@ "@types/chokidar": "^2.1.3", "@types/command-line-args": "^5.0.0", "@types/fs-extra": "^5.1.0", - "@types/jest": "^24.0.18", - "@types/node": "^11.13.21", + "@types/jest": "^24.0.19", + "@types/node": "^11.13.22", "fs-extra": "^7.0.1", "jest": "^24.9.0", "node-loader": "^0.6.0", @@ -41,8 +41,8 @@ "ts-loader": "^5.4.5", "tslint": "^5.20.0", "tslint-microsoft-contrib": "^6.2.0", - "typescript": "^3.6.3", - "webpack": "^4.41.0", + "typescript": "^3.6.4", + "webpack": "^4.41.2", "webpack-cli": "^3.3.9" }, "types": "out/main.d.ts", From bccc03cd1bedaa72d8b79c16cc1f26d6955fd2b4 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 20:40:13 -0700 Subject: [PATCH 20/23] Started to move declaration construction for "import from" statements to binder. --- server/src/analyzer/binder.ts | 210 +++++++++++++++++++--------- server/src/analyzer/typeAnalyzer.ts | 1 - 2 files changed, 142 insertions(+), 69 deletions(-) diff --git a/server/src/analyzer/binder.ts b/server/src/analyzer/binder.ts index 87511d4fc..3b6ed9fec 100644 --- a/server/src/analyzer/binder.ts +++ b/server/src/analyzer/binder.ts @@ -27,18 +27,18 @@ import StringMap from '../common/stringMap'; import { TextRange } from '../common/textRange'; import { AssignmentExpressionNode, AssignmentNode, AugmentedAssignmentExpressionNode, AwaitExpressionNode, ClassNode, DelNode, ExceptNode, ExpressionNode, ForNode, - FunctionNode, GlobalNode, IfNode, ImportAsNode, ImportFromAsNode, LambdaNode, - ListComprehensionNode, ModuleNameNode, ModuleNode, NameNode, NonlocalNode, ParseNode, - ParseNodeArray, ParseNodeType, RaiseNode, StatementNode, StringListNode, SuiteNode, - TryNode, TypeAnnotationExpressionNode, WhileNode, WithNode, YieldExpressionNode, - YieldFromExpressionNode } from '../parser/parseNodes'; + FunctionNode, GlobalNode, IfNode, ImportAsNode, ImportFromNode, LambdaNode, + ListComprehensionNode, ModuleNameNode, ModuleNode, NameNode, NonlocalNode, + ParseNode, ParseNodeArray, ParseNodeType, RaiseNode, StatementNode, StringListNode, + SuiteNode, TryNode, TypeAnnotationExpressionNode, WhileNode, WithNode, + YieldExpressionNode, YieldFromExpressionNode } from '../parser/parseNodes'; import * as StringTokenUtils from '../parser/stringTokenUtils'; import { StringTokenFlags } from '../parser/tokenizerTypes'; import { AnalyzerFileInfo } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; import { AliasDeclaration, DeclarationType, ModuleLoaderActions } from './declaration'; import * as DocStringUtils from './docStringUtils'; -import { ImportResult, ImportType } from './importResult'; +import { ImplicitImport, ImportResult, ImportType } from './importResult'; import { defaultTypeSourceId, TypeSourceId } from './inferredType'; import * as ParseTreeUtils from './parseTreeUtils'; import { ParseTreeWalker } from './parseTreeWalker'; @@ -606,83 +606,157 @@ export abstract class Binder extends ParseTreeWalker { symbolName = firstNamePartValue; } - if (symbolName) { - const symbol = this._bindNameToScope(this._currentScope, symbolName); + const symbol = this._bindNameToScope(this._currentScope, symbolName); - const importInfo = AnalyzerNodeInfo.getImportInfo(node.module); - assert(importInfo !== undefined); + const importInfo = AnalyzerNodeInfo.getImportInfo(node.module); + assert(importInfo !== undefined); - if (importInfo && importInfo.isImportFound && importInfo.resolvedPaths.length > 0 && symbol) { - // 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.firstNamePart === firstNamePartValue); + if (importInfo && importInfo.isImportFound && importInfo.resolvedPaths.length > 0 && symbol) { + // 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.firstNamePart === firstNamePartValue); - const newDecl: AliasDeclaration = existingDecl as AliasDeclaration || { - type: DeclarationType.Alias, - path: '', - range: getEmptyRange(), - firstNamePart: firstNamePartValue, - implicitImports: new Map() - }; + 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 === 1) { - newDecl.path = importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1]; - this._addImplicitImportsToLoaderActions(importInfo, newDecl); - } else { - // Fill in the remaining name parts. - let curLoaderActions: ModuleLoaderActions = newDecl; + // Add the implicit imports for this module if it's the last + // name part we're resolving. + 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. + let curLoaderActions: ModuleLoaderActions = newDecl; - for (let i = 1; i < node.module.nameParts.length; i++) { - if (i >= importInfo.resolvedPaths.length) { - break; - } - - const namePartValue = node.module.nameParts[i].nameToken.value; - - // Is there an existing loader action for this name? - let loaderActions = curLoaderActions.implicitImports.get(namePartValue); - if (!loaderActions) { - // Allocate a new loader action. - loaderActions = { - path: '', - implicitImports: new Map() - }; - curLoaderActions.implicitImports.set(namePartValue, loaderActions); - } - - // If this is the last name part we're resolving, add in the - // implicit imports as well. - if (i === node.module.nameParts.length - 1) { - loaderActions.path = importInfo.resolvedPaths[i]; - this._addImplicitImportsToLoaderActions(importInfo, loaderActions); - } - - curLoaderActions = loaderActions; + for (let i = 1; i < node.module.nameParts.length; i++) { + if (i >= importInfo.resolvedPaths.length) { + break; } - } - if (!existingDecl) { - symbol.addDeclaration(newDecl); + const namePartValue = node.module.nameParts[i].nameToken.value; + + // Is there an existing loader action for this name? + let loaderActions = curLoaderActions.implicitImports.get(namePartValue); + if (!loaderActions) { + // Allocate a new loader action. + loaderActions = { + path: '', + implicitImports: new Map() + }; + curLoaderActions.implicitImports.set(namePartValue, loaderActions); + } + + // If this is the last name part we're resolving, add in the + // implicit imports as well. + if (i === node.module.nameParts.length - 1) { + loaderActions.path = importInfo.resolvedPaths[i]; + this._addImplicitImportsToLoaderActions(importInfo, loaderActions); + } + + curLoaderActions = loaderActions; } } + + if (!existingDecl) { + symbol.addDeclaration(newDecl); + } } } return true; } - visitImportFromAs(node: ImportFromAsNode): boolean { - const nameNode = node.alias || node.name; - this._bindNameToScope(this._currentScope, nameNode.nameToken.value); + visitImportFrom(node: ImportFromNode): boolean { + const importInfo = AnalyzerNodeInfo.getImportInfo(node.module); + + let resolvedPath = ''; + if (importInfo && importInfo.isImportFound) { + resolvedPath = importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1]; + } + + if (node.isWildcardImport) { + if (resolvedPath) { + const lookupInfo = this._fileInfo.importLookup(resolvedPath); + if (lookupInfo) { + lookupInfo.symbolTable.forEach((_, name) => { + const symbol = this._bindNameToScope(this._currentScope, name); + if (symbol) { + const aliasDecl: AliasDeclaration = { + type: DeclarationType.Alias, + path: resolvedPath, + range: getEmptyRange(), + symbolName: name, + implicitImports: new Map() + }; + symbol.addDeclaration(aliasDecl); + } + }); + + // Also add all of the implicitly-imported modules for + // the import module. + if (importInfo && importInfo.implicitImports) { + importInfo.implicitImports.forEach(implicitImport => { + const symbol = this._bindNameToScope(this._currentScope, implicitImport.name); + if (symbol) { + const aliasDecl: AliasDeclaration = { + type: DeclarationType.Alias, + path: implicitImport.path, + range: getEmptyRange(), + implicitImports: new Map() + }; + symbol.addDeclaration(aliasDecl); + } + }); + } + } + } + } else { + node.imports.forEach(importSymbolNode => { + const importedName = importSymbolNode.name.nameToken.value; + const nameNode = importSymbolNode.alias || importSymbolNode.name; + const symbol = this._bindNameToScope(this._currentScope, nameNode.nameToken.value); + + if (symbol && resolvedPath) { + let aliasDecl: AliasDeclaration; + + // Is the import referring to an implicitly-imported module? + let implicitImport: ImplicitImport | undefined; + if (importInfo && importInfo.implicitImports) { + implicitImport = importInfo.implicitImports.find(imp => imp.name === importedName); + } + + if (implicitImport) { + aliasDecl = { + type: DeclarationType.Alias, + path: implicitImport.path, + range: getEmptyRange(), + implicitImports: new Map() + }; + } else { + aliasDecl = { + type: DeclarationType.Alias, + path: resolvedPath, + range: getEmptyRange(), + symbolName: importedName, + implicitImports: new Map() + }; + } + symbol.addDeclaration(aliasDecl); + } + }); + } return true; } diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index 437736f39..bff3648e4 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -1374,7 +1374,6 @@ export class TypeAnalyzer extends ParseTreeWalker { const resolvedPath = importInfo.resolvedPaths.length > 0 ? importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1] : ''; - // Empty list implies "import *" if (node.isWildcardImport) { const moduleType = this._getModuleTypeForImportPath(importInfo, resolvedPath); if (moduleType) { From cb0013cb39bd31c83e5362f24407ec9f1de0144e Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 22:48:51 -0700 Subject: [PATCH 21/23] Finished moving alias declaration creation to binder. --- server/src/analyzer/binder.ts | 49 ++++--- server/src/analyzer/program.ts | 54 ++++---- server/src/analyzer/typeAnalyzer.ts | 203 ++++++++++------------------ 3 files changed, 125 insertions(+), 181 deletions(-) diff --git a/server/src/analyzer/binder.ts b/server/src/analyzer/binder.ts index 3b6ed9fec..fbd1be431 100644 --- a/server/src/analyzer/binder.ts +++ b/server/src/analyzer/binder.ts @@ -687,7 +687,7 @@ export abstract class Binder extends ParseTreeWalker { } if (node.isWildcardImport) { - if (resolvedPath) { + if (importInfo && importInfo.implicitImports) { const lookupInfo = this._fileInfo.importLookup(resolvedPath); if (lookupInfo) { lookupInfo.symbolTable.forEach((_, name) => { @@ -703,24 +703,22 @@ export abstract class Binder extends ParseTreeWalker { symbol.addDeclaration(aliasDecl); } }); - - // Also add all of the implicitly-imported modules for - // the import module. - if (importInfo && importInfo.implicitImports) { - importInfo.implicitImports.forEach(implicitImport => { - const symbol = this._bindNameToScope(this._currentScope, implicitImport.name); - if (symbol) { - const aliasDecl: AliasDeclaration = { - type: DeclarationType.Alias, - path: implicitImport.path, - range: getEmptyRange(), - implicitImports: new Map() - }; - symbol.addDeclaration(aliasDecl); - } - }); - } } + + // Also add all of the implicitly-imported modules for + // the import module. + importInfo.implicitImports.forEach(implicitImport => { + const symbol = this._bindNameToScope(this._currentScope, implicitImport.name); + if (symbol) { + const aliasDecl: AliasDeclaration = { + type: DeclarationType.Alias, + path: implicitImport.path, + range: getEmptyRange(), + implicitImports: new Map() + }; + symbol.addDeclaration(aliasDecl); + } + }); } } else { node.imports.forEach(importSymbolNode => { @@ -728,8 +726,8 @@ export abstract class Binder extends ParseTreeWalker { const nameNode = importSymbolNode.alias || importSymbolNode.name; const symbol = this._bindNameToScope(this._currentScope, nameNode.nameToken.value); - if (symbol && resolvedPath) { - let aliasDecl: AliasDeclaration; + if (symbol) { + let aliasDecl: AliasDeclaration | undefined; // Is the import referring to an implicitly-imported module? let implicitImport: ImplicitImport | undefined; @@ -744,7 +742,7 @@ export abstract class Binder extends ParseTreeWalker { range: getEmptyRange(), implicitImports: new Map() }; - } else { + } else if (resolvedPath) { aliasDecl = { type: DeclarationType.Alias, path: resolvedPath, @@ -753,7 +751,10 @@ export abstract class Binder extends ParseTreeWalker { implicitImports: new Map() }; } - symbol.addDeclaration(aliasDecl); + + if (aliasDecl) { + symbol.addDeclaration(aliasDecl); + } } }); } @@ -1131,11 +1132,7 @@ export class ModuleScopeBinder extends Binder { this._addParentLinks(moduleNode, moduleNode.statements); this.walkMultiple(moduleNode.statements); - // Associate the module's scope with the module type. - const moduleType = ModuleType.create(this._currentScope.getSymbolTable()); this._moduleDocString = this._getDocString((this._scopedNode as ModuleNode).statements); - moduleType.docString = this._moduleDocString; - AnalyzerNodeInfo.setExpressionType(this._scopedNode, moduleType); } bind() { diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index 1bf7b5455..bb2b20ed5 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -505,7 +505,8 @@ export class Program { } private _bindFile(fileToAnalyze: SourceFileInfo, - options: ConfigOptions, importResolver: ImportResolver) { + options: ConfigOptions, importResolver: ImportResolver, + recursionMap: Map = new Map()) { if (!this._isFileNeeded(fileToAnalyze) || !fileToAnalyze.sourceFile.isBindingRequired()) { return; @@ -513,11 +514,6 @@ export class Program { this._parseFile(fileToAnalyze, options, importResolver); - // Parse any other files that this file depends upon. - fileToAnalyze.imports.forEach(importedFile => { - this._parseFile(importedFile, options, importResolver); - }); - // We need to parse and bind the builtins import first. let builtinsScope: Scope | undefined; if (fileToAnalyze.builtinsImport) { @@ -529,6 +525,18 @@ export class Program { builtinsScope = AnalyzerNodeInfo.getScope(parseResults.parseTree); assert(builtinsScope !== undefined); } + + const filePath = fileToAnalyze.sourceFile.getFilePath(); + if (recursionMap.has(filePath)) { + // Avoid infinite recursion for cyclical dependencies. + return; + } + + // Bind any other files that this file depends upon. + recursionMap.set(filePath, true); + fileToAnalyze.imports.forEach(importedFile => { + this._bindFile(importedFile, options, importResolver, recursionMap); + }); } fileToAnalyze.sourceFile.bind(options, this._lookUpImport, builtinsScope); @@ -1188,30 +1196,26 @@ export class Program { const newImportPathMap = new Map(); imports.forEach(importResult => { if (importResult.isImportFound) { - if (!this._isImportAllowed(sourceFileInfo, importResult, importResult.isStubFile)) { - return; - } - - if (importResult.resolvedPaths.length > 0) { - const filePath = importResult.resolvedPaths[ - importResult.resolvedPaths.length - 1]; - if (filePath) { - newImportPathMap.set(filePath, { - isTypeshedFile: !!importResult.isTypeshedFile, - isThirdPartyImport: importResult.importType === ImportType.ThirdParty - }); + if (this._isImportAllowed(sourceFileInfo, importResult, importResult.isStubFile)) { + if (importResult.resolvedPaths.length > 0) { + const filePath = importResult.resolvedPaths[ + importResult.resolvedPaths.length - 1]; + if (filePath) { + newImportPathMap.set(filePath, { + isTypeshedFile: !!importResult.isTypeshedFile, + isThirdPartyImport: importResult.importType === ImportType.ThirdParty + }); + } } } importResult.implicitImports.forEach(implicitImport => { - if (!this._isImportAllowed(sourceFileInfo, importResult, implicitImport.isStubFile)) { - return; + if (this._isImportAllowed(sourceFileInfo, importResult, implicitImport.isStubFile)) { + newImportPathMap.set(implicitImport.path, { + isTypeshedFile: !!importResult.isTypeshedFile, + isThirdPartyImport: importResult.importType === ImportType.ThirdParty + }); } - - newImportPathMap.set(implicitImport.path, { - isTypeshedFile: !!importResult.isTypeshedFile, - isThirdPartyImport: importResult.importType === ImportType.ThirdParty - }); }); } else if (options.verboseOutput) { if (!sourceFileInfo.isTypeshedFile || options.diagnosticSettings.reportTypeshedErrors) { diff --git a/server/src/analyzer/typeAnalyzer.ts b/server/src/analyzer/typeAnalyzer.ts index bff3648e4..6390f6783 100644 --- a/server/src/analyzer/typeAnalyzer.ts +++ b/server/src/analyzer/typeAnalyzer.ts @@ -12,8 +12,7 @@ import * as assert from 'assert'; import { DiagnosticLevel } from '../common/configOptions'; -import { AddMissingOptionalToParamAction, Diagnostic, - DiagnosticAddendum, DiagnosticTextRange, getEmptyRange } from '../common/diagnostic'; +import { AddMissingOptionalToParamAction, Diagnostic, DiagnosticAddendum } from '../common/diagnostic'; import { DiagnosticRule } from '../common/diagnosticRules'; import { convertOffsetsToRange } from '../common/positionUtils'; import { PythonVersion } from '../common/pythonVersion'; @@ -22,15 +21,16 @@ import { AssertNode, AssignmentExpressionNode, AssignmentNode, AugmentedAssignme BinaryExpressionNode, BreakNode, CallExpressionNode, ClassNode, ContinueNode, DecoratorNode, DelNode, ErrorExpressionNode, ExceptNode, ExpressionNode, FormatStringNode, ForNode, FunctionNode, IfNode, ImportAsNode, ImportFromNode, IndexExpressionNode, LambdaNode, - ListComprehensionNode, MemberAccessExpressionNode, ModuleNode, NameNode, ParameterCategory, ParameterNode, - ParseNode, ParseNodeType, RaiseNode, ReturnNode, SliceExpressionNode, StringListNode, - SuiteNode, TernaryExpressionNode, TryNode, TupleExpressionNode, + ListComprehensionNode, MemberAccessExpressionNode, ModuleNode, NameNode, ParameterCategory, + ParameterNode, ParseNode, ParseNodeType, RaiseNode, ReturnNode, SliceExpressionNode, + StringListNode, SuiteNode, TernaryExpressionNode, TryNode, TupleExpressionNode, TypeAnnotationExpressionNode, UnaryExpressionNode, UnpackExpressionNode, WhileNode, WithNode, YieldExpressionNode, YieldFromExpressionNode } from '../parser/parseNodes'; import { KeywordType } from '../parser/tokenizerTypes'; import { AnalyzerFileInfo } from './analyzerFileInfo'; import * as AnalyzerNodeInfo from './analyzerNodeInfo'; -import { AliasDeclaration, Declaration, DeclarationType, ModuleLoaderActions, VariableDeclaration } from './declaration'; +import { AliasDeclaration, Declaration, DeclarationType, ModuleLoaderActions, + VariableDeclaration } from './declaration'; import * as DeclarationUtils from './declarationUtils'; import { EvaluatorFlags, ExpressionEvaluator } from './expressionEvaluator'; import { ImportResult, ImportType } from './importResult'; @@ -1311,17 +1311,9 @@ export class TypeAnalyzer extends ParseTreeWalker { } // Look up the symbol to find the alias declaration. - const symbolWithScope = this._currentScope.lookUpSymbolRecursive( - symbolNameNode.nameToken.value); - assert(symbolWithScope !== undefined); - const aliasDecl = symbolWithScope!.symbol.getDeclarations().find( - decl => decl.type === DeclarationType.Alias); - let symbolType: Type | undefined; - if (aliasDecl && aliasDecl.type === DeclarationType.Alias) { - symbolType = this._getSymbolTypeForAliasDeclaration(aliasDecl); - } - + let symbol: Symbol | undefined; + [symbol, symbolType] = this._getAliasedSymbolTypeForName(symbolNameNode.nameToken.value); if (!symbolType) { symbolType = UnknownType.create(); } @@ -1348,7 +1340,7 @@ export class TypeAnalyzer extends ParseTreeWalker { DiagnosticRule.reportUnusedImport, `Import '${ node.alias.nameToken.value }' is not accessed`); } else { - if (symbolWithScope && !symbolWithScope.symbol.isAccessed()) { + if (symbol && !symbol.isAccessed()) { const nameParts = node.module.nameParts; if (nameParts.length > 0) { const multipartName = nameParts.map(np => np.nameToken.value).join('.'); @@ -1369,77 +1361,44 @@ export class TypeAnalyzer extends ParseTreeWalker { visitImportFrom(node: ImportFromNode): boolean { const importInfo = AnalyzerNodeInfo.getImportInfo(node.module); + let symbol: Symbol | undefined; + let symbolType: Type | undefined; if (importInfo && importInfo.isImportFound) { const resolvedPath = importInfo.resolvedPaths.length > 0 ? importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1] : ''; if (node.isWildcardImport) { - const moduleType = this._getModuleTypeForImportPath(importInfo, resolvedPath); - if (moduleType) { + if (resolvedPath) { // Import the fields in the current permanent scope. - const moduleFields = moduleType.fields; - moduleFields.forEach((boundValue, fieldName) => { - this._addSymbolToPermanentScope(fieldName); - this._addTypeSourceToName(fieldName, TypeUtils.getEffectiveTypeOfSymbol(boundValue), - node.id, boundValue.hasDeclarations() ? boundValue.getDeclarations()[0] : undefined); - }); + const lookupInfo = this._fileInfo.importLookup(resolvedPath); + if (lookupInfo) { + lookupInfo.symbolTable.forEach((_, name) => { + [symbol, symbolType] = this._getAliasedSymbolTypeForName(name); + if (symbol) { + this._addTypeSourceToName(name, symbolType || UnknownType.create(), + node.id); + } + }); + } - // Import the fields in the current permanent scope. importInfo.implicitImports.forEach(implicitImport => { - const moduleType = this._getModuleTypeForImportPath(importInfo, resolvedPath); - if (moduleType) { - this._addSymbolToPermanentScope(implicitImport.name); - this._addTypeSourceToName(implicitImport.name, moduleType, node.id); + [symbol, symbolType] = this._getAliasedSymbolTypeForName(implicitImport.name); + if (symbol) { + this._addTypeSourceToName(implicitImport.name, + symbolType || UnknownType.create(), node.id); } }); } } else { node.imports.forEach(importAs => { - const name = importAs.name.nameToken.value; const aliasNode = importAs.alias || importAs.name; - let symbolType: Type | undefined; - let declaration: AliasDeclaration | undefined; - - // Is the name referring to an implicit import? - const implicitImport = importInfo.implicitImports.find(impImport => impImport.name === name); - if (implicitImport) { - const moduleType = this._getModuleTypeForImportPath(importInfo, implicitImport.path); - if (moduleType && this._fileInfo.importLookup(implicitImport.path)) { - symbolType = moduleType; - declaration = { - type: DeclarationType.Alias, - path: implicitImport.path, - range: getEmptyRange(), - implicitImports: new Map() - }; - } - } else { - const moduleType = this._getModuleTypeForImportPath(importInfo, resolvedPath); - if (moduleType) { - const symbol = ModuleType.getField(moduleType, name); - - // For imports of the form "from . import X", the symbol - // will have no declarations. - if (symbol && symbol.hasDeclarations()) { - symbolType = TypeUtils.getEffectiveTypeOfSymbol(symbol); - declaration = { - type: DeclarationType.Alias, - path: importInfo.resolvedPaths[importInfo.resolvedPaths.length - 1], - symbolName: importAs.name.nameToken.value, - range: getEmptyRange(), - implicitImports: new Map() - }; - } else { - this._addError( - `'${ importAs.name.nameToken.value }' is unknown import symbol`, - importAs.name - ); - } - } - } - + [symbol, symbolType] = this._getAliasedSymbolTypeForName(aliasNode.nameToken.value); if (!symbolType) { + this._addError( + `'${ importAs.name.nameToken.value }' is unknown import symbol`, + importAs.name + ); symbolType = UnknownType.create(); } @@ -1447,42 +1406,27 @@ export class TypeAnalyzer extends ParseTreeWalker { if (importAs.alias) { this._updateExpressionTypeForNode(importAs.alias, symbolType); } - - this._assignTypeToNameNode(aliasNode, symbolType, declaration); - - // Python files generated by protoc ("_pb2.py" files) contain - // unused imports. Don't report these because they're in generated - // files that shouldn't be edited. - if (importInfo.importName !== '__future__' && - !this._fileInfo.filePath.endsWith('_pb2.py')) { - - this._conditionallyReportUnusedName(aliasNode, false, - this._fileInfo.diagnosticSettings.reportUnusedImport, - DiagnosticRule.reportUnusedImport, - `Import '${ aliasNode.nameToken.value }' is not accessed`); - } + this._addTypeSourceToName(aliasNode.nameToken.value, symbolType, node.id); + this._assignTypeToNameNode(aliasNode, symbolType); }); } - } else { - // We were unable to resolve the import. Bind the names (or aliases) - // to an unknown type. - if (!node.isWildcardImport) { - node.imports.forEach(importAs => { - const aliasNode = importAs.alias || importAs.name; - const symbolType = UnknownType.create(); + } - this._updateExpressionTypeForNode(importAs.name, symbolType); - if (importAs.alias) { - this._updateExpressionTypeForNode(importAs.name, symbolType); - } + if (!node.isWildcardImport) { + node.imports.forEach(importAs => { + const aliasNode = importAs.alias || importAs.name; + // Python files generated by protoc ("_pb2.py" files) contain + // unused imports. Don't report these because they're in generated + // files that shouldn't be edited. + if ((!importInfo || importInfo.importName !== '__future__') && + !this._fileInfo.filePath.endsWith('_pb2.py')) { - this._assignTypeToNameNode(aliasNode, symbolType); this._conditionallyReportUnusedName(aliasNode, false, this._fileInfo.diagnosticSettings.reportUnusedImport, DiagnosticRule.reportUnusedImport, `Import '${ aliasNode.nameToken.value }' is not accessed`); - }); - } + } + }); } return false; @@ -1555,15 +1499,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); + private _getAliasedSymbolTypeForName(name: string): [Symbol | undefined, Type | undefined] { + const symbolWithScope = this._currentScope.lookUpSymbolRecursive(name); + if (!symbolWithScope) { + return [undefined, 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; + const aliasDecl = symbolWithScope.symbol.getDeclarations().find( + decl => decl.type === DeclarationType.Alias); + + let symbolType: Type | undefined; + if (aliasDecl && aliasDecl.type === DeclarationType.Alias) { + if (aliasDecl.symbolName) { + assert(aliasDecl.path); + const lookupResults = this._fileInfo.importLookup(aliasDecl.path); + if (lookupResults) { + const symbol = lookupResults.symbolTable.get(aliasDecl.symbolName); + if (symbol) { + symbolType = TypeUtils.getEffectiveTypeOfSymbol(symbol); + } + } + } else { + // Build a module type that corresponds to the declaration and + // its associated loader actions. + const moduleType = ModuleType.create(); + this._applyLoaderActionsToModuleType(moduleType, aliasDecl); + symbolType = moduleType; + } + } + + return [symbolWithScope ? symbolWithScope.symbol : undefined, symbolType]; } private _applyLoaderActionsToModuleType(moduleType: ModuleType, loaderActions: ModuleLoaderActions) { @@ -3427,28 +3392,6 @@ export class TypeAnalyzer extends ParseTreeWalker { this._addAssignmentTypeConstraint(node, type); } - // Finds the nearest permanent scope (as opposed to temporary scope) and - // adds a new symbol with the specified name if it doesn't already exist. - private _addSymbolToPermanentScope(name: string) { - const permanentScope = ScopeUtils.getPermanentScope(this._currentScope); - assert(permanentScope.getType() !== ScopeType.Temporary); - - let symbol = permanentScope.lookUpSymbol(name); - if (!symbol) { - symbol = permanentScope.addSymbol(name, SymbolFlags.ClassMember); - } - - // Variables that are defined within a module or a class - // are considered public by default. Don't flag them - // "not access" unless the name indicates that it's private. - const scopeType = permanentScope.getType(); - if (scopeType === ScopeType.Class || scopeType === ScopeType.Module) { - if (!this._isSymbolPrivate(name, scopeType)) { - this._setSymbolAccessed(symbol); - } - } - } - private _addTypeSourceToName(name: string, type: Type, typeSourceId: TypeSourceId, declaration?: Declaration) { From 204b4528166ad7e35b395316b6dd96028be82a27 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Oct 2019 23:18:57 -0700 Subject: [PATCH 22/23] Improved responsiveness by making recursive binding to honor time limit. --- server/src/analyzer/program.ts | 39 ++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index bb2b20ed5..f358ecd05 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -307,7 +307,9 @@ export class Program { // Now do binding of the open files. for (const sourceFileInfo of openFiles) { - this._bindFile(sourceFileInfo, options, importResolver); + if (this._bindFile(sourceFileInfo, options, importResolver, isTimeElapsedOpenFiles)) { + return true; + } if (isTimeElapsedOpenFiles()) { return true; @@ -504,12 +506,15 @@ export class Program { } } + // Binds the specified file and all of its dependencies, recursively. If + // it runs out of time, it returns true. If it completes, it returns false. private _bindFile(fileToAnalyze: SourceFileInfo, options: ConfigOptions, importResolver: ImportResolver, - recursionMap: Map = new Map()) { + timeElapsedCallback: () => boolean, + recursionMap: Map = new Map()): boolean { if (!this._isFileNeeded(fileToAnalyze) || !fileToAnalyze.sourceFile.isBindingRequired()) { - return; + return false; } this._parseFile(fileToAnalyze, options, importResolver); @@ -517,7 +522,11 @@ export class Program { // We need to parse and bind the builtins import first. let builtinsScope: Scope | undefined; if (fileToAnalyze.builtinsImport) { - this._bindFile(fileToAnalyze.builtinsImport, options, importResolver); + if (this._bindFile(fileToAnalyze.builtinsImport, options, + importResolver, timeElapsedCallback)) { + + return true; + } // Get the builtins scope to pass to the binding pass. const parseResults = fileToAnalyze.builtinsImport.sourceFile.getParseResults(); @@ -529,17 +538,26 @@ export class Program { const filePath = fileToAnalyze.sourceFile.getFilePath(); if (recursionMap.has(filePath)) { // Avoid infinite recursion for cyclical dependencies. - return; + return false; } // Bind any other files that this file depends upon. recursionMap.set(filePath, true); - fileToAnalyze.imports.forEach(importedFile => { - this._bindFile(importedFile, options, importResolver, recursionMap); - }); + for (const importedFile of fileToAnalyze.imports) { + if (this._bindFile(importedFile, options, importResolver, + timeElapsedCallback, recursionMap)) { + + return true; + } + } + } + + if (timeElapsedCallback()) { + return true; } fileToAnalyze.sourceFile.bind(options, this._lookUpImport, builtinsScope); + return false; } private _lookUpImport = (filePath: string): ImportLookupResult | undefined => { @@ -705,7 +723,10 @@ export class Program { } // Make sure the file is parsed and bound. - this._bindFile(fileToAnalyze, options, importResolver); + if (this._bindFile(fileToAnalyze, options, importResolver, timeElapsedCallback)) { + return true; + } + if (timeElapsedCallback()) { return true; } From aa51a7f547d4d9ba2c5986f45cd2101de0f8f6df Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 20 Oct 2019 02:21:25 -0700 Subject: [PATCH 23/23] Fixed bug where type analysis pass wasn't being reset back to 1 when markReanalysisRequired was called, so we would sometimes hit the max analysis limit. --- server/src/analyzer/sourceFile.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index cac3cbcc6..43b551f95 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -331,6 +331,7 @@ export class SourceFile { markReanalysisRequired(): void { // Keep the parse info, but reset the analysis to the beginning. this._parseTreeNeedsCleaning = true; + this._typeAnalysisPassNumber = 1; this._isTypeAnalysisFinalized = false; this._isTypeAnalysisPassNeeded = true; @@ -789,8 +790,7 @@ export class SourceFile { private _cleanParseTreeIfRequired() { if (this._parseResults) { if (this._parseTreeNeedsCleaning) { - const cleanerWalker = new ParseTreeCleanerWalker( - this._parseResults.parseTree); + const cleanerWalker = new ParseTreeCleanerWalker(this._parseResults.parseTree); cleanerWalker.clean(); this._parseTreeNeedsCleaning = false; }