From 375e35e56d855c07b5f350c5d6c4dda43ee26e04 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 27 Apr 2019 19:04:37 -0700 Subject: [PATCH] Added a bunch of verbose diagnostic information to help resolve import resolution issues. --- docs/command-line.md | 1 + server/src/analyzer/importResolver.ts | 79 ++++++++++++++++++------- server/src/analyzer/importResult.ts | 4 ++ server/src/analyzer/program.ts | 28 ++++++--- server/src/analyzer/pythonPathUtils.ts | 26 +++++--- server/src/analyzer/service.ts | 24 ++++++-- server/src/analyzer/sourceFile.ts | 6 +- server/src/common/commandLineOptions.ts | 3 + server/src/common/configOptions.ts | 3 + server/src/pyright.ts | 3 + server/src/server.ts | 1 + 11 files changed, 134 insertions(+), 44 deletions(-) diff --git a/docs/command-line.md b/docs/command-line.md index d08a98c34..b79d61558 100644 --- a/docs/command-line.md +++ b/docs/command-line.md @@ -9,6 +9,7 @@ Pyright can be run as either a VS Code extension or as a node-based command-line | --stats | Print detailed performance stats | | -t, --typeshed-path DIRECTORY | Use typeshed type stubs at this location (1) | | -v, --venv-path DIRECTORY | Directory that contains virtual environments (2) | +| --verbose | Emit verbose diagnostics | | -w, --watch | Continue to run and watch for changes (3) | diff --git a/server/src/analyzer/importResolver.ts b/server/src/analyzer/importResolver.ts index afcb34bf3..0837135ae 100644 --- a/server/src/analyzer/importResolver.ts +++ b/server/src/analyzer/importResolver.ts @@ -11,7 +11,6 @@ import * as fs from 'fs'; import { ConfigOptions, ExecutionEnvironment } from '../common/configOptions'; -import { ConsoleInterface } from '../common/console'; import { combinePaths, getDirectoryPath, getFileSystemEntries, isDirectory, isFile, stripFileExtension, stripTrailingDirectorySeparator } from '../common/pathUtils'; import { is3x, versionToString } from '../common/pythonVersion'; @@ -38,18 +37,20 @@ export class ImportResolver { // Resolves the import and returns the path if it exists, otherwise // returns undefined. - resolveImport(moduleDescriptor: ImportedModuleDescriptor, consoleInterface: ConsoleInterface): ImportResult { - let importName = this._formatImportName(moduleDescriptor); + resolveImport(moduleDescriptor: ImportedModuleDescriptor): ImportResult { + const importName = this._formatImportName(moduleDescriptor); + const importFailureInfo: string[] = []; // Find the site packages for the configured virtual environment. if (this._cachedPythonSearchPaths === undefined) { this._cachedPythonSearchPaths = PythonPathUtils.findPythonSearchPaths( - this._configOptions, this._executionEnvironment, consoleInterface); + this._configOptions, this._executionEnvironment, importFailureInfo); } // First check for a typeshed file. if (moduleDescriptor.leadingDots === 0 && moduleDescriptor.nameParts.length > 0) { - let builtInImport = this._findTypeshedPath(moduleDescriptor, importName, true); + let builtInImport = this._findTypeshedPath(moduleDescriptor, importName, + true, importFailureInfo); if (builtInImport) { builtInImport.isTypeshedFile = true; return builtInImport; @@ -58,7 +59,8 @@ export class ImportResolver { // Is it a relative import? if (moduleDescriptor.leadingDots > 0) { - let relativeImport = this._resolveRelativeImport(moduleDescriptor, importName); + let relativeImport = this._resolveRelativeImport(moduleDescriptor, + importName, importFailureInfo); if (relativeImport) { return relativeImport; } @@ -67,7 +69,7 @@ export class ImportResolver { // Look for it in the root directory of the execution environment. let localImport = this._resolveAbsoluteImport( - this._executionEnvironment.root, moduleDescriptor, importName); + this._executionEnvironment.root, moduleDescriptor, importName, importFailureInfo); if (localImport && localImport.importFound) { return localImport; } @@ -75,7 +77,8 @@ export class ImportResolver { for (let i = 0; i < this._executionEnvironment.extraPaths.length; i++) { let extraPath = this._executionEnvironment.extraPaths[i]; - localImport = this._resolveAbsoluteImport(extraPath, moduleDescriptor, importName); + localImport = this._resolveAbsoluteImport(extraPath, moduleDescriptor, + importName, importFailureInfo); if (localImport && localImport.importFound) { return localImport; } @@ -89,14 +92,15 @@ export class ImportResolver { // Check for a typings file. if (this._configOptions.typingsPath) { let typingsImport = this._resolveAbsoluteImport( - this._configOptions.typingsPath, moduleDescriptor, importName); + this._configOptions.typingsPath, moduleDescriptor, importName, importFailureInfo); if (typingsImport && typingsImport.importFound) { return typingsImport; } } // Check for a typeshed file. - let typeshedImport = this._findTypeshedPath(moduleDescriptor, importName, false); + let typeshedImport = this._findTypeshedPath(moduleDescriptor, importName, + false, importFailureInfo); if (typeshedImport) { typeshedImport.isTypeshedFile = true; return typeshedImport; @@ -108,7 +112,7 @@ export class ImportResolver { // Allow partial resolution because some third-party packages // use tricks to populate their package namespaces. let thirdPartyImport = this._resolveAbsoluteImport( - searchPath, moduleDescriptor, importName, true); + searchPath, moduleDescriptor, importName, importFailureInfo, true); if (thirdPartyImport) { thirdPartyImport.importType = ImportType.ThirdParty; return thirdPartyImport; @@ -126,6 +130,7 @@ export class ImportResolver { return { importName, importFound: false, + importFailureInfo, resolvedPaths: [], importType: ImportType.Local, isNamespacePackage: false, @@ -135,7 +140,9 @@ export class ImportResolver { } private _findTypeshedPath(moduleDescriptor: ImportedModuleDescriptor, importName: string, - isStdLib: boolean): ImportResult | undefined { + isStdLib: boolean, importFailureInfo: string[]): ImportResult | undefined { + + importFailureInfo.push('Looking for typeshed path'); let typeshedPath = ''; @@ -144,12 +151,14 @@ export class ImportResolver { if (this._configOptions.typeshedPath) { const possibleTypeshedPath = this._configOptions.typeshedPath; if (fs.existsSync(possibleTypeshedPath) && isDirectory(possibleTypeshedPath)) { + importFailureInfo.push(`Found typeshed path '${ typeshedPath }'`); typeshedPath = possibleTypeshedPath; } } else if (this._cachedPythonSearchPaths) { for (let searchPath of this._cachedPythonSearchPaths) { const possibleTypeshedPath = combinePaths(searchPath, 'typeshed'); if (fs.existsSync(possibleTypeshedPath) && isDirectory(possibleTypeshedPath)) { + importFailureInfo.push(`Found typeshed path '${ typeshedPath }'`); typeshedPath = possibleTypeshedPath; break; } @@ -159,17 +168,20 @@ export class ImportResolver { // If typeshed directory wasn't found in other locations, use the fallback. if (!typeshedPath) { typeshedPath = PythonPathUtils.getTypeShedFallbackPath() || ''; + importFailureInfo.push(`Using typeshed fallback path '${ typeshedPath }'`); } typeshedPath = PythonPathUtils.getTypeshedSubdirectory(typeshedPath, isStdLib); if (!fs.existsSync(typeshedPath) || !isDirectory(typeshedPath)) { + importFailureInfo.push(`Typeshed path '${ typeshedPath }' is not a valid directory`); return undefined; } // We currently support only 3.x. let pythonVersion = this._executionEnvironment.pythonVersion; if (!is3x(pythonVersion)) { + importFailureInfo.push(`Python version < 3.0 not supported`); return undefined; } @@ -181,7 +193,8 @@ export class ImportResolver { minorVersion === 0 ? '3' : '2and3'; let testPath = combinePaths(typeshedPath, pythonVersionString); if (fs.existsSync(testPath)) { - let importInfo = this._resolveAbsoluteImport(testPath, moduleDescriptor, importName); + let importInfo = this._resolveAbsoluteImport(testPath, moduleDescriptor, + importName, importFailureInfo); if (importInfo && importInfo.importFound) { if (isStdLib) { importInfo.importType = ImportType.BuiltIn; @@ -197,11 +210,14 @@ export class ImportResolver { minorVersion--; } + importFailureInfo.push(`Typeshed path not found`); return undefined; } private _resolveRelativeImport(moduleDescriptor: ImportedModuleDescriptor, - importName: string): ImportResult | undefined { + importName: string, importFailureInfo: string[]): ImportResult | undefined { + + importFailureInfo.push('Attempting to resolve relative import'); // Determine which search path this file is part of. let curDir = getDirectoryPath(this._sourceFilePath); @@ -210,13 +226,16 @@ export class ImportResolver { } // Now try to match the module parts from the current directory location. - return this._resolveAbsoluteImport(curDir, moduleDescriptor, importName); + return this._resolveAbsoluteImport(curDir, moduleDescriptor, + importName, importFailureInfo); } // Follows import resolution algorithm defined in PEP-420: // https://www.python.org/dev/peps/pep-0420/ private _resolveAbsoluteImport(rootPath: string, moduleDescriptor: ImportedModuleDescriptor, - importName: string, allowPartial = false): ImportResult | undefined { + importName: string, importFailureInfo: string[], allowPartial = false): ImportResult | undefined { + + importFailureInfo.push(`Attempting to resolve using root path '${ rootPath }'`); // Starting at the specified path, walk the file system to find the // specified module. @@ -228,14 +247,18 @@ export class ImportResolver { // Handle the "from . import XXX" case. if (moduleDescriptor.nameParts.length === 0) { - let pyFilePath = combinePaths(dirPath, '__init__.py'); - let pyiFilePath = pyFilePath + 'i'; + const pyFilePath = combinePaths(dirPath, '__init__.py'); + const pyiFilePath = pyFilePath + 'i'; + if (fs.existsSync(pyiFilePath) && isFile(pyiFilePath)) { + importFailureInfo.push(`Resolved import with file '${ pyiFilePath }'`); resolvedPaths.push(pyiFilePath); isStubFile = true; } else if (fs.existsSync(pyFilePath) && isFile(pyFilePath)) { + importFailureInfo.push(`Resolved import with file '${ pyFilePath }'`); resolvedPaths.push(pyFilePath); } else { + importFailureInfo.push(`Partially resolved import with directory '${ dirPath }'`); resolvedPaths.push(dirPath); isNamespacePackage = true; } @@ -246,27 +269,38 @@ export class ImportResolver { for (let i = 0; i < moduleDescriptor.nameParts.length; i++) { dirPath = combinePaths(dirPath, moduleDescriptor.nameParts[i]); if (!fs.existsSync(dirPath) || !isDirectory(dirPath)) { + importFailureInfo.push(`Could not find directory '${ dirPath }'`); + // We weren't able to find the subdirectory. See if we can // find a ".py" or ".pyi" file with this name. - let pyFilePath = stripTrailingDirectorySeparator(dirPath) + '.py'; - let pyiFilePath = pyFilePath + 'i'; + const pyFilePath = stripTrailingDirectorySeparator(dirPath) + '.py'; + const pyiFilePath = pyFilePath + 'i'; + if (fs.existsSync(pyiFilePath) && isFile(pyiFilePath)) { + importFailureInfo.push(`Resolved import with file '${ pyiFilePath }'`); resolvedPaths.push(pyiFilePath); isStubFile = true; } else if (fs.existsSync(pyFilePath) && isFile(pyFilePath)) { + importFailureInfo.push(`Resolved import with file '${ pyFilePath }'`); resolvedPaths.push(pyFilePath); + } else { + importFailureInfo.push(`Did not find file '${ pyiFilePath }' or '${ pyFilePath }'`); } break; } - let pyFilePath = combinePaths(dirPath, '__init__.py'); - let pyiFilePath = pyFilePath + 'i'; + const pyFilePath = combinePaths(dirPath, '__init__.py'); + const pyiFilePath = pyFilePath + 'i'; + if (fs.existsSync(pyiFilePath) && isFile(pyiFilePath)) { + importFailureInfo.push(`Resolved import with file '${ pyiFilePath }'`); resolvedPaths.push(pyiFilePath); isStubFile = true; } else if (fs.existsSync(pyFilePath) && isFile(pyFilePath)) { + importFailureInfo.push(`Resolved import with file '${ pyFilePath }'`); resolvedPaths.push(pyFilePath); } else { + importFailureInfo.push(`Partially resolved import with directory '${ dirPath }'`); resolvedPaths.push(dirPath); if (i === moduleDescriptor.nameParts.length - 1) { isNamespacePackage = true; @@ -295,6 +329,7 @@ export class ImportResolver { return { importName, importFound, + importFailureInfo, importType: ImportType.Local, resolvedPaths, searchPath: rootPath, diff --git a/server/src/analyzer/importResult.ts b/server/src/analyzer/importResult.ts index 22dc53786..4b35fbd93 100644 --- a/server/src/analyzer/importResult.ts +++ b/server/src/analyzer/importResult.ts @@ -26,6 +26,10 @@ export interface ImportResult { // True if import was resolved to a module or file. importFound: boolean; + // If importFound is false, may contain strings that help + // diagnose the import resolution failure. + importFailureInfo?: string[]; + // Type of import (built-in, local, third-party). importType: ImportType; diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts index 37bff9ea1..ce4c821ed 100644 --- a/server/src/analyzer/program.ts +++ b/server/src/analyzer/program.ts @@ -310,7 +310,7 @@ export class Program { } if (fileToParse.sourceFile.parse(options)) { - this._updateSourceFileImports(fileToParse); + this._updateSourceFileImports(fileToParse, options); } } @@ -715,15 +715,17 @@ export class Program { return false; } - private _updateSourceFileImports(sourceFileInfo: SourceFileInfo): SourceFileInfo[] { - let filesAdded: SourceFileInfo[] = []; + private _updateSourceFileImports(sourceFileInfo: SourceFileInfo, + options: ConfigOptions): SourceFileInfo[] { + + const filesAdded: SourceFileInfo[] = []; // Get the new list of imports and see if it changed from the last // list of imports for this file. - let imports = sourceFileInfo.sourceFile.getImports(); + const imports = sourceFileInfo.sourceFile.getImports(); // Create a map of unique imports, since imports can appear more than once. - let newImportPathMap: { [name: string]: boolean } = {}; + const newImportPathMap: { [name: string]: boolean } = {}; imports.forEach(importResult => { if (importResult.importFound) { // Don't explore any third-party files unless they're type stub files. @@ -731,7 +733,7 @@ export class Program { // Namespace packages have no __init__.py file, so the resolved // path points to a directory. if (!importResult.isNamespacePackage && importResult.resolvedPaths.length > 0) { - let filePath = importResult.resolvedPaths[ + const filePath = importResult.resolvedPaths[ importResult.resolvedPaths.length - 1]; newImportPathMap[filePath] = !!importResult.isTypeshedFile; } @@ -740,12 +742,22 @@ export class Program { newImportPathMap[implicitImport.path] = !!importResult.isTypeshedFile; }); } + } else if (options.verboseOutput) { + if (!sourceFileInfo.isTypeshedFile || options.reportTypeshedErrors) { + this._console.log(`Could not import '${ importResult.importName }' ` + + `in file '${ sourceFileInfo.sourceFile.getFilePath() }'`); + if (importResult.importFailureInfo) { + importResult.importFailureInfo.forEach(diag => { + this._console.log(` ${ diag }`); + }); + } + } } }); - let updatedImportMap: { [name: string]: SourceFileInfo } = {}; + const updatedImportMap: { [name: string]: SourceFileInfo } = {}; sourceFileInfo.imports.forEach(importInfo => { - let oldFilePath = importInfo.sourceFile.getFilePath(); + const oldFilePath = importInfo.sourceFile.getFilePath(); // A previous import was removed. if (newImportPathMap[oldFilePath] === undefined) { diff --git a/server/src/analyzer/pythonPathUtils.ts b/server/src/analyzer/pythonPathUtils.ts index 9852593e0..9ce6d187f 100644 --- a/server/src/analyzer/pythonPathUtils.ts +++ b/server/src/analyzer/pythonPathUtils.ts @@ -37,7 +37,9 @@ export class PythonPathUtils { } static findPythonSearchPaths(configOptions: ConfigOptions, execEnv: ExecutionEnvironment | undefined, - consoleInterface: ConsoleInterface): string[] | undefined { + importFailureInfo: string[]): string[] | undefined { + + importFailureInfo.push('Finding python search paths'); let venvPath: string | undefined; if (execEnv && execEnv.venv) { @@ -54,7 +56,10 @@ export class PythonPathUtils { let libPath = combinePaths(venvPath, 'lib'); let sitePackagesPath = combinePaths(libPath, 'site-packages'); if (fs.existsSync(sitePackagesPath)) { + importFailureInfo.push(`Found path '${ sitePackagesPath }'`); return [sitePackagesPath]; + } else { + importFailureInfo.push(`Did not find '${ sitePackagesPath }', so looking for python subdirectory`); } // We didn't find a site-packages directory directly in the lib @@ -65,18 +70,21 @@ export class PythonPathUtils { if (dirName.startsWith('python')) { let dirPath = combinePaths(libPath, dirName, 'site-packages'); if (fs.existsSync(dirPath)) { + importFailureInfo.push(`Found path '${ dirPath }'`); return [dirPath]; } } } } + importFailureInfo.push(`Did not find site-packages. Falling back on python interpreter.`); + // Fall back on the python interpreter. - return this.getPythonPathFromPythonInterpreter(configOptions.pythonPath, consoleInterface); + return this.getPythonPathFromPythonInterpreter(configOptions.pythonPath, importFailureInfo); } static getPythonPathFromPythonInterpreter(interpreterPath: string | undefined, - consoleInterface: ConsoleInterface): string[] { + importFailureInfo: string[]): string[] { const searchKey = interpreterPath || ''; @@ -92,9 +100,11 @@ export class PythonPathUtils { let execOutput: string; if (interpreterPath) { + importFailureInfo.push(`Executing interpreter at '${ interpreterPath }'`); execOutput = child_process.execFileSync( interpreterPath, commandLineArgs, { encoding: 'utf8' }); } else { + importFailureInfo.push(`Executing python interpreter`); execOutput = child_process.execFileSync( 'python', commandLineArgs, { encoding: 'utf8' }); } @@ -124,18 +134,20 @@ export class PythonPathUtils { } if (pythonPaths.length === 0) { - consoleInterface.error(`Attempted to get python import paths from python interpreter but ` + - `found no valid directories`); + importFailureInfo.push(`Found no valid directories`); } } else { - consoleInterface.error(`Attempted to get python import paths from python interpreter but ` + - `could not parse output: '${ execOutput }'`); + importFailureInfo.push(`Could not parse output: '${ execOutput }'`); } } catch { pythonPaths = []; } cachedSearchPaths[searchKey] = pythonPaths; + importFailureInfo.push(`Received ${ pythonPaths.length } paths from interpreter`); + pythonPaths.forEach(path => { + importFailureInfo.push(` ${ path }`); + }); return pythonPaths; } diff --git a/server/src/analyzer/service.ts b/server/src/analyzer/service.ts index 84df9c77d..b41e1b4af 100644 --- a/server/src/analyzer/service.ts +++ b/server/src/analyzer/service.ts @@ -232,6 +232,10 @@ export class AnalyzerService { } } + if (commandLineOptions.verboseOutput) { + configOptions.verboseOutput = true; + } + // Do some sanity checks on the specified settings and report missing // or inconsistent information. if (configOptions.venvPath) { @@ -245,14 +249,26 @@ export class AnalyzerService { `venvPath must be used in conjunction with venv setting, which was omitted.`); } else { const fullVenvPath = combinePaths(configOptions.venvPath, configOptions.defaultVenv); + if (!fs.existsSync(fullVenvPath) || !isDirectory(fullVenvPath)) { this._console.log( `venv ${ configOptions.defaultVenv } subdirectory not found ` + `in venv path ${ configOptions.venvPath }.`); - } else if (PythonPathUtils.findPythonSearchPaths(configOptions, undefined, this._console) === undefined) { - this._console.log( - `site-packages directory cannot be located for venvPath ` + - `${ configOptions.venvPath } and venv ${ configOptions.defaultVenv }.`); + } else { + const importFailureInfo: string[] = []; + if (PythonPathUtils.findPythonSearchPaths(configOptions, undefined, + importFailureInfo) === undefined) { + + this._console.log( + `site-packages directory cannot be located for venvPath ` + + `${ configOptions.venvPath } and venv ${ configOptions.defaultVenv }.`); + + if (configOptions.verboseOutput) { + importFailureInfo.forEach(diag => { + this._console.log(` ${ diag }`); + }); + } + } } } } else { diff --git a/server/src/analyzer/sourceFile.ts b/server/src/analyzer/sourceFile.ts index 437477d36..1fc1619b9 100644 --- a/server/src/analyzer/sourceFile.ts +++ b/server/src/analyzer/sourceFile.ts @@ -597,7 +597,7 @@ export class SourceFile { leadingDots: 0, nameParts: ['builtins'], importedSymbols: undefined - }, this._console); + }); // Avoid importing builtins from the builtins.pyi file itself. if (builtinsImportResult.resolvedPaths.length === 0 || @@ -612,7 +612,7 @@ export class SourceFile { leadingDots: 0, nameParts: ['typing'], importedSymbols: undefined - }, this._console); + }); // Avoid importing typing from the typing.pyi file itself. let typingModulePath: string | undefined; @@ -627,7 +627,7 @@ export class SourceFile { leadingDots: moduleImport.leadingDots, nameParts: moduleImport.nameParts, importedSymbols: moduleImport.importedSymbols - }, this._console); + }); imports.push(importResult); // Associate the import results with the module import diff --git a/server/src/common/commandLineOptions.ts b/server/src/common/commandLineOptions.ts index 28ed64e6d..2f913129e 100644 --- a/server/src/common/commandLineOptions.ts +++ b/server/src/common/commandLineOptions.ts @@ -40,6 +40,9 @@ export class CommandLineOptions { // Absolute execution root (current working directory). executionRoot: string; + // Emit verbose information to console? + verboseOutput?: boolean; + // Indicates that the settings came from VS Code rather than // from the command-line. Useful for providing clearer error // messages. diff --git a/server/src/common/configOptions.ts b/server/src/common/configOptions.ts index 45a96c0d6..d0b512962 100644 --- a/server/src/common/configOptions.ts +++ b/server/src/common/configOptions.ts @@ -73,6 +73,9 @@ export class ConfigOptions { // if they are included in the transitive closure of included files. ignore: string[] = []; + // Emit verbose information to console? + verboseOutput: boolean; + //--------------------------------------------------------------- // Diagnostics Settings diff --git a/server/src/pyright.ts b/server/src/pyright.ts index f87435204..5781ce570 100644 --- a/server/src/pyright.ts +++ b/server/src/pyright.ts @@ -38,6 +38,7 @@ function processArgs() { { name: 'stats' }, { name: 'typeshed-path', alias: 't', type: String }, { name: 'venv-path', alias: 'v', type: String }, + { name: 'verbose', type: Boolean }, { name: 'watch', alias: 'w', type: Boolean } ]; @@ -75,6 +76,8 @@ function processArgs() { options.typeshedPath = combinePaths(process.cwd(), normalizePath(args['typeshed-path'])); } + options.verboseOutput = args.verbose !== undefined; + let watch = args.watch !== undefined; options.watch = watch; diff --git a/server/src/server.ts b/server/src/server.ts index e4a9eeaf3..8ccbaac9f 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -169,6 +169,7 @@ _connection.onCompletion(params => { function updateOptionsAndRestartService(settings?: Settings) { let commandLineOptions = new CommandLineOptions(_rootPath, true); commandLineOptions.watch = true; + commandLineOptions.verboseOutput = true; if (settings && settings.python) { if (settings.python.venvPath) {