-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Support module-level strict
config flag
#12174
base: master
Are you sure you want to change the base?
Conversation
The current behavior of `strict = True` in a module-level config section actually enables all strictness flags globally, which seems like a bug. This change changes a module-level `strict = True` option to act as though all of the per-module strictness flags were enabled.
This comment has been minimized.
This comment has been minimized.
mypy/config_parser.py
Outdated
@@ -205,14 +207,17 @@ def parse_config_file(options: Options, set_strict_flags: Callable[[], None], | |||
os.environ['MYPY_CONFIG_FILE_DIR'] = os.path.dirname( | |||
os.path.abspath(config_file)) | |||
|
|||
set_strict_flags_toplevel = partial(set_strict_flags, True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can create a real function instead? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed a small refactor using inline functions with a stronger unit test.
FYI the CI pipelines seem to be failing due to: pypa/setuptools#3102 |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
I pushed a fix for CI in #12181 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mypy/config_parser.py
Outdated
@@ -523,7 +532,7 @@ def parse_mypy_comments( | |||
stderr = StringIO() | |||
strict_found = False | |||
|
|||
def set_strict_flags() -> None: | |||
def set_strict_flags(ignored: Any) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer having reasonable name/type even if ignored.
Any update on this? The current behavior feels like a bug to me too, I'd love to see this fixed :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
03fa283
to
0317f43
Compare
Diff from mypy_primer, showing the effect of this PR on open source code: AutoSplit (https://github.com/Toufool/AutoSplit)
+ src/AutoSplit.py:25:1: error: Module "capture_method" does not explicitly export attribute "CaptureMethodBase" [attr-defined]
pyjwt (https://github.com/jpadilla/pyjwt)
+ jwt/algorithms.py:190: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
+ jwt/algorithms.py:195: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
+ jwt/algorithms.py:199: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
+ jwt/api_jws.py:182: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
+ jwt/api_jws.py:228: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
+ jwt/api_jwt.py:236: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
+ jwt/api_jwt.py:287: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
|
Description
The current behavior of
strict = True
in a module-level config section actually enables all strictness flags globally, which seems like a bug. This change changes a module-levelstrict = True
option to act as though all of the per-module strictness flags were enabled.Test Plan
In addition to adding a basic unit test (the
strict
flag is hard to test since it is essentially a macro flag) I tested manually using a config file like this:I then confirmed that strictness flags were applied to non-
foo
packages (I removed some type hints and saw an error) and that strictness was not applied to thefoo
package (removed some type hints and saw no errors).Personally I expected the
strict
config flag to behave this way already, so I don't think a change to the docs is needed.