Skip to content

Commit

Permalink
Fix a bunch of Clippy lints (#420)
Browse files Browse the repository at this point in the history
  • Loading branch information
cdisselkoen authored Nov 10, 2023
1 parent 0487415 commit 2a86d36
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 38 deletions.
2 changes: 1 addition & 1 deletion cedar-policy-core/src/ast/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl std::fmt::Display for Pattern {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
for pc in self.elems.as_ref() {
match pc {
PatternElem::Char(c) if c == &'*' => write!(f, r#"\*"#)?,
PatternElem::Char('*') => write!(f, r#"\*"#)?,
PatternElem::Char(c) => write!(f, "{}", c.escape_debug())?,
PatternElem::Wildcard => write!(f, r#"*"#)?,
}
Expand Down
6 changes: 3 additions & 3 deletions cedar-policy-core/src/ast/policy_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ impl PolicySet {
//`template_ventry` is None, so `templates` has `t` and we never use the `HashSet::new()`
self.template_to_links_map
.entry(t.id().clone())
.or_insert_with(HashSet::new)
.or_default()
.insert(policy.id().clone());
}
if let Some(ventry) = link_ventry {
Expand Down Expand Up @@ -374,10 +374,10 @@ impl PolicySet {
self.templates.entry(new_id.clone()),
) {
(Entry::Vacant(links_entry), Entry::Vacant(_)) => {
//We will never use the HashSet::new() because we just found `t` above
//We will never use the .or_default() because we just found `t` above
self.template_to_links_map
.entry(template_id)
.or_insert_with(HashSet::new)
.or_default()
.insert(new_id);
Ok(links_entry.insert(r))
}
Expand Down
16 changes: 7 additions & 9 deletions cedar-policy-core/src/ast/restricted_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,7 @@ impl<'a> BorrowedRestrictedExpr<'a> {

/// Iterate over the elements of the set if this `RestrictedExpr` is a set,
/// or `None` if it is not a set
pub fn as_set_elements<'s>(
&'s self,
) -> Option<impl Iterator<Item = BorrowedRestrictedExpr<'s>>> {
pub fn as_set_elements(&self) -> Option<impl Iterator<Item = BorrowedRestrictedExpr<'_>>> {
match self.expr_kind() {
ExprKind::Set(set) => Some(set.iter().map(BorrowedRestrictedExpr::new_unchecked)), // since the RestrictedExpr invariant holds for the input set, it will hold for each element as well
_ => None,
Expand All @@ -249,9 +247,9 @@ impl<'a> BorrowedRestrictedExpr<'a> {

/// Iterate over the (key, value) pairs of the record if this
/// `RestrictedExpr` is a record, or `None` if it is not a record
pub fn as_record_pairs<'s>(
&'s self,
) -> Option<impl Iterator<Item = (&'s SmolStr, BorrowedRestrictedExpr<'s>)>> {
pub fn as_record_pairs(
&self,
) -> Option<impl Iterator<Item = (&'_ SmolStr, BorrowedRestrictedExpr<'_>)>> {
match self.expr_kind() {
ExprKind::Record(map) => Some(
map.iter()
Expand All @@ -264,9 +262,9 @@ impl<'a> BorrowedRestrictedExpr<'a> {
/// Get the name and args of the called extension function if this
/// `RestrictedExpr` is an extension function call, or `None` if it is not
/// an extension function call
pub fn as_extn_fn_call<'s>(
&'s self,
) -> Option<(&Name, impl Iterator<Item = BorrowedRestrictedExpr<'s>>)> {
pub fn as_extn_fn_call(
&self,
) -> Option<(&Name, impl Iterator<Item = BorrowedRestrictedExpr<'_>>)> {
match self.expr_kind() {
ExprKind::ExtensionFunctionApp { fn_name, args } => Some((
fn_name,
Expand Down
7 changes: 6 additions & 1 deletion cedar-policy-core/src/entities/json/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,19 @@ impl FromIterator<(SmolStr, CedarValueJson)> for JsonRecord {

impl JsonRecord {
/// Iterate over the (k, v) pairs in the record
pub fn iter<'s>(&'s self) -> impl Iterator<Item = (&'s SmolStr, &'s CedarValueJson)> {
pub fn iter(&self) -> impl Iterator<Item = (&'_ SmolStr, &'_ CedarValueJson)> {
self.values.iter()
}

/// Get the number of attributes in the record
pub fn len(&self) -> usize {
self.values.len()
}

/// Is the record empty (no attributes)
pub fn is_empty(&self) -> bool {
self.values.is_empty()
}
}

/// Structure expected by the `__entity` escape
Expand Down
4 changes: 2 additions & 2 deletions cedar-policy-core/src/est/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1537,13 +1537,13 @@ fn display_cedarvaluejson(f: &mut std::fmt::Formatter<'_>, v: &CedarValueJson) -
});
match style {
Some(ast::CallStyle::MethodStyle) => {
display_cedarvaluejson(f, &arg)?;
display_cedarvaluejson(f, arg)?;
write!(f, ".{ext_fn}()")?;
Ok(())
}
Some(ast::CallStyle::FunctionStyle) | None => {
write!(f, "{ext_fn}(")?;
display_cedarvaluejson(f, &arg)?;
display_cedarvaluejson(f, arg)?;
write!(f, ")")?;
Ok(())
}
Expand Down
9 changes: 6 additions & 3 deletions cedar-policy-core/src/extensions/ipaddr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,16 @@ impl IPAddr {

fn parse_prefix(s: &str, max: u8, max_len: u8) -> Result<u8, String> {
if s.len() > max_len as usize {
return Err(format!("error parsing prefix: string length is too large"));
return Err(format!(
"error parsing prefix: string length {} is too large",
s.len()
));
}
if s.chars().any(|c| !c.is_ascii_digit()) {
return Err(format!("error parsing prefix: encountered non-digit"));
return Err(format!("error parsing prefix `{s}`: encountered non-digit"));
}
if s.starts_with('0') && s != "0" {
return Err(format!("error parsing prefix: leading zero(s)"));
return Err(format!("error parsing prefix `{s}`: leading zero(s)"));
}
let res: u8 = s
.parse()
Expand Down
6 changes: 3 additions & 3 deletions cedar-policy-validator/src/coreschema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ impl ast::RequestSchema for ValidatorSchema {
// the remaining checks require knowing about the action.
match request.action() {
EntityUIDEntry::Concrete(action) => {
let validator_action_id = self.get_action_id(&*action).ok_or_else(|| {
let validator_action_id = self.get_action_id(action).ok_or_else(|| {
RequestValidationError::UndeclaredAction {
action: Arc::clone(action),
}
Expand Down Expand Up @@ -224,10 +224,10 @@ impl ast::RequestSchema for ValidatorSchema {

impl<'a> ast::RequestSchema for CoreSchema<'a> {
type Error = RequestValidationError;
fn validate_request<'e>(
fn validate_request(
&self,
request: &ast::Request,
extensions: Extensions<'e>,
extensions: Extensions<'_>,
) -> Result<(), Self::Error> {
self.schema.validate_request(request, extensions)
}
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy-validator/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ impl Type {
Some(pairs) => {
let record: HashMap<_, BorrowedRestrictedExpr<'_>> = pairs.collect();
for (k, attr_val) in &record {
match attrs.get_attr(&k) {
match attrs.get_attr(k) {
Some(attr_ty) => {
if !attr_ty
.attr_type
Expand Down
19 changes: 8 additions & 11 deletions cedar-policy/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1977,17 +1977,14 @@ impl PolicySet {
// _before_ calling `self.ast.link` because `link` mutates the policy
// set by creating a new link entry in a hashmap. This happens even when
// trying to link a static policy, which we want to error on here.
let template = match self.templates.get(&template_id) {
Some(template) => template,
None => {
return Err(if self.policies.contains_key(&template_id) {
PolicySetError::ExpectedTemplate
} else {
PolicySetError::LinkingError(ast::LinkingError::NoSuchTemplate {
id: template_id.0,
})
});
}
let Some(template) = self.templates.get(&template_id) else {
return Err(if self.policies.contains_key(&template_id) {
PolicySetError::ExpectedTemplate
} else {
PolicySetError::LinkingError(ast::LinkingError::NoSuchTemplate {
id: template_id.0,
})
});
};

let linked_ast = self
Expand Down
8 changes: 4 additions & 4 deletions cedar-policy/src/frontend/is_authorized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ fn parse_instantiations(
};
}
match policies.link(template_id, instance_id, vals) {
Ok(_) => Ok(()),
Ok(()) => Ok(()),
Err(e) => Err(vec![format!("Error instantiating template: {e}")]),
}
}
Expand Down Expand Up @@ -397,7 +397,7 @@ impl RecvdSlice {
if let Some(t_inst_list) = template_instantiations {
for instantiation in t_inst_list {
match parse_instantiations(&mut policies, instantiation) {
Ok(_) => (),
Ok(()) => (),
Err(err) => errs.extend(err),
}
}
Expand All @@ -420,7 +420,7 @@ fn parse_policy_set_from_individual_policies(
for (id, policy_src) in policies {
match Policy::parse(Some(id.clone()), policy_src) {
Ok(p) => match policy_set.add(p) {
Ok(_) => {}
Ok(()) => {}
Err(err) => {
errs.push(format!("couldn't add policy to set due to error: {err}"));
}
Expand All @@ -436,7 +436,7 @@ fn parse_policy_set_from_individual_policies(
for (id, policy_src) in templates {
match Template::parse(Some(id.clone()), policy_src) {
Ok(p) => match policy_set.add_template(p) {
Ok(_) => {}
Ok(()) => {}
Err(err) => {
errs.push(format!("couldn't add policy to set due to error: {err}"));
}
Expand Down
5 changes: 5 additions & 0 deletions cedar-policy/src/integration_testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ fn constant_true() -> bool {
/// For relative paths, return the absolute path, assuming that the path
/// is relative to the root of the `CedarIntegrationTests` repo.
/// For absolute paths, return them unchanged.
///
/// # Panics
///
/// Panics if the environment variable `CARGO_MANIFEST_DIR` is not set.
/// This variable should be set by Cargo at build-time.
pub fn resolve_integration_test_path(path: impl AsRef<Path>) -> PathBuf {
if path.as_ref().is_relative() {
let mut full_path = PathBuf::new();
Expand Down

0 comments on commit 2a86d36

Please sign in to comment.