pull-pylance-with-pyright-1.1.316 (#5401)

package.json

@@ -39,4 +39,4 @@
         "typescript": "~4.4.4",
         "yargs": "^16.2.0"
     }
-}
+}

packages/pyright-internal/package.json

@@ -42,4 +42,4 @@
         "ts-jest": "^27.1.5",
         "typescript": "~4.4.4"
     }
-}
+}

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<string>());
+        if (!sourceFileInfo) {
+            return;
         }
+
+        sourceFileInfo.chainedSourceFile = chainedFilePath ? this.getSourceFileInfo(chainedFilePath) : undefined;
+        sourceFileInfo.sourceFile.markDirty();
+        this._markFileDirtyRecursive(sourceFileInfo, new Set<string>());
+
+        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<string, SourceFileInfo>();
@@ -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

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;
         }
     }

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<T extends SourceFileInfo>(fileInfo: T): Set<T> {
-    const importedByFiles = new Set<T>();
-    _collectImportedByFiles(fileInfo, importedByFiles);
-    return importedByFiles;
+export function collectImportedByCells<T extends SourceFileInfo>(program: ProgramView, fileInfo: T): Set<T> {
+    // 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<T>();
+    _collectImportedByCells(fileInfo, importedByCells);
+    return importedByCells;
+}
+
+export function verifyNoCyclesInChainedFiles<T extends SourceFileInfo>(fileInfo: T): void {
+    let nextChainedFile = fileInfo.chainedSourceFile;
+    if (!nextChainedFile) {
+        return;
+    }
+
+    const set = new Set<string>([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<T extends SourceFileInfo>(program: ProgramView, fileInfo: T): T[] {
+    // We want to create reverse map of all chained files.
+    const map = new Map<SourceFileInfo, SourceFileInfo>();
+    for (const file of program.getSourceFileInfoList()) {
+        if (!file.chainedSourceFile) {
+            continue;
+        }
+
+        map.set(file.chainedSourceFile, file);
+    }
+
+    const visited = new Set<SourceFileInfo>();
+
+    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<SourceFileInfo>) {
+function _collectImportedByCells(fileInfo: SourceFileInfo, importedByCells: Set<SourceFileInfo>) {
     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);
     });
 }

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) {

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)

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) {