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

Fix spurious errors when using --follow-imports=normal #76

Merged
merged 2 commits into from Jul 6, 2023

Conversation

Jammf
Copy link
Contributor

@Jammf Jammf commented Jul 5, 2023

Fixes #62

When generating diagnostics for a file while using --follow-imports=normal, mypy will emit errors that occur in both the requested file and any other files it may import. However, the extension doesn't check which file an error originates from, leading to errors from other files being incorrectly reported as being for the requested file. These changes fix that behavior by stripping out all errors originating from files other than the requested file.

Additionally, this changes the default mypy configuration to be --follow-imports=normal. This makes it more similar to the previous behavior before #52, where --follow-imports=silent (which is not supported by dmypy) was used to suppress error messages while still allowing mypy to pull in useful type information from imported files.

@Jammf
Copy link
Contributor Author

Jammf commented Jul 5, 2023

@microsoft-github-policy-service agree

@karthiknadig karthiknadig self-requested a review July 5, 2023 17:36
@karthiknadig karthiknadig added the bug Issue identified by VS Code Team member as probable bug label Jul 5, 2023
@karthiknadig karthiknadig self-assigned this Jul 5, 2023
@karthiknadig
Copy link
Member

@Jammf Seems like there is an issue with python 3.7, probably need to special case it for 3.7.

@Jammf
Copy link
Contributor Author

Jammf commented Jul 5, 2023

It looks like there might be some issue with the .nox/tests venv that dmypy is running in? I found the following line in the testing logs, which is result.stderr from the call to dmypy (and since result.stdout is empty, no diagnostics are published, which is why the tests are failing):

{'type': 4, 'message': 'The typed_ast package is not installed.\nYou can install it with `python3 -m pip install typed-ast`.\n'}

This seems to just be an issue with 3.7, and doesn't seem to be related to the changes I made (I ran local tests against main and that also had the same errors).

That said, I'm pretty stumped on how to actually fix the tests, or even why that error is occurring in the first place. It does look like typed_ast has reached EOL (along with Python 3.7 itself), but I can't see how that would cause the tests to suddenly fail.

@karthiknadig
Copy link
Member

I have a fix for this. Basically typed-ast reached end of life yesterday. They released a new version, that version has multiple linux *.so which breaks our bundling.

@Jammf Jammf force-pushed the fix-follow-imports-normal branch from ee7c971 to 2620b27 Compare July 6, 2023 18:32
@karthiknadig karthiknadig enabled auto-merge (squash) July 6, 2023 18:53
@karthiknadig
Copy link
Member

After this is merged it will be available in the next pre-release (usually the next workday)

@karthiknadig karthiknadig merged commit 544d86d into microsoft:main Jul 6, 2023
18 checks passed
@Jammf Jammf deleted the fix-follow-imports-normal branch July 6, 2023 19:03
yaegassy added a commit to yaegassy/coc-mypy that referenced this pull request Jul 7, 2023
yaegassy added a commit to yaegassy/coc-mypy that referenced this pull request Jul 8, 2023
REF: <microsoft/vscode-mypy#76>

---

--follow-imports=normal seems to have trouble with diagnositcs not
updating when using dmypy.

This may be because the server used by coc-mypy does not support it yet.

Revert back to the --follow-imports=skip.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rogue "missing return statement" errors
3 participants