Repository navigation
Handle pipe context across signature help, inlay hints, and providers - #803
Conversation
eitsupi
left a comment
There was a problem hiding this comment.
ChatGPT-assisted code review
I reviewed this PR with ChatGPT. Overall, the implementation looks good, particularly the handling of implicit pipe arguments, placeholders, and argument matching without evaluating user code. The regression test coverage is also appreciated.
ChatGPT identified two non-blocking points worth considering:
-
%T>%support: The tee pipe is handled in class indexing, references, and refactoring, but not in signature help or inlay hints (R/inlay_hint.R,src/provider_index.c, andsrc/signature.c). Since%T>%passes its LHS to the RHS in the same way as%>%, it would be good to handle it consistently. -
Extract refactoring restrictions: The new check in
refactor_expression_movable()prevents extracting any subexpression within a pipe RHS, including independent expressions such as1 + 2inx |> sum(1 + 2). I understand the conservative approach to avoid breaking placeholder-dependent expressions, but it may be worth narrowing this restriction in a follow-up.
Neither seems serious enough to block this PR. Approved. Thanks for the comprehensive work!
Piped calls currently lose their implicit argument context in several language features. For example,
x |> findInterval()highlightsxinstead ofvec, andx |> fun1(1, 2)labels the explicit arguments asarg1/arg2instead ofarg2/arg3.This change accounts for implicit pipe arguments and explicit native
_or magrittr.placeholders in signature help and inlay hints for|>,%>%,%<>%, and%T>%, including placeholders after the cursor, nested calls, and incomplete edits. Cached inlay hints are invalidated when their pipe context changes.It also fixes the related provider issues:
x |> obj$run()without evaluating user code.x %>% targetin call hierarchy and CodeLens while preserving namespace and local binding identity.1 + 2inx |> sum(1 + 2). Keep whole-RHS calls, placeholder-dependent expressions, and data-mask expressions in their pipe context; extracting the whole pipe remains available.Magrittr source-file discovery (
"file.R" %>% source()) is outside this PR.Validation
test-code-action.Randtest-lintr.R. A worker probe confirmed it loaded the user-level lintr configuration rather than the isolated test configuration. Both affected files passed with the isolated serial runner.git diff --checkpass.