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

Should not use --follow-imports=skip #89

Closed
PeterJCLaw opened this issue Jul 15, 2023 · 3 comments · Fixed by #90
Closed

Should not use --follow-imports=skip #89

PeterJCLaw opened this issue Jul 15, 2023 · 3 comments · Fixed by #90
Assignees

Comments

@PeterJCLaw
Copy link
Contributor

This extension improves on the default behaviour of the main Python extension's handling of mypy in terms of the options it passes, however it appears to silently pass --follow-imports=skip. This causes all imports to be replaced by Any, in turn substantially limiting the usefulness of the type checking by causing both:

  • spurious errors when used in combination with warn_return_any (possibly other cases too), and
  • true errors to be missed, since any types imported (even from within the same project) are considered as Any and thus fully compatible with any other type
@levrik
Copy link

levrik commented Jul 17, 2023

I think this has been changed in #76 already. At least upgrading to the current pre-release has fixed this issue for me.

@karthiknadig karthiknadig self-assigned this Jul 17, 2023
@PeterJCLaw
Copy link
Contributor Author

Thanks, I hadn't spotted that. Unfortunately I don't think that that fixes the issue here -- which is that extension is setting a value at all that can therefore differ from what a project has configured.

If I recall correctly, mypy treats CLI options at a higher priority than those found in any configuration files, meaning that the extension passing any value here will always override what the user has configured for their project. While --follow-imports=normal (as set by #76) is much more likely to match what projects use, it is still an explicit override which may differ from what users see when running on the command line/in CI/etc.

@karthiknadig
Copy link
Member

@PeterJCLaw We don't really have a reason for this default other than it was what was in defaults for the python extension. We can remove it. We are open to a PR removing it.

PeterJCLaw added a commit to PeterJCLaw/vscode-mypy that referenced this issue Jul 17, 2023
mypy prioritises values from configuration on its command line
over that provided in configuration files. This means that the
value previously provided here could override the settings for a
project, cause spurious errors and/or hiding true errors.

Removing this default value allows users freedom to configure
their projects however they like, without the extension injecting
unexpected additional configuration within the IDE context.

Fixes microsoft#89
PeterJCLaw added a commit to PeterJCLaw/vscode-mypy that referenced this issue Jul 17, 2023
mypy prioritises values from configuration on its command line
over that provided in configuration files. This means that the
value previously provided here could override the settings for a
project, cause spurious errors and/or hiding true errors.

Removing this default value allows users freedom to configure
their projects however they like, without the extension injecting
unexpected additional configuration within the IDE context.

Tested manually by opening a project which has a `setup.cfg` and
changing the `follow_imports` value within that file, then checking
the errors reported from `mypy` change accordingly.

Fixes microsoft#89
karthiknadig pushed a commit that referenced this issue Jul 18, 2023
mypy prioritises values from configuration on its command line
over that provided in configuration files. This means that the
value previously provided here could override the settings for a
project, cause spurious errors and/or hiding true errors.

Removing this default value allows users freedom to configure
their projects however they like, without the extension injecting
unexpected additional configuration within the IDE context.

Tested manually by opening a project which has a `setup.cfg` and
changing the `follow_imports` value within that file, then checking
the errors reported from `mypy` change accordingly.

Fixes #89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants