Use Default to handle missing entry in sequence (#836)

This uses a more complex but ultimately more correct approach that fixes
#672. By creating default error kinds for several of the AST and HIR
types that can appear in sequences, sequence parsing recovery now
inserts error entries to correspond to places where commas appear at the
start of a sequence or back-to-back in the sequence. The end result is
better signature help due to recovered HIR including placeholder error
entries.

compiler/qsc_ast/src/ast.rs

@@ -7,7 +7,7 @@
 
 use indenter::{indented, Format, Indented};
 use num_bigint::BigInt;
-use qsc_data_structures::span::Span;
+use qsc_data_structures::span::{Span, WithSpan};
 use std::{
     cmp::Ordering,
     fmt::{self, Display, Formatter, Write},
@@ -107,6 +107,22 @@ impl Hash for NodeId {
     }
 }
 
+/// Trait that allows creation of a default value with a span.
+pub trait DefaultWithSpan {
+    /// Creates a default value with the given span by using the `Default` and `WithSpan` traits.
+    #[must_use]
+    fn default_with_span(span: Span) -> Self;
+}
+
+impl<T> DefaultWithSpan for Box<T>
+where
+    T: Default + WithSpan,
+{
+    fn default_with_span(span: Span) -> Self {
+        Box::new(T::default().with_span(span))
+    }
+}
+
 /// The root node of an AST.
 #[derive(Clone, Debug, Default, PartialEq)]
 pub struct Package {
@@ -317,7 +333,7 @@ impl Display for Attr {
 }
 
 /// A type definition.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Default)]
 pub struct TyDef {
     /// The node ID.
     pub id: NodeId,
@@ -333,15 +349,24 @@ impl Display for TyDef {
     }
 }
 
+impl WithSpan for TyDef {
+    fn with_span(self, span: Span) -> Self {
+        Self { span, ..self }
+    }
+}
+
 /// A type definition kind.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Default)]
 pub enum TyDefKind {
     /// A field definition with an optional name but required type.
     Field(Option<Box<Ident>>, Box<Ty>),
     /// A parenthesized type definition.
     Paren(Box<TyDef>),
     /// A tuple.
     Tuple(Box<[Box<TyDef>]>),
+    /// An invalid type definition.
+    #[default]
+    Err,
 }
 
 impl Display for TyDefKind {
@@ -372,6 +397,7 @@ impl Display for TyDefKind {
                     }
                 }
             }
+            TyDefKind::Err => write!(indent, "Err")?,
         }
         Ok(())
     }
@@ -541,7 +567,7 @@ impl Display for FunctorExprKind {
 }
 
 /// A type.
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)]
 pub struct Ty {
     /// The node ID.
     pub id: NodeId,
@@ -557,8 +583,22 @@ impl Display for Ty {
     }
 }
 
+impl WithSpan for Ty {
+    fn with_span(self, span: Span) -> Self {
+        Self { span, ..self }
+    }
+}
+
+impl DefaultWithSpan for Ty {
+    /// Creates a default value with the given span by using the `Default` and `WithSpan` traits.
+    #[must_use]
+    fn default_with_span(span: Span) -> Self {
+        Self::default().with_span(span)
+    }
+}
+
 /// A type kind.
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)]
 pub enum TyKind {
     /// An array type.
     Array(Box<Ty>),
@@ -574,6 +614,9 @@ pub enum TyKind {
     Param(Box<Ident>),
     /// A tuple type.
     Tuple(Box<[Ty]>),
+    /// An invalid type.
+    #[default]
+    Err,
 }
 
 impl Display for TyKind {
@@ -607,6 +650,7 @@ impl Display for TyKind {
                     }
                 }
             }
+            TyKind::Err => write!(indent, "Err")?,
         }
         Ok(())
     }
@@ -722,6 +766,12 @@ impl Display for Expr {
     }
 }
 
+impl WithSpan for Expr {
+    fn with_span(self, span: Span) -> Self {
+        Self { span, ..self }
+    }
+}
+
 /// An expression kind.
 #[derive(Clone, Debug, Default, PartialEq)]
 pub enum ExprKind {
@@ -1082,7 +1132,7 @@ pub enum StringComponent {
 }
 
 /// A pattern.
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)]
 pub struct Pat {
     /// The node ID.
     pub id: NodeId,
@@ -1098,8 +1148,14 @@ impl Display for Pat {
     }
 }
 
+impl WithSpan for Pat {
+    fn with_span(self, span: Span) -> Self {
+        Self { span, ..self }
+    }
+}
+
 /// A pattern kind.
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)]
 pub enum PatKind {
     /// A binding with an optional type annotation.
     Bind(Box<Ident>, Option<Box<Ty>>),
@@ -1111,6 +1167,9 @@ pub enum PatKind {
     Paren(Box<Pat>),
     /// A tuple: `(a, b, c)`.
     Tuple(Box<[Box<Pat>]>),
+    /// An invalid pattern.
+    #[default]
+    Err,
 }
 
 impl Display for PatKind {
@@ -1150,13 +1209,14 @@ impl Display for PatKind {
                     }
                 }
             }
+            PatKind::Err => write!(indent, "Err")?,
         }
         Ok(())
     }
 }
 
 /// A qubit initializer.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Default)]
 pub struct QubitInit {
     /// The node ID.
     pub id: NodeId,
@@ -1172,8 +1232,14 @@ impl Display for QubitInit {
     }
 }
 
+impl WithSpan for QubitInit {
+    fn with_span(self, span: Span) -> Self {
+        Self { span, ..self }
+    }
+}
+
 /// A qubit initializer kind.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, Default)]
 pub enum QubitInitKind {
     /// An array of qubits: `Qubit[a]`.
     Array(Box<Expr>),
@@ -1183,6 +1249,9 @@ pub enum QubitInitKind {
     Single,
     /// A tuple: `(a, b, c)`.
     Tuple(Box<[Box<QubitInit>]>),
+    /// An invalid initializer.
+    #[default]
+    Err,
 }
 
 impl Display for QubitInitKind {
@@ -1211,13 +1280,14 @@ impl Display for QubitInitKind {
                     }
                 }
             }
+            QubitInitKind::Err => write!(indent, "Err")?,
         }
         Ok(())
     }
 }
 
 /// A path to a declaration.
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)]
 pub struct Path {
     /// The node ID.
     pub id: NodeId,
@@ -1240,6 +1310,12 @@ impl Display for Path {
     }
 }
 
+impl WithSpan for Path {
+    fn with_span(self, span: Span) -> Self {
+        Self { span, ..self }
+    }
+}
+
 /// An identifier.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub struct Ident {
@@ -1251,6 +1327,22 @@ pub struct Ident {
     pub name: Rc<str>,
 }
 
+impl Default for Ident {
+    fn default() -> Self {
+        Ident {
+            id: NodeId::default(),
+            span: Span::default(),
+            name: "".into(),
+        }
+    }
+}
+
+impl WithSpan for Ident {
+    fn with_span(self, span: Span) -> Self {
+        Self { span, ..self }
+    }
+}
+
 impl Display for Ident {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         write!(f, "Ident {} {} \"{}\"", self.id, self.span, self.name)

compiler/qsc_ast/src/mut_visit.rs

@@ -130,6 +130,7 @@ pub fn walk_ty_def(vis: &mut impl MutVisitor, def: &mut TyDef) {
         }
         TyDefKind::Paren(def) => vis.visit_ty_def(def),
         TyDefKind::Tuple(defs) => defs.iter_mut().for_each(|d| vis.visit_ty_def(d)),
+        TyDefKind::Err => {}
     }
 }
 
@@ -184,7 +185,7 @@ pub fn walk_ty(vis: &mut impl MutVisitor, ty: &mut Ty) {
             vis.visit_ty(rhs);
             functors.iter_mut().for_each(|f| vis.visit_functor_expr(f));
         }
-        TyKind::Hole => {}
+        TyKind::Hole | TyKind::Err => {}
         TyKind::Paren(ty) => vis.visit_ty(ty),
         TyKind::Param(name) => vis.visit_ident(name),
         TyKind::Path(path) => vis.visit_path(path),
@@ -313,7 +314,7 @@ pub fn walk_pat(vis: &mut impl MutVisitor, pat: &mut Pat) {
             ty.iter_mut().for_each(|t| vis.visit_ty(t));
         }
         PatKind::Discard(ty) => ty.iter_mut().for_each(|t| vis.visit_ty(t)),
-        PatKind::Elided => {}
+        PatKind::Elided | PatKind::Err => {}
         PatKind::Paren(pat) => vis.visit_pat(pat),
         PatKind::Tuple(pats) => pats.iter_mut().for_each(|p| vis.visit_pat(p)),
     }
@@ -325,7 +326,7 @@ pub fn walk_qubit_init(vis: &mut impl MutVisitor, init: &mut QubitInit) {
     match &mut *init.kind {
         QubitInitKind::Array(len) => vis.visit_expr(len),
         QubitInitKind::Paren(init) => vis.visit_qubit_init(init),
-        QubitInitKind::Single => {}
+        QubitInitKind::Single | QubitInitKind::Err => {}
         QubitInitKind::Tuple(inits) => inits.iter_mut().for_each(|i| vis.visit_qubit_init(i)),
     }
 }

compiler/qsc_ast/src/visit.rs

@@ -117,6 +117,7 @@ pub fn walk_ty_def<'a>(vis: &mut impl Visitor<'a>, def: &'a TyDef) {
         }
         TyDefKind::Paren(def) => vis.visit_ty_def(def),
         TyDefKind::Tuple(defs) => defs.iter().for_each(|d| vis.visit_ty_def(d)),
+        TyDefKind::Err => {}
     }
 }
 
@@ -161,7 +162,7 @@ pub fn walk_ty<'a>(vis: &mut impl Visitor<'a>, ty: &'a Ty) {
             vis.visit_ty(rhs);
             functors.iter().for_each(|f| vis.visit_functor_expr(f));
         }
-        TyKind::Hole => {}
+        TyKind::Hole | TyKind::Err => {}
         TyKind::Paren(ty) => vis.visit_ty(ty),
         TyKind::Path(path) => vis.visit_path(path),
         TyKind::Param(name) => vis.visit_ident(name),
@@ -283,7 +284,7 @@ pub fn walk_pat<'a>(vis: &mut impl Visitor<'a>, pat: &'a Pat) {
             ty.iter().for_each(|t| vis.visit_ty(t));
         }
         PatKind::Discard(ty) => ty.iter().for_each(|t| vis.visit_ty(t)),
-        PatKind::Elided => {}
+        PatKind::Elided | PatKind::Err => {}
         PatKind::Paren(pat) => vis.visit_pat(pat),
         PatKind::Tuple(pats) => pats.iter().for_each(|p| vis.visit_pat(p)),
     }
@@ -293,7 +294,7 @@ pub fn walk_qubit_init<'a>(vis: &mut impl Visitor<'a>, init: &'a QubitInit) {
     match &*init.kind {
         QubitInitKind::Array(len) => vis.visit_expr(len),
         QubitInitKind::Paren(init) => vis.visit_qubit_init(init),
-        QubitInitKind::Single => {}
+        QubitInitKind::Single | QubitInitKind::Err => {}
         QubitInitKind::Tuple(inits) => inits.iter().for_each(|i| vis.visit_qubit_init(i)),
     }
 }

compiler/qsc_data_structures/src/span.rs

@@ -74,3 +74,9 @@ impl From<Span> for SourceSpan {
         Self::from((value.lo as usize)..(value.hi as usize))
     }
 }
+
+#[allow(clippy::module_name_repetitions)]
+pub trait WithSpan {
+    #[must_use]
+    fn with_span(self, span: Span) -> Self;
+}

compiler/qsc_eval/src/lower.rs

@@ -213,6 +213,7 @@ impl Lowerer {
             hir::PatKind::Tuple(elems) => {
                 fir::PatKind::Tuple(elems.iter().map(|pat| self.lower_pat(pat)).collect())
             }
+            hir::PatKind::Err => unreachable!("error pat should not be present"),
         };
 
         let pat = fir::Pat { id, span, ty, kind };
@@ -389,6 +390,7 @@ impl Lowerer {
             hir::PatKind::Tuple(items) => {
                 fir::PatKind::Tuple(items.iter().map(|i| self.lower_pat(i)).collect())
             }
+            hir::PatKind::Err => unreachable!("error pat should not be present"),
         };
 
         let pat = fir::Pat {
@@ -410,6 +412,7 @@ impl Lowerer {
             hir::QubitInitKind::Tuple(items) => {
                 fir::QubitInitKind::Tuple(items.iter().map(|i| self.lower_qubit_init(i)).collect())
             }
+            hir::QubitInitKind::Err => unreachable!("error qubit init should not be present"),
         };
 
         fir::QubitInit {

compiler/qsc_frontend/src/lower.rs

@@ -666,6 +666,7 @@ impl With<'_> {
             ast::PatKind::Tuple(items) => {
                 hir::PatKind::Tuple(items.iter().map(|i| self.lower_pat(i)).collect())
             }
+            ast::PatKind::Err => hir::PatKind::Err,
         };
 
         hir::Pat {
@@ -692,6 +693,7 @@ impl With<'_> {
             ast::QubitInitKind::Tuple(items) => {
                 hir::QubitInitKind::Tuple(items.iter().map(|i| self.lower_qubit_init(i)).collect())
             }
+            ast::QubitInitKind::Err => hir::QubitInitKind::Err,
         };
 
         hir::QubitInit {