Repository: astral-sh/ruff
File: crates/ruff_linter/src/rules/flake8_pytest_style/helpers.rs (Rust)

Code before:
/// These hook wrappers intentionally use `return` in generator functions as part of the
/// pytest hook wrapper protocol.
///
/// See: <https://docs.pytest.org/en/stable/how-to/writing_hook_functions.html#hook-wrappers-executing-around-other-hooks>
pub(crate) fn is_pytest_hookimpl_wrapper(decorator: &Decorator, semantic: &SemanticModel) -> bool {
    let Expr::Call(call) = &decorator.expression else {
        return false;
    };

    // Check if it's pytest.hookimpl
    let is_hookimpl = semantic
        .resolve_qualified_name(&call.func)
        .is_some_and(|name| matches!(name.segments(), ["pytest", "hookimpl"]));

    if !is_hookimpl {
        return false;
    }

    // Check for wrapper=True keyword argument
    call.arguments.keywords.iter().any(|keyword| {
        keyword.arg.as_ref().is_some_and(|arg| arg == "wrapper")
            && matches!(
                &keyword.value,
                Expr::BooleanLiteral(ast::ExprBooleanLiteral { value: true, .. })
            )
    })
}

/// Whether the currently checked `func` is likely to be a Pytest test.
///
/// A normal Pytest test function is one whose name starts with `test` and is either:
///
/// * Placed at module-level, or
/// * Placed within a class whose name starts with `Test` and does not have an `__init__` method.
///
/// During test discovery, Pytest respects a few settings which we do not have access to.
/// This function is thus prone to both false positives and false negatives.
///
/// References:
/// - [`pytest` documentation: Conventions for Python test discovery](https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery)
/// - [`pytest` documentation: Changing naming conventions](https://docs.pytest.org/en/stable/example/pythoncollection.html#changing-naming-conventions)
pub(crate) fn is_likely_pytest_test(func: &StmtFunctionDef, checker: &Checker) -> bool {
    let semantic = checker.semantic();

    if !func.name.starts_with("test") {
        return false;
    }

    if semantic.scope_id.is_global() {
        return true;
    }

Review comment (refactor):
We have a helper for this kind of thing:

https://github.com/astral-sh/ruff/blob/0f373603ebd007196af4f211e14d421024a6b947/crates/ruff_python_ast/src/nodes.rs#L3317-L3318

and a smaller one for the boolean literal:

https://github.com/astral-sh/ruff/blob/0f373603ebd007196af4f211e14d421024a6b947/crates/ruff_python_ast/src/helpers.rs#L675-L676

I think we may want to accept more arguments here, though. I checked `help(pytest.hookimpl)` in the REPL, and this is the signature:

```
class HookimplMarker(builtins.object)
 |  __call__(
 |      self,
 |      function: _F | None = None,
 |      hookwrapper: bool = False,
 |      optionalhook: bool = False,
 |      tryfirst: bool = False,
 |      trylast: bool = False,
 |      specname: str | None = None,
 |      wrapper: bool = False
 |  ) -> _F | Callable[[_F], _F]
```

so it seems that `wrapper` could technically also be passed positionally. I also think the `hookwrapper` argument has a similar function, although it's referred to as the "old-style hook wrapper" in the [pluggy docs](https://pluggy.readthedocs.io/en/stable/index.html#wrappers), which the pytest docs redirected me to.

If we decide to accept either `wrapper` or `hookwrapper` and look for them positionally too, we could do something like:

```rust
let hookwrapper = arguments.find_argument_value("hookwrapper", 1);
let wrapper = arguments.find_argument_value("wrapper", 6);
```

and then process the returned `Option`s.

I'm assuming that most usage in the wild will be with a keyword `wrapper` argument, though.

Code after:
///
/// These hook wrappers intentionally use `return` in generator functions as part of the
/// pytest hook wrapper protocol.
///
/// See: <https://docs.pytest.org/en/stable/how-to/writing_hook_functions.html#hook-wrappers-executing-around-other-hooks>
pub(crate) fn is_pytest_hookimpl_wrapper(decorator: &Decorator, semantic: &SemanticModel) -> bool {
    let Expr::Call(call) = &decorator.expression else {
        return false;
    };

    // Check if it's pytest.hookimpl
    let is_hookimpl = semantic
        .resolve_qualified_name(&call.func)
        .is_some_and(|name| matches!(name.segments(), ["pytest", "hookimpl"]));

    if !is_hookimpl {
        return false;
    }

    let wrapper = call.arguments.find_argument_value("wrapper", 6);
    let hookwrapper = call.arguments.find_argument_value("hookwrapper", 1);

    wrapper.or(hookwrapper).is_some_and(is_const_true)
}

/// Whether the currently checked `func` is likely to be a Pytest test.
///
/// A normal Pytest test function is one whose name starts with `test` and is either:
///
/// * Placed at module-level, or
/// * Placed within a class whose name starts with `Test` and does not have an `__init__` method.
///
/// During test discovery, Pytest respects a few settings which we do not have access to.
/// This function is thus prone to both false positives and false negatives.
///
/// References:
/// - [`pytest` documentation: Conventions for Python test discovery](https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery)
/// - [`pytest` documentation: Changing naming conventions](https://docs.pytest.org/en/stable/example/pythoncollection.html#changing-naming-conventions)
pub(crate) fn is_likely_pytest_test(func: &StmtFunctionDef, checker: &Checker) -> bool {
    let semantic = checker.semantic();

    if !func.name.starts_with("test") {
        return false;
    }

    if semantic.scope_id.is_global() {
        return true;
    }

    let ScopeKind::Class(class) = semantic.current_scope().kind else {
        return false;