Merge refactor/diag: consolidate diagnostic-construction helpers

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: olwang

crates/rsscript/src/analyzer.rs

@@ -2955,45 +2955,39 @@ impl Analyzer<'_> {
                             "Remove `pure`, or change the parameter to `read` if the function does not mutate it.",
                         ),
                     };
-                    self.diagnostics.push(
-                        Diagnostic::error(
-                            code::INVALID_PURE_EFFECT,
-                            format!(
-                                "`{}` is declared pure but parameter `{}` uses `{}`.",
-                                function.name,
-                                param.name,
-                                param.effect.map_or("unknown", data_effect_name)
-                            ),
-                            param.span.clone(),
-                            label,
-                        )
-                        .with_cause(cause)
-                        .with_fix("remove_pure_or_use_read", fix, "manual"),
-                    );
+                    self.diagnostics.push(checks::diagnostic_helpers::error_cause_manual_fix(
+                        code::INVALID_PURE_EFFECT,
+                        format!(
+                            "`{}` is declared pure but parameter `{}` uses `{}`.",
+                            function.name,
+                            param.name,
+                            param.effect.map_or("unknown", data_effect_name)
+                        ),
+                        param.span.clone(),
+                        label,
+                        cause,
+                        "remove_pure_or_use_read",
+                        fix,
+                    ));
                 }
 
                 for effect in function
                     .effects
                     .iter()
                     .filter(|effect| matches!(effect, EffectDecl::Retains(_)))
                 {
-                    self.diagnostics.push(
-                        Diagnostic::error(
-                            code::INVALID_PURE_EFFECT,
-                            format!(
-                                "`{}` is declared pure but also retains a parameter.",
-                                function.name
-                            ),
-                            function.span.clone(),
-                            "retention in pure function",
-                        )
-                        .with_cause("A `pure` function must not retain parameters after returning.")
-                        .with_fix(
-                            "remove_pure_or_retains",
-                            format!("Remove `pure` or remove `{}`.", effect_display(effect)),
-                            "manual",
+                    self.diagnostics.push(checks::diagnostic_helpers::error_cause_manual_fix(
+                        code::INVALID_PURE_EFFECT,
+                        format!(
+                            "`{}` is declared pure but also retains a parameter.",
+                            function.name
                         ),
-                    );
+                        function.span.clone(),
+                        "retention in pure function",
+                        "A `pure` function must not retain parameters after returning.",
+                        "remove_pure_or_retains",
+                        format!("Remove `pure` or remove `{}`.", effect_display(effect)),
+                    ));
                 }
             }
         }
@@ -4728,67 +4722,46 @@ impl Analyzer<'_> {
         callee: &Callee,
         span: &crate::diagnostic::Span,
     ) {
-        self.diagnostics.push(
-            Diagnostic::error(
-                code::INVALID_PURE_EFFECT,
-                format!(
-                    "`{function_name}` is declared pure but calls non-pure function `{}`.",
-                    callee_display(callee)
-                ),
-                span.clone(),
-                "non-pure call in pure function",
-            )
-            .with_cause(
-                "A `pure` function may only call constructors, enum variants, or functions also declared `effects(pure)`.",
-            )
-            .with_fix(
-                "remove_pure_or_call_pure",
-                "Remove `pure`, or call only APIs whose signatures are declared `effects(pure)`.",
-                "manual",
+        self.diagnostics.push(checks::diagnostic_helpers::error_cause_manual_fix(
+            code::INVALID_PURE_EFFECT,
+            format!(
+                "`{function_name}` is declared pure but calls non-pure function `{}`.",
+                callee_display(callee)
             ),
-        );
+            span.clone(),
+            "non-pure call in pure function",
+            "A `pure` function may only call constructors, enum variants, or functions also declared `effects(pure)`.",
+            "remove_pure_or_call_pure",
+            "Remove `pure`, or call only APIs whose signatures are declared `effects(pure)`.",
+        ));
     }
 
     fn pure_manage_diagnostic(&mut self, function_name: &str, span: &crate::diagnostic::Span) {
-        self.diagnostics.push(
-            Diagnostic::error(
-                code::INVALID_PURE_EFFECT,
-                format!("`{function_name}` is declared pure but uses `manage`."),
-                span.clone(),
-                "manage in pure function",
-            )
-            .with_cause(
-                "`manage` consumes a local value and changes its ownership boundary; `pure` functions may observe inputs but must not consume local values.",
-            )
-            .with_fix(
-                "remove_manage_or_pure",
-                "Move the `manage` operation outside the pure function, or remove `pure`.",
-                "manual",
-            ),
-        );
+        self.diagnostics.push(checks::diagnostic_helpers::error_cause_manual_fix(
+            code::INVALID_PURE_EFFECT,
+            format!("`{function_name}` is declared pure but uses `manage`."),
+            span.clone(),
+            "manage in pure function",
+            "`manage` consumes a local value and changes its ownership boundary; `pure` functions may observe inputs but must not consume local values.",
+            "remove_manage_or_pure",
+            "Move the `manage` operation outside the pure function, or remove `pure`.",
+        ));
     }
 
     fn pure_with_resource_diagnostic(
         &mut self,
         function_name: &str,
         span: &crate::diagnostic::Span,
     ) {
-        self.diagnostics.push(
-            Diagnostic::error(
-                code::INVALID_PURE_EFFECT,
-                format!("`{function_name}` is declared pure but opens a resource scope."),
-                span.clone(),
-                "resource scope in pure function",
-            )
-            .with_cause(
-                "`with` introduces deterministic resource lifetime behavior; `pure` functions may observe inputs but must not open resource scopes.",
-            )
-            .with_fix(
-                "remove_with_or_pure",
-                "Move resource handling outside the pure function, or remove `pure`.",
-                "manual",
-            ),
-        );
+        self.diagnostics.push(checks::diagnostic_helpers::error_cause_manual_fix(
+            code::INVALID_PURE_EFFECT,
+            format!("`{function_name}` is declared pure but opens a resource scope."),
+            span.clone(),
+            "resource scope in pure function",
+            "`with` introduces deterministic resource lifetime behavior; `pure` functions may observe inputs but must not open resource scopes.",
+            "remove_with_or_pure",
+            "Move resource handling outside the pure function, or remove `pure`.",
+        ));
     }
 
     fn pure_resource_return_diagnostic(
@@ -4797,24 +4770,17 @@ impl Analyzer<'_> {
         span: crate::diagnostic::Span,
         resource_name: &str,
     ) {
-        self.diagnostics.push(
-            Diagnostic::error(
-                code::INVALID_PURE_EFFECT,
-                format!(
-                    "`{function_name}` is declared pure but returns resource `{resource_name}`."
-                ),
-                span,
-                "resource return in pure function",
-            )
-            .with_cause(
-                "Returning a resource creates a lifetime boundary; `pure` functions must not open or return resources.",
-            )
-            .with_fix(
-                "remove_resource_return_or_pure",
-                "Return an ordinary value, or remove `pure` from the resource-producing function.",
-                "manual",
+        self.diagnostics.push(checks::diagnostic_helpers::error_cause_manual_fix(
+            code::INVALID_PURE_EFFECT,
+            format!(
+                "`{function_name}` is declared pure but returns resource `{resource_name}`."
             ),
-        );
+            span,
+            "resource return in pure function",
+            "Returning a resource creates a lifetime boundary; `pure` functions must not open or return resources.",
+            "remove_resource_return_or_pure",
+            "Return an ordinary value, or remove `pure` from the resource-producing function.",
+        ));
     }
 
     fn resource_return_type_name<'a>(&self, ty: &'a TypeRef) -> Option<&'a str> {

crates/rsscript/src/checks/body/effects.rs

@@ -1,4 +1,5 @@
 use super::*;
+use crate::checks::diagnostic_helpers::error_cause_manual_fix;
 
 pub(super) fn check_manage_operand_is_local(
     analyzer: &mut Analyzer<'_>,
@@ -28,20 +29,15 @@ pub(super) fn check_manage_operand_is_local(
 }
 
 pub(super) fn read_view_mutation_diagnostic(analyzer: &mut Analyzer<'_>, name: &str, span: Span) {
-    analyzer.diagnostics.push(
-        Diagnostic::error(
-            code::READ_VIEW_MUTATION,
-            format!("`{name}` is a read view from a `for` loop and cannot be used as an exclusive value."),
-            span,
-            "read view mutation",
-        )
-        .with_cause("RSScript `for` iterates `List<T>` by read view for non-Copy struct elements, so the loop variable does not own the element.")
-        .with_fix(
-            "copy_before_mutating",
-            "Create a fresh local copy before mutation, or use an explicit partitioning API that grants exclusive element ownership.",
-            "manual",
-        ),
-    );
+    analyzer.diagnostics.push(error_cause_manual_fix(
+        code::READ_VIEW_MUTATION,
+        format!("`{name}` is a read view from a `for` loop and cannot be used as an exclusive value."),
+        span,
+        "read view mutation",
+        "RSScript `for` iterates `List<T>` by read view for non-Copy struct elements, so the loop variable does not own the element.",
+        "copy_before_mutating",
+        "Create a fresh local copy before mutation, or use an explicit partitioning API that grants exclusive element ownership.",
+    ));
 }
 
 pub(super) fn check_take_operand_is_local(
@@ -278,26 +274,21 @@ pub(super) fn fresh_return_target_type(return_ty: &TypeRef) -> &TypeRef {
 }
 
 pub(super) fn managed_to_local_diagnostic(analyzer: &mut Analyzer<'_>, managed_to_local: ManagedToLocalUse) {
-    analyzer.diagnostics.push(
-        Diagnostic::error(
-            code::MANAGED_TO_LOCAL,
-            format!(
-                "managed value cannot be converted to local binding `{}`.",
-                managed_to_local.local_name
-            ),
-            managed_to_local.span,
-            "managed value used as local",
-        )
-        .with_cause(format!(
+    analyzer.diagnostics.push(error_cause_manual_fix(
+        code::MANAGED_TO_LOCAL,
+        format!(
+            "managed value cannot be converted to local binding `{}`.",
+            managed_to_local.local_name
+        ),
+        managed_to_local.span,
+        "managed value used as local",
+        format!(
             "`{}` is already managed; RSScript has no managed -> local conversion.",
             managed_to_local.managed_name
-        ))
-        .with_fix(
-            "create_local",
-            "Create the value as `local` at its creation point.",
-            "manual",
         ),
-    );
+        "create_local",
+        "Create the value as `local` at its creation point.",
+    ));
 }
 
 pub(super) fn take_handle_field_diagnostic(analyzer: &mut Analyzer<'_>, field: &TakeHandleField) {
@@ -315,29 +306,24 @@ pub(super) fn take_handle_field_diagnostic(analyzer: &mut Analyzer<'_>, field: &
 }
 
 pub(super) fn retained_local_diagnostic(analyzer: &mut Analyzer<'_>, retained: RetainedLocalUse) {
-    analyzer.diagnostics.push(
-        Diagnostic::error(
-            code::LOCAL_VALUE_RETAINED,
-            format!(
-                "retaining API `{}` cannot retain local value `{}`.",
-                retained.callee, retained.name
-            ),
-            retained.span,
-            "local value retained",
-        )
-        .with_cause(format!(
+    analyzer.diagnostics.push(error_cause_manual_fix(
+        code::LOCAL_VALUE_RETAINED,
+        format!(
+            "retaining API `{}` cannot retain local value `{}`.",
+            retained.callee, retained.name
+        ),
+        retained.span,
+        "local value retained",
+        format!(
             "`{}` declares `effects(retains({}))`.",
             retained.callee, retained.param
-        ))
-        .with_fix(
-            "manage_local",
-            format!(
-                "Pass `{}` through `manage {}` before retaining it.",
-                retained.param, retained.name
-            ),
-            "manual",
         ),
-    );
+        "manage_local",
+        format!(
+            "Pass `{}` through `manage {}` before retaining it.",
+            retained.param, retained.name
+        ),
+    ));
 }
 
 pub(super) fn retained_closure_capture_diagnostic(
@@ -485,25 +471,20 @@ pub(super) fn try_error_type_mismatch_diagnostic(
 }
 
 pub(super) fn moved_use_diagnostic(analyzer: &mut Analyzer<'_>, moved_use: MovedUse) {
-    analyzer.diagnostics.push(
-        Diagnostic::error(
-            code::USE_AFTER_MANAGE,
-            format!(
-                "`{}` was moved into the managed runtime by `manage {}`.",
-                moved_use.name, moved_use.name
-            ),
-            moved_use.use_span,
-            "used after manage",
-        )
-        .with_cause(format!(
+    analyzer.diagnostics.push(error_cause_manual_fix(
+        code::USE_AFTER_MANAGE,
+        format!(
+            "`{}` was moved into the managed runtime by `manage {}`.",
+            moved_use.name, moved_use.name
+        ),
+        moved_use.use_span,
+        "used after manage",
+        format!(
             "The move happened at {}:{}.",
             moved_use.move_span.line, moved_use.move_span.column
-        ))
-        .with_fix(
-            "move_use_before_manage",
-            format!("Move this use before `manage {}`.", moved_use.name),
-            "manual",
         ),
-    );
+        "move_use_before_manage",
+        format!("Move this use before `manage {}`.", moved_use.name),
+    ));
 }