Conversation
13dc76f to
cb0cbbe
Compare
- Refactor prepare_units() to drain pending_work instead of scanning all definitions/references. Dedup and skip stale items. - Remove clear_declarations() — no longer needed. - Move Object/Module/Class bootstrap to Graph::new(). - Rename resolve_all() to resolve(). - Update boot fast path check to documents.is_empty().
a99303e to
01bc2d8
Compare
| declarations.insert( | ||
| *OBJECT_ID, | ||
| Declaration::Namespace(Namespace::Class(Box::new(ClassDeclaration::new( | ||
| "Object".to_string(), | ||
| *OBJECT_ID, | ||
| )))), | ||
| ); | ||
| declarations.insert( | ||
| *MODULE_ID, | ||
| Declaration::Namespace(Namespace::Class(Box::new(ClassDeclaration::new( | ||
| "Module".to_string(), | ||
| *OBJECT_ID, | ||
| )))), | ||
| ); | ||
| declarations.insert( | ||
| *CLASS_ID, | ||
| Declaration::Namespace(Namespace::Class(Box::new(ClassDeclaration::new( | ||
| "Class".to_string(), | ||
| *OBJECT_ID, | ||
| )))), | ||
| ); |
There was a problem hiding this comment.
Actually, I think we may finally be at the stage where we can get rid of this since we now index RBS files.
However, I do want to avoid the mistake we did in the Ruby LSP of requiring all tests to always index RBS definitions first to have the base understanding.
Mostly, we are verifying algorithms and certain special cases, so we don't need exactly the RBS definitions to properly validate resolution.
Here's my suggestion.
- Let's stop adding these synthetic declarations ahead of time (we may still need their IDs as constants though since they are special)
- And then to get resolution tests to a state that actually verifies the full logic, let's just add a helper that always indexes the important core RBS definitions, but hard coded. The logic to find and index RBS files isn't what's being tested, it's the algorithms. This is all we need for all ancestor tests:
fn resolution_context() -> GraphTest {
let mut context = GraphTest::new();
context.index_rbs_uri("file:///fake_core.rbs", {
r"
class BasicObject; end
module Kernel; end
class Object < BasicObject
include Kernel
end
class Module; end
class Class < Module; end
"
});
context
}Feel free to do this in a separate PR though. You're going to have to update many tests.
| /// Verifies that incremental resolution produces identical results to a fresh | ||
| /// full resolution by building the same final state through two different paths. | ||
| #[test] | ||
| fn incremental_resolution_matches_fresh_resolution() { |
There was a problem hiding this comment.
I wonder if there's a way to transform this into a general check that we run on CI. Maybe it can be a part of the check to index and resolve the top 100 gems.
We can randomize file operations (ignoring certain files initially, then adding them or removing certain files) and compare.
01bc2d8 to
bbb7485
Compare
- Rename `invalidation_tests` module to `incremental_resolution_tests` - Add post-resolve assertions to all invalidation tests - Replace weak lexical scope test with one that proves resolution switches from inheritance to lexical scope after a new file is added - Add incremental-vs-full comparison test - Remove redundant `multiple_definitions` test
bbb7485 to
bfc6aef
Compare
Incremental Resolution
resolve()now processes only the work items accumulated byconsume_document_changes()calls, rather than clearing all declarations and rebuilding from scratch.How it works
When files are indexed (via
consume_document_changesordelete_document), the invalidation engine andextend()push targeted work items topending_work. Whenresolve()is called, it drains this queue and processes only those items. Unaffected declarations and resolved names are preserved.Key design changes
Built-in declarations (Object, Module, Class) are created in
Graph::new(). These are part of the Ruby object model and must always exist. Moving them out of resolution means the graph is always in a valid state, andresolve()doesn't need initialization logic.clear_declarations()is removed. With incremental resolution, declarations are individually removed or updated by the invalidation engine. The resolver only creates or modifies declarations for items inpending_work.prepare_units()drainspending_workinstead of scanning all definitions/references. It classifies items into the convergence loop (namespace definitions + constant references, sorted by depth) and remaining definitions (methods, attrs, variables). Stale items (removed between indexing and resolution) are skipped. Duplicates (same ID accumulated across multipleconsume_document_changescalls) are deduplicated.resolve_all()renamed toresolve(). The method no longer resolves "all" — it resolves only pending work.Test cleanup
mod invalidation_tests→mod incremental_resolution_tests