diff --git a/src/check/report.rs b/src/check/report.rs index e14e582..69a5432 100644 --- a/src/check/report.rs +++ b/src/check/report.rs @@ -33,6 +33,6 @@ impl Report { /// Returns true if no issues were found. #[must_use] pub fn is_valid(&self) -> bool { - self.invalid_items.is_empty() + !self.invalid_items.iter().any(|item| !item.is_disabled) } } diff --git a/src/check/validators/constant_names.rs b/src/check/validators/constant_names.rs index ab8d2a9..568328a 100644 --- a/src/check/validators/constant_names.rs +++ b/src/check/validators/constant_names.rs @@ -54,13 +54,19 @@ fn validate_name(parsed: &Parsed, v: &VariableDefinition) -> Option .attrs .iter() .any(|a| matches!(a, VariableAttribute::Constant(_) | VariableAttribute::Immutable(_))); - let name = &v.name.as_ref().unwrap().name; - if is_constant && !is_valid_constant_name(name) { - Some(InvalidItem::new(ValidatorKind::Constant, parsed, v.loc, name.clone())) - } else { - None + if !is_constant { + return None } + + v.name.as_ref().and_then(|name| { + let name_string = &name.name; + if is_valid_constant_name(name_string) { + None + } else { + Some(InvalidItem::new(ValidatorKind::Constant, parsed, name.loc, name_string.clone())) + } + }) } #[cfg(test)] diff --git a/src/check/validators/src_names_internal.rs b/src/check/validators/src_names_internal.rs index b14c3f7..349d35f 100644 --- a/src/check/validators/src_names_internal.rs +++ b/src/check/validators/src_names_internal.rs @@ -49,7 +49,7 @@ fn is_valid_internal_or_private_name(name: &str) -> bool { fn validate_name(parsed: &Parsed, f: &FunctionDefinition) -> Option { let name = f.name(); if f.is_internal_or_private() && !is_valid_internal_or_private_name(&name) { - Some(InvalidItem::new(ValidatorKind::Src, parsed, f.loc, name)) + Some(InvalidItem::new(ValidatorKind::Src, parsed, f.name_loc, name)) } else { None } diff --git a/src/check/validators/test_names.rs b/src/check/validators/test_names.rs index 76d9f78..a36118f 100644 --- a/src/check/validators/test_names.rs +++ b/src/check/validators/test_names.rs @@ -82,7 +82,7 @@ fn is_test_function(f: &FunctionDefinition) -> bool { fn validate_name(parsed: &Parsed, f: &FunctionDefinition) -> Option { let name = f.name(); if is_test_function(f) && !is_valid_test_name(&name) { - Some(InvalidItem::new(ValidatorKind::Test, parsed, f.loc, name)) + Some(InvalidItem::new(ValidatorKind::Test, parsed, f.name_loc, name)) } else { None } diff --git a/tests/check-proj1-AllFindings/src/Counter.sol b/tests/check-proj1-AllFindings/src/Counter.sol index 9fb8f2e..25b6db7 100644 --- a/tests/check-proj1-AllFindings/src/Counter.sol +++ b/tests/check-proj1-AllFindings/src/Counter.sol @@ -30,4 +30,5 @@ contract Counter { } // scopelint: this directive is invalid +// Extra line break at the bottom is intention to ensure formatting fails. diff --git a/tests/check-proj1-AllFindings/src/CounterIgnored3.sol b/tests/check-proj1-AllFindings/src/CounterIgnored3.sol index 69390ac..fc2175b 100644 --- a/tests/check-proj1-AllFindings/src/CounterIgnored3.sol +++ b/tests/check-proj1-AllFindings/src/CounterIgnored3.sol @@ -1,4 +1,5 @@ -// This file is identical to `Counter.sol` except it has ignore statements. +// This file is identical to `Counter.sol` except it has ignore statements and adds multiline +// function declaration tests. pragma solidity ^0.8.17; @@ -32,4 +33,22 @@ contract CounterIgnored1 { function _privateHasLeadingUnderscore() private { number += 1000; } + + // -------- Multiline function declaration tests -------- + function prettyLongMethodName( // scopelint: disable-line + address someImportantUser, + uint256 someImportantNumber, + bytes32 someImportantData + ) internal { + // do something + } + + // scopelint: disable-next-line + function prettyLongMethodName2( + address someImportantUser, + uint256 someImportantNumber, + bytes32 someImportantData + ) internal { + // do something + } }