Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential fix for bind failure during source file check #6408

Merged

Conversation

rchiodo
Copy link
Collaborator

@rchiodo rchiodo commented Nov 9, 2023

Addresses microsoft/pylance-release#5097

Saw a potential spot where this problem might occur:

Binding is required but it doesn't happen because a file is skipped. Check is still called in that situation.
Also added more information when binding is required but the check happens anyway.

This comment has been minimized.

packages/pyright-internal/src/analyzer/sourceFile.ts Outdated Show resolved Hide resolved
@@ -1890,12 +1890,16 @@ export class Program {
return false;
}

if (!this._disableChecker) {
if (!this._disableChecker && !fileToCheck.sourceFile.isFileDeleted()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be better to add the check for isFileDeleted() within _bindFile? I guess it depends on whether there are other places that call _bindFile that do not already check for this condition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_bindFile actually already checks for this and skips binding if the file is deleted. I was thinking that maybe _bindFile should return a boolean indicating if it actually bound the file or not (instead of doing this check here), so callers could skip calling things that depend upon the bind happening.

Copy link
Contributor

github-actions bot commented Nov 9, 2023

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@rchiodo rchiodo merged commit 2436c8d into microsoft:main Nov 9, 2023
12 checks passed
@rchiodo rchiodo deleted the rchiodo/potential_fix_for_binding_failure branch November 9, 2023 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants