trailofbits / dylint

Run Rust lints from dynamic libraries
https://blog.trailofbits.com/2021/11/09/write-rust-lints-without-forking-clippy/
Apache License 2.0
367 stars 21 forks source link

How to use rust-analyzer inside a lint library? #1274

Closed hlf20010508 closed 1 month ago

hlf20010508 commented 1 month ago

I saw the VS Code integration in README.md, but it didn't work.

image

It reported unresolved extern crate.

cargo build and cargo test passed, cargo dylint list --path . failed and reported

error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort`

warning: `lint` (lib) generated 17 warnings
error: could not compile `lint` (lib) due to 1 previous error; 17 warnings emitted
Error: command failed: "cargo" "build" "--release" "--target-dir" "C:\\Users\\Administrator\\Programs\\telegram-onedrive\\target\\dylint/libraries\\nightly-2024-05-02-x86_64-pc-windows-msvc"

I've enabled "rust-analyzer.rustc.source": "discover", in vscode settings.json.

I've searched for hours, but I don't know what to do.

What I want is able to use r-a to write lints.

I would appreciate if someone can help me.

smoelius commented 1 month ago

Hi, @hlf20010508. Sorry for the difficulty. The r-a problems sound distinct from this one:

error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort`

warning: `lint` (lib) generated 17 warnings
error: could not compile `lint` (lib) due to 1 previous error; 17 warnings emitted
Error: command failed: "cargo" "build" "--release" "--target-dir" "C:\\Users\\Administrator\\Programs\\telegram-onedrive\\target\\dylint/libraries\\nightly-2024-05-02-x86_64-pc-windows-msvc"

I don't believe I've seen that error message before. Googling revealed this: https://users.rust-lang.org/t/panic-unwind-is-not-compiled-with-this-crates-panic-strategy-abort/97154 Is it possible you have CARGO_PROFILE_RELEASE_PANIC set?

Can I presume you ran cargo dylint new lint before running cargo dylint list --path .? Does your directory structure look like this? https://github.com/trailofbits/dylint/tree/master/internal/template

Regarding the r-a problems, is the directory containing the lint lint opened as a workspace in VS Code?

hlf20010508 commented 1 month ago

Thanks for your reply. I do have a setting in Cargo.toml in parent project

[profile.release]
panic = 'abort'

After commenting this line, cargo dylint list --path . ran successfully.

But the r-a problem remains. Yes I used cargo dylint new lint first, in my existing project, and added it as workspace member. Here's my project screenshot:

image

smoelius commented 1 month ago

I'm glad you were able to resolve some of the issues.

If you run rustup component list --installed, do you see something like this?

cargo-aarch64-apple-darwin
clippy-aarch64-apple-darwin
llvm-tools-aarch64-apple-darwin
rust-docs-aarch64-apple-darwin
rust-src                        # <-- important one
rust-std-aarch64-apple-darwin
rustc-aarch64-apple-darwin
rustc-dev-aarch64-apple-darwin
rustfmt-aarch64-apple-darwin

Most notably, do you see rust-src?

If you do see rust-src, then I'm going to have to try to get access to a Windows box and see if I can reproduce the problem. I don't have any other ideas at the moment.

hlf20010508 commented 1 month ago

I really appreciate your help, I don't have rustc-dev installed. Now it works fine. By the way, I saw your comment in https://github.com/trailofbits/dylint/issues/26#issuecomment-866718949 said that

anything passed to Dylint after -- is passed to cargo check

Is there a way to pass it to cargo clippy instead of cargo check? I still want to be basically linted by clippy, and use dylint only to do custom lint.

smoelius commented 1 month ago

I really appreciate your help,

I appreciate you using Dylint! 🙏

Is there a way to pass it to cargo clippy instead of cargo check? I still want to be basically linted by clippy, and use dylint only to do custom lint.

To be sure I understand, you want cargo dylint ... -- ARGS to execute cargo clippy ARGS?

You can't do this exactly. I think your best bet would be to run two commands (e.g., cargo dylint ... and cargo clippy ARGS).

Please let me know if I've misunderstood what you're asking.


I doubt this will be helpful, but we do package Clippy in shared library, e.g., the following command will lint a project with Clippy:

cargo dylint --git https://github.com/trailofbits/dylint --pattern examples/testing/clippy

This is really just meant for testing purposes, though.

hlf20010508 commented 1 month ago

Like Clippy, Dylint runs cargo check under-the-hood

I mean can I config to let Dylint run cargo clippy under-the-hood?

I guess Dylint append it's custom lint result to cargo check's result right?

I think your best bet would be to run two commands (e.g., cargo dylint ... and cargo clippy ARGS).

If I do so, I think the process would be cargo check + dylint + cargo clippy. So the cargo check is unnecessary.

smoelius commented 1 month ago

I mean can I config to let Dylint run cargo clippy under-the-hood?

No, not currently. TBH, I think of them as distinct tools, so I don't see this feature being added.

I guess Dylint append it's custom lint result to cargo check's result right?

Essentially, yes. To be slightly more precise, Dylint adds its lints to the set that cargo check runs by default, so then the results of both sets of lints are intermingled. Clippy works similarly.

If I do so, I think the process would be cargo check + dylint + cargo clippy. So the cargo check is unnecessary.

You are correct in that running cargo dylint and cargo clippy amounts to running cargo check twice, so there is some redundancy/overhead. If you absolutely want to avoid this redundancy, you can use the Clippy library in examples/testing/clippy (like I mentioned above). But cargo check is pretty quick. So I wouldn't expect the overhead to be noticeable.

FWIW, I run Clippy and Dylint on pretty much all of my projects, and I run them as two separate commands.

hlf20010508 commented 1 month ago

Thanks for your answer. I'll run two commands.

Can you point out which part of this repo contains the logic of this

Dylint adds its lints to the set that cargo check runs by default

I want to have a look if I can do something for it, or just for study.

hlf20010508 commented 1 month ago

Currently I use python to combine outputs of dylint and clippy.

import subprocess

dylint_command = [
    "cargo",
    "dylint",
    "--all",
    "--",
    "--all-targets",
    "--message-format=json",
]
dylint_output = subprocess.run(dylint_command, capture_output=True, text=True)

clippy_command = ["cargo", "clippy", "--all-targets", "--message-format=json"]
clippy_output = subprocess.run(clippy_command, capture_output=True, text=True)

combined_output = dylint_output.stdout + clippy_output.stdout

print(combined_output)

and then set r-a setting to

{
    "rust-analyzer.check.overrideCommand": [
        "python",
        "lint/check.py"
    ],
}

It works as what I expected.

smoelius commented 1 month ago

Can you point out which part of this repo contains the logic of this

Dylint adds its lints to the set that cargo check runs by default

I want to have a look if I can do something for it, or just for study.

Sure. In Clippy, it happens here: https://github.com/rust-lang/rust-clippy/blob/2fc74a39318e7786a0e335e6232dcf346c1a4df1/src/driver.rs#L144-L155

And In Dylint, it happens here: https://github.com/trailofbits/dylint/blob/353a81655009eba73d949228f8539ac995ae72d9/driver/src/lib.rs#L240-L292

A high-level summary is: you construct a rustc_interface::Config, set its register_lints callback, and then run the compiler with that Config.

For Clippy, the callback just registers all of the lints in Clippy.

Dylint's implementation is a little more complicated. It allows each library to have its own register_lints function. Dylint's callback calls each of them. But there are a some additional complexities on top of that, e.g., Dylint allows listing the lints in a library.

If it's helpful, I wrote this blogpost around the time Dylint was released: https://blog.trailofbits.com/2021/11/09/write-rust-lints-without-forking-clippy/

smoelius commented 1 month ago

Currently I use python to combine outputs of dylint and clippy. ... It works as what I expected.

Nice!

I am not a Python expert. Do you know if there's a way to do this without relying on the file system (e.g., lint/check.py)?

hlf20010508 commented 1 month ago

I've simplified the script to

import os

os.system("cargo dylint --all -- --all-targets --message-format=json")
os.system("cargo clippy --all-targets --message-format=json")

Do you know if there's a way to do this without relying on the file system?

Yes, by using

python -c "import os;os.system('cargo dylint --all -- --all-targets --message-format=json');os.system('cargo clippy --all-targets --message-format=json')"

it can be run without a file, and it's not too lengthy.

And thanks for your knowledge, it helped me a lot.

hlf20010508 commented 1 month ago

For r-a setting

{
    "rust-analyzer.check.overrideCommand": [
        "python",
        "-c",
        "import os;os.system('cargo dylint --all -- --all-targets --message-format=json');os.system('cargo clippy --all-targets --message-format=json')"
    ],
}