Repository: cedar-policy/cedar
Pull request #1351 (2024-12-02 23:32:49)

Attach source code to certain errors
## Description of changes

## Issue #s, if available

#977, #1335

## Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

- [x] A change "invisible" to users (e.g., documentation, changes to "internal" crates like `cedar-policy-core`, `cedar-validator`, etc.)

I confirm that this PR (choose one, and delete the other options):

- [x] Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that [`cedar-spec`](https://github.com/cedar-policy/cedar-spec) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

I confirm that [`docs.cedarpolicy.com`](https://docs.cedarpolicy.com/) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar language specification.



Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 1b220a5c..689afe43 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -355,6 +355,7 @@ dependencies = [
  "arbitrary",
  "chrono",
  "cool_asserts",
+ "educe",
  "either",
  "itertools 0.13.0",
  "lalrpop",
@@ -834,6 +835,18 @@ version = "0.3.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10"
 
+[[package]]
+name = "educe"
+version = "0.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1d7bc049e1bd8cdeb31b68bbd586a9464ecf9f3944af3958a7a9d0f8b9799417"
+dependencies = [
+ "enum-ordinalize",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "either"
 version = "1.13.0"
@@ -855,6 +868,26 @@ version = "0.3.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f"
 
+[[package]]
+name = "enum-ordinalize"
+version = "4.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "fea0dcfa4e54eeb516fe454635a95753ddd39acda650ce703031c6973e315dd5"
+dependencies = [
+ "enum-ordinalize-derive",
+]
+
+[[package]]
+name = "enum-ordinalize-derive"
+version = "4.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0d28318a75d4aead5c4db25382e8ef717932d0346600cacae6357eb5941bc5ff"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "equivalent"
 version = "1.0.1"
diff --git a/cedar-policy-core/Cargo.toml b/cedar-policy-core/Cargo.toml
index 92be2b8a..9c0fa7ba 100644
--- a/cedar-policy-core/Cargo.toml
+++ b/cedar-policy-core/Cargo.toml
@@ -28,6 +28,7 @@ stacker = "0.1.15"
 arbitrary = { version = "1", features = ["derive"], optional = true }
 miette = { version = "7.4.0", features = ["serde"] }
 nonempty = "0.10.0"
+educe = "0.6.0"
 
 # decimal extension requires regex
 regex = { version = "1.8", features = ["unicode"], optional = true }
diff --git a/cedar-policy-core/src/ast/policy.rs b/cedar-policy-core/src/ast/policy.rs
index 384e5d7c..d6fbac83 100644
--- a/cedar-policy-core/src/ast/policy.rs
+++ b/cedar-policy-core/src/ast/policy.rs
@@ -16,6 +16,7 @@
 
 use crate::ast::*;
 use crate::parser::Loc;
+use educe::Educe;
 use itertools::Itertools;
 use miette::Diagnostic;
 use nonempty::{nonempty, NonEmpty};
@@ -1495,10 +1496,10 @@ impl PrincipalConstraint {
     /// Fill in the Slot, if any, with the given EUID
     pub fn with_filled_slot(self, euid: Arc<EntityUID>) -> Self {
         match self.constraint {
-            PrincipalOrResourceConstraint::Eq(EntityReference::Slot) => Self {
+            PrincipalOrResourceConstraint::Eq(EntityReference::Slot(_)) => Self {
                 constraint: PrincipalOrResourceConstraint::Eq(EntityReference::EUID(euid)),
             },
-            PrincipalOrResourceConstraint::In(EntityReference::Slot) => Self {
+            PrincipalOrResourceConstraint::In(EntityReference::Slot(_)) => Self {
                 constraint: PrincipalOrResourceConstraint::In(EntityReference::EUID(euid)),
             },
             _ => self,
@@ -1626,10 +1627,10 @@ impl ResourceConstraint {
     /// Fill in the Slot, if any, with the given EUID
     pub fn with_filled_slot(self, euid: Arc<EntityUID>) -> Self {
         match self.constraint {
-            PrincipalOrResourceConstraint::Eq(EntityReference::Slot) => Self {
+            PrincipalOrResourceConstraint::Eq(EntityReference::Slot(_)) => Self {
                 constraint: PrincipalOrResourceConstraint::Eq(EntityReference::EUID(euid)),
             },
-            PrincipalOrResourceConstraint::In(EntityReference::Slot) => Self {
+            PrincipalOrResourceConstraint::In(EntityReference::Slot(_)) => Self {
                 constraint: PrincipalOrResourceConstraint::In(EntityReference::EUID(euid)),
             },
             _ => self,
@@ -1672,12 +1673,19 @@ impl From<&ResourceConstraint> for proto::ResourceConstraint {
 }
 
 /// A reference to an EntityUID that may be a Slot
-#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
+#[derive(Educe, Serialize, Deserialize, Clone, Debug, Eq)]
+#[educe(Hash, PartialEq, PartialOrd, Ord)]
 pub enum EntityReference {
     /// Reference to a literal EUID
     EUID(Arc<EntityUID>),
     /// Template Slot
-    Slot,
+    Slot(
+        #[serde(skip)]
+        #[educe(PartialEq(ignore))]
+        #[educe(PartialOrd(ignore))]
+        #[educe(Hash(ignore))]
+        Option<Loc>,
+    ),
 }
 
 impl EntityReference {
@@ -1694,7 +1702,9 @@ impl EntityReference {
     pub fn into_expr(&self, slot: SlotId) -> Expr {
         match self {
             EntityReference::EUID(euid) => Expr::val(euid.clone()),
-            EntityReference::Slot => Expr::slot(slot),
+            EntityReference::Slot(loc) => {
+                Expr::slot(slot).with_maybe_source_loc(loc.as_ref().cloned())
+            }
         }
     }
 }
@@ -1713,7 +1723,7 @@ impl From<&proto::EntityReference> for EntityReference {
                 match proto::entity_reference::Ty::try_from(ty.to_owned())
                     .expect("decode should succeed")
                 {
-                    proto::entity_reference::Ty::Slot => EntityReference::Slot,
+                    proto::entity_reference::Ty::Slot => EntityReference::Slot(None),
                 }
             }
             proto::entity_reference::Data::Euid(euid) => {
@@ -1732,7 +1742,7 @@ impl From<&EntityReference> for proto::EntityReference {
                     euid.as_ref(),
                 ))),
             },
-            EntityReference::Slot => Self {
+            EntityReference::Slot(_) => Self {
                 data: Some(proto::entity_reference::Data::Ty(
                     proto::entity_reference::Ty::Slot.into(),
                 )),
@@ -1758,6 +1768,12 @@ impl Diagnostic for UnexpectedSlotError {
             }),
         }
     }
+
+    fn source_code(&self) -> Option<&dyn miette::SourceCode> {
+        match self {
+            Self::FoundSlot(Slot { loc, .. }) => loc.as_ref().map(|l| l as &dyn miette::SourceCode),
+        }
+    }
 }
 
 impl From<EntityUID> for EntityReference {
@@ -1825,12 +1841,12 @@ impl PrincipalOrResourceConstraint {
 
     /// Constrained to equal a slot
     pub fn is_eq_slot() -> Self {
-        PrincipalOrResourceConstraint::Eq(EntityReference::Slot)
+        PrincipalOrResourceConstraint::Eq(EntityReference::Slot(None))
     }
 
     /// Constrained to be in a slot
     pub fn is_in_slot() -> Self {
-        PrincipalOrResourceConstraint::In(EntityReference::Slot)
+        PrincipalOrResourceConstraint::In(EntityReference::Slot(None))
     }
 
     /// Hierarchical constraint.
@@ -1840,7 +1856,7 @@ impl PrincipalOrResourceConstraint {
 
     /// Type constraint additionally constrained to be in a slot.
     pub fn is_entity_type_in_slot(entity_type: Arc<EntityType>) -> Self {
-        PrincipalOrResourceConstraint::IsIn(entity_type, EntityReference::Slot)
+        PrincipalOrResourceConstraint::IsIn(entity_type, EntityReference::Slot(None))
     }
 
     /// Type constraint with a hierarchical constraint.
@@ -1901,11 +1917,11 @@ impl PrincipalOrResourceConstraint {
         match self {
             PrincipalOrResourceConstraint::Any => None,
             PrincipalOrResourceConstraint::In(EntityReference::EUID(euid)) => Some(euid),
-            PrincipalOrResourceConstraint::In(EntityReference::Slot) => None,
+            PrincipalOrResourceConstraint::In(EntityReference::Slot(_)) => None,
             PrincipalOrResourceConstraint::Eq(EntityReference::EUID(euid)) => Some(euid),
-            PrincipalOrResourceConstraint::Eq(EntityReference::Slot) => None,
+            PrincipalOrResourceConstraint::Eq(EntityReference::Slot(_)) => None,
             PrincipalOrResourceConstraint::IsIn(_, EntityReference::EUID(euid)) => Some(euid),
-            PrincipalOrResourceConstraint::IsIn(_, EntityReference::Slot) => None,
+            PrincipalOrResourceConstraint::IsIn(_, EntityReference::Slot(_)) => None,
             PrincipalOrResourceConstraint::Is(_) => None,
         }
     }
@@ -2315,9 +2331,9 @@ pub(crate) mod test_generators {
         let v = vec![
             PrincipalOrResourceConstraint::any(),
             PrincipalOrResourceConstraint::is_eq(euid.clone()),
-            PrincipalOrResourceConstraint::Eq(EntityReference::Slot),
+            PrincipalOrResourceConstraint::Eq(EntityReference::Slot(None)),
             PrincipalOrResourceConstraint::is_in(euid),
-            PrincipalOrResourceConstraint::In(EntityReference::Slot),
+            PrincipalOrResourceConstraint::In(EntityReference::Slot(None)),
         ];
 
         v.into_iter()
@@ -2601,7 +2617,7 @@ mod test {
             Some(&e)
         );
         assert_eq!(
-            PrincipalOrResourceConstraint::In(EntityReference::Slot).get_euid(),
+            PrincipalOrResourceConstraint::In(EntityReference::Slot(None)).get_euid(),
             None
         );
         assert_eq!(
@@ -2609,7 +2625,7 @@ mod test {
             Some(&e)
         );
         assert_eq!(
-            PrincipalOrResourceConstraint::Eq(EntityReference::Slot).get_euid(),
+            PrincipalOrResourceConstraint::Eq(EntityReference::Slot(None)).get_euid(),
             None
         );
         assert_eq!(
@@ -2627,7 +2643,7 @@ mod test {
         assert_eq!(
             PrincipalOrResourceConstraint::IsIn(
                 Arc::new("T".parse().unwrap()),
-                EntityReference::Slot
+                EntityReference::Slot(None)
             )
             .get_euid(),
             None
@@ -2688,7 +2704,7 @@ mod test {
 
     #[test]
     fn euid_into_expr() {
-        let e = EntityReference::Slot;
+        let e = EntityReference::Slot(None);
         assert_eq!(
             e.into_expr(SlotId::principal()),
             Expr::slot(SlotId::principal())
@@ -2702,7 +2718,7 @@ mod test {
 
     #[test]
     fn por_constraint_display() {
-        let t = PrincipalOrResourceConstraint::Eq(EntityReference::Slot);
+        let t = PrincipalOrResourceConstraint::Eq(EntityReference::Slot(None));
         let s = t.display(PrincipalOrResource::Principal);
         assert_eq!(s, "principal == ?principal");
         let t = PrincipalOrResourceConstraint::Eq(EntityReference::euid(Arc::new(
@@ -2720,7 +2736,7 @@ mod test {
                 "expected a static policy, got a template containing the slot ?principal"
                 )
                 .help("try removing the template slot(s) from this policy")
-                .exactly_one_underline("permit(principal == ?principal, action, resource);")
+                .exactly_one_underline("?principal")
                 .build()
             );
         });
@@ -2775,8 +2791,8 @@ mod test {
             EntityReference::from(&proto::EntityReference::from(&er1))
         );
         assert_eq!(
-            EntityReference::Slot,
-            EntityReference::from(&proto::EntityReference::from(&EntityReference::Slot))
+            EntityReference::Slot(None),
+            EntityReference::from(&proto::EntityReference::from(&EntityReference::Slot(None)))
         );
 
         let read_euid: Arc<EntityUID> = Arc::new(EntityUID::with_eid("read"));
diff --git a/cedar-policy-core/src/error_macros.rs b/cedar-policy-core/src/error_macros.rs
index 77f5fd44..d9d26a68 100644
--- a/cedar-policy-core/src/error_macros.rs
+++ b/cedar-policy-core/src/error_macros.rs
@@ -37,15 +37,15 @@ macro_rules! impl_diagnostic_from_source_loc_field {
 /// of a field of type `Option<Loc>`
 #[macro_export]
 macro_rules! impl_diagnostic_from_source_loc_opt_field {
-    ( $i:ident ) => {
+    ( $($id:ident).+ ) => {
         fn source_code(&self) -> Option<&dyn miette::SourceCode> {
-            self.$i
+            self.$($id).+
                 .as_ref()
                 .map(|loc| &loc.src as &dyn miette::SourceCode)
         }
 
         fn labels(&self) -> Option<Box<dyn Iterator<Item = miette::LabeledSpan> + '_>> {
-            self.$i
+            self.$($id).+
                 .as_ref()
                 .map(|loc| Box::new(std::iter::once(miette::LabeledSpan::underline(loc.span))) as _)
         }
diff --git a/cedar-policy-core/src/est/expr.rs b/cedar-policy-core/src/est/expr.rs
index e87b6aea..fbc6b74f 100644
--- a/cedar-policy-core/src/est/expr.rs
+++ b/cedar-policy-core/src/est/expr.rs
@@ -23,6 +23,7 @@ use crate::entities::json::{
 use crate::extensions::Extensions;
 use crate::jsonvalue::JsonValueWithNoDuplicateKeys;
 use crate::parser::cst::{self, Ident};
+use crate::parser::cst_to_ast;
 use crate::parser::err::{ParseErrors, ToASTError, ToASTErrorKind};
 use crate::parser::unescape::to_unescaped_string;
 use crate::parser::util::flatten_tuple_2;
@@ -89,24 +90,21 @@ impl<'de> Deserialize<'de> for Expr {
                         return Err(serde::de::Error::custom(format!("JSON object representing an `Expr` should have only one key, but found two keys: `{k}` and `{k2}`")));
                     }
                 };
-                match ast::Name::parse_unqualified_name(&k)
-                    .map(|name| name.is_known_extension_func_name())
-                {
-                    Ok(true) => {
-                        // `k` is the name of an extension function. We assume that no such keys
-                        // are valid keys for `ExprNoExt`, so we must parse as an `ExtFuncCall`.
-                        let obj = serde_json::json!({ k: v });
-                        let extfunccall =
-                            serde_json::from_value(obj).map_err(serde::de::Error::custom)?;
-                        Ok(Expr::ExtFuncCall(extfunccall))
-                    }
-                    _ => {
-                        // not a valid extension function, so we expect it to work for `ExprNoExt`.
-                        let obj = serde_json::json!({ k: v });
-                        let exprnoext =
-                            serde_json::from_value(obj).map_err(serde::de::Error::custom)?;
-                        Ok(Expr::ExprNoExt(exprnoext))
-                    }
+                if cst_to_ast::is_known_extension_func_str(&k) {
+                    // `k` is the name of an extension function or method. We assume that
+                    // no such keys are valid keys for `ExprNoExt`, so we must parse as an
+                    // `ExtFuncCall`.
+                    let obj = serde_json::json!({ k: v });
+                    let extfunccall =
+                        serde_json::from_value(obj).map_err(serde::de::Error::custom)?;
+                    Ok(Expr::ExtFuncCall(extfunccall))
+                } else {
+                    // not a valid extension function or method, so we expect it
+                    // to work for `ExprNoExt`.
+                    let obj = serde_json::json!({ k: v });
+                    let exprnoext =
+                        serde_json::from_value(obj).map_err(serde::de::Error::custom)?;
+                    Ok(Expr::ExprNoExt(exprnoext))
                 }
             }
         }
@@ -965,7 +963,7 @@ impl Expr {
                                 errs,
                             )
                         })?;
-                        if !fn_name.is_known_extension_func_name() {
+                        if !cst_to_ast::is_known_extension_func_name(&fn_name) {
                             return Err(FromJsonError::UnknownExtensionFunction(fn_name));
                         }
                         Ok(ast::Expr::call_extension_fn(
diff --git a/cedar-policy-core/src/est/scope_constraints.rs b/cedar-policy-core/src/est/scope_constraints.rs
index 4a42355f..f62409ae 100644
--- a/cedar-policy-core/src/est/scope_constraints.rs
+++ b/cedar-policy-core/src/est/scope_constraints.rs
@@ -600,7 +600,7 @@ impl From<ast::PrincipalOrResourceConstraint> for PrincipalConstraint {
                     entity: EntityUidJson::ImplicitEntityEscape((&*e).into()),
                 })
             }
-            ast::PrincipalOrResourceConstraint::Eq(ast::EntityReference::Slot) => {
+            ast::PrincipalOrResourceConstraint::Eq(ast::EntityReference::Slot(_)) => {
                 PrincipalConstraint::Eq(EqConstraint::Slot {
                     slot: ast::SlotId::principal(),
                 })
@@ -610,7 +610,7 @@ impl From<ast::PrincipalOrResourceConstraint> for PrincipalConstraint {
                     entity: EntityUidJson::ImplicitEntityEscape((&*e).into()),
                 })
             }
-            ast::PrincipalOrResourceConstraint::In(ast::EntityReference::Slot) => {
+            ast::PrincipalOrResourceConstraint::In(ast::EntityReference::Slot(_)) => {
                 PrincipalConstraint::In(PrincipalOrResourceInConstraint::Slot {
                     slot: ast::SlotId::principal(),
                 })
@@ -622,7 +622,7 @@ impl From<ast::PrincipalOrResourceConstraint> for PrincipalConstraint {
                         ast::EntityReference::EUID(e) => PrincipalOrResourceInConstraint::Entity {
                             entity: EntityUidJson::ImplicitEntityEscape((&*e).into()),
                         },
-                        ast::EntityReference::Slot => PrincipalOrResourceInConstraint::Slot {
+                        ast::EntityReference::Slot(_) => PrincipalOrResourceInConstraint::Slot {
                             slot: ast::SlotId::principal(),
                         },
                     }),
@@ -647,7 +647,7 @@ impl From<ast::PrincipalOrResourceConstraint> for ResourceConstraint {
                     entity: EntityUidJson::ImplicitEntityEscape((&*e).into()),
                 })
             }
-            ast::PrincipalOrResourceConstraint::Eq(ast::EntityReference::Slot) => {
+            ast::PrincipalOrResourceConstraint::Eq(ast::EntityReference::Slot(_)) => {
                 ResourceConstraint::Eq(EqConstraint::Slot {
                     slot: ast::SlotId::resource(),
                 })
@@ -657,7 +657,7 @@ impl From<ast::PrincipalOrResourceConstraint> for ResourceConstraint {
                     entity: EntityUidJson::ImplicitEntityEscape((&*e).into()),
                 })
             }
-            ast::PrincipalOrResourceConstraint::In(ast::EntityReference::Slot) => {
+            ast::PrincipalOrResourceConstraint::In(ast::EntityReference::Slot(_)) => {
                 ResourceConstraint::In(PrincipalOrResourceInConstraint::Slot {
                     slot: ast::SlotId::resource(),
                 })
@@ -669,7 +669,7 @@ impl From<ast::PrincipalOrResourceConstraint> for ResourceConstraint {
                         ast::EntityReference::EUID(e) => PrincipalOrResourceInConstraint::Entity {
                             entity: EntityUidJson::ImplicitEntityEscape((&*e).into()),
                         },
-                        ast::EntityReference::Slot => PrincipalOrResourceInConstraint::Slot {
+                        ast::EntityReference::Slot(_) => PrincipalOrResourceInConstraint::Slot {
                             slot: ast::SlotId::resource(),
                         },
                     }),
@@ -700,7 +700,7 @@ impl TryFrom<PrincipalConstraint> for ast::PrincipalOrResourceConstraint {
             PrincipalConstraint::Eq(EqConstraint::Slot { slot }) => {
                 if slot == ast::SlotId::principal() {
                     Ok(ast::PrincipalOrResourceConstraint::Eq(
-                        ast::EntityReference::Slot,
+                        ast::EntityReference::Slot(None),
                     ))
                 } else {
                     Err(Self::Error::InvalidSlotName)
@@ -714,7 +714,7 @@ impl TryFrom<PrincipalConstraint> for ast::PrincipalOrResourceConstraint {
             PrincipalConstraint::In(PrincipalOrResourceInConstraint::Slot { slot }) => {
                 if slot == ast::SlotId::principal() {
                     Ok(ast::PrincipalOrResourceConstraint::In(
-                        ast::EntityReference::Slot,
+                        ast::EntityReference::Slot(None),
                     ))
                 } else {
                     Err(Self::Error::InvalidSlotName)
@@ -765,7 +765,7 @@ impl TryFrom<ResourceConstraint> for ast::PrincipalOrResourceConstraint {
             ResourceConstraint::Eq(EqConstraint::Slot { slot }) => {
                 if slot == ast::SlotId::resource() {
                     Ok(ast::PrincipalOrResourceConstraint::Eq(
-                        ast::EntityReference::Slot,
+                        ast::EntityReference::Slot(None),
                     ))
                 } else {
                     Err(Self::Error::InvalidSlotName)
@@ -779,7 +779,7 @@ impl TryFrom<ResourceConstraint> for ast::PrincipalOrResourceConstraint {
             ResourceConstraint::In(PrincipalOrResourceInConstraint::Slot { slot }) => {
                 if slot == ast::SlotId::resource() {
                     Ok(ast::PrincipalOrResourceConstraint::In(
-                        ast::EntityReference::Slot,
+                        ast::EntityReference::Slot(None),
                     ))
                 } else {
                     Err(Self::Error::InvalidSlotName)
diff --git a/cedar-policy-core/src/parser.rs b/cedar-policy-core/src/parser.rs
index c8ed91e0..b885b449 100644
--- a/cedar-policy-core/src/parser.rs
+++ b/cedar-policy-core/src/parser.rs
@@ -19,7 +19,7 @@
 /// Concrete Syntax Tree def used as parser first pass
 pub mod cst;
 /// Step two: convert CST to package AST
-mod cst_to_ast;
+pub mod cst_to_ast;
 /// error handling utilities
 pub mod err;
 /// implementations for formatting, like `Display`
@@ -300,7 +300,7 @@ pub(crate) mod test_utils {
         msg: &ExpectedErrorMessage<'_>,
     ) {
         assert!(
-            errs.iter().any(|e| msg.matches(Some(src), e)),
+            errs.iter().any(|e| msg.matches(e)),
             "for the following input:\n{src}\nexpected some error to match the following:\n{msg}\nbut actual errors were:\n{:?}", // the Debug representation of `miette::Report` is the pretty one, for some reason
             miette::Report::new(errs.clone()),
         );
diff --git a/cedar-policy-core/src/parser/cst_to_ast.rs b/cedar-policy-core/src/parser/cst_to_ast.rs
index cebc8e02..c3abf2b4 100644
--- a/cedar-policy-core/src/parser/cst_to_ast.rs
+++ b/cedar-policy-core/src/parser/cst_to_ast.rs
@@ -65,8 +65,12 @@ type Result<T> = std::result::Result<T, ParseErrors>;
 
 // for storing extension function names per callstyle
 struct ExtStyles<'a> {
+    /// All extension function names (just functions, not methods), as `Name`s
     functions: HashSet<&'a ast::Name>,
+    /// All extension function methods. `UnreservedId` is appropriate because methods cannot be namespaced.
     methods: HashSet<ast::UnreservedId>,
+    /// All extension function and method names (both qualified and unqualified), in their string (`Display`) form
+    functions_and_methods_as_str: HashSet<SmolStr>,
 }
 
 // Store extension function call styles
@@ -76,13 +80,24 @@ lazy_static::lazy_static! {
 fn load_styles() -> ExtStyles<'static> {
     let mut functions = HashSet::new();
     let mut methods = HashSet::new();
+    let mut functions_and_methods_as_str = HashSet::new();
     for func in crate::extensions::Extensions::all_available().all_funcs() {
+        functions_and_methods_as_str.insert(func.name().to_smolstr());
         match func.style() {
-            CallStyle::FunctionStyle => functions.insert(func.name()),
-            CallStyle::MethodStyle => methods.insert(func.name().basename()),
+            CallStyle::FunctionStyle => {
+                functions.insert(func.name());
+            }
+            CallStyle::MethodStyle => {
+                debug_assert!(func.name().is_unqualified());
+                methods.insert(func.name().basename());
+            }
         };
     }
-    ExtStyles { functions, methods }
+    ExtStyles {
+        functions,
+        methods,
+        functions_and_methods_as_str,
+    }
 }
 
 impl Node<Option<cst::Policies>> {
@@ -1605,6 +1620,20 @@ impl Node<Option<cst::Name>> {
     }
 }
 
+/// If this [`ast::Name`] is a known extension function/method name or not
+pub(crate) fn is_known_extension_func_name(name: &ast::Name) -> bool {
+    EXTENSION_STYLES.functions.contains(name)
+        || (name.0.path.is_empty() && EXTENSION_STYLES.methods.contains(&name.basename()))
+}
+
+/// If this [`SmolStr`] is a known extension function/method name or not. Works
+/// with both qualified and unqualified `s`. (As of this writing, there are no
+/// qualified extension function/method names, so qualified `s` always results
+/// in `false`.)
+pub(crate) fn is_known_extension_func_str(s: &SmolStr) -> bool {
+    EXTENSION_STYLES.functions_and_methods_as_str.contains(s)
+}
+
 impl ast::Name {
     /// Convert the `Name` into a `String` attribute, which fails if it had any namespaces
     fn into_valid_attr(self, loc: Loc) -> Result<SmolStr> {
@@ -1615,12 +1644,6 @@ impl ast::Name {
         }
     }
 
-    /// If this name is a known extension function/method name or not
-    pub(crate) fn is_known_extension_func_name(&self) -> bool {
-        EXTENSION_STYLES.functions.contains(self)
-            || (self.0.path.is_empty() && EXTENSION_STYLES.methods.contains(&self.basename()))
-    }
-
     fn into_func(self, args: Vec<ast::Expr>, loc: Loc) -> Result<ast::Expr> {
         // error on standard methods
         if self.0.path.is_empty() {
diff --git a/cedar-policy-core/src/parser/cst_to_ast/to_ref_or_refs.rs b/cedar-policy-core/src/parser/cst_to_ast/to_ref_or_refs.rs
index 9f52be65..0dc03685 100644
--- a/cedar-policy-core/src/parser/cst_to_ast/to_ref_or_refs.rs
+++ b/cedar-policy-core/src/parser/cst_to_ast/to_ref_or_refs.rs
@@ -33,7 +33,7 @@ use crate::{
 /// in the policy scope.
 trait RefKind: Sized {
     fn err_str() -> &'static str;
-    fn create_single_ref(e: EntityUID, loc: &Loc) -> Result<Self>;
+    fn create_single_ref(e: EntityUID) -> Result<Self>;
     fn create_multiple_refs(loc: &Loc) -> Result<fn(Vec<EntityUID>) -> Self>;
     fn create_slot(loc: &Loc) -> Result<Self>;
 }
@@ -45,7 +45,7 @@ impl RefKind for SingleEntity {
         "an entity uid"
     }
 
-    fn create_single_ref(e: EntityUID, _loc: &Loc) -> Result<Self> {
+    fn create_single_ref(e: EntityUID) -> Result<Self> {
         Ok(SingleEntity(e))
     }
 
@@ -77,11 +77,11 @@ impl RefKind for EntityReference {
         "an entity uid or matching template slot"
     }
 
-    fn create_slot(_loc: &Loc) -> Result<Self> {
-        Ok(EntityReference::Slot)
+    fn create_slot(loc: &Loc) -> Result<Self> {
+        Ok(EntityReference::Slot(Some(loc.clone())))
     }
 
-    fn create_single_ref(e: EntityUID, _loc: &Loc) -> Result<Self> {
+    fn create_single_ref(e: EntityUID) -> Result<Self> {
         Ok(EntityReference::euid(Arc::new(e)))
     }
 
@@ -122,7 +122,7 @@ impl RefKind for OneOrMultipleRefs {
         .into())
     }
 
-    fn create_single_ref(e: EntityUID, _loc: &Loc) -> Result<Self> {
+    fn create_single_ref(e: EntityUID) -> Result<Self> {
         Ok(OneOrMultipleRefs::Single(e))
     }
 
@@ -211,7 +211,7 @@ impl Node<Option<cst::Primary>> {
                     ))
                     .into())
             }
-            cst::Primary::Ref(x) => T::create_single_ref(x.to_ref()?, &self.loc),
+            cst::Primary::Ref(x) => T::create_single_ref(x.to_ref()?),
             cst::Primary::Name(name) => {
                 let found = match name.as_inner() {
                     Some(name) => format!("name `{name}`"),
diff --git a/cedar-policy-core/src/parser/err.rs b/cedar-policy-core/src/parser/err.rs
index 8e8254b9..238da340 100644
--- a/cedar-policy-core/src/parser/err.rs
+++ b/cedar-policy-core/src/parser/err.rs
@@ -18,6 +18,7 @@ use std::collections::{BTreeSet, HashMap, HashSet};
 use std::fmt::{self, Display, Write};
 use std::iter;
 use std::ops::{Deref, DerefMut};
+use std::sync::Arc;
 
 use either::Either;
 use lalrpop_util as lalr;
@@ -472,14 +473,23 @@ pub mod parse_errors {
     use super::*;
 
     /// Details about a `ExpectedStaticPolicy` error.
-    #[derive(Debug, Clone, Diagnostic, Error, PartialEq, Eq)]
+    #[derive(Debug, Clone, Error, PartialEq, Eq)]
     #[error("expected a static policy, got a template containing the slot {}", slot.id)]
-    #[diagnostic(help("try removing the template slot(s) from this policy"))]
     pub struct ExpectedStaticPolicy {
         /// Slot that was found (which is not valid in a static policy)
         pub(crate) slot: ast::Slot,
     }
 
+    impl Diagnostic for ExpectedStaticPolicy {
+        fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
+            Some(Box::new(
+                "try removing the template slot(s) from this policy",
+            ))
+        }
+
+        impl_diagnostic_from_source_loc_opt_field!(slot.loc);
+    }
+
     impl From<ast::UnexpectedSlotError> for ExpectedStaticPolicy {
         fn from(err: ast::UnexpectedSlotError) -> Self {
             match err {
@@ -573,6 +583,7 @@ pub mod parse_errors {
 #[derive(Clone, Debug, Error, PartialEq, Eq)]
 pub struct ToCSTError {
     err: OwnedRawParseError,
+    src: Arc<str>,
 }
 
 impl ToCSTError {
@@ -592,14 +603,15 @@ impl ToCSTError {
         }
     }
 
-    pub(crate) fn from_raw_parse_err(err: RawParseError<'_>) -> Self {
+    pub(crate) fn from_raw_parse_err(err: RawParseError<'_>, src: Arc<str>) -> Self {
         Self {
             err: err.map_token(|token| token.to_string()),
+            src,
         }
     }
 
-    pub(crate) fn from_raw_err_recovery(recovery: RawErrorRecovery<'_>) -> Self {
-        Self::from_raw_parse_err(recovery.error)
+    pub(crate) fn from_raw_err_recovery(recovery: RawErrorRecovery<'_>, src: Arc<str>) -> Self {
+        Self::from_raw_parse_err(recovery.error, src)
     }
 }
 
@@ -622,6 +634,10 @@ impl Display for ToCSTError {
 }
 
 impl Diagnostic for ToCSTError {
+    fn source_code(&self) -> Option<&dyn miette::SourceCode> {
+        Some(&self.src as &dyn miette::SourceCode)
+    }
+
     fn labels(&self) -> Option<Box<dyn Iterator<Item = LabeledSpan> + '_>> {
         let primary_source_span = self.primary_source_span();
         let labeled_span = match &self.err {
diff --git a/cedar-policy-core/src/parser/text_to_cst.rs b/cedar-policy-core/src/parser/text_to_cst.rs
index e9596410..aaf7cce1 100644
--- a/cedar-policy-core/src/parser/text_to_cst.rs
+++ b/cedar-policy-core/src/parser/text_to_cst.rs
@@ -52,13 +52,13 @@ fn parse_collect_errors<'a, P, T>(
 
     let errors = errs
         .into_iter()
-        .map(err::ToCSTError::from_raw_err_recovery)
+        .map(|rc| err::ToCSTError::from_raw_err_recovery(rc, Arc::from(text)))
         .map(Into::into);
     let parsed = match result {
         Ok(parsed) => parsed,
         Err(e) => {
             return Err(err::ParseErrors::new(
-                err::ToCSTError::from_raw_parse_err(e).into(),
+                err::ToCSTError::from_raw_parse_err(e, Arc::from(text)).into(),
                 errors,
             ));
         }
diff --git a/cedar-policy-validator/src/cedar_schema/err.rs b/cedar-policy-validator/src/cedar_schema/err.rs
index a385abc0..232529cb 100644
--- a/cedar-policy-validator/src/cedar_schema/err.rs
+++ b/cedar-policy-validator/src/cedar_schema/err.rs
@@ -18,6 +18,7 @@ use std::{
     collections::{HashMap, HashSet},
     fmt::Display,
     iter::{Chain, Once},
+    sync::Arc,
     vec,
 };
 
@@ -102,27 +103,27 @@ lazy_static! {
 pub struct ParseError {
     /// Error generated by lalrpop
     err: OwnedRawParseError,
+    /// Source code
+    src: Arc<str>,
 }
 
-impl From<RawParseError<'_>> for ParseError {
-    fn from(err: RawParseError<'_>) -> Self {
+impl ParseError {
+    pub(crate) fn from_raw_parse_error(err: RawParseError<'_>, src: Arc<str>) -> Self {
         Self {
             err: err.map_token(|token| token.to_string()),
+            src,
         }
     }
-}
 
-impl From<RawErrorRecovery<'_>> for ParseError {
-    fn from(recovery: RawErrorRecovery<'_>) -> Self {
-        recovery.error.into()
+    pub(crate) fn from_raw_error_recovery(recovery: RawErrorRecovery<'_>, src: Arc<str>) -> Self {
+        Self::from_raw_parse_error(recovery.error, src)
     }
 }
 
 impl ParseError {
     /// Extract a primary source span locating the error.
     pub fn primary_source_span(&self) -> SourceSpan {
-        let Self { err } = self;
-        match err {
+        match &self.err {
             OwnedRawParseError::InvalidToken { location } => SourceSpan::from(*location),
             OwnedRawParseError::UnrecognizedEof { location, .. } => SourceSpan::from(*location),
             OwnedRawParseError::UnrecognizedToken {
@@ -139,7 +140,7 @@ impl ParseError {
 
 impl Display for ParseError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let Self { err } = self;
+        let Self { err, .. } = self;
         match err {
             OwnedRawParseError::InvalidToken { .. } => write!(f, "invalid token"),
             OwnedRawParseError::UnrecognizedEof { .. } => write!(f, "unexpected end of input"),
@@ -161,9 +162,13 @@ impl Display for ParseError {
 impl std::error::Error for ParseError {}
 
 impl Diagnostic for ParseError {
+    fn source_code(&self) -> Option<&dyn miette::SourceCode> {
+        Some(&self.src as &dyn miette::SourceCode)
+    }
+
     fn labels(&self) -> Option<Box<dyn Iterator<Item = LabeledSpan> + '_>> {
         let primary_source_span = self.primary_source_span();
-        let Self { err } = self;
+        let Self { err, .. } = self;
         let labeled_span = match err {
             OwnedRawParseError::InvalidToken { .. } => LabeledSpan::underline(primary_source_span),
             OwnedRawParseError::UnrecognizedEof { expected, .. } => LabeledSpan::new_with_span(
diff --git a/cedar-policy-validator/src/cedar_schema/parser.rs b/cedar-policy-validator/src/cedar_schema/parser.rs
index b999eb90..a4397ecd 100644
--- a/cedar-policy-validator/src/cedar_schema/parser.rs
+++ b/cedar-policy-validator/src/cedar_schema/parser.rs
@@ -62,12 +62,15 @@ fn parse_collect_errors<'a, P, T>(
 
     let errors = errs
         .into_iter()
-        .map(Into::into)
+        .map(|rc| ParseError::from_raw_error_recovery(rc, Arc::from(text)))
         .collect::<Vec<ParseError>>();
     let parsed = match result {
         Ok(parsed) => parsed,
         Err(e) => {
-            return Err(ParseErrors::new(e.into(), errors));
+            return Err(ParseErrors::new(
+                ParseError::from_raw_parse_error(e, Arc::from(text)),
+                errors,
+            ));
         }
     };
     match ParseErrors::from_iter(errors) {
diff --git a/cedar-policy-validator/src/rbac.rs b/cedar-policy-validator/src/rbac.rs
index ea97569f..b27ad86f 100644
--- a/cedar-policy-validator/src/rbac.rs
+++ b/cedar-policy-validator/src/rbac.rs
@@ -361,19 +361,21 @@ impl Validator {
             PrincipalOrResourceConstraint::In(EntityReference::EUID(euid)) => {
                 Box::new(self.schema.get_entity_types_in(euid.as_ref()).into_iter())
             }
-            PrincipalOrResourceConstraint::Eq(EntityReference::Slot)
-            | PrincipalOrResourceConstraint::In(EntityReference::Slot) => {
+            PrincipalOrResourceConstraint::Eq(EntityReference::Slot(_))
+            | PrincipalOrResourceConstraint::In(EntityReference::Slot(_)) => {
                 Box::new(self.schema.known_entity_types())
             }
             PrincipalOrResourceConstraint::Is(entity_type)
-            | PrincipalOrResourceConstraint::IsIn(entity_type, EntityReference::Slot) => Box::new(
-                if self.schema.is_known_entity_type(entity_type) {
-                    Some(entity_type.as_ref())
-                } else {
-                    None
-                }
-                .into_iter(),
-            ),
+            | PrincipalOrResourceConstraint::IsIn(entity_type, EntityReference::Slot(_)) => {
+                Box::new(
+                    if self.schema.is_known_entity_type(entity_type) {
+                        Some(entity_type.as_ref())
+                    } else {
+                        None
+                    }
+                    .into_iter(),
+                )
+            }
             PrincipalOrResourceConstraint::IsIn(entity_type, EntityReference::EUID(in_entity)) => {
                 Box::new(
                     self.schema
diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md
index 6faee2ab..ff9ef26e 100644
--- a/cedar-policy/CHANGELOG.md
+++ b/cedar-policy/CHANGELOG.md
@@ -25,6 +25,10 @@ Cedar Language Version: TBD
 - Stopped emitting warnings for identifiers containing certain printable ASCII
   characters (e.g., `/` and `:`) (#1336, resolving #621)
 
+### Fixed
+
+- Attach source code to certain errors so that `miette::Report`s derived from these errors are self-contained (#1351, resolving #977 and #1335)
+
 ## [4.2.2] - 2024-11-11
 Cedar Language version: 4.1
 
diff --git a/cedar-policy/src/api.rs b/cedar-policy/src/api.rs
index 56a286b4..c83ea676 100644
--- a/cedar-policy/src/api.rs
+++ b/cedar-policy/src/api.rs
@@ -2599,13 +2599,13 @@ impl Template {
             ast::PrincipalOrResourceConstraint::In(eref) => {
                 TemplatePrincipalConstraint::In(match eref {
                     ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
-                    ast::EntityReference::Slot => None,
+                    ast::EntityReference::Slot(_) => None,
                 })
             }
             ast::PrincipalOrResourceConstraint::Eq(eref) => {
                 TemplatePrincipalConstraint::Eq(match eref {
                     ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
-                    ast::EntityReference::Slot => None,
+                    ast::EntityReference::Slot(_) => None,
                 })
             }
             ast::PrincipalOrResourceConstraint::Is(entity_type) => {
@@ -2616,7 +2616,7 @@ impl Template {
                     entity_type.as_ref().clone().into(),
                     match eref {
                         ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
-                        ast::EntityReference::Slot => None,
+                        ast::EntityReference::Slot(_) => None,
                     },
                 )
             }
@@ -2642,13 +2642,13 @@ impl Template {
             ast::PrincipalOrResourceConstraint::In(eref) => {
                 TemplateResourceConstraint::In(match eref {
                     ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
-                    ast::EntityReference::Slot => None,
+                    ast::EntityReference::Slot(_) => None,
                 })
             }
             ast::PrincipalOrResourceConstraint::Eq(eref) => {
                 TemplateResourceConstraint::Eq(match eref {
                     ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
-                    ast::EntityReference::Slot => None,
+                    ast::EntityReference::Slot(_) => None,
                 })
             }
             ast::PrincipalOrResourceConstraint::Is(entity_type) => {
@@ -2659,7 +2659,7 @@ impl Template {
                     entity_type.as_ref().clone().into(),
                     match eref {
                         ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
-                        ast::EntityReference::Slot => None,
+                        ast::EntityReference::Slot(_) => None,
                     },
                 )
             }
@@ -3002,7 +3002,9 @@ impl Policy {
             ast::EntityReference::EUID(euid) => EntityUid::ref_cast(euid),
             // PANIC SAFETY: This `unwrap` here is safe due the invariant (values total map) on policies.
             #[allow(clippy::unwrap_used)]
-            ast::EntityReference::Slot => EntityUid::ref_cast(self.ast.env().get(&slot).unwrap()),
+            ast::EntityReference::Slot(_) => {
+                EntityUid::ref_cast(self.ast.env().get(&slot).unwrap())
+            }
         }
     }
 
diff --git a/cedar-policy/src/ffi/utils.rs b/cedar-policy/src/ffi/utils.rs
index 9cb417a7..966f81e1 100644
--- a/cedar-policy/src/ffi/utils.rs
+++ b/cedar-policy/src/ffi/utils.rs
@@ -735,7 +735,7 @@ mod test {
             &err,
             &ExpectedErrorMessageBuilder::error("failed to parse policy from string")
                 .source("expected a static policy, got a template containing the slot ?principal")
-                .exactly_one_underline(src)
+                .exactly_one_underline("?principal")
                 .help("try removing the template slot(s) from this policy")
                 .build(),
         );
@@ -907,7 +907,7 @@ mod test {
                 "failed to parse policy with id `policy0` from string",
             )
             .source("expected a static policy, got a template containing the slot ?principal")
-            .exactly_one_underline("permit(principal == ?principal, action, resource);")
+            .exactly_one_underline("?principal")
             .help("try removing the template slot(s) from this policy")
             .build(),
         );


Test patch:
diff --git a/cedar-policy-core/src/test_utils.rs b/cedar-policy-core/src/test_utils.rs
index 61586856..f5d0d7bb 100644
--- a/cedar-policy-core/src/test_utils.rs
+++ b/cedar-policy-core/src/test_utils.rs
@@ -157,15 +157,11 @@ impl<'a> ExpectedErrorMessage<'a> {
     /// Return a boolean indicating whether a given error matches this expected message.
     /// (If you want to assert that it matches, use [`expect_err()`] instead,
     /// for much better assertion-failure messages.)
-    ///
-    /// `src` is the full source text (which the miette labels index into).
-    /// It can be omitted only in the case where we expect no underlines.
-    /// Panics if this invariant is violated.
-    pub fn matches(&self, src: Option<&'a str>, error: &impl miette::Diagnostic) -> bool {
+    pub fn matches(&self, error: &impl miette::Diagnostic) -> bool {
         self.matches_error(error)
             && self.matches_help(error)
             && self.matches_source(error)
-            && self.matches_underlines(src, error)
+            && self.matches_underlines(error)
     }
 
     /// Internal helper: whether the main error message matches
@@ -285,15 +281,16 @@ impl<'a> ExpectedErrorMessage<'a> {
     /// `src` is the full source text (which the miette labels index into).
     /// It can be omitted only in the case where we expect no underlines.
     /// Panics if this invariant is violated.
-    fn matches_underlines(&self, src: Option<&'a str>, err: &impl miette::Diagnostic) -> bool {
+    fn matches_underlines(&self, err: &impl miette::Diagnostic) -> bool {
         let expected_num_labels = self.underlines.len();
         let actual_num_labels = err.labels().map(|iter| iter.count()).unwrap_or(0);
         if expected_num_labels != actual_num_labels {
             return false;
         }
         if expected_num_labels != 0 {
-            let src =
-                src.expect("src can be `None` only in the case where we expect no underlines");
+            let src = err
+                .source_code()
+                .expect("err.source_code() should be `Some` if we are expecting underlines");
             for (expected, actual) in self
                 .underlines
                 .iter()
@@ -302,11 +299,8 @@ impl<'a> ExpectedErrorMessage<'a> {
                 let (expected_snippet, expected_label) = expected;
                 let actual_snippet = {
                     let span = actual.inner();
-                    if span.offset() < src.len() {
-                        &src[span.offset()..span.offset() + span.len()]
-                    } else {
-                        ""
-                    }
+                    std::str::from_utf8(src.read_span(span, 0, 0).expect("should read span").data())
+                        .expect("should be utf8 encoded")
                 };
                 let actual_label = actual.label();
                 if expected_snippet != &actual_snippet {