Allow multiple public methods in scripts with at least one run method (#38)

* Allow multiple public methods as long as there is at least one run method

* disable the stdsimd feature for the ahash crate

* Revert previous commit and remove clippy

* Update taplo `0.13.0`

* Remove missing_docs flag from cargo doc

* Add back clippy

* Fix clippy nits

* Fix clippy nits

* Upgrade version, rename files

* Fix names

* Fix mod.rs import name

Author: garyghayrat

Diff for: .github/workflows/ci.yml

@@ -41,24 +41,31 @@ jobs:
           profile: minimal
           toolchain: nightly
           override: true
-          components: clippy, rustfmt
+          components: rustfmt
 
       - name: cargo fmt
         uses: actions-rs/cargo@v1
         with:
           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 for: Cargo.lock

@@ -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"

Diff for: Cargo.toml

@@ -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 for: README.md

@@ -87,7 +87,7 @@ pub fn parse(file: &Path) -> Result<Parsed, Box<dyn Error>> {
     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<report::Report, Box<dyn Error>> {
             // 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 for: src/check/mod.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 for: src/check/validators/constant_names.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 for: src/check/validators/mod.rs

@@ -50,27 +50,18 @@ pub fn validate(parsed: &Parsed) -> Vec<InvalidItem> {
                 "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 for: src/check/validators/script_one_pubic_run_method.rs renamed to src/check/validators/script_has_public_run_method.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 for: src/check/validators/src_names_internal.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 for: src/check/validators/test_names.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::<String>()
 }