diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64e4d8a..eb6d0f1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,7 +41,7 @@ jobs: profile: minimal toolchain: nightly override: true - components: clippy, rustfmt + components: rustfmt - name: cargo fmt uses: actions-rs/cargo@v1 @@ -49,16 +49,23 @@ jobs: command: fmt args: --all --check - - name: cargo clippy - uses: actions-rs/clippy-check@v1 - with: - args: --all --all-features -- -D warnings - token: ${{ secrets.GITHUB_TOKEN }} - - name: cargo doc uses: actions-rs/cargo@v1 env: - RUSTDOCFLAGS: '-D missing_docs -D rustdoc::missing_doc_code_examples' + RUSTDOCFLAGS: "-D rustdoc::missing_doc_code_examples" with: command: doc args: --workspace --all-features --no-deps --document-private-items + + clippy: + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@clippy + - uses: Swatinem/rust-cache@v2 + with: + cache-on-failure: true + - run: cargo clippy --workspace --all-targets --all-features + env: + RUSTFLAGS: -Dwarnings diff --git a/Cargo.lock b/Cargo.lock index 0cfbff1..aaf6960 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,13 +4,15 @@ version = 3 [[package]] name = "ahash" -version = "0.7.6" +version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" +checksum = "e89da841a80418a9b391ebaea17f5c112ffaaa96f621d2c285b5174da76b9011" dependencies = [ + "cfg-if", "getrandom", "once_cell", "version_check", + "zerocopy", ] [[package]] @@ -179,7 +181,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -760,7 +762,7 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "scopelint" -version = "0.0.20" +version = "0.0.21" dependencies = [ "clap", "colored", @@ -861,9 +863,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.29" +version = "2.0.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" +checksum = "239814284fd6f1a4ffe4ca893952cdd93c224b6a1571c9a9eadd670295c0c9e2" dependencies = [ "proc-macro2", "quote", @@ -872,9 +874,9 @@ dependencies = [ [[package]] name = "taplo" -version = "0.12.1" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d22577122cf37c9aea9a9363f6f01bddaa1977617fe5abc59e221f23d114817" +checksum = "52af7500383164409ea5e18ca813f70ded8dcb4ad044638dea8f0a43cd797942" dependencies = [ "ahash", "arc-swap", @@ -1204,3 +1206,23 @@ name = "windows_x86_64_msvc" version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" + +[[package]] +name = "zerocopy" +version = "0.7.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae87e3fcd617500e5d106f0380cf7b77f3c6092aae37191433159dda23cfb087" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.7.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15e934569e47891f7d9411f1a451d947a60e000ab3bd24fbb970f000387d1b3b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.32", +] diff --git a/Cargo.toml b/Cargo.toml index f609adf..2009a46 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ license = "MIT" name = "scopelint" repository = "https://github.com/ScopeLift/scopelint" - version = "0.0.20" + version = "0.0.21" [dependencies] clap = { version = "4.4.2", features = ["derive"] } @@ -16,5 +16,5 @@ once_cell = "1.16.0" regex = "1.6.0" solang-parser = "0.3.2" - taplo = "0.12.1" - walkdir = "2.3.2" + taplo = "0.13.0" + walkdir = "2.3.2" \ No newline at end of file diff --git a/README.md b/README.md index 4f0134e..4ab2d6f 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,13 @@ -# ScopeLint +# `ScopeLint` A simple and opinionated tool designed for basic formatting/linting of Solidity and TOML code in foundry projects. -- [Installation](#installation) -- [Usage](#usage) - - [`scopelint fmt`](#scopelint-fmt) - - [`scopelint check`](#scopelint-check) - - [`scopelint spec`](#scopelint-spec) - +- [`ScopeLint`](#scopelint) + - [Installation](#installation) + - [Usage](#usage) + - [`scopelint fmt`](#scopelint-fmt) + - [`scopelint check`](#scopelint-check) + - [`scopelint spec`](#scopelint-spec) ## Installation diff --git a/src/check/mod.rs b/src/check/mod.rs index d1bf6bf..c8a3082 100644 --- a/src/check/mod.rs +++ b/src/check/mod.rs @@ -87,7 +87,7 @@ pub fn parse(file: &Path) -> Result> { let src = &fs::read_to_string(file)?; let (pt, comments) = solang_parser::parse(src, 0).map_err(|d| { - eprintln!("{:?}", d); + eprintln!("{d:?}"); "Failed to parse file".to_string() })?; @@ -140,7 +140,7 @@ fn validate(paths: [&str; 3]) -> Result> { // Run all checks. results.add_items(validators::test_names::validate(&parsed)); results.add_items(validators::src_names_internal::validate(&parsed)); - results.add_items(validators::script_one_pubic_run_method::validate(&parsed)); + results.add_items(validators::script_has_public_run_method::validate(&parsed)); results.add_items(validators::constant_names::validate(&parsed)); } } diff --git a/src/check/validators/constant_names.rs b/src/check/validators/constant_names.rs index 568328a..6f18db6 100644 --- a/src/check/validators/constant_names.rs +++ b/src/check/validators/constant_names.rs @@ -76,7 +76,7 @@ mod tests { #[test] fn test_validate() { - let content = r#" + let content = r" contract MyContract { // These have the constant or immutable keyword and should be valid. uint256 constant MAX_UINT256 = type(uint256).max; @@ -90,7 +90,7 @@ mod tests { address alice = address(123); uint256 aliceBalance = 500; } - "#; + "; let expected_findings = ExpectedFindings::new(2); expected_findings.assert_eq(content, &validate); @@ -135,11 +135,11 @@ mod tests { ]; for name in allowed_names { - assert_eq!(is_valid_constant_name(name), true, "{name}"); + assert!(is_valid_constant_name(name), "{name}"); } for name in disallowed_names { - assert_eq!(is_valid_constant_name(name), false, "{name}"); + assert!(!is_valid_constant_name(name), "{name}"); } } } diff --git a/src/check/validators/mod.rs b/src/check/validators/mod.rs index 4fed59d..629c4b7 100644 --- a/src/check/validators/mod.rs +++ b/src/check/validators/mod.rs @@ -5,7 +5,7 @@ pub mod formatting; pub mod constant_names; /// Validates that a script has a single public method named `run`. -pub mod script_one_pubic_run_method; +pub mod script_has_public_run_method; /// Validates that internal and private function names are prefixed with an underscore. pub mod src_names_internal; diff --git a/src/check/validators/script_one_pubic_run_method.rs b/src/check/validators/script_has_public_run_method.rs similarity index 77% rename from src/check/validators/script_one_pubic_run_method.rs rename to src/check/validators/script_has_public_run_method.rs index 7e93c53..8b3e2da 100644 --- a/src/check/validators/script_one_pubic_run_method.rs +++ b/src/check/validators/script_has_public_run_method.rs @@ -50,27 +50,18 @@ pub fn validate(parsed: &Parsed) -> Vec { "No `run` method found".to_string(), )] } - 1 => { - if public_methods[0] == "run" { + _ => { + if public_methods.contains(&"run".to_string()) { Vec::new() } else { vec![InvalidItem::new( ValidatorKind::Script, parsed, - contract_loc.unwrap(), - "The only public method must be named `run`".to_string(), + contract_loc.unwrap(), // + "No `run` method found".to_string(), )] } } - _ => { - vec![InvalidItem::new( - ValidatorKind::Script, - parsed, - contract_loc.unwrap(), - format!("Scripts must have a single public method named `run` (excluding `setUp`), but the following methods were found: {public_methods:?}"), - - )] - } } } @@ -82,52 +73,59 @@ mod tests { #[test] fn test_validate() { // TODO add another test for the third match arm - let content_good = r#" + let content_good = r" contract MyContract { function run() public {} } - "#; + "; - // The number after `bad` on the variable name indicates the match arm covered. - let content_bad0 = r#" - contract MyContract {} - "#; - - let content_bad1 = r#" - contract MyContract { - function notRun() public {} - } - "#; - - let content_bad2_variant0 = r#" + let content_good_variant0 = r" contract MyContract { function run() public {} function run(string memory config) public {} } - "#; + "; - let content_bad2_variant1 = r#" + let content_good_variant1 = r" contract MyContract { function run() public {} function foo() public {} } - "#; + "; + + let content_good_variant2 = r" + contract MyContract { + function run(address admin) public {} + } + "; + + // The number after `bad` on the variable name indicates the match arm covered. + let content_bad0 = r" + contract MyContract {} + "; + + let content_bad1 = r" + contract MyContract { + function notRun() public {} + } + "; - let content_bad2_variant2 = r#" + let content_bad2_variant0 = r" contract MyContract { function foo() public {} function bar() public {} } - "#; + "; let expected_findings_good = ExpectedFindings::new(0); expected_findings_good.assert_eq(content_good, &validate); + expected_findings_good.assert_eq(content_good_variant0, &validate); + expected_findings_good.assert_eq(content_good_variant1, &validate); + expected_findings_good.assert_eq(content_good_variant2, &validate); let expected_findings_bad = ExpectedFindings { script: 1, ..Default::default() }; expected_findings_bad.assert_eq(content_bad0, &validate); expected_findings_bad.assert_eq(content_bad1, &validate); expected_findings_bad.assert_eq(content_bad2_variant0, &validate); - expected_findings_bad.assert_eq(content_bad2_variant1, &validate); - expected_findings_bad.assert_eq(content_bad2_variant2, &validate); } } diff --git a/src/check/validators/src_names_internal.rs b/src/check/validators/src_names_internal.rs index 349d35f..2461642 100644 --- a/src/check/validators/src_names_internal.rs +++ b/src/check/validators/src_names_internal.rs @@ -62,7 +62,7 @@ mod tests { #[test] fn test_validate() { - let content = r#" + let content = r" contract MyContract { // Valid names for internal or private src methods. function _myInternalMethod() internal {} @@ -76,7 +76,7 @@ mod tests { function myPublicMethod() public {} function myExternalMethod() external {} } - "#; + "; let expected_findings = ExpectedFindings { src: 2, ..ExpectedFindings::default() }; expected_findings.assert_eq(content, &validate); diff --git a/src/check/validators/test_names.rs b/src/check/validators/test_names.rs index a36118f..8390414 100644 --- a/src/check/validators/test_names.rs +++ b/src/check/validators/test_names.rs @@ -95,7 +95,7 @@ mod tests { #[test] fn test_validate() { - let content = r#" + let content = r" contract MyContract { // Good test names. function test_Description() public {} @@ -117,7 +117,7 @@ mod tests { function _testDescription() public {} function _testDescriptionMoreInfo() public {} } - "#; + "; let expected_findings = ExpectedFindings { test: 3, ..ExpectedFindings::default() }; expected_findings.assert_eq(content, &validate); @@ -172,11 +172,11 @@ mod tests { ]; for name in allowed_names { - assert_eq!(is_valid_test_name(name), true, "{name}"); + assert!(is_valid_test_name(name), "{name}"); } for name in disallowed_names { - assert_eq!(is_valid_test_name(name), false, "{name}"); + assert!(!is_valid_test_name(name), "{name}"); } } } diff --git a/src/spec/mod.rs b/src/spec/mod.rs index 7d8dce3..cef11ad 100644 --- a/src/spec/mod.rs +++ b/src/spec/mod.rs @@ -256,7 +256,6 @@ fn trimmed_fn_name_to_requirement(trimmed_fn_name: &str) -> String { trimmed_fn_name .replace('_', ":") .chars() - .enumerate() - .map(|(_i, c)| if c.is_uppercase() { format!(" {c}") } else { c.to_string() }) + .map(|c| if c.is_uppercase() { format!(" {c}") } else { c.to_string() }) .collect::() } diff --git a/tests/check.rs b/tests/check.rs index 99493fd..fff2b2f 100644 --- a/tests/check.rs +++ b/tests/check.rs @@ -32,8 +32,6 @@ fn test_check_proj1_all_findings() { "Invalid constant or immutable name in ./src/Counter.sol on line 5: badImmutable", "Invalid constant or immutable name in ./src/Counter.sol on line 6: bad_constant", "Invalid constant or immutable name in ./test/Counter.t.sol on line 7: testVal", - r#"Invalid script interface in ./script/Counter.s.sol: Scripts must have a single public method named `run` (excluding `setUp`), but the following methods were found: ["run", "runExternal"]"#, - r#"Invalid script interface in ./script/Counter2.s.sol: Scripts must have a single public method named `run` (excluding `setUp`), but the following methods were found: ["run", "anotherPublic", "thirdPublic"]"#, "Invalid src method name in ./src/Counter.sol on line 23: internalShouldHaveLeadingUnderscore", "Invalid src method name in ./src/Counter.sol on line 25: privateShouldHaveLeadingUnderscore", "Invalid test name in ./test/Counter.t.sol on line 16: testIncrementBadName",