diff --git a/tamer/src/asg/air.rs b/tamer/src/asg/air.rs index f316bd5e..ef1550e5 100644 --- a/tamer/src/asg/air.rs +++ b/tamer/src/asg/air.rs @@ -164,10 +164,12 @@ impl ParseState for AirAggregate { Transition(Empty).incomplete() } - // Packages are identified by their paths. - (st @ Toplevel(..), AirBind(BindIdent(id))) => { - Transition(st).err(AsgError::InvalidBindContext(id)) - } + // Packages are identified by canonical paths relative to the + // project root. + (Toplevel(oi_pkg), AirBind(BindIdent(name))) => oi_pkg + .assign_canonical_name(ctx.asg_mut(), name) + .map(|_| ()) + .transition(Toplevel(oi_pkg)), (Toplevel(oi_pkg), tok @ AirDoc(DocIndepClause(..))) => { diagnostic_todo!( diff --git a/tamer/src/asg/air/expr/test.rs b/tamer/src/asg/air/expr/test.rs index 4f6d7aae..ad5be03b 100644 --- a/tamer/src/asg/air/expr/test.rs +++ b/tamer/src/asg/air/expr/test.rs @@ -22,7 +22,7 @@ use crate::asg::{ air::{ test::{ asg_from_toks, parse_as_pkg_body, pkg_expect_ident_obj, - pkg_expect_ident_oi, pkg_get_ident_obj, pkg_lookup, + pkg_expect_ident_oi, pkg_lookup, }, Air, AirAggregate, }, @@ -464,82 +464,6 @@ fn expr_close_unbalanced() { assert_eq!(expr.span(), S2.merge(S4).unwrap()); } -#[test] -fn expr_bind_to_empty() { - let id_pre = SPair("pre".into(), S2); - let id_noexpr_a = SPair("noexpr_a".into(), S4); - let id_good = SPair("good".into(), S6); - let id_noexpr_b = SPair("noexpr_b".into(), S8); - - #[rustfmt::skip] - let toks = vec![ - // We need to first bring ourselves out of the context of the - // package header, - // otherwise the bind will be interpreted as a bind to the - // package itself. - Air::ExprStart(ExprOp::Sum, S1), - Air::BindIdent(id_pre), - Air::ExprEnd(S3), - - // No open expression to bind to. - Air::BindIdent(id_noexpr_a), - - // Post-recovery create an expression. - Air::ExprStart(ExprOp::Sum, S5), - Air::BindIdent(id_good), - Air::ExprEnd(S7), - - // Once again we have nothing to bind to. - Air::BindIdent(id_noexpr_b), - ]; - - let mut sut = parse_as_pkg_body(toks); - - assert_eq!( - #[rustfmt::skip] - vec![ - Ok(Parsed::Incomplete), // PkgStart - // Just to get out of a package header context - Ok(Parsed::Incomplete), // ExprStart (pre) - Ok(Parsed::Incomplete), // BindIdent (pre) - Ok(Parsed::Incomplete), // ExprEnd (pre) - - // Now that we've encountered an expression, - // we want an error specific to expression binding, - // since it's likely that a bind token was issued too late, - // rather than trying to interpret this as being back in a - // package context and binding to the package. - Err(ParseError::StateError(AsgError::InvalidBindContext( - id_noexpr_a - ))), - - // RECOVERY - Ok(Parsed::Incomplete), // ExprStart - Ok(Parsed::Incomplete), // BindIdent - Ok(Parsed::Incomplete), // ExprEnd - - // Another error after a successful expression. - Err(ParseError::StateError(AsgError::InvalidBindContext( - id_noexpr_b - ))), - // RECOVERY - Ok(Parsed::Incomplete), // PkgEnd - ], - sut.by_ref().collect::>(), - ); - - let asg = sut.finalize().unwrap().into_context(); - - // Neither of the identifiers outside of expressions should exist on the - // graph. - assert_eq!(None, pkg_get_ident_obj::(&asg, id_noexpr_a)); - assert_eq!(None, pkg_get_ident_obj::(&asg, id_noexpr_b)); - - // Verify that the expression was successfully added after recovery. - let expr = pkg_expect_ident_obj::(&asg, id_good); - assert_eq!(expr.span(), S5.merge(S7).unwrap()); -} - // Subexpressions should not only have edges to their parent, // but those edges ought to be ordered, // allowing TAME to handle non-commutative expressions. diff --git a/tamer/src/asg/air/test.rs b/tamer/src/asg/air/test.rs index ede82ded..c1f9ceaf 100644 --- a/tamer/src/asg/air/test.rs +++ b/tamer/src/asg/air/test.rs @@ -426,6 +426,58 @@ fn nested_open_pkg() { ); } +#[test] +fn pkg_canonical_name() { + let name = SPair("foo/bar".into(), S2); + + #[rustfmt::skip] + let toks = vec![ + PkgStart(S1), + BindIdent(name), + PkgEnd(S3), + ]; + + let mut sut = Sut::parse(toks.into_iter()); + assert!(sut.all(|x| x.is_ok())); + + let asg = sut.finalize().unwrap().into_context(); + + let pkg = asg + .root(S1) + .edges_filtered::(&asg) + .next() + .expect("cannot find package from root"); + + assert_eq!(Some(name), pkg.resolve(&asg).canonical_name()); +} + +#[test] +fn pkg_cannot_rename() { + let name = SPair("foo/bar".into(), S2); + let name2 = SPair("bad/rename".into(), S3); + + #[rustfmt::skip] + let toks = vec![ + PkgStart(S1), + BindIdent(name), + // Attempt to provide a name a second time. + BindIdent(name2), + // RECOVERY: Just ignore it. + PkgEnd(S3), + ]; + + assert_eq!( + vec![ + Ok(Incomplete), // PkgStart + Ok(Incomplete), // BindIdent + Err(ParseError::StateError(AsgError::PkgRename(name, name2))), + // RECOVERY: Ignore the attempted rename + Ok(Incomplete), // PkgEnd + ], + Sut::parse(toks.into_iter()).collect::>(), + ); +} + #[test] fn pkg_import() { let pathspec = SPair("foo/bar".into(), S2); @@ -452,7 +504,7 @@ fn pkg_import() { .expect("cannot find imported package") .resolve(&asg); - assert_eq!(pathspec, import.pathspec()); + assert_eq!(Some(pathspec), import.import_path()); } // Documentation can be mixed in with objects in a literate style. @@ -529,15 +581,6 @@ pub fn pkg_lookup(asg: &Asg, name: SPair) -> Option> { asg.lookup(oi_pkg, name) } -pub fn pkg_get_ident_oi>( - asg: &Asg, - name: SPair, -) -> Option> { - pkg_lookup(asg, name) - .and_then(|oi| oi.edges(asg).next()) - .and_then(|oi| oi.narrow()) -} - pub fn pkg_expect_ident_oi>( asg: &Asg, name: SPair, @@ -554,13 +597,6 @@ pub fn pkg_expect_ident_oi>( .expect(&format!("ident `{name}` was not of expected ObjectKind")) } -pub fn pkg_get_ident_obj>( - asg: &Asg, - name: SPair, -) -> Option<&O> { - pkg_get_ident_oi(asg, name).map(|oi| oi.resolve(asg)) -} - pub fn pkg_expect_ident_obj>( asg: &Asg, name: SPair, diff --git a/tamer/src/asg/error.rs b/tamer/src/asg/error.rs index bfa187ff..29210427 100644 --- a/tamer/src/asg/error.rs +++ b/tamer/src/asg/error.rs @@ -61,6 +61,14 @@ pub enum AsgError { /// whereas _declaring_ an identifier provides metadata about it. IdentRedefine(SPair, Span), + /// Attempted to rename a package from the first [`SPair`] to the + /// second. + /// + /// "Rename" here means that the package already had a name and we were + /// provided another, + /// which is almost certainly a mistake. + PkgRename(SPair, SPair), + /// Attempted to open a package while defining another package. /// /// Packages cannot be nested. @@ -147,6 +155,12 @@ impl Display for AsgError { IdentRedefine(spair, _) => { write!(f, "cannot redefine {}", TtQuote::wrap(spair)) } + PkgRename(from, to) => write!( + f, + "attempted to rename package {} to {}", + TtQuote::wrap(from), + TtQuote::wrap(to) + ), NestedPkgStart(_, _) => write!(f, "cannot nest packages"), InvalidPkgEndContext(_) => { write!(f, "invalid context for package close",) @@ -222,6 +236,12 @@ impl Diagnostic for AsgError { .help(" defined and its definition cannot be changed."), ], + PkgRename(from, to) => vec![ + from.note("package was originally named here"), + to.error("attempted to rename package here"), + to.help("a package cannot have its name changed"), + ], + NestedPkgStart(second, first) => vec![ first.note("this package is still being defined"), second.error("attempted to open another package here"), @@ -274,15 +294,8 @@ impl Diagnostic for AsgError { vec![span.error("there is no open template to close here")] } - InvalidBindContext(span) => vec![ - span.error( - "there is no active expression to bind this identifier to", - ), - span.help( - "an identifier must be bound to an expression before \ - the expression is closed", - ), - ], + InvalidBindContext(span) => vec![span + .error("there is no active object to bind this identifier to")], InvalidRefContext(ident) => vec![ident.error( "cannot reference the value of an expression from outside \ diff --git a/tamer/src/asg/graph/object/pkg.rs b/tamer/src/asg/graph/object/pkg.rs index 53b336c8..8df6a438 100644 --- a/tamer/src/asg/graph/object/pkg.rs +++ b/tamer/src/asg/graph/object/pkg.rs @@ -22,9 +22,9 @@ use super::{prelude::*, Doc, Ident, NameableMissingObject, Tpl}; use crate::{ f::Functor, + fmt::{DisplayWrapper, TtQuote}, parse::{util::SPair, Token}, span::Span, - sym::st::raw::WS_EMPTY, }; use std::fmt::Display; @@ -34,23 +34,17 @@ use super::ObjectKind; #[derive(Debug, PartialEq, Eq)] pub struct Pkg(Span, PathSpec); -/// Package path specification used to import this package. -/// -/// TODO: This is simply punting on handling of imports for now. -type PathSpec = SPair; - impl Pkg { /// Create a new package intended to serve as the compilation unit, /// with an empty pathspec. pub fn new>(span: S) -> Self { - let s = span.into(); - Self(s, SPair(WS_EMPTY, s)) + Self(span.into(), PathSpec::Unnamed) } /// Represent a package imported according to the provided /// [`PathSpec`]. - pub fn new_imported(pathspec: PathSpec) -> Self { - Self(pathspec.span(), pathspec) + pub fn new_imported(pathspec: SPair) -> Self { + Self(pathspec.span(), PathSpec::TodoImport(pathspec)) } pub fn span(&self) -> Span { @@ -59,9 +53,39 @@ impl Pkg { } } - pub fn pathspec(&self) -> PathSpec { + /// Attempt to assign a canonical name to this package. + /// + /// Only [`PathSpec::Unnamed`] packages may have a named assigned, + /// otherwise an [`AsgError::PkgRename`] [`Err`] will be returned. + pub fn assign_canonical_name( + self, + name: SPair, + ) -> Result { match self { - Self(_, pathspec) => *pathspec, + Self(span, PathSpec::Unnamed) => { + Ok(Self(span, PathSpec::Canonical(name))) + } + Self(_, PathSpec::Canonical(orig) | PathSpec::TodoImport(orig)) => { + Err((self, AsgError::PkgRename(orig, name))) + } + } + } + + /// The canonical name for this package assigned by + /// [`Self::assign_canonical_name`], + /// if any. + pub fn canonical_name(&self) -> Option { + match self { + Self(_, pathspec) => pathspec.canonical_name(), + } + } + + /// The import path to this package as provided by + /// [`Self::new_imported`], + /// if any. + pub fn import_path(&self) -> Option { + match self { + Self(_, pathspec) => pathspec.import_path(), } } } @@ -86,6 +110,70 @@ impl Functor for Pkg { } } +#[derive(Debug, PartialEq, Eq)] +enum PathSpec { + /// Unnamed package. + Unnamed, + + /// Canonical package name. + /// + /// This is the name of the package relative to the project root. + /// This is like the module name after `crate::` in Rust, + /// but with `/` package separators in place of `::`. + Canonical(SPair), + + /// Import path relative to the current package + /// (which is likely the compilation unit). + /// + /// TODO: This will be replaced with [`Self::Canonical`] once that is + /// working and relative paths can be resolved against the active + /// package. + TodoImport(SPair), +} + +impl PathSpec { + fn canonical_name(&self) -> Option { + use PathSpec::*; + + match self { + Unnamed => None, + Canonical(spair) => Some(*spair), + TodoImport(_) => None, + } + } + + fn import_path(&self) -> Option { + use PathSpec::*; + + match self { + Unnamed => None, + // TODO: Let's wait to see if we actually need this, + // since we'll need to allocate and intern a `/`-prefixed + // symbol. + Canonical(_) => None, + TodoImport(spair) => Some(*spair), + } + } +} + +impl Display for PathSpec { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + use PathSpec::*; + + match self { + Unnamed => { + write!(f, "unnamed package") + } + Canonical(spair) => write!(f, "package {}", TtQuote::wrap(spair)), + TodoImport(spair) => write!( + f, + "package import {} relative to compilation unit", + TtQuote::wrap(spair) + ), + } + } +} + object_rel! { /// Packages serve as a root for all identifiers defined therein, /// and so an edge to [`Ident`] will never be a cross edge. @@ -112,6 +200,17 @@ impl ObjectIndex { self.map_obj(asg, Pkg::fmap(|open| open.merge(span).unwrap_or(open))) } + /// Attempt to assign a canonical name to this package. + /// + /// This assignment will fail if the package already has a name. + pub fn assign_canonical_name( + self, + asg: &mut Asg, + name: SPair, + ) -> Result { + self.try_map_obj(asg, |pkg| pkg.assign_canonical_name(name)) + } + /// Indicate that a package should be imported at the provided /// pathspec. /// diff --git a/tamer/src/asg/graph/xmli.rs b/tamer/src/asg/graph/xmli.rs index 269f8cd6..d6fee977 100644 --- a/tamer/src/asg/graph/xmli.rs +++ b/tamer/src/asg/graph/xmli.rs @@ -236,7 +236,7 @@ impl<'a> TreeContext<'a> { /// Emit a package import statement. fn emit_import(&mut self, pkg: &Pkg, depth: Depth) -> Option { - let ps = pkg.pathspec(); + let ps = pkg.import_path()?; self.push(Xirf::attr(QN_PACKAGE, ps.symbol(), (ps.span(), ps.span()))); Some(Xirf::open(