diff --git a/.gitignore b/.gitignore index a1f5a1c..20ab4e1 100644 --- a/.gitignore +++ b/.gitignore @@ -10,4 +10,4 @@ Cargo.lock .tabnine/ vscode/ objectscript-lsp/objectscript-tests/local/ -objectscript-lsp/documentation/ \ No newline at end of file +objectscript-lsp/documentation/code-changes-AI diff --git a/README.md b/README.md index b756575..d40ecb9 100644 --- a/README.md +++ b/README.md @@ -6,19 +6,12 @@ An [ObjectScript](https://docs.intersystems.com/latest/csp/docbook/DocBook.UI.Pa # Introduction -This Zed extension uses the [tree-sitter-objectscript](https://github.com/intersystems/tree-sitter-objectscript) grammar to provide syntax highlighting and code injections for `.cls` files containing ObjectScript. Since ObjectScripts supports a number of embedded languages, you should install grammars for the following languages otherwise you may see areas that appear to lack syntax coloring. +This Zed extension uses the [tree-sitter-objectscript](https://github.com/intersystems/tree-sitter-objectscript) grammars and the `objectscript-lsp` crate (local to this repo) to provide syntax highlighting, code injections, and language support for `.cls`, `.mac`, `.rtn`, `.inc`, `.int` and `.xml` files containing ObjectScript. Install the following extensions below to get syntax highlighting for any code injected into objectscript that is `sql` or `html`. -- SQL -- HTML -- Python -- JavaScript -- CSS -- XML -- Markdown +- [SQL](https://zed.dev/extensions/sql) +- [HTML](https://zed.dev/extensions/html) -**NOTE**: The ObjectScript `.cls` syntax supports some sophisticated constructs and as such it can take 15-60 seconds for Zed's WASM machinery to build the parser before syntax coloring becomes available. - -Currently this extension only provides syntax coloring support. +The current features supported in the `ObjectScript language server` are `goto_definition`, `goto_implementation`, `refactor`, and `diagnostics`. These features are described in detail in the `objectscript-lsp/documentation/features` folder. ## Reporting Issues @@ -46,4 +39,3 @@ RUST_LOG = "language,extension=trace" Cloning and the building a debug Zed with these `RUST_LOG` settings gives fairly detailed log output including diagnosing bad `.scm` rules. - diff --git a/documentation/dev-extension-setup.md b/documentation/dev-extension-setup.md new file mode 100644 index 0000000..fa9ea8f --- /dev/null +++ b/documentation/dev-extension-setup.md @@ -0,0 +1,12 @@ +# Using the Zed ObjectScript Extension (Dev Extension) +The Dev Extension allows you to reference the most recent updates from this repo before they get officially merged into the `zed-extensions` repo. + +The official extension is here: [objectscript extension](https://zed.dev/extensions/objectscript) + +**STEPS** +`PREREQS`: Make sure you have the Zed editor downloaded and the `zed-objectscript` repo locally. + +1. Open Zed Editor +2. Go to command Palette (Command Shift P) -> `zed: Extensions` +3. Click Install Dev Extension +4. Point it to the `zed-objectscript` folder on your local device. diff --git a/languages/xml/injections.scm b/languages/xml/injections.scm index cb2249d..67dc797 100644 --- a/languages/xml/injections.scm +++ b/languages/xml/injections.scm @@ -1,18 +1,46 @@ -( - element - (STag (Name) @_start_tag) - (content (CDSect (CData) @injection.content)) - (ETag (Name) @_end_tag) - (#eq? @_start_tag "Implementation") - (#eq? @_end_tag "Implementation") - (#set! injection.language "objectscript") -) -( - element - (STag (Name) @_start_tag) - (content (CharData) @injection.content) - (ETag (Name) @_end_tag) - (#eq? @_start_tag "Implementation") - (#eq? @_end_tag "Implementation") - (#set! injection.language "objectscript") -) +((Comment) @injection.content + (#set! injection.language "comment")) + +((element + (STag + (Name) @_name) + (content) @injection.content) + (#eq? @_name "style") + (#set! injection.combined) + (#set! injection.include-children) + (#set! injection.language "css")) + +((element + (STag + (Name) @_name) + (content) @injection.content) + (#eq? @_name "script") + (#set! injection.combined) + (#set! injection.include-children) + (#set! injection.language "javascript")) + +((element + (STag + (Name) @_name) + (content) @injection.content) + (#eq? @_name "pma:table") + (#set! injection.combined) + (#set! injection.include-children) + (#set! injection.language "sql")) + +(element + (STag + (Name) @_name) + (content + (CDSect + (CData) @injection.content)) + (#eq? @_name "Implementation") + (#set! injection.language "objectscript")) + +(element + (STag + (Name) @_name) + (content + (CharData) @injection.content) + (#eq? @_name "Implementation") + (#set! injection.language "objectscript")) diff --git a/objectscript-lsp/docs/agent-change-documentation.md b/objectscript-lsp/docs/agent-change-documentation.md deleted file mode 100644 index 866c80a..0000000 --- a/objectscript-lsp/docs/agent-change-documentation.md +++ /dev/null @@ -1,683 +0,0 @@ -# Agent Change Documentation: Comprehensive LSP Test Suite - -## 1. Problem - -### Original issue - -The ObjectScript LSP server had minimal test coverage. The existing `test-suite.md` only documented goto-definition cases for classes with incomplete structure, and the Rust test file (`src/test.rs`) had only 14 test functions — mostly covering variables, inheritance keywords, refactoring, and a single goto-implementation test. Major LSP features were untested or under-tested: - -- No diagnostic tests (syntax error detection, XML injection) -- No class method call goto-definition tests -- No oref resolution tests across different AST contexts (do_parameter, job_argument) -- No multiple inheritance direction tests -- No deep hierarchy override tests -- No ordering/timing tests (child opened before parent) -- No edge case tests (undefined symbols, missing classes) -- No didOpen/didChange document sync tests -- No refactoring edge case tests - -### Evidence - -- User request: "I need to fully test all the current language server features (gotodef, goto_implementation, document diagnostics) as well as make sure they work consistently even with different ordering of indexing or opening of files" -- Existing `objectscript-tests/test-suite.md` was 124 lines, incomplete, and class-focused only -- Existing `src/test.rs` had 14 test functions total - -### Scope - -In scope: - -- All LSP features currently implemented: goto_definition, goto_implementation, diagnostics, code actions/refactoring, document sync -- Test fixture creation for all scenarios -- Comprehensive test-suite.md documentation with numbered test cases -- Rust integration tests exercising the `ProjectData` and `ProjectState` APIs directly -- Ordering/timing scenarios (open order independence) -- Edge cases (undefined symbols, missing classes, empty results) - -Out of scope: - -- Fixing the compilation error in `lsp.rs` (match syntax error at line 794) — user stated they already fixed it -- Adding new LSP features -- Performance testing -- End-to-end LSP protocol testing via JSON-RPC - ---- - -## 2. Solution - -### Summary - -Created 26 new test fixture files across 7 new directories, rewrote `objectscript-tests/test-suite.md` as a structured 264-line test plan with 7 sections and numbered cases, and added 30 new Rust test functions to `src/test.rs` covering goto-definition, goto-implementation, diagnostics, document sync, ordering, and refactoring. - -### Technical approach - -1. **Test fixtures**: Created minimal ObjectScript `.cls`, `.mac`, and `.xml` files that exercise specific LSP behaviors in isolation. Each fixture directory maps to a section in the test suite. - -2. **Test-suite.md**: Restructured as a formal test plan with numbered cases in tables, organized by feature (Goto Definition, Goto Implementation, Diagnostics, Code Actions, Document Sync, Ordering, Capabilities). - -3. **Rust tests**: Added tests that follow the existing pattern — use `setup_backend_and_workspace()` or `ProjectState::new()` to create state, call `handle_document_opened()` for ordering tests, and assert on `get_method_definition()`, `get_method_overrides()`, `get_class_implementations()`, `get_variable_definition()`, `get_class_definition()`, `get_oref_method_definition()`, and `collect_error_nodes()`. - -### Design decisions - -- **Test at the ProjectData/ProjectState API level, not the LSP handler level.** - - Reason: The existing tests use this pattern. It avoids needing a mock LSP Client while still exercising the core logic. - - Alternatives considered: Full LSP protocol tests via `LspService::build()` (only used in `src/lsp.rs` tests for the diagnostic handler). - - Trade-off: Does not test the node-type dispatch logic in `goto_definition()` handler directly, but exercises the underlying resolution logic. - -- **One fixture directory per test category.** - - Reason: Keeps workspace indexing fast and isolated. Each test indexes only the files it needs. - - Trade-off: Some fixture duplication (e.g., multiple "target" classes) but avoids cross-test interference. - -- **Used `ProjectState::new()` + `handle_document_opened()` for ordering tests instead of `setup_backend_and_workspace()`.** - - Reason: Ordering tests need to control the exact sequence of file loading. `setup_backend_and_workspace()` indexes everything at once. - -### Important behavior changes - -- No runtime behavior changes. All changes are test infrastructure. -- Removed debug `eprintln!` statements from one existing test (`test_routine_goto_definition_variable`). - ---- - -## 3. Reconstruction Guide - -### Base repository state - -- Branch: `v1.5.0` -- Base commit: `dac4873bc0e639f12ee353fa42b60df09bb571d5` -- Comparison target: `HEAD` (unstaged working tree changes) - -### Prerequisites - -- Rust toolchain (edition 2024 support required) -- All crate dependencies already in `Cargo.lock` - -### Step-by-step recreation - -1. Create 7 new directories under `objectscript-tests/`: - ``` - diagnostics/ - gotodef/class-method-call/ - gotodef/multiple-inheritance/ - gotodef/oref-contexts/ - gotodef/routines/ - implementation/ - ordering/ - ``` - -2. Create 26 fixture files (detailed below in Section 4). - -3. Replace `objectscript-tests/test-suite.md` with the comprehensive 264-line test plan. - -4. Modify `src/test.rs`: - - Remove debug `eprintln!` statements from `test_routine_goto_definition_variable` - - Add 30 new test functions after the existing test, organized by feature section - -5. Verify compilation: `cargo test --no-run` - -### Exact patch - -The full diff for `src/test.rs` is 899 lines. Due to size, it was saved during evidence collection at: -`/Users/hkimura/.claude/projects/-Users-hkimura-zed-objectscript-objectscript-lsp-crates-objectscript-core/2d445cc9-da63-4802-9450-fed4010773f7/tool-results/bk4bjaexz.txt` - -Regenerate with: -```bash -cd objectscript-lsp && git diff HEAD -- src/test.rs -``` - ---- - -## 4. ALL Changes - -### Change manifest - -| # | File | Status | Category | Why changed | How to recreate | -|---|------|--------|----------|-------------|-----------------| -| 1 | `objectscript-tests/test-suite.md` | Untracked (new) | Docs | Complete rewrite of test plan | Write 264-line structured test plan | -| 2 | `src/test.rs` | Modified | Test | Add 30 new integration tests | Append test functions after existing tests | -| 3 | `objectscript-tests/diagnostics/clean.cls` | Untracked (new) | Test | Fixture: clean class with no parse errors | Write minimal valid class | -| 4 | `objectscript-tests/diagnostics/syntax-error.cls` | Untracked (new) | Test | Fixture: class with intentional syntax error (`set =`) | Write class with malformed set command | -| 5 | `objectscript-tests/diagnostics/clean.mac` | Untracked (new) | Test | Fixture: clean routine with no parse errors | Write minimal valid routine | -| 6 | `objectscript-tests/diagnostics/multiple-errors.mac` | Untracked (new) | Test | Fixture: routine with 2+ syntax errors | Write routine with `set =` and unclosed string | -| 7 | `objectscript-tests/diagnostics/injected-clean.xml` | Untracked (new) | Test | Fixture: XML with valid ObjectScript in CDATA | Write XML export with valid `set x = 1` | -| 8 | `objectscript-tests/diagnostics/injected-error.xml` | Untracked (new) | Test | Fixture: XML with ObjectScript errors in CDATA | Write XML export with `set =` error | -| 9 | `objectscript-tests/gotodef/class-method-call/caller.cls` | Untracked (new) | Test | Fixture: class that calls `##class(Demo.Utility).Helper()` | Write class with class method call | -| 10 | `objectscript-tests/gotodef/class-method-call/utility.cls` | Untracked (new) | Test | Fixture: target class with Helper/Compute methods | Write utility class with two classmethods | -| 11 | `objectscript-tests/gotodef/multiple-inheritance/base.cls` | Untracked (new) | Test | Fixture: base class with Shared/Common methods | Write root class | -| 12 | `objectscript-tests/gotodef/multiple-inheritance/left-parent.cls` | Untracked (new) | Test | Fixture: left parent overriding base | Write class extending Demo.Base | -| 13 | `objectscript-tests/gotodef/multiple-inheritance/right-parent.cls` | Untracked (new) | Test | Fixture: right parent overriding base | Write class extending Demo.Base | -| 14 | `objectscript-tests/gotodef/multiple-inheritance/child-default.cls` | Untracked (new) | Test | Fixture: child with default (left) inheritance | Write class extending both parents | -| 15 | `objectscript-tests/gotodef/multiple-inheritance/child-right.cls` | Untracked (new) | Test | Fixture: child with `[Inheritance = right]` | Write class with right inheritance keyword | -| 16 | `objectscript-tests/gotodef/oref-contexts/oref-do-parameter.cls` | Untracked (new) | Test | Fixture: oref method call in `do obj.Run()` | Write class with do_parameter oref | -| 17 | `objectscript-tests/gotodef/oref-contexts/oref-job-argument.cls` | Untracked (new) | Test | Fixture: oref method call in `job worker.Execute()` | Write class with job_argument oref | -| 18 | `objectscript-tests/gotodef/oref-contexts/target.cls` | Untracked (new) | Test | Fixture: target class for oref resolution | Write Demo.Target with %New/Run/Execute | -| 19 | `objectscript-tests/gotodef/routines/tag-calls.mac` | Untracked (new) | Test | Fixture: routine with tag calls (do/goto) | Write routine with helper/finish labels | -| 20 | `objectscript-tests/gotodef/routines/cross-routine-ref.mac` | Untracked (new) | Test | Fixture: cross-routine reference `do helper^tagcalls` | Write short routine referencing tagcalls | -| 21 | `objectscript-tests/gotodef/routines/offset-goto.mac` | Untracked (new) | Test | Fixture: numeric offset `do main+2` | Write routine with offset reference | -| 22 | `objectscript-tests/implementation/no-overrides.cls` | Untracked (new) | Test | Fixture: class with method that has no overrides | Write standalone class | -| 23 | `objectscript-tests/implementation/deep-super.cls` | Untracked (new) | Test | Fixture: top of 3-level hierarchy | Write base class with DeepMethod | -| 24 | `objectscript-tests/implementation/deep-mid.cls` | Untracked (new) | Test | Fixture: middle of hierarchy | Write class extending Demo.DeepSuper | -| 25 | `objectscript-tests/implementation/deep-leaf-one.cls` | Untracked (new) | Test | Fixture: leaf override #1 | Write class extending Demo.DeepMid | -| 26 | `objectscript-tests/implementation/deep-leaf-two.cls` | Untracked (new) | Test | Fixture: leaf override #2 | Write class extending Demo.DeepMid | -| 27 | `objectscript-tests/ordering/parent.cls` | Untracked (new) | Test | Fixture: parent class for ordering tests | Write Demo.OrderParent with Greet/Work | -| 28 | `objectscript-tests/ordering/child.cls` | Untracked (new) | Test | Fixture: child class for ordering tests | Write Demo.OrderChild extending parent | - -### Detailed file changes - -#### `src/test.rs` - -- Status: Modified, unstaged -- Category: Test -- Change type: Feature (new tests) -- Problem addressed: Minimal test coverage for LSP features -- Solution implemented: Added 30 new test functions covering 6 feature areas -- Exact changes: - - Removed 4 `eprintln!` debug lines and 1 unused variable from `test_routine_goto_definition_variable` - - Added test section: **GOTO DEFINITION — CLASS METHOD CALLS** (3 tests) - - `test_goto_def_class_method_call_resolves_method` - - `test_goto_def_class_method_call_nonexistent_returns_empty` - - `test_goto_def_class_reference_resolves_to_class` - - Added test section: **GOTO DEFINITION — OREF CONTEXTS** (2 tests) - - `test_goto_def_oref_resolves_method_in_target_class` - - `test_goto_def_oref_job_argument_resolves_method` - - Added test section: **GOTO DEFINITION — MULTIPLE INHERITANCE** (2 tests) - - `test_goto_def_multiple_inheritance_default_left` - - `test_goto_def_multiple_inheritance_right_direction` - - Added test section: **GOTO IMPLEMENTATION — DEEP HIERARCHY** (5 tests) - - `test_goto_implementation_deep_hierarchy_from_super` - - `test_goto_implementation_deep_hierarchy_from_mid` - - `test_goto_implementation_no_overrides_returns_empty` - - `test_goto_implementation_class_subclasses` - - `test_goto_implementation_class_with_no_subclasses` - - Added test section: **DIAGNOSTICS** (6 tests) - - `test_diagnostics_clean_cls_has_no_errors` - - `test_diagnostics_syntax_error_cls_has_errors` - - `test_diagnostics_clean_routine_has_no_errors` - - `test_diagnostics_multiple_errors_routine` - - `test_diagnostics_xml_injected_clean_has_no_errors` - - `test_diagnostics_xml_injected_error_has_errors` - - Added test section: **ORDERING / TIMING** (4 tests) - - `test_ordering_child_opened_before_parent` - - `test_ordering_parent_opened_before_child` - - `test_ordering_missing_class_reference_returns_empty` - - `test_ordering_duplicate_open_is_idempotent` - - Added test section: **GOTO DEFINITION — EDGE CASES** (2 tests) - - `test_goto_def_undefined_symbol_returns_empty` - - `test_goto_def_nonexistent_class_returns_empty` - - Added test section: **DOCUMENT SYNC — didOpen** (3 tests) - - `test_did_open_cls_populates_class_id_and_name` - - `test_did_open_routine_populates_class_name_as_routine` - - `test_did_open_xml_no_class_id` - - Added test section: **DOCUMENT SYNC — update_document** (1 test) - - `test_update_document_rebuilds_semantics` - - Added test section: **REFACTORING** (2 tests) - - `test_refactor_no_changes_returns_none` - - `test_refactor_workspace_excludes_xml` -- Side effects: None (test-only changes) -- Verification: Not run (compilation check was interrupted by user) -- Notes: XML diagnostic tests use `ProjectState` to parse XML internally (avoiding direct `tree_sitter_xml` dependency which is only in `objectscript-core`) - -#### `objectscript-tests/test-suite.md` - -- Status: Untracked (new file, complete rewrite replacing the old tracked version) -- Category: Docs -- Change type: Documentation -- Problem addressed: Old test-suite.md was 124 lines, incomplete, unstructured -- Solution implemented: 264-line structured test plan with 7 sections, numbered test cases in tables, fixture directory listing -- Exact changes: - - Section 1: Goto Definition (8 subsections, ~40 test cases) - - Section 2: Goto Implementation (3 subsections, ~10 test cases) - - Section 3: Document Diagnostics (3 subsections, ~12 test cases) - - Section 4: Code Actions/Refactoring (3 subsections, ~12 test cases) - - Section 5: Document Sync (3 subsections, ~12 test cases) - - Section 6: Ordering & Timing (2 subsections, ~7 test cases) - - Section 7: Capability Negotiation (3 test cases) - - Fixture directory tree reference -- Side effects: None -- Verification: Visual review - -#### `objectscript-tests/diagnostics/clean.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Need a valid .cls file that produces zero parse errors -- Solution implemented: Minimal `Class Demo.Clean` with one valid `ClassMethod` -- Exact changes: 10-line file with `Set x = 1`, `Write x`, `Quit 1` -- Verification: Used in `test_diagnostics_clean_cls_has_no_errors` - -#### `objectscript-tests/diagnostics/syntax-error.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Need a .cls file that intentionally produces a parse error -- Solution implemented: `Set x =` (missing RHS) inside a ClassMethod -- Exact changes: 10-line file with intentionally broken `Set` command -- Verification: Used in `test_diagnostics_syntax_error_cls_has_errors` - -#### `objectscript-tests/diagnostics/clean.mac` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Need a valid routine file for zero-error diagnostic test -- Solution implemented: `ROUTINE clean` with `set x = 1` / `w x` / `quit` -- Verification: Used in `test_diagnostics_clean_routine_has_no_errors` - -#### `objectscript-tests/diagnostics/multiple-errors.mac` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Need a routine with 2+ distinct parse errors -- Solution implemented: `set =` (missing LHS) and unclosed string literal `"hello` -- Verification: Used in `test_diagnostics_multiple_errors_routine` - -#### `objectscript-tests/diagnostics/injected-clean.xml` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Need XML with valid ObjectScript in CDATA to verify zero injected errors -- Solution implemented: XML Export with `` -- Verification: Used in `test_diagnostics_xml_injected_clean_has_no_errors` - -#### `objectscript-tests/diagnostics/injected-error.xml` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Need XML with ObjectScript errors in CDATA to verify error detection -- Solution implemented: XML Export with `set =` and unclosed string in CDATA block -- Verification: Used in `test_diagnostics_xml_injected_error_has_errors` - -#### `objectscript-tests/gotodef/class-method-call/caller.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Need a class that calls `##class(Demo.Utility).Helper()` -- Solution implemented: `Class Demo.Caller` with ClassMethod that calls Demo.Utility methods -- Verification: Used in `test_goto_def_class_method_call_resolves_method` - -#### `objectscript-tests/gotodef/class-method-call/utility.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Change type: New fixture -- Problem addressed: Target class for class method call resolution -- Solution implemented: `Class Demo.Utility` with `Helper()` and `Compute()` classmethods -- Verification: Used in class method call tests - -#### `objectscript-tests/gotodef/multiple-inheritance/base.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Root of multi-inheritance test hierarchy -- Solution implemented: `Class Demo.Base` with `Shared()` and `Common()` methods -- Verification: Used in multiple inheritance tests - -#### `objectscript-tests/gotodef/multiple-inheritance/left-parent.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Left parent in multi-inheritance chain -- Solution implemented: `Class Demo.LeftParent Extends Demo.Base` overriding both methods -- Verification: Used in multiple inheritance tests - -#### `objectscript-tests/gotodef/multiple-inheritance/right-parent.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Right parent in multi-inheritance chain -- Solution implemented: `Class Demo.RightParent Extends Demo.Base` overriding both methods -- Verification: Used in multiple inheritance tests - -#### `objectscript-tests/gotodef/multiple-inheritance/child-default.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Child using default (left) inheritance direction -- Solution implemented: `Class Demo.ChildDefault Extends (Demo.LeftParent, Demo.RightParent)` -- Verification: Used in `test_goto_def_multiple_inheritance_default_left` - -#### `objectscript-tests/gotodef/multiple-inheritance/child-right.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Child using explicit right inheritance direction -- Solution implemented: `Class Demo.ChildRight Extends (...) [Inheritance = right]` -- Verification: Used in `test_goto_def_multiple_inheritance_right_direction` - -#### `objectscript-tests/gotodef/oref-contexts/oref-do-parameter.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Oref method call in `do obj.Run()` context -- Solution implemented: `Class Demo.OrefDoParam` with `[ProcedureBlock = 0]` method doing `do obj.Run()` -- Verification: Used in `test_goto_def_oref_resolves_method_in_target_class` - -#### `objectscript-tests/gotodef/oref-contexts/oref-job-argument.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Oref method call in `job worker.Execute()` context -- Solution implemented: `Class Demo.OrefJobArg` with `job worker.Execute()` statement -- Verification: Used in `test_goto_def_oref_job_argument_resolves_method` - -#### `objectscript-tests/gotodef/oref-contexts/target.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Target class that oref variables resolve to -- Solution implemented: `Class Demo.Target` with `%New()`, `Run()`, `Execute()` methods -- Verification: Used in both oref context tests - -#### `objectscript-tests/gotodef/routines/tag-calls.mac` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Routine with labels for tag call / goto resolution -- Solution implemented: `ROUTINE tagcalls` with `main`, `helper`, `finish` labels and `do`/`goto` calls -- Verification: Available for routine goto-definition tests - -#### `objectscript-tests/gotodef/routines/cross-routine-ref.mac` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Cross-routine reference (`do helper^tagcalls`) -- Solution implemented: `ROUTINE crossref` with `do helper^tagcalls` -- Verification: Available for cross-routine resolution tests - -#### `objectscript-tests/gotodef/routines/offset-goto.mac` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Numeric offset goto (`do main+2`) -- Solution implemented: `ROUTINE offsetgoto` with `do main+2` targeting a specific line -- Verification: Available for numeric offset tests - -#### `objectscript-tests/implementation/no-overrides.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Class with no subclasses for negative implementation test -- Solution implemented: `Class Demo.NoOverrides` with single `Unique()` method -- Verification: Used in `test_goto_implementation_no_overrides_returns_empty` and `test_goto_implementation_class_with_no_subclasses` - -#### `objectscript-tests/implementation/deep-super.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Top of 3-level override hierarchy -- Solution implemented: `Class Demo.DeepSuper` with `DeepMethod()` -- Verification: Used in `test_goto_implementation_deep_hierarchy_from_super` - -#### `objectscript-tests/implementation/deep-mid.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Middle of 3-level hierarchy -- Solution implemented: `Class Demo.DeepMid Extends Demo.DeepSuper` overriding `DeepMethod()` -- Verification: Used in deep hierarchy tests - -#### `objectscript-tests/implementation/deep-leaf-one.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Leaf #1 of hierarchy -- Solution implemented: `Class Demo.DeepLeafOne Extends Demo.DeepMid` overriding `DeepMethod()` -- Verification: Used in `test_goto_implementation_deep_hierarchy_from_mid` - -#### `objectscript-tests/implementation/deep-leaf-two.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Leaf #2 of hierarchy -- Solution implemented: `Class Demo.DeepLeafTwo Extends Demo.DeepMid` overriding `DeepMethod()` -- Verification: Used in `test_goto_implementation_deep_hierarchy_from_mid` - -#### `objectscript-tests/ordering/parent.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Parent class for index-order tests -- Solution implemented: `Class Demo.OrderParent` with `Greet()` and `Work()` methods -- Verification: Used in ordering tests - -#### `objectscript-tests/ordering/child.cls` - -- Status: Untracked (new) -- Category: Test fixture -- Problem addressed: Child class for index-order tests -- Solution implemented: `Class Demo.OrderChild Extends Demo.OrderParent` overriding `Greet()` -- Verification: Used in `test_ordering_child_opened_before_parent` and `test_ordering_parent_opened_before_child` - ---- - -## 5. Tests and Verification - -### Commands run - -| Command | Result | Evidence / Notes | -|---------|--------|------------------| -| `cargo test --no-run` | Not completed | User interrupted the compilation check | -| `cargo check` | Not run on final state | Was run earlier; had pre-existing errors in `lsp.rs` which user said they fixed | - -### Automated tests - -- Added (30 new test functions): - - `test_goto_def_class_method_call_resolves_method` — verifies method_defs lookup for `Demo.Utility.Helper` - - `test_goto_def_class_method_call_nonexistent_returns_empty` — verifies missing method returns None - - `test_goto_def_class_reference_resolves_to_class` — verifies `get_class_definition` for Demo.Utility - - `test_goto_def_oref_resolves_method_in_target_class` — verifies oref resolution in do_parameter - - `test_goto_def_oref_job_argument_resolves_method` — verifies oref resolution in job_argument - - `test_goto_def_multiple_inheritance_default_left` — verifies inheritance direction defaults - - `test_goto_def_multiple_inheritance_right_direction` — verifies `[Inheritance = right]` keyword - - `test_goto_implementation_deep_hierarchy_from_super` — verifies overrides of top-level method - - `test_goto_implementation_deep_hierarchy_from_mid` — verifies overrides of mid-level method - - `test_goto_implementation_no_overrides_returns_empty` — verifies empty result for leaf method - - `test_goto_implementation_class_subclasses` — verifies `get_class_implementations` - - `test_goto_implementation_class_with_no_subclasses` — verifies empty for standalone class - - `test_diagnostics_clean_cls_has_no_errors` — verifies zero ERROR nodes for valid .cls - - `test_diagnostics_syntax_error_cls_has_errors` — verifies ERROR nodes for broken .cls - - `test_diagnostics_clean_routine_has_no_errors` — verifies zero ERROR nodes for valid .mac - - `test_diagnostics_multiple_errors_routine` — verifies 2+ ERROR nodes for broken .mac - - `test_diagnostics_xml_injected_clean_has_no_errors` — verifies zero injected errors for valid XML - - `test_diagnostics_xml_injected_error_has_errors` — verifies injected errors for broken XML - - `test_ordering_child_opened_before_parent` — verifies late-binding resolution - - `test_ordering_parent_opened_before_child` — verifies normal order resolution - - `test_ordering_missing_class_reference_returns_empty` — verifies no crash on missing class - - `test_ordering_duplicate_open_is_idempotent` — verifies version update on re-open - - `test_goto_def_undefined_symbol_returns_empty` — verifies empty for nonexistent variable - - `test_goto_def_nonexistent_class_returns_empty` — verifies empty for missing class - - `test_did_open_cls_populates_class_id_and_name` — verifies .cls document tracking - - `test_did_open_routine_populates_class_name_as_routine` — verifies .mac document tracking - - `test_did_open_xml_no_class_id` — verifies .xml has no class_id - - `test_update_document_rebuilds_semantics` — verifies semantics survive update - - `test_refactor_no_changes_returns_none` — verifies None for modern syntax - - `test_refactor_workspace_excludes_xml` — verifies XML excluded from refactor - -### Manual verification - -- None performed (test-only changes, compilation was interrupted) - -### Not verified - -- Full compilation of `src/test.rs` against the crate — user interrupted `cargo test --no-run` -- Actual test execution — no `cargo test` was run -- XML diagnostic tests depend on `tree_sitter_objectscript_playground::LANGUAGE_OBJECTSCRIPT` being available (it is in workspace deps) -- The `tree_sitter_objectscript::LANGUAGE_OBJECTSCRIPT_UDL` import in diagnostic tests requires the `tree-sitter-objectscript` crate (in workspace deps) - ---- - -## 6. Dependencies, Config, and Environment Changes - -### Dependencies - -No dependency changes were made as part of this test suite work. - -### Configuration - -No configuration changes were made. - -### Environment assumptions - -No new environment assumptions were introduced. - ---- - -## 7. Generated Files, Build Artifacts, and Lockfiles - -No generated files, build artifacts, or lockfiles changed. - ---- - -## 8. Risks and Follow-ups - -### Risks - -- **Compilation not verified on final state**: The `cargo test --no-run` was interrupted. The tests may have minor compilation issues (e.g., the `tree_sitter_objectscript` import for `.cls` diagnostics tests). -- **Oref tests may fail if `get_oref_method_definition` requires variables to be in the dependency graph**: The oref fixtures use `[ProcedureBlock = 0]` to make variables public, but the oref lookup may need the full call graph to resolve `obj` → `Demo.Target`. -- **Some fixture files may parse differently than expected**: The tree-sitter grammars for ObjectScript have specific node kinds; if the fixture syntax doesn't produce the expected AST, tests may fail. - -### Follow-up work - -- Run `cargo test` and fix any compilation or assertion failures -- Add tests for: - - `did_change` incremental edit behavior - - `did_change_watched_files` file watcher behavior - - Code action response structure (what actions are offered for each file type) - - Execute command flow (document refactor vs workspace refactor) - - Capability negotiation (formatting flag) -- Consider adding the routine tag-call fixture tests (fixtures exist but no Rust test exercises them yet at the handler level) -- Consider end-to-end LSP protocol tests for `goto_definition` handler dispatch logic - -### Rollback plan - -1. Delete all untracked files: `git clean -fd objectscript-tests/diagnostics objectscript-tests/gotodef/class-method-call objectscript-tests/gotodef/multiple-inheritance objectscript-tests/gotodef/oref-contexts objectscript-tests/gotodef/routines objectscript-tests/implementation objectscript-tests/ordering` -2. Restore `src/test.rs`: `git checkout -- src/test.rs` -3. Restore `objectscript-tests/test-suite.md`: `git checkout -- objectscript-tests/test-suite.md` -4. Run `cargo test` to confirm previous state works - ---- - -## 9. Completeness Checklist - -- [x] Every file from `git status --short` is documented (modified files noted as pre-existing changes not part of this work, except `src/test.rs` and `test-suite.md`) -- [x] Every staged file is documented (none staged) -- [x] Every unstaged file is documented (`src/test.rs`) -- [x] Every untracked file is documented (26 fixtures + test-suite.md = 27 files) -- [x] Every deleted file is documented (none deleted) -- [x] Every renamed file is documented (none renamed) -- [x] Every generated file is documented (none generated) -- [x] Every lockfile is documented (none changed) -- [x] Every dependency change is documented (none) -- [x] Every config change is documented (none) -- [x] Every test change is documented (30 new tests) -- [x] Every documentation change is documented (test-suite.md rewrite) -- [x] Verification commands are listed with results -- [x] Unverified areas are explicitly listed -- [x] The reconstruction guide is complete enough to duplicate the implementation - ---- - -## Appendix A: Full Unified Diff - -The full patch for `src/test.rs` (899 lines) was saved during evidence collection. Regenerate with: - -```bash -cd objectscript-lsp && git diff HEAD -- src/test.rs -``` - -For the complete working tree diff: - -```bash -cd objectscript-lsp && git diff HEAD -``` - ---- - -## Appendix B: Git Status and File Inventory - -### `git status --short` (test-relevant files only) - -```text - M src/test.rs -?? objectscript-tests/diagnostics/ -?? objectscript-tests/gotodef/class-method-call/ -?? objectscript-tests/gotodef/multiple-inheritance/ -?? objectscript-tests/gotodef/oref-contexts/ -?? objectscript-tests/gotodef/routines/ -?? objectscript-tests/implementation/ -?? objectscript-tests/ordering/ -?? objectscript-tests/test-suite.md -``` - -### Untracked files (26 fixture files + 1 doc) - -```text -objectscript-tests/diagnostics/clean.cls -objectscript-tests/diagnostics/clean.mac -objectscript-tests/diagnostics/injected-clean.xml -objectscript-tests/diagnostics/injected-error.xml -objectscript-tests/diagnostics/multiple-errors.mac -objectscript-tests/diagnostics/syntax-error.cls -objectscript-tests/gotodef/class-method-call/caller.cls -objectscript-tests/gotodef/class-method-call/utility.cls -objectscript-tests/gotodef/multiple-inheritance/base.cls -objectscript-tests/gotodef/multiple-inheritance/child-default.cls -objectscript-tests/gotodef/multiple-inheritance/child-right.cls -objectscript-tests/gotodef/multiple-inheritance/left-parent.cls -objectscript-tests/gotodef/multiple-inheritance/right-parent.cls -objectscript-tests/gotodef/oref-contexts/oref-do-parameter.cls -objectscript-tests/gotodef/oref-contexts/oref-job-argument.cls -objectscript-tests/gotodef/oref-contexts/target.cls -objectscript-tests/gotodef/routines/cross-routine-ref.mac -objectscript-tests/gotodef/routines/offset-goto.mac -objectscript-tests/gotodef/routines/tag-calls.mac -objectscript-tests/implementation/deep-leaf-one.cls -objectscript-tests/implementation/deep-leaf-two.cls -objectscript-tests/implementation/deep-mid.cls -objectscript-tests/implementation/deep-super.cls -objectscript-tests/implementation/no-overrides.cls -objectscript-tests/ordering/child.cls -objectscript-tests/ordering/parent.cls -objectscript-tests/test-suite.md -``` - ---- - -## Appendix C: New Test Functions Summary - -| # | Function name | Type | Fixture directory | -|---|---------------|------|-------------------| -| 1 | `test_goto_def_class_method_call_resolves_method` | async | gotodef/class-method-call | -| 2 | `test_goto_def_class_method_call_nonexistent_returns_empty` | async | gotodef/class-method-call | -| 3 | `test_goto_def_class_reference_resolves_to_class` | async | gotodef/class-method-call | -| 4 | `test_goto_def_oref_resolves_method_in_target_class` | async | gotodef/oref-contexts | -| 5 | `test_goto_def_oref_job_argument_resolves_method` | async | gotodef/oref-contexts | -| 6 | `test_goto_def_multiple_inheritance_default_left` | async | gotodef/multiple-inheritance | -| 7 | `test_goto_def_multiple_inheritance_right_direction` | async | gotodef/multiple-inheritance | -| 8 | `test_goto_implementation_deep_hierarchy_from_super` | async | implementation | -| 9 | `test_goto_implementation_deep_hierarchy_from_mid` | async | implementation | -| 10 | `test_goto_implementation_no_overrides_returns_empty` | async | implementation | -| 11 | `test_goto_implementation_class_subclasses` | async | implementation | -| 12 | `test_goto_implementation_class_with_no_subclasses` | async | implementation | -| 13 | `test_diagnostics_clean_cls_has_no_errors` | sync | diagnostics | -| 14 | `test_diagnostics_syntax_error_cls_has_errors` | sync | diagnostics | -| 15 | `test_diagnostics_clean_routine_has_no_errors` | sync | diagnostics | -| 16 | `test_diagnostics_multiple_errors_routine` | sync | diagnostics | -| 17 | `test_diagnostics_xml_injected_clean_has_no_errors` | sync | diagnostics | -| 18 | `test_diagnostics_xml_injected_error_has_errors` | sync | diagnostics | -| 19 | `test_ordering_child_opened_before_parent` | async | ordering | -| 20 | `test_ordering_parent_opened_before_child` | async | ordering | -| 21 | `test_ordering_missing_class_reference_returns_empty` | async | ordering | -| 22 | `test_ordering_duplicate_open_is_idempotent` | async | ordering | -| 23 | `test_goto_def_undefined_symbol_returns_empty` | async | gotodef/class-method-call | -| 24 | `test_goto_def_nonexistent_class_returns_empty` | async | gotodef/class-method-call | -| 25 | `test_did_open_cls_populates_class_id_and_name` | sync | (inline content) | -| 26 | `test_did_open_routine_populates_class_name_as_routine` | sync | (inline content) | -| 27 | `test_did_open_xml_no_class_id` | sync | (inline content) | -| 28 | `test_update_document_rebuilds_semantics` | async | ordering | -| 29 | `test_refactor_no_changes_returns_none` | sync | (inline content) | -| 30 | `test_refactor_workspace_excludes_xml` | async | diagnostics | diff --git a/objectscript-lsp/documentation/features/diagnostics.md b/objectscript-lsp/documentation/features/diagnostics.md new file mode 100644 index 0000000..aff4c5e --- /dev/null +++ b/objectscript-lsp/documentation/features/diagnostics.md @@ -0,0 +1,51 @@ +# Diagnostics Feature + +## Overview + +The diagnostics feature reports syntax errors in ObjectScript and XML files. It uses tree-sitter parse trees to detect error nodes and reports them as LSP diagnostics with severity `ERROR`. + +## Supported File Types + +| File Type | Diagnostics Provided | +|---|---| +| Class (`.cls`) | ObjectScript syntax errors | +| Routine (`.mac`, `.int`, `.rtn`, `.inc`) | ObjectScript syntax errors | +| XML (`.xml`) | XML syntax errors + ObjectScript errors in `` blocks | + +## Diagnostic Types + +### ObjectScript Syntax Errors + +Reported when tree-sitter produces error or missing nodes in the parse tree. Each error node becomes a diagnostic at that node's source range. + +**Context-aware messages** — when the error occurs in a recognizable context, a more helpful message is produced: + +| Context | Message | +|---|---| +| After `Set` keyword with no target | "Expected a variable name, got {text}" | +| After `set_target` with no `=` | "Expected '=' or another variable name..." | +| After `=` in `Set` with no expression | "Expected an expression, {text} is not a valid expression." | +| After expression in `Set` | "Unexpected, {text} after an expression. Expected a binary operator or end of SET command" | +| All other cases | "Syntax Error: Unexpected {text}" | + +### XML Syntax Errors + +For XML files, tree-sitter XML error nodes produce diagnostics with the message format: `"XML syntax error: Unexpected {text}"`. + +### Mixed-Language Diagnostics (XML + ObjectScript) + +For XML files that contain ObjectScript code inside `` CDATA blocks, the LSP performs a second diagnostic pass: + +1. Finds all `` elements in the XML tree +2. Extracts the CDATA or CharData content from each +3. Parses each extracted region as ObjectScript using an independent parser +4. Reports any ObjectScript syntax errors found within those regions + +This provides syntax checking for ObjectScript code that is embedded within XML class export files. + +## Behavior + +- Diagnostics are computed on-demand via the `textDocument/diagnostic` pull model (not push-based `textDocument/publishDiagnostics`) +- Each diagnostic request returns a full report for the requested document +- Inter-file dependencies are not tracked (a change in one file does not trigger diagnostics in another) +- Workspace-level diagnostics are not supported (only per-document) diff --git a/objectscript-lsp/documentation/features/goto-def.md b/objectscript-lsp/documentation/features/goto-def.md new file mode 100644 index 0000000..62ed409 --- /dev/null +++ b/objectscript-lsp/documentation/features/goto-def.md @@ -0,0 +1,84 @@ +# Goto-Definition Feature + +## Overview + +The goto-definition feature resolves the definition site of a symbol under the cursor. It handles classes, methods (instance, class, procedures, subroutines, relative dot methods, and oref-chained), variables (private and public), superclass navigation, and numeric line offsets. + +## Supported Symbols + +### Classes + +| Cursor position | Navigation target | +|---|---| +| Class name at its own definition (`Class MyClass Extends Super`) | The superclass(es) it extends | +| Class name used as a reference (`##class(Demo.Person)`) | The class definition file of that class name | +| Routine name in a line reference (`^RoutineName`) | The routine file | + +### Methods + +| Cursor position | Navigation target | +|---|---| +| Method name at its definition site (`Method Save()`) | The superclass method it overrides (if any) | +| Relative dot method call (`..MethodName()`) | The method definition in the same class | +| Class method call (`##class(Cls).MethodName()`) | The method definition in the referenced class | +| Same-file label call (`Do Label`, `Write Label`) | The label/subroutine definition in the current file | +| Cross-routine label call (`Do Label^Routine`, `$$Label^Routine`) | The label definition in the referenced routine | +| Label with offset (`Do Label+3^Routine`) | The method definition, offset by N lines | +| Oref method call, cursor on method (`obj.Method()`) | The method definition in the resolved class of `obj` | +| Oref method from `%New` (`##class(Cls).%New().Method()`) | The method definition in `Cls` | + +### Variables + +| Cursor position | Navigation target | +|---|---| +| Local variable reference | The closest preceding definition of that variable in the same scope (or parent scopes) | +| Public variable (non-ProcedureBlock or declared public) | The closest preceding definition, potentially across calling methods via the dependency graph | +| Oref variable, cursor on variable name (`obj` in `obj.Method()`) | The assignment site where the oref was created | +| Global variable (`^GlobalName`) | Same resolution as local variables (scope-based lookup) | + +### Line Offsets + +| Cursor position | Navigation target | +|---|---| +| Numeric literal in a tag call (`3` in `Do Label+3`) | The line N rows below the current position | + +## Variable Resolution Rules + +Variable resolution picks the **closest definition that appears before the reference**: + +- If multiple `Set` commands assign the same variable in the same scope, goto-def resolves to the one nearest (but still before) the cursor. +- If no definition exists in the current scope, parent scopes and ancestor scopes are checked. +- For public variables, the DependencyGraph traces callers (via BFS) and finds the definition in the shallowest ancestor method that defines the variable before the call site. + +**Example:** +```objectscript +Set x = 2 +Set y = 3 +Set x = 3 +Write x ; goto-def on x here → resolves to "Set x = 3" +``` + +## Oref Resolution Rules + +When resolving `obj.Method()`, the LSP must determine what class `obj` is an instance of: + +1. Finds the assignment of `obj` (e.g., `Set obj = ##class(Demo.Person).%New()`) +2. Extracts the class from the `%New()` call +3. Looks up `Method` in that class's method definitions + +If the variable is public, the same DependencyGraph-based ancestor traversal is used to find the assignment across method boundaries. + +## Scopes + +Scopes that create boundaries for variable resolution: +- Classes +- Methods +- Subroutines / Procedures +- Conditional blocks (`If`/`ElseIf`/`Else`) + +## Inheritance Rules for Superclass Navigation + +When navigating from a method definition to its superclass override, the LSP uses the OverrideIndex which respects ObjectScript's inheritance rules: if a class inherits from multiple superclasses that define the same method, the override resolution follows the language's defined precedence order. + +## TODO +I still need to use `kill` and `new` statements in my analysis of what variable definitions are actually valid from a given method. If a `kill` statement appears, any definitions that came before that should be nullified. diff --git a/objectscript-lsp/documentation/features/goto-implementation.md b/objectscript-lsp/documentation/features/goto-implementation.md new file mode 100644 index 0000000..ad53651 --- /dev/null +++ b/objectscript-lsp/documentation/features/goto-implementation.md @@ -0,0 +1,27 @@ +# Goto-Implementation Feature + +## Overview + +The goto-implementation feature navigates from a class or method definition to its **subclass overrides**. This is the inverse of goto-definition's superclass navigation — where goto-def goes "up" the inheritance chain, goto-implementation goes "down" to find subclasses and overriding methods. + +## Supported Symbols + +### Classes + +| Cursor position | Navigation target | +|---|---| +| Class name at its own definition (`Class MyClass`) | All subclasses that extend this class | +| Class name used as a reference (`##class(MyClass)`) | All subclasses that extend the referenced class | + +### Methods + +| Cursor position | Navigation target | +|---|---| +| Method name at its definition site (`Method Save()`) | All subclass methods that override this method | +| Method name in a class method call (`##class(Cls).Method()`) | All subclass methods that override the referenced method | + +## Behavior + +- If exactly one implementation is found, the editor navigates directly to it. +- If multiple implementations are found, the editor presents a picker with all locations. +- If no implementations are found, a warning message is shown: "No implementations were found for the given symbol." diff --git a/objectscript-lsp/documentation/features/implementations/diagnostics-implementation.md b/objectscript-lsp/documentation/features/implementations/diagnostics-implementation.md new file mode 100644 index 0000000..45fd51a --- /dev/null +++ b/objectscript-lsp/documentation/features/implementations/diagnostics-implementation.md @@ -0,0 +1,179 @@ +# Diagnostics Implementation + +## Entry Point + +`lsp.rs:588` — `async fn diagnostic` + +## Architecture + +``` +┌─────────────────────────────────────────────────────────────┐ +│ LSP Handler (lsp.rs:diagnostic) │ +│ - Gets document snapshot (file_type, content, tree) │ +│ - Calls push_host_syntax_diagnostics for all file types │ +│ - For XML: additionally calls │ +│ push_xml_injected_objectscript_diagnostics │ +│ - Returns FullDocumentDiagnosticReport │ +└────────────────────────────┬────────────────────────────────┘ + │ + ┌──────────────┼──────────────┐ + │ │ + ▼ ▼ +┌──────────────────────────┐ ┌──────────────────────────────┐ +│ push_host_syntax_ │ │ push_xml_injected_ │ +│ diagnostics() │ │ objectscript_diagnostics() │ +│ │ │ │ +│ - collect_error_nodes() │ │ - xml_objectscript_ │ +│ - diagnostic_message() │ │ implementation_ranges() │ +│ │ │ - Parse each range as OS │ +│ │ │ - push_host_syntax_ │ +│ │ │ diagnostics() per range │ +└──────────────────────────┘ └──────────────────────────────┘ +``` + +## Server Capabilities + +Registered in `build_caps` (`lsp.rs:296`): + +```rust +diagnostic_provider: Some(DiagnosticServerCapabilities::Options(DiagnosticOptions { + identifier: None, + inter_file_dependencies: false, + workspace_diagnostics: false, + work_done_progress_options: Default::default(), +})) +``` + +This registers pull-based diagnostics (client requests via `textDocument/diagnostic`), not push-based. + +## Handler Flow (`lsp.rs:588`) + +``` +1. Get document URI from params +2. Get project from document URL +3. Take snapshot: (file_type, content, tree) from project state +4. Call push_host_syntax_diagnostics(diagnostics, content, tree, file_type) +5. If file_type == XML: + a. Record host error count + b. Call push_xml_injected_objectscript_diagnostics(diagnostics, content, tree) + c. Log host vs total error counts +6. Return FullDocumentDiagnosticReport with all diagnostics +``` + +## Key Functions + +### `push_host_syntax_diagnostics` (`lsp.rs:131`) + +Finds all error nodes in the document's tree-sitter parse tree and converts them to LSP diagnostics. + +``` +1. Call collect_error_nodes(tree.root_node()) → Vec +2. For each error node: + a. Convert node range to LSP range via ts_range_to_lsp_range + b. Get error text from node byte range + c. Generate message: + - If XML: "XML syntax error: Unexpected {text}" + - Else: try diagnostic_message(node, text) for context-aware message + - Fallback: "Syntax Error: Unexpected {text}" + d. Push Diagnostic with severity ERROR +``` + +### `push_xml_injected_objectscript_diagnostics` (`lsp.rs:164`) + +Finds ObjectScript code embedded in XML `` blocks and runs syntax checking on each. + +``` +1. Call xml_objectscript_implementation_ranges(tree.root_node(), content) → Vec +2. For each range: + a. Extract text at range, skip if empty/whitespace + b. Create fresh Parser with ObjectScript language + c. Set parser included_ranges to [range] (restricts parsing to that region) + d. Parse content with the constrained parser + e. Call push_host_syntax_diagnostics on the resulting tree (as FileType::Routine) +``` + +### `collect_error_nodes` (`common.rs:1082`) + +Recursively walks the tree-sitter tree collecting error and missing nodes. + +``` +1. Start with root node, walk with TreeCursor +2. visit_errors(node, cursor, out): + a. If node has no error, is not error, and is not missing → return (prune subtree) + b. If node is_error() or is_missing() → push to output + c. Recurse into children +``` + +The early return on `!node.has_error()` prunes healthy subtrees for efficiency. + +### `xml_objectscript_implementation_ranges` (`common.rs:1090`) + +Uses a tree-sitter query to find ObjectScript code inside XML Implementation elements. + +``` +1. Create Query from XML_OBJECTSCRIPT_INJECTIONS_QUERY +2. Run query against XML tree root +3. For each match, find capture named "injection.content" +4. Push non-empty ranges to output +5. Sort and deduplicate by (start_byte, end_byte) +``` + +**Query pattern (`common.rs:14`):** +```scheme +(element + (STag + (Name) @_name) + (content + (CDSect + (CData) @injection.content)) + (#eq? @_name "Implementation") + (#set! injection.language "objectscript")) + +(element + (STag + (Name) @_name) + (content + (CharData) @injection.content) + (#eq? @_name "Implementation") + (#set! injection.language "objectscript")) +``` + +Two patterns handle both `` wrapped content and plain text content within `` tags. + +### `diagnostic_message` (`src/common.rs:4`) + +Generates context-aware error messages by inspecting the previous sibling of the error node. Currently only provides enhanced messages for `command_set` errors: + +``` +1. Get prev_named_sibling of error node +2. If sibling is a "statement": + a. Get its first child command + b. If "command_set": + - Get last child of command_set + - Match on child kind: + - "keyword_set" → "Expected a variable name, got {text}" + - "set_argument" → inspect further: + - Last child is set_target with = sibling → "Expected an expression..." + - Last child is set_target without = → "Expected '=' or another variable name..." + - Last child is expression → "Unexpected {text} after expression..." +3. Return None if no context-aware message applies (falls back to generic message) +``` + +## Diagnostic Properties + +All diagnostics produced share these properties: + +| Property | Value | +|---|---| +| `severity` | `DiagnosticSeverity::ERROR` | +| `code` | `None` | +| `source` | `None` | +| `related_information` | `None` | +| `tags` | `None` | + +## XML Mixed-Language Strategy + +The XML diagnostic pass uses parser `set_included_ranges` to constrain the ObjectScript parser to only the bytes within an `` block. This means: +- The parser sees the raw content bytes at their original offsets in the file +- Error ranges reported by the ObjectScript parser map directly to the correct positions in the XML document +- No offset translation is needed — the LSP range conversion works on the same content string diff --git a/objectscript-lsp/documentation/features/implementations/goto-def-implementation.md b/objectscript-lsp/documentation/features/implementations/goto-def-implementation.md new file mode 100644 index 0000000..c017af2 --- /dev/null +++ b/objectscript-lsp/documentation/features/implementations/goto-def-implementation.md @@ -0,0 +1,256 @@ +# Goto-Definition Implementation + +## Architecture + +``` +┌─────────────────────────────────────────────────────────────┐ +│ LSP Handler (lsp.rs:goto_definition) │ +│ - Resolves cursor position to tree-sitter node │ +│ - Classifies node via get_outer_type_from_identifier() │ +│ - Dispatches to resolution function by MemberType │ +└────────────────────────────┬────────────────────────────────┘ + │ + ┌───────────────────┼───────────────────────┐ + │ │ │ + ▼ ▼ ▼ +┌─────────────────┐ ┌────────────────┐ ┌─────────────────────┐ +│ get_class_ │ │ get_method_ │ │ get_variable_ │ +│ definition() │ │ definition() │ │ definition() │ +│ │ │ │ │ │ +│ get_class_ │ │ get_oref_ │ │ find_classes_ │ +│ superclasses() │ │ definitions() │ │ from_oref() │ +│ │ │ │ │ │ +│ get_method_ │ │ │ │ │ +│ superclass() │ │ │ │ │ +└─────────────────┘ └────────────────┘ └─────────────────────┘ + │ │ │ + └───────────────────┼───────────────────────┘ + ▼ + ┌──────────────────────────────┐ + │ Data Structures │ + │ - ScopeTree (per document) │ + │ - GlobalSemanticModel │ + │ - DependencyGraph │ + │ - OverrideIndex │ + └──────────────────────────────┘ +``` + +## Node Classification + +When the cursor is on an identifier, the handler calls `get_outer_type_from_identifier` (`common.rs:907`) to determine the `MemberType` of the enclosing construct. This drives which resolution path is used. + +| Tree-sitter node kind | MemberType produced | Notes | +|---|---|---| +| `class_name` (in `class_definition`) | `ClassDef` | The class's own name | +| `class_name` (elsewhere) | `Class` | A class reference | +| `method_name` → `oref_method` → `relative_dot_method` | `RelativeMethodCall` | `..MethodName()` | +| `method_name` → `oref_method` (other parent) | `OrefMethod` | `obj.Method()` | +| `method_name` → `routine_tag_call`/`print_argument`/`goto_argument`/`extrinsic_function`/`line_ref` | `RoutineMethodCall` | `Do Label`, `$$Label`, etc. | +| `method_name` → `class_method_call`/`system_defined_function` | `ClassMethodCall` | `##class(Cls).Method()` | +| `method_name` → `method_definition` | `MethodDef` | At the definition site itself | +| `lvn` → `oref_chain_expr`/`class_ref` | `OrefMethod` | The variable portion of `x.Method()` | +| `lvn` (other parent) | `LocalVariable` | | +| `gvn` | `GlobalVariable` | | + +Additionally, bare `routine_name` nodes and `numeric_literal` nodes (for line offsets) are handled without going through `MemberType`. + +## Resolution Functions + +### `get_class_superclasses` (`workspace.rs:1541`) + +- Looks up the class in `GlobalSemanticModel` using its `ClassId` +- Iterates over `class.inherited_classes` +- Returns the location(s) of each superclass definition + +### `get_method_superclass` (`workspace.rs:1503`) + +- Finds the `MethodRef` for the method in the current class +- Looks up `override_index.overrides` to find the superclass `MethodRef` it overrides +- Returns the location from the superclass method symbol + +### `get_class_definition` (`workspace.rs:1439`) + +- Looks up the class name in `self.classes` → `ClassId` +- Retrieves the class symbol from `GlobalSemanticModel` +- Returns the `(url, range)` of the class definition + +### `get_method_definition` (`workspace.rs:1134`) + +- Looks up `method_defs[class_name][method_name]` → `MethodRef` +- Checks `GlobalSemanticModel` for a public method symbol first +- Falls back to `scope_tree.get_private_method_symbol` for private methods +- If an offset is provided, adjusts the target range by adding the offset to the start row +- Returns the definition location + +### `get_oref_definitions` (`workspace.rs:1382`) + +- Calls `find_classes_from_oref` to determine what class the oref variable is an instance of +- If `resolve_method = true`: calls `get_method_definition` for each resolved class +- If `resolve_method = false`: returns the variable definition locations directly + +### `get_variable_definition` (`workspace.rs:1190`) + +Three-tier resolution (see algorithm below). + +### `find_classes_from_oref` (`workspace.rs:1627`) + +Resolves what class an oref variable is an instance of (see algorithm below). + +## Key Data Structures + +### ScopeTree (`scope_tree.rs`) + +Per-document tree of lexical scopes. Each `Scope` node contains: +- `private_variable_defs`: variable name → `Vec` (private definitions in this scope) +- `public_var_defs`: variable name → `Vec` (public definitions) +- `variable_symbols`: `Vec` (location + dependency metadata) +- `children`: child scope IDs +- `method`: optional method name this scope belongs to + +The scope tree is used for local variable lookup. `find_current_scope(point)` finds the innermost scope containing a position, and `get_scope_children` collects all nested scopes for recursive search. + +### DependencyGraph (`dependency_tracker.rs`) + +A directed graph (`petgraph::DiGraph`) where: +- Nodes = methods (`MethodRef`) +- Edges = caller → callee, weighted by the call-site `Range` + +`all_ancestors(target)` performs BFS over incoming edges to find all transitive callers, returning `(MethodRef, call_site_Range, depth)` triples sorted by depth. This enables cross-method variable resolution for public variables. + +### OverrideIndex (`override_index.rs`) + +Tracks method overriding relationships: +- `overrides`: subclass `MethodRef` → superclass `MethodRef` it overrides +- `overridden_by`: superclass `MethodRef` → all subclass `MethodRef`s that override it +- `effective_public_methods`: per-class map of method name → resolved `MethodRef` + +Used by `get_method_superclass` for navigating up the inheritance chain. + +### GlobalSemanticModel + +Stores workspace-wide symbols: +- Class symbols (`ClassGlobalSymbol`) — name, url, location +- Method symbols (`MethodGlobalSymbol`) — for public methods +- Variable symbols (`VariableGlobalSymbol`) — for public variables + +### ProjectState Fields + +- `method_defs: HashMap>` — class name → method name → MethodRef +- `pub_var_defs: HashMap>>>` — variable name → method → scope → refs +- `classes: HashMap` — class name → ClassId + +## Variable Resolution Algorithm + +`get_variable_definition` (`workspace.rs:1190`): + +``` +Input: (document_url, cursor_point, variable_name) + +1. Find the current method name from scope tree +2. Find the current scope ID from scope tree +3. Look up the MethodRef for this method + +TIER 1 — Private variable in scope: + scope_tree.get_variable_definition(name, scope_id) → Vec<(ScopeId, Vec)> + For each (child_scope_id, ranges) where child is current scope or a child of it: + For each range that ends BEFORE cursor: + If first definition seen in this scope → record it + If later definition in same scope → replace (closest-before wins) + If any found → return + +TIER 2 — Public variable in current method's scope: + scope_tree.pub_variable_in_scope(name, scope_id) → Vec<(ScopeId, Vec)> + For each VariableRef with pub_id: + Resolve via global_semantic_model.get_variable_symbol(method_ref, id, scope_id) + Same closest-before-cursor logic + If any found → return + +TIER 3 — Public variable from ancestor callers: + Check is_variable_public(method_ref, variable_name) + If public: + Get node_index from dependency_graph + Get pub_var_defs[variable_name] → all methods that define this variable + BFS all_ancestors(node_index) → (ancestor_ref, call_range, depth) + For each ancestor at current depth: + For each definition in ancestor that ends BEFORE the call site: + Same closest-before logic + Stop at first depth that yields results (shallow callers win) +``` + +## Oref Resolution Algorithm + +`find_classes_from_oref` (`workspace.rs:1627`): + +``` +Input: (oref_variable_name, method_name_called, current_class, call_site_range) + +1. Find current document, method, and scope +2. Determine if variable is public via is_variable_public() +3. Look up all oref references in scope via get_oref_references(name, scope_id) + +For each variable ref found: + If PRIVATE: + Get variable from local_semantic_model + Check variable.is_oref == true + Extract variable.cls (the class it was instantiated as) + Look up method_defs[oref_class][method_called] → MethodRef + Apply closest-before-cursor logic + If PUBLIC: + Get variable from global_semantic_model + Same is_oref + cls check + Same method lookup + +If public and nothing found locally: + Traverse DependencyGraph ancestors (same BFS pattern as variable resolution) + For each ancestor that defines this variable: + Check is_oref + extract class name + Resolve method in that class + Stop at shallowest depth with results + +Returns: (Vec<(MethodRef, Range)>, Vec<(Url, Range)>) + - First vec: the resolved method refs + their call ranges + - Second vec: the variable definition locations +``` + +## `is_variable_public` (`workspace.rs:1406`) + +Determines if a variable is public (visible across method boundaries): +- `true` if the method has `ProcedureBlock = 0` +- `true` if the class has `ProcedureBlock = 0` and the method doesn't override it +- `true` if the variable is in `method.public_variables_declared` +- `false` otherwise + +## Dispatch Logic in LSP Handler + +The handler (`lsp.rs:649-1174`) follows this flow: + +1. Get document content, tree, class_id, class_name from project state +2. Convert LSP position → tree-sitter Point +3. Find smallest named node at that point +4. If node is `identifier`/`objectscript_identifier`/`objectscript_identifier_special`: + - Get parent node → classify via `get_outer_type_from_identifier` + - Match on `MemberType` → dispatch to appropriate resolution function +5. Else if node is `routine_name` → `get_class_definition` +6. Else if node is `gvn` → find identifier child → `get_variable_definition` +7. Else if node is `lvn` (bare, no children match above) → `get_variable_definition` +8. Else if node is `numeric_literal` in a tag call → compute line offset +9. Convert results to LSP Locations: + - 0 results → `None` + - 1 result → `GotoDefinitionResponse::Scalar` + - 2+ results → `GotoDefinitionResponse::Array` + +## OrefMethod Sub-dispatch + +The `MemberType::OrefMethod` case has additional branching based on the node kind: + +- **`method_name`**: cursor is on the method being called + - Parent is `oref_method` → grandparent is `oref_chain_segment` or `do_parameter` + - Extract the variable from the first child of the oref expression + - If variable is `lvn`: call `get_oref_definitions(var, method, class, range, true)` + - If variable is `class_method_call` with `%New`: extract class directly → `get_method_definition` + +- **`lvn`**: cursor is on the object variable itself + - Parent determines context: + - `class_ref` → parent is `class_method_call` or `oref_chain_expr` → extract method name → `get_oref_definitions(..., false)` + - `do_parameter` / `job_argument` → extract method from sibling `oref_method` node → `get_oref_definitions(..., false)` + - `oref_chain_expr` → extract method from second child → `get_oref_definitions(..., false)` diff --git a/objectscript-lsp/documentation/features/implementations/goto-implementation-impl.md b/objectscript-lsp/documentation/features/implementations/goto-implementation-impl.md new file mode 100644 index 0000000..0273150 --- /dev/null +++ b/objectscript-lsp/documentation/features/implementations/goto-implementation-impl.md @@ -0,0 +1,158 @@ +# Goto-Implementation Implementation + +## Entry Point + +`lsp.rs:1176` — `async fn goto_implementation` + +## Architecture + +``` +┌─────────────────────────────────────────────────────────────┐ +│ LSP Handler (lsp.rs:goto_implementation) │ +│ - Resolves cursor position to tree-sitter node │ +│ - Classifies node via get_outer_type_from_identifier() │ +│ - Dispatches by MemberType (Class, ClassMethodCall, │ +│ MethodDef only) │ +└────────────────────────────┬────────────────────────────────┘ + │ + ┌──────────────┼──────────────┐ + │ │ │ + ▼ ▼ ▼ + ┌──────────────┐ ┌─────────────┐ ┌─────────────┐ + │ get_class_ │ │ get_method_ │ │ get_method_ │ + │ implementa- │ │ overrides() │ │ overrides() │ + │ tions() │ │ (from call) │ │ (from def) │ + └──────────────┘ └─────────────┘ └─────────────┘ + │ │ │ + └──────────────┼──────────────┘ + ▼ + ┌──────────────────────────────┐ + │ Data Structures │ + │ - OverrideIndex │ + │ - Dependents (class index) │ + │ - GlobalSemanticModel │ + └──────────────────────────────┘ +``` + +## Node Classification + +Only three `MemberType` variants are handled; all others return `Ok(None)`: + +| Tree-sitter node kind | MemberType | Handling | +|---|---|---| +| `class_name` (any) | `Class` | Further branched by parent node kind | +| `method_name` → `class_method_call` | `ClassMethodCall` | Method override lookup | +| `method_name` → `method_definition` | `MethodDef` | Method override lookup | + +## Resolution Paths + +### 1. Class (`MemberType::Class`) + +Two sub-cases based on the parent node of `class_name`: + +#### 1a. Class name in `class_definition` (own definition) + +Uses the current document's `class_id` directly. + +**Resolution:** `get_class_implementations` (`workspace.rs:1457`) +- Looks up `dependent_class_index.dependent_classes[class_id]` → `Vec` +- For each dependent class ID, retrieves the class symbol from `GlobalSemanticModel` +- Returns all `(url, range)` pairs for the subclass definitions + +#### 1b. Class name as a reference (elsewhere) + +Extracts the full class name string from the `class_name` node, then looks up its `ClassId` from `self.classes`. + +**Resolution:** Same `get_class_implementations` with the looked-up `ClassId`. + +--- + +### 2. Class Method Call (`MemberType::ClassMethodCall`) + +When the cursor is on the method name in `##class(ClassName).MethodName()`. + +**Resolution:** +- Extracts the class name from the `class_ref` child node +- Looks up `method_defs[class_name][method_name]` → `MethodRef` +- Calls `get_method_overrides` (`workspace.rs:1576`) + +--- + +### 3. Method Definition (`MemberType::MethodDef`) + +When the cursor is on a method name at its definition site. + +**Resolution:** +- Gets the class from `GlobalSemanticModel` using the current `class_id` +- Looks up `class.methods[method_name]` → `MethodRef` +- Calls `get_method_overrides` + +--- + +## Resolution Functions + +### `get_class_implementations` (`workspace.rs:1457`) + +``` +Input: class_id + +1. Look up dependent_class_index.dependent_classes[class_id] → Vec +2. For each dependent ClassId: + a. Get class struct from GlobalSemanticModel → extract class name + b. Look up class name in self.classes → ClassId (symbol id) + c. Get class symbol from GlobalSemanticModel → (url, range) +3. Return all locations +``` + +### `get_method_overrides` (`workspace.rs:1576`) + +``` +Input: method_ref (the superclass method) + +1. Look up override_index.overridden_by[method_ref] → Vec +2. For each overriding MethodRef: + a. Try GlobalSemanticModel.get_method_symbol → (url, range) [public methods] + b. Fallback: get document by class url → scope_tree.get_private_method_symbol → (url, range) [private methods] +3. Return all locations +``` + +## Key Data Structures + +### Dependents (`dependency_tracker.rs`) + +```rust +pub struct Dependents { + pub dependent_classes: HashMap>, +} +``` + +Maps each class to all classes that directly inherit from it. Built during workspace indexing when inheritance relationships are resolved. + +### OverrideIndex (`override_index.rs`) + +The `overridden_by` field is the key structure for this feature: + +```rust +pub overridden_by: HashMap> +``` + +Maps a superclass method → all subclass methods that override it. This is the inverse of the `overrides` map used by goto-definition. + +## Dispatch Logic + +The handler (`lsp.rs:1176-1361`) follows this flow: + +1. Get document content, tree, class_id from project state +2. Convert LSP position → tree-sitter Point +3. Find smallest named node at that point +4. If node is `identifier` or `objectscript_identifier`: + - Get parent node → classify via `get_outer_type_from_identifier` + - Match on `MemberType`: + - `Class` → branch on parent kind → `get_class_implementations` + - `ClassMethodCall` → extract class + method → `get_method_overrides` + - `MethodDef` → get method from class → `get_method_overrides` + - All others → return `None` +5. Convert results to LSP Locations: + - 0 results → warning message + `None` + - 1 result → `GotoImplementationResponse::Scalar` + - 2+ results → `GotoImplementationResponse::Array` diff --git a/objectscript-lsp/documentation/features/implementations/refactor-implementation.md b/objectscript-lsp/documentation/features/implementations/refactor-implementation.md new file mode 100644 index 0000000..5149d27 --- /dev/null +++ b/objectscript-lsp/documentation/features/implementations/refactor-implementation.md @@ -0,0 +1,311 @@ +# Refactor Feature Implementation + +## Entry Points + +- **Code Action Provider:** `lsp.rs:410` — `async fn code_action` +- **Command Executor:** `lsp.rs:507` — `async fn execute_command` +- **Core Logic:** `refactor.rs` (1687 lines) + +## Architecture + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Code Action Provider (lsp.rs:code_action) │ +│ - Checks if refactor.rewrite kind is requested │ +│ - Builds menu of available refactor commands per file type │ +│ - Returns CodeAction list with command references │ +└────────────────────────────┬────────────────────────────────┘ + │ user selects action + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ Execute Command (lsp.rs:execute_command) │ +│ - Parses URI + RefactorLevel from command arguments │ +│ - Dispatches to document or workspace refactor │ +│ - Sends workspace/applyEdit to editor │ +└────────────────────────────┬────────────────────────────────┘ + │ + ┌──────────────┼──────────────┐ + │ │ │ + ▼ ▼ ▼ + ┌──────────────┐ ┌─────────────┐ ┌─────────────┐ + │ refactor_ │ │ refactor_ │ │ refactor_ │ + │ legacy_do_ │ │ conditionals│ │ for_ │ + │ statements()│ │ () │ │ statements()│ + └──────────────┘ └─────────────┘ └─────────────┘ +``` + +## LSP Integration + +### Commands + +| Command ID | Scope | +|---|---| +| `objectscript.refactorDocument` | Single file | +| `objectscript.refactorWorkspace` | All workspace files | +| `objectscript.refactorWorkspaceDottedDo` | Legacy alias (Do only, workspace) | + +### Command Arguments + +Each command receives two arguments: +1. Document URI (string) +2. Refactor level: `"all"`, `"do"`, `"conditionals"`, or `"for"` + +### Code Action Generation (`lsp.rs:410`) + +``` +1. Check refactor_kind_requested — only proceed if refactor.rewrite is in the requested kinds +2. Get document file type and parse tree +3. If document has no parse errors: + - Add document-scoped refactor actions for each applicable level + - "Refactor All Code in this document" is marked as is_preferred +4. If not XML and not an automatic trigger: + - Add workspace-scoped refactor actions for all 4 levels +5. Return the action list +``` + +### Execute Command (`lsp.rs:507`) + +``` +1. Parse URI from arguments[0] +2. Parse RefactorLevel from arguments[1] (or command name for legacy) +3. Get project from document URL +4. If document command: + - Call build_document_refactor_edit → full-document TextEdit + - Send workspace/applyEdit +5. If workspace command: + - Call collect_workspace_refactor_changes → HashMap> + - Send workspace/applyEdit with all changes +``` + +### Helper Functions (`lsp.rs`) + +| Function | Location | Purpose | +|---|---|---| +| `refactor_kind_requested` | `lsp.rs:74` | Checks if `refactor.rewrite` is in the requested CodeActionKinds | +| `refactor_title` | `lsp.rs:100` | Generates display title (e.g., "Refactor Legacy Do Commands in workspace") | +| `selectable_document_refactor_levels` | `lsp.rs:110` | Returns available levels per file type | +| `build_refactor_command` | `lsp.rs:195` | Builds LSP Command struct with title + arguments | +| `build_document_refactor_edit` | `lsp.rs:211` | Creates WorkspaceEdit for single document | +| `collect_workspace_refactor_changes` | `lsp.rs:229` | Creates WorkspaceEdit for all workspace documents | +| `command_refactor_level_argument` | `lsp.rs:258` | Parses level from command arguments | + +## Core Refactoring Logic (`refactor.rs`) + +### Shared Infrastructure + +#### `update_tree_and_content` (`refactor.rs:10`) + +Performs an in-place replacement on both the content string and the tree-sitter tree (via `InputEdit`). All refactoring operations use this to maintain tree consistency during multi-step transformations. + +#### `build_old_statement_struct` (`refactor.rs:579`) + +Parses a legacy command node into an `OldStatement` struct: + +```rust +pub struct OldStatement { + pub last_expression_end_byte: Option, // end of condition/for-param + pub last_expression_end_point: Option, + pub statement_ranges: Vec>, // body statement byte ranges + pub keyword_old_range: tree_sitter::Range, // the legacy keyword node + pub command_range: tree_sitter::Range, // entire command node + pub comment_range: Option, // comment before statements + pub comment_after_last_statement_range: Option, + pub statements_after: Vec>, // do_statement_after nodes +} +``` + +This struct captures the structural components of legacy statements needed for transformation. + +#### `build_replacement_string_block` (`refactor.rs:787`) + +Formats statements into a block with curly braces: +``` + { + statement1 + statement2 +} +``` + +Uses 3-space indentation inside blocks relative to the base indent. + +#### `remove_unreachable_statements` (`refactor.rs:43`) + +Uses tree-sitter queries to find legacy statements with no body (no expression + no statement), then removes them entirely. Processes removals in reverse byte order to maintain valid positions. + +--- + +### 1. Legacy Do Refactoring + +**Public entry:** `refactor_legacy_do_statements` (`refactor.rs:1549`) + +**Algorithm:** + +``` +1. Parse content into tree-sitter tree +2. Collect existing routine member names (labels, tags, procedures) +3. Loop: + a. Query for (command_do (keyword_do_old)) nodes + b. Find the first one that actually has a dotted body + (is_old_do_with_dotted_body checks for lines with expected dot depth) + c. Find the enclosing subroutine/procedure name + d. Generate a unique subroutine name (e.g., MainSubroutine1) + e. Build the new subroutine body: + - Strip dot prefixes from each line + - Track brace depth for indentation + - If $TEST-modifying commands present: wrap with temp=$TEST / $TEST=temp + - Append quit if body doesn't end with quit/return + f. Build the replacement call: "do SubroutineName" + any do_statement_after nodes + g. Insert generated subroutine after the enclosing routine member + h. Replace the old do block with the new call + i. Re-parse tree and continue loop +4. After all do blocks are extracted: + - Normalize spacing for all subroutines (refactor_spacing_for_subroutines) +``` + +**Key functions:** + +| Function | Purpose | +|---|---| +| `direct_dotted_body_depth` | Determines the dot depth of lines following a `do` command | +| `dotted_body_line_ranges` | Collects byte ranges of all dotted body lines at the expected depth | +| `strip_dotted_prefix` | Removes leading dots from a line, returning clean content | +| `build_generated_dotted_do` | Builds the full subroutine text (name + body) | +| `build_new_do_call` | Builds the replacement `do SubroutineName` call with any after-statements | +| `generate_subroutine_name` | Creates a unique name by appending/incrementing `Subroutine{N}` | +| `find_do_statement_subroutine` | Walks up/backward to find the enclosing tag/procedure name and range | +| `changes_test_variable` | Checks if the do block contains `$TEST`/`JOB`/`LOCK`/`OPEN`/`READ` | +| `refactor_spacing_for_subroutines` | Normalizes indentation after extraction (4-space base indent) | +| `has_routine_member_between` | Determines where to insert the generated subroutine | + +**Processing order:** The algorithm processes dotted do statements from top to bottom (sorted by `start_byte`), but only processes one per loop iteration (re-parsing after each change). + +--- + +### 2. Conditional Refactoring + +**Public entry:** `refactor_conditionals` (`refactor.rs:850`) + +**Algorithm:** + +``` +1. Parse content with appropriate language (UDL for .cls, routine grammar for .mac/.int etc.) +2. Remove unreachable conditionals: + - Old if with no expression AND no statement → remove + - Old else with no statement → remove +3. Refactor if-else pairs (loop until no more changes): + - Query: source_file > statement(command_if(keyword_old_if)) followed by statement(command_else) + - For each pair: build block-form if/else +4. Refactor standalone old if (loop until no more changes): + - Query: command_if(keyword_old_if) + - Convert to block form +5. Refactor standalone old else (loop until no more changes): + - Query: command_else(keyword_oldelse) + - Convert to "if $TEST = 0 { ... }" block form +``` + +**Key functions:** + +| Function | Purpose | +|---|---| +| `remove_unreachable_conditionals` | First pass: removes dead if/else statements | +| `refactor_old_conditional_command` | Orchestrates all three conditional sub-passes | +| `refactor_if_else_statements` | Handles paired if+else → if/else block | +| `refactor_old_if_statements` | Handles standalone old if → if block | +| `refactor_old_else_statements` | Handles standalone old else → if $TEST = 0 block | + +**If-else transformation rules:** +- If the `if` has no expression: insert `$TEST` as condition +- If the `if` has expression but no body: negate expression, use else body as if body +- If both have bodies: create full `if (expr) { ... } else { ... }` block + +--- + +### 3. For Statement Refactoring + +**Public entry:** `refactor_for_statements` (`refactor.rs:822`) + +**Algorithm:** + +``` +1. Parse content with appropriate language +2. Remove unreachable for statements: + - Old for with no parameter AND no statement → remove +3. Refactor old for (loop until no more changes): + - Query: command_for(keyword_old_for) + - Convert to block form with curly braces +``` + +**Key functions:** + +| Function | Purpose | +|---|---| +| `remove_unreachable_for_statements` | First pass: removes dead for statements | +| `refactor_legacy_for_statements` | Loop converting old for → block form | +| `refactor_old_for_statements` | Single transformation of one for statement | + +--- + +## Workspace-Level Refactoring (`workspace.rs`) + +### `refactor_document` (`workspace.rs:171`) + +``` +1. Get document file type and content +2. Skip XML files (no refactoring supported) +3. Dispatch by RefactorLevel: + - DoCommands → refactor_legacy_do_statements (routine only) + - Conditionals → refactor_conditionals + - ForCommands → refactor_for_statements + - All → chain all three in sequence +4. If output == input → return None (no changes) +5. Return updated content +``` + +### `refactor` (`workspace.rs:219`) + +``` +1. Collect applicable document URLs: + - DoCommands: only routine files + - Others: all documents +2. For each URL: call refactor_document +3. Return Vec<(updated_content, url)> for all changed files +``` + +## Data Flow + +``` +User selects "Refactor All Code in workspace" + → execute_command receives (uri, "all") + → collect_workspace_refactor_changes calls project.refactor(RefactorLevel::All) + → for each document: + → refactor_legacy_do_statements (if routine) + → refactor_conditionals (on result) + → refactor_for_statements (on result) + → returns HashMap> + → workspace/applyEdit sent to editor + → editor applies all changes atomically +``` + +## Tree-Sitter Queries Used + +| Refactoring | Query Pattern | Purpose | +|---|---|---| +| Do commands | `(command_do (keyword_do_old)) @command` | Find legacy dotted do blocks | +| If (unreachable) | `(command_if (keyword_old_if) (expression)? @condition (statement)? @statement) @command_if` | Find removable if statements | +| Else (unreachable) | `(command_else (keyword_oldelse) (statement)? @statement) @command` | Find removable else statements | +| If-else pairs | `(source_file (statement (command_if (keyword_old_if)) @command_if) . (statement (command_else) @command_else))` | Find adjacent if/else to pair | +| Standalone if | `(command_if (keyword_old_if)) @command` | Find remaining old if | +| Standalone else | `(command_else (keyword_oldelse)) @command_else` | Find remaining old else | +| For (unreachable) | `(command_for (keyword_for) (for_parameter)? @param (statement)? @statement) @command` | Find removable for (new keyword) | +| For (old, unreachable) | `(command_for (keyword_old_for) (for_parameter)? @param (statement)? @statement) @command` | Find removable for (old keyword) | +| For (old) | `(command_for (keyword_old_for)) @command` | Find remaining old for to convert | + +## Processing Strategy + +All refactoring passes use the same loop-until-stable pattern: +1. Query for the first matching node +2. Transform it (in-place string replacement + tree edit) +3. Re-parse the tree +4. Repeat until no matches found + +This one-at-a-time approach ensures each transformation works on a valid tree, since byte offsets shift after each replacement. diff --git a/objectscript-lsp/documentation/features/refactor.md b/objectscript-lsp/documentation/features/refactor.md new file mode 100644 index 0000000..d28a8d7 --- /dev/null +++ b/objectscript-lsp/documentation/features/refactor.md @@ -0,0 +1,126 @@ +# Refactor Feature (Code Actions -> Refactor) + +## Refactoring Types + +### 1. Legacy Do Commands (Dotted Do → Subroutines) + +Converts legacy dotted `Do` blocks into named private subroutines with explicit `Do subroutineName` calls. + +**Before:** +```objectscript +Main + do + . set x = 1 + . write x + write "done" + q +``` + +**After:** +```objectscript +Main + do MainSubroutine1 + write "done" + q + +MainSubroutine1 Private + set x = 1 + write x + quit +``` + +**Details:** +- The generated subroutine name is derived from the enclosing label/procedure name + `Subroutine` + depth number (e.g., `MainSubroutine1`) +- If the name conflicts with an existing routine member, the depth number is incremented +- If the do block contains `$TEST`, `JOB`, `LOCK`, `OPEN`, or `READ` (commands that modify `$TEST`), the generated subroutine saves and restores `$TEST` via a `temp` variable +- A trailing `quit` is appended unless the body already ends with `quit` or `return` +- Comments are preserved in the generated call and subroutine +- After extraction, subroutine spacing/indentation is normalized +- Only available for routine files (`.mac`, `.int`, `.rtn`, `.inc`) + +--- + +### 2. Legacy If/Else Commands (Single-line → Block Form) + +Converts legacy single-line `If`/`Else` conditionals into modern block-form `if`/`else` with curly braces. + +**Before:** +```objectscript + If x=1 Write "yes" + Else Write "no" +``` + +**After:** +```objectscript + if x=1 { + Write "yes" + } + else { + Write "no" + } +``` + +**Rules:** +- If the `If` has no expression and no statements (unreachable), the entire `If`/`Else` pair is removed +- Argumentless `If` (no expression) with statements → becomes `if $TEST` +- If the `If` has an expression but no statements, and there's an `Else` with statements → transforms into `if '(expression)` (negated condition) with the else body +- Adjacent `If`/`Else` pairs are refactored together into `if/else` blocks +- Standalone old `Else` without a paired `If` → becomes `if $TEST = 0` +- Comments between keywords and statements are preserved +- Available for both class files and routine files + +--- + +### 3. Legacy For Commands (Single-line → Block Form) + +Converts legacy single-line `For` loops into modern block-form with curly braces. + +**Before:** +```objectscript + For i=1:1:10 Write i +``` + +**After:** +```objectscript + for i=1:1:10 { + Write i + } +``` + +**Rules:** +- If the `For` has no parameters and no statements (unreachable), it is removed +- The for parameter (loop variable/range) is preserved as-is +- Comments are preserved +- Available for both class files and routine files + +--- + +## Scope Options + +Each refactoring type can be applied at two scopes: + +| Scope | Behavior | +|---|---| +| **Document** | Refactors only the current file | +| **Workspace** | Refactors all applicable files in the workspace | + +## Available Refactoring Levels per File Type + +| File Type | Available Levels | +|---|---| +| Routine (`.mac`, `.int`, `.rtn`, `.inc`) | Do Commands, Conditionals, For Commands, All | +| Class (`.cls`) | Conditionals, For Commands, All | +| XML (`.xml`) | None | + +The "All" level applies all applicable transformations in sequence: Do Commands (if routine) → Conditionals → For Commands. + +## Preconditions + +- **Document must parse without errors** — document-level refactors are only offered when the tree-sitter parse has no error nodes +- **Workspace refactors are manually triggered only** — they don't appear in automatic code action suggestions (only on explicit user request) + +## Behavior + +- If the refactoring produces no changes (code is already in modern form), the code action is a no-op +- The refactoring replaces the entire document content with the updated version via a `workspace/applyEdit` request +- If the editor rejects the edit, a warning message is logged diff --git a/objectscript-lsp/src/lsp.rs b/objectscript-lsp/src/lsp.rs index fd2cea8..4e8e566 100644 --- a/objectscript-lsp/src/lsp.rs +++ b/objectscript-lsp/src/lsp.rs @@ -1361,7 +1361,6 @@ impl LanguageServer for BackendWrapper { } async fn shutdown(&self) -> Result<()> { - // need to look more into if this is good for doing nothing Ok(()) } @@ -1577,8 +1576,6 @@ impl LanguageServer for BackendWrapper { project.update_document(uri, new_tree, file_type, new_version, old_text.as_str()); } } - - // async fn did_close(&self, params: DidCloseTextDocumentParams) {} } #[cfg(test)]