feat: add scopelint fix (#60)

Author: thelostone-mc

DEV.md

@@ -160,6 +160,8 @@ The project uses GitHub Actions for continuous integration:
 ## Development Workflow
 
 **Command Summary:**
+- `scopelint check` - Run convention checks
+- `scopelint fix` - Apply safe fixes (e.g. remove unused imports), then run check
 - `scopelint-dev` - Local development version (debug build)
 - `scopelint-beta` - Beta release version (release build with -beta suffix)
 - `scopelint` - Production version (from crates.io)

README.md

@@ -7,6 +7,7 @@ A simple and opinionated tool designed for basic formatting/linting of Solidity
   - [Usage](#usage)
     - [`scopelint fmt`](#scopelint-fmt)
     - [`scopelint check`](#scopelint-check)
+    - [`scopelint fix`](#scopelint-fix)
     - [`scopelint spec`](#scopelint-spec)
   - [Development](#development)
 
@@ -19,10 +20,11 @@ When using the [ScopeLift Foundry template](https://github.com/ScopeLift/foundry
 
 ## Usage
 
-Once installed there are three commands:
+Once installed there are four commands:
 
 - `scopelint fmt`
 - `scopelint check`
+- `scopelint fix`
 - `scopelint spec`
 
 For all commands, please open issues for any bug reports, suggestions, or feature requests.
@@ -94,6 +96,14 @@ However, you can ignore specific rules for specific files using:
 
    Supported rules: `error`, `import`, `variable`, `constant`, `test`, `script`, `src`, `eip712`
 
+### `scopelint fix`
+
+Applies safe, automatic fixes and then runs `scopelint check`. Currently supports:
+
+- **Unused imports**: Removes unused symbols from named imports (`import { A, B } from "..."`) and removes entire aliased import lines (`import "..." as Alias`) when the alias is unused.
+
+Only findings that are not ignored (via inline comments or `.scopelint`) are fixed. After fixing, any remaining convention or formatting issues are reported as with `scopelint check`.
+
 ### `scopelint spec`
 
 Most developers don't have formal specifications they are building towards, and instead only have a general idea of what they want their contracts to do.

src/check/mod.rs

@@ -9,6 +9,7 @@ use colored::Colorize;
 use itertools::Itertools;
 use solang_parser::pt::{Loc, SourceUnit};
 use std::{
+    collections::HashSet,
     error::Error,
     ffi::OsStr,
     fs,
@@ -51,6 +52,90 @@ pub fn run(taplo_opts: taplo::formatter::Options) -> Result<(), Box<dyn Error>>
     }
 }
 
+/// Applies safe fixes (e.g. remove unused imports), then runs check.
+///
+/// # Errors
+///
+/// Returns an error if fixes could not be applied or if convention checks still fail after
+/// fixing.
+pub fn run_fix(taplo_opts: taplo::formatter::Options) -> Result<(), Box<dyn Error>> {
+    let path_config = CheckPaths::load();
+    let results = validate(&path_config)?;
+
+    let fixable_imports: Vec<&utils::InvalidItem> = results
+        .items()
+        .iter()
+        .filter(|item| {
+            item.kind == utils::ValidatorKind::Import && !item.is_disabled && !item.is_ignored
+        })
+        .collect();
+
+    if fixable_imports.is_empty() {
+        // No fixable import issues; run normal check and return its result.
+        let valid_names = validate_conventions();
+        let valid_fmt = validators::formatting::validate(taplo_opts);
+        if valid_names.is_ok() && valid_fmt.is_ok() {
+            return Ok(());
+        }
+        return Err("One or more checks failed, review above output".into());
+    }
+
+    let file_config = file_config::FileConfig::load();
+
+    // Group fixable import items by file and collect symbol names to remove.
+    let by_file: std::collections::HashMap<&str, HashSet<String>> = fixable_imports
+        .iter()
+        .map(|item| {
+            let symbol = extract_unused_import_symbol(&item.text);
+            (item.file.as_str(), symbol)
+        })
+        .fold(std::collections::HashMap::new(), |mut acc, (file, symbol)| {
+            acc.entry(file).or_default().insert(symbol);
+            acc
+        });
+
+    let mut fixed_count = 0_usize;
+    for (file_path, symbols) in &by_file {
+        let path = Path::new(file_path);
+        if !path.exists() {
+            continue;
+        }
+        let mut parsed = parse(path)?;
+        parsed.file_config = file_config.clone();
+        parsed.path_config = path_config.clone();
+
+        if let Some(new_src) = validators::unused_imports::fix_source(&parsed, Some(symbols)) {
+            fs::write(path, new_src)?;
+            fixed_count += 1;
+        }
+    }
+
+    if fixed_count > 0 {
+        eprintln!("{}: Fixed unused imports in {} file(s)", "info".bold().green(), fixed_count);
+    }
+
+    // Re-run check and report any remaining issues.
+    let valid_names = validate_conventions();
+    let valid_fmt = validators::formatting::validate(taplo_opts);
+    if valid_names.is_ok() && valid_fmt.is_ok() {
+        Ok(())
+    } else {
+        Err("One or more checks failed, review above output".into())
+    }
+}
+
+/// Extracts the symbol name from an "Unused import: '`SymbolName`'" message.
+fn extract_unused_import_symbol(text: &str) -> String {
+    const PREFIX: &str = "Unused import: '";
+    const SUFFIX: char = '\'';
+    if let Some(stripped) = text.strip_prefix(PREFIX) {
+        if let Some(symbol) = stripped.strip_suffix(SUFFIX) {
+            return symbol.to_string();
+        }
+    }
+    text.to_string()
+}
+
 // =============================
 // ======== Validations ========
 // =============================

src/check/report.rs

@@ -30,6 +30,12 @@ impl Report {
         self.invalid_items.extend(items);
     }
 
+    /// Returns all invalid items (including ignored/disabled).
+    #[must_use]
+    pub fn items(&self) -> &[InvalidItem] {
+        &self.invalid_items
+    }
+
     /// Returns true if no issues were found.
     #[must_use]
     pub fn is_valid(&self) -> bool {

src/check/validators/unused_imports.rs

@@ -3,12 +3,16 @@ use crate::check::{
     Parsed,
 };
 use regex::Regex;
-use std::sync::LazyLock;
+use std::{collections::HashSet, sync::LazyLock};
 
 // Regex to match import statements with symbol lists: `import {Symbol1, Symbol2} from "...";`
 static RE_IMPORT_SYMBOL_LIST: LazyLock<Regex> =
     LazyLock::new(|| Regex::new(r#"import\s*\{([^}]+)\}\s+from\s+"[^"]+";"#).unwrap());
 
+// Same but with path captured for fix_source (reconstructing the statement).
+static RE_IMPORT_SYMBOL_LIST_WITH_PATH: LazyLock<Regex> =
+    LazyLock::new(|| Regex::new(r#"import\s*\{([^}]+)\}\s+from\s+"([^"]+)";"#).unwrap());
+
 // Regex to match aliased imports: `import "..." as Alias;`
 static RE_IMPORT_ALIAS: LazyLock<Regex> =
     LazyLock::new(|| Regex::new(r#"import\s+"[^"]+"\s+as\s+(\w+);"#).unwrap());
@@ -141,10 +145,96 @@ fn is_symbol_used_excluding_imports(
     false
 }
 
+/// Returns the source with unused imports removed, or `None` if no changes.
+///
+/// - `only_remove`: if `Some(set)`, only remove symbols in the set (e.g. fixable from report). If
+///   `None`, remove all unused imports.
+///
+/// # Panics
+///
+/// Panics if a regex capture group is missing (should not happen with the current patterns).
+#[must_use]
+#[allow(clippy::implicit_hasher)]
+pub fn fix_source(parsed: &Parsed, only_remove: Option<&HashSet<String>>) -> Option<String> {
+    let mut import_ranges: Vec<(usize, usize)> = Vec::new();
+    for cap in RE_IMPORT_SYMBOL_LIST_WITH_PATH.captures_iter(&parsed.src) {
+        let m = cap.get(0).expect("capture 0 always present");
+        import_ranges.push((m.start(), m.end()));
+    }
+    for cap in RE_IMPORT_ALIAS.captures_iter(&parsed.src) {
+        let m = cap.get(0).expect("capture 0 always present");
+        import_ranges.push((m.start(), m.end()));
+    }
+
+    let mut edits: Vec<(usize, usize, String)> = Vec::new();
+
+    // Named imports: `import { A, B } from "path";`
+    for cap in RE_IMPORT_SYMBOL_LIST_WITH_PATH.captures_iter(&parsed.src) {
+        let m = cap.get(0).expect("capture 0 always present");
+        let start = m.start();
+        let end = m.end();
+        let symbols_str = cap.get(1).expect("capture 1 always present").as_str();
+        let path = cap.get(2).expect("capture 2 always present").as_str();
+
+        let mut kept: Vec<&str> = Vec::new();
+        for symbol_part in symbols_str.split(',') {
+            let symbol_part = symbol_part.trim();
+            let name =
+                symbol_part.split_once(" as ").map_or(symbol_part, |(_, alias)| alias.trim());
+            let should_remove = only_remove.map_or_else(
+                || !is_symbol_used_excluding_imports(&parsed.src, name, &import_ranges),
+                |set| set.contains(name),
+            );
+            if !should_remove {
+                kept.push(symbol_part);
+            }
+        }
+
+        if kept.is_empty() {
+            edits.push((start, end, String::new()));
+        } else if kept.len() < symbol_part_count(symbols_str) {
+            let new_list = kept.join(", ");
+            edits.push((start, end, format!(r#"import {{ {new_list} }} from "{path}";"#)));
+        }
+    }
+
+    // Aliased imports: `import "..." as Alias;`
+    for cap in RE_IMPORT_ALIAS.captures_iter(&parsed.src) {
+        let m = cap.get(0).expect("capture 0 always present");
+        let start = m.start();
+        let end = m.end();
+        let alias = cap.get(1).expect("capture 1 always present").as_str();
+        let should_remove = only_remove.map_or_else(
+            || !is_symbol_used_excluding_imports(&parsed.src, alias, &import_ranges),
+            |set| set.contains(alias),
+        );
+        if should_remove {
+            edits.push((start, end, String::new()));
+        }
+    }
+
+    if edits.is_empty() {
+        return None;
+    }
+
+    // Apply from end to start so offsets stay valid.
+    edits.sort_by_key(|(s, _e, _r)| std::cmp::Reverse(*s));
+    let mut out = parsed.src.clone();
+    for (start, end, replacement) in edits {
+        out = format!("{}{}{}", &out[..start], replacement, &out[end..]);
+    }
+    Some(out)
+}
+
+fn symbol_part_count(symbols_str: &str) -> usize {
+    symbols_str.split(',').filter(|s| !s.trim().is_empty()).count()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::check::utils::ExpectedFindings;
+    use itertools::Itertools;
 
     #[test]
     fn test_no_unused_imports() {
@@ -262,4 +352,70 @@ mod tests {
         };
         expected_findings.assert_eq(content, &validate);
     }
+
+    fn parsed_from_src(content: &str) -> crate::check::Parsed {
+        use crate::check::{comments::Comments, inline_config::InlineConfig};
+        use std::path::PathBuf;
+
+        let (pt, comments) = crate::parser::parse_solidity(content, 0).expect("parse");
+        let comments = Comments::new(comments, content);
+        let (inline_config_items, invalid_inline_config_items): (Vec<_>, Vec<_>) =
+            comments.parse_inline_config_items().partition_result();
+        let inline_config = InlineConfig::new(inline_config_items, content);
+        crate::check::Parsed {
+            file: PathBuf::from("./src/Contract.sol"),
+            src: content.to_string(),
+            pt,
+            comments,
+            inline_config,
+            invalid_inline_config_items,
+            file_config: crate::check::file_config::FileConfig::default(),
+            path_config: crate::foundry_config::CheckPaths::default(),
+        }
+    }
+
+    #[test]
+    fn test_fix_source_removes_unused_from_named_import() {
+        let content = r#"import {ERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+
+contract MyContract {
+    ERC20 public token;
+}
+"#;
+        let parsed = parsed_from_src(content);
+        let fixed = fix_source(&parsed, None).unwrap();
+        assert!(
+            fixed.contains(
+                r#"import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";"#
+            ),
+            "expected single used symbol in import, got: {fixed:?}"
+        );
+        assert!(!fixed.contains("IERC20"));
+    }
+
+    #[test]
+    fn test_fix_source_removes_whole_aliased_import() {
+        let content = r#"import "@openzeppelin/contracts/token/ERC20/ERC20.sol" as OZERC20;
+
+contract MyContract {
+}
+"#;
+        let parsed = parsed_from_src(content);
+        let fixed = fix_source(&parsed, None).unwrap();
+        assert!(!fixed.contains("OZERC20"));
+        assert!(!fixed.contains("as OZERC20"));
+    }
+
+    #[test]
+    fn test_fix_source_no_change_when_all_used() {
+        let content = r#"import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+
+contract MyContract {
+    ERC20 public token;
+}
+"#;
+        let parsed = parsed_from_src(content);
+        let fixed = fix_source(&parsed, None);
+        assert!(fixed.is_none());
+    }
 }

src/config.rs

@@ -39,6 +39,9 @@ pub enum Subcommands {
         /// Show changes without modifying files.
         check: bool,
     },
+    #[clap(about = "Applies safe fixes (e.g. remove unused imports), then runs check.")]
+    /// Applies safe fixes (e.g. remove unused imports), then runs check.
+    Fix,
     #[clap(about = "Generates a specification for the current project from test names.")]
     /// Generates a specification for the current project from test names.
     Spec {

src/lib.rs

@@ -42,6 +42,7 @@ pub fn run(opts: &config::Opts) -> Result<(), Box<dyn Error>> {
     match &opts.subcommand {
         config::Subcommands::Check => check::run(taplo_opts),
         config::Subcommands::Fmt { check } => fmt::run(taplo_opts, *check),
+        config::Subcommands::Fix => check::run_fix(taplo_opts),
         config::Subcommands::Spec { show_internal } => spec::run(*show_internal),
     }
 }