Filter private symbols from stubs if they are internal types#19121
Filter private symbols from stubs if they are internal types#19121
Conversation
|
This comment was marked as outdated.
This comment was marked as outdated.
| public_explicit_type_alias: TypeAlias = Literal[1] | ||
| _private_explicit_type_alias: TypeAlias = Literal[1] |
There was a problem hiding this comment.
@AlexWaygood why "A private type alias that is explicitly annotated with typing.TypeAlias"? It looks like the behavior doesn't change either way.
There was a problem hiding this comment.
i.e., the naive alternative passes
diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs
index 9a6190cb5..0ee759051 100644
--- a/crates/ty_ide/src/completion.rs
+++ b/crates/ty_ide/src/completion.rs
@@ -536,6 +536,9 @@ _private_type_var_tuple = TypeVarTuple("_private_type_var_tuple")
public_explicit_type_alias: TypeAlias = Literal[1]
_private_explicit_type_alias: TypeAlias = Literal[1]
+public_implicit_type_alias = Literal[1]
+_private_implicit_type_alias = Literal[1]
+
class PublicProtocol(Protocol):
def method(self) -> None: ...
@@ -558,6 +561,8 @@ class _PrivateProtocol(Protocol):
test.assert_completions_do_not_include("_private_type_var_tuple");
test.assert_completions_include("public_explicit_type_alias");
test.assert_completions_include("_private_explicit_type_alias");
+ test.assert_completions_include("public_implicit_type_alias");
+ test.assert_completions_include("_private_implicit_type_alias");
test.assert_completions_include("PublicProtocol");
test.assert_completions_do_not_include("_PrivateProtocol");
}There was a problem hiding this comment.
oh whoops -- for some reason I thought that my patch excluded explicitly declared type aliases with private names. But you're quite right -- it doesn't do that. We'd probably need to add a new variant to the DynamicType enum to distinguish type-alias Todo types from other Todo types in order to get that working. (Shouldn't be too hard to do as a followup, I can pick it up.)
There was a problem hiding this comment.
I'll leave it to you then :)
| /// Unlike [`private_symbols_in_stub`], this test doesn't use a `.pyi` file so all of the names | ||
| /// are visible. | ||
| #[test] | ||
| fn private_symbols_in_module() { |
There was a problem hiding this comment.
It could be nice to avoid the repetition here, but I don't love optimizing tests for that early. Thoughts?
There was a problem hiding this comment.
We could also just test a few of the "do not include" cases here.
There was a problem hiding this comment.
For just two tests, I think this is fine to be honest. (And it seems like two cases is what we want.)
74af9ee to
65f7338
Compare
This implements filtering of private symbols from stub files based on type information as discussed in #19102. It extends the previous implementation to apply to all stub files, instead of just the
builtinsmodule, and uses type information to retain private names that are may be relevant at runtime.