Fix types/provide-type resolving the wrong token in notebook cells#3947
Fix types/provide-type resolving the wrong token in notebook cells#3947qnox wants to merge 1 commit into
Conversation
Summary: `provide_type` passed the request position straight to `from_lsp_position(position, None)`, ignoring the notebook cell. For a cell other than the first, an LSP position is cell-relative, so dropping the cell offset resolves the position in the concatenated module at the wrong line — landing in an earlier cell. The IDE drives hover/type resolution (and the completion-trigger type lookup) through this method, so types in every cell after the first came back wrong or empty. Thread the code-cell index (as the completion handler already does) through `provide_type` and translate the position with it, so cell-relative positions map to the correct concatenated-module offset.
|
Hi @qnox! Thank you for your pull request and welcome to our community. Action RequiredIn 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. ProcessIn 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 If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thanks! Once the CLA is signed we can merge it |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
Hi @qnox thank you for the fix, it is a hard requirement to sign the CLA before contributing to Pyrefly. We are glad to have you aboard and for us to merge your PR after you sign this https://code.facebook.com/cla/individual |
3 participants
Summary
provide_typepassed the request position straight tofrom_lsp_position(position, None), ignoring the notebook cell. For a cell other than the first, an LSP position is cell-relative, so dropping the cell offset resolves the position in the concatenated module at the wrong line - landing in an earlier cell. The IDE drives hover/type resolution (and the completion-trigger type lookup) through this method, so types in every cell after the first came back wrong or empty.Thread the code-cell index (as the completion handler already does) through
provide_typeand translate the position with it so cell-relative positions map to the correct concatenated-module offset.Test Plan
Added a regression test
test_notebook_provide_type_second_cellinpyrefly/lib/test/lsp/lsp_interaction/notebook_provide_type.rs: requests the type ofbarin the second cell and assertstyping.Literal[1]. Fails on main (wrong cell resolved), passes with the fix.