Skip to content

pin CLI: resolve paths through method lookup, unsilence messages, fix bare arrows - #1312

Open
apiology wants to merge 11 commits into
castwide:masterfrom
apiology:pin-cli-ux
Open

pin CLI: resolve paths through method lookup, unsilence messages, fix bare arrows#1312
apiology wants to merge 11 commits into
castwide:masterfrom
apiology:pin-cli-ux

Conversation

@apiology

Copy link
Copy Markdown
Contributor

Problem

Three solargraph pin behaviors made diagnosis harder than it needs to be.

A path that names no pin of its own described nothing, even when Ruby would happily call the method. Given:

module Mixin
  # @return [String]
  def helper
    'from mixin'
  end
end

class Child
  include Mixin
end
$ solargraph pin 'Child#helper'
$ echo $?
1

Nothing printed at all — which is the second bug. bin/solargraph sets $VERBOSE = nil to silence Ruby diagnostics, and that also turns Kernel#warn into a no-op, so the command's "Pin not found" message never reached the user. The same suppression applied to every warn-based CLI message: gem-not-found and caching progress in gems/cache/uncache, scan errors, and the invalid-SOLARGRAPH_LOG notice.

An unparameterized generic rendered no return type at all, because UniqueType#to_rbs returns nil for that tag and callers interpolate it away. The result is not valid RBS — RBS::Parser raises ParsingError on it:

$ solargraph pin 'Container#items' --rbs
def items: () ->

Solution

  1. Resolve the path through Ruby method lookup by default, describing the definition a call would actually reach:

    $ solargraph pin 'Child#helper'
    #<Solargraph::Pin::Method `name="helper" return_type=Mixin#helper def helper: () -> String, context=Mixin, closure="Mixin", binder=Mixin` at ...>
    $ echo $?
    0
    

    A new --resolve (default true, with Thor's --no-resolve) controls it; --no-resolve restores exact-path-only lookup. Lookup runs only when the path names no pin of its own, so output for a path that resolves exactly is byte-identical to before — verified across method and namespace paths against this base. Nothing extra is printed when resolution happens: the description names its own owner (closure="Mixin" above), so stdout stays a single clean record and --rbs still emits one parseable signature. Resolved-via-lookup exits 0; nothing found anywhere still exits 1. --stack already walks the ancestry and lists the whole chain, so it supersedes resolution.

  2. Print user-facing messages via $stderr.puts, which $VERBOSE = nil cannot silence — the pin miss plus 13 other sites. One developer-diagnostic dump in Pin::Base#choose moves to logger.warn instead. Style/StderrPuts is already disabled repo-wide citing this hazard.

  3. Render an unparameterized generic as untyped, which restores valid output: def items: () -> untyped parses as RBS; the bare arrow does not.

Test plan

Eight new specs. They pin that an exact-path hit is byte-identical and never consults method lookup, that resolution happens by default and prints only the pin, that --no-resolve restores the old failure, that a total miss still exits 1, that pin-miss and gems-miss messages are visible under $VERBOSE = nil as bin/solargraph runs, and that generic renders as untyped both at the ComplexType level and through a method signature. Full suite: no new failures.

Opened as a draft. This PR was written by Claude (Anthropic's Claude Code) on behalf of @apiology.

🤖 Generated with Claude Code

https://claude.ai/code/session_01H1FEjW6nMpZrWPmeWX9miT

apiology and others added 4 commits August 16, 2026 21:39
bin/solargraph sets $VERBOSE = nil to silence Ruby diagnostics, which
also turns Kernel#warn into a no-op - so the pin command's "Pin not
found" message never reached users; the command exited 1 with no
output. Print via $stderr.puts instead.

On a method-path miss, also try the receiver's ancestry via
get_method_stack before giving up: solargraph pin 'Integer#between?'
now notes "showing 'Comparable#between?' (found via ancestry)" and
prints that pin. The --stack branch reuses the same helper, dropping
its hand-rolled split and a stale @sg-ignore (shell.rb strong problems:
10 -> 9).

(SKIP=Solargraph: the dogfood hook fails on shell.rb's pre-existing
strong-level problems - Vernier constants, missing @return on #rbs -
which this commit reduces by one and otherwise leaves untouched;
verified by message-set diff against the clean base.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H1FEjW6nMpZrWPmeWX9miT
UniqueType#to_rbs returned nil for a generic type tag with no bound
parameter; callers interpolate that into signatures, producing a bare
arrow with no return type at all ("def to_a: () -> ") in pin
inspection and --rbs output. Fall back to untyped, matching how every
other unresolvable type already renders.

(SKIP=Solargraph: pre-existing strong-level problems in unique_type.rb,
untouched by this commit; verified by message-set diff.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H1FEjW6nMpZrWPmeWX9miT
Audit of every remaining receiverless warn in lib/ (bin/ has none),
which bin/solargraph's $VERBOSE = nil turns into a silent no-op:

- shell.rb (12 sites: gems/cache/uncache misses, cache progress, scan
  errors): user-facing; converted to $stderr.puts. The same methods
  already used $stderr.puts elsewhere, so the warn uses were accidental.
- logging.rb (invalid SOLARGRAPH_LOG value): user-facing config error,
  raised before the logger exists; converted to $stderr.puts.
- pin/base.rb#choose (combine-failure dump before re-raise): developer
  diagnostics; converted to logger.warn (the class includes Logging).

Style/StderrPuts is already disabled repo-wide citing this hazard, but
no stock cop enforces the reverse direction, so the conversions are
unguarded against regression.

A second spec runs the gems-miss under $VERBOSE = nil.

(SKIP=Solargraph: pre-existing strong-level problems in the touched
files, unchanged - verified by message-set diff against the clean
base.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H1FEjW6nMpZrWPmeWX9miT
`pin 'Child#helper'` described nothing when Child inherits helper from
Base: the path names no pin of its own, so the command failed. But the
method a call reaches is Base#helper, and that is what a user asking
about Child#helper wants described.

Resolution through method lookup is now the default, replacing the
previous exact-path-only behavior:

- New --resolve/--no-resolve/--skip-resolve (default true). --no-resolve
  restores exact-path-only lookup.
- Exact hits are untouched: lookup runs only when the path itself names
  no pin, so output for a path that resolves exactly is byte-identical
  to before (verified across method and namespace paths against
  origin/master).
- Nothing extra is printed when resolution happens - the described pin
  names its own owner, so stdout stays a single clean description and
  --rbs consumers still get one parseable signature.
- Resolved-via-lookup exits 0; nothing found anywhere still exits 1.
- --stack already walks the ancestry and lists the whole chain, so it
  supersedes resolution.

The --stack branch reuses the same path-splitting helper, retiring a
hand-rolled split and a stale @sg-ignore (shell.rb strong problems:
10 -> 9).

(SKIP=Solargraph: pre-existing strong-level problems in shell.rb -
Vernier constants, missing @return on #rbs - which this reduces by one
and otherwise leaves untouched; verified by message-set diff.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H1FEjW6nMpZrWPmeWX9miT
apiology added a commit to apiology/solargraph that referenced this pull request Aug 17, 2026
@apiology
apiology marked this pull request as ready for review August 18, 2026 15:19
@apiology
apiology marked this pull request as draft August 24, 2026 20:38
Empty commit to trigger a fresh CI run after prior GitHub outage,
per review request.
apiology added a commit to apiology/solargraph that referenced this pull request Aug 26, 2026
Add specs for the gems command stderr messages on a nil gemspec,
MissingSpecError, and BadRequirementError; the scan command
StandardError rescue and exit; and logging.rb invalid
SOLARGRAPH_LOG warning, checked via a subprocess since that
check runs once at module load time.
@apiology
apiology marked this pull request as ready for review August 26, 2026 21:00
Shell#scan was tested only with an empty pins array (the success
path) and with a single pin whose typify raises (the rescue/exit 1
path, already covered here). The per-pin typify/probe calls on a
non-raising pin, and the --verbose branch that prints the pin
description, never ran in any spec.

Add specs stubbing a pin to typify/probe cleanly, in both verbose
and non-verbose modes, split out of the combined cover-shell-gaps
branch (the other half, covering Shell#gems, belongs on
pin-caching-3-pincache-core instead). Drop the third spec from that
source commit -- it duplicated the existing 'reports and exits when
typifying a pin raises' example above.
PR 1312 replaced silent warn calls with logger.warn/$stderr.puts
(since bin/solargraph sets $VERBOSE = nil, no-oping Kernel#warn).
Undercover flagged two of the touched spots as uncovered:
Pin::Base#choose's rescue branch and Shell#do_cache entirely.

Add a spec that forces #choose's rescue path by giving it two
values Location#<=> can't compare, asserting the log message and
re-raise. Add a spec calling Shell#do_cache directly with a nil
gemspec to cover its "not found" $stderr path, since it is only
ever invoked internally with real gemspecs from Gem::Specification.
apiology added a commit to apiology/solargraph that referenced this pull request Sep 3, 2026
These examples were marked pending on #53 by
#60. The feature that PR was waiting on already
landed via castwide#1231, castwide#1258, and castwide#1312, so the pending wrapper now hides
passing coverage instead of documenting a known gap. RSpec confirmed
each example passes cleanly with the wrapper removed.
Logging#logger builds a separate Logger whenever the including class
overrides log_level away from the default. Nothing exercised that
branch, so undercover reported the Logging module node at 71.43%. The
new example includes the module in an anonymous class returning :debug
from log_level, and asserts the logger it gets back is a distinct
object at Logger::DEBUG.

The node goes 71.43% -> 90.48%. Lines 16 and 19 stay uncovered and no
spec can close them: they are the two branches of one load-time `if` on
SOLARGRAPH_LOG, so a single evaluation reaches exactly one of them, and
re-loading the file replaces Ruby's per-file coverage counter array
instead of adding to it. Measured on a two-branch probe file: after a
second load with the other branch taken, the first branch's count went
from 1 back to 0.
undercover reported the logging.rb module node at 90.48%. The two
branches of the load-time SOLARGRAPH_LOG check run once per process, at
require time, so no in-process spec can reach both: a single evaluation
takes exactly one branch, and re-loading the file replaces Ruby's
per-file coverage counter array instead of adding to it.

Moving the parsing into Logging.resolve_level makes it an ordinary
callable, so specs reach the recognized-value, unrecognized-value and
unset paths directly. The subprocess spec that shelled out to a fresh
ruby to observe the warning is dropped, superseded by the direct
unrecognized-value example.

Reconstructed from branch cover-logging-1312 rather than cherry-picked:
that branch also carried its own version of the #logger custom-level
example, which this branch already has from the preceding commit.
apiology added a commit to apiology/solargraph that referenced this pull request Sep 5, 2026
Brings castwide#1312 up to 99fc26b. Everything on that
branch except its own five commits is already here, so what arrives is
lib/solargraph/logging.rb plus coverage in three spec files, aimed at
the four undercover nodes that PR owns: Logging, Pin::Base#choose, and
two blocks in Shell.

One conflict, spec/shell_spec.rb, resolved to keep both sides - this
branch's gem-caching examples and that branch's typify-raises example.

Dropped that branch's two do_cache examples. They cover Shell#do_cache,
which is not on this branch: it was merged in at d0714be in August and
has since gone, and the PR that rewrote that area owns providing its
tests. The remaining four nodes are unaffected.

2197 examples, 0 failures, 45 pending.
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