Rework #[clippy::dump] attribute for debugging

[xFrednet]

Hey @rust-lang/clippy, this adds a new #[clippy::print_hir] attribute that prints the node to the console using {:#?}. Personally, I use print debugging quite a lot while working on Clippy, and this is a simple shortcut that also works in the playground (Once this has been synced). The question is now, if we want to have this attribute. Are there any concerns? I think it's similar to our #[clippy::author] attribute.

I haven't added a test, as the .stdout file would require updates with every HIR change inside rustc. Here are some examples, for the current implementation

`do_something(&map);`

Expr {
    hir_id: HirId {
        owner: DefId(0:7 ~ aaa[995b]::main),
        local_id: 21,
    },
    kind: Call(
        Expr {
            hir_id: HirId {
                owner: DefId(0:7 ~ aaa[995b]::main),
                local_id: 17,
            },
            kind: Path(
                Resolved(
                    None,
                    Path {
                        span: tests/ui/aaa.rs:23:5: 23:17 (#0),
                        res: Def(
                            Fn,
                            DefId(0:6 ~ aaa[995b]::do_something),
                        ),
                        segments: [
                            PathSegment {
                                ident: do_something#0,
                                hir_id: Some(
                                    HirId {
                                        owner: DefId(0:7 ~ aaa[995b]::main),
                                        local_id: 16,
                                    },
                                ),
                                res: Some(
                                    Err,
                                ),
                                args: None,
                                infer_args: true,
                            },
                        ],
                    },
                ),
            ),
            span: tests/ui/aaa.rs:23:5: 23:17 (#0),
        },
        [
            Expr {
                hir_id: HirId {
                    owner: DefId(0:7 ~ aaa[995b]::main),
                    local_id: 20,
                },
                kind: AddrOf(
                    Ref,
                    Not,
                    Expr {
                        hir_id: HirId {
                            owner: DefId(0:7 ~ aaa[995b]::main),
                            local_id: 19,
                        },
                        kind: Path(
                            Resolved(
                                None,
                                Path {
                                    span: tests/ui/aaa.rs:23:19: 23:22 (#0),
                                    res: Local(
                                        HirId {
                                            owner: DefId(0:7 ~ aaa[995b]::main),
                                            local_id: 15,
                                        },
                                    ),
                                    segments: [
                                        PathSegment {
                                            ident: map#0,
                                            hir_id: Some(
                                                HirId {
                                                    owner: DefId(0:7 ~ aaa[995b]::main),
                                                    local_id: 18,
                                                },
                                            ),
                                            res: Some(
                                                Local(
                                                    HirId {
                                                        owner: DefId(0:7 ~ aaa[995b]::main),
                                                        local_id: 15,
                                                    },
                                                ),
                                            ),
                                            args: None,
                                            infer_args: true,
                                        },
                                    ],
                                },
                            ),
                        ),
                        span: tests/ui/aaa.rs:23:19: 23:22 (#0),
                    },
                ),
                span: tests/ui/aaa.rs:23:18: 23:22 (#0),
            },
        ],
    ),
    span: tests/ui/aaa.rs:23:5: 23:23 (#0),
}

`use std::collections::HashMap;`

Item {
    ident: HashMap#0,
    def_id: DefId(0:5 ~ aaa[995b]::{misc#1}),
    kind: Use(
        Path {
            span: tests/ui/aaa.rs:8:5: 8:30 (#0),
            res: Def(
                Struct,
                DefId(1:1294 ~ std[928b]::collections::hash::map::HashMap),
            ),
            segments: [
                PathSegment {
                    ident: std#0,
                    hir_id: Some(
                        HirId {
                            owner: DefId(0:5 ~ aaa[995b]::{misc#1}),
                            local_id: 1,
                        },
                    ),
                    res: Some(
                        Def(
                            Mod,
                            DefId(1:0 ~ std[928b]),
                        ),
                    ),
                    args: None,
                    infer_args: false,
                },
                PathSegment {
                    ident: collections#0,
                    hir_id: Some(
                        HirId {
                            owner: DefId(0:5 ~ aaa[995b]::{misc#1}),
                            local_id: 2,
                        },
                    ),
                    res: Some(
                        Def(
                            Mod,
                            DefId(1:1193 ~ std[928b]::collections),
                        ),
                    ),
                    args: None,
                    infer_args: false,
                },
                PathSegment {
                    ident: HashMap#0,
                    hir_id: Some(
                        HirId {
                            owner: DefId(0:5 ~ aaa[995b]::{misc#1}),
                            local_id: 3,
                        },
                    ),
                    res: Some(
                        Err,
                    ),
                    args: None,
                    infer_args: false,
                },
            ],
        },
        Single,
    ),
    vis: Spanned {
        node: Inherited,
        span: tests/ui/aaa.rs:8:1: 8:1 (#0),
    },
    span: tests/ui/aaa.rs:8:1: 8:31 (#0),
}

`"100"`

Expr {
    hir_id: HirId {
        owner: DefId(0:7 ~ aaa[995b]::main),
        local_id: 27,
    },
    kind: Lit(
        Spanned {
            node: Str(
                "100",
                Cooked,
            ),
            span: tests/ui/aaa.rs:28:9: 28:14 (#0),
        },
    ),
    span: tests/ui/aaa.rs:28:9: 28:14 (#0),
}

---

changelog: Added [clippy::print_hir] to inspect rustc's internal representation

[rust-highfive]

r? @Manishearth

(rust-highfive has picked a reviewer for you, use r? to override)

[Manishearth]

This seems okay to me!

[flip1995]

Isn't this what the inspector lint with #[clippy::dump] already does?

[xFrednet]

Isn't this what the inspector lint with #[clippy::dump] already does?

We have a #[clippy::dump] attribute? o.O Well playing around with the attribute it looks like it wants to provide some additional information, but it sadly doesn't work on every node. I tested a few positions and none gave me any output. (Playground) So, I guess the difference is that clippy::print_hir really just prints the hir while clippy::dump tries to add additional information, which might not always work. Could you maybe give an example, were clippy::dump works?

[flip1995]

clippy::dump predates me, and I think I never used it myself. Though I think this is the better name for this attribute. Do you think it is worth investing time in the inspector to get it to a point where it works (with or without the additional information) as you would expect instead of this new attribute?

This new "lint" is a really trivial implementation, so I wouldn't be opposed to adding it, but we should then figure out what to do with the inspector. I don't think we need both tools?

[xFrednet]

The name does sound better! We could just replace the old implementation of clippy::dump with this one. Most of the code looks like it just names the values. I think it would be better to just use the debug output, as it closely represents the actual structure and requires less maintenance. If values are ambiguous, the effort would be better spent in improving the documentation in rustc itself, IMO. The old implementation would also sometimes resolve some objects, like Body objects from BodyIds, which we would lose, but that won't be a big deal as it doesn't seem to be working anyways. Then we also don't have to handle recursion etc.

[flip1995]

That sounds good to me. The inspector lint really didn't receive much love in the last few (at least 3-4) years. It is from a time where you couldn't just debug print the HIR and get something sensible. So I would be good with just replacing that implementation with this. If we should find, that resolving things would make sense, we can try to do this in this simpler impl instead of trying to fix legacy code.

[flip1995]

@bors r+

Thanks!

[bors]

📌 Commit ccedc64 has been approved by flip1995

[bors]

⌛ Testing commit ccedc64 with merge 95de4dc...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 95de4dc to master...