Please revert new source finding behaviour or add some way to exclude paths

[intgr]

Bug Report

I think the PR "find_sources: find build sources recursively" #9614 is a good and worthwhile change. But in practice it causes surprising regressions for many users. It's not just spurious warnings, mypy often refuses to run entirely ("Found 1 error in 1 file (errors prevented further checking)").

In many cases, there aren't good work-arounds, short of restructuring one's project directory tree.

I think the magnitude of this issue is large enough that the best course of action is to revert the change and issue a hotfix release, until some work-around for those cases is created.

There are many examples of common things that throw off mypy:

1. Creating even an empty virtualenv causes mypy to fail to run.
2. Having __init__.py files under any directory that's not a valid Python module name.
3. Many npm packages include .py files, which get installed in node_modules directory, some of which may have Python 2.7 code that causes syntax errors.
4. Having copies of directories or files with same .py file names.

Some more examples in comments here: #4675 (comment)

To Reproduce

% mkdir test1
% cd test1
% echo 'a: str = 10' > test.py
% mkdir -p test/invalid-module
% touch test/invalid-module/__init__.py
% mypy .
invalid-module is not a valid Python package name

% mkdir test2
% cd test2
% python3 -m venv ./venv
% echo 'a: str = 10' > test.py
% mypy .
venv/lib/python3.9/site-packages/setuptools/_distutils/command/bdist_msi.py:355: error: expected an indented block
Found 1 error in 1 file (errors prevented further checking)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

% mkdir test3
% cd test3
% echo 'a: str = 10' > test.py
% npm install gulp-sass
% mypy .
node_modules/node-sass/node_modules/node-gyp/gyp/pylib/gyp/__init__.py:37: error: invalid syntax
Found 1 error in 1 file (errors prevented further checking)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

% mkdir test4
% cd test4
% echo 'a: str = 10' > test.py
% mkdir backup
% cp test.py backup/
% mypy .
test.py: error: Duplicate module named 'test' (also at './backup/test.py')
test.py: error: Are you missing an __init__.py?
Found 2 errors in 1 file (errors prevented further checking)

Your Environment

• Mypy version used: 0.800
• Mypy command-line flags: none
• Mypy configuration options from mypy.ini (and other config files): none
• Python version used: 3.9
• Operating system and version: macOS 10.15

[JukkaL]

@intgr Thanks for reporting the issue! These all seem real regressions. It may be worth disabling the new functionality until there are ways to work around them.

Brainstorming:

• Provide a command-line (and config file) option such as --ignore-dir which allows skipping some directories. For example, mypy --ignore-dir venv ..
• Disable the recursive behavior unless using --namespace-packages (this is ad hoc and only a partial workaround, though).
• Automatically filter out known problematic directory names such as site-packages and node_modules. These would always have to be explicitly listed on the command line when the intent is to actually type check their contents.
• Add a note suggesting the use of --ignore-dir in some of the more obvious/serious cases.

@hauntsaninja What do you think?

[volans-]

I was about to open a similar task and found this one. In my case a fixture file that is in the test tree and was not caught with 0.790 (because one of the intermediate directories doesn't have the __init__.py file) it's now caught with mypy 0.800 and not excluded as expected with the [mypy-{pattern}] section with ignore_errors = True.

In my case the structure that works on 0.790 and fails on 0.800 is:

$ tree
.
├── mypkg
│   ├── __init__.py
│   └── tests
│       ├── __init__.py
│       └── fixtures
│           └── invalid
│               ├── __init__.py
│               └── invalid.py
└── setup.cfg

With:

$ cat mypkg/tests/fixtures/invalid/invalid.py
invalid =  # noqa: E999

$ cat setup.cfg
[mypy]

[mypy-mypkg.tests.*]
ignore_errors = True

With 0.800 gives:

$ mypy mypkg/
mypkg/tests/fixtures/invalid/invalid.py:1: error: invalid syntax
Found 1 error in 1 file (errors prevented further checking)

While with 0.790:

$ mypy mypkg/
Success: no issues found in 2 source files

[hauntsaninja]

@JukkaL Yeah, we should add an ignore option (#4675 (comment)).
I think it'd be useful to support ignoring files as well (e.g. setup.py, conftest.py) in order to avoid duplicate module issues.

So concretely:

• Support an --ignore-path flag that essentially supports a subset of gitignore syntax. The logic is applied in both find_sources and modulefinder
• Never recursively explore node_modules / site-packages
• Suggest --ignore-path for duplicate module errors

Finally, note 1 isn't a new issue (you can repro with mypy 0.790).

[JukkaL]

@hauntsaninja Sounds good!

There is still the question of whether we should release a new version with this new option, or should we first revert the change in a bugfix release. Even if we decide to add the option, we can just cherry-pick the new feature on top the the 0.800 release branch to speed up the release process (it would still be 0.810 instead of 0.801 because there's a new feature). I'm fine with both options, but we should do something pretty soon (maybe by early next week).

[JukkaL]

Also, maybe we should ignore all directories starting with a dot, such as .git.

[hauntsaninja]

I'll try and get a PR in sometime before Saturday evening.

Yup, we already do that (not new to v0.800):

mypy/mypy/find_sources.py

Line 106 in 4d5a1bc

if name == '__pycache__' or name.startswith('.') or name.endswith('~'):

[rmspeers]

I ran into an issue which seems related to item 2 in the post starting this thread, that "Having init.py files under any directory that's not a valid Python module name.". In 0.790, a directory name example_name_here (that contains a module that setup.py declares as example-name-here) is accepted and as it has an __init__.py, the module is scanned. As of 0.800, this no longer is the case and I get the invalid directory name error. Is there guidance for what needs to be adjusted in my source to upgrade to 0.800, if my structure is no longer acceptable?

[hauntsaninja]

mypy v0.800 didn't really change the "invalid module name" logic. If you're getting an error now, but weren't getting one with mypy v0.790, it's probably because mypy was not checking your code at all.

It's not clear to me from your message what your directory structure is. It sounds like:

example_name_here
├── example-name-here
│   └── __init__.py
└── setup.py

In which case, rename example-name-here to example_name_here.

[intgr]

Finally, note 1 isn't a new issue (you can repro with mypy 0.790).

Yeah, I had simplified the test case too much, fixed it now. The real example I saw was like you mention in your last comment; the code was entirely ignored by the previous mypy version.

[rmspeers]

mypy v0.800 didn't really change the "invalid module name" logic. If you're getting an error now, but weren't getting one with mypy v0.790, it's probably because mypy was not checking your code at all.

It's not clear to me from your message what your directory structure is. It sounds like:

example_name_here
├── example-name-here
│   └── __init__.py
└── setup.py

In which case, rename example-name-here to example_name_here.

Thanks @hauntsaninja for looking at this. Here is my structure:

.
├── MANIFEST.in
├── __init__.py
├── example_name_here
│   ├── __init__.py
│   ├── example_utils.py
│   ├── py.typed
│   └── test
│       ├── __init__.py
│       ├── test_example_utils.py
├── setup.cfg
└── setup.py

[hauntsaninja]

Yeah, you need to remove that stray __init__.py that you've got at the top level. The relevant change in behaviour in mypy v0.800 is that we use absolute paths to crawl, which fixes various inconsistencies between passing relative paths and absolute paths to mypy or issues caused by cwd. This was previously a common complaint or source of confusion, eg #9677, #8726 and many others.

That is, with mypy 0.790 on your tree, mypy example_name_here would work, but mypy $(pwd)/example_name_here would fail.

[rmspeers]

Thank you, that resolved!

[intgr]

I hate to be that guy again :) but I would be very pleased to see this release.

[hauntsaninja]

#10062 is the issue to follow for the release with this change. Jukka should be releasing soon :)