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

cargo --config doesn't (seem) to work with lints.* #13018

Closed
kristof-mattei opened this issue Nov 20, 2023 · 5 comments
Closed

cargo --config doesn't (seem) to work with lints.* #13018

kristof-mattei opened this issue Nov 20, 2023 · 5 comments
Labels
C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@kristof-mattei
Copy link

kristof-mattei commented Nov 20, 2023

Problem

I'm trying to set up a separate linting strategy for local development with everything as a warning, but still allow me to compile. Who cares about dead code / unused imports when I'm iterating over a feature? I'll tidy up later.

But on the the CI I want things dialed up to the max.

With the release of lints support in Rust 1.74 I've been trying to get it to work together with --config <PATH> / --config KEY=VALUE, and I've not been successful.

Steps

  1. cargo new lint-config
  2. Put the following in src/main.rs:
    fn main() {
        let vârïàblê = "non-ascii";
    
        println!("Hello, {}!", vârïàblê);
    }
  3. Put the following in ci.toml:
    [lints.rust]
    non_ascii_idents="deny"
  4. Try and build with
    • cargo --config "lints.rust.non_ascii_idents = \"deny\"" build
    • cargo --config lints.rust.non_ascii_idents=\"deny\" build
    • cargo --config "lints.rust.non_ascii_idents='deny'" build
    • cargo --config 'lints.rust.non_ascii_idents="deny"' build
    • cargo --config ci.toml build
      In all situations I've tried forbid instead of deny, and moving build before config.
  5. Notice that in none of the builds we get a failure, even though we expect it to.

Possible Solution(s)

No response

Notes

  1. Moving the lint from ci.toml to Cargo.toml and building with cargo build produces the expected result:
    kristof@NOSTROMO:~/lint-config$ cargo build
       Compiling lint-config v0.1.0 (/home/kristof/lint-config)
    error: identifier contains non-ASCII characters
     --> src/main.rs:2:9
      |
    2 |     let vârïàblê = "non-ascii";
      |         ^^^^^^^^
      |
      = note: requested on the command line with `-D non-ascii-idents`
    
    error: could not compile `lint-config` (bin "lint-config") due to previous error    
  2. Making sure this is supported I tried with cargo --config build.target-dir=\"somewhere-else\" build which changes the target dir & works as expected.

Version

cargo 1.74.0 (ecb9851af 2023-10-18)
release: 1.74.0
commit-hash: ecb9851afd3095e988daaa35a48bc7f3cb748e04
commit-date: 2023-10-18
host: x86_64-unknown-linux-gnu
libgit2: 1.7.1 (sys:0.18.0 vendored)
libcurl: 8.4.0-DEV (sys:0.4.68+curl-8.4.0 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Ubuntu 23.10 (mantic) [64-bit]
@kristof-mattei kristof-mattei added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Nov 20, 2023
@weihanglo
Copy link
Member

--config CLI arg is for setting cargo configuration value, i.e. .cargo/config.toml. It is not for Cargo.toml.

@weihanglo
Copy link
Member

Here is the strategy rust-lang/cargo itself uses currently.

Set lint rules for development in Cargo.toml:

cargo/Cargo.toml

Lines 109 to 121 in 71cd3a9

[workspace.lints.rust]
rust_2018_idioms = "warn" # TODO: could this be removed?
[workspace.lints.rustdoc]
private_intra_doc_links = "allow"
[workspace.lints.clippy]
all = { level = "allow", priority = -1 }
dbg_macro = "warn"
disallowed_methods = "warn"
print_stderr = "warn"
print_stdout = "warn"
self_named_module_files = "warn"

-D warnings in CI as a separate job. Note that -D warnings is set just because rust-lang/cargo only warns on a really small set of clippy rules. Generally speaking -D warnings is not recommended as it might break your CI bewteen Rust releases.

- run: cargo clippy --workspace --all-targets --no-deps -- -D warnings

@kristof-mattei
Copy link
Author

kristof-mattei commented Nov 20, 2023

Okay so if I understand correctly, -D warnings or --deny=warnings turns all warnings into deny.

It does not mean that there is a table with lints named warnings that are now denied (like -Dclippy::pedantic)?

@weihanglo
Copy link
Member

Not exactly. For -D warnings it is set warnings lint group as denied.

To set rustc built-in lints in [lints] in Cargo.toml, specify them under [lints.rust] table. See this doc for more. Basically, the syntax is like:

[lints.<tool>]
lint-rule-name = "<level>"

I am going to close this, as it is not about bugs in Cargo. Feel free to open new issues for any doc enhancement idea.

@epage
Copy link
Contributor

epage commented Nov 21, 2023

Generally speaking -D warnings is not recommended as it might break your CI bewteen Rust releases.

Clarification: setting that in CI is the recommended way but you likely want to pin to a specific rust release. I use my MSRV.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

3 participants