Skip to content

Commit

Permalink
Plumbed spans through alias definitions, allowing for more precise er…
Browse files Browse the repository at this point in the history 
…ror messages

closes #570
  • Loading branch information
@rben01
rben01 authored Sep 29, 2024
1 parent e4a3a59 commit 5b6ee4e
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 53 deletions.
78 changes: 53 additions & 25 deletions numbat/src/decorator.rs
View file
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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;
}
}
Expand Down
39 changes: 32 additions & 7 deletions numbat/src/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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,
},
),
]),
],
}),
);
Expand Down
50 changes: 36 additions & 14 deletions numbat/src/prefix_parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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))
}
}
}
Expand All @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -294,7 +316,7 @@ mod tests {
true,
false,
"meter",
Span::dummy(),
AliasSpanInfo::dummy(),
)
.unwrap();
prefix_parser
Expand All @@ -304,7 +326,7 @@ mod tests {
true,
false,
"meter",
Span::dummy(),
AliasSpanInfo::dummy(),
)
.unwrap();

Expand All @@ -315,7 +337,7 @@ mod tests {
true,
true,
"byte",
Span::dummy(),
AliasSpanInfo::dummy(),
)
.unwrap();
prefix_parser
Expand All @@ -325,7 +347,7 @@ mod tests {
true,
true,
"byte",
Span::dummy(),
AliasSpanInfo::dummy(),
)
.unwrap();

Expand All @@ -336,7 +358,7 @@ mod tests {
false,
false,
"me",
Span::dummy(),
AliasSpanInfo::dummy(),
)
.unwrap();

Expand Down
18 changes: 12 additions & 6 deletions numbat/src/prefix_transformer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion numbat/src/typed_ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(", "),
Expand Down

0 comments on commit 5b6ee4e

Please sign in to comment.