Migrate clippy::author to rustc_ast::FormatArgs

[Niki4tap]

This PR aims to migrate clippy::author lint to new FormatArgs AST, since it's been ICEing after the change.

repro.rs:

#![feature(stmt_expr_attributes)]

fn main() {
    #[clippy::author]
    {
        println!("Hello, {}!", "ferris");
    }
}

Current ICE

if let ExprKind::Block(block, None) = expr.kind
    && block.stmts.len() == 1
    && let StmtKind::Semi(e) = block.stmts[0].kind
    && let ExprKind::Block(block1, None) = e.kind
    && block1.stmts.len() == 1
    && let StmtKind::Semi(e1) = block1.stmts[0].kind
    && let ExprKind::Call(func, args) = e1.kind
    && let ExprKind::Path(ref qpath) = func.kind
    && match_qpath(qpath, &["$crate", "io", "_print"])
    && args.len() == 1
    && let ExprKind::Call(func1, args1) = args[0].kind
    && let ExprKind::Path(ref qpath1) = func1.kind
thread 'rustc' panicked at 'path_to_string: called for lang item qpath', clippy_lints/src/utils/author.rs:746:36
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new

note: Clippy version: clippy 0.1.70 (928349795 2023-04-21)

query stack during panic:
#0 [analysis] running analysis passes on this crate
end of query stack

After this PR

if let ExprKind::Block(block, None) = expr.kind
    && block.stmts.len() == 1
    && let StmtKind::Semi(e) = block.stmts[0].kind
    && let Some(macro_call) = macros::root_macro_call_first_node(cx, e)
    && cx.tcx.is_diagnostic_item(sym::println_macro, macro_call.def_id)
    && let args = macros::collect_format_args(cx, |_fmt_args| {}, e, macro_call.expn)
    && args.len() == 1
    && let ExprKind::Lit(ref lit) = args[0].kind
    && let LitKind::Str(s, _) = lit.node
    && s.as_str() == "ferris"
    && block.expr.is_none()
{
    // report your lint here
}

r? @Alexendoo

I've seen you migrate other lints, so I think your feedback will be valuable here. Thanks in advance :)
(if anyone else has an opinion that might be useful here, you're free to comment too)

Also, I'm not exactly sure if a new utility function is needed here, or it would better to reuse existing ones like clippy_utils::macros::find_format_args.

note: I think author lint is mostly internal, and the change is too, but on the other hand it's a bugfix, so I'm not sure what to put in the changelog, let there be none for now :)

changelog: none

[Alexendoo]

Thanks for this, didn't know clippy::author had support for that

Also, I'm not exactly sure if a new utility function is needed here, or it would better to reuse existing ones like clippy_utils::macros::find_format_args.

A new utility function could be nice, I'm not that happy with the current API. However we can't match on the HIR of format_args as collect_format_args does because that was holding back progress in rustc, it ought to be based on find_format_args/find_format_arg_expr

[Niki4tap]

A new utility function could be nice, I'm not that happy with the current API. However we can't match on the HIR of format_args as collect_format_args does because that was holding back progress in rustc, it ought to be based on find_format_args/find_format_arg_expr

Ah, yeah that makes sense. I've changed collect_format_args implementation to use these instead of HIR matching, should be fine now ^^

[Alexendoo]

It'd want to be callback: impl FnOnce(&FormatArgs, Vec<...>) rather than returning the Vec, lints generally want access to both at the same time

[Niki4tap]

Oh, sorry for misunderstanding then. Although I'm not quite sure how to apply it in the author lint, if we don't return the args, it wouldn't quite fit into the if-chain author lint tries to build, i. e.

   && macros::collect_from_args(cx, |args, _fmt_args| { if /* another if-chain */ { /* report your lint here */ } }, e, macro_call.expn)

- this does not only introduce another level of indentation, but also requires some code refactoring to support, which is, in my opinion, quite a bit out of scope for the author lint.

If you have any ideas on how to mitigate this, I would be happy to hear, otherwise I can close the PR and submit an issue on this.

[Alexendoo]

For author hmm that's tricky, I guess you could always print a comment saying to take a look at the function

[bors]

☔ The latest upstream changes (presumably #10647) made this pull request unmergeable. Please resolve the merge conflicts.

[Alexendoo]

Since #11444 was merged find_format_args now returns a value instead of taking a callback making this easier if you wanted to pick it up again, otherwise I'll add it to my todo list

[xFrednet]

Hey @Alexendoo, this is a ping from triage. It seems like @Niki4tap hasn't responded to your last message where you suggested that you'll pick this PR up. Do we want to close this one due to inactivity?

[Alexendoo]

Yeah I'll take a look at it after #12567 which changes how they work (again 😛)