-
-
Notifications
You must be signed in to change notification settings - Fork 597
feat(gazelle): Add type-checking only dependencies to pyi_deps #3014
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks! This will be a really nice improvement.
Couple requests:
- Guard this behind a gazelle directive. Maybe
# gazelle:python_generate_pyi_deps
(defaultfalse
for now).- Or
python_generate_pyi_deps_attribute
?python_use_pyi_deps_attribute
?
- Or
- Add the directive to
gazelle/README.md
- the table of directives and a section for the directive. - In the section for the directive, document that only two forms are supported:
if typing.TYPE_CHECKING:
andif TYPE_CHECKING:
.
For example, if someone doesor other odd stuff like that, then the imports won't be marked asimport typing as foo if foo.TYPE_CHECKING: ...
pyi_deps
.
@@ -55,6 +55,8 @@ END_UNRELEASED_TEMPLATE | |||
{#v0-0-0-changed} | |||
### Changed | |||
* (gazelle) Types for exposed members of `python.ParserOutput` are now all public. | |||
* (gazelle) Dependencies added to satisfy type-only imports (`if TYPE_CHECKING`) and type stub packages are |
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.
Nit: Wrap at ~80 chars please
condition := node.Child(1) | ||
|
||
// Handle `if TYPE_CHECKING:` | ||
if condition.Type() == sitterNodeTypeIdentifier && condition.Content(p.code) == "TYPE_CHECKING" { |
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.
Can tree sitter (or something else) inspect the underlying python object? That might allow us to support cases when people do odd things like import typing as foo
or from typing import TYPE_CHECKING as TypeCheckingWithCamelCaseBecaseTheManagerLovesJava
.
To clarify: I don't mind only supporting the two sane cases, but if it's trivial to support the insane arbitrary cases that would be nice.
if dep.TypeCheckingOnly { | ||
t.pyiDeps.Add(dep) | ||
} else { | ||
t.deps.Add(dep) | ||
} |
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.
Can this use the new addDependency
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.
Update gazelle/python/testdata/add_type_stub_packages/README.md
:
-it is added to the `deps` of the `py_library` target
+it is added to the `pyi_deps` of the `py_library` target
|
||
manifest: | ||
modules_mapping: | ||
boto3: boto3 |
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.
Let's add boto3_stubs
to assert that the TYPE_CHECKING
and _stubs
/_types
support interact well.
from typing import TYPE_CHECKING | ||
|
||
# foo should be added as a pyi_deps, since it is only imported in a type-checking context, but baz should be | ||
# added as a deps. |
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.
Nice comment, thanks!
@@ -0,0 +1 @@ | |||
# gazelle:python_generation_mode file |
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 think we'll need to add test cases for package and project generation mode too 🫤
#2538 added the attribute
pyi_deps
to python rules, intended to be used for dependencies that are only used for type-checking purposes. This PR adds support for this attribute to gazelle:if TYPE_CHECKING:
block), the dependency is added topyi_deps
instead ofdeps
;boto3-stubs
) are now added topyi_deps
instead ofdeps
.