Potential fix for bind failure during source file check (#6408)

* Potential fix for binding failure during check * Fix prettier * Review feedback
microsoft · Nov 9, 2023 · 2436c8d · 2436c8d
1 parent a95a808
commit 2436c8d
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 14 deletions.
diff --git a/packages/pyright-internal/src/analyzer/program.ts b/packages/pyright-internal/src/analyzer/program.ts
@@ -1703,16 +1703,16 @@ 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.
+    // Binds the specified file and all of its dependencies, recursively.
+    // Returns true if the file was bound or it didn't need to be bound.
     private _bindFile(
         fileToAnalyze: SourceFileInfo,
         content?: string,
         skipFileNeededCheck?: boolean,
         isImplicitImport?: boolean
-    ): void {
+    ): boolean {
         if (!this._isFileNeeded(fileToAnalyze, skipFileNeededCheck) || !fileToAnalyze.sourceFile.isBindingRequired()) {
-            return;
+            return !fileToAnalyze.sourceFile.isBindingRequired();
         }
 
         this._parseFile(fileToAnalyze, content, skipFileNeededCheck);
@@ -1759,6 +1759,7 @@ export class Program {
         fileToAnalyze.effectiveFutureImports = futureImports.size > 0 ? futureImports : undefined;
 
         fileToAnalyze.sourceFile.bind(this._configOptions, this._lookUpImport, builtinsScope, futureImports);
+        return true;
     }
 
     private _getEffectiveFutureImports(futureImports: Set<string>, chainedSourceFile: SourceFileInfo): Set<string> {
@@ -1895,22 +1896,30 @@ export class Program {
                 // their results can affect this file's result.
                 const dependentFiles = this._checkDependentFiles(fileToCheck, chainedByList, token);
 
-                this._bindFile(fileToCheck);
+                const boundFile = this._bindFile(
+                    fileToCheck,
+                    undefined,
+                    // If binding is required we want to make sure to bind the file, otherwise
+                    // the sourceFile.check below will fail.
+                    /* skipFileNeededCheck */ fileToCheck.sourceFile.isBindingRequired()
+                );
                 if (this._preCheckCallback) {
                     const parseResults = fileToCheck.sourceFile.getParseResults();
                     if (parseResults) {
                         this._preCheckCallback(parseResults, this._evaluator!);
                     }
                 }
 
-                const execEnv = this._configOptions.findExecEnvironment(fileToCheck.sourceFile.getFilePath());
-                fileToCheck.sourceFile.check(
-                    this.configOptions,
-                    this._importResolver,
-                    this._evaluator!,
-                    this._createSourceMapper(execEnv, token, fileToCheck),
-                    dependentFiles
-                );
+                if (boundFile) {
+                    const execEnv = this._configOptions.findExecEnvironment(fileToCheck.sourceFile.getFilePath());
+                    fileToCheck.sourceFile.check(
+                        this.configOptions,
+                        this._importResolver,
+                        this._evaluator!,
+                        this._createSourceMapper(execEnv, token, fileToCheck),
+                        dependentFiles
+                    );
+                }
             }
 
             // For very large programs, we may need to discard the evaluator and

diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts
@@ -135,6 +135,37 @@ class WriteableData {
     builtinsImport: ImportResult | undefined;
     // True if the file appears to have been deleted.
     isFileDeleted = false;
+
+    debugPrint() {
+        return `WritableData: 
+ diagnosticVersion=${this.diagnosticVersion}, 
+ noCircularDependencyConfirmed=${this.noCircularDependencyConfirmed}, 
+ isBindingNeeded=${this.isBindingNeeded},
+ isBindingInProgress=${this.isBindingInProgress},
+ isCheckingNeeded=${this.isCheckingNeeded},
+ isFileDeleted=${this.isFileDeleted},
+ hitMaxImportDepth=${this.hitMaxImportDepth},
+ parseTreeNeedsCleaning=${this.parseTreeNeedsCleaning},
+ fileContentsVersion=${this.fileContentsVersion},
+ analyzedFileContentsVersion=${this.analyzedFileContentsVersion},
+ clientDocumentVersion=${this.clientDocumentVersion},
+ lastFileContentLength=${this.lastFileContentLength},
+ lastFileContentHash=${this.lastFileContentHash},
+ typeIgnoreAll=${this.typeIgnoreAll},
+ imports=${this.imports?.length},
+ builtinsImport=${this.builtinsImport?.importName},
+ circularDependencies=${this.circularDependencies?.length},
+ parseDiagnostics=${this.parseDiagnostics?.length},
+ commentDiagnostics=${this.commentDiagnostics?.length},
+ bindDiagnostics=${this.bindDiagnostics?.length},
+ checkerDiagnostics=${this.checkerDiagnostics?.length},
+ accumulatedDiagnostics=${this.accumulatedDiagnostics?.length},
+ typeIgnoreLines=${this.typeIgnoreLines?.size},
+ pyrightIgnoreLines=${this.pyrightIgnoreLines?.size},
+ checkTime=${this.checkTime},
+ clientDocumentContents=${this.clientDocumentContents?.length},
+ parseResults=${this.parseResults?.parseTree.length}`;
+    }
 }
 
 export interface SourceFileEditMode {
@@ -778,7 +809,7 @@ export class SourceFile {
         dependentFiles?: ParseResults[]
     ) {
         assert(!this.isParseRequired(), 'Check called before parsing');
-        assert(!this.isBindingRequired(), 'Check called before binding');
+        assert(!this.isBindingRequired(), `Check called before binding: state=${this._writableData.debugPrint()}`);
         assert(!this._writableData.isBindingInProgress, 'Check called while binding in progress');
         assert(this.isCheckingRequired(), 'Check called unnecessarily');
         assert(this._writableData.parseResults !== undefined, 'Parse results not available');