Code cleanups from running type checking lints #11938
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
eli-schwartz
commented
Jul 3, 2023
eli-schwartz
commented
- refactor to allow mypy to actually check some code
- move more mypy-only imports into TYPE_CHECKING blocks
- enable a new mypy error category
tristan957
approved these changes
Jul 5, 2023
| @@ -2068,7 +2068,7 @@ def _generate_deps(self, state: 'ModuleState', library: str, packages: T.List[st | |||
| ofile.write(package + '\n') | |||
| return build.Data([mesonlib.File(True, outdir, fname)], install_dir, install_dir, mesonlib.FileMode(), state.subproject) | |||
|
|
|||
| def _get_vapi_link_with(self, target: build.CustomTarget) -> T.List[build.LibTypes]: | |||
| def _get_vapi_link_with(self, target: CustomTarget) -> T.List[build.LibTypes]: | |||
There was a problem hiding this comment.
Seems like you could remove the build from build.LibTypes here too.
There was a problem hiding this comment.
Technically yes, but also not really. It could be done if that also gets imported at the top level, which it currently isn't.
What I did so far is remove build. from any objects that were already imported anyway, since in all those cases we might as well use it now that we went to all the effort of importing it.
|
Yeah, looks like functools.singledispatch actually uses type annotations at runtime, similar to pydantic, so while PEP 563 postponed evaluation is totally fine, not importing the module is a problem. I can roll back that specific import. |
|
Looks like this PR will eventually conflict with the one that I just opened :( |
|
There are now conflicts and CI has issues, but other than that, LGTM. |
eli-schwartz
force-pushed
the
type-checking-lints
branch
from
eadf68e
to
9f2abd0
Compare
dcbaker
approved these changes
Jul 19, 2023
eli-schwartz
force-pushed
the
type-checking-lints
branch
from
9f2abd0
to
55c98d2
Compare
So that we can later reference them.
A linker mixin has to be able to align with the base linker it will be used for, in order to reference super(). Since they weren't inherited, calls to super() resulted in mypy errors, which we ignored, and casting. Use the same trick we use for compilers, and make the linker inherit from the base linker type when running under mypy, and from object at runtime.
These don't have new errors or old ignored ones, but add them anyway so we can generally validate their sanity.
This detects cases where module A imports a function from B, and C imports that same function from A instead of B. It's not part of the API contract of A, and causes innocent refactoring to break things.
We already import a bunch of objects directly from ..build but don't use them nearly as much as we can. This resulted both in longer lines and s minor performance difference since python has to resolve the name binding the long way. There's no reason not to rewrite these names to use the direct imports. Found while investigating the fact that Executable was imported but never used. It's easier to just use it.
Mostly detected with flake8-type-checking. Also quote T.cast() first arguments, since those are not affected by future annotations.
eli-schwartz
force-pushed
the
type-checking-lints
branch
from
55c98d2
to
0bb1647
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.