diff --git a/compiler/noirc_frontend/src/elaborator/function.rs b/compiler/noirc_frontend/src/elaborator/function.rs index 5aede693019..cc3102473f9 100644 --- a/compiler/noirc_frontend/src/elaborator/function.rs +++ b/compiler/noirc_frontend/src/elaborator/function.rs @@ -744,7 +744,9 @@ impl Elaborator<'_> { for parameter in &func_meta.parameter_idents { let name = self.interner.definition_name(parameter.id).to_owned(); let warn_if_unused = !(func_meta.trait_impl.is_some() && name == "self"); - let warn_if_not_mutated = false; + // The unnecessary-mut check only considers definitions that are actually mutable, + // so this is a no-op for parameters without `mut`. + let warn_if_not_mutated = true; // We allow shadowing here because there's no outer scope to shadow // (duplicate parameter names were already checked in `resolve_function_parameters`) let allow_shadowing = true; diff --git a/compiler/noirc_frontend/src/tests/arrays.rs b/compiler/noirc_frontend/src/tests/arrays.rs index 161c6866370..1b331563c13 100644 --- a/compiler/noirc_frontend/src/tests/arrays.rs +++ b/compiler/noirc_frontend/src/tests/arrays.rs @@ -152,7 +152,7 @@ fn array_length_overflow_during_monomorphization() { #[test] fn constant_index_out_of_bounds() { let src = r#" - fn main(a: u32, mut c: [u32; 2]) { + fn main(a: u32, c: [u32; 2]) { if (a == c[0]) { assert((c[0] == 12)); } else if (a == c[1]) { diff --git a/compiler/noirc_frontend/src/tests/functions.rs b/compiler/noirc_frontend/src/tests/functions.rs index 83fbd66496f..8426eb578c6 100644 --- a/compiler/noirc_frontend/src/tests/functions.rs +++ b/compiler/noirc_frontend/src/tests/functions.rs @@ -515,6 +515,8 @@ fn rejects_mutable_tuple_pattern_in_main_param() { fn main(mut (a, b): pub (Field, Field)) -> pub Field { ^^^^^^^^^^ Entry point parameter must use a simple identifier pattern ~~~~~~~~~~ Destructuring patterns are not allowed here; bind to a name and destructure inside the body + ^ variable does not need to be mutable + ^ variable does not need to be mutable a + b } "#; @@ -743,3 +745,90 @@ fn error_on_empty_composite_array_param_and_out_of_bounds_index() { "#; check_errors(src); } + +#[test] +fn warns_on_unnecessary_mut_function_parameter() { + let src = r#" + fn foo(mut x: Field) -> Field { + ^ variable does not need to be mutable + x + } + + fn main() { + assert(foo(1) == 1); + } + "#; + check_errors(src); +} + +#[test] +fn warns_on_unnecessary_mut_self_parameter() { + let src = r#" + struct Counter { + count: Field, + } + + impl Counter { + fn count(mut self) -> Field { + ^^^^ variable does not need to be mutable + self.count + } + } + + fn main() { + let counter = Counter { count: 1 }; + assert(counter.count() == 1); + } + "#; + check_errors(src); +} + +#[test] +fn does_not_warn_on_mutated_mut_function_parameter() { + let src = r#" + fn foo(mut x: Field) -> Field { + x = x + 1; + x + } + + fn main() { + assert(foo(1) == 2); + } + "#; + assert_no_errors(src); +} + +#[test] +fn does_not_warn_on_unnecessary_mut_parameter_with_underscore_name() { + let src = r#" + fn foo(mut _x: Field) -> Field { + 1 + } + + fn main() { + assert(foo(1) == 1); + } + "#; + assert_no_errors(src); +} + +#[test] +fn does_not_warn_on_unmutated_mut_reference_self_parameter() { + let src = r#" + struct Counter { + count: Field, + } + + impl Counter { + fn count(&mut self) -> Field { + self.count + } + } + + fn main() { + let mut counter = Counter { count: 1 }; + assert(counter.count() == 1); + } + "#; + assert_no_errors(src); +} diff --git a/noir_stdlib/src/collections/bounded_vec.nr b/noir_stdlib/src/collections/bounded_vec.nr index 8f6302548b7..84e302212f5 100644 --- a/noir_stdlib/src/collections/bounded_vec.nr +++ b/noir_stdlib/src/collections/bounded_vec.nr @@ -578,7 +578,7 @@ impl BoundedVec { /// let vec: BoundedVec = BoundedVec::from_parts([1, 2, 3, 0], 3); /// assert_eq(vec.len(), 3); /// ``` - pub fn from_parts(mut array: [T; MaxLen], len: u32) -> Self { + pub fn from_parts(array: [T; MaxLen], len: u32) -> Self { assert(len <= MaxLen); BoundedVec { storage: array, len } } diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__acir_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__acir_stderr.snap index 6e113d8f1f6..a3c1ad13820 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__acir_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__acir_stderr.snap @@ -2,6 +2,13 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:39:9 + │ +39 │ mut b: (Field, &mut u128, (bool, bool, u8), (Field, u8), [i64; 4]), + │ - + │ + error: Assertion failed: Stack too deep ┌─ src/main.nr:44:15 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__brillig_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__brillig_stderr.snap index 6e113d8f1f6..a3c1ad13820 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__brillig_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__brillig_stderr.snap @@ -2,6 +2,13 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:39:9 + │ +39 │ mut b: (Field, &mut u128, (bool, bool, u8), (Field, u8), [i64; 4]), + │ - + │ + error: Assertion failed: Stack too deep ┌─ src/main.nr:44:15 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__comptime_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__comptime_stderr.snap index 464735d43cf..d9e16f155ee 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__comptime_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/brillig_mem_layout_regression/execute__tests__comptime_stderr.snap @@ -2,6 +2,13 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:39:9 + │ +39 │ mut b: (Field, &mut u128, (bool, bool, u8), (Field, u8), [i64; 4]), + │ - + │ + error: Comptime Evaluation Depth Overflow ┌─ src/main.nr:42:5 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__acir_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__acir_stderr.snap index 54580209c68..6ae10ea0e01 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__acir_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__acir_stderr.snap @@ -2,6 +2,13 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:6:13 + │ +6 │ fn main(mut x: [Foo; 3], y: pub u32) { + │ - + │ + error: Assertion failed: Index out of bounds, array has size 3, but index was 4 ┌─ src/main.nr:7:12 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__brillig_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__brillig_stderr.snap index 17c23971aad..77ce31b1c26 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__brillig_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__brillig_stderr.snap @@ -2,6 +2,13 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:6:13 + │ +6 │ fn main(mut x: [Foo; 3], y: pub u32) { + │ - + │ + error: Assertion failed: Index out of bounds ┌─ src/main.nr:7:12 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__comptime_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__comptime_stderr.snap index b337fe6d0fe..09c270ceac0 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__comptime_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/dyn_index_fail_nested_array/execute__tests__comptime_stderr.snap @@ -2,6 +2,13 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:6:13 + │ +6 │ fn main(mut x: [Foo; 3], y: pub u32) { + │ - + │ + error: Index out of bounds: 4 is out of bounds for the array of length 3 ┌─ src/main.nr:7:12 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__acir_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__acir_stderr.snap index 72555329994..b49c322309c 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__acir_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__acir_stderr.snap @@ -2,6 +2,27 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:20:15 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + +warning: variable does not need to be mutable + ┌─ src/main.nr:20:27 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + +warning: variable does not need to be mutable + ┌─ src/main.nr:20:45 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + warning: Return variable contains a constant value ┌─ src/main.nr:14:5 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__brillig_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__brillig_stderr.snap index c75d6f2a87d..3cd7725b2de 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__brillig_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__brillig_stderr.snap @@ -2,6 +2,27 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:20:15 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + +warning: variable does not need to be mutable + ┌─ src/main.nr:20:27 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + +warning: variable does not need to be mutable + ┌─ src/main.nr:20:45 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + error: Assertion failed: attempt to add with overflow ┌─ src/main.nr:29:21 │ diff --git a/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__comptime_stderr.snap b/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__comptime_stderr.snap index 01c0b290f8b..bf6e94e4080 100644 --- a/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__comptime_stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/execution_failure/regression_8231/execute__tests__comptime_stderr.snap @@ -2,6 +2,27 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- +warning: variable does not need to be mutable + ┌─ src/main.nr:20:15 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + +warning: variable does not need to be mutable + ┌─ src/main.nr:20:27 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + +warning: variable does not need to be mutable + ┌─ src/main.nr:20:45 + │ +20 │ fn func_1(mut a: u64, mut b: [bool; 1], mut c: [bool; 1]) -> (i64, [bool; 1]) { + │ - + │ + error: Attempt to add with overflow ┌─ src/main.nr:29:21 │