Skip to content

Support blocks with implicit parameters (_1, it) and backtick commands - #1320

Open
apiology wants to merge 8 commits into
castwide:masterfrom
apiology:fix-numblock-return-type
Open

apiology wants to merge 8 commits into
castwide:masterfrom
apiology:fix-numblock-return-type

Conversation

@apiology

@apiology apiology commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Three block and literal forms have no type at --level strong, so calls on them are unresolved and enclosing methods lose their return type.

# @return [Array<String>]
def numbered
  'a,b'.split(',').map { _1.upcase }   # Unresolved call to _1
end

# @return [Array<String>]
def implicit_it
  'a,b'.split(',').map { it.upcase }   # Unresolved call to it
end

# @return [String]
def backticks
  `hostname`.strip                     # Unresolved call to strip
end

All are clean in explicit form. A wrong tag also passes silently on the implicit block form: @return [Array<Integer>] on 'a,b'.split(',').map { it } reports nothing.

_1 produces a numblock node, so the call was chained as though no block were passed. it arrives as an ordinary block missing only its parameter — hence the silent pass. Backticks produce an xstr node, absent from the literal type table. Neither block form records parameters as an args node does, so both are synthesized; a local named it still wins, matching Ruby.

This PR was written by Claude Code on behalf of @apiology.

apiology and others added 4 commits August 19, 2026 13:24
`xs.any? { x.include?(_1) }` failed to typecheck while the identical
`xs.any? { |s| x.include?(s) }` was clean, and `xs.map { _1.upcase }`
inferred `Enumerator<undefined, Array>` instead of `Array<String>`.

The `parser` gem emits `numblock` rather than `block` for a block that
uses numbered parameters. `:numblock` was not registered with
`Solargraph::Parser::NodeProcessor` and was not recognized anywhere that
tests for `:block`, so no `Pin::Block` was created, the call was chained
as if it had no block, and `_1` resolved to nothing.

- Register `:numblock` with `BlockNode`, which now synthesizes `_1.._N`
  parameter pins from the node's numbered-parameter count (a numblock
  stores that Integer where a block stores its args node).
- Recognize `:numblock` alongside `:block` in `NodeChainer#generate_links`
  and `#passed_block`, in `NodeMethods.call_nodes_from`, in
  `DeepInference::FUNCTION_VALUE` and its two open-coded `:block` checks,
  and in `TypeChecker#call_problems`.

`it` / `itblock` is a separate node type and is not covered here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AsvDi68YqsKoBtS2kg9ch
`xs.map { it.upcase }` reported `Unresolved call to it` while
`xs.map { |s| s.upcase }` was clean, and `@return [Array<Integer>]` on
`xs.map { it }` was accepted where the explicit-parameter form is
correctly rejected.

Prism translates an `it` block into an ordinary :block node with an empty
args node, so nothing on the node records the parameter and ArgsNode has
nothing to build from. The only signal is an `it` local variable
reference in the body, so BlockNode scans for one and synthesizes the
parameter when it finds it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AsvDi68YqsKoBtS2kg9ch
`\`echo hi\`.strip` reported "Unresolved call to strip", and any method
returning a backtick result reported "return type could not be inferred".

`infer_literal_node_type` had cases for `str` and `dstr` but none for
`xstr`, so `NodeChainer#generate_links` fell through to its else branch
and pushed a bare `Chain::Link` with no type. Adding `xstr` to the
existing String group covers both the literal and interpolated forms,
which parse to the same node type and differ only in the xstr's children.

The considered alternative was to model an xstr as a send to Kernel#`,
which is typed `(String) -> String` in RBS. Not taken: `dstr` is the
controlling precedent, since it is also interpolated and also typed as a
literal rather than as a call, and every other entry in this method is a
node-type-to-class mapping. Routing one node type through call
resolution would make it the only exception.

This removes the need for the `@sg-ignore Need backtick support` comment
and its accompanying manual `@type [String]` in solargraph.gemspec,
which typecheck now reports as an unneeded @sg-ignore comment.
@apiology apiology changed the title Support blocks with implicit parameters (_1, it) Support blocks with implicit parameters (_1, it) and backtick commands Aug 19, 2026
apiology added a commit to apiology/solargraph that referenced this pull request Aug 21, 2026
…h 2026-08-04

# Conflicts:
#	lib/solargraph/parser/parser_gem/node_chainer.rb
#	lib/solargraph/parser/parser_gem/node_methods.rb
#	lib/solargraph/parser/parser_gem/node_processors/block_node.rb
#	spec/parser/node_processor_spec.rb
#	spec/type_checker/levels/strong_spec.rb
Comment thread lib/solargraph/parser/parser_gem/node_processors/block_node.rb
Rename add_parameter to add_implicit_parameter, per review
comment. Document the numblock node shape (a regular block with
no args node) on add_implicit_parameters, and add a full sexp
example to the numbered_parameter_count docstring. Replace the
placeholder "Translate to something flow sensitive typing
understands" sg-ignore text with a real slug identifying the
actual limitation: flow-sensitive typing does not narrow through
a predicate method (like Parser.is_ast_node?) that wraps an
is_a? check internally.
apiology added a commit to apiology/solargraph that referenced this pull request Sep 1, 2026
Docstring and inline comment on blame_description exceeded the
review's 3-line and 1-line budgets. Also swap the magic-string
comparison for the existing Source::Chain::Link#undefined? predicate.

Point the numblock pending spec at castwide#1320, which
covers the missing NodeChainer :numblock branch, instead of narrating
the gap inline.
# Conflicts:
#	spec/parser/node_processor_spec.rb
Address review: shorten the add_implicit_parameters and
numbered_parameter_count docstrings to fit the repo's per-method
comment budget, dropping a redundant numblock aside and an
s-expression parse-tree example. Add a concrete example to
shadowed_it_local?'s docstring showing why an explicit and a
synthesized `it` parameter can't be told apart, so honoring
Ruby's real scoping for an explicit `it` parameter would also
break nested implicit `it` blocks.
The implicit-parameter methods carried 8- and 11-line docstrings where
the repo standard is 1-3 lines, budgeted per method. Each now states
only the constraint a cold reader needs: why these blocks have no args
node, why an empty args node is a precondition rather than a guess, and
why an outer explicit `|it|` does not shadow the inner block's implicit
one.

The five ignore markers used a tool-limitation:* slug, a taxonomy from a
downstream audit that no other marker in lib/ uses, and spread one of
them over five lines. They now carry single-line plain-English reasons
matching the convention already in lib/. numbered_parameter_count guards
with a literal is_a?, so it takes the catalogued "needs to narrow down
type with an if is_a? check" string; the Parser.is_ast_node? sites name
that distinct gap instead, which no upstream issue covers.

Comments only. Typecheck at strong reports the same four problems before
and after, shifted by the 25 removed lines.
@apiology
apiology marked this pull request as ready for review September 6, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant