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

Updating an existing dylint lint #1264

Closed newcomertv closed 1 month ago

newcomertv commented 1 month ago

Hey,

The project is awesome and really helped me out in a pinch when I needed to write a quick and dirty lint.

After abandoning my quick lint and coming back to give it some much needed love I came across this issue:

Based on the documentation I wasn't able how to figure out how to update an existing dylint lint. My steps to reproduce are probably pretty vague since I originally wrote the lint a couple months ago..

Steps I did:

At this point I'm expecting there to be a magic trick how to re-register a dylint lint without needing to run cargo dylint new

NOTE: I have a rust-toolchain.toml

Link to project: https://github.com/newcomertv/parameter-prefix-dylint-rs

smoelius commented 1 month ago

Hi, @newcomertv. Thanks for your kind words, and thanks for providing the link to the repo.

The "magic trick" is this:

cargo dylint upgrade .

This will update your project's rust-toolchain file to a newer nightly channel, and update the clippy_utils dependency in the Cargo.toml file.

I tried this on you project and ran into an issue with cx.opt_span_lint. I think you're project may have been bitten by: https://github.com/rust-lang/rust/pull/125410

I removed this line and everything seemed to build: https://github.com/newcomertv/parameter-prefix-dylint-rs/blob/aa41c9481cbd3d01fd2dc877eec6a1f7f094b650/src/lib.rs#L119

I would need to study that PR more to know if this is the "right" fix, though.

Please let me know if I have misunderstood what you were asking about.

newcomertv commented 1 month ago

Thanks for the response!

After running cargo dylint upgrade it updated the toolchain to the nightly from the 25th of July. (4 days ago) I'm guessing that was my latest rust nightly already installed.

Attempting to build failed due to being unable to find the extern crates.

After a closer look the rust-toolchain for dylint is set to "nightly" Meanwhile the toolchain for the lint was set to "nightly-<DATE>"

After changing the rust-toolchain to also reflect "nightly" everything builds.

Running: 'cargo dylint list --path .' yields the following:


Building workspace metadata entry `function_parameter_prefix`
   Compiling proc-macro2 v1.0.82
   Compiling unicode-ident v1.0.12
   Compiling serde v1.0.202
   Compiling hashbrown v0.14.5
   Compiling equivalent v1.0.1
   Compiling serde_json v1.0.117
   Compiling winnow v0.6.8
   Compiling semver v1.0.23
   Compiling thiserror v1.0.61
   Compiling camino v1.1.7
   Compiling itoa v1.0.11
   Compiling winnow v0.5.40
   Compiling anyhow v1.0.85
   Compiling ryu v1.0.18
   Compiling rustc_apfloat v0.2.1+llvm-462a31f5a5ab
   Compiling paste v1.0.15
   Compiling rustversion v1.0.17
   Compiling bitflags v1.3.2
   Compiling rustc-semver v1.1.0
   Compiling smallvec v1.13.2
   Compiling either v1.12.0
   Compiling arrayvec v0.7.4
   Compiling itertools v0.12.1
   Compiling quote v1.0.36
   Compiling indexmap v2.2.6
   Compiling syn v2.0.64
   Compiling serde_derive v1.0.202
   Compiling thiserror-impl v1.0.61
   Compiling serde_spanned v0.6.6
   Compiling toml_datetime v0.6.6
   Compiling toml_edit v0.22.13
   Compiling toml v0.8.13
   Compiling dylint_linting v3.1.2
   Compiling cargo-platform v0.1.8
   Compiling toml_edit v0.19.15
   Compiling cargo_metadata v0.18.1
   Compiling toml v0.7.8
   Compiling dylint_internal v3.1.2
   Compiling clippy_config v0.1.82 (https://github.com/rust-lang/rust-clippy?rev=37f4fbb92913586b73a35772efd00eccd1cbbe13#37f4fbb9)
   Compiling clippy_utils v0.1.82 (https://github.com/rust-lang/rust-clippy?rev=37f4fbb92913586b73a35772efd00eccd1cbbe13#37f4fbb9)
   Compiling function_parameter_prefix v0.1.0 (/home/nikolas/git/third-party/dylint/parameter-prefix-dylint-rs)
    Finished `release` profile [optimized] target(s) in 12.75s
Error: Could not find "/<path to git\>/git/third-party/dylint/parameter-prefix-dylint-rs/target/dylint/libraries/nightly-x86_64-unknown-linux-gnu/release/libfunction_parameter_prefix@nightly-x86_64-unknown-linux-gnu.so" despite successful build
newcomertv commented 1 month ago

My understanding is its supposed to work with the specific date as well. With dylint being supposed to take whatever nightly my lint specifies.

This doesn't work so I try removing the date however dylint expects a date and is unable to tell me that it built without encoding the nightly version in the library version.

EDIT: renaming the produced so to the expected output name works.

newcomertv commented 1 month ago

I'm closing the issue since the original question has been answered. I'm still unsure if my behavior is considered expected. If not I'll open a different issue.

Thanks for the help and the great library

smoelius commented 1 month ago

After running cargo dylint upgrade it updated the toolchain to the nightly from the 25th of July. (4 days ago) I'm guessing that was my latest rust nightly already installed.

This is actually based on what Clippy uses: https://github.com/rust-lang/rust-clippy/blob/f7db8952e611e359ce3fb04c85d2ffb92036a55c/rust-toolchain#L2

Since clippy_utils uses Rust's internal APIs, Dylint has to upgrade the clippy_utils version and Rust toolchain channel in tandem (i.e., to make sure they stay in sync).

My understanding is its supposed to work with the specific date as well. With dylint being supposed to take whatever nightly my lint specifies.

At a high level, that is correct. But how was your lint specifying the date? In other words, what date were you expecting Dylint to find that it was not?

The way Dylint normally finds the date is from the filename (as I think you figured out).

Also, I noticed that the repo does not include a .cargo/config.toml file. You might consider adding one like this: https://github.com/trailofbits/dylint/blob/08992ae72df0e2098673d3dc25b4d44df21dc353/examples/general/.cargo/config.toml#L4-L5 That will cause a file with the toolchain date to be generated automatically.