[DRAFT] feat(code-action): add refactoring and quickfix actions for Nix code#755
Draft
[DRAFT] feat(code-action): add refactoring and quickfix actions for Nix code#755
Conversation
- Add code actions for quoting/unquoting attribute names, flattening and packing attribute sets, and converting JSON to Nix - Implement quickfixes for removing redundant keywords and unused bindings - Provide extract-to-file and with-to-let/inherit refactoring actions - Update diagnostics and tests to cover new code actions
- Add code actions to convert self-assignments and select expressions to inherit statements, and vice versa, for single static attribute names - Implement string rewrite actions to convert between double-quoted and indented Nix strings - Introduce helper functions for percent-encoding, workspace edits, and string escaping for Nix syntax - Expand supported codeActionKinds in server capabilities and protocol - Update tests to cover new code actions and kinds
- Swap order of <lspserver/SourceCode.h> and <llvm/Support/JSON.h> includes in CodeAction.cpp to match expected dependencies - No functional changes to Protocol.cpp
inclyc
approved these changes
Jan 3, 2026
Member
inclyc
left a comment
inclyc
left a comment
There was a problem hiding this comment.
I think this is a great direction, and the code quality is impressive. I'll stay on top of reviewing future PRs—let’s push these features forward!
|
|
||
| /// \brief Add "Open in noogle.dev" action for lib function references. | ||
| /// Example: lib.mapAttrs -> opens https://noogle.dev/f/lib/mapAttrs | ||
| void addNoogleAction(const nixf::Node &N, const nixf::ParentMapAnalysis &PM, |
Member
There was a problem hiding this comment.
This is impressive! Let's CC noogle's main dev @hsjobeki.
I'd love to get your thoughts on this, @hsjobeki! We’re working on an 'Open in noogle.dev' feature for nixd that maps library functions directly to Noogle search results. Since you’re the expert here, your feedback on the integration—and any unique insights you have on how we can make this more seamless for users—would be invaluable.
|
|
||
| /// \brief Add flatten code action for nested attribute sets. | ||
| /// Example: { foo = { bar = 1; }; } -> { foo.bar = 1; } | ||
| void addFlattenAction(const nixf::Node &N, const nixf::ParentMapAnalysis &PM, |
Member
There was a problem hiding this comment.
I’ve tested this feature manually, and I think it would be better to collect all nested attributes during the packing process. For example, converting this:
{
x.a = 1;
x.b = 2;
}into this:
{
x = { a = 1; b = 2; };
}What do you think about handling it this way?
| } | ||
|
|
||
| /// \brief Add refactoring code actions for attribute names (quote/unquote). | ||
| void addAttrNameActions(const nixf::Node &N, const nixf::ParentMapAnalysis &PM, |
| /// 1. Prompt the user for a file path | ||
| /// 2. Create the new file with the expression | ||
| /// 3. Replace the selection with an import statement | ||
| void addExtractToFileAction(const nixf::Node &N, const URIForFile &URI, |
Member
There was a problem hiding this comment.
It looks like this change might require corresponding updates on the LSP client side as well?
|
|
||
| /// \brief Add "Convert with to let/inherit" action for with expressions. | ||
| /// Example: with lib; { x = foo; } -> let inherit (lib) foo; in { x = foo; } | ||
| void addWithToLetAction(const nixf::Node &N, const nixf::ParentMapAnalysis &PM, |
| for (std::size_t i = DiagsBefore; i < Diags.size(); ++i) { | ||
| auto &D = Diags[i]; | ||
|
|
||
| // Only process unused formal diagnostics |
Collaborator
Author
|
@inclyc Claude Codeに適当に実装させてみて網羅的に実装出来ることが確認取れました。 |
Member
@takeokunn Thank you very much for the explanation. I believe that leveraging AI to enhance efficiency is a necessary and acceptable way to boost productivity. As long as code quality is ensured through human testing and review, I have full confidence in your experience to move this project forward! @takeokunn 説明ありがとうございます。AIを活用して効率を上げることは、現代において必要かつ許容されるべき生産性の向上手段だと考えています。コードの品質に関しては、実機テストやレビューを通じた「人の目」で担保できますし、あなたの経験があればこのプロジェクトを安心してお任せできると確信しています！ |
3 tasks
takeokunn
added a commit
that referenced
this pull request
Jan 4, 2026
…756) ## Summary Add refactoring code actions to quote/unquote attribute names in attribute sets. - `{ foo = 1; }` → Offer "Quote attribute name" → `{ "foo" = 1; }` - `{ "bar" = 1; }` → Offer "Unquote attribute name" → `{ bar = 1; }` This is part of the code actions initiative (#466, #755). ## Changes | File | Description | |-----------------------------------------------|----------------------------------------| | `nixd/lib/Controller/CodeAction.cpp` | Add quote/unquote refactoring logic | | `nixd/lib/Controller/LifeTime.cpp` | Register `refactor.rewrite` capability | | `nixd/lspserver/include/lspserver/Protocol.h` | Add `REFACTOR_REWRITE_KIND` constant | | `nixd/lspserver/src/Protocol.cpp` | Define `REFACTOR_REWRITE_KIND` | | `nixd/tools/nixd/test/code-action/*.md` | 11 test files for various cases | | `nixd/tools/nixd/test/initialize.md` | Update capability expectations | ## Behavior ### Quote action offered when: - Cursor is on an unquoted attribute name (`foo`) - Parent is an attribute set (not a let binding) ### Unquote action offered when: - Cursor is on a quoted static attribute name (`"foo"`) - The string is a valid Nix identifier (starts with letter/underscore, contains only valid chars) - The string is not a Nix keyword (if, then, else, etc.) ### Explicitly excluded: - Let bindings (`let foo = 1; in foo`) - no actions offered - Invalid identifiers (`"123"`, `"foo.bar"`) - no unquote action - Keywords (`"true"`, `"if"`) - no unquote action ## Test Plan - [x] All existing tests pass - [x] New lit tests for each code action scenario (11 test files) - [x] Manual testing in editor <img width="1812" height="1240" alt="CleanShot 2026-01-04 at 14 39 08@2x" src="https://github.com/user-attachments/assets/72d3056b-35b7-4ba9-b32d-dff9d3888bc6" /> <img width="1808" height="1238" alt="CleanShot 2026-01-04 at 14 38 43@2x" src="https://github.com/user-attachments/assets/5a322b46-458e-4cc1-b898-83bcbdc912d9" /> ## Related - Issue: #466 - Draft PR: #755 (this implements Split 1)
takeokunn
added a commit
that referenced
this pull request
Jan 4, 2026
…757) ## Summary Adds a new "Flatten nested attribute set" code action that transforms nested attribute sets into dot notation: ```nix # Before { foo = { bar = 1; }; } # After { foo.bar = 1; } ``` This follows the existing pattern established in #756 for code actions. ## Changes | File | Description | |-----------------------------------------------------------|------------------------------------------------------------------------------------------| | nixd/lib/Controller/CodeAction.cpp | Add getFlattenableBinds() helper and addFlattenAttrsAction() for the flatten refactoring | | nixd/tools/nixd/test/code-action/flatten-attrs.md | Basic test for simple nested attribute flattening | | nixd/tools/nixd/test/code-action/flatten-attrs-multi.md | Test for nested sets with multiple bindings | | nixd/tools/nixd/test/code-action/flatten-attrs-deep.md | Test for deeply nested attribute paths | | nixd/tools/nixd/test/code-action/flatten-attrs-rec.md | Test that rec sets are excluded | | nixd/tools/nixd/test/code-action/flatten-attrs-inherit.md | Test that sets with inherit are excluded | | nixd/tools/nixd/test/code-action/flatten-attrs-dynamic.md | Test that dynamic ${} names are excluded | | nixd/tools/nixd/test/code-action/flatten-attrs-empty.md | Test that empty nested sets are excluded | ## Behavior Flatten action offered when: - Cursor is on a binding whose value is an attribute set - All attribute names (outer and inner) are static identifiers - The nested set contains at least one binding ## Explicitly excluded: - Recursive attribute sets (rec { ... }): Flattening would change semantics - Inherited bindings (inherit x;): Cannot be represented in dot notation - Dynamic attribute names (${expr}): Cannot be statically flattened - Empty nested sets ({ foo = {}; }): No meaningful transformation ## Test Plan - Added 7 lit tests covering positive and negative cases - Tests verify the action is offered only when appropriate - Tests verify correct transformation output for various input shapes - All existing tests pass <img width="954" height="424" alt="CleanShot 2026-01-04 at 20 12 35@2x" src="https://github.com/user-attachments/assets/e0ccbeb2-705a-44dd-8e76-0eaa06f65b48" /> <img width="846" height="412" alt="CleanShot 2026-01-04 at 20 14 09@2x" src="https://github.com/user-attachments/assets/0ce5109c-e640-4c46-b642-3225ae574561" /> ## Related - Issue: #466 - Draft PR: #755 (this implements Split 2)
takeokunn
added a commit
that referenced
this pull request
Jan 5, 2026
…#758) ## Summary Add a new code action that converts dotted attribute paths to nested attribute sets - the inverse of the "Flatten nested attribute set" action from #757. **Single Pack:** ```nix { foo.bar = 1; } → { foo = { bar = 1; }; } ``` **Bulk Pack** (multiple sibling bindings with same prefix): ```nix { foo.bar = 1; foo.baz = 2; } → { foo = { bar = 1; baz = 2; }; } ``` ## Changes | File | Description | |------|-------------| | nixd/lib/Controller/CodeAction.cpp | Add packAttrs function implementing the pack transformation with single and bulk modes | | **Single Pack Tests** | | | pack-attrs.md | Basic test for dotted path packing | | pack-attrs-deep.md | Test for multi-level dotted paths (packs one level at a time) | | pack-attrs-quoted.md | Test for preserving quoted attribute names | | pack-attrs-multiline.md | Test for multiline attribute set formatting | | pack-attrs-values.md | Test for various value types (empty sets, let expressions, nested sets) | | **Bulk Pack Tests** | | | pack-attrs-bulk.md | Test for bulk pack with sibling bindings | | pack-attrs-bulk-quoted.md | Test for bulk pack with quoted keys requiring proper quoting | | pack-attrs-escape.md | Test for escaping special characters (", \\, $) in quoted keys | | pack-attrs-sibling-inherit.md | Test for pack with inherit sibling (non-conflicting) | | pack-attrs-sibling-different-prefix.md | Test for pack with different prefix siblings | | **Negative Tests** | | | pack-attrs-single.md | Negative test for single-segment paths | | pack-attrs-rec.md | Negative test for recursive attribute sets | | pack-attrs-dynamic.md | Negative test for dynamic interpolation in paths | | pack-attrs-conflict.md | Negative test for non-path binding conflicts (e.g., `{ foo = 1; foo.bar = 2; }`) | | pack-attrs-sibling-dynamic.md | Negative test for dynamic attributes in sibling bindings | | flatten-attrs-deep.md | Updated to verify both Pack and Flatten actions offered together | ## Behavior ### Single Pack Action ("Pack dotted path to nested set") Action is offered when: - Cursor is on a dotted attribute path with 2+ segments (e.g., `foo.bar`) - The attribute is inside a non-recursive attribute set - No sibling bindings share the same first segment prefix ### Bulk Pack Action ("Pack all 'X' bindings to nested set") Action is offered when: - Multiple sibling bindings share the same first segment prefix (e.g., `foo.bar` and `foo.baz`) - All sibling binding paths are static (no dynamic interpolation) - No conflicting non-path binding exists for the prefix Transformation combines all matching siblings: ```nix { foo.bar = 1; foo.baz = 2; foo.qux = 3; } → { foo = { bar = 1; baz = 2; qux = 3; }; } ``` ### Explicitly Excluded - **Single-segment paths** (e.g., `foo = 1`): Nothing to pack - **Recursive attribute sets** (`rec { ... }`): Packing would change semantics - **Dynamic attribute paths** (e.g., `foo.${bar}`): Cannot safely transform interpolated paths - **Conflicting non-path bindings** (e.g., `{ foo = 1; foo.bar = 2; }`): Would create invalid Nix code - **Dynamic sibling attributes**: When any sibling has dynamic interpolation, bulk pack is disabled ### Deep Path Handling Deep paths are packed one level at a time: - `a.b.c = 1` → `a = { b.c = 1; }` (first application) - `a = { b.c = 1; }` → `a = { b = { c = 1; }; }` (second application) ### Key Quoting and Escaping The action correctly handles: - **Quoted keys**: Keys requiring quotes (e.g., `"1foo"`) are properly quoted in output - **Special character escaping**: Characters like `"`, `\`, and `$` are correctly escaped ## Test Plan - ✅ **All CI checks passing** - **16 test files** covering positive and negative cases - **Positive tests** verify correct transformation output for: - Basic single pack - Deep paths (multi-level) - Quoted attribute names - Multiline formatting - Various value types (empty sets, let expressions, nested sets) - Bulk pack with multiple siblings - Bulk pack with special characters in keys - Non-conflicting sibling scenarios (inherit, different prefixes) - **Negative tests** verify action is NOT offered in excluded scenarios: - Single-segment paths - Recursive attribute sets - Dynamic interpolation - Conflicting non-path bindings - Dynamic sibling attributes <img width="876" height="404" alt="CleanShot 2026-01-05 at 22 59 36@2x" src="https://github.com/user-attachments/assets/b32ba748-9ff6-4d40-95d1-8a51e7e82bd3" /> <img width="984" height="412" alt="CleanShot 2026-01-06 at 03 05 10@2x" src="https://github.com/user-attachments/assets/bb69bdc7-9d1f-40b1-8b21-a3ad125c91f2" /> ## Related - Part of #466 - Inverse of #757 (flatten nested attribute set) - Draft PR: #755 (this implements Split 3)
3 tasks
inclyc
pushed a commit
that referenced
this pull request
Jan 8, 2026
…763) ## Summary Add a new code action that converts selected JSON text to Nix expression syntax. This is a selection-based action that works on arbitrary text within a Nix file, allowing users to quickly convert JSON data structures to their Nix equivalents. **Example:** ```json {"foo": 1, "bar": true, "nested": {"key": "value"}} → { foo = 1; bar = true; nested = { key = "value"; }; } ``` ## Changes | File | Description | |------------------------------------|------------------------------------------------------------------------| | nixd/lib/Controller/CodeAction.cpp | Add escapeNixString(), jsonToNix(), and addJsonToNixAction() functions | | Positive Tests | | | json-to-nix.md | Basic test for simple JSON object conversion | | json-to-nix-array.md | Test for JSON arrays | | json-to-nix-nested.md | Test for nested structures with proper indentation | | json-to-nix-numeric.md | Test for numeric types (integer and float) | | json-to-nix-types.md | Test for all JSON types (null, boolean, string, number) | | json-to-nix-escaping.md | Test for escape sequences in strings | | json-to-nix-interpolation.md | Test for ${ interpolation escaping | | json-to-nix-special-keys.md | Test for keys requiring quoting | | Negative Tests | | | json-to-nix-invalid.md | Test that invalid JSON does NOT offer the action | | json-to-nix-empty.md | Test that empty objects {} do NOT offer the action | | json-to-nix-empty-array.md | Test that empty arrays [] do NOT offer the action | | Edge Case Tests | | | json-to-nix-depth-limit.md | Test safety limit for deeply nested structures | | json-to-nix-width-limit.md | Test safety limit for very wide arrays/objects | ## Behavior Action offered when: - User has selected text in a Nix file - Selected text starts with { or [ - Selected text is valid JSON - JSON is non-empty (has at least one element) Explicitly excluded: - Invalid JSON: No action offered - Empty structures ({}, []): Already valid Nix, no transformation needed - Text not starting with { or [: Quick rejection for performance Safety limits: - Depth limit: 100 levels of nesting (prevents stack overflow) - Width limit: 10,000 elements per array/object (prevents memory issues) Escape handling: - " → \" - \ → \\ - ${ → \${ (prevent Nix interpolation) - \n, \r, \t → preserved escape sequences ## Test Plan - [x] 13 regression tests covering positive, negative, and edge cases - [x] All existing tests pass - [x] Manual testing in editor <img width="1160" height="1022" alt="CleanShot 2026-01-08 at 02 23 20@2x" src="https://github.com/user-attachments/assets/7e2005df-7c47-4bad-8d4c-c087a97c0e0a" /> ## Related - Issue: #466 - Draft PR: #755 (this implements Split 4)
4 tasks
inclyc
pushed a commit
that referenced
this pull request
Jan 9, 2026
…nctions (#764) ## Summary Add a new code action to open noogle.dev documentation for `lib.*` function paths directly from the editor. When the cursor is on a `lib.X` or `lib.X.Y.Z` expression, the action opens the corresponding noogle.dev documentation page in the user's browser via `window/showDocument`. **Example:** - `lib.optionalString` → Opens `https://noogle.dev/f/lib/optionalString` - `lib.strings.optionalString` → Opens `https://noogle.dev/f/lib/strings/optionalString` ## Changes | File | Description | |------|-------------| | nixd/include/nixd/Controller/Controller.h | Add `onCodeActionResolve` handler and `ShowDocument` callback | | nixd/lib/Controller/CodeAction.cpp | Add `buildNoogleUrl()` and `addNoogleDocAction()` functions | | nixd/lib/Controller/Support.cpp | Register `codeAction/resolve` and `window/showDocument` methods | | nixd/lspserver/include/lspserver/Protocol.h | Add `CodeAction.data`, `ShowDocumentParams`, `ShowDocumentResult` | | nixd/lspserver/src/Protocol.cpp | JSON serialization for new protocol types | ## Behavior Action offered when: - Cursor is on an `ExprSelect` with base variable `lib` - Path contains at least one static attribute name Action NOT offered when: - Just `lib` variable without selection - Dynamic attributes (e.g., `lib.${x}`) - Non-lib bases (e.g., `pkgs.hello`) ## Test Plan - [x] `noogle-lib-simple.md` - Basic `lib.X` path - [x] `noogle-lib-nested.md` - Nested `lib.X.Y` path - [x] `noogle-lib-var-only.md` - Negative: just `lib` variable - [x] `noogle-not-lib.md` - Negative: non-lib select <img width="1510" height="1046" alt="CleanShot 2026-01-08 at 22 15 19@2x" src="https://github.com/user-attachments/assets/ad9f9f9a-6c99-4969-8f77-6cef7d1a3852" /> ## Related - Issue: #466 - Draft PR: #755 (this implements Split 5)
inclyc
pushed a commit
that referenced
this pull request
Jan 11, 2026
…767) ## Summary This PR adds a new "Extract to File" code action that extracts Nix expressions to separate files, automatically generating appropriate import statements with free variable handling. **Example transformation:** Before: ```nix { pkgs }: { buildInputs = [ pkgs.foo pkgs.bar ]; } ``` After extraction: ``` - New file (buildInputs.nix): { pkgs }: [ pkgs.foo pkgs.bar ] - Original file: { pkgs }: { buildInputs = import ./buildInputs.nix { inherit pkgs; }; } ``` ## Changes | File | Description | |---------------------------------------------------|---------------------------------------------------------------------------------| | nixd/lib/Controller/CodeActions/ExtractToFile.cpp | Core implementation with free variable detection and unique filename generation | | nixd/lib/Controller/CodeActions/ExtractToFile.h | Public interface declaration | | nixd/lib/Controller/CodeAction.cpp | Integration with code action dispatcher | | nixd/lib/meson.build | Build system integration | | nixd/lspserver/include/lspserver/Protocol.h | LSP resource operations (CreateFile, RenameFile, DeleteFile) | | nixd/lspserver/src/Protocol.cpp | JSON serialization for resource operations | ## Behavior The action is offered when: - Cursor is on a non-trivial expression (attribute sets, lists, lambdas, let bindings, if expressions, etc.) - Cursor is on an attribute name (extracts the binding's value expression) Free Variable Detection: - Automatically detects variables used but defined outside the expression - Wraps extracted code in a lambda with free variables as arguments - Generates import ./file.nix { inherit var1 var2; } syntax - Warns about with scope variables in action title (these may require manual adjustment) Filename Generation: - Uses binding key name if inside a binding (e.g., buildInputs.nix) - Falls back to expression type (e.g., extracted-lambda.nix, extracted-attrs.nix) - Automatically generates unique filenames (name-1.nix, name-2.nix) if file exists ## Explicitly Excluded - Trivial expressions: single variables, literals, paths, select expressions - Empty attribute sets {} - Empty lists [] ## Test Plan - CI passes - 4 regression tests covering: - Basic attribute set extraction (extract-attrs.md) - List expression extraction (extract-list.md) - Free variable detection (extract-with-free-vars.md) - with scope variable warning (extract-with-scope-vars.md) ## Related - Issue: #466 - Draft PR: #755 (this implements Split 6)
This was referenced Jan 11, 2026
inclyc
pushed a commit
that referenced
this pull request
Jan 13, 2026
## Summary Add a new code action that converts `with` expressions to explicit `let/inherit` syntax. **Example transformation:** ```nix # Before with lib; optionalString true "yes" # After let inherit (lib) optionalString; in optionalString true "yes" ``` ## Features - Only offered when cursor is on the with keyword - Automatically collects variables used from the with scope - Variables are sorted alphabetically in the inherit list - Handles nested with expressions correctly - Skips unused with (handled by existing "remove with" quickfix) ## Test Plan - Added 10 regression tests covering: - Basic conversion - Multiple variables with alphabetic sorting - Complex source expressions (e.g., pkgs.lib) - Nested with expressions - Variable shadowing - Cursor position validation - Unused with handling ## Related - Issue: #466 - Draft PR: #755 (this implements Split 7)
inclyc
pushed a commit
that referenced
this pull request
Jan 13, 2026
…770) ## Summary Add a new code action that converts explicit bindings to `inherit` syntax. **Example transformations:** ```nix # Pattern 1: Simple variable assignment # Before { foo = foo; bar = bar; } # After { inherit foo; inherit bar; } # Pattern 2: Select from source # Before { x = lib.x; y = lib.y; } # After { inherit (lib) x; inherit (lib) y; } # Pattern 3: Nested source # Before { x = lib.nested.x; } # After { inherit (lib.nested) x; } ``` ## Features - Only offered when cursor is on a binding node - Supports simple { x = x; } pattern - Supports { a = source.a; } pattern with arbitrary source depth - Properly handles quoted attribute names (e.g., "foo-bar") - Works in recursive attribute sets (inherit takes from lexical scope) ## Edge Cases (action NOT offered) - Multi-segment attribute paths: { foo.bar = baz; } - Name mismatch: { foo = bar; } - Selector mismatch: { foo = lib.bar; } - Select with default value: { foo = lib.foo or null; } - Dynamic/interpolated attribute names ## Test Plan - Added 9 regression tests covering: - Basic { x = x; } conversion - { a = lib.a; } conversion with source - Nested source { x = lib.nested.x; } - Quoted attribute names - Recursive attribute sets - Name mismatch (negative) - Selector mismatch (negative) - Multi-segment path (negative) - Default value (negative) ## Related - Issue: #466 - Draft PR: #755 (this implements Split 8)
inclyc
pushed a commit
that referenced
this pull request
Jan 13, 2026
…773) ## Summary This PR adds a new "Convert to explicit binding" code action that converts inherit statements to explicit bindings: - Simple inherit: `{ inherit x; }` → `{ x = x; }` - Inherit from expression: `{ inherit (b) a; }` → `{ a = b.a; }` | Before | After | |------------------------------|--------------------------| | `{ inherit x; }` | `{ x = x; }` | | `{ inherit (b) a; }` | `{ a = b.a; }` | | `{ inherit (foo.bar) baz; }` | `{ baz = foo.bar.baz; }` | ## Changes | File | Description | |--------------------------------------------------------|------------------------------| | `nixd/lib/Controller/CodeActions/InheritToBinding.h` | Public interface declaration | | `nixd/lib/Controller/CodeActions/InheritToBinding.cpp` | Core implementation | | `nixd/lib/Controller/CodeAction.cpp` | Integration with dispatcher | | `nixd/lib/meson.build` | Build system integration | ## Behavior The action is offered when: - Cursor is on an `inherit` statement - The inherit has exactly **one** name (single-name only) Limitations: - Multi-name inherits (`inherit x y z;`) not supported - Only static (non-interpolated) attribute names handled ## Test Plan - [ ] CI passes - 5 regression tests: - `basic.md`: Simple inherit (`inherit x` → `x = x`) - `with-expr.md`: Inherit from variable (`inherit (b) a` → `a = b.a`) - `complex-expr.md`: Nested expression (`inherit (foo.bar) baz`) - `long-name.md`: Long attribute names - `multi-name-negative.md`: Multi-name inherits do NOT offer action ## Related - Issue: #466 - Draft PR: #755 (this implements Split 9)
2 tasks
inclyc
pushed a commit
that referenced
this pull request
Jan 14, 2026
## Summary
Add code action to convert between double-quoted strings (`"..."`) and
indented strings (`''...''`).
**Example Transformation:**
| Before | After |
|---------------------|-------------------|
| `"hello\nworld"` | `''hello↵world''` |
| `''hello ${name}''` | `"hello ${name}"` |
## Changes
| File | Description |
|-----------------------------------------------------|-------------------------------------|
| `nixd/lib/Controller/CodeActions/RewriteString.h` | Header file |
| `nixd/lib/Controller/CodeActions/RewriteString.cpp` | Implementation
with escape handling |
| `nixd/lib/Controller/CodeAction.cpp` | Register the action |
| `nixd/lib/meson.build` | Add source to build |
## Behavior
**Action offered when:**
- Cursor is on a string literal (double-quoted or indented)
**Action NOT offered when:**
- Cursor is on non-string nodes (integers, paths, booleans, etc.)
**Handles:**
- Escape sequence conversion (`\n`, `\t`, `\\`, `\"`, `\${` ↔ literal
characters)
- Quote escaping (`"` in content → `\"` when converting to
double-quoted)
- Interpolation preservation (`${...}` maintained in both formats)
- Empty strings
## Test Plan
- [x] All 11 rewrite-string tests pass
- [x] Build with sanitizers succeeds
- Tests cover:
- `to-indented.md` / `to-dquote.md`: Basic conversion
- `empty-string.md`: Empty string handling
- `escape-*.md`: Escape sequences (newline, tab, backslash, quotes)
- `interpolation.md` / `to-dquote-interpolation.md`: Interpolation
preservation
- `not-string.md`: **Negative test** - action not offered on non-string
## Related
- Issue: #466
- Draft PR: #755 (this implements Split 10)
2 tasks
takeokunn
added a commit
that referenced
this pull request
Jan 28, 2026
#777) ## Summary Add code action to add undefined variables to the formals of enclosing lambda expressions. **Example Transformation:** | Before | After | |-----------------|-------------------| | `{x}: y` | `{x, y}: y` | | `{ }: y` | `{ y }: y` | | `{ ... }: y` | `{ y, ... }: y` | | `{x, ...}: y` | `{x, y, ...}: y` | ## Changes | File | Description | |---------------------------------------------------|-----------------------------------------------| | `nixd/lib/Controller/CodeActions/AddToFormals.h` | Header file for add-to-formals action | | `nixd/lib/Controller/CodeActions/AddToFormals.cpp`| Implementation with 4 insertion point cases | | `nixd/lib/Controller/CodeAction.cpp` | Register the action (requires VLA) | | `nixd/lib/meson.build` | Add source to build | ## Behavior **Action offered when:** - Cursor is on an undefined variable (ExprVar node) - Enclosing lambda uses formals-style syntax `{...}: body` **Action NOT offered when:** - Variable is already defined (in any scope) - Lambda uses simple argument style `x: body` - No enclosing lambda exists **Handles 4 insertion cases:** 1. Empty formals `{ }:` → `{ y }:` 2. Normal `{ a }:` → `{ a, y }:` 3. Ellipsis only `{ ... }:` → `{ y, ... }:` 4. With ellipsis `{ a, ... }:` → `{ a, y, ... }:` ## Test Plan - [x] All 8 add-to-formals tests pass - [x] Build succeeds - Tests cover: - `basic.md`: Single existing formal - `empty-formals.md`: Empty formals `{ }` - `ellipsis-only.md`: Ellipsis-only `{ ... }` - `multiple-formals.md`: Multiple existing formals - `with-ellipsis.md`: Formals with ellipsis - `nested-lambda.md`: Nested lambda (adds to innermost) - `already-defined-negative.md`: **Negative** - variable already in formals - `no-formals-negative.md`: **Negative** - simple `x: body` syntax ## Related - Issue: #466 - Draft PR: #755 (this implements Split 11)
This was referenced Jan 28, 2026
takeokunn
added a commit
that referenced
this pull request
Feb 7, 2026
## Summary Add code action to remove unused bindings from let expressions. **Example Transformation:** | Before | After | |--------------------------|-----------------------------------------------| | `let x = 1; in 2` | `let in 2` | | `let x = 1; y = 2; in y` | `let y = 2; in y` | | `let x = 1; y = 2; in 3` | `let y = 2; in 3` (each gets separate action) | ## Changes | File | Description | |---------------------------------------|-----------------------------------------------------| | `libnixf/src/Sema/VariableLookup.cpp` | Generate fix for `DK_UnusedDefLet` diagnostics | | `nixd/lib/Controller/CodeAction.cpp` | Set `isPreferred: true` for unused binding quickfix | ## Behavior **Action offered when:** - Cursor is on an unused let binding - Diagnostic `DK_UnusedDefLet` exists **Action NOT offered when:** - Binding is used in the let body - Binding is an `inherit` statement (intentionally excluded) **Handles removal:** - Removes full binding range including `= value;` - Works with single-line and multiline let expressions ## Test Plan - [x] All 4 unused-def tests pass - [x] Build succeeds with `-Dwerror=true` - Tests cover: - `basic.md`: Remove single unused binding - `multiple-bindings.md`: Remove one of multiple bindings - `all-unused.md`: All bindings unused (each gets separate action) - `multiline.md`: Multiline let expression formatting - `unquote-let-invalid.md`: Interaction with unquote action (updated) - `unquote-let-keyword.md`: Interaction with unquote action (updated) ## Related - Issue: #466 - Draft PR: #755 (this implements Split 13)
This was referenced Feb 14, 2026
takeokunn
added a commit
that referenced
this pull request
Feb 14, 2026
Mark DK_EmptyInherit diagnostic as isPreferred in the code action handler so that editors surface the existing "remove `inherit` keyword" fix as a preferred quickfix. Closes part of #466.
This was referenced Feb 14, 2026
Contributor
|
hey @takeokunn, it seems this PR is getting delayed by trying to ship every code action at the same time Do you think it would be possible to split up this PR into multiple smaller PRs so the refactoring and the code actions that are already able to be tested and shipped could be merged without having to synchronously wait for the whole batch? |
Member
Following up on this—we’ve accepted a batch of smaller PRs including #768 and #770. The full breakdown is now updated in the description of this PR. Also some of them has been released in 2.9.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements comprehensive code actions for nixd, addressing #466 and incorporating community suggestions from the discussion thread.
Note: This is a draft PR. Features will be split into separate PRs for incremental review and merge.
Features Implemented
Quick Fixes (diagnostic-based)
{x}: y→{x, y}: y{x, y}: x→{x}: xlet unused = 1; in 2→2Refactoring Actions
foo↔"foo"{ foo = { bar = 1; }; }→{ foo.bar = 1; }{ foo.bar = 1; }→{ foo = { bar = 1; }; }lib.*function documentationwithtolet/inherit-with lib; { x = foo; }→let inherit (lib) foo; in { x = foo; }{ x = x; }→{ inherit x; }/{ a = b.a; }→{ inherit (b) a; }{ inherit x; }→{ x = x; }"hello"↔''hello''LSP Enhancements
refactor.extract,refactor.rewrite,source.fixAllChanges
libnixf/src/Sema/VariableLookup.cppnixd/lib/Controller/CodeAction.cppnixd/lspserver/include/lspserver/Protocol.hnixd/lspserver/src/Protocol.cppnixd/lib/Controller/LifeTime.cppnixd/tools/nixd/test/code-action/*.mdPlanned Split
Each PR is self-contained and independently reviewable.
CodeAction.cpp,LifeTime.cppquote-attr.md,unquote-attr.mdfoo↔"foo"in attribute namesCodeAction.cppflatten-attrs.md{ foo = { bar = 1; }; }→{ foo.bar = 1; }CodeAction.cpppack-attrs.md{ foo.bar = 1; }→{ foo = { bar = 1; }; }CodeAction.cppjson-to-nix.mdCodeAction.cppnoogle-lib.mdlib.*function docs with URL encodingCodeAction.cppextract-to-file.mdCodeAction.cppwith-to-let.mdwith lib; expr→let inherit (lib) ...; in exprCodeAction.cppconvert-to-inherit.md{ x = x; }→{ inherit x; }CodeAction.cppinherit-to-binding.md{ inherit x; }→{ x = x; }CodeAction.cpprewrite-string.md,rewrite-string-reverse.md"..."↔''...''VariableLookup.cpp,CodeAction.cppundefined-var.md{x}: y→{x, y}: ywith isPreferredVariableLookup.cpp,CodeAction.cppunused-formal.md{x, y}: x→{x}: xwith isPreferredVariableLookup.cpp,CodeAction.cppunused-def.mdlet unused = 1; in x→xwith isPreferredrecCodeAction.cppextra-rec.mdrec { x = 1; }→{ x = 1; }withCodeAction.cppextra-with.mdCodeAction.cppempty-inherit.mdinherit;→ (removed)CodeAction.cppredundant-paren.md((x))→xCodeAction.cpp,Protocol.h,Protocol.cppReview Order Recommendation
Test Plan
Related
issue: #466