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 aborting if we have unsupported configurations #5022

Open
clux opened this issue Oct 11, 2021 · 3 comments · May be fixed by #5025
Open

allow aborting if we have unsupported configurations #5022

clux opened this issue Oct 11, 2021 · 3 comments · May be fixed by #5025
Labels
feature-request good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted

Comments

@clux
Copy link

clux commented Oct 11, 2021

Good evening!
This is a feature request (AFAIKT there's no configuration option or duplicate issue for this).

In many cases where nightly rustfmt is applied by CI in public repositories, users will often run cargo fmt themselves directly or through their editor causing warnings arising from using the stable rustfmt where the nightly is required. E.g.:

$ cargo fmt
Warning: can't set `reorder_impl_items = true`, unstable features are only available in nightly channel.
Warning: can't set `overflow_delimited_expr = true`, unstable features are only available in nightly channel.
Warning: can't set `blank_lines_upper_bound = 2`, unstable features are only available in nightly channel.
Warning: can't set `inline_attribute_width = 80`, unstable features are only available in nightly channel.
Warning: can't set `comment_width = 110`, unstable features are only available in nightly channel.

The problem here is that cargo fmt will succeed regardless (and there is no way to avoid this). Succeeding here is strictly worse than not doing anything because it will reformat the entire file / project with the wrong settings (and it's not always clear to the user that this has happened).

(This is mostly a problem in projects that do not compile with nightly - and lack an override - but uses nightly for convenience tooling like rustfmt).

My thought is that there could maybe be a configuration for abort_on_unrecognised_options causing rustfmt to exit 1 where it normally would emit a warning.

@calebcartwright
Copy link
Member

Love this idea, thanks for the suggestion!

@calebcartwright calebcartwright added feature-request good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted labels Oct 13, 2021
@calebcartwright
Copy link
Member

calebcartwright commented Oct 13, 2021

cc @ytmimi in case you'd have any interest working on this since it's something a bit different (no worries if not though!)

Main portions in the code to start digging into would be

let (config, config_path) = load_config(None, Some(options.clone()))?;

and

rustfmt/src/config/mod.rs

Lines 327 to 350 in f7c4a44

pub fn load_config<O: CliOptions>(
file_path: Option<&Path>,
options: Option<O>,
) -> Result<(Config, Option<PathBuf>), Error> {
let over_ride = match options {
Some(ref opts) => config_path(opts)?,
None => None,
};
let result = if let Some(over_ride) = over_ride {
Config::from_toml_path(over_ride.as_ref()).map(|p| (p, Some(over_ride.to_owned())))
} else if let Some(file_path) = file_path {
Config::from_resolved_toml_path(file_path)
} else {
Ok((Config::default(), None))
};
result.map(|(mut c, p)| {
if let Some(options) = options {
options.apply_to(&mut c);
}
(c, p)
})
}

Thought some minor tweaks may be needed to track whether the config attempted to use a nightly option outside nightly

@ytmimi
Copy link
Contributor

ytmimi commented Oct 13, 2021

Thanks for thinking of me!

Also, I appreciate you pointing out a place to start. I'm eager to take a crack at it, and I'll let you know if I run into any problems or have any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants