rulewright 0.1.1

Executable engineering standards for Rust workspaces
Documentation
#[cfg(test)]
use googletest::prelude::*;
use std::collections::HashSet;

use ra_ap_syntax::{AstNode, ast, ast::HasArgList, ast::HasName};

use super::support::path_expr_name;
use crate::{AstCtx, Example, Violation};

#[rustfmt::skip]
const EXAMPLES: &[Example] = &[
    Example {
        label: "clone of loop item varies by iteration",
        code: "fn f(v: Vec<String>) { for x in &v { let y = x.clone(); } }",
        pass: true,
    },
    Example {
        label: "clone in while loop",
        code: "fn f(s: &String) { while true { let y = s.clone(); } }",
        pass: false,
    },
    Example {
        label: "clone in loop loop",
        code: "fn f(s: &String) { loop { let y = s.clone(); break; } }",
        pass: false,
    },
    Example {
        label: "to_owned in loop",
        code: "fn f(s: &str) { loop { let y = s.to_owned(); break; } }",
        pass: false,
    },
    Example {
        label: "clone outside loop",
        code: "fn f(s: &String) { let y = s.clone(); }",
        pass: true,
    },
    Example {
        label: "clone in test module",
        code: "#[cfg(test)]\nmod tests {\n    fn f(v: Vec<String>) { for x in &v { let y = x.clone(); } }\n}",
        pass: true,
    },
];

crate::ast_rule!(
    clone_in_loop,
    "Flag `.clone()` and `.to_owned()` calls on loop-invariant receivers inside loop bodies.",
    "A loop-invariant ownership conversion may be avoidable or movable outside the loop. Borrow or restructure when possible. \
     Conversions of values bound inside the loop are not reported because they commonly express a required per-item ownership transfer. \
     This syntax-only rule cannot distinguish heap copies from cheap Arc, Rc, or Copy clones, so intentional cheap clones should be suppressed with a reason.",
    Medium,
    default = false,
);

fn check_clone_in_loop(ctx: &AstCtx<'_>) -> Vec<Violation> {
    let clone_calls = ctx
        .nodes::<ast::MethodCallExpr>()
        .filter(|call| !ctx.is_in_test(call))
        .filter(|call| {
            call.name_ref()
                .is_some_and(|name| matches!(name.text().as_str(), "clone" | "to_owned"))
                && call
                    .arg_list()
                    .is_none_or(|arguments| arguments.args().next().is_none())
        })
        .filter(|call| enclosing_loop_body(call).is_some_and(|body| !receiver_varies(call, &body)));

    clone_calls
        .filter_map(|call| {
            let method = call.name_ref()?.text().to_string();

            Some(ctx.violation(
                &call,
                format!(".{method}() inside a loop — consider borrowing or restructuring to avoid repeated ownership conversions"),
            ))
        })
        .collect()
}

fn enclosing_loop_body(call: &ast::MethodCallExpr) -> Option<ast::BlockExpr> {
    call.syntax()
        .ancestors()
        .filter_map(ast::BlockExpr::cast)
        .find(|body| {
            body.syntax().parent().is_some_and(|parent| {
                ast::ForExpr::can_cast(parent.kind())
                    || ast::WhileExpr::can_cast(parent.kind())
                    || ast::LoopExpr::can_cast(parent.kind())
            })
        })
}

fn receiver_varies(call: &ast::MethodCallExpr, body: &ast::BlockExpr) -> bool {
    let mut bindings: HashSet<String> = body
        .syntax()
        .descendants()
        .filter(|node| node.text_range().start() < call.syntax().text_range().start())
        .filter_map(ast::IdentPat::cast)
        .filter_map(|pattern| pattern.name().map(|name| name.text().to_string()))
        .collect();

    if let Some(for_loop) = body.syntax().parent().and_then(ast::ForExpr::cast)
        && let Some(pattern) = for_loop.pat()
    {
        bindings.extend(
            pattern
                .syntax()
                .descendants()
                .filter_map(ast::IdentPat::cast)
                .filter_map(|pattern| pattern.name().map(|name| name.text().to_string())),
        );
    }

    call.receiver().is_some_and(|receiver| {
        receiver
            .syntax()
            .descendants()
            .filter_map(ast::PathExpr::cast)
            .filter_map(|path| path_expr_name(&ast::Expr::PathExpr(path)))
            .any(|name| bindings.contains(&name))
    })
}

crate::rulewright_ast_test!(check_clone_in_loop, {
    crate::example_tests!(EXAMPLES, check_clone_in_loop);

    #[gtest]
    fn clone_in_for_iterator_is_evaluated_once() -> Result<()> {
        verify_true!(run("fn f(tokens: Tokens) { for token in tokens.clone() {} }").is_empty())?;

        Ok(())
    }

    #[gtest]
    fn clone_of_local_created_inside_loop_is_not_reported() -> Result<()> {
        verify_true!(
            run("fn f(values: &[&str]) { for value in values { let owned = value.to_string(); let copy = owned.clone(); } }")
                .is_empty()
        )?;

        Ok(())
    }

    #[gtest]
    fn clone_of_outer_value_inside_nested_loop_is_reported() -> Result<()> {
        verify_eq!(
            run("fn f(values: &[String]) { for value in values { for _ in 0..2 { let copy = value.clone(); } } }")
                .len(),
            1
        )
    }

    #[gtest]
    fn syntax_only_rule_is_opt_in() -> Result<()> {
        let rule = crate::all_rules()
            .into_iter()
            .find(|rule| rule.name == "rust_clone_in_loop")
            .or_fail()?;

        verify_false!(rule.default_enabled)
    }
});