[ty] List available members for a given type

[sharkdp]

Summary

Provide a new ide_support::all_members function that lists all available attributes for a given type.

To do:

• Probably respect __all__ when accessing modules?
• Decide if we want to keep ty_extensions.all_members and the new <str_literal> in <tuple> inference support, that is probably only useful for this purpose.
• More tests

Test Plan

New Markdown tests

[github-actions]

mypy_primer results

No ecosystem changes detected ✅

[sharkdp]

This might be an interesting correctness-vs-UX tradeoff. When given a obj: str | None object, and tab-completing on obj.<tab>, it's technically correct to only show attributes that are available on str and None (so basically nothing, except for a few special methods on object that nobody wants to call directly).

But the more likely scenario is probably that the developer actually wants to access an attribute on str, and just forgot to eliminate the None possibility. So it might be more useful to show attributes that are available on str or None here instead? Not sure.

[InSyncWithFoo]

I feel like this is a semantic/definition problem. all_members() is defined as:

The ty_extensions.all_members function allows access to a list of accessible members/attributes on a given object.

...and that is exactly what it should do, no special cases allowed.

Cases like str | None demand a different function that would list members that are available on at least one union member.

[sharkdp]

Well I would change that docstring obviously, if we would decide to deliberate return members that are not available 😄 This function is only here to support that precise use case (providing completion results for the LPS). Making it available via ty_extensions.all_members was just a convenient way for me to test it. I'm not sure if this should really be exposed. Or if we should maybe move it to ty_extensions.internal or similar.

[BurntSushi]

I'm inclined to stick to what you have here for now (preferring correctness), and then loosen it up as use cases arise. Maybe, e.g., it makes sense to only loosen it in certain scenarios.

[dhruvmanila]

For reference, Pyright / Pylance does provide all members of the union type. In the following example, decode is only present on bytes while other methods are present on both:

[MichaReiser]

I suspect that this will require some iteration and/or even configurations. E.g. r-a has an option that allows you to configure if r-a should hide members to which you don't have access to due to visibility constraint. Hiding them is probably the right default when working with third-party code, but for first-party code, I often want to see all members, and I'll then change the visibility of the member.

So I think we should move forward here as we can change this easily in the future

[BurntSushi]

Yeah I think this should be easy to change.

I can imagine wanting configuration for respecting __all__ too.

[BurntSushi]

Decide if we want to keep ty_extensions.all_members and the new <str_literal> in inference support, that is probably only useful for this purpose.

I think this seems like the main blocker here for getting this merged. I'd be in favor of moving it to ty_extensions.internal, and if a use case arises to expose it in ty_extensions proper, then we could consider moving it then. Are there any downsides to exposing it in ty_extensions.internal?

[BurntSushi]

Probably respect __all__ when accessing modules?

From what I can tell, it might be undesirable to respect __all__, which I am loosely basing on this comment.

However, we do want to respect it when there's a glob-star import, but ty already does that (and I've added tests for it).

A relayed question here is whether we should respect __dir__ when defined on objects. I suspect not, since the cases where it's useful seem likely to be dynamic? Although I haven't done a survey.

the new <str_literal> in <tuple> inference support, that is probably only useful for this purpose

This I am not sure about. How do we go about deciding these sorts of questions?

Otherwise, I think a review at this point would be helpful for me, so I'm going to take this out of draft.

[dhruvmanila]

Overall, this looks good.

I've left a few comments stating the relevance of __all__ and the re-export conventions for imported symbols but that all can be done in a follow-up.

[dhruvmanila]

I'm sure you must've thought about this but in the future it would be useful to provide more information to the client like docstring, item kind, etc. and I'm not exactly sure how this API could help with that. I'm curious to hear your thoughts if any.

[dhruvmanila]

We could just return a Vec<Type<'db>> or FxHashSet<Type<'db>> here but then there's also another question of whether to actually store information like docstring on the type itself or whether to store it somewhere else.

[BurntSushi]

I 100% agree that we will want more information.

My rough plan here is that completions will use the AllMembers API directly, and I'd like to augment it to return more than just names. I don't know exactly what it should return yet. A Type seems like a good start I think? But that all_members as a ty_extension could still just return a tuple of strings.

[dhruvmanila]

Can we add a test case which populates the negative set in intersection? Like, if not isinstance(...)

[BurntSushi]

Done! I added this test:

```py
from ty_extensions import all_members, static_assert

class A:
    on_all: int = 1
    only_on_a: str = "a"
    only_on_ab: str = "a"
    only_on_ac: str = "a"

class B:
    on_all: int = 2
    only_on_b: str = "b"
    only_on_ab: str = "b"
    only_on_bc: str = "b"

class C:
    on_all: int = 3
    only_on_c: str = "c"
    only_on_ac: str = "c"
    only_on_bc: str = "c"


def f(intersection: object):
    if isinstance(intersection, A):
        if isinstance(intersection, B):
            if not isinstance(intersection, C):
                reveal_type(intersection)  # revealed: A & B & ~C
                static_assert("on_all" in all_members(intersection))
                static_assert("only_on_a" in all_members(intersection))
                static_assert("only_on_b" in all_members(intersection))
                static_assert("only_on_c" not in all_members(intersection))
                static_assert("only_on_ab" in all_members(intersection))
                static_assert("only_on_ac" in all_members(intersection))
                static_assert("only_on_bc" in all_members(intersection))
```

[dhruvmanila]

the new <str_literal> in <tuple> inference support, that is probably only useful for this purpose

This behavior exists and is valid at runtime so I'm not sure what do you mean by "only useful for this purpose" (cc @sharkdp)

[sharkdp]

the new <str_literal> in <tuple> inference support, that is probably only useful for this purpose

This behavior exists and is valid at runtime so I'm not sure what do you mean by "only useful for this purpose" (cc @sharkdp)

There's (hopefully) nothing wrong with the precise True/False type inference for these binary expressions. I was merely questioning if we should keep the extra code just for the purpose of writing these mdtests. For users of ty, there's probably not much value in inferring literal True/False over bool for these cases that are unlikely to occur in real world code.

I guess @AlexWaygood could also use a this feature for some protocol tests (instead of always listing all members)? And maybe the all tests could also use a similar technique

[BurntSushi]

I'm going to bring this in, but happy to address any other feedback in a follow-up PR!

[BurntSushi]

And thank you @sharkdp for doing the lion's share of the work here!!!

[carljm]

Looks great! Just a couple minor nits, not worth a separate PR but for consideration if a future PR touches these areas

[carljm]

I'm not sure what the "type error" being referred to here is -- the only error I see in this section is static-assert-error? I would expect this header to say something more like "statically unknown returns bool"

[BurntSushi]

Yeah, I was calling static-assert-error a type error, which I guess is not quite right? Can you say more about "statically unknown returns bool"? ty is still returning an error here when x in y is both unknown and used in a static context. What about, "Statically unknown results in error in static context" instead?

[carljm]

It seems like you're more commenting on the behavior of static_assert here -- "static context" isn't really a thing otherwise. static_assert is really just a testing tool for us to easily make assertions about things being statically known. It asserts that some expression evaluates to Literal[True] and emits static-assert-error otherwise. But (outside of a different mdtest specifically for that purpose) I don't think mdtests should be commenting on the behavior of static_assert; the focus should be on the type system behavior under test (which in this case is <literal-str> in <tuple>). And the relevant behavior being tested here is that we infer the in expression as bool (not as Literal[True] or Literal[False]) if we can't statically determine the result of the in test.

On second look I think what this really underscores is that static_assert is just the wrong tool for this particular test; it would be clearer here to simply use reveal_type and assert that the revealed type of the expression is bool.

[BurntSushi]

Got it! Thank you for talking me through this. I really appreciate it!

I put up a new PR with these fixes: #18388

[carljm]

this calls it a list, but it's a tuple