Skip to content

Commit 8b6aefb

Browse files
committed
Improve the plan structure
1 parent 72d0bbc commit 8b6aefb

1 file changed

Lines changed: 44 additions & 41 deletions

File tree

docs/plans/2026-01-21-dsl-definitions-design.md

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ struct DslDefinition {
4949
}
5050
```
5151

52-
The `DslDefinition` is pushed onto the nesting stack when visiting its block, so any definitions inside (methods, nested DSL calls, etc.) become members of the `DslDefinition` rather than the lexical parent.
52+
The `DslDefinition` is pushed onto the nesting stack (via new `Nesting::Dsl` variant) when visiting its block, so any definitions inside (methods, nested DSL calls, etc.) become members of the `DslDefinition` rather than the lexical parent.
53+
54+
A `ConstantReference` is also created for `receiver_name` so it gets resolved naturally in Phase 2.
5355

5456
#### ConstantToDslDefinition
5557

@@ -77,14 +79,14 @@ For non-constant cases (`var = Class.new` or just `Class.new`), only `DslDefinit
7779

7880
### Phase 2: Resolution
7981

80-
Standard resolution runs, which resolves `receiver_name` to a fully qualified name (e.g., `Class` in scope `Foo``::Class`).
82+
Standard resolution runs, which resolves `receiver_name` to a fully qualified name (e.g., `Class` in scope `Foo``::Class`). The `ConstantReference` created for `receiver_name` ensures it gets resolved through the normal reference resolution path.
8183

82-
### Phase 3: DSL Processing (New Phase)
84+
### Phase 3: DSL Processing
8385

84-
After resolution, process all `ConstantToDslDefinition`s:
86+
A new method `process_dsl_definitions()` is called after `resolve_all()`. It processes all `ConstantToDslDefinition`s:
8587

8688
1. Look up the referenced `DslDefinition`
87-
2. Get the resolved receiver name
89+
2. Get the resolved receiver via `graph.names().get(&receiver_name)`
8890
3. Dispatch to handler based on resolved receiver + method name:
8991

9092
```text
@@ -98,11 +100,19 @@ resolved receiver = ::Module, method = "new" → ModuleNewHandler
98100
- **Handler matches (e.g., `::Class.new`)**: Transform `ConstantToDslDefinition` into `DynamicClassDefinition` with the constant name. Creates `ClassDeclaration`.
99101
- **No handler matches (e.g., `SomeClass.new`)**: Transform `ConstantToDslDefinition` into regular `ConstantDefinition`. We don't know what `SomeClass.new` returns.
100102

101-
Each handler also decides what to do with owned members (reassign, discard, etc.).
103+
Each handler also decides what to do with owned members (reassign to the new definition, discard, etc.). When members are reassigned, their `lexical_nesting_id` is NOT updated - it preserves the lexical structure as written in source code.
102104

103105
### Phase 4: Incremental Resolution
104106

105-
Run resolution again for any new definitions created in Phase 3. This should be fast since the set is small.
107+
Call `resolve_all()` again for any new definitions created in Phase 3. The full resolution approach is acceptable since the set of new definitions is small.
108+
109+
**Full resolution flow:**
110+
111+
```rust
112+
resolver.resolve_all(); // Phase 2
113+
resolver.process_dsl_definitions(); // Phase 3
114+
resolver.resolve_all(); // Phase 4
115+
```
106116

107117
## DslDefinition Details
108118

@@ -129,7 +139,7 @@ struct DslArgumentList {
129139

130140
### Block Ownership
131141

132-
All DSL calls automatically own their block contents. This simplifies indexing logicthe handler decides what to do with owned definitions during processing:
142+
All DSL calls automatically own their block contents. This simplifies indexing logic - the handler decides what to do with owned definitions during processing:
133143

134144
- Reassign them to a new owner
135145
- Discard them
@@ -138,7 +148,7 @@ All DSL calls automatically own their block contents. This simplifies indexing l
138148

139149
### Lifecycle
140150

141-
`DslDefinition` is kept after processing. It shares a declaration with any generated definitions, enabling tooling features like "hover over `belongs_to` → see generated methods".
151+
`DslDefinition` is kept after processing. It shares a declaration with generated definitions, enabling tooling features like "hover over `belongs_to` → see generated methods".
142152

143153
## Example: Class.new
144154

@@ -156,6 +166,7 @@ end
156166
- `DslDefinition(receiver: Name(Class), method: "new", members: [MethodDefinition(baz)])`
157167
- `ConstantToDslDefinition(constant: Bar, dsl: DslDefinition)` - links constant to DSL
158168
- `MethodDefinition(baz)` with lexical nesting pointing to DslDefinition
169+
- `ConstantReference` for `Class` (for receiver resolution)
159170

160171
**Resolution:**
161172

@@ -211,7 +222,7 @@ end
211222

212223
**DSL Processing (via RSpec plugin):**
213224

214-
- Plugin handler decides how to represent theseperhaps as test group declarations with the helper method scoped appropriately.
225+
- Plugin handler decides how to represent these - perhaps as test group declarations with the helper method scoped appropriately.
215226

216227
## New Types
217228

@@ -245,7 +256,7 @@ struct DynamicClassDefinition {
245256
name_id: Option<NameId>, // None for anonymous classes
246257
uri_id: UriId,
247258
offset: Offset,
248-
members: Vec<DefinitionId>, // Consistent with ClassDefinition
259+
members: Vec<DefinitionId>,
249260
superclass_ref: Option<ReferenceId>,
250261
mixins: Vec<Mixin>,
251262
lexical_nesting_id: Option<DefinitionId>,
@@ -258,7 +269,7 @@ struct DynamicModuleDefinition {
258269
name_id: Option<NameId>, // None for anonymous modules
259270
uri_id: UriId,
260271
offset: Offset,
261-
members: Vec<DefinitionId>, // Consistent with ModuleDefinition
272+
members: Vec<DefinitionId>,
262273
mixins: Vec<Mixin>,
263274
lexical_nesting_id: Option<DefinitionId>,
264275
comments: Vec<Comment>,
@@ -291,8 +302,6 @@ struct DslHandlerRegistry {
291302

292303
### DefinitionId Generation
293304

294-
`DslDefinition` generates its ID using receiver_name and method_name:
295-
296305
```rust
297306
impl DslDefinition {
298307
pub fn id(&self) -> DefinitionId {
@@ -305,11 +314,24 @@ impl DslDefinition {
305314
))
306315
}
307316
}
317+
318+
impl DynamicClassDefinition {
319+
pub fn id(&self) -> DefinitionId {
320+
// Same location as source DslDefinition, with "Class.new" suffix
321+
DefinitionId::from(&format!("{}{}Class.new", *self.uri_id, self.offset.start()))
322+
}
323+
}
324+
325+
impl DynamicModuleDefinition {
326+
pub fn id(&self) -> DefinitionId {
327+
DefinitionId::from(&format!("{}{}Module.new", *self.uri_id, self.offset.start()))
328+
}
329+
}
308330
```
309331

310-
## Implementation Scope
332+
## Scope
311333

312-
### Phase 1 (This Issue)
334+
### In Scope (This Issue)
313335

314336
- Add `DslDefinition` struct
315337
- Add `ConstantToDslDefinition` struct
@@ -318,12 +340,15 @@ impl DslDefinition {
318340
- Add `index_dsl_definition_target` helper in indexer (similar to `index_constant_alias_target`)
319341
- Add `add_constant_to_dsl_definition` helper in indexer
320342
- Implement `Nesting::Dsl` variant for nesting stack
321-
- Add DSL processing phase with `DslHandler` trait
343+
- Add `process_dsl_definitions()` method for Phase 3
322344
- Implement `Class.new` handler
323345
- Implement `Module.new` handler
324346

325-
### Future Work
347+
### Out of Scope
326348

349+
- Conditional assignments (`Foo = cond ? Class.new : Module.new`)
350+
- Method chaining (`Class.new.freeze`)
351+
- Late assignment of anonymous classes (`klass = Class.new; Foo = klass`)
327352
- `Struct.new` handler (complex: inherits from Struct, generates accessor methods)
328353
- Plugin API for external DSL targets
329354
- RSpec/Minitest support via plugins
@@ -332,29 +357,7 @@ impl DslDefinition {
332357

333358
1. **Memory impact**: One-to-many relationship between definitions and declarations. Monitor memory usage during implementation.
334359

335-
## Design Decisions Made
336-
337-
1. **Constant assignment detection**: Use `ConstantToDslDefinition` to link constants to DSL calls. Similar pattern to `index_constant_alias_target` in existing code. Detected in `visit_constant_write_node` via new `index_dsl_definition_target` helper.
338-
339-
2. **Anonymous classes**: `DynamicClassDefinition` uses `name_id: Option<NameId>`. Named classes get `Some(name)` from `ConstantToDslDefinition`, anonymous classes get `None` and no declaration is created.
340-
341-
3. **Declaration membership**: For `Bar = Class.new`, `Declaration(Foo::Bar)` contains two definitions: `DslDefinition` and `DynamicClassDefinition`. This enables tooling to trace generated code back to the DSL call.
342-
343-
4. **Unknown receiver fallback**: If `ConstantToDslDefinition` references a DSL whose receiver doesn't match any handler (e.g., `Foo = SomeClass.new`), it becomes a regular `ConstantDefinition`.
344-
345-
5. **lexical_nesting_id preservation**: When members are reassigned from `DslDefinition` to `DynamicClassDefinition`, their `lexical_nesting_id` is NOT updated. It preserves the lexical structure as written in source code.
346-
347-
6. **Phase 4 resolution**: Simply call `resolve_all` again. The set of new definitions is small, so full resolution is acceptable.
348-
349-
7. **Edge cases not handled**: Conditional assignments (`Foo = cond ? Class.new : Module.new`), method chaining (`Class.new.freeze`), and late assignment of anonymous classes (`klass = Class.new; Foo = klass`) are out of scope.
350-
351-
8. **Phase 3 location**: New method `process_dsl_definitions()` called after `resolve_all()`. Caller does: `resolver.resolve_all(); resolver.process_dsl_definitions(); resolver.resolve_all();`
352-
353-
9. **Receiver resolution**: During indexing, create a `ConstantReference` for `DslDefinition.receiver_name`. It gets resolved naturally in Phase 2. Phase 3 looks it up via `graph.names().get(&receiver_name)`.
354-
355-
10. **DynamicClassDefinition ID**: Use same location as source `DslDefinition` with DSL name: `format!("{}{}Class.new", uri_id, offset)`. Similarly `Module.new` for `DynamicModuleDefinition`.
356-
357-
## Test Cases Needed
360+
## Test Cases
358361

359362
1. **Unknown receiver fallback**:
360363

0 commit comments

Comments
 (0)