Skip to content

Add specs for reverted fixes and uncovered branches - #1262

Open
apiology wants to merge 8 commits into
castwide:masterfrom
apiology:worktree-backfill-regression-tests
Open

Add specs for reverted fixes and uncovered branches#1262
apiology wants to merge 8 commits into
castwide:masterfrom
apiology:worktree-backfill-regression-tests

Conversation

@apiology

@apiology apiology commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Problem: Several fixes in this repo have been reverted or silently re-broken with the full suite green, because no spec asserted the fixed behavior — most recently an attempt to infer through Class<generic<T>>#new, which degraded arity checking for any Class<X> with a generic anywhere inside X:

# k : Class<Array<generic<T>>>
k.new(1, 2, 3, 4)
# reported: Too many arguments to Class#initialize
# expected: Too many arguments to Array.new

That was caught by hand and reverted, and CI would not have caught it — nor the constant-alias resolution, the cache_next_gemspec re-entrancy guard, the class-wide combined_pins_in_memory, the Chain::Call.new signature external callers depend on, or the gemspec packaging a top-level sig/.

Solution: Adds eight examples asserting each of those behaviors directly, plus seven covering units and branches no spec reached (YardMap::Cache#get_path_pins, DomainDirective.closure_at and #process_directive, and Chain::Call's non-macro directive fallthrough).

Spec-only; no lib/ change.

Test plan: Full suite 1661 examples, 0 failures, 67 pending. Both generic-Class arity specs verified to fail when the reverted change is replayed on this base.

apiology and others added 2 commits August 5, 2026 09:09
Adds coverage for cases that were fixed via revert or [regression] PRs
but never got a direct test:

- ApiMap#qualify resolving a bare constant alias (castwide#1029/castwide#1041/castwide#1048)
- Library#cache_next_gemspec re-entrancy guard (castwide#983)
- DocMap#combined_pins_in_memory being shared class-wide, not per
  instance (castwide#983)
- Chain::Call.new accepting no location arg, for callers like
  solargraph-rails (castwide#940)
- gemspec packaging not including a top-level sig/ dir that RBS
  auto-discovers in installed gems (castwide#1146 / castwide#1144)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RGdFFjeEiPNiN6VfWRfCUa
Satisfies RSpec/DescribeClass without an exclude-list entry.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RGdFFjeEiPNiN6VfWRfCUa
@apiology
apiology marked this pull request as ready for review August 5, 2026 13:43
apiology added a commit to apiology/solargraph that referenced this pull request Aug 5, 2026
apiology added a commit to apiology/solargraph that referenced this pull request Aug 5, 2026
castwide#1262 backfilled a regression test for
DocMap#combined_pins_in_memory being shared across instances rather than
memoized per instance - but castwide#1252 (already merged into
this branch) moved that exact mechanism from DocMap into PinCache as part
of its instance-based rewrite: DocMap#pin_cache now calls
workspace.fresh_pincache directly (not the memoized workspace.pin_cache),
so the class-level in-memory cache is on PinCache, keyed by
[gem name, gem version, RBS cache key] and additionally scoped per
yard_plugins.

Replaced the DocMap-targeting test (which failed with
NoMethodError since that method no longer exists there) with two
PinCache-targeting tests: one for the same guarantee (shared across
instances with matching yard_plugins), one new one covering the added
yard_plugins scoping dimension that did not exist in the original
DocMap-based cache.
A macro-generated `@!method` whose `@return` is a duck type resolves
correctly: the generated pin carries `return_type.tag == "#quack"`, and the
type resolves through ApiMap#get_complex_type_methods to a Pin::DuckMethod.
This is easy to believe otherwise, because `solargraph pin` renders the pin
via UniqueType#to_rbs and RBS has no duck-type syntax, so any duck type
prints as `untyped` — identically for a plain method, a `@!method` directive,
and a macro-generated one.

Pin the working behavior so the macro path stays covered, alongside the
existing class-name case.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MEDQFCJ2M7gaYkUQVpiQzn
apiology added a commit to apiology/solargraph that referenced this pull request Aug 16, 2026
An earlier attempt at inferring through a Class<generic<T>> receiver
changed ComplexType::TypeMethods#namespace/#namespace_type/#scope so
that any Class<X> whose X *contains* a generic degraded to a bare
Class. ComplexType#generic? is recursive, so this caught far more than
Class<generic<T>>: with it applied, `k.new` on a Class<Box<generic<T>>>
receiver stopped reporting "Not enough arguments to Box.new" at all,
and `k.new(1, 2, 3, 4)` on a Class<Array<generic<T>>> receiver reported
"Too many arguments to Class#initialize" - arity checked against
Class's own initializer rather than Array's.

The full suite was green under that change, so nothing caught it. These
two specs pin both messages.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CXmnT5gSB1PheL9UbiGEVA
apiology added a commit to apiology/solargraph that referenced this pull request Aug 18, 2026
Brings in eef28f3, which pins Class#initialize arity checking for
generic Class receivers. Spec-only.

Conflict in spec/type_checker/levels/strong_spec.rb was both sides
appending examples at the end of the same describe block; resolved by
keeping both, restoring the `end` that closes this branch's last example.
apiology added a commit to apiology/solargraph that referenced this pull request Aug 26, 2026
…eric Class receivers

# Conflicts:
#	spec/type_checker/levels/strong_spec.rb
@apiology
apiology marked this pull request as draft August 31, 2026 17:35
Move the why into the example name and keep a bare issue link,
per repo comment-style conventions.
undercover flagged lib/solargraph/yard_map/cache.rb:5-24 and
lib/solargraph/yard_map/directives/domain_directive.rb:6-28 as
uncovered against castwide/solargraph:master. Both methods had zero
test coverage: Cache is never referenced outside its own file, and
DomainDirective.closure_at has no spec at all.

Adds direct unit specs for both, plus DomainDirective.process_directive
end to end.

No upstream castwide/solargraph PR introduces this code -- the
@sg-ignore comments that triggered the diff hunks landed directly on
integration branch 2026-08-04 via commit 4529157 (Typecheck cleanup
batch 18).
inferred_pins's elsif !p.directives.empty? branch (process_directive
call) was never exercised: undercover flagged call.rb:404:467 at
91.67% coverage, and per-line SimpleCov data confirmed the two
0-hit lines were the process_directive call and its return check.

Pin#macros is a strict subset of Pin#directives - only tag_name
"macro" entries - so a pin with a non-macro directive (e.g.
@!visibility) has empty macros but non-empty directives, reaching
the elsif. Added a spec where such a pin's directive names no
registered macro, so process_directive resolves nothing and the
pin falls through to ordinary body-probing.
@apiology apiology changed the title Backfill regression tests for previously-reverted/regressed behavior Add specs for reverted fixes and uncovered branches Sep 5, 2026
@apiology
apiology marked this pull request as ready for review September 5, 2026 13:11
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