Clean up code actions

Author: AJenbo

docs/todo/refactor.md

@@ -350,54 +350,4 @@ constructor.
 
 ---
 
-## Code actions: shared edit-building helpers
-
-**What to do.** Complementary to (and independent of) A34 in the
-backlog:
-
-1. **`single_file_edit` helper.** `WorkspaceEdit` for one file is
-   open-coded in ~35 files (~50 `document_changes: None` literals,
-   8-12 lines each). Add
-   `single_file_edit(uri, edits) -> WorkspaceEdit` /
-   `single_edit(uri, range, text)` to `code_actions/mod.rs` and adopt.
-2. **Adopt `util::byte_range_to_lsp_range`.** The helper exists but is
-   used in code actions only by `phpstan/add_throws.rs`; ~29 other
-   files hand-build `Range { start: offset_to_position(..), end: .. }`
-   (183 `offset_to_position` calls).
-3. **Consolidate indent helpers.** Nine near-identical
-   line-indent-at-offset extractors and two copies of
-   `detect_indent_unit` across `extract_function.rs`,
-   `generate_property_hooks.rs`, `update_docblock.rs`,
-   `convert_switch_to_match.rs`, `extract_constant.rs`,
-   `phpstan/new_static.rs`, `implement_methods.rs`. Provide
-   `indent_of_line_at` / `indent_unit` next to
-   `detect_indent_from_members`.
-4. **Unify the name-deduplication helpers.** `deduplicate_name` in
-   `extract_variable.rs` (checks a `&[String]` of in-scope names,
-   numeric suffix), `deduplicate_constant_name` in `extract_constant.rs`
-   (same shape, `_N` suffix), and `deduplicate_name` in
-   `extract_function.rs` (scans sibling method names or `function <name>`
-   patterns in the file) all solve "make this name unique." Back them
-   with one helper in `code_actions/naming.rs` that takes an
-   `exists: impl Fn(&str) -> bool` predicate and a suffix separator. (The
-   pure casing transforms — `to_camel_case`, `snake_to_camel`,
-   `to_pascal_case`, `string_to_screaming_snake`, `capitalise` — already
-   live in `code_actions/naming.rs`.)
-5. **`find_docblock_above_line` helper.** At least three independent
-   copies (`phpstan/remove_throws.rs`, `phpstan/add_throws.rs`,
-   `phpstan/add_iterable_type.rs`) locate the `/** */` block above a
-   line; `update_docblock.rs` additionally owns a private docblock
-   line-model (`parse_docblock_lines`/`rebuild_docblock`) that the
-   other handlers re-approximate. Extract a shared
-   `code_actions/docblock_edit.rs`.
-6. **Relocate PHPStan-specific logic out of `code_actions/mod.rs`**
-   (`expand_sibling_checked_exception_diags`,
-   `clear_phpstan_diagnostics_after_resolve`) into `phpstan/mod.rs`.
-
-**Why it matters.** Several new code actions are scheduled (A40, A41,
-H4-H24, FX rules); each currently starts by copying this plumbing from
-a neighbour, adding another divergent copy per action.
-
----
-
 

src/code_actions/change_visibility.rs

@@ -24,7 +24,6 @@
 //! This is a single-file edit — it does not update call sites or
 //! subclass overrides in other files.
 
-use std::collections::HashMap;
 use std::sync::Arc;
 
 #[cfg(test)]
@@ -227,23 +226,14 @@ impl Backend {
         let end_pos = offset_to_position(content, vis_end);
 
         let doc_uri: Url = uri.parse().ok()?;
-        let mut changes = HashMap::new();
-        changes.insert(
+        Some(crate::code_actions::single_edit(
             doc_uri,
-            vec![TextEdit {
-                range: Range {
-                    start: start_pos,
-                    end: end_pos,
-                },
-                new_text: target_vis.to_string(),
-            }],
-        );
-
-        Some(WorkspaceEdit {
-            changes: Some(changes),
-            document_changes: None,
-            change_annotations: None,
-        })
+            Range {
+                start: start_pos,
+                end: end_pos,
+            },
+            target_vis.to_string(),
+        ))
     }
 
     // ── Private helpers ─────────────────────────────────────────────────

src/code_actions/convert_switch_to_match.rs

@@ -9,8 +9,6 @@
 //! - No case body falls through to the next without `break`/`return`/`throw`.
 //! - The switch subject is a simple expression.
 
-use std::collections::HashMap;
-
 use mago_span::HasSpan;
 use mago_syntax::cst::*;
 use tower_lsp::lsp_types::*;
@@ -60,27 +58,18 @@ impl Backend {
         let start_pos = offset_to_position(content, switch_start as usize);
         let end_pos = offset_to_position(content, switch_end as usize);
 
-        let mut changes = HashMap::new();
-        changes.insert(
-            doc_uri,
-            vec![TextEdit {
-                range: Range {
-                    start: start_pos,
-                    end: end_pos,
-                },
-                new_text: replacement,
-            }],
-        );
-
         out.push(CodeActionOrCommand::CodeAction(CodeAction {
             title: "Convert to match expression".to_string(),
             kind: Some(CodeActionKind::new("refactor.rewrite")),
             diagnostics: None,
-            edit: Some(WorkspaceEdit {
-                changes: Some(changes),
-                document_changes: None,
-                change_annotations: None,
-            }),
+            edit: Some(crate::code_actions::single_edit(
+                doc_uri,
+                Range {
+                    start: start_pos,
+                    end: end_pos,
+                },
+                replacement,
+            )),
             command: None,
             is_preferred: Some(false),
             disabled: None,

src/code_actions/convert_to_arrow_function.rs

@@ -9,8 +9,6 @@
 //! - The closure does not have a `void` or `never` return type hint.
 //! - PHP version is >= 7.4.
 
-use std::collections::HashMap;
-
 use mago_span::HasSpan;
 use mago_syntax::cst::access::Access;
 use mago_syntax::cst::call::Call;
@@ -62,27 +60,18 @@ impl Backend {
         let start_pos = offset_to_position(content, closure_start as usize);
         let end_pos = offset_to_position(content, closure_end as usize);
 
-        let mut changes = HashMap::new();
-        changes.insert(
-            doc_uri,
-            vec![TextEdit {
-                range: Range {
-                    start: start_pos,
-                    end: end_pos,
-                },
-                new_text: replacement,
-            }],
-        );
-
         out.push(CodeActionOrCommand::CodeAction(CodeAction {
             title: "Convert to arrow function".to_string(),
             kind: Some(CodeActionKind::new("refactor.rewrite")),
             diagnostics: None,
-            edit: Some(WorkspaceEdit {
-                changes: Some(changes),
-                document_changes: None,
-                change_annotations: None,
-            }),
+            edit: Some(crate::code_actions::single_edit(
+                doc_uri,
+                Range {
+                    start: start_pos,
+                    end: end_pos,
+                },
+                replacement,
+            )),
             command: None,
             is_preferred: Some(false),
             disabled: None,

src/code_actions/convert_to_closure.rs

@@ -9,7 +9,7 @@
 //!
 //! The action is always offered when the cursor is on an arrow function.
 
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
 
 use mago_span::HasSpan;
 use mago_syntax::cst::access::Access;
@@ -57,27 +57,18 @@ impl Backend {
         let start_pos = offset_to_position(content, arrow_start as usize);
         let end_pos = offset_to_position(content, arrow_end as usize);
 
-        let mut changes = HashMap::new();
-        changes.insert(
-            doc_uri,
-            vec![TextEdit {
-                range: Range {
-                    start: start_pos,
-                    end: end_pos,
-                },
-                new_text: replacement,
-            }],
-        );
-
         out.push(CodeActionOrCommand::CodeAction(CodeAction {
             title: "Convert to closure".to_string(),
             kind: Some(CodeActionKind::new("refactor.rewrite")),
             diagnostics: None,
-            edit: Some(WorkspaceEdit {
-                changes: Some(changes),
-                document_changes: None,
-                change_annotations: None,
-            }),
+            edit: Some(crate::code_actions::single_edit(
+                doc_uri,
+                Range {
+                    start: start_pos,
+                    end: end_pos,
+                },
+                replacement,
+            )),
             command: None,
             is_preferred: Some(false),
             disabled: None,

src/code_actions/convert_to_instance_variable.rs

@@ -14,8 +14,6 @@
 //! - The `$this` variable is never offered for conversion.
 //! - Only works inside a method body of a class-like declaration.
 
-use std::collections::HashMap;
-
 use mago_span::HasSpan;
 use mago_syntax::cst::class_like::member::ClassLikeMember;
 use mago_syntax::cst::class_like::method::MethodBody;
@@ -511,14 +509,7 @@ impl Backend {
                 .then(a.range.start.character.cmp(&b.range.start.character))
         });
 
-        let mut changes = HashMap::new();
-        changes.insert(doc_uri, edits);
-
-        Some(WorkspaceEdit {
-            changes: Some(changes),
-            document_changes: None,
-            change_annotations: None,
-        })
+        Some(crate::code_actions::single_file_edit(doc_uri, edits))
     }
 }
 

src/code_actions/docblock_edit.rs

@@ -0,0 +1,92 @@
+//! Shared docblock-location helpers for code actions.
+//!
+//! Several PHPStan quickfixes need to locate the `/** … */` block that sits
+//! directly above a function, method, or property signature so they can
+//! rewrite or remove tags inside it.  This module owns the single
+//! implementation they share.
+
+/// Information about a docblock found above a given line.
+pub(crate) struct DocblockAbove {
+    /// Byte offset of the start of the docblock (first char of the `/**`
+    /// line, including leading whitespace).
+    pub(crate) start: usize,
+    /// Byte offset just past the end of the docblock (past the `*/` line,
+    /// including its trailing newline).
+    pub(crate) end: usize,
+    /// The raw docblock text including indentation.
+    pub(crate) text: String,
+}
+
+/// Find the docblock immediately above the given line.
+///
+/// The diagnostic line is the function/method/property signature.  The
+/// docblock (if any) sits directly above it, possibly separated by blank
+/// lines or attribute (`#[…]`) lines.
+pub(crate) fn find_docblock_above_line(content: &str, line: usize) -> Option<DocblockAbove> {
+    let lines: Vec<&str> = content.lines().collect();
+    if line == 0 || line > lines.len() {
+        return None;
+    }
+
+    // Walk backward from the line before the diagnostic to find `*/`.
+    let mut doc_end_line = None;
+    for i in (0..line).rev() {
+        let trimmed = lines[i].trim();
+        if trimmed.is_empty() {
+            continue;
+        }
+        if trimmed.ends_with("*/") {
+            doc_end_line = Some(i);
+            break;
+        }
+        // Attributes (#[...]) can appear between docblock and declaration.
+        if trimmed.starts_with("#[") {
+            continue;
+        }
+        // Anything else means no docblock.
+        break;
+    }
+
+    let end_line = doc_end_line?;
+
+    // Walk backward from end_line to find `/**`.
+    let mut doc_start_line = None;
+    for i in (0..=end_line).rev() {
+        let trimmed = lines[i].trim();
+        if trimmed.contains("/**") {
+            doc_start_line = Some(i);
+            break;
+        }
+        // Should be a `*`-prefixed line or end-of-docblock.
+        if !trimmed.starts_with('*') && !trimmed.ends_with("*/") {
+            break;
+        }
+    }
+
+    let start_line = doc_start_line?;
+
+    // Convert line numbers to byte offsets.
+    let mut byte_offset = 0;
+    let mut start_byte = 0;
+    let mut end_byte = 0;
+    for (i, line_text) in lines.iter().enumerate() {
+        if i == start_line {
+            start_byte = byte_offset;
+        }
+        byte_offset += line_text.len() + 1; // +1 for newline
+        if i == end_line {
+            end_byte = byte_offset; // include trailing newline
+        }
+    }
+
+    let text = content
+        .get(start_byte..end_byte.min(content.len()))
+        .unwrap_or("")
+        .to_string();
+
+    Some(DocblockAbove {
+        start: start_byte,
+        end: end_byte.min(content.len()),
+        text,
+    })
+}