Skip `deprecated` exports in autoimport

[DanielNoord]

Summary

References https://github.com/facebook/pyrefly/issues/1685

This implements one of the improvements discussed in the issue: taking into account deprecations when checking exports for autoimport.

I have added two tests. One tests whether we suggest imports from the stdlib. It doesn't seem we test this yet and I was figuring out how the tests worked and came up with this. I can remove it, but since I wrote it I thought I might as well keep it? It also shows an improvement we should make (typing over ast).

The second test covers the change I make in the PR. It is pretty simple really.

Test Plan

See above. Added two tests.

Also tried to run the CI on my local fork in https://github.com/DanielNoord/pyrefly/pull/1 but it seems the setup is quite different from the checks I see running on other PRs in this repository.

I did. There are quite a lot of warnings about unused lint ignores and unused exports, but my changes seem okay? Let me know if I missed anything.

[meta-cla[bot]]

Hi @DanielNoord!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[DanielNoord]

For the reviewer, I also found: https://github.com/facebook/pyrefly/blob/c0ebd8ce89543649ebe757181b22902e45dd0f86/pyrefly/lib/state/lsp.rs#L2226-L2230

There is a difference between completions and code actions, I guess? I'm not quite sure what the difference between them is, but I think the code on main already shows there is a potential for drift. In completions deprecation is taken into account while for code actions it isn't. I have now excluded deprecated exports from code actions (causing another type of drift with completions), mainly because search_exports_exact doesn't really provide a nice API to "downrank" handles and just either returns them or not.

Do we need to address this drift? And if so, how? And does this PR actually address the error I'm trying to fix? Or does the code action not really matter in practice?

[meta-cla[bot]]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[DanielNoord]

but then the bigger question of "how do we sort these correctly" is still unanswered.... I'll comment on the original task for that

This is also what I struggle with. I'm scared of overscoping this PR but am not sure what you're preferred course of action is. If this already provides value in your opinion I would merge it, but can also understand if you first want to answer the bigger question :)

[kinto0]

but then the bigger question of "how do we sort these correctly" is still unanswered.... I'll comment on the original task for that

This is also what I struggle with. I'm scared of overscoping this PR but am not sure what you're preferred course of action is. If this already provides value in your opinion I would merge it, but can also understand if you first want to answer the bigger question :)

I think this provides value if it adds the (deprecated) field. But I think removing quick fixes might not be what some users want. Let me look into how pyright sorts / filters these completion items....

[DanielNoord]

@kinto0 I have changed the test and code. It now shows we prefer the import from b.py over a.py due to deprecation and that deprecation is being shown in the action as well. Let me know what you think!

[DanielNoord]

@kinto0 Is there anything left to do here? If possible I'd like to avoid future merge conflicts :smile:

[meta-codesync[bot]]

@kinto0 has imported this pull request. If you are a Meta employee, you can view this in D89058677.

[kinto0]

@kinto0 Is there anything left to do here? If possible I'd like to avoid future merge conflicts 😄

sorry!!! not sure how I missed this. I swear I imported it when I renamed it. oh well, thanks for fixing the conflicts.

[meta-codesync[bot]]

@kinto0 merged this pull request in facebook/pyrefly@2b19816c043672b4a326fa44aaa527a991ce2279.