LSP: auto-require completion for unrequired constants#446
Open
ahogappa wants to merge 7 commits intoruby:masterfrom
Open
LSP: auto-require completion for unrequired constants#446ahogappa wants to merge 7 commits intoruby:masterfrom
ahogappa wants to merge 7 commits intoruby:masterfrom
Conversation
These are generated by `rbs collection install` for individual workspaces; they are not part of typeprof's own development artifacts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Read the workspace's rbs_collection.yaml (or the path from typeprof.conf's `rbs_collection` key) and load its lockfile, passing the result through to Service so subsequent RBS loads can resolve gems from the collection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a Ruby file containing `require 'name'` is processed, parse the literal name and call `RBS::EnvironmentLoader#add(library: name)` on a Service-local loader, then feed the resolved RBS files through `update_rbs_file` so the genv learns about the library's types. Library names are deduped via @requested_rbs_libraries so repeat edits don't re-load. Unknown libraries roll back the loader's libs set so subsequent valid requires aren't blocked. The dynamic loader uses `core_root: nil` because the env's core RBS is already loaded at Service initialization. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a lightweight ConstantCatalog that scans stdlib + rbs_collection RBS files (without loading them into the type env) and maps each cpath to a require name. Resolution priority for the require name uses `Gem.find_files` validation: dirname as-is (e.g. `open-uri`) → `-` → `/` (e.g. `net-http` → `net/http`) → fall back to the slash form. `Service#each_const_completion` yields candidates from the catalog first (so the require_name annotation wins over identically-named env entries when rbs_collection eagerly preloads gems), then from the env. The LSP completion handler emits matching candidates with `additionalTextEdits` that insert `require '<name>'` right after any shebang/magic comments. The insert position walks past any existing require lines in that block and cancels the auto-insert when the same require is already present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Service used to construct `TypeProf::LSP::ConstantCatalog` itself, introducing a Core → LSP dependency. Build the catalog on the LSP Server side (per workspace) and pass it to Service through `options[:constant_catalog]`; Service treats it as a duck-typed object with `each_match`. `each_const_completion` becomes a no-op for the catalog branch when the option is omitted (e.g. CLI use). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace three ad hoc ivars (@dynamic_rbs_loader, @requested_rbs_libraries, @loaded_rbs_paths) with a single @rbs_loader (eagerly built, core_root: nil + collection). Library-name dedup now leverages @rbs_loader.libs (rbs gem's own Set), and path-level dedup uses the existing @rbs_text_nodes. The new load_library_for_require also computes new_libs = libs - prev_libs and filters each_signature to only files belonging to the just-added libraries — previously the path dedup alone allowed every collection lib's RBS files to be re-fed to update_rbs_file on the first dynamic require. Behavior change: failed `require 'foo'` literals are now retried on the next keystroke (loader.libs is rolled back). Previously @requested_rbs_libraries kept the failed name and never retried within a session. The retry path is harmless and lets typeprof recognize a gem installed mid-session. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stop eagerly walking the rbs_collection libraries into the type env at Service init: the env now contains only core RBS at startup, and any collection (or stdlib) gem is loaded into the env when the user's Ruby source contains `require 'name'`. This is the same lazy strategy that already applied to stdlib-only setups; collection just gets the same treatment now. `load_rbs_declarations` no longer takes the rbs_collection argument and no longer calls `add_collection`, so $raw_rbs_env caching is now used for every Service (with or without collection). The persistent `@rbs_loader` registers the collection's fullpath via `repository.add` so dynamic `loader.add(library: name)` can still find collection gems when the user requires them — without pre-adding them to `loader.libs`. With collection no longer eager, swap the yield order in `each_const_completion` to env → catalog. Catalog entries can have a wrong require_name when a stdlib gem reopens a core class (`abbrev` reopens `Array`, etc.); env-first dedup ensures `Array` etc. resolve to their core entry without an `additionalTextEdits` suggestion. Measured init time on a workspace with rbs_collection.lock.yaml containing 12 gems: 1st Service.new 923ms → 327ms; 2nd Service.new 807ms → 196ms (cache now applies regardless of collection). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an LSP completion that suggests unrequired constants (
CSV,JSON,Net::HTTP, gems from rbs_collection, etc.) and auto-inserts the correspondingrequireline viaadditionalTextEditswhen accepted.How to try
Type an unrequired constant in your editor (
CS,JSO,Net::H, …). Completion shows the candidate withfrom 'csv'detail; accepting insertsrequire 'csv'near the file head while putting the constant text at the cursor.How it works
Two RBS-walking passes, intentionally split for startup cost:
ConstantCatalog(~75ms at startup): parses stdlib + rbs_collection RBS just to extractcpath → require_name. No env load.require 'csv'is actually seen in a.rb,RBS::EnvironmentLoader#add(library: ...)runs and the resolved files go throughupdate_rbs_fileto enrich the type env.Loading everything into the env eagerly would cost ~700ms+ (dominated by
define_all/run_allper RBS file). Splitting keeps startup fast; the expensive work happens per-library, only when actually needed.Require-name resolution uses
Gem.find_filesvalidation:dirname as-is (
open-uri) →-→/(net-http→net/http) → fall back to slash form.Known limitations
bundle exec typeprof --lsprecommended.Gem.find_filesreads the LSP process's load path; plaintypeprof --lspmakes project gems fall to the slash heuristic. stdlib is unaffected.requireliteral is found in the current file. No workspace-wide require map is built or consulted.require 'csv'keeps csv loaded for the session.Rake::TaskLibetc. all suggestrequire 'rake', not file-levelrake/testtask. Fine when the top-level require auto-loads sub-files (json, uri, …); wrong when the gem requires explicit sub-file requires (e.g.rake/testtask).Rake::TestTask(not in upstream rake RBS) won't appear.Tests
bundle exec rake test: 435 tests pass (+9 over master: catalog 8 + integration 2)