Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating an existing dylint lint #1264

Closed
newcomertv opened this issue Jul 26, 2024 · 5 comments
Closed

Updating an existing dylint lint #1264

newcomertv opened this issue Jul 26, 2024 · 5 comments

Comments

@newcomertv
Copy link

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:

  • Create a new lint using dylint new ..

  • Edit

  • Use a bunch

  • Update toolchains

  • Use a bunch

  • Edit

  • cargo build

  • -> lint doesn't update, uses old description and way of operating (the old .so)

  • run cargo clean

  • library not found

  • cargo build

  • library not found

  • cargo dylint --lib-path path/to/release/lint.so

  • invalid filename, does not respect the format

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
Copy link
Collaborator

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: rust-lang/rust#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
Copy link
Author

newcomertv commented Jul 29, 2024

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
Copy link
Author

newcomertv commented Jul 29, 2024

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
Copy link
Author

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
Copy link
Collaborator

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:

[target.'cfg(all())']
linker = "dylint-link"

That will cause a file with the toolchain date to be generated automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants