Skip to content

Add --disallow-any-generics to --strict #5685

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

Merged
merged 28 commits into from
Oct 11, 2018

Conversation

TV4Fun
Copy link
Contributor

@TV4Fun TV4Fun commented Sep 27, 2018

This is a tiny bit hacky as --disallow-any-generics is not defined
through add_invertible_flag. Resolves #5684.

@ilevkivskyi

This is a tiny bit hacky as --disallow-any-generics is not defined
through `add_invertible_flag`.
@TV4Fun
Copy link
Contributor Author

TV4Fun commented Sep 27, 2018

BTW, I think --disallow-any-generics is not a good name for this flag, as it implies that it doesn't allow generics explicitly parameterized with Any. I think a better name would be --disallow-unparameterized-generics.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Thanks! Could you please add a test for this (see check-flags.test)?

mypy/main.py Outdated
'type parameters').dest
assert dest is not None
strict_flag_names.append('--disallow-any-generics')
strict_flag_assignments.append((dest, True))
Copy link
Member

Choose a reason for hiding this comment

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

Why can't we just use add_invertible_flag here? Like, for example, --disallow-subclassing-any above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that does make more sense. Fixed to do that now.

gvanrossum and others added 25 commits October 4, 2018 14:32
In preparation of working on PEP 420 namespaces, here's some initial work. IT DOES NOT IMPLEMENT NAMESPACES YET. What it does do:

- Move nearly 400 lines from build.py to a new file, `modulefinder.py`.  This includes `SearchPaths`, `BuildSource`, `FindModuleCache`, `mypy-path`, `default_lib_path`, `get_site_packages` (now without underscore!), and `compute_search_paths`.
- Slight refactor to `FindModuleCache` so that the `search_paths` are passed to the constructor instead of to `find_module`.
- Removed `search_paths` and `python_executable` from the signature of `find_module` and `find_modules_recursive`, and also from the cache key (**this may be the most controversial change** -- it's not just a refactor).
- Add a (non-functional) `--namespace-packages` flag to `main.py` and add new global config option `namespace_packages`. (These are not used yet.)

I'm presenting this as a separate PR because it's a significant refactor without change of functionality -- everything I plan to do after this will mostly tweak the code in `modulefinder.py`. It seems fairer to reviewers to be able to review this massive code move without having to worry too much about "what else changed".

(Note that this is part of my general plan to move functionality out of `build.py` -- that file is (still) way too bulky.)
* Get rid of trivial type_check_only() function

* Remove bin_dir -- it's no longer used

* Fix wrong import in testcheck.py

* Fix indent of compute_search_paths() args

* Minor cleanup in mypy/test/testcheck.py

* Cleanups in mypy/dmypy_server.py and mypy/server/

* Fix lint

* Fix lint fix
…on#5711)

This commit fixes two issues with said code sample:
  - The math import was missing.
  - The function argument didn't match the variable name used in the
    function body.

Also made comment spacing consistent.
In conjunction with a change that makes mypyc able to directly
generate calls to `__bool__`, this allows us to generate a much more
direct check for truthiness of TypeInfo.
Fixes python#4872 
Fixes python#3876
Fixes python#2678 
Fixes python#5199 
Fixes python#5493 
(It also fixes a bunch of similar issues previously closed as duplicates, except one, see below).

This PR fixes a problems when mypy commits to soon to using outer context for type inference. This is done by:
* Postponing inference to inner (argument) context in situations where type inferred from outer (return) context doesn't satisfy bounds or constraints.
* Adding a special case for situation where optional return is inferred against optional context. In such situation, unwrapping the optional is a better idea in 99% of cases. (Note: this doesn't affect type safety, only gives empirically more reasonable inferred types.)

In general, instead of adding a special case, it would be better to use inner and outer context at the same time, but this a big change (see comment in code), and using the simple special case fixes majority of issues. Among reported issues, only python#5311 will stay unfixed.
Replace the `var_arg` and `kw_arg` attributes with methods in
`CallableType`.  This speeds up type checking slightly and improves 
memory use, as we only construct the value as needed.

Subtype checks for callables are now slower, but that's fine since we
construct callables at least 10x as frequently as we perform callable
subtype checks.

Also remove a somewhat expensive assertion.

This seems to give about 2-3% performance improvement on self-check
(not compiled).
This implements various small optimizations to make AST conversion
from the typed_ast AST faster in compiled mypy. This makes compiled
mypy about 4% faster on self-check, and it also makes non-compiled
mypy around 1% faster on self-check.

Notable included optimizations:

* Implement visit methods more efficiently. Don't inherit visitors from
  typed_ast visitor classes.
* Refactor decorated methods away, as they are slow with mypyc.
* Refactor away many non-native attribute lookups.
* Refactor away some nested functions as they are slow with mypyc.

Also removed some dead code.

I'll implement the same optimizations for Python 2 AST conversions in
a separate PR.
Tentative implementation of PEP 420. Fixes python#1645.

Clarification of the implementation:

- `candidate_base_dirs` is a list of `(dir, verify)` tuples, laboriously pre-computed.
- The old code just looped over this list, looking for `dir/<module>`, and if found, it would verify that there were `__init__.py` files in all the right places (except if `verify` is false); the first success would be the hit;
- In PEP 420 mode, if the above approach finds no hits, we do something different for those paths that failed due to `__init__` verification; essentially we narrow down the list of candidate paths by checking for `__init__` files from the top down. Hopefully the last test added clarifies this.
This seems to speed up compiled mypy by up to about 4%, with marginal
changes to memory use.
Note that python#5728 is not fixed by this yet! The behavior is unchanged
except it is no longer listed in the `--help` output.

Also add `file=sys.stderr` to a few of the `print()` calls for other
deprecated options.

Also mark it deprecated in the docs (but don't delete those docs yet).
This reverts commit fc89540.
@TV4Fun
Copy link
Contributor Author

TV4Fun commented Oct 4, 2018

Added a couple of test cases for --disallow-any-generics. Interestingly, though there were already two test cases for --disallow-any-generics, neither actually tested for an error condition, so added that. I wanted to add a test for --strict as there aren't any now, but that seemed to create some errors unrelated to my test case:

----------------------------- Captured stderr call -----------------------------
Expected:
  main:7: error: Missing type parameters for generic type (diff)
Actual:
  tmp/builtins.pyi:8: error: Function is missing a type annotation (diff)
  tmp/builtins.pyi:34: error: Call to untyped function "object" in typed context (diff)
  main:7: error: Missing type parameters for generic type (diff)

Alignment of first line difference:
  E: main:7: error: Missing type parameters for generic type
  A: tmp/builtins.pyi:8: error: Function is missing a type annotation

The test that caused this was:

[case testCheckStrictAnyGeneric]
# flags: --strict
from typing import TypeVar, Callable

T = TypeVar('T')
C = Callable[[], T]

def f(c: C) -> None:  # E: Missing type parameters for generic type
    pass
[builtins fixtures/list.pyi]
[out]

Not sure if this is a known issue or not.

@JelleZijlstra
Copy link
Member

This is probably because the stubs we use for builtins in tests aren't --strict-clean. Might be worth fixing.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests!

I think your issue with errors in builtins will be fixed by not using the builtins fixture you don't need.

Alternatively, you can just fix the errors in that file (fixtures/list.pyi).


def f(c: C):
pass
[builtins fixtures/list.pyi]
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need the fixture here and in other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, removed.

@ilevkivskyi
Copy link
Member

(Also would be good to fix the commit history, but it is not essential since we will squash anyway.)

@TV4Fun
Copy link
Contributor Author

TV4Fun commented Oct 10, 2018

Comments addressed. I agree the commit history is a little messed up, but as you said, you are going to squash anyway.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Thanks! Looks ready now. I have only one suggestion about one test.

@ilevkivskyi ilevkivskyi merged commit d5b4f74 into python:master Oct 11, 2018
@ilevkivskyi
Copy link
Member

ilevkivskyi commented Oct 11, 2018

@gvanrossum Does it make sense to cherry-pick in the release branch? The point is that it should only affect people who use --strict, and those are presumably fine with the extra errors this may cause.

Edit: merge -> cherry-pick.

@gvanrossum
Copy link
Member

I'm sorry, the 0.640 release train has already closed its doors and is firing up its engine. It's too late too get on board. Take the next one in 3-4 weeks!

suutari-ai pushed a commit to suutari/meterelf that referenced this pull request Jan 27, 2019
This flag is enabled by "mypy --strict" since mypy 0.650 so I want to
enable it in the default configuration too.

See python/mypy#5685
@TV4Fun TV4Fun deleted the strict-dissalow-any-generics branch July 9, 2021 13:34
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.