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

Making a Dylint binary? #839

Open
tigerros opened this issue Sep 15, 2023 · 15 comments
Open

Making a Dylint binary? #839

tigerros opened this issue Sep 15, 2023 · 15 comments
Labels
future enhancement Expands the project's scope

Comments

@tigerros
Copy link

How would one go about making a Dylint binary? In the guide, it only seems to cover libraries, but I would like to make a binary that can run in any package, like cargo clippy. Am I missing something?

@smoelius
Copy link
Collaborator

I want to be sure I understand what you are asking for. You want to create a set of lints, and a binary that runs those lints on an arbitrary project. Is that right?

If so, that's not really what Dylint is meant for. It's not discouraged, but it would likely require some engineering.

For example, Dylint expects lints to be encapsulated in dynamic libraries. So the binary would need to have a way of obtaining such libraries, e.g., by downloading or building them.

Having said that, Dylint itself can be used as a library. I am open to suggestions on how to make it more useable as a library.

Please let me know if I have misunderstood your question.

@tigerros
Copy link
Author

tigerros commented Sep 16, 2023

You want to create a set of lints, and a binary that runs those lints on an arbitrary project.

Exactly. As I mentioned, the ideal outcome would be a crates.io-distributed binary that runs similarly to cargo clippy.

So the binary would need to have a way of obtaining such libraries, e.g., by downloading or building them.

What would be a "proper" way of going about this? I really don't have any experience with rustc, sorry.

@smoelius
Copy link
Collaborator

Sorry, I don't follow. What do you mean by "proper"?

@tigerros
Copy link
Author

tigerros commented Sep 16, 2023

Nothing in particular. I simply mean a semi-reliable method that's not too "hacky". But to be honest, anything will suffice. I'm just looking for a starting point.

@smoelius
Copy link
Collaborator

smoelius commented Sep 16, 2023

Maybe? One advantage this approach would have is that you could reuse Dylint's drivers, which are extensible by design.

But if the driver only needs to run a fixed set of lints, one could argue they should be statically linked, like they are in Clippy.

Actually, here is a way one might accomplish that.

I very recently added a constituent feature to dylint_linting. The purpose of that feature is to make it easier to link a Dylint lint into "something larger." Up until now, "something larger" has been a library, but I think it could also be a binary.

You might be able to use that feature as follows. Build and test Dylint libraries as normal. Then, when you are ready to link the lints into a binary, enable the constituent feature on dylint_linting.

Here is the idea in slightly more detail.

The driver would have a register_lints function, similar to this one from Dylint's general library:

pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) {

The driver would call that function instead of calling one from a dynamic library, the way Dylint drivers do:

loaded_lib.register_lints(sess, lint_store);

The constituent feature would be enabled on dylint_linting when the lints are linked into the driver, similar to how it is enabled when the general-purpose lints are linked into Dylint's general library:

await_holding_span_guard = { path = "await_holding_span_guard", features = ["rlib"] }

rlib = ["dylint_linting/constituent"]

Sorry, that was a mouthful. But I would be happy to expand on any of the above.

@tigerros
Copy link
Author

I'll try it out, thanks!

@tigerros
Copy link
Author

tigerros commented Sep 16, 2023

I've got the linter structure just like in the examples, but I'm confused about how to actually call and use the lints from the binary main.rs. What do you mean by "driver"? The main.rs, or something else?

@smoelius
Copy link
Collaborator

The situation is not as simple as one would hope.

Most linters that use rustc's linting interface are divided into two binaries: a command line tool (e.g., a cargo subcommand) and a driver. The driver is essentially a wrapper around the rust compiler and is invoked by Cargo for each crate that needs to be compiled. The command line tool tells Cargo where to find the driver by setting the RUSTC_WORKSPACE_WRAPPER environment variable. Both Clippy and Dylint work this way.

This blog post provides more details: https://blog.trailofbits.com/2021/11/09/write-rust-lints-without-forking-clippy/

So what I imagine you are writing now as your main.rs will be your command line tool. But you will additionally have to write a driver, invoke Cargo, etc. as described above.

@tigerros
Copy link
Author

Thanks a lot! I've been putting this off for the last two weeks, but I had a look at the Clippy source code and it doesn't seem that bad. This does mean that I won't be using Dylint though. Would love to see an update that focuses on creating binary linters,

@smoelius
Copy link
Collaborator

@tigerros I'm attaching a future-enhancement label to this issue.

While Dylint could be used to help create a linting binary (e.g., like I described here), that's not really what Dylint was meant for.

I think the idea of creating a linting binary framework has merit. It's just a little outside of Dylint's scope.

@smoelius smoelius added the future enhancement Expands the project's scope label Nov 20, 2023
@tigerros
Copy link
Author

tigerros commented Nov 20, 2023

@smoelius I saw rust-marker being suggested somewhere, as a way to create a CLI linter. However, I'm not quite sure what it is, I've just kind of skimmed over it, and the docs aren't stellar. You're actually one of the contributors, so I'm wondering if you forgot about it or if it's not what I was looking for. I saw the "crate lint" example, but that seems similar to Dylint, because it's a cdylib, not a binary.

@smoelius
Copy link
Collaborator

Marker is similar to Dylint, in that it allows you to write and run individual lints.

A (perhaps "the") key difference is that Marker provides its own, stable linting API, whereas Dylint provides direct access to the Rust compiler linting APIs.

However, for both Dylint and Marker, the compiled artifacts are cdylibs. In other words, I think some "finagling" would be required to build a linting binary using either tool.

cc: @xFrednet (who should not feel obligated to comment)

@xFrednet
Copy link

@smoelius summarized it pretty well. The interface of Marker is currently a bit more limited than the rustc one dylint exposes. Though, I think it's sufficient for a good number of lints already and more user-friendly in some regards.

Regarding the requested feature in the context of Marker: We have rust-marker/marker#258, which requests the implementation of static linking for lint crates. We also have an RFC rust-marker/marker#307 which suggests a way to potentially combine multiple lint crates into a single cdylib.

@tigerros could you explain a bit how you would like to use this feature? Marker and I believe dylint, both provide an CLI which hides the lint crate compilation. For example, in Marker you can use cargo marker and that will download specified lint crates and run them. The CLI basically behaves like Clippy, just with a few extra steps.

@tigerros
Copy link
Author

@xFrednet Yes, I want to make something like Clippy, just for a specific framework. I think Marker will do nicely. I'm having some issues with installation though. I get this output when running cargo marker setup --auto-install-toolchain. I have rustc, rustc-dev and llvm-tools installed. Error log:

cargo marker setup --auto-install-toolchain
info: syncing channel updates for 'nightly-2023-10-05-x86_64-pc-windows-msvc'
724.5 KiB / 724.5 KiB (100 %) 573.5 KiB/s in  1s ETA:  0s
info: latest update on 2023-10-05, rust version 1.75.0-nightly (2bbb61989 2023-10-04)
info: component 'llvm-tools' for target 'x86_64-pc-windows-msvc' is up to date
info: component 'rustc-dev' for target 'x86_64-pc-windows-msvc' is up to date

  nightly-2023-10-05-x86_64-pc-windows-msvc unchanged - rustc 1.75.0-nightly (2bbb61989 2023-10-04)

info: checking for self-update

      Marker compiling rustc driver v0.4.0 with nightly-2023-10-05
    Updating crates.io index
  Installing marker_rustc_driver v0.4.0
    Updating crates.io index
   Compiling proc-macro2 v1.0.69
   Compiling unicode-ident v1.0.12
   Compiling windows_x86_64_msvc v0.48.5
   Compiling cfg-if v1.0.0
   Compiling once_cell v1.18.0
   Compiling cc v1.0.83
   Compiling winapi v0.3.9
   Compiling thiserror v1.0.50
   Compiling rustc-demangle v0.1.23
   Compiling lazy_static v1.4.0
   Compiling tracing-core v0.1.32
   Compiling sharded-slab v0.1.7
   Compiling windows-targets v0.48.5
   Compiling thread_local v1.1.7
   Compiling windows-sys v0.48.0
   Compiling quote v1.0.33
   Compiling serde v1.0.193
   Compiling pin-project-lite v0.2.13
   Compiling syn v2.0.39
   Compiling unicode-linebreak v0.1.5
   Compiling is_ci v1.1.1
   Compiling smawk v0.3.2
   Compiling unicode-width v0.1.11
   Compiling tracing v0.1.40
   Compiling backtrace v0.3.69
   Compiling textwrap v0.15.2
   Compiling tracing-subscriber v0.3.18
   Compiling is-terminal v0.4.9
   Compiling terminal_size v0.1.17
   Compiling supports-unicode v2.0.0
   Compiling supports-color v2.1.0
   Compiling supports-hyperlinks v2.1.0
   Compiling camino v1.1.6
   Compiling owo-colors v3.5.0
   Compiling either v1.9.0
   Compiling backtrace-ext v0.2.1
   Compiling tracing-error v0.2.0
   Compiling rustc_tools_util v0.3.0
   Compiling yansi v1.0.0-rc.1
   Compiling itertools v0.11.0
   Compiling marker_rustc_driver v0.4.0
   Compiling libloading v0.8.1
   Compiling bumpalo v3.14.0
   Compiling typed-builder-macro v0.16.2
   Compiling thiserror-impl v1.0.50
   Compiling miette-derive v5.10.0
   Compiling serde_derive v1.0.193
   Compiling visibility v0.1.0
   Compiling typed-builder v0.16.2
   Compiling marker_api v0.4.0
   Compiling miette v5.10.0
   Compiling marker_error v0.4.0
   Compiling marker_utils v0.4.0
   Compiling marker_adapter v0.4.0
error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:2:1
  |
2 | #![feature(rustc_private)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:3:1
  |
3 | #![feature(let_chains)]
  | ^^^^^^^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:4:1
  |
4 | #![feature(lint_reasons)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:5:1
  |
5 | #![feature(iter_collect_into)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:6:1
  |
6 | #![feature(non_exhaustive_omitted_patterns_lint)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:7:1
  |
7 | #![feature(once_cell_try)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:5:12
  |
5 | #![feature(iter_collect_into)]
  |            ^^^^^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\aurel\.cargo\registry\src\index.crates.io-6f17d22bba15001f\marker_rustc_driver-0.4.0\src\lib.rs:7:12
  |
7 | #![feature(once_cell_try)]
  |            ^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0554`.
error: could not compile `marker_rustc_driver` (lib) due to 8 previous errors
warning: build failed, waiting for other jobs to finish...
error: failed to compile `marker_rustc_driver v0.4.0`, intermediate artifacts can be found at `C:\Users\aurel\AppData\Local\Temp\cargo-installQlgqm7`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.
  x Failed to build the custom marker rustc driver
  help: Make sure that you have the rustc-dev and llvm-tools components installed.
        Try running the following command:
        cargo marker setup --auto-install-toolchain

        or:
        rustup toolchain install {toolchain} --component rustc-dev llvm-tools

@xFrednet
Copy link

Awesome, if you run into any problems, you're always welcome to create an issue in the Marker repo. These kinds of issues are an enormous help.

For the bug: I don't use windows, so this might have slipped through. I've created rust-marker/marker#318 to track this bug and also included a suggestion on how to fix it for now.

Was the CLI the main reason for requesting static linking of lint crates, or are there other motivations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future enhancement Expands the project's scope
Projects
None yet
Development

No branches or pull requests

3 participants