Plumbed spans through alias definitions, allowing for more precise er…

numbat/src/decorator.rs

@@ -1,46 +1,74 @@
-use crate::{prefix_parser::AcceptsPrefix, unit::CanonicalName};
+use crate::{prefix_parser::AcceptsPrefix, span::Span, unit::CanonicalName};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Decorator<'a> {
     MetricPrefixes,
     BinaryPrefixes,
-    Aliases(Vec<(&'a str, Option<AcceptsPrefix>)>),
+    Aliases(Vec<(&'a str, Option<AcceptsPrefix>, Span)>),
     Url(String),
     Name(String),
     Description(String),
 }
 
-pub fn name_and_aliases<'a>(
+/// Get an iterator of data computed from a name and/or its alias's `AcceptsPrefix` and
+/// `Span`. If `name` itself is in the list of aliases, then it (or more precisely, the
+/// data computed from it) will be placed at the front of the iterator
+///
+/// `f` says how to turn a triple of data associated with `name` or an alias, `(name,
+/// accepts_prefix, Option<span>)`, into a `T`. The generality is really just here to
+/// decide whether to yield `(&'a String, AcceptsPrefix)` or a `(&'a String,
+/// AcceptsPrefix, Span)`.
+fn name_and_aliases_inner<'a, T: 'a>(
     name: &'a str,
-    decorators: &[Decorator<'a>],
-) -> Box<dyn Iterator<Item = (&'a str, AcceptsPrefix)> + 'a> {
-    let aliases = {
-        let mut aliases_vec = vec![];
-        for decorator in decorators {
-            if let Decorator::Aliases(aliases) = decorator {
-                aliases_vec = aliases
-                    .iter()
-                    .map(|(name, accepts_prefix)| {
-                        (*name, accepts_prefix.unwrap_or(AcceptsPrefix::only_long()))
-                    })
-                    .collect();
+    decorators: &'a [Decorator],
+    f: impl 'a + Fn(&'a str, AcceptsPrefix, Option<Span>) -> T,
+) -> impl 'a + Iterator<Item = T> {
+    // contains all the aliases of `name`, starting with `name` itself
+    let mut aliases_vec = vec![f(name, AcceptsPrefix::only_long(), None)];
+
+    for decorator in decorators {
+        if let Decorator::Aliases(aliases) = decorator {
+            for (n, ap, span) in aliases {
+                let ap = ap.unwrap_or(AcceptsPrefix::only_long());
+                if *n == name {
+                    // use the AcceptsPrefix from the alias, but the span from `name`
+                    // itself; this way we always report a conflicting `name` first
+                    // before reporting any of its aliases. in effect we swallow aliases
+                    // equal to `name` itself (but keep their metadata)
+                    aliases_vec[0] = f(n, ap, None);
+                } else {
+                    aliases_vec.push(f(n, ap, Some(*span)));
+                }
             }
         }
-        aliases_vec
-    };
-
-    if !aliases.iter().any(|(n, _)| n == &name) {
-        let name_iter = std::iter::once((name, AcceptsPrefix::only_long()));
-        Box::new(name_iter.chain(aliases))
-    } else {
-        Box::new(aliases.into_iter())
     }
+
+    aliases_vec.into_iter()
+}
+
+/// Returns iterator of `(name_or_alias, accepts_prefix)` for the given name
+pub fn name_and_aliases<'a>(
+    name: &'a str,
+    decorators: &'a [Decorator],
+) -> impl 'a + Iterator<Item = (&'a str, AcceptsPrefix)> {
+    name_and_aliases_inner(name, decorators, |n, accepts_prefix, _| (n, accepts_prefix))
+}
+
+/// Returns iterator of `(name_or_alias, accepts_prefix, span)` for the given name
+pub fn name_and_aliases_spans<'a>(
+    name: &'a str,
+    name_span: Span,
+    decorators: &'a [Decorator],
+) -> impl 'a + Iterator<Item = (&'a str, AcceptsPrefix, Span)> {
+    name_and_aliases_inner(name, decorators, move |n, accepts_prefix, span| {
+        (n, accepts_prefix, span.unwrap_or(name_span))
+    })
 }
 
 pub fn get_canonical_unit_name(unit_name: &str, decorators: &[Decorator]) -> CanonicalName {
     for decorator in decorators {
         if let Decorator::Aliases(aliases) = decorator {
-            for (alias, accepts_prefix) in aliases {
+            for (alias, accepts_prefix, _) in aliases {
                 match accepts_prefix {
                     &Some(ap) if ap.short => {
                         return CanonicalName::new(alias, ap);
@@ -92,7 +120,7 @@ pub fn description(decorators: &[Decorator]) -> Option<String> {
 pub fn contains_aliases_with_prefixes(decorates: &[Decorator]) -> bool {
     for decorator in decorates {
         if let Decorator::Aliases(aliases) = decorator {
-            if aliases.iter().any(|(_, prefixes)| prefixes.is_some()) {
+            if aliases.iter().any(|(_, prefixes, _)| prefixes.is_some()) {
                 return true;
             }
         }

numbat/src/parser.rs

@@ -369,14 +369,17 @@ impl<'a> Parser<'a> {
     fn list_of_aliases(
         &mut self,
         tokens: &[Token<'a>],
-    ) -> Result<Vec<(&'a str, Option<AcceptsPrefix>)>> {
+    ) -> Result<Vec<(&'a str, Option<AcceptsPrefix>, Span)>> {
         if self.match_exact(tokens, TokenKind::RightParen).is_some() {
             return Ok(vec![]);
         }
 
-        let mut identifiers = vec![(self.identifier(tokens)?, self.accepts_prefix(tokens)?)];
+        let span = self.peek(tokens).span;
+        let mut identifiers = vec![(self.identifier(tokens)?, self.accepts_prefix(tokens)?, span)];
+
         while self.match_exact(tokens, TokenKind::Comma).is_some() {
-            identifiers.push((self.identifier(tokens)?, self.accepts_prefix(tokens)?));
+            let span = self.peek(tokens).span;
+            identifiers.push((self.identifier(tokens)?, self.accepts_prefix(tokens)?, span));
         }
 
         if self.match_exact(tokens, TokenKind::RightParen).is_none() {
@@ -1987,9 +1990,12 @@ mod tests {
     use std::fmt::Write;
 
     use super::*;
-    use crate::ast::{
-        binop, boolean, conditional, factorial, identifier, list, logical_neg, negate, scalar,
-        struct_, ReplaceSpans,
+    use crate::{
+        ast::{
+            binop, boolean, conditional, factorial, identifier, list, logical_neg, negate, scalar,
+            struct_, ReplaceSpans,
+        },
+        span::ByteIndex,
     };
 
     #[track_caller]
@@ -2438,7 +2444,26 @@ mod tests {
                 )),
                 decorators: vec![
                     decorator::Decorator::Name("myvar".into()),
-                    decorator::Decorator::Aliases(vec![("foo", None), ("bar", None)]),
+                    decorator::Decorator::Aliases(vec![
+                        (
+                            "foo",
+                            None,
+                            Span {
+                                start: ByteIndex(24),
+                                end: ByteIndex(27),
+                                code_source_id: 0,
+                            },
+                        ),
+                        (
+                            "bar",
+                            None,
+                            Span {
+                                start: ByteIndex(29),
+                                end: ByteIndex(32),
+                                code_source_id: 0,
+                            },
+                        ),
+                    ]),
                 ],
             }),
         );

numbat/src/prefix_parser.rs

@@ -52,6 +52,25 @@ impl AcceptsPrefix {
     }
 }
 
+/// The spans associated with an alias passed to `@aliases`
+#[derive(Debug, Clone, Copy)]
+pub(crate) struct AliasSpanInfo {
+    /// The span of the name to which the alias refers
+    pub(crate) name_span: Span,
+    /// The span of the alias itself (in an `@aliases` decorator)
+    pub(crate) alias_span: Span,
+}
+
+impl AliasSpanInfo {
+    #[cfg(test)]
+    fn dummy() -> Self {
+        Self {
+            name_span: Span::dummy(),
+            alias_span: Span::dummy(),
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 struct UnitInfo {
     definition_span: Span,
@@ -152,23 +171,23 @@ impl PrefixParser {
     fn ensure_name_is_available(
         &self,
         name: &str,
-        conflict_span: Span,
+        definition_span: Span,
         clash_with_other_identifiers: bool,
     ) -> Result<()> {
         if self.reserved_identifiers.contains(&name) {
-            return Err(NameResolutionError::ReservedIdentifier(conflict_span));
+            return Err(NameResolutionError::ReservedIdentifier(definition_span));
         }
 
         if clash_with_other_identifiers {
             if let Some(original_span) = self.other_identifiers.get(name) {
-                return Err(self.identifier_clash_error(name, conflict_span, *original_span));
+                return Err(self.identifier_clash_error(name, definition_span, *original_span));
             }
         }
 
         match self.parse(name) {
             PrefixParserResult::Identifier(_) => Ok(()),
             PrefixParserResult::UnitIdentifier(original_span, _, _, _) => {
-                Err(self.identifier_clash_error(name, conflict_span, original_span))
+                Err(self.identifier_clash_error(name, definition_span, original_span))
             }
         }
     }
@@ -180,9 +199,12 @@ impl PrefixParser {
         metric: bool,
         binary: bool,
         full_name: &str,
-        definition_span: Span,
+        AliasSpanInfo {
+            name_span,
+            alias_span,
+        }: AliasSpanInfo,
     ) -> Result<()> {
-        self.ensure_name_is_available(unit_name, definition_span, true)?;
+        self.ensure_name_is_available(unit_name, alias_span, true)?;
 
         for (prefix_long, prefixes_short, prefix) in Self::prefixes() {
             if !(prefix.is_metric() && metric || prefix.is_binary() && binary) {
@@ -192,23 +214,23 @@ impl PrefixParser {
             if accepts_prefix.long {
                 self.ensure_name_is_available(
                     &format!("{prefix_long}{unit_name}"),
-                    definition_span,
+                    alias_span,
                     true,
                 )?;
             }
             if accepts_prefix.short {
                 for prefix_short in *prefixes_short {
                     self.ensure_name_is_available(
                         &format!("{prefix_short}{unit_name}"),
-                        definition_span,
+                        alias_span,
                         true,
                     )?;
                 }
             }
         }
 
         let unit_info = UnitInfo {
-            definition_span,
+            definition_span: name_span,
             accepts_prefix,
             metric_prefixes: metric,
             binary_prefixes: binary,
@@ -294,7 +316,7 @@ mod tests {
                 true,
                 false,
                 "meter",
-                Span::dummy(),
+                AliasSpanInfo::dummy(),
             )
             .unwrap();
         prefix_parser
@@ -304,7 +326,7 @@ mod tests {
                 true,
                 false,
                 "meter",
-                Span::dummy(),
+                AliasSpanInfo::dummy(),
             )
             .unwrap();
 
@@ -315,7 +337,7 @@ mod tests {
                 true,
                 true,
                 "byte",
-                Span::dummy(),
+                AliasSpanInfo::dummy(),
             )
             .unwrap();
         prefix_parser
@@ -325,7 +347,7 @@ mod tests {
                 true,
                 true,
                 "byte",
-                Span::dummy(),
+                AliasSpanInfo::dummy(),
             )
             .unwrap();
 
@@ -336,7 +358,7 @@ mod tests {
                 false,
                 false,
                 "me",
-                Span::dummy(),
+                AliasSpanInfo::dummy(),
             )
             .unwrap();
 

numbat/src/prefix_transformer.rs

@@ -2,7 +2,7 @@ use crate::{
     ast::{DefineVariable, Expression, Statement, StringPart},
     decorator::{self, Decorator},
     name_resolution::NameResolutionError,
-    prefix_parser::{PrefixParser, PrefixParserResult},
+    prefix_parser::{AliasSpanInfo, PrefixParser, PrefixParserResult},
     span::Span,
 };
 
@@ -135,20 +135,26 @@ impl Transformer {
     pub(crate) fn register_name_and_aliases(
         &mut self,
         name: &str,
+        name_span: Span,
         decorators: &[Decorator],
-        conflict_span: Span,
     ) -> Result<()> {
         let mut unit_names = vec![];
         let metric_prefixes = Self::has_decorator(decorators, Decorator::MetricPrefixes);
         let binary_prefixes = Self::has_decorator(decorators, Decorator::BinaryPrefixes);
-        for (alias, accepts_prefix) in decorator::name_and_aliases(name, decorators) {
+
+        for (alias, accepts_prefix, alias_span) in
+            decorator::name_and_aliases_spans(name, name_span, decorators)
+        {
             self.prefix_parser.add_unit(
                 alias,
                 accepts_prefix,
                 metric_prefixes,
                 binary_prefixes,
                 name,
-                conflict_span,
+                AliasSpanInfo {
+                    name_span,
+                    alias_span,
+                },
             )?;
             unit_names.push(alias.to_string());
         }
@@ -189,7 +195,7 @@ impl Transformer {
         Ok(match statement {
             Statement::Expression(expr) => Statement::Expression(self.transform_expression(expr)),
             Statement::DefineBaseUnit(span, name, dexpr, decorators) => {
-                self.register_name_and_aliases(name, &decorators, span)?;
+                self.register_name_and_aliases(name, span, &decorators)?;
                 Statement::DefineBaseUnit(span, name, dexpr, decorators)
             }
             Statement::DefineDerivedUnit {
@@ -200,7 +206,7 @@ impl Transformer {
                 type_annotation,
                 decorators,
             } => {
-                self.register_name_and_aliases(identifier, &decorators, identifier_span)?;
+                self.register_name_and_aliases(identifier, identifier_span, &decorators)?;
                 Statement::DefineDerivedUnit {
                     identifier_span,
                     identifier,

numbat/src/typed_ast.rs

@@ -846,7 +846,7 @@ fn decorator_markup(decorators: &Vec<Decorator>) -> Markup {
                     m::decorator("@aliases")
                         + m::operator("(")
                         + Itertools::intersperse(
-                            names.iter().map(|(name, accepts_prefix)| {
+                            names.iter().map(|(name, accepts_prefix, _)| {
                                 m::unit(name) + accepts_prefix_markup(accepts_prefix)
                             }),
                             m::operator(", "),