Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into cdisselkoen/simplify-…
Browse files Browse the repository at this point in the history
…typechecker

Signed-off-by: Craig Disselkoen <[email protected]>
  • Loading branch information
cdisselkoen committed Dec 31, 2024
2 parents 478a481 + d1f1185 commit 9be9906
Show file tree
Hide file tree
Showing 73 changed files with 2,713 additions and 1,161 deletions.
164 changes: 71 additions & 93 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ rust-2018-idioms = "deny"
# means we'll see it in local runs.
[workspace.lints.clippy]
nursery = { level = "warn", priority = -1 }
needless_pass_by_value = "warn"

# These lints may be worth enforcing, but cause a lot of noise at the moment.
use_self = "allow"
Expand Down
6 changes: 3 additions & 3 deletions cedar-policy-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
miette = { version = "7.4.0", features = ["fancy"] }
thiserror = "2.0"
semver = "1.0.23"
semver = "1.0.24"
prost = {version = "0.13", optional = true}

[build-dependencies]
Expand All @@ -37,8 +37,8 @@ protobufs = ["dep:prost", "dep:prost-build", "cedar-policy/protobufs", "cedar-po
[dev-dependencies]
assert_cmd = "2.0"
tempfile = "3"
glob = "0.3.1"
predicates = "3.1.0"
glob = "0.3.2"
predicates = "3.1.3"
rstest = "0.23.0"

# We override the name of the binary for src/main.rs, which otherwise would be
Expand Down
8 changes: 4 additions & 4 deletions cedar-policy-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,7 @@ fn translate_policy_inner(args: &TranslatePolicyArgs) -> Result<String> {
let translate = match args.direction {
PolicyTranslationDirection::CedarToJson => translate_policy_to_json,
};
read_from_file_or_stdin(args.input_file.clone(), "policy").and_then(translate)
read_from_file_or_stdin(args.input_file.as_ref(), "policy").and_then(translate)
}

pub fn translate_policy(args: &TranslatePolicyArgs) -> CedarExitCode {
Expand Down Expand Up @@ -984,7 +984,7 @@ fn translate_schema_inner(args: &TranslateSchemaArgs) -> Result<String> {
SchemaTranslationDirection::JsonToCedar => translate_schema_to_cedar,
SchemaTranslationDirection::CedarToJson => translate_schema_to_json,
};
read_from_file_or_stdin(args.input_file.clone(), "schema").and_then(translate)
read_from_file_or_stdin(args.input_file.as_ref(), "schema").and_then(translate)
}

pub fn translate_schema(args: &TranslateSchemaArgs) -> CedarExitCode {
Expand Down Expand Up @@ -1377,7 +1377,7 @@ fn load_entities(entities_filename: impl AsRef<Path>, schema: Option<&Schema>) -
/// This will rename template-linked policies to the id of their template, which may
/// cause id conflicts, so only call this function before instancing
/// templates into the policy set.
fn rename_from_id_annotation(ps: PolicySet) -> Result<PolicySet> {
fn rename_from_id_annotation(ps: &PolicySet) -> Result<PolicySet> {
let mut new_ps = PolicySet::new();
let t_iter = ps.templates().map(|t| match t.annotation("id") {
None => Ok(t.clone()),
Expand Down Expand Up @@ -1443,7 +1443,7 @@ fn read_cedar_policy_set(
Report::new(err).with_source_code(NamedSource::new(name, ps_str))
})
.wrap_err_with(|| format!("failed to parse {context}"))?;
rename_from_id_annotation(ps)
rename_from_id_annotation(&ps)
}

/// Read a policy set, static policy or policy template, in Cedar JSON (EST) syntax, from the file given
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ mod test {
let test_data_root = PathBuf::from(r"../sample-data/sandbox_b");
let mut schema_file = test_data_root.clone();
schema_file.push("schema.cedarschema");
let mut old_policies_file = test_data_root.clone();
let mut old_policies_file = test_data_root;
old_policies_file.push("policies_4.cedar");
let new_policies_file = old_policies_file.clone();

Expand Down
22 changes: 8 additions & 14 deletions cedar-policy-cli/tests/sample.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn run_link_test(
links_file: impl Into<String>,
template_id: impl Into<String>,
linked_id: impl Into<String>,
env: HashMap<SlotId, String>,
env: impl IntoIterator<Item = (SlotId, String)>,
expected: CedarExitCode,
) {
let cmd = LinkArgs {
Expand All @@ -117,7 +117,9 @@ fn run_link_test(
},
template_id: template_id.into(),
new_id: linked_id.into(),
arguments: Arguments { data: env },
arguments: Arguments {
data: HashMap::from_iter(env),
},
};
let output = link(&cmd);
assert_eq!(output, expected);
Expand Down Expand Up @@ -792,9 +794,7 @@ fn test_link_samples() {
&linked_file_name,
"AccessVacation",
"AliceAccess",
[(SlotId::principal(), "User::\"alice\"".to_string())]
.into_iter()
.collect(),
[(SlotId::principal(), "User::\"alice\"".to_string())],
CedarExitCode::Failure,
);

Expand All @@ -803,9 +803,7 @@ fn test_link_samples() {
&linked_file_name,
"AccessVacation",
"AliceAccess",
[(SlotId::principal(), "invalid".to_string())]
.into_iter()
.collect(),
[(SlotId::principal(), "invalid".to_string())],
CedarExitCode::Failure,
);

Expand All @@ -814,9 +812,7 @@ fn test_link_samples() {
&linked_file_name,
"AccessVacation",
"AliceAccess",
[(SlotId::principal(), "User::\"alice\"".to_string())]
.into_iter()
.collect(),
[(SlotId::principal(), "User::\"alice\"".to_string())],
CedarExitCode::Success,
);

Expand Down Expand Up @@ -845,9 +841,7 @@ fn test_link_samples() {
&linked_file_name,
"AccessVacation",
"BobAccess",
[(SlotId::principal(), "User::\"bob\"".to_string())]
.into_iter()
.collect(),
[(SlotId::principal(), "User::\"bob\"".to_string())],
CedarExitCode::Success,
);

Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ repository.workspace = true

[dependencies]
serde = { version = "1.0", features = ["derive", "rc"] }
serde_with = { version = "3.0", features = ["json"] }
serde_with = { version = "3.12", features = ["json"] }
serde_json = "1.0"
lalrpop-util = { version = "0.22.0", features = ["lexer"] }
lazy_static = "1.4"
Expand Down
2 changes: 2 additions & 0 deletions cedar-policy-core/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,5 @@ mod value;
pub use value::*;
mod expr_iterator;
pub use expr_iterator::*;
mod annotation;
pub use annotation::*;
182 changes: 182 additions & 0 deletions cedar-policy-core/src/ast/annotation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/*
* Copyright Cedar Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

use std::collections::BTreeMap;

use educe::Educe;
use serde::{Deserialize, Serialize};
use smol_str::SmolStr;

use crate::parser::Loc;

use super::AnyId;

/// Struct which holds the annotations for a policy
#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
pub struct Annotations(
#[serde(default)]
#[serde(skip_serializing_if = "BTreeMap::is_empty")]
#[serde(with = "::serde_with::rust::maps_duplicate_key_is_error")]
BTreeMap<AnyId, Annotation>,
);

impl std::fmt::Display for Annotations {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
for (k, v) in &self.0 {
writeln!(f, "@{k}({v})")?
}
Ok(())
}
}

impl Annotations {
/// Create a new empty `Annotations` (with no annotations)
pub fn new() -> Self {
Self(BTreeMap::new())
}

/// Get an annotation by key
pub fn get(&self, key: &AnyId) -> Option<&Annotation> {
self.0.get(key)
}

/// Iterate over all annotations
pub fn iter(&self) -> impl Iterator<Item = (&AnyId, &Annotation)> {
self.0.iter()
}

/// Tell if it's empty
pub fn is_empty(&self) -> bool {
self.0.is_empty()
}
}

/// Wraps the [`BTreeMap`] into an opaque type so we can change it later if need be
#[derive(Debug)]
pub struct IntoIter(std::collections::btree_map::IntoIter<AnyId, Annotation>);

impl Iterator for IntoIter {
type Item = (AnyId, Annotation);

fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}
}

impl IntoIterator for Annotations {
type Item = (AnyId, Annotation);

type IntoIter = IntoIter;

fn into_iter(self) -> Self::IntoIter {
IntoIter(self.0.into_iter())
}
}

impl Default for Annotations {
fn default() -> Self {
Self::new()
}
}

impl FromIterator<(AnyId, Annotation)> for Annotations {
fn from_iter<T: IntoIterator<Item = (AnyId, Annotation)>>(iter: T) -> Self {
Self(BTreeMap::from_iter(iter))
}
}

impl From<BTreeMap<AnyId, Annotation>> for Annotations {
fn from(value: BTreeMap<AnyId, Annotation>) -> Self {
Self(value)
}
}

/// Struct which holds the value of a particular annotation
#[derive(Educe, Clone, Debug, Serialize, Deserialize, Default)]
#[educe(Hash, PartialEq, Eq, PartialOrd, Ord)]
#[serde(transparent)]
#[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
#[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
pub struct Annotation {
/// Annotation value
pub val: SmolStr,
/// Source location. Note this is the location of _the entire key-value
/// pair_ for the annotation, not just `val` above
#[serde(skip)]
#[educe(Hash(ignore))]
#[educe(PartialEq(ignore))]
#[educe(PartialOrd(ignore))]
pub loc: Option<Loc>,
}

impl std::fmt::Display for Annotation {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "\"{}\"", self.val.escape_debug())
}
}

impl Annotation {
/// Construct an Annotation with an optional value. This function is used
/// to construct annotations from the CST and EST representation where a
/// value is not required, but an absent value is equivalent to `""`.
/// Here, a `None` constructs an annotation containing the value `""`.`
pub fn with_optional_value(val: Option<SmolStr>, loc: Option<Loc>) -> Self {
Self {
val: val.unwrap_or_default(),
loc,
}
}
}

impl AsRef<str> for Annotation {
fn as_ref(&self) -> &str {
&self.val
}
}

#[cfg(feature = "protobufs")]
impl From<&crate::ast::proto::Annotation> for Annotation {
fn from(v: &crate::ast::proto::Annotation) -> Self {
Self {
val: v.val.clone().into(),
loc: None,
}
}
}

#[cfg(feature = "protobufs")]
impl From<&Annotation> for crate::ast::proto::Annotation {
fn from(v: &Annotation) -> Self {
Self {
val: v.val.to_string(),
}
}
}

#[cfg(feature = "arbitrary")]
impl<'a> arbitrary::Arbitrary<'a> for Annotation {
fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
Ok(Self {
val: u.arbitrary::<&str>()?.into(),
loc: None,
})
}

fn size_hint(depth: usize) -> (usize, Option<usize>) {
<&str as arbitrary::Arbitrary>::size_hint(depth)
}
}
8 changes: 8 additions & 0 deletions cedar-policy-core/src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,14 @@ impl<T: Clone> std::fmt::Display for Expr<T> {
}
}

impl<T: Clone> BoundedDisplay for Expr<T> {
fn fmt(&self, f: &mut impl std::fmt::Write, n: Option<usize>) -> std::fmt::Result {
// Like the `std::fmt::Display` impl, we convert to EST and use the EST
// pretty-printer. Note that converting AST->EST is lossless and infallible.
BoundedDisplay::fmt(&crate::est::Expr::from(self.clone()), f, n)
}
}

impl std::str::FromStr for Expr {
type Err = ParseErrors;

Expand Down
Loading

0 comments on commit 9be9906

Please sign in to comment.