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

Allow specifying both project-wide and package-specific clippy.toml settings #7353

Open
jplatte opened this issue Jun 15, 2021 · 8 comments
Open
Assignees
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. E-medium Call for participation: Medium difficulty level problem and requires some initial experience.

Comments

@jplatte
Copy link
Contributor

jplatte commented Jun 15, 2021

For Ruma, I would like to have both project-wide and package-specific .clippy.toml settings:

  • msrv (and import_rename once it's available) as a project-wide setting
  • Possibly a project-specific msrv override
  • disallowed_type project-specific setting to disallow HashMap / HashSet use within proc-macros

It seems like this is not currently possible, with the first .clippy.toml found in one of the parent directories of the package being checked being used for all settings. Maybe an alternative to the current filesystem hierarchy based config lookup¹ would be to only search the package dir² and the workspace root³ for clippy configuration and make any settings in the former overwrite settings from the latter?

¹ which AFAIK has proved problematic for Cargo, with some people wanting to move certain settings to Cargo.toml for better reproducability
² CARGO_MANIFEST_PATH
³ workspace_root in the cargo metadata --format-version 1 output

@flip1995 flip1995 added C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. E-medium Call for participation: Medium difficulty level problem and requires some initial experience. labels Jun 15, 2021
@eholk
Copy link
Contributor

eholk commented Jun 22, 2021

Just to make sure I understand the algorithm you're proposing, let me try to restate it. Let's say you have a directory tree like this:

workspace/
    .clippy.toml
    project1/
        .clippy.toml
    project2/
        .clippy.toml

The idea is that when running clippy on project1, you'd first read workspace/.clippy.toml, and then read workspace/project1/.clippy.toml, with settings from project1/.clippy.toml overriding those in workspace/.clippy.toml when they both specify the same settings. Is this right?

This seems pretty reasonable, and in line with what a lot of other tools do.

For the sake of argument, what if instead we made this explicit by adding something like inherit = "../.clippy.toml" to project1/.clippy.toml and didn't do the implicit path search? Would the explicit approach be preferable or more flexible?

@jplatte
Copy link
Contributor Author

jplatte commented Jun 22, 2021

@eholk That's exactly what I'm proposing, yes.

Sure, inherit would be more flexible – you could inherit settings even outside of workspaces, have multiple "base" configs if you want the same settings across a few but not all projects in a workspace and even define larger hierarchies of config files.

I suppose in Ruma this could be used to have a single .macros.clippy.toml that's used for all the macro crates. That would work even without explicit inheritance though, by symlinking that file into the macro crate dirs. So I think I would still prefer my original proposal rather than the customizable config inheritance. At least for me personally, it seemed like an obvious thing to try, whereas config inheritance would have to be prominent enough in the docs.

@eholk
Copy link
Contributor

eholk commented Jun 22, 2021

Thanks! I imagine the two approaches could work together if there was really a need for both, but it's probably better to get the simplest thing that works first.

rust-lang/cargo#5034 is probably relevant here as well.

@eholk
Copy link
Contributor

eholk commented Jun 22, 2021

@severinrudie
Copy link

@rustbot claim

@severinrudie
Copy link

@rustbot release-assignment

😢 Sorry all

@Centri3
Copy link
Member

Centri3 commented Jun 11, 2023

I'd like to take a crack at this, @rustbot claim

@Centri3
Copy link
Member

Centri3 commented Jun 11, 2023

It's worth noting you can kind of do this already (I think) using .cargo/config.toml by setting CLIPPY_CONF_DIR. Not ideal though as 1, you'll need to add it to a new directory, 2, get a ton of warnings if you have a global clippy.toml, and 3, can't inherit. So this will still be very useful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. E-medium Call for participation: Medium difficulty level problem and requires some initial experience.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants