Make custom lints #[allow]-able without warnings

[danielhenrymantilla]

First of all, awesome project and initiative, I love it!

---

No matter how good the custom-defined lints may be, at some point we may wish to #[allow(…)]-silence/gag them. Alas, the naïve #[allow(lint_name)], e.g., #[allow(allow_clippy)], triggers warnings from the unknown_lint lint.

Would it be possible to offer a way to prevent this? I personally have two ideas in that regard:

• Prefix / namespace the lints, much like clippy does: #[allow(dylint::allow_clippy)]. Then do the shenanigans to make rustc understand about this prefix (is it a custom tool?)

• Whenever the pass including a driver is being run, have --cfg dylint_pass(that_lint_name) added to rustc. Given that several lints may be coalesced into a single driver pass (the very point of dylint's design, IIUC), this may enable several such cfgs simultaneously, which is not a problem but something to be mindful of.

  This way, one can, at least, do something like:

  #![cfg_attr(dylint_pass(lint_name), allow(lint_name))]

[smoelius]

Thanks for your kind words, @danielhenrymantilla, and thanks for this suggestion. :)

The way I am dealing with this problem now is to just allow the unknown_lint lint, e.g.:

dylint/dylint/src/lib.rs

Lines 209 to 210 in 3966910

	#[allow(unknown_lints)]
	#[allow(question_mark_in_expression)]

If we put all of the examples in a dylint namespace, I think that would force users that want to allow lints to #![register_tool(dylint)]. And I think that feature is still unstable (rust-lang/rust#66079). So users that allow lints would then have to compile with a nightly compiler, which I would prefer to avoid.

Your second idea is intriguing one. I'm just wondering whether it's a little heavy-handed compared to #[allow(unknown_lints)].

I want to be sure that we're solving the right problem, though. You would like the example lints to be put in their own namespace? Because if you write your own lints, you could put them in their own namespace. This might be a viable solution if you don't mind having to compile with a nightly compiler.

If your code originates from dylint-template, you would replace delcare_lint with declare_tool_lint on this line:
https://github.com/trailofbits/dylint-template/blob/8d75aa91ebae10a75c08c3f9f3cbadc77018e925/src/fill_me_in.rs#L4
Then this line would become something like pub my_namespace::MY_LINT_NAME:
https://github.com/trailofbits/dylint-template/blob/8d75aa91ebae10a75c08c3f9f3cbadc77018e925/src/fill_me_in.rs#L20

Apologies if I've misunderstood your intent.

[danielhenrymantilla]

Thanks for the remarks, I didn't know well the implication of tool lints w.r.t. stability, so indeed the first solution is not that good, yet 👍

W.r.t. the second one, basically, the issue with #[allow(unknown_lints)] is kind of similar to #[allow(deprecated)] when there is one deprecated item which you don't mind using: it's too coarse and thus allows any kind of unknown lint.

• e.g., imagine writing: #![forbid(unsafe)] or #![forbid(unsafe_blocks)] or #![forbid(code_unsafe)]. We'd really like to have the unknown_lints warn us against such typos.

It's still the least bad solution in the interim, but I was wondering how hard would it be for cargo dylint to integrate those --cfg additions I was suggesting. Actually, if you are not against the idea, I might take care of submitting a PR for those myself 🙂

---

Asides

• Thanks for the indications w.r.t. namespacing the lints; I wasn't originally planning on doing so, but I actually like the idea very much!

• I came up with #[clippy::author] which, even if they appear within clippy's documentation, seems to be a valuable starting point for beginners wanting to lint against specific expression / statement patterns (e.g., your env_literal lint).

  I thus personally think it could be valuable to directly mention it, as well as the dylint-template repo (I'd even suggest making a cargo dylint new my_lint subcommand which would basically do a (git clone …/dylint-template my_lint && cd my_lint && bash script_name.sh my_lint) + print on stdout the workspace metadata lines to load it by path, with a commented out repo line).

  That could make for a great "Getting Started" section allowing people to start hacking right away 🙂

[smoelius]

W.r.t. the second one, basically, the issue with #[allow(unknown_lints)] is kind of similar to #[allow(deprecated)] when there is one deprecated item which you don't mind using: it's too coarse and thus allows any kind of unknown lint.

• e.g., imagine writing: #![forbid(unsafe)] or #![forbid(unsafe_blocks)] or #![forbid(code_unsafe)]. We'd really like to have the unknown_lints warn us against such typos.

I get it, and I agree that the #[allow(unknown_lints)] solution is not ideal.

It's still the least bad solution in the interim, but I was wondering how hard would it be for cargo dylint to integrate those --cfg additions I was suggesting. Actually, if you are not against the idea, I might take care of submitting a PR for those myself 🙂

I don't think it would be too hard. I think my main concern is the additional complexity it would introduce when there is a workable solution now, and a better solution potentially around the corner.

• I came up with #[clippy::author] ...

Thanks! I didn't know this existed.

which, even if they appear within clippy's documentation, seems to be a valuable starting point for beginners wanting to lint against specific expression / statement patterns (e.g., your env_literal lint).
I thus personally think it could be valuable to directly mention it, ...

Done (ee8fb3c).

as well as the dylint-template repo (I'd even suggest making a cargo dylint new my_lint subcommand which would basically do a (git clone …/dylint-template my_lint && cd my_lint && bash script_name.sh my_lint) + print on stdout the workspace metadata lines to load it by path, with a commented out repo line).
That could make for a great "Getting Started" section allowing people to start hacking right away 🙂

These are good suggestions. I've opened issues for them (#31 and #32).

[smoelius]

• Whenever the pass including a driver is being run, have --cfg dylint_pass(that_lint_name) added to rustc. Given that several lints may be coalesced into a single driver pass (the very point of dylint's design, IIUC), this may enable several such cfgs simultaneously, which is not a problem but something to be mindful of.

Given that stabilization of register_tool has stalled, I have reevaluated my position on this.

The one change I would like to make is that the option be:

--cfg 'dylint_lib="library_name"'

So in code, one would write:

#![cfg_attr(dylint_lib="library_name", allow(lint_name))]

(As before, there could be multiple such cfgs.)

One reason for doing it this way is: getting the library name is easy, but getting the lint / lint pass names is more challenging, because one has to load and run code from the library.

Moreover, I think this approach is intuitive. Lints are already loaded "in batches," i.e., with each library being a "batch." This approach would similarly enable these lint level settings "in batches."

@danielhenrymantilla I'm curious if you'd still be interested to work on this.

[danielhenrymantilla]

Heh, FWIW I was interested by this but have been super busy, thank you @smoelius for taking care of it! 👌

[smoelius]

No problem. It was a really good suggestion. Thanks again for proposing it. 🙏