Future-incompatible warnings should always print a warning, even if lints are allowed

[nikomatsakis]

Currently, if a future-incompatible lint is set to allow (either manually or via cap-lints), no warning is printed. This is bad because it means that if you author a library that is widely used, but which you are not actively using at the moment, you might not notice that it will break in the future -- moreover, your users won't either, since cargo will cap lints when it builds your library as a dependency.

So basically the behavior for future-incompatible lints would be that they can either be "warn" or "deny" but can never be completely silenced. If cap-lints is set to allow, then while other lints will be set to allow, the future-incompatible lints will be set to warn (but not deny).

cc @rust-lang/compiler @brson

[nikomatsakis]

I'm marking this as mentor because I'd be happy to mentor it (though until July 15 I'm on vacation). I marked it as easy because think this should be relatively straightforward, though I may be wrong about that. I'll try to look at the code in a bit and print out a summary of what would need to be done.

[hgallagher1993]

If it is fairly straight forward I'll have a go at it

[brson]

You got it @hgallagher1993!

[hgallagher1993]

Hi @nikomatsakis I was just wondering did you get a chance to look at this task just to see if it's fairly straightforward 😃

[pnkfelix]

@hgallagher1993 doubtful since @nikomatsakis noted that he was on vacation until July 15th.

[hgallagher1993]

@pnkfelix I read his message as he was going on vacation on the 15th...woops 😆

[nikomatsakis]

@hgallagher1993 glad to hear you are interested! I never did take a look, but I can do so now!

[nikomatsakis]

@hgallagher1993 ok, sorry for the delay. I did a bit of digging. I think the changes you need to make are roughly this:

In src/librustc/lint.rs, there is a function called set_level -- it changes the level of reporting for a lint. Currently, it first checks something called the lint_cap, which records a maximum level (this is used by cargo so that you don't see "unused variable" warnings for your dependencies).

What we want to do I think is change that function to also check whether self.future_incompatible(lint).is_some(), in which case this is a future-incompatibility warning. In that case, after applying the lint-cap, we should set the level to max(level, warn) -- in other words, ensure the level is at least warn, regardless of what the lint-cap says.

We can also add some test cases for this using the testing framework. The simplest test would be something that takes some future-incompatible lint and sets it to #[allow] -- we should still get a warning. We can also have a test that passes -A cap-lints=allow, which should yield the same result. I can help you more w/ writing said tests if you need.

(One twist: we may want to add a "dummy" future-incompatible lint just for testing purposes...)

[nikomatsakis]

That said, @wycats has been arguing to me that this would be a bad thing to do. It may be worth having a wider discussion if there are other approaches we might consider.

[nikomatsakis]

I think, if I may take a stab at summarizing @wycats's point-of-view, that his position is:

• seeing warnings in code you don't control is annoying (true)
• users are unlikely to file issues or know how to respond (seems true)
  • they may well respond by just downgrading
• we would be better off finding other ways to alert crates.io maintainers of upcoming problems, and finding other ways to alert end-users of the need for action
  • for example, we already try to alert crate owners to warnings that we uncover via crater runs, but we could try to have some kind of dashboard or other thing you can look at...or something...
    • this might include other statistics too, like docs etc
  • we could encourage people to cargo yank soon-to-be-broken versions and give a reason, for example citing the upcoming change
    • this could then warn users that they should update their dependencies to newly available version

[jacwah]

@hgallagher1993 Do you still want to fix this issue? Otherwise I'd be willing to handle it.

@nikomatsakis Considering @wycats' comments, do you think the implementation you outlined above still should be followed?

Some thoughts:

users are unlikely to file issues or know how to respond

Maybe the message could link to the crate's homepage/repository and invite the user to contact the maintainer?

seeing warnings in code you don't control is annoying

This is very true. Say there's a very stable application working well and doing it's thing. There's no reason to change it or upgrade Rust version. This is a case where silencing future incompatible warnings may be desirable.

[nikomatsakis]

@jacwah I'm honestly not sure what to do here. What we're doing now doesn't seem right, but just enabling the warnings also doesn't seem ideal. I think perhaps what is needed is better integration with cargo and maybe even crates.io -- i.e., if we (the compiler team) could leave annotations on abandoned crates, and those could be summarized to people who depend on them, that might be ideal.

cc @wycats @alexcrichton