Skip to content

Commit dc748b3

Browse files
shaobo-he-awskhieta
authored andcommitted
Fix formatter soundness check (#865)
Signed-off-by: Shaobo He <[email protected]>
1 parent 7e9c0dc commit dc748b3

File tree

1 file changed

+58
-7
lines changed
  • cedar-policy-formatter/src/pprint

1 file changed

+58
-7
lines changed

cedar-policy-formatter/src/pprint/fmt.rs

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

17+
use std::collections::BTreeMap;
18+
1719
use miette::{miette, Result, WrapErr};
1820

19-
use cedar_policy_core::ast::{PolicySet, Template};
21+
use cedar_policy_core::ast::PolicySet;
2022
use cedar_policy_core::parser::parse_policyset;
2123
use cedar_policy_core::parser::{err::ParseErrors, text_to_cst::parse_policies};
24+
use smol_str::ToSmolStr;
2225

2326
use crate::token::get_comment;
2427

@@ -42,15 +45,19 @@ fn tree_to_pretty<T: Doc>(t: &T, context: &mut config::Context<'_>) -> Result<St
4245
fn soundness_check(ps: &str, ast: &PolicySet) -> Result<()> {
4346
let formatted_ast = parse_policyset(ps).wrap_err("formatter produces invalid policies")?;
4447
let (formatted_policies, policies) = (
45-
formatted_ast.templates().collect::<Vec<&Template>>(),
46-
ast.templates().collect::<Vec<&Template>>(),
48+
formatted_ast
49+
.policies()
50+
.map(|p| (p.id().to_smolstr(), p))
51+
.collect::<BTreeMap<_, _>>(),
52+
ast.policies()
53+
.map(|p| (p.id().to_smolstr(), p))
54+
.collect::<BTreeMap<_, _>>(),
4755
);
4856

4957
if formatted_policies.len() != policies.len() {
5058
return Err(miette!("missing formatted policies"));
5159
}
52-
53-
for (f_p, p) in formatted_policies.into_iter().zip(policies.into_iter()) {
60+
for ((f_p_id, f_p), (p_id, p)) in formatted_policies.into_iter().zip(policies.into_iter()) {
5461
let (f_anno, anno) = (
5562
f_p.annotations()
5663
.map(|(k, v)| (k, &v.val))
@@ -59,7 +66,8 @@ fn soundness_check(ps: &str, ast: &PolicySet) -> Result<()> {
5966
.map(|(k, v)| (k, &v.val))
6067
.collect::<std::collections::BTreeMap<_, _>>(),
6168
);
62-
if !(f_anno == anno
69+
if !(f_p_id == p_id
70+
&& f_anno == anno
6371
&& f_p.effect() == p.effect()
6472
&& f_p.principal_constraint() == p.principal_constraint()
6573
&& f_p.action_constraint() == p.action_constraint()
@@ -69,7 +77,7 @@ fn soundness_check(ps: &str, ast: &PolicySet) -> Result<()> {
6977
.eq_shape(p.non_head_constraints()))
7078
{
7179
return Err(miette!(
72-
"policies differ in meaning or annotations:\noriginal: {p}\nformatted: {f_p}"
80+
"policies differ in policy ids or meaning or annotations:\noriginal: {p}\nformatted: {f_p}"
7381
));
7482
}
7583
}
@@ -167,6 +175,49 @@ mod tests {
167175
);
168176
}
169177

178+
#[test]
179+
fn test_soundness_check() {
180+
let p1 = r#"permit (principal, action, resource)
181+
when { "
182+
183+
a
184+
" };"#;
185+
let p2 = r#"permit (principal, action, resource)
186+
when { "
187+
a
188+
" };"#;
189+
assert!(soundness_check(p2, &parse_policyset(p1).unwrap()).is_err());
190+
191+
let p1 = r#"
192+
permit (principal, action, resource)
193+
when { "a"};
194+
permit (principal, action, resource)
195+
when { "
196+
197+
a
198+
" };"#;
199+
let p2 = r#"
200+
permit (principal, action, resource)
201+
when { "
202+
a
203+
" };
204+
permit (principal, action, resource)
205+
when { "a"};"#;
206+
assert!(soundness_check(p2, &parse_policyset(p1).unwrap()).is_err());
207+
208+
let p1 = r#"
209+
permit (principal, action, resource)
210+
when { "a" };
211+
permit (principal, action, resource)
212+
when { "b" };"#;
213+
let p2 = r#"
214+
permit (principal, action, resource)
215+
when { "a" };
216+
permit (principal, action, resource)
217+
when { "b"};"#;
218+
assert!(soundness_check(p2, &parse_policyset(p1).unwrap()).is_ok());
219+
}
220+
170221
#[test]
171222
fn test_format_files() {
172223
use std::fs::read_to_string;

0 commit comments

Comments
 (0)