Simplify diagnostic reporting

2026-01-14 16:08:02 -05:00 · 2023-07-24 13:58:12 +10:00
parent 267a86fb2c
commit 870c9eed2a
8 changed files with 361 additions and 648 deletions
--- a/valuescript_compiler/src/compile_enum_value.rs
+++ b/valuescript_compiler/src/compile_enum_value.rs
@@ -2,9 +2,9 @@ use swc_common::Spanned;

 use crate::{
  asm::{Number, Object, Value},
+  diagnostic::DiagnosticReporter,
  module_compiler::ModuleCompiler,
  static_eval_expr::static_eval_expr,
-  Diagnostic, DiagnosticLevel,
 };

 pub fn compile_enum_value(mc: &mut ModuleCompiler, ts_enum: &swc_ecma_ast::TsEnumDecl) -> Value {
@@ -28,11 +28,7 @@ pub fn compile_enum_value(mc: &mut ModuleCompiler, ts_enum: &swc_ecma_ast::TsEnu
          _ => None,
        },
        None => {
-          mc.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::InternalError,
-            message: "TODO: Static eval failed".to_string(),
-            span: init.span(),
-          });
+          mc.internal_error(init.span(), "Static eval failed");

          None
        }
@@ -46,11 +42,7 @@ pub fn compile_enum_value(mc: &mut ModuleCompiler, ts_enum: &swc_ecma_ast::TsEnu
        let id = match next_default_id {
          Some(id) => id,
          None => {
-            mc.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::Error,
-              message: "Missing required initializer".to_string(),
-              span: member.span,
-            });
+            mc.error(member.span, "Missing required initializer");

            0.0
          }
--- a/valuescript_compiler/src/diagnostic.rs
+++ b/valuescript_compiler/src/diagnostic.rs
@@ -51,4 +51,87 @@ impl Diagnostic {
        .unwrap_or(swc_common::DUMMY_SP),
    })
  }
+
+  pub fn todo(span: swc_common::Span, message: &str) -> Self {
+    Diagnostic {
+      level: DiagnosticLevel::InternalError,
+      message: format!("TODO: {}", message),
+      span,
+    }
+  }
+
+  pub fn error(span: swc_common::Span, message: &str) -> Self {
+    Diagnostic {
+      level: DiagnosticLevel::Error,
+      message: message.to_string(),
+      span,
+    }
+  }
+
+  pub fn internal_error(span: swc_common::Span, message: &str) -> Self {
+    Diagnostic {
+      level: DiagnosticLevel::InternalError,
+      message: message.to_string(),
+      span,
+    }
+  }
+
+  pub fn not_supported(span: swc_common::Span, message: &str) -> Self {
+    Diagnostic {
+      level: DiagnosticLevel::Error,
+      message: format!("Not supported: {}", message),
+      span,
+    }
+  }
+
+  pub fn lint(span: swc_common::Span, message: &str) -> Self {
+    Diagnostic {
+      level: DiagnosticLevel::Lint,
+      message: message.to_string(),
+      span,
+    }
+  }
+}
+
+pub trait DiagnosticContainer {
+  fn diagnostics_mut(&mut self) -> &mut Vec<Diagnostic>;
+}
+
+pub trait DiagnosticReporter {
+  fn todo(&mut self, span: swc_common::Span, message: &str);
+  fn error(&mut self, span: swc_common::Span, message: &str);
+  fn internal_error(&mut self, span: swc_common::Span, message: &str);
+  fn not_supported(&mut self, span: swc_common::Span, message: &str);
+  fn lint(&mut self, span: swc_common::Span, message: &str);
+}
+
+impl<T> DiagnosticReporter for T
+where
+  T: DiagnosticContainer,
+{
+  fn todo(&mut self, span: swc_common::Span, message: &str) {
+    self.diagnostics_mut().push(Diagnostic::todo(span, message));
+  }
+
+  fn error(&mut self, span: swc_common::Span, message: &str) {
+    self
+      .diagnostics_mut()
+      .push(Diagnostic::error(span, message));
+  }
+
+  fn internal_error(&mut self, span: swc_common::Span, message: &str) {
+    self
+      .diagnostics_mut()
+      .push(Diagnostic::internal_error(span, message));
+  }
+
+  fn not_supported(&mut self, span: swc_common::Span, message: &str) {
+    self
+      .diagnostics_mut()
+      .push(Diagnostic::not_supported(span, message));
+  }
+
+  fn lint(&mut self, span: swc_common::Span, message: &str) {
+    self.diagnostics_mut().push(Diagnostic::lint(span, message));
+  }
 }
--- a/valuescript_compiler/src/expression_compiler.rs
+++ b/valuescript_compiler/src/expression_compiler.rs
@@ -4,7 +4,7 @@ use queues::*;
 use swc_common::Spanned;

 use crate::asm::{Array, Instruction, Label, Number, Object, Register, Value};
-use crate::diagnostic::{Diagnostic, DiagnosticLevel};
+use crate::diagnostic::{Diagnostic, DiagnosticContainer, DiagnosticReporter};
 use crate::function_compiler::{FunctionCompiler, Functionish, QueuedFunction};
 use crate::ident::Ident as CrateIdent;
 use crate::scope::{NameId, OwnerId};
@@ -65,6 +65,12 @@ pub struct ExpressionCompiler<'a, 'fnc> {
  pub fnc: &'a mut FunctionCompiler<'fnc>,
 }

+impl<'a, 'fnc> DiagnosticContainer for ExpressionCompiler<'a, 'fnc> {
+  fn diagnostics_mut(&mut self) -> &mut Vec<Diagnostic> {
+    self.fnc.diagnostics_mut()
+  }
+}
+
 impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
  pub fn compile_top_level(
    &mut self,
@@ -97,7 +103,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
      Assign(assign_exp) => self.assign_expression(assign_exp, false, target_register),
      Member(member_exp) => self.member_expression(member_exp, target_register),
      SuperProp(super_prop) => {
-        self.fnc.todo(super_prop.span, "SuperProp expression");
+        self.todo(super_prop.span, "SuperProp expression");
        CompiledExpression::empty()
      }
      Cond(cond_exp) => self.cond_expression(cond_exp, target_register),
@@ -109,9 +115,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
          _ => self.call_expression(call_exp, target_register),
        },
        _ => {
-          self
-            .fnc
-            .todo(call_exp.callee.span(), "non-expression callee");
+          self.todo(call_exp.callee.span(), "non-expression callee");

          CompiledExpression::empty()
        }
@@ -128,44 +132,42 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
      Lit(lit) => self.compile_literal(lit).to_ce(),
      Tpl(tpl) => self.template_literal(tpl, target_register),
      TaggedTpl(tagged_tpl) => {
-        self.fnc.todo(tagged_tpl.span, "TaggedTpl expression");
+        self.todo(tagged_tpl.span, "TaggedTpl expression");
        CompiledExpression::empty()
      }
      Arrow(arrow) => self.arrow_expression(arrow, target_register),
      Class(class_exp) => {
-        self.fnc.todo(class_exp.span(), "Class expression");
+        self.todo(class_exp.span(), "Class expression");
        CompiledExpression::empty()
      }
      Yield(yield_expr) => self.yield_expr(yield_expr, target_register),
      MetaProp(meta_prop) => {
-        self.fnc.todo(meta_prop.span, "MetaProp expression");
+        self.todo(meta_prop.span, "MetaProp expression");
        CompiledExpression::empty()
      }
      Await(await_exp) => {
-        self.fnc.todo(await_exp.span, "Await expression");
+        self.todo(await_exp.span, "Await expression");
        CompiledExpression::empty()
      }
      Paren(p) => self.compile(&p.expr, target_register),
      JSXMember(jsx_member) => {
-        self.fnc.todo(jsx_member.span(), "JSXMember expression");
+        self.todo(jsx_member.span(), "JSXMember expression");
        CompiledExpression::empty()
      }
      JSXNamespacedName(jsx_namespaced_name) => {
-        self
-          .fnc
-          .todo(jsx_namespaced_name.span(), "JSXNamespacedName expression");
+        self.todo(jsx_namespaced_name.span(), "JSXNamespacedName expression");
        CompiledExpression::empty()
      }
      JSXEmpty(jsx_empty) => {
-        self.fnc.todo(jsx_empty.span(), "JSXEmpty expression");
+        self.todo(jsx_empty.span(), "JSXEmpty expression");
        CompiledExpression::empty()
      }
      JSXElement(jsx_element) => {
-        self.fnc.todo(jsx_element.span(), "JSXElement expression");
+        self.todo(jsx_element.span(), "JSXElement expression");
        CompiledExpression::empty()
      }
      JSXFragment(jsx_fragment) => {
-        self.fnc.todo(jsx_fragment.span(), "JSXFragment expression");
+        self.todo(jsx_fragment.span(), "JSXFragment expression");
        CompiledExpression::empty()
      }
      TsTypeAssertion(ts_type_assertion) => self.compile(&ts_type_assertion.expr, target_register),
@@ -175,26 +177,20 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
      TsNonNull(ts_non_null_exp) => self.compile(&ts_non_null_exp.expr, target_register),
      TsAs(ts_as_exp) => self.compile(&ts_as_exp.expr, target_register),
      TsInstantiation(ts_instantiation) => {
-        self
-          .fnc
-          .todo(ts_instantiation.span, "TsInstantiation expression");
+        self.todo(ts_instantiation.span, "TsInstantiation expression");

        CompiledExpression::empty()
      }
      PrivateName(private_name) => {
-        self.fnc.todo(private_name.span, "PrivateName expression");
+        self.todo(private_name.span, "PrivateName expression");
        CompiledExpression::empty()
      }
      OptChain(opt_chain) => {
-        self.fnc.todo(opt_chain.span, "OptChain expression");
+        self.todo(opt_chain.span, "OptChain expression");
        CompiledExpression::empty()
      }
      Invalid(invalid) => {
-        self.fnc.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Invalid expression".to_string(),
-          span: invalid.span,
-        });
+        self.error(invalid.span, "Invalid expression");

        CompiledExpression::empty()
      }
@@ -244,9 +240,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
      match make_unary_op(un_exp.op, arg.value.clone(), target.clone()) {
        Some(i) => i,
        None => {
-          self
-            .fnc
-            .todo(un_exp.span, &format!("Unary operator {:?}", un_exp.op));
+          self.todo(un_exp.span, &format!("Unary operator {:?}", un_exp.op));

          return CompiledExpression::empty();
        }
@@ -306,11 +300,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
    };

    if let Some(err_msg) = err_msg {
-      self.fnc.diagnostics.push(Diagnostic {
-        level: DiagnosticLevel::Error,
-        message: err_msg,
-        span: ident.span,
-      });
+      self.error(ident.span, &err_msg);
    }

    if let Some(reg) = reg {
@@ -417,12 +407,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
          self.get_register_for_ident_mutation(&CrateIdent::from_swc_ident(&ident.id)),
        ),
        _ => {
-          self.fnc.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::Error,
-            message: "Invalid lvalue expression".to_string(),
-            span: pat.span(),
-          });
-
+          self.error(pat.span(), "Invalid lvalue expression");
          let bad_reg = self.fnc.allocate_numbered_reg("_bad_lvalue");

          TargetAccessor::Register(bad_reg)
@@ -493,7 +478,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {

      match &object_exp.props[i] {
        PropOrSpread::Spread(spread) => {
-          self.fnc.todo(spread.span(), "spread expression");
+          self.todo(spread.span(), "spread expression");
        }
        PropOrSpread::Prop(prop) => match &**prop {
          Prop::Shorthand(ident) => {
@@ -520,10 +505,10 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {

            object_asm.properties.push((prop_key, prop_value));
          }
-          Prop::Assign(assign) => self.fnc.todo(assign.span(), "Assign prop"),
-          Prop::Getter(getter) => self.fnc.todo(getter.span(), "Getter prop"),
-          Prop::Setter(setter) => self.fnc.todo(setter.span(), "Setter prop"),
-          Prop::Method(method) => self.fnc.todo(method.span(), "Method prop"),
+          Prop::Assign(assign) => self.todo(assign.span(), "Assign prop"),
+          Prop::Getter(getter) => self.todo(getter.span(), "Getter prop"),
+          Prop::Setter(setter) => self.todo(setter.span(), "Setter prop"),
+          Prop::Method(method) => self.todo(method.span(), "Method prop"),
        },
      }
    }
@@ -583,9 +568,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
      swc_ecma_ast::MemberProp::Ident(ident) => Value::String(ident.sym.to_string()).to_ce(),
      swc_ecma_ast::MemberProp::Computed(computed) => self.compile(&computed.expr, target_register),
      swc_ecma_ast::MemberProp::PrivateName(private_name) => {
-        self
-          .fnc
-          .todo(private_name.span(), "private name member property");
+        self.todo(private_name.span(), "private name member property");

        CompiledExpression::empty()
      }
@@ -766,9 +749,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
    let callee = match &call_exp.callee {
      swc_ecma_ast::Callee::Expr(expr) => self.compile(expr, None),
      _ => {
-        self
-          .fnc
-          .todo(call_exp.callee.span(), "non-expression callee");
+        self.todo(call_exp.callee.span(), "non-expression callee");

        CompiledExpression::empty()
      }
@@ -1049,14 +1030,13 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
          _ => continue,
        },
        None => {
-          self.fnc.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::InternalError,
-            message: format!(
+          self.internal_error(
+            cap.span(),
+            &format!(
              "Failed to find capture {:?} for scope {:?}",
              cap, self.fnc.owner_id
            ),
-            span: cap.span(),
-          });
+          );

          continue;
        }
@@ -1085,22 +1065,21 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {

        if let Some(tdz_end) = cap_name.tdz_end {
          if span.lo() <= tdz_end {
-            self.fnc.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::Error,
-              message: match &fn_name {
+            self.error(
+              span,
+              &match &fn_name {
                Some(name) => format!(
                  "Referencing {} is invalid because it binds {} before its declaration (temporal \
-                    dead zone)",
+                  dead zone)",
                  name, cap_name.sym,
                ),
                None => format!(
                  "Expression is invalid because capturing {} binds its value before its \
-                    declaration (temporal dead zone)",
+                  declaration (temporal dead zone)",
                  cap_name.sym,
                ),
              },
-              span,
-            });
+            );
          }
        }
      }
@@ -1113,14 +1092,13 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
            _ => continue,
          },
          None => {
-            self.fnc.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::InternalError,
-              message: format!(
+            self.internal_error(
+              cap.span(),
+              &format!(
                "Failed to find capture {:?} for scope {:?}",
                cap, self.fnc.owner_id
              ),
-              span: cap.span(),
-            });
+            );

            continue;
          }
@@ -1247,11 +1225,10 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
        false => None,
      },
      _ => {
-        self.fnc.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: format!("Failed to lookup identifier `{}`", ident.sym),
-          span: ident.span,
-        });
+        self.internal_error(
+          ident.span,
+          &format!("Failed to lookup identifier `{}`", ident.sym),
+        );

        None
      }
@@ -1306,7 +1283,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
    match value_from_literal(lit) {
      Ok(value) => value,
      Err(err) => {
-        self.fnc.todo(lit.span(), err);
+        self.todo(lit.span(), err);
        Value::Register(self.fnc.allocate_numbered_reg("_todo_unsupported_literal"))
      }
    }
@@ -1320,14 +1297,13 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
        let ident_reg = self.fnc.get_pattern_register(pat);

        if register != &ident_reg {
-          self.fnc.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::InternalError,
-            message: format!(
+          self.internal_error(
+            pat.span(),
+            &format!(
              "Register mismatch for parameter {} (expected {}, got {})",
              ident.id.sym, ident_reg, register
            ),
-            span: pat.span(),
-          });
+          );

          // Note: We still have this sensible interpretation, so emitting it
          // may help troubleshooting the error above. Hopefully it never
@@ -1397,9 +1373,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
              }
            }
            ObjectPatProp::Rest(rest) => {
-              self
-                .fnc
-                .todo(rest.span, "Rest pattern in object destructuring");
+              self.todo(rest.span, "Rest pattern in object destructuring");
            }
          }
        }
--- a/valuescript_compiler/src/function_compiler.rs
+++ b/valuescript_compiler/src/function_compiler.rs
@@ -9,7 +9,7 @@ use crate::asm::{
  Value,
 };
 use crate::compile_enum_value::compile_enum_value;
-use crate::diagnostic::{Diagnostic, DiagnosticLevel};
+use crate::diagnostic::{Diagnostic, DiagnosticContainer, DiagnosticLevel, DiagnosticReporter};
 use crate::expression_compiler::CompiledExpression;
 use crate::expression_compiler::ExpressionCompiler;
 use crate::ident::Ident;
@@ -78,6 +78,12 @@ pub struct FunctionCompiler<'a> {
  pub diagnostics: Vec<Diagnostic>,
 }

+impl<'a> DiagnosticContainer for FunctionCompiler<'a> {
+  fn diagnostics_mut(&mut self) -> &mut Vec<Diagnostic> {
+    &mut self.diagnostics
+  }
+}
+
 impl<'a> FunctionCompiler<'a> {
  pub fn new(mc: &'a mut ModuleCompiler, owner_id: OwnerId) -> Self {
    let reg_allocator = match mc.scope_analysis.reg_allocators.get(&owner_id) {
@@ -148,30 +154,6 @@ impl<'a> FunctionCompiler<'a> {
      .lookup_by_name_id(&self.owner_id, name_id)
  }

-  pub fn todo(&mut self, span: swc_common::Span, message: &str) {
-    self.diagnostics.push(Diagnostic {
-      level: DiagnosticLevel::InternalError,
-      message: format!("TODO: {}", message),
-      span,
-    });
-  }
-
-  pub fn error(&mut self, span: swc_common::Span, message: &str) {
-    self.diagnostics.push(Diagnostic {
-      level: DiagnosticLevel::Error,
-      message: message.to_string(),
-      span,
-    });
-  }
-
-  pub fn internal_error(&mut self, span: swc_common::Span, message: &str) {
-    self.diagnostics.push(Diagnostic {
-      level: DiagnosticLevel::InternalError,
-      message: message.to_string(),
-      span,
-    });
-  }
-
  pub fn allocate_defn(&mut self, name: &str) -> Pointer {
    let allocated_name = self.mc.definition_allocator.allocate(&name.to_string());

@@ -438,14 +420,13 @@ impl<'a> FunctionCompiler<'a> {
    match self.mc.scope_analysis.lookup_value(&self.owner_id, ident) {
      Some(Value::Register(reg)) => reg,
      lookup_result => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: format!(
+        self.internal_error(
+          ident.span,
+          &format!(
            "Register should have been allocated for variable {}, instead: {:?}",
            ident.sym, lookup_result,
          ),
-          span: ident.span,
-        });
+        );

        self.allocate_numbered_reg("_error_variable_without_register")
      }
@@ -519,11 +500,7 @@ impl<'a> FunctionCompiler<'a> {
      Empty(_) => {}
      Debugger(debugger) => self.todo(debugger.span, "Debugger statement"),
      With(with) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Not supported: With statement".to_string(),
-          span: with.span,
-        });
+        self.not_supported(with.span, "With statement");
      }

      Return(ret_stmt) => {
@@ -571,11 +548,7 @@ impl<'a> FunctionCompiler<'a> {
            self.push(Instruction::Jmp(loop_labels.break_.ref_()));
          }
          None => {
-            self.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::Error,
-              message: "break statement outside loop".to_string(),
-              span: break_.span,
-            });
+            self.error(break_.span, "break statement outside loop");
          }
        }
      }
@@ -596,11 +569,7 @@ impl<'a> FunctionCompiler<'a> {
          }
        }

-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "continue statement outside loop".to_string(),
-          span: continue_.span,
-        });
+        self.error(continue_.span, "continue statement outside loop");
      }
      If(if_) => {
        self.if_(if_);
@@ -734,11 +703,7 @@ impl<'a> FunctionCompiler<'a> {
        // default:
        None => {
          if default_i.is_some() {
-            ec.fnc.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::Error,
-              message: "A switch can only have one default".to_string(),
-              span: case.span,
-            });
+            ec.error(case.span, "A switch can only have one default");
          }

          default_i = Some(i);
@@ -1201,15 +1166,14 @@ impl<'a> FunctionCompiler<'a> {
            definition_pointer: match self.lookup_value(&Ident::from_swc_ident(&fn_decl.ident)) {
              Some(Value::Pointer(p)) => p,
              _ => {
-                self.diagnostics.push(Diagnostic {
-                  level: DiagnosticLevel::InternalError,
-                  message: format!(
+                self.internal_error(
+                  fn_decl.ident.span,
+                  &format!(
                    "Lookup of function {} was not a pointer, lookup_result: {:?}",
                    fn_decl.ident.sym,
                    self.lookup_value(&Ident::from_swc_ident(&fn_decl.ident))
                  ),
-                  span: fn_decl.ident.span,
-                });
+                );

                return;
              }
@@ -1230,11 +1194,10 @@ impl<'a> FunctionCompiler<'a> {
        {
          Some(Value::Pointer(p)) => p,
          _ => {
-            self.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::InternalError,
-              message: format!("Pointer for {} should have been in scope", ts_enum.id.sym),
-              span: ts_enum.id.span,
-            });
+            self.internal_error(
+              ts_enum.id.span,
+              &format!("Pointer for {} should have been in scope", ts_enum.id.sym),
+            );

            return;
          }
@@ -1267,13 +1230,11 @@ impl<'a> FunctionCompiler<'a> {
            // undefined
          }
          _ => {
-            self.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::InternalError,
-              message: "Expected destructuring declaration without initializer \
-                to be caught in the parser. Pattern has not been compiled."
-                .to_string(),
-              span: decl.span(),
-            });
+            self.internal_error(
+              decl.span(),
+              "Expected destructuring declaration without initializer to be caught in the parser. \
+                Pattern has not been compiled.",
+            );
          }
        },
      }
--- a/valuescript_compiler/src/module_compiler.rs
+++ b/valuescript_compiler/src/module_compiler.rs
@@ -11,7 +11,7 @@ use crate::asm::{
  Pointer, Register, Value,
 };
 use crate::compile_enum_value::compile_enum_value;
-use crate::diagnostic::{Diagnostic, DiagnosticLevel};
+use crate::diagnostic::{Diagnostic, DiagnosticContainer, DiagnosticReporter};
 use crate::expression_compiler::{CompiledExpression, ExpressionCompiler};
 use crate::function_compiler::{FunctionCompiler, Functionish};
 use crate::ident::Ident;
@@ -102,23 +102,13 @@ pub struct ModuleCompiler {
  pub module: Module,
 }

+impl DiagnosticContainer for ModuleCompiler {
+  fn diagnostics_mut(&mut self) -> &mut Vec<Diagnostic> {
+    &mut self.diagnostics
+  }
+}
+
 impl ModuleCompiler {
-  fn todo(&mut self, span: swc_common::Span, message: &str) {
-    self.diagnostics.push(Diagnostic {
-      level: DiagnosticLevel::InternalError,
-      message: format!("TODO: {}", message),
-      span,
-    });
-  }
-
-  fn not_supported(&mut self, span: swc_common::Span, message: &str) {
-    self.diagnostics.push(Diagnostic {
-      level: DiagnosticLevel::Error,
-      message: format!("Not supported: {}", message),
-      span,
-    });
-  }
-
  fn allocate_defn(&mut self, name: &str) -> Pointer {
    let allocated_name = self.definition_allocator.allocate(&name.to_string());

@@ -142,12 +132,7 @@ impl ModuleCompiler {
      Module(module) => module,
      Script(script) => {
        let mut self_ = Self::default();
-
-        self_.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Scripts are not supported".to_string(),
-          span: script.span,
-        });
+        self_.error(script.span, "Scripts are not supported");

        return self_;
      }
@@ -292,11 +277,7 @@ impl ModuleCompiler {
      let init = match &decl.init {
        Some(_) => &decl.init,
        _ => {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::Error,
-            message: "const variable without initializer".to_string(),
-            span: decl.init.span(),
-          });
+          self.error(decl.init.span(), "const variable without initializer");

          &None
        }
@@ -321,22 +302,12 @@ impl ModuleCompiler {
          Some(name) => match &name.value {
            Value::Pointer(p) => p.clone(),
            _ => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::InternalError,
-                message: "Expected pointer for module constant".to_string(),
-                span: ident.span(),
-              });
-
+              self.internal_error(ident.span(), "Expected pointer for module constant");
              continue;
            }
          },
          None => {
-            self.diagnostics.push(Diagnostic {
-              level: DiagnosticLevel::InternalError,
-              message: "Failed to lookup name".to_string(),
-              span: ident.span(),
-            });
-
+            self.internal_error(ident.span(), "Failed to lookup name");
            continue;
          }
        };
@@ -367,11 +338,10 @@ impl ModuleCompiler {
    {
      Some(Value::Pointer(p)) => p,
      _ => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: format!("Pointer for {} should have been in scope", fn_name),
-          span: fn_.ident.span,
-        });
+        self.internal_error(
+          fn_.ident.span,
+          &format!("Pointer for {} should have been in scope", fn_name),
+        );

        return;
      }
@@ -400,11 +370,10 @@ impl ModuleCompiler {
    {
      Some(Value::Pointer(p)) => p,
      _ => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: format!("Pointer for {} should have been in scope", ts_enum.id.sym),
-          span: ts_enum.id.span,
-        });
+        self.internal_error(
+          ts_enum.id.span,
+          &format!("Pointer for {} should have been in scope", ts_enum.id.sym),
+        );

        return;
      }
@@ -444,11 +413,10 @@ impl ModuleCompiler {
            {
              Some(Value::Pointer(p)) => p,
              _ => {
-                self.diagnostics.push(Diagnostic {
-                  level: DiagnosticLevel::InternalError,
-                  message: format!("Definition for {} should have been in scope", fn_name),
-                  span: ident.span,
-                });
+                self.internal_error(
+                  ident.span,
+                  &format!("Definition for {} should have been in scope", fn_name),
+                );

                return;
              }
@@ -561,16 +529,15 @@ impl ModuleCompiler {
            {
              Some(Value::Pointer(p)) => Some(p),
              lookup_result => {
-                self.diagnostics.push(Diagnostic {
-                  level: DiagnosticLevel::InternalError,
-                  message: format!(
+                self.internal_error(
+                  named.orig.span(),
+                  &format!(
                    "{} should have been a pointer, but it was {:?}, ref: {:?}",
                    orig_name,
                    lookup_result,
                    self.scope_analysis.refs.get(&orig_name.span)
                  ),
-                  span: named.orig.span(),
-                });
+                );

                None
              }
@@ -599,12 +566,7 @@ impl ModuleCompiler {
          let namespace_name = match &namespace.name {
            ModuleExportName::Ident(ident) => ident.sym.to_string(),
            ModuleExportName::Str(_) => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::InternalError,
-                message: "exporting a non-identifier".to_string(),
-                span: namespace.span,
-              });
-
+              self.internal_error(namespace.span, "exporting a non-identifier");
              "_todo_export_non_ident".to_string()
            }
          };
@@ -614,12 +576,7 @@ impl ModuleCompiler {
          let src = match &en.src {
            Some(src) => src.value.to_string(),
            None => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::InternalError,
-                message: "exporting a namespace without a source".to_string(),
-                span: namespace.span,
-              });
-
+              self.internal_error(namespace.span, "exporting a namespace without a source");
              "_error_export_namespace_without_src".to_string()
            }
          };
@@ -686,11 +643,10 @@ impl ModuleCompiler {
          {
            Some(Value::Pointer(p)) => p,
            _ => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::InternalError,
-                message: format!("Imported name {} should have been a pointer", local_name),
-                span: named.span,
-              });
+              self.internal_error(
+                named.span,
+                &format!("Imported name {} should have been a pointer", local_name),
+              );

              self.allocate_defn(local_name.as_str())
            }
@@ -722,11 +678,10 @@ impl ModuleCompiler {
          {
            Some(Value::Pointer(p)) => p,
            _ => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::InternalError,
-                message: format!("Imported name {} should have been a pointer", local_name),
-                span: default.span,
-              });
+              self.internal_error(
+                default.span,
+                &format!("Imported name {} should have been a pointer", local_name),
+              );

              self.allocate_defn(local_name.as_str())
            }
@@ -751,11 +706,10 @@ impl ModuleCompiler {
          {
            Some(Value::Pointer(p)) => p,
            _ => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::InternalError,
-                message: format!("Imported name {} should have been a pointer", local_name),
-                span: namespace.span,
-              });
+              self.internal_error(
+                namespace.span,
+                &format!("Imported name {} should have been a pointer", local_name),
+              );

              self.allocate_defn(local_name.as_str())
            }
@@ -807,11 +761,10 @@ impl ModuleCompiler {
      {
        Some(Value::Pointer(p)) => p,
        _ => {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::InternalError,
-            message: format!("Definition for {} should have been in scope", ident.sym),
-            span: class.span, // FIXME: make class_name ident and use that span
-          });
+          self.internal_error(
+            class.span, // FIXME: make class_name ident and use that span
+            &format!("Definition for {} should have been in scope", ident.sym),
+          );

          self.allocate_defn_numbered("_scope_error")
        }
--- a/valuescript_compiler/src/scope.rs
+++ b/valuescript_compiler/src/scope.rs
@@ -4,7 +4,7 @@ use swc_common::Spanned;

 use valuescript_common::BUILTIN_NAMES;

-use crate::diagnostic::{Diagnostic, DiagnosticLevel};
+use crate::diagnostic::Diagnostic;
 use crate::{asm::Builtin, constants::CONSTANTS};

 #[derive(Hash, PartialEq, Eq, Clone, Debug, PartialOrd, Ord)]
@@ -72,14 +72,13 @@ impl ScopeTrait for Scope {
    let old_mapping = self.borrow_mut().name_map.insert(name.clone(), name_id);

    if old_mapping.is_some() {
-      diagnostics.push(Diagnostic {
-        level: DiagnosticLevel::Error,
-        message: format!(
+      diagnostics.push(Diagnostic::error(
+        span,
+        &format!(
          "Scope overwrite of `{}` occurred (TODO: being permissive about this)",
          name
        ),
-        span,
-      });
+      ));
    }
  }

--- a/valuescript_compiler/src/scope_analysis.rs
+++ b/valuescript_compiler/src/scope_analysis.rs
@@ -9,12 +9,13 @@ use valuescript_common::BUILTIN_NAMES;
 use crate::{
  asm::{Builtin, Register, Value},
  constants::CONSTANTS,
+  diagnostic::{DiagnosticContainer, DiagnosticReporter},
  ident::Ident,
  name_allocator::{PointerAllocator, RegAllocator},
  scope::{init_std_scope, NameId, OwnerId, Scope, ScopeTrait},
 };

-use super::diagnostic::{Diagnostic, DiagnosticLevel};
+use super::diagnostic::Diagnostic;

 // TODO: Find a use for these or remove them
 #[derive(Clone, Debug)]
@@ -75,6 +76,12 @@ pub struct ScopeAnalysis {
  pub reg_allocators: HashMap<OwnerId, RegAllocator>,
 }

+impl DiagnosticContainer for ScopeAnalysis {
+  fn diagnostics_mut(&mut self) -> &mut Vec<Diagnostic> {
+    &mut self.diagnostics
+  }
+}
+
 impl ScopeAnalysis {
  pub fn run(module: &swc_ecma_ast::Module) -> ScopeAnalysis {
    let mut sa = ScopeAnalysis::default();
@@ -311,11 +318,7 @@ impl ScopeAnalysis {
    let name = match self.names.get_mut(name_id) {
      Some(name) => name,
      None => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: format!("Expected name_id in names: {:?}", name_id),
-          span: ref_,
-        });
+        self.internal_error(ref_, &format!("Expected name_id in names: {:?}", name_id));
        return;
      }
    };
@@ -375,25 +378,13 @@ impl ScopeAnalysis {
        }
        ModuleDecl::ExportAll(_) => {}
        ModuleDecl::TsImportEquals(ts_import_equals) => {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::Error,
-            message: "TsImportEquals is not supported".to_string(),
-            span: ts_import_equals.span,
-          });
+          self.not_supported(ts_import_equals.span, "TsImportEquals");
        }
        ModuleDecl::TsExportAssignment(ts_export_assignment) => {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::Error,
-            message: "TsExportAssignment is not supported".to_string(),
-            span: ts_export_assignment.span,
-          });
+          self.not_supported(ts_export_assignment.span, "TsExportAssignment");
        }
        ModuleDecl::TsNamespaceExport(ts_namespace_export) => {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::Error,
-            message: "TsNamespaceExport is not supported".to_string(),
-            span: ts_namespace_export.span,
-          });
+          self.not_supported(ts_namespace_export.span, "TsNamespaceExport");
        }
      },
      ModuleItem::Stmt(stmt) => {
@@ -440,11 +431,7 @@ impl ScopeAnalysis {

        match &named_specifier.orig {
          ModuleExportName::Ident(ident) => self.ident(scope, &Ident::from_swc_ident(ident)),
-          ModuleExportName::Str(_) => self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::InternalError,
-            message: "TODO: ModuleExportName::Str".to_string(),
-            span: export_specifier.span(),
-          }),
+          ModuleExportName::Str(_) => self.todo(export_specifier.span(), "ModuleExportName::Str"),
        }
      }
      Default(default_specifier) => {
@@ -452,11 +439,7 @@ impl ScopeAnalysis {
      }
      Namespace(namespace_specifier) => match &namespace_specifier.name {
        ModuleExportName::Ident(ident) => self.ident(scope, &Ident::from_swc_ident(ident)),
-        ModuleExportName::Str(_) => self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: ModuleExportName::Str".to_string(),
-          span: export_specifier.span(),
-        }),
+        ModuleExportName::Str(_) => self.todo(export_specifier.span(), "ModuleExportName::Str"),
      },
    }
  }
@@ -473,11 +456,9 @@ impl ScopeAnalysis {

    match decl {
      Decl::Class(class_decl) => {
-        self.class_(scope, &Some(class_decl.ident.clone()), &class_decl.class);
-      }
-      Decl::Fn(fn_decl) => {
-        self.function(scope, &Some(fn_decl.ident.clone()), &fn_decl.function);
+        self.class_(scope, &Some(class_decl.ident.clone()), &class_decl.class)
      }
+      Decl::Fn(fn_decl) => self.function(scope, &Some(fn_decl.ident.clone()), &fn_decl.function),
      Decl::Var(var_decl) => {
        for decl in &var_decl.decls {
          self.var_declarator(scope, decl);
@@ -496,13 +477,7 @@ impl ScopeAnalysis {
          }
        }
      }
-      Decl::TsModule(ts_module) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "TsModule declaration is not supported".to_string(),
-          span: ts_module.span,
-        });
-      }
+      Decl::TsModule(ts_module) => self.not_supported(ts_module.span, "TsModule declaration"),
    }
  }

@@ -821,20 +796,11 @@ impl ScopeAnalysis {
      Pat::Assign(assign_pat) => {
        self.get_pat_idents_impl(idents, &assign_pat.left);
      }
-      Pat::Expr(expr) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "Pattern expression not expected in this context".to_string(),
-          span: expr.span(),
-        });
-      }
-      Pat::Invalid(invalid) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Invalid pattern".to_string(),
-          span: invalid.span,
-        });
-      }
+      Pat::Expr(expr) => self.internal_error(
+        expr.span(),
+        "Pattern expression not expected in this context",
+      ),
+      Pat::Invalid(invalid) => self.error(invalid.span, "Invalid pattern"),
    }
  }

@@ -890,19 +856,9 @@ impl ScopeAnalysis {
        self.var_declarator_pat(scope, &assign.left);
        self.expr(scope, &assign.right);
      }
-      Pat::Invalid(invalid) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Invalid pattern".to_string(),
-          span: invalid.span,
-        });
-      }
+      Pat::Invalid(invalid) => self.error(invalid.span, "Invalid pattern"),
      Pat::Expr(expr) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "Pattern expression not expected in declarator".to_string(),
-          span: expr.span(),
-        });
+        self.internal_error(expr.span(), "Pattern expression not expected in declarator")
      }
    }
  }
@@ -1092,13 +1048,7 @@ impl ScopeAnalysis {
          self.expr(scope, arg);
        }
      }
-      Expr::Await(await_) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "await is not supported".to_string(),
-          span: await_.span,
-        });
-      }
+      Expr::Await(await_) => self.todo(await_.span, "async/await"),
      Expr::Member(member) => self.member(scope, member),
      Expr::Call(call) => self.call(scope, call),
      Expr::New(new) => {
@@ -1145,56 +1095,16 @@ impl ScopeAnalysis {
          OptChainBase::Member(member) => self.member(scope, member),
        }
      }
-      Expr::SuperProp(super_prop) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: super_prop".to_string(),
-          span: super_prop.span,
-        });
-      }
-      Expr::JSXMember(_) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: jsx_member".to_string(),
-          span: expr.span(),
-        });
-      }
-      Expr::JSXNamespacedName(_) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: jsx_namespaced_name".to_string(),
-          span: expr.span(),
-        });
-      }
+      Expr::SuperProp(super_prop) => self.todo(super_prop.span, "super_prop"),
      Expr::JSXEmpty(_) => {}
-      Expr::JSXElement(jsx_element) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: jsx_element".to_string(),
-          span: jsx_element.span,
-        });
-      }
-      Expr::JSXFragment(jsx_fragment) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: jsx_fragment".to_string(),
-          span: jsx_fragment.span,
-        });
-      }
+      Expr::JSXNamespacedName(_)
+      | Expr::JSXElement(_)
+      | Expr::JSXFragment(_)
+      | Expr::JSXMember(_) => self.todo(expr.span(), "JSX"),
      Expr::TsInstantiation(ts_instantiation) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: ts_instantiation".to_string(),
-          span: ts_instantiation.span,
-        });
-      }
-      Expr::PrivateName(private_name) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: private_name".to_string(),
-          span: private_name.span,
-        });
+        self.todo(ts_instantiation.span, "TsInstantiation")
      }
+      Expr::PrivateName(private_name) => self.todo(private_name.span, "PrivateName"),
    }
  }

@@ -1288,63 +1198,52 @@ impl ScopeAnalysis {
        self.mutate_expr(scope, &member.obj, optional);
      }
      Expr::Call(call) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Call expressions cannot be mutated".to_string(),
-          span: call.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          call.span,
+          "Call expressions cannot be mutated",
+        ));
      }
      Expr::New(new) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "New expressions cannot be mutated".to_string(),
-          span: new.span,
-        });
-      }
-      Expr::Paren(paren) => {
-        self.mutate_expr(scope, &paren.expr, optional);
+        diagnostic = Some(Diagnostic::error(
+          new.span,
+          "New expressions cannot be mutated",
+        ));
      }
+      Expr::Paren(paren) => self.mutate_expr(scope, &paren.expr, optional),
      Expr::Tpl(tpl) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Template literals cannot be mutated".to_string(),
-          span: tpl.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          tpl.span,
+          "Template literals cannot be mutated",
+        ));
      }
      Expr::TaggedTpl(tagged_tpl) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Tagged template literals cannot be mutated".to_string(),
-          span: tagged_tpl.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          tagged_tpl.span,
+          "Tagged template literals cannot be mutated",
+        ));
      }
      Expr::Arrow(arrow) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Arrow functions cannot be mutated".to_string(),
-          span: arrow.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          arrow.span,
+          "Arrow functions cannot be mutated",
+        ));
      }
      Expr::Class(class) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Class expressions cannot be mutated".to_string(),
-          span: class.class.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          class.class.span,
+          "Class expressions cannot be mutated",
+        ));
      }
      Expr::MetaProp(meta_prop) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Meta properties cannot be mutated".to_string(),
-          span: meta_prop.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          meta_prop.span,
+          "Meta properties cannot be mutated",
+        ));
      }
      Expr::Invalid(invalid) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Invalid expression".to_string(),
-          span: invalid.span,
-        });
+        self
+          .diagnostics
+          .push(Diagnostic::error(invalid.span, "Invalid expression"));
      }
      Expr::TsTypeAssertion(ts_type_assertion) => {
        self.mutate_expr(scope, &ts_type_assertion.expr, optional);
@@ -1359,156 +1258,53 @@ impl ScopeAnalysis {
        self.mutate_expr(scope, &as_expr.expr, optional);
      }
      Expr::OptChain(opt_chain) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Optional property accesses (a?.b) cannot be mutated".to_string(),
-          span: opt_chain.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          opt_chain.span,
+          "Optional property accesses (a?.b) cannot be mutated",
+        ));
      }

      Expr::This(this) => {
        self.mutate_ident(scope, &Ident::this(this.span), optional);
      }
      Expr::Array(array) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Mutating a (non-pattern) array expression is not valid. \
-            This is an unusual case that can occur with things like [a,b]+=c."
-            .to_string(),
-          span: array.span,
-        });
+        diagnostic = Some(Diagnostic::error(
+          array.span,
+          "Mutating a (non-pattern) array expression is not valid. This is an unusual case that \
+            can occur with things like [a,b]+=c.",
+        ));
      }
      Expr::Object(object) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Mutating a (non-pattern) object expression is not valid. \
-            This is an unusual case - it's not clear whether SWC ever emit it. \
+        diagnostic = Some(Diagnostic::error(
+          object.span,
+          "Mutating a (non-pattern) object expression is not valid. \
+            This is an unusual case - it's not clear whether SWC ever emits it. \
            Please consider creating an issue: \
-            https://github.com/ValueScript/issues/new."
-            .to_string(),
-          span: object.span,
-        });
-      }
-      Expr::Fn(fn_) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate fn".to_string(),
-          span: fn_.function.span,
-        });
-      }
-      Expr::Unary(unary) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate unary".to_string(),
-          span: unary.span,
-        });
-      }
-      Expr::Update(update) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate update".to_string(),
-          span: update.span,
-        });
-      }
-      Expr::Bin(bin) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate binary exp".to_string(),
-          span: bin.span,
-        });
-      }
-      Expr::Assign(assign) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate assignment".to_string(),
-          span: assign.span,
-        });
+            https://github.com/ValueScript/issues/new.",
+        ));
      }
+      Expr::Fn(fn_) => diagnostic = Some(Diagnostic::todo(fn_.span(), "mutate fn")),
+      Expr::Unary(unary) => diagnostic = Some(Diagnostic::todo(unary.span, "mutate unary")),
+      Expr::Update(update) => diagnostic = Some(Diagnostic::todo(update.span, "mutate update")),
+      Expr::Bin(bin) => diagnostic = Some(Diagnostic::todo(bin.span, "mutate binary exp")),
+      Expr::Assign(assign) => diagnostic = Some(Diagnostic::todo(assign.span, "mutate assignment")),
      Expr::SuperProp(super_prop) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate super_prop".to_string(),
-          span: super_prop.span,
-        });
-      }
-      Expr::Cond(cond) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate cond".to_string(),
-          span: cond.span,
-        });
-      }
-      Expr::Seq(seq) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate seq".to_string(),
-          span: seq.span,
-        });
-      }
-      Expr::Lit(_) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate lit".to_string(),
-          span: expr.span(),
-        });
-      }
-      Expr::Yield(yield_) => {
-        diagnostic = Some(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate yield".to_string(),
-          span: yield_.span,
-        });
-      }
-      Expr::Await(await_) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "TODO: mutate await".to_string(),
-          span: await_.span,
-        });
-      }
-      Expr::JSXMember(_) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate jsx_member".to_string(),
-          span: expr.span(),
-        });
-      }
-      Expr::JSXNamespacedName(_) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate jsx_namespaced_name".to_string(),
-          span: expr.span(),
-        });
+        diagnostic = Some(Diagnostic::todo(super_prop.span, "mutate super_prop"))
      }
+      Expr::Cond(cond) => diagnostic = Some(Diagnostic::todo(cond.span, "mutate cond")),
+      Expr::Seq(seq) => diagnostic = Some(Diagnostic::todo(seq.span, "mutate seq")),
+      Expr::Lit(_) => diagnostic = Some(Diagnostic::todo(expr.span(), "mutate lit")),
+      Expr::Yield(yield_) => diagnostic = Some(Diagnostic::todo(yield_.span, "mutate yield")),
+      Expr::Await(await_) => self.todo(await_.span, "mutate await"),
+      Expr::JSXMember(_) => self.todo(expr.span(), "mutate jsx_member"),
+      Expr::JSXNamespacedName(_) => self.todo(expr.span(), "mutate jsx_namespaced_name"),
      Expr::JSXEmpty(_) => {}
-      Expr::JSXElement(jsx_element) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate jsx_element".to_string(),
-          span: jsx_element.span,
-        });
-      }
-      Expr::JSXFragment(jsx_fragment) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate jsx_fragment".to_string(),
-          span: jsx_fragment.span,
-        });
-      }
+      Expr::JSXElement(jsx_element) => self.todo(jsx_element.span, "mutate jsx_element"),
+      Expr::JSXFragment(jsx_fragment) => self.todo(jsx_fragment.span, "mutate jsx_fragment"),
      Expr::TsInstantiation(ts_instantiation) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate ts_instantiation".to_string(),
-          span: ts_instantiation.span,
-        });
-      }
-      Expr::PrivateName(private_name) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "TODO: mutate private_name".to_string(),
-          span: private_name.span,
-        });
+        self.todo(ts_instantiation.span, "mutate ts_instantiation")
      }
+      Expr::PrivateName(private_name) => self.todo(private_name.span, "mutate private_name"),
    }

    if !optional {
@@ -1563,16 +1359,8 @@ impl ScopeAnalysis {
        self.mutate_pat(scope, &assign_pat.left);
        self.expr(scope, &assign_pat.right);
      }
-      Pat::Invalid(invalid) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Invalid pattern".to_string(),
-          span: invalid.span,
-        });
-      }
-      Pat::Expr(expr) => {
-        self.mutate_expr(scope, expr, false);
-      }
+      Pat::Invalid(invalid) => self.error(invalid.span, "Invalid pattern"),
+      Pat::Expr(expr) => self.mutate_expr(scope, expr, false),
    }
  }

@@ -1580,11 +1368,7 @@ impl ScopeAnalysis {
    let name_id = match scope.get(&ident.sym) {
      Some(name_id) => name_id,
      None => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Unresolved reference".to_string(),
-          span: ident.span,
-        });
+        self.error(ident.span, "Unresolved reference");
        return;
      }
    };
@@ -1592,11 +1376,7 @@ impl ScopeAnalysis {
    let name = match self.names.get_mut(&name_id) {
      Some(name) => name,
      None => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "Expected name_id in names".to_string(),
-          span: ident.span,
-        });
+        self.internal_error(ident.span, "Expected name_id in names");
        return;
      }
    };
@@ -1622,11 +1402,7 @@ impl ScopeAnalysis {
    let name_id = match scope.get(&ident.sym) {
      Some(name_id) => name_id,
      None => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Unresolved reference".to_string(),
-          span: ident.span,
-        });
+        self.error(ident.span, "Unresolved reference");
        return;
      }
    };
@@ -1643,11 +1419,7 @@ impl ScopeAnalysis {
    let name = match self.names.get(&name_id) {
      Some(name) => name.clone(),
      None => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "Expected name_id in names".to_string(),
-          span: ident.span,
-        });
+        self.internal_error(ident.span, "Expected name_id in names");
        return;
      }
    };
@@ -1701,11 +1473,10 @@ impl ScopeAnalysis {
          self.function(scope, &None, &method.function);
        }
        swc_ecma_ast::Prop::Assign(assign) => {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::InternalError,
-            message: "TODO: implement property assignments (what are these?)".to_string(),
-            span: assign.key.span, // TODO: Proper span of assign
-          });
+          self.todo(
+            assign.key.span, // TODO: Proper span of assign
+            "implement property assignments (what are these?)",
+          );
        }
      },
      PropOrSpread::Spread(spread) => {
@@ -1723,13 +1494,7 @@ impl ScopeAnalysis {
      }
      Stmt::Empty(_) => {}
      Stmt::Debugger(_) => {}
-      Stmt::With(with) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Not supported: with statement".to_string(),
-          span: with.span,
-        });
-      }
+      Stmt::With(with) => self.not_supported(with.span, "with statements"),
      Stmt::Return(return_) => {
        if let Some(arg) = &return_.arg {
          self.expr(scope, arg);
@@ -1915,13 +1680,7 @@ impl ScopeAnalysis {
      Pat::Expr(expr) => {
        self.expr(scope, expr);
      }
-      Pat::Invalid(invalid) => {
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          message: "Invalid pattern".to_string(),
-          span: invalid.span,
-        });
-      }
+      Pat::Invalid(invalid) => self.error(invalid.span, "Invalid pattern"),
    }
  }

@@ -1937,32 +1696,29 @@ impl ScopeAnalysis {
        if name.type_ == NameType::Let {
          match name_id {
            NameId::Span(span) => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::Lint,
-                message: format!(
+              self.diagnostics.push(Diagnostic::lint(
+                *span,
+                &format!(
                  "`{}` should be declared using `const` because it is implicitly \
                  const due to capture",
                  name.sym
                ),
-                span: *span,
-              });
+              ));
            }
            NameId::This(_) | NameId::Builtin(_) | NameId::Constant(_) => {
-              self.diagnostics.push(Diagnostic {
-                level: DiagnosticLevel::InternalError,
-                message: "Builtin/constant/this should not have type_ let".to_string(),
-                span: swc_common::DUMMY_SP,
-              });
+              self.diagnostics.push(Diagnostic::internal_error(
+                swc_common::DUMMY_SP,
+                "Builtin/constant/this should not have type_ let",
+              ));
            }
          }
        }

        for mutation in &name.mutations {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::Error,
-            message: format!("Cannot mutate captured variable `{}`", name.sym),
-            span: *mutation,
-          });
+          self.diagnostics.push(Diagnostic::error(
+            *mutation,
+            &format!("Cannot mutate captured variable `{}`", name.sym),
+          ));
        }
      }
    }
@@ -1973,11 +1729,10 @@ impl ScopeAnalysis {
      Some(name) => name,
      None => {
        // TODO: Add a name lookup helper that does this diagnostic
-        self.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::InternalError,
-          message: "NameId not found".to_string(),
-          span: swc_common::DUMMY_SP,
-        });
+        self.diagnostics.push(Diagnostic::internal_error(
+          swc_common::DUMMY_SP,
+          "NameId not found",
+        ));

        return None;
      }
@@ -1987,11 +1742,10 @@ impl ScopeAnalysis {
      match name_id {
        NameId::Span(span) => Some(OwnerId::Span(*span)),
        NameId::This(_) => {
-          self.diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::InternalError,
-            message: "NameId::This should not be associated with NameType::Function".to_string(),
-            span: swc_common::DUMMY_SP,
-          });
+          self.diagnostics.push(Diagnostic::internal_error(
+            swc_common::DUMMY_SP,
+            "NameId::This should not be associated with NameType::Function",
+          ));

          None
        }
@@ -2091,11 +1845,10 @@ impl ScopeAnalysis {

      if name.effectively_const {
        for mutation in &name.mutations {
-          diagnostics.push(Diagnostic {
-            level: DiagnosticLevel::Error,
-            message: format!("Cannot mutate const {}", name.sym),
-            span: *mutation,
-          });
+          diagnostics.push(Diagnostic::error(
+            *mutation,
+            &format!("Cannot mutate const {}", name.sym),
+          ));
        }
      }
    }
@@ -2156,14 +1909,13 @@ impl ScopeAnalysis {
        continue;
      }

-      diagnostics.push(Diagnostic {
-        level: DiagnosticLevel::Error,
-        message: format!(
+      diagnostics.push(Diagnostic::error(
+        ref_.span,
+        &format!(
          "Referencing {} is invalid before its declaration (temporal dead zone)",
          name.sym,
        ),
-        span: ref_.span,
-      });
+      ));
    }

    self.diagnostics.append(&mut diagnostics);
--- a/valuescript_compiler/src/target_accessor.rs
+++ b/valuescript_compiler/src/target_accessor.rs
@@ -2,9 +2,9 @@ use std::mem::take;

 use crate::{
  asm::{Instruction, Register, Value},
+  diagnostic::DiagnosticReporter,
  expression_compiler::{CompiledExpression, ExpressionCompiler},
  ident::Ident as CrateIdent,
-  Diagnostic, DiagnosticLevel,
 };
 use swc_common::Spanned;

@@ -90,11 +90,10 @@ impl TargetAccessor {
        TargetAccessor::make_todo(ec)
      }
      _ => {
-        ec.fnc.diagnostics.push(Diagnostic {
-          level: DiagnosticLevel::Error,
-          span: expr.span(),
-          message: format!("Invalid target {}", get_expr_type_str(expr)),
-        });
+        ec.error(
+          expr.span(),
+          &format!("Invalid target {}", get_expr_type_str(expr)),
+        );

        TargetAccessor::make_bad(ec)
      }