Skip to content

Commit fbc9b7c

Browse files
shaobo-he-awsshaobo-he-aws
committed
clean ups
Signed-off-by: Shaobo He <[email protected]>
1 parent 801b67a commit fbc9b7c

File tree

5 files changed

+36
-36
lines changed

5 files changed

+36
-36
lines changed

‎cedar-policy-core/src/ast/policy.rs‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,19 +1193,22 @@ impl TemplateBody {
11931193
/// the negation of each of the policy's "unless" conditions.
11941194
pub fn condition(&self) -> Expr {
11951195
match self {
1196-
TemplateBody::TemplateBody(TemplateBodyImpl { .. }) => Expr::and(
1197-
self.principal_constraint_expr(),
1196+
TemplateBody::TemplateBody(TemplateBodyImpl { .. }) => {
1197+
let loc = self.loc().cloned();
11981198
Expr::and(
1199-
self.action_constraint_expr(),
1199+
self.principal_constraint_expr(),
12001200
Expr::and(
1201-
self.resource_constraint_expr(),
1202-
self.non_scope_constraints().clone(),
1201+
self.action_constraint_expr(),
1202+
Expr::and(
1203+
self.resource_constraint_expr(),
1204+
self.non_scope_constraints().clone(),
1205+
)
1206+
.with_maybe_source_loc(loc.clone()),
12031207
)
1204-
.with_maybe_source_loc(self.loc().cloned()),
1208+
.with_maybe_source_loc(loc.clone()),
12051209
)
1206-
.with_maybe_source_loc(self.loc().cloned()),
1207-
)
1208-
.with_maybe_source_loc(self.loc().cloned()),
1210+
.with_maybe_source_loc(loc)
1211+
}
12091212
#[cfg(feature = "tolerant-ast")]
12101213
TemplateBody::TemplateBodyError(_, _) => DEFAULT_ERROR_EXPR.as_ref().clone(),
12111214
}

‎cedar-policy-core/src/evaluator.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,10 @@ impl<'e> RestrictedEvaluator<'e> {
259259
// also, if there is an error, set its source location to the source
260260
// location of the input expression as well, unless it already had a
261261
// more specific location
262-
res.map(|pval| pval.with_maybe_source_loc(expr.source_loc().cloned()))
262+
let expr_loc = expr.source_loc().cloned();
263+
res.map(|pval| pval.with_maybe_source_loc(expr_loc.clone()))
263264
.map_err(|err| match err.source_loc().cloned() {
264-
None => err.with_maybe_source_loc(expr.source_loc().cloned()),
265+
None => err.with_maybe_source_loc(expr_loc),
265266
Some(_) => err,
266267
})
267268
}
@@ -432,9 +433,10 @@ impl<'e> Evaluator<'e> {
432433
// also, if there is an error, set its source location to the source
433434
// location of the input expression as well, unless it already had a
434435
// more specific location
435-
res.map(|pval| pval.with_maybe_source_loc(expr.source_loc().cloned()))
436+
let expr_loc = expr.source_loc().cloned();
437+
res.map(|pval| pval.with_maybe_source_loc(expr_loc.clone()))
436438
.map_err(|err| match err.source_loc().cloned() {
437-
None => err.with_maybe_source_loc(expr.source_loc().cloned()),
439+
None => err.with_maybe_source_loc(expr_loc),
438440
Some(_) => err,
439441
})
440442
}

‎cedar-policy-core/src/validator/cedar_schema/ast.rs‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
use std::{collections::BTreeMap, iter::once, sync::Arc};
17+
use std::{collections::BTreeMap, iter::once};
1818

1919
use crate::{
2020
ast::{Annotation, Annotations, AnyId, Id, InternalName},
@@ -57,14 +57,8 @@ pub fn deduplicate_annotations<T>(
5757
if let Some((old_key, _)) = unique_annotations.get_key_value(&key) {
5858
return Err(UserError::DuplicateAnnotations(
5959
key.node,
60-
Node::with_source_loc(
61-
(),
62-
old_key
63-
.loc
64-
.clone()
65-
.unwrap_or_else(|| Loc::new(0..0, Arc::from(""))),
66-
),
67-
Node::with_source_loc((), key.loc.unwrap_or_else(|| Loc::new(0..0, Arc::from("")))),
60+
Node::with_maybe_source_loc((), old_key.loc.clone()),
61+
Node::with_maybe_source_loc((), key.loc),
6862
));
6963
} else {
7064
unique_annotations.insert(key, value);
@@ -91,24 +85,24 @@ pub struct Path(Node<PathInternal>);
9185
impl Path {
9286
/// Create a [`Path`] with a single entry
9387
pub fn single(basename: Id, loc: Option<Loc>) -> Self {
94-
Self(Node::with_source_loc(
88+
Self(Node::with_maybe_source_loc(
9589
PathInternal {
9690
basename,
9791
namespace: vec![],
9892
},
99-
loc.unwrap_or_else(|| Loc::new(0..0, Arc::from(""))),
93+
loc,
10094
))
10195
}
10296

10397
/// Create [`Path`] with a head and an iterator. Most significant name first.
10498
pub fn new(basename: Id, namespace: impl IntoIterator<Item = Id>, loc: Option<Loc>) -> Self {
10599
let namespace = namespace.into_iter().collect();
106-
Self(Node::with_source_loc(
100+
Self(Node::with_maybe_source_loc(
107101
PathInternal {
108102
basename,
109103
namespace,
110104
},
111-
loc.unwrap_or_else(|| Loc::new(0..0, Arc::from(""))),
105+
loc,
112106
))
113107
}
114108

‎cedar-policy-core/src/validator/cedar_schema/grammar.lalrpop‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ AppDecls: Node<NonEmpty<Node<AppDecl>>> = {
203203
// Type := PRIMTYPE | Path | SetType | RecType
204204
pub Type: Node<SType> = {
205205
<p:Path>
206-
=> { let loc = p.loc().cloned().unwrap_or_else(|| Loc::new(0..0, Arc::from(""))); Node::with_source_loc(SType::Ident(p), loc) },
206+
=> { let loc = p.loc().cloned(); Node::with_maybe_source_loc(SType::Ident(p), loc) },
207207
<l:@L> SET "<" <t:Type> ">" <r:@R>
208208
=> Node::with_source_loc(SType::Set(Box::new(t)), Loc::new(l..r, Arc::clone(src))),
209209
<l:@L> "{" <ds:AttrDecls?> "}" <r:@R>
@@ -228,8 +228,8 @@ Comma<E>: Vec<E> = {
228228

229229
// IDENT := ['_''a'-'z''A'-'Z']['_''a'-'z''A'-'Z''0'-'9']* - RESERVED
230230
Ident: Node<Id> = {
231-
<id: AnyIdent> =>? Id::from_str(id.node.as_ref()).map(|i| Node::with_source_loc(i, id.loc.clone().unwrap_or_else(|| Loc::new(0..0, Arc::from(""))))).map_err(|err : crate::parser::err::ParseErrors| ParseError::User {
232-
error: UserError::ReservedIdentifierUsed(Node::with_source_loc(id.node.to_smolstr(), id.loc.clone().unwrap_or_else(|| Loc::new(0..0, Arc::from("")))))
231+
<id: AnyIdent> =>? Id::from_str(id.node.as_ref()).map(|i| Node::with_maybe_source_loc(i, id.loc.clone())).map_err(|err : crate::parser::err::ParseErrors| ParseError::User {
232+
error: UserError::ReservedIdentifierUsed(Node::with_maybe_source_loc(id.node.to_smolstr(), id.loc.clone()))
233233
}),
234234
}
235235

‎cedar-policy-core/src/validator/typecheck.rs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,6 +1193,7 @@ impl<'a> SingleEnvTypechecker<'a> {
11931193
let ExprKind::BinaryApp { op, arg1, arg2 } = bin_expr.expr_kind() else {
11941194
panic!("`typecheck_binary` called with an expression kind other than `BinaryApp`");
11951195
};
1196+
let bin_expr_loc = bin_expr.source_loc().cloned();
11961197

11971198
match op {
11981199
// The arguments to `==` may typecheck with any type, but we will
@@ -1531,7 +1532,7 @@ impl<'a> SingleEnvTypechecker<'a> {
15311532
// should be unreachable, as we already typechecked that this matches
15321533
// `Type::any_entity_reference()`
15331534
type_errors.push(ValidationError::internal_invariant_violation(
1534-
bin_expr.source_loc().cloned(),
1535+
bin_expr_loc.clone(),
15351536
self.policy_id.clone(),
15361537
));
15371538
return TypecheckAnswer::fail(
@@ -1550,7 +1551,7 @@ impl<'a> SingleEnvTypechecker<'a> {
15501551
// Not an entity type; should be unreachable, as we already typechecked
15511552
// that this matches `Type::any_entity_reference()`
15521553
type_errors.push(ValidationError::internal_invariant_violation(
1553-
bin_expr.source_loc().cloned(),
1554+
bin_expr_loc.clone(),
15541555
self.policy_id.clone(),
15551556
));
15561557
return TypecheckAnswer::fail(
@@ -1602,7 +1603,7 @@ impl<'a> SingleEnvTypechecker<'a> {
16021603
// should be unreachable, as we already typechecked that this matches
16031604
// `Type::any_entity_reference()`
16041605
type_errors.push(ValidationError::internal_invariant_violation(
1605-
bin_expr.source_loc().cloned(),
1606+
bin_expr_loc.clone(),
16061607
self.policy_id.clone(),
16071608
));
16081609
return TypecheckAnswer::fail(
@@ -1622,7 +1623,7 @@ impl<'a> SingleEnvTypechecker<'a> {
16221623
// `Type::any_entity_reference()`
16231624
type_errors.push(
16241625
ValidationError::internal_invariant_violation(
1625-
bin_expr.source_loc().cloned(),
1626+
bin_expr_loc.clone(),
16261627
self.policy_id.clone(),
16271628
),
16281629
);
@@ -1646,7 +1647,7 @@ impl<'a> SingleEnvTypechecker<'a> {
16461647
EntityRecordKind::Record { .. } => None,
16471648
};
16481649
type_errors.push(ValidationError::no_tags_allowed(
1649-
bin_expr.source_loc().cloned(),
1650+
bin_expr_loc.clone(),
16501651
self.policy_id.clone(),
16511652
entity_ty.cloned(),
16521653
));
@@ -1667,7 +1668,7 @@ impl<'a> SingleEnvTypechecker<'a> {
16671668
Ok(ty) => ty,
16681669
Err(e) => {
16691670
type_errors.push(ValidationError::incompatible_types(
1670-
bin_expr.source_loc().cloned(),
1671+
bin_expr_loc.clone(),
16711672
self.policy_id.clone(),
16721673
tag_types.into_iter().cloned(),
16731674
e,
@@ -1688,7 +1689,7 @@ impl<'a> SingleEnvTypechecker<'a> {
16881689
}
16891690
} else {
16901691
type_errors.push(ValidationError::unsafe_tag_access(
1691-
bin_expr.source_loc().cloned(),
1692+
bin_expr_loc.clone(),
16921693
self.policy_id.clone(),
16931694
match kind {
16941695
EntityRecordKind::Entity(lub) => Some(lub.clone()),

0 commit comments

Comments
 (0)