Skip to content

Commit

Permalink
chore: enforce clippy::semicolon_if_nothing_returned linting rule (#…
Browse files Browse the repository at this point in the history
…1139)

* chore: warn on `clippy::semicolon_if_nothing_returned`

* chore: address warnings
  • Loading branch information
TomAFrench authored Apr 12, 2023
1 parent daf5c9d commit 24adcc0
Show file tree
Hide file tree
Showing 33 changed files with 67 additions and 55 deletions.
1 change: 1 addition & 0 deletions crates/arena/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

// For now we use a wrapper around generational-arena
pub use generational_arena::{Arena, Index};
3 changes: 2 additions & 1 deletion crates/fm/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

mod file_map;
mod file_reader;
Expand Down Expand Up @@ -136,7 +137,7 @@ mod tests {

let file_id = fm.add_file(&file_path, FileType::Normal).unwrap();

assert!(fm.path(file_id).ends_with("foo"))
assert!(fm.path(file_id).ends_with("foo"));
}
#[test]
fn path_resolve_sub_module() {
Expand Down
1 change: 1 addition & 0 deletions crates/iter-extended/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

use std::collections::BTreeMap;

Expand Down
1 change: 1 addition & 0 deletions crates/nargo/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

//! Nargo is the package manager for Noir
//! This name was used because it sounds like `cargo` and
Expand Down
1 change: 1 addition & 0 deletions crates/nargo_cli/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

//! Nargo is the package manager for Noir
//! This name was used because it sounds like `cargo` and
Expand Down
7 changes: 4 additions & 3 deletions crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

use std::{collections::BTreeMap, str};

Expand Down Expand Up @@ -283,11 +284,11 @@ impl Abi {
InputValue::String(string) => {
let str_as_fields =
string.bytes().map(|byte| FieldElement::from_be_bytes_reduce(&[byte]));
encoded_value.extend(str_as_fields)
encoded_value.extend(str_as_fields);
}
InputValue::Struct(object) => {
for value in object.into_values() {
encoded_value.extend(Self::encode_value(value)?)
encoded_value.extend(Self::encode_value(value)?);
}
}
}
Expand Down Expand Up @@ -442,6 +443,6 @@ mod test {
}

// We also decode the return value (we can do this immediately as we know it shares a witness with an input).
assert_eq!(return_value.unwrap(), reconstructed_inputs["thing2"])
assert_eq!(return_value.unwrap(), reconstructed_inputs["thing2"]);
}
}
1 change: 1 addition & 0 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

use acvm::Language;
use clap::Args;
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_errors/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

mod position;
pub mod reporter;
Expand Down
3 changes: 2 additions & 1 deletion crates/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

mod errors;
mod ssa;
Expand Down Expand Up @@ -253,7 +254,7 @@ impl Evaluator {
let new_name = format!("{name}.{inner_name}");
new_fields.insert(new_name, value.clone());
}
self.generate_struct_witnesses(struct_witnesses, &new_fields)?
self.generate_struct_witnesses(struct_witnesses, &new_fields)?;
}
AbiType::String { length } => {
let typ = AbiType::Integer { sign: noirc_abi::Sign::Unsigned, width: 8 };
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ pub(crate) fn bound_constraint_with_offset(
0 => evaluator.push_opcode(AcirOpcode::Arithmetic(aof)),
1 => {
let expr = boolean_expr(&aof, evaluator);
evaluator.push_opcode(AcirOpcode::Arithmetic(expr))
evaluator.push_opcode(AcirOpcode::Arithmetic(expr));
}
2 => {
let y = expression_to_witness(boolean_expr(&aof, evaluator), evaluator);
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl InternalVar {
&self.expression
}
pub(crate) fn set_id(&mut self, id: NodeId) {
self.id = Some(id)
self.id = Some(id);
}
pub(crate) fn get_id(&self) -> Option<NodeId> {
self.id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ fn prepare_inputs(
let mut inputs: Vec<FunctionInput> = Vec::new();

for argument in arguments {
inputs.extend(resolve_node_id(argument, acir_gen, cfg, evaluator))
inputs.extend(resolve_node_id(argument, acir_gen, cfg, evaluator));
}
inputs
}
Expand Down Expand Up @@ -212,7 +212,7 @@ fn resolve_array(
arr_element.set_witness(witness);
acir_gen.memory.insert(array.id, i, arr_element);

inputs.push(func_input)
inputs.push(func_input);
}

inputs
Expand Down Expand Up @@ -329,7 +329,7 @@ fn evaluate_println(
fn format_field_string(field: FieldElement) -> String {
let mut trimmed_field = field.to_hex().trim_start_matches('0').to_owned();
if trimmed_field.len() % 2 != 0 {
trimmed_field = "0".to_owned() + &trimmed_field
trimmed_field = "0".to_owned() + &trimmed_field;
};
"0x".to_owned() + &trimmed_field
}
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa/conditional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,14 +855,14 @@ impl DecisionTree {
&& left_arrays.is_empty()
&& right_arrays.is_empty() =>
{
candidates.push(Segment::new(left_node, right_node))
candidates.push(Segment::new(left_node, right_node));
}

(
Operation::Store { array_id: left_array, index: left_index, .. },
Operation::Store { array_id: right_array, index: right_index, .. },
) if left_array == right_array && left_index == right_index => {
candidates.push(Segment::new(left_node, right_node))
candidates.push(Segment::new(left_node, right_node));
}
_ => (),
}
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl SsaContext {
result = format!("{var}");
}
if result.is_empty() {
result = format!("unknown {:?}", id.0.into_raw_parts().0)
result = format!("unknown {:?}", id.0.into_raw_parts().0);
}
result
}
Expand Down Expand Up @@ -250,7 +250,7 @@ impl SsaContext {

pub(crate) fn print_instructions(&self, instructions: &[NodeId]) {
for id in instructions {
self.print_node(*id)
self.print_node(*id);
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ fn inline(
decision,
)?;
if result && nested_call {
result = false
result = false;
}
}
Ok(result)
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ fn block_overflow(
if let Some(r_const) = ctx.get_as_constant(rhs) {
let r_type = ctx[rhs].get_type();
if r_const.to_u128() > r_type.bits() as u128 {
ins.mark = Mark::ReplaceWith(ctx.zero_with_type(ins.res_type))
ins.mark = Mark::ReplaceWith(ctx.zero_with_type(ins.res_type));
} else {
let rhs = ctx
.get_or_create_const(FieldElement::from(2_i128).pow(&r_const), r_type);
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1225,7 +1225,7 @@ impl Operation {
Cond { condition, val_true: lhs, val_false: rhs } => {
*condition = f(*condition);
*lhs = f(*lhs);
*rhs = f(*rhs)
*rhs = f(*rhs);
}
Load { index, .. } => *index = f(*index),
Store { index, value, predicate, .. } => {
Expand Down Expand Up @@ -1291,7 +1291,7 @@ impl Operation {
Nop => (),
Call { func, arguments, .. } => {
f(*func);
arguments.iter().copied().for_each(f)
arguments.iter().copied().for_each(f);
}
Return(values) => values.iter().copied().for_each(f),
Result { call_instruction, .. } => {
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/optimizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ fn cse_block_with_anchor(
let mut activate_cse = true;
// We do not want to replace any print intrinsics as we want them to remain in order and unchanged
if let builtin::Opcode::Println(_) = opcode {
activate_cse = false
activate_cse = false;
}

for arg in args {
Expand Down
6 changes: 3 additions & 3 deletions crates/noirc_frontend/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,9 @@ impl CrateGraph {
return;
}
for dep in graph[source].dependencies.iter() {
go(graph, visited, res, dep.crate_id)
go(graph, visited, res, dep.crate_id);
}
res.push(source)
res.push(source);
}
}

Expand Down Expand Up @@ -163,7 +163,7 @@ impl CrateGraph {
}
impl CrateData {
fn add_dep(&mut self, name: CrateName, crate_id: CrateId) {
self.dependencies.push(Dependency { crate_id, name })
self.dependencies.push(Dependency { crate_id, name });
}
}
impl std::ops::Index<CrateId> for CrateGraph {
Expand Down
8 changes: 4 additions & 4 deletions crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct UnresolvedFunctions {

impl UnresolvedFunctions {
pub fn push_fn(&mut self, mod_id: LocalModuleId, func_id: FuncId, func: NoirFunction) {
self.functions.push((mod_id, func_id, func))
self.functions.push((mod_id, func_id, func));
}
}

Expand Down Expand Up @@ -243,7 +243,7 @@ fn collect_impls(
} else if typ != Type::Error && crate_id == LOCAL_CRATE {
let span = *span;
let error = DefCollectorErrorKind::NonStructTypeInImpl { span };
errors.push(error.into_file_diagnostic(unresolved.file_id))
errors.push(error.into_file_diagnostic(unresolved.file_id));
}
}
}
Expand All @@ -261,7 +261,7 @@ where
Errs: IntoIterator<Item = Err>,
Err: Into<CustomDiagnostic>,
{
errors.extend(new_errors.into_iter().map(|err| err.into().in_file(file)))
errors.extend(new_errors.into_iter().map(|err| err.into().in_file(file)));
}

/// Separate the globals Vec into two. The first element in the tuple will be the
Expand Down Expand Up @@ -476,6 +476,6 @@ fn type_check_functions(
errors: &mut Vec<FileDiagnostic>,
) {
for (file, func) in file_func_ids {
extend_errors(errors, file, type_check_func(interner, func))
extend_errors(errors, file, type_check_func(interner, func));
}
}
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn collect_defs(

// First resolve the module declarations
for decl in ast.module_decls {
collector.parse_module_declaration(context, &decl, crate_id, errors)
collector.parse_module_declaration(context, &decl, crate_id, errors);
}

collector.collect_submodules(context, crate_id, ast.submodules, file_id, errors);
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/hir/resolution/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ fn resolve_name_in_module(
return Err(PathResolutionError::ExternalContractUsed(segment.clone()));
}

current_ns = found_ns
current_ns = found_ns;
}

Ok(current_ns)
Expand Down
16 changes: 8 additions & 8 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl<'a> Resolver<'a> {
}

fn push_err(&mut self, err: ResolverError) {
self.errors.push(err)
self.errors.push(err);
}

fn current_lambda_index(&self) -> usize {
Expand Down Expand Up @@ -410,7 +410,7 @@ impl<'a> Resolver<'a> {
});

// Fix the generic count so we can continue typechecking
args.resize_with(expected_generic_count, || Type::Error)
args.resize_with(expected_generic_count, || Type::Error);
}

Type::Struct(struct_type, args)
Expand Down Expand Up @@ -555,7 +555,7 @@ impl<'a> Resolver<'a> {
name: generic.0.contents.clone(),
first_span: *first_span,
second_span: span,
})
});
} else {
self.generics.push((name, typevar.clone(), span));
}
Expand Down Expand Up @@ -616,7 +616,7 @@ impl<'a> Resolver<'a> {

for (pattern, typ, visibility) in func.parameters().iter().cloned() {
if visibility == noirc_abi::AbiVisibility::Public && !self.pub_allowed(func) {
self.push_err(ResolverError::UnnecessaryPub { ident: func.name_ident().clone() })
self.push_err(ResolverError::UnnecessaryPub { ident: func.name_ident().clone() });
}

let pattern = self.resolve_pattern(pattern, DefinitionKind::Local(None));
Expand All @@ -634,13 +634,13 @@ impl<'a> Resolver<'a> {
&& return_type.as_ref() != &Type::Unit
&& func.def.return_visibility != noirc_abi::AbiVisibility::Public
{
self.push_err(ResolverError::NecessaryPub { ident: func.name_ident().clone() })
self.push_err(ResolverError::NecessaryPub { ident: func.name_ident().clone() });
}

if attributes == Some(Attribute::Test) && !parameters.is_empty() {
self.push_err(ResolverError::TestFunctionHasParameters {
span: func.name_ident().span(),
})
});
}

let mut typ = Type::Function(parameter_types, return_type);
Expand Down Expand Up @@ -1017,7 +1017,7 @@ impl<'a> Resolver<'a> {
}
Pattern::Mutable(pattern, span) => {
if let Some(first_mut) = mutable {
self.push_err(ResolverError::UnnecessaryMut { first_mut, second_mut: span })
self.push_err(ResolverError::UnnecessaryMut { first_mut, second_mut: span });
}

let pattern = self.resolve_pattern_mutable(*pattern, Some(span), definition);
Expand Down Expand Up @@ -1491,7 +1491,7 @@ mod test {
fn path_unresolved_error(err: ResolverError, expected_unresolved_path: &str) {
match err {
ResolverError::PathResolutionError(PathResolutionError::Unresolved(name)) => {
assert_eq!(name.to_string(), expected_unresolved_path)
assert_eq!(name.to_string(), expected_unresolved_path);
}
_ => unimplemented!("expected an unresolved path"),
}
Expand Down
Loading

0 comments on commit 24adcc0

Please sign in to comment.