From 74d8f3c4d273f23a1f6677510e76d2efd11222b3 Mon Sep 17 00:00:00 2001 From: PylanceBot <99766470+PylanceBot@users.noreply.github.com> Date: Wed, 28 Jun 2023 10:35:29 -0700 Subject: [PATCH] pull-pylance-with-pyright-1.1.316 (#5401) --- package.json | 2 +- packages/pyright-internal/package.json | 2 +- .../pyright-internal/src/analyzer/program.ts | 106 +++++++++++------- .../src/analyzer/sourceFile.ts | 1 + .../src/analyzer/sourceFileInfoUtils.ts | 85 ++++++++++++-- .../src/backgroundThreadBase.ts | 2 +- .../src/languageServerBase.ts | 1 + .../documentSymbolCollector.ts | 6 +- .../src/tests/chainedSourceFiles.test.ts | 60 +++++++++- .../pyright-internal/src/workspaceFactory.ts | 3 +- packages/pyright/package.json | 2 +- packages/vscode-pyright/package-lock.json | 4 +- packages/vscode-pyright/package.json | 6 +- 13 files changed, 219 insertions(+), 61 deletions(-) diff --git a/package.json b/package.json index 539814ceeeb3..1554af7a1c82 100644 --- a/package.json +++ b/package.json @@ -39,4 +39,4 @@ "typescript": "~4.4.4", "yargs": "^16.2.0" } -} \ No newline at end of file +} diff --git a/packages/pyright-internal/package.json b/packages/pyright-internal/package.json index 3c1105330ce0..c320c2927c6b 100644 --- a/packages/pyright-internal/package.json +++ b/packages/pyright-internal/package.json @@ -42,4 +42,4 @@ "ts-jest": "^27.1.5", "typescript": "~4.4.4" } -} \ No newline at end of file +} diff --git a/packages/pyright-internal/src/analyzer/program.ts b/packages/pyright-internal/src/analyzer/program.ts index 7dd85286f5ce..2c19d775185b 100644 --- a/packages/pyright-internal/src/analyzer/program.ts +++ b/packages/pyright-internal/src/analyzer/program.ts @@ -44,7 +44,7 @@ import { ImportResult, ImportType } from './importResult'; import { getDocString } from './parseTreeUtils'; import { Scope } from './scope'; import { IPythonMode, SourceFile } from './sourceFile'; -import { collectImportedByFiles, isUserCode } from './sourceFileInfoUtils'; +import { createChainedByList, isUserCode, verifyNoCyclesInChainedFiles } from './sourceFileInfoUtils'; import { SourceMapper } from './sourceMapper'; import { Symbol } from './symbol'; import { createTracePrinter } from './tracePrinter'; @@ -402,6 +402,7 @@ export class Program { sourceFileInfo.diagnosticsVersion = 0; } + verifyNoCyclesInChainedFiles(sourceFileInfo); sourceFileInfo.sourceFile.setClientVersion(version, contents); } @@ -412,12 +413,15 @@ export class Program { updateChainedFilePath(filePath: string, chainedFilePath: string | undefined) { const sourceFileInfo = this.getSourceFileInfo(filePath); - if (sourceFileInfo) { - sourceFileInfo.chainedSourceFile = chainedFilePath ? this.getSourceFileInfo(chainedFilePath) : undefined; - - sourceFileInfo.sourceFile.markDirty(); - this._markFileDirtyRecursive(sourceFileInfo, new Set()); + if (!sourceFileInfo) { + return; } + + sourceFileInfo.chainedSourceFile = chainedFilePath ? this.getSourceFileInfo(chainedFilePath) : undefined; + sourceFileInfo.sourceFile.markDirty(); + this._markFileDirtyRecursive(sourceFileInfo, new Set()); + + verifyNoCyclesInChainedFiles(sourceFileInfo); } setFileClosed(filePath: string, isTracked?: boolean): FileDiagnostics[] { @@ -1848,7 +1852,7 @@ export class Program { return false; } - private _checkTypes(fileToCheck: SourceFileInfo, token: CancellationToken) { + private _checkTypes(fileToCheck: SourceFileInfo, token: CancellationToken, chainedByList?: SourceFileInfo[]) { return this._logTracker.log(`analyzing: ${fileToCheck.sourceFile.getFilePath()}`, (logState) => { // If the file isn't needed because it was eliminated from the // transitive closure or deleted, skip the file rather than wasting @@ -1871,38 +1875,9 @@ export class Program { if (!this._disableChecker) { // For ipython, make sure we check all its dependent files first since // their results can affect this file's result. - let dependentFiles: ParseResults[] | undefined = undefined; - if (fileToCheck.sourceFile.getIPythonMode() === IPythonMode.CellDocs) { - // Parse file to get up to date dependency graph. - this._parseFile(fileToCheck); - - dependentFiles = []; - const importedByFiles = collectImportedByFiles(fileToCheck); - for (const file of importedByFiles) { - if (!isUserCode(file)) { - continue; - } - - // If the file is already analyzed, it will be no op. - // And make sure we don't dump parse tree and etc while - // recursively calling checker. Otherwise, inner check - // can dump parse tree required by outer check. - const handle = this._cacheManager.pauseTracking(); - try { - this._checkTypes(file, token); - } finally { - handle.dispose(); - } - - const parseResults = file.sourceFile.getParseResults(); - if (parseResults) { - dependentFiles.push(parseResults); - } - } - } + const dependentFiles = this._checkDependentFiles(fileToCheck, chainedByList, token); this._bindFile(fileToCheck); - if (this._preCheckCallback) { const parseResults = fileToCheck.sourceFile.getParseResults(); if (parseResults) { @@ -1928,7 +1903,11 @@ export class Program { if (this._configOptions.diagnosticRuleSet.reportImportCycles !== 'none') { // Don't detect import cycles when doing type stub generation. Some // third-party modules are pretty convoluted. - if (!this._allowedThirdPartyImports) { + // Or if the file is the notebook cell. notebook cell can't have cycles. + if ( + !this._allowedThirdPartyImports && + fileToCheck.sourceFile.getIPythonMode() !== IPythonMode.CellDocs + ) { // We need to force all of the files to be parsed and build // a closure map for the files. const closureMap = new Map(); @@ -1955,6 +1934,57 @@ export class Program { }); } + private _checkDependentFiles( + fileToCheck: SourceFileInfo, + chainedByList: SourceFileInfo[] | undefined, + token: CancellationToken + ) { + if (fileToCheck.sourceFile.getIPythonMode() !== IPythonMode.CellDocs) { + return undefined; + } + + // If we don't have chainedByList, it means none of them are checked yet. + const needToRunChecker = !chainedByList; + + chainedByList = chainedByList ?? createChainedByList(this, fileToCheck); + const index = chainedByList.findIndex((v) => v === fileToCheck); + if (index < 0) { + return undefined; + } + + const startIndex = index + 1; + if (startIndex >= chainedByList.length) { + return undefined; + } + + if (needToRunChecker) { + // If the file is already analyzed, it will be no op. + // And make sure we don't dump parse tree and etc while + // calling checker. Otherwise, checkType can dump parse + // tree required by outer check. + const handle = this._cacheManager.pauseTracking(); + try { + for (let i = chainedByList.length - 1; i >= startIndex; i--) { + this._checkTypes(chainedByList[i], token, chainedByList); + } + } finally { + handle.dispose(); + } + } + + const dependentFiles = []; + for (let i = startIndex; i < chainedByList.length; i++) { + const file = chainedByList[i]; + + const parseResults = file.sourceFile.getParseResults(); + if (parseResults) { + dependentFiles.push(parseResults); + } + } + + return dependentFiles; + } + // Builds a map of files that includes the specified file and all of the files // it imports (recursively) and ensures that all such files. If any of these files // have already been checked (they and their recursive imports have completed the diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts index b4587254b3c9..49b55b2dcd07 100644 --- a/packages/pyright-internal/src/analyzer/sourceFile.ts +++ b/packages/pyright-internal/src/analyzer/sourceFile.ts @@ -434,6 +434,7 @@ export class SourceFile { return this.fileSystem.readFileSync(this._filePath, 'utf8'); } catch (error) { + this._console.error(`Error reading file "${this._filePath}": ${error}`); return undefined; } } diff --git a/packages/pyright-internal/src/analyzer/sourceFileInfoUtils.ts b/packages/pyright-internal/src/analyzer/sourceFileInfoUtils.ts index bac134e2735b..13643b6870b4 100644 --- a/packages/pyright-internal/src/analyzer/sourceFileInfoUtils.ts +++ b/packages/pyright-internal/src/analyzer/sourceFileInfoUtils.ts @@ -6,26 +6,93 @@ * Functions that operate on SourceFileInfo objects. */ -import { SourceFileInfo } from '../common/extensibility'; +import { assert, fail } from '../common/debug'; +import { ProgramView, SourceFileInfo } from '../common/extensibility'; +import { IPythonMode } from './sourceFile'; export function isUserCode(fileInfo: SourceFileInfo | undefined) { return !!fileInfo && fileInfo.isTracked && !fileInfo.isThirdPartyImport && !fileInfo.isTypeshedFile; } -export function collectImportedByFiles(fileInfo: T): Set { - const importedByFiles = new Set(); - _collectImportedByFiles(fileInfo, importedByFiles); - return importedByFiles; +export function collectImportedByCells(program: ProgramView, fileInfo: T): Set { + // The ImportedBy only works when files are parsed. Due to the lazy-loading nature of our system, + // we can't ensure that all files within the program are parsed, which might lead to an incomplete dependency graph. + // Parsing all regular files goes against our lazy-nature, but for notebook cells, which we open by default, + // it makes sense to force complete parsing since they'll be parsed at some point anyway due to things like + // `semantic tokens` or `checkers`. + _parseAllOpenCells(program); + + const importedByCells = new Set(); + _collectImportedByCells(fileInfo, importedByCells); + return importedByCells; +} + +export function verifyNoCyclesInChainedFiles(fileInfo: T): void { + let nextChainedFile = fileInfo.chainedSourceFile; + if (!nextChainedFile) { + return; + } + + const set = new Set([fileInfo.sourceFile.getFilePath()]); + while (nextChainedFile) { + const path = nextChainedFile.sourceFile.getFilePath(); + if (set.has(path)) { + // We found a cycle. + fail(`Found a cycle in implicit imports files`); + } + + set.add(path); + nextChainedFile = nextChainedFile.chainedSourceFile; + } +} + +export function createChainedByList(program: ProgramView, fileInfo: T): T[] { + // We want to create reverse map of all chained files. + const map = new Map(); + for (const file of program.getSourceFileInfoList()) { + if (!file.chainedSourceFile) { + continue; + } + + map.set(file.chainedSourceFile, file); + } + + const visited = new Set(); + + const chainedByList: SourceFileInfo[] = [fileInfo]; + let current: SourceFileInfo | undefined = fileInfo; + while (current) { + assert(!visited.has(current), 'detected a cycle in chained files'); + visited.add(current); + + current = map.get(current); + if (current) { + chainedByList.push(current); + } + } + + return chainedByList as T[]; +} + +function _parseAllOpenCells(program: ProgramView): void { + for (const file of program.getSourceFileInfoList()) { + if (file.sourceFile.getIPythonMode() !== IPythonMode.CellDocs) { + continue; + } + + program.getParseResults(file.sourceFile.getFilePath()); + program.handleMemoryHighUsage(); + } } -function _collectImportedByFiles(fileInfo: SourceFileInfo, importedByFiles: Set) { +function _collectImportedByCells(fileInfo: SourceFileInfo, importedByCells: Set) { fileInfo.importedBy.forEach((dep) => { - if (importedByFiles.has(dep)) { + if (importedByCells.has(dep)) { // Already visited. return; } - importedByFiles.add(dep); - _collectImportedByFiles(dep, importedByFiles); + importedByCells.add(dep); + _collectImportedByCells(dep, importedByCells); }); } diff --git a/packages/pyright-internal/src/backgroundThreadBase.ts b/packages/pyright-internal/src/backgroundThreadBase.ts index 7632f63a68f2..66c48ab0dd26 100644 --- a/packages/pyright-internal/src/backgroundThreadBase.ts +++ b/packages/pyright-internal/src/backgroundThreadBase.ts @@ -51,7 +51,7 @@ export class BackgroundThreadBase { // Stash the base directory into a global variable. (global as any).__rootDirectory = data.rootDirectory; - this.fs = fileSystem ?? new PyrightFileSystem(createFromRealFileSystem(this.getConsole())); + this.fs = new PyrightFileSystem(fileSystem ?? createFromRealFileSystem(this.getConsole())); } protected log(level: LogLevel, msg: string) { diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index fc0062ab77a1..35b8af1e2630 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -368,6 +368,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface { this.workspaceFactory = new WorkspaceFactory( this.console, this.uriParser, + /* isWeb */ false, this.createAnalyzerServiceForWorkspace.bind(this), this.isPythonPathImmutable.bind(this), this.onWorkspaceCreated.bind(this) diff --git a/packages/pyright-internal/src/languageService/documentSymbolCollector.ts b/packages/pyright-internal/src/languageService/documentSymbolCollector.ts index aefb7ea2bddf..bc46152f72f4 100644 --- a/packages/pyright-internal/src/languageService/documentSymbolCollector.ts +++ b/packages/pyright-internal/src/languageService/documentSymbolCollector.ts @@ -30,7 +30,7 @@ import { ParseTreeWalker } from '../analyzer/parseTreeWalker'; import { ScopeType } from '../analyzer/scope'; import * as ScopeUtils from '../analyzer/scopeUtils'; import { IPythonMode } from '../analyzer/sourceFile'; -import { collectImportedByFiles } from '../analyzer/sourceFileInfoUtils'; +import { collectImportedByCells } from '../analyzer/sourceFileInfoUtils'; import { isStubFile } from '../analyzer/sourceMapper'; import { Symbol } from '../analyzer/symbol'; import { TypeEvaluator } from '../analyzer/typeEvaluatorTypes'; @@ -182,7 +182,7 @@ export class DocumentSymbolCollector extends ParseTreeWalker { }); const sourceFileInfo = program.getSourceFileInfo(fileInfo.filePath); - if (sourceFileInfo && sourceFileInfo.sourceFile.getIPythonMode() !== IPythonMode.None) { + if (sourceFileInfo && sourceFileInfo.sourceFile.getIPythonMode() === IPythonMode.CellDocs) { // Add declarations from chained source files let builtinsScope = fileInfo.builtinsScope; while (builtinsScope && builtinsScope.type === ScopeType.Module) { @@ -192,7 +192,7 @@ export class DocumentSymbolCollector extends ParseTreeWalker { } // Add declarations from files that implicitly import the target file. - const implicitlyImportedBy = collectImportedByFiles(sourceFileInfo); + const implicitlyImportedBy = collectImportedByCells(program, sourceFileInfo); implicitlyImportedBy.forEach((implicitImport) => { const parseTree = program.getParseResults(implicitImport.sourceFile.getFilePath())?.parseTree; if (parseTree) { diff --git a/packages/pyright-internal/src/tests/chainedSourceFiles.test.ts b/packages/pyright-internal/src/tests/chainedSourceFiles.test.ts index 0910fd2ce4c8..4620a5b1eacd 100644 --- a/packages/pyright-internal/src/tests/chainedSourceFiles.test.ts +++ b/packages/pyright-internal/src/tests/chainedSourceFiles.test.ts @@ -195,6 +195,64 @@ test('chained files with 1000s of files', async () => { assert.strictEqual(initialDiags.length, 0); }); +test('imported by files', async () => { + const code = ` +// @filename: test1.py +//// import [|/*marker*/os|] + +// @filename: test2.py +//// os.path.join() + `; + + const basePath = normalizeSlashes('/'); + const { data, service } = createServiceWithChainedSourceFiles(basePath, code); + analyze(service.test_program); + + const marker = data.markerPositions.get('marker')!; + const range = data.ranges.find((r) => r.marker === marker)!; + + const parseResults = service.getParseResult(marker.fileName)!; + const diagnostics = await service.getDiagnosticsForRange( + marker.fileName, + convertOffsetsToRange(range.pos, range.end, parseResults.tokenizerOutput.lines), + CancellationToken.None + ); + + assert.strictEqual(diagnostics.length, 0); +}); + +test('re ordering cells', async () => { + const code = ` +// @filename: test1.py +//// import [|/*marker*/os|] + +// @filename: test2.py +//// /*bottom*/os.path.join() + `; + + const basePath = normalizeSlashes('/'); + const { data, service } = createServiceWithChainedSourceFiles(basePath, code); + analyze(service.test_program); + + const marker = data.markerPositions.get('marker')!; + const range = data.ranges.find((r) => r.marker === marker)!; + + const bottom = data.markerPositions.get('bottom')!; + + service.updateChainedFilePath(bottom.fileName, undefined); + service.updateChainedFilePath(marker.fileName, bottom.fileName); + analyze(service.test_program); + + const parseResults = service.getParseResult(marker.fileName)!; + const diagnostics = await service.getDiagnosticsForRange( + marker.fileName, + convertOffsetsToRange(range.pos, range.end, parseResults.tokenizerOutput.lines), + CancellationToken.None + ); + + assert.strictEqual(diagnostics.length, 1); +}); + function createServiceWithChainedSourceFiles(basePath: string, code: string) { const service = new AnalyzerService( 'test service', @@ -211,7 +269,7 @@ function createServiceWithChainedSourceFiles(basePath: string, code: string) { let chainedFilePath: string | undefined; for (const file of data.files) { - service.setFileOpened(file.fileName, 1, file.content, IPythonMode.None, chainedFilePath); + service.setFileOpened(file.fileName, 1, file.content, IPythonMode.CellDocs, chainedFilePath); chainedFilePath = file.fileName; } return { data, service }; diff --git a/packages/pyright-internal/src/workspaceFactory.ts b/packages/pyright-internal/src/workspaceFactory.ts index 4d526d31b84f..d15d7a58f5b0 100644 --- a/packages/pyright-internal/src/workspaceFactory.ts +++ b/packages/pyright-internal/src/workspaceFactory.ts @@ -96,6 +96,7 @@ export class WorkspaceFactory { constructor( private readonly _console: ConsoleInterface, private readonly _uriParser: UriParser, + private readonly _isWeb: boolean, private readonly _createService: ( name: string, rootPath: string, @@ -387,7 +388,7 @@ export class WorkspaceFactory { kinds: string[] ) { // Update the kind based of the uri is local or not - if (!kinds.includes(WellKnownWorkspaceKinds.Default) && !this._uriParser.isLocal(rootUri)) { + if (!kinds.includes(WellKnownWorkspaceKinds.Default) && (!this._uriParser.isLocal(rootUri) || this._isWeb)) { // Web based workspace should be limited. kinds = [...kinds, WellKnownWorkspaceKinds.Limited]; } diff --git a/packages/pyright/package.json b/packages/pyright/package.json index 990007a81247..38704f402bb3 100644 --- a/packages/pyright/package.json +++ b/packages/pyright/package.json @@ -42,4 +42,4 @@ "pyright": "index.js", "pyright-langserver": "langserver.index.js" } -} \ No newline at end of file +} diff --git a/packages/vscode-pyright/package-lock.json b/packages/vscode-pyright/package-lock.json index d5d7c68fb1e9..0422e28af2ae 100644 --- a/packages/vscode-pyright/package-lock.json +++ b/packages/vscode-pyright/package-lock.json @@ -10,14 +10,14 @@ "license": "MIT", "dependencies": { "vscode-jsonrpc": "8.1.0", - "vscode-languageclient": "8.1.0", + "vscode-languageclient": "^8.1.0", "vscode-languageserver": "8.1.0", "vscode-languageserver-protocol": "3.17.3" }, "devDependencies": { "@types/copy-webpack-plugin": "^10.1.0", "@types/node": "^17.0.45", - "@types/vscode": "~1.78.0", + "@types/vscode": "^1.78.0", "copy-webpack-plugin": "^11.0.0", "detect-indent": "^6.1.0", "esbuild-loader": "^3.0.1", diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index 2e8285265756..2737b2068135 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -951,14 +951,14 @@ }, "dependencies": { "vscode-jsonrpc": "8.1.0", - "vscode-languageclient": "8.1.0", + "vscode-languageclient": "^8.1.0", "vscode-languageserver": "8.1.0", "vscode-languageserver-protocol": "3.17.3" }, "devDependencies": { "@types/copy-webpack-plugin": "^10.1.0", "@types/node": "^17.0.45", - "@types/vscode": "~1.78.0", + "@types/vscode": "^1.78.0", "copy-webpack-plugin": "^11.0.0", "detect-indent": "^6.1.0", "esbuild-loader": "^3.0.1", @@ -969,4 +969,4 @@ "webpack": "^5.85.0", "webpack-cli": "^5.1.1" } -} \ No newline at end of file +}