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

Merge configs from parent directories #4179

Merged
merged 6 commits into from
May 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions ci/integration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,25 +71,27 @@ function show_head {
echo "Head commit of ${INTEGRATION}: $head"
}

tempdir=$(mktemp -d)

case ${INTEGRATION} in
cargo)
git clone --depth=1 https://github.com/rust-lang/${INTEGRATION}.git
cd ${INTEGRATION}
git clone --depth=1 https://github.com/rust-lang/${INTEGRATION}.git ${tempdir}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a temp dir so we don’t pick up rustfmt’s rustfmt.toml. This shouldn’t matter for this PR bc of the feature flag, but will long term if the PR lands.

cd ${tempdir}
show_head
export CFG_DISABLE_CROSS_TESTS=1
check_fmt_with_all_tests
cd -
;;
crater)
git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git
cd ${INTEGRATION}
git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git ${tempdir}
cd ${tempdir}
show_head
check_fmt_with_lib_tests
cd -
;;
*)
git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git
cd ${INTEGRATION}
git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git ${tempdir}
cd ${tempdir}
show_head
check_fmt_with_all_tests
cd -
Expand Down
21 changes: 14 additions & 7 deletions rustfmt-core/rustfmt-bin/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ fn format_string(input: String, opt: Opt) -> Result<i32> {

enum FileConfig {
Default,
Local(Config, Option<PathBuf>),
Local(Config, Option<Vec<PathBuf>>),
}

struct FileConfigPair<'a> {
Expand Down Expand Up @@ -457,9 +457,9 @@ impl<'a> Iterator for FileConfigPairIter<'a> {
let config = if self.has_config_from_commandline {
FileConfig::Default
} else {
let (local_config, config_path) =
let (local_config, config_paths) =
load_config(Some(file.parent()?), Some(self.opt)).ok()?;
FileConfig::Local(local_config, config_path)
FileConfig::Local(local_config, config_paths)
};

Some(FileConfigPair { file, config })
Expand All @@ -483,11 +483,18 @@ fn format(opt: Opt) -> Result<i32> {
return Err(format_err!("Error: `{}` is a directory", dir.display()));
}

let (default_config, config_path) = load_config(None, Some(&opt))?;
let (default_config, config_paths) = load_config(None, Some(&opt))?;

if opt.verbose {
if let Some(path) = config_path.as_ref() {
println!("Using rustfmt config file {}", path.display());
if let Some(paths) = config_paths.as_ref() {
println!(
"Using rustfmt config file(s) {}",
paths
.into_iter()
.map(|p| p.display().to_string())
.collect::<Vec<_>>()
.join(","),
);
}
}

Expand All @@ -496,7 +503,7 @@ fn format(opt: Opt) -> Result<i32> {
verbosity: opt.verbosity(),
};

let inputs = FileConfigPairIter::new(&opt, config_path.is_some()).collect::<Vec<_>>();
let inputs = FileConfigPairIter::new(&opt, config_paths.is_some()).collect::<Vec<_>>();
let format_report = format_inputs(
inputs.iter().map(|p| {
(
Expand Down
192 changes: 146 additions & 46 deletions rustfmt-core/rustfmt-lib/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,43 @@ impl PartialConfig {

::toml::to_string(&cloned).map_err(ToTomlError)
}

pub fn from_toml_path(file_path: &Path) -> Result<PartialConfig, Error> {
let mut file = File::open(&file_path)?;
let mut toml = String::new();
file.read_to_string(&mut toml)?;
PartialConfig::from_toml(&toml).map_err(|err| Error::new(ErrorKind::InvalidData, err))
}

fn from_toml(toml: &str) -> Result<PartialConfig, String> {
let parsed: ::toml::Value = toml
.parse()
.map_err(|e| format!("Could not parse TOML: {}", e))?;
let mut err = String::new();
let table = parsed
.as_table()
.ok_or_else(|| String::from("Parsed config was not table"))?;
for key in table.keys() {
if !Config::is_valid_name(key) {
let msg = &format!("Warning: Unknown configuration option `{}`\n", key);
err.push_str(msg)
}
}
match parsed.try_into() {
Ok(parsed_config) => {
if !err.is_empty() {
eprint!("{}", err);
}
Ok(parsed_config)
}
Err(e) => {
err.push_str("Error: Decoding config file failed:\n");
err.push_str(format!("{}\n", e).as_str());
err.push_str("Please check your config file.");
Err(err)
}
}
}
}

impl Config {
Expand Down Expand Up @@ -197,11 +234,8 @@ impl Config {
/// Returns a `Config` if the config could be read and parsed from
/// the file, otherwise errors.
pub fn from_toml_path(file_path: &Path) -> Result<Config, Error> {
let mut file = File::open(&file_path)?;
let mut toml = String::new();
file.read_to_string(&mut toml)?;
Config::from_toml(&toml, file_path.parent().unwrap())
.map_err(|err| Error::new(ErrorKind::InvalidData, err))
let partial_config = PartialConfig::from_toml_path(file_path)?;
Ok(Config::default().fill_from_parsed_config(partial_config, file_path.parent().unwrap()))
}

/// Resolves the config for input in `dir`.
Expand All @@ -213,85 +247,77 @@ impl Config {
///
/// Returns the `Config` to use, and the path of the project file if there was
/// one.
pub fn from_resolved_toml_path(dir: &Path) -> Result<(Config, Option<PathBuf>), Error> {
pub fn from_resolved_toml_path(dir: &Path) -> Result<(Config, Option<Vec<PathBuf>>), Error> {
/// Try to find a project file in the given directory and its parents.
/// Returns the path of a the nearest project file if one exists,
/// or `None` if no project file was found.
fn resolve_project_file(dir: &Path) -> Result<Option<PathBuf>, Error> {
fn resolve_project_files(dir: &Path) -> Result<Option<Vec<PathBuf>>, Error> {
let mut current = if dir.is_relative() {
env::current_dir()?.join(dir)
} else {
dir.to_path_buf()
};

current = dunce::canonicalize(current)?;
let mut paths = Vec::new();

loop {
match get_toml_path(&current) {
Ok(Some(path)) => return Ok(Some(path)),
Err(e) => return Err(e),
_ => (),
}
let current_toml_path = get_toml_path(&current)?;
paths.push(current_toml_path);

// If the current directory has no parent, we're done searching.
if !current.pop() {
break;
}
}

// List of closest -> most distant rustfmt config from the current directory.
let config_paths: Option<Vec<_>> = paths.into_iter().filter(|p| p.is_some()).collect();
let has_paths = config_paths
.as_ref()
.map_or(false, |paths| !paths.is_empty());
if has_paths {
return Ok(config_paths);
}

// If nothing was found, check in the home directory.
if let Some(home_dir) = dirs::home_dir() {
if let Some(path) = get_toml_path(&home_dir)? {
return Ok(Some(path));
return Ok(Some(vec![path]));
}
}

// If none was found ther either, check in the user's configuration directory.
if let Some(mut config_dir) = dirs::config_dir() {
config_dir.push("rustfmt");
if let Some(path) = get_toml_path(&config_dir)? {
return Ok(Some(path));
return Ok(Some(vec![path]));
}
}

Ok(None)
}

match resolve_project_file(dir)? {
match resolve_project_files(dir)? {
None => Ok((Config::default(), None)),
Some(path) => Config::from_toml_path(&path).map(|config| (config, Some(path))),
Some(paths) => {
let mut config = Config::default();
let mut used_paths = Vec::with_capacity(paths.len());
for path in paths.into_iter().rev() {
let partial_config = PartialConfig::from_toml_path(&path)?;
config = config.fill_from_parsed_config(partial_config, &path);
used_paths.push(path);
}

Ok((config, Some(used_paths)))
}
}
}

pub fn from_toml(toml: &str, dir: &Path) -> Result<Config, String> {
let parsed: ::toml::Value = toml
.parse()
.map_err(|e| format!("Could not parse TOML: {}", e))?;
let mut err = String::new();
let table = parsed
.as_table()
.ok_or_else(|| String::from("Parsed config was not table"))?;
for key in table.keys() {
if !Config::is_valid_name(key) {
let msg = &format!("Warning: Unknown configuration option `{}`\n", key);
err.push_str(msg)
}
}
match parsed.try_into() {
Ok(parsed_config) => {
if !err.is_empty() {
eprint!("{}", err);
}
let config = Config::default().fill_from_parsed_config(parsed_config, dir);
Ok(config)
}
Err(e) => {
err.push_str("Error: Decoding config file failed:\n");
err.push_str(format!("{}\n", e).as_str());
err.push_str("Please check your config file.");
Err(err)
}
}
let partial_config = PartialConfig::from_toml(toml)?;
let config = Config::default().fill_from_parsed_config(partial_config, dir);
Ok(config)
}
}

Expand All @@ -300,14 +326,14 @@ impl Config {
pub fn load_config<O: CliOptions>(
file_path: Option<&Path>,
options: Option<&O>,
) -> Result<(Config, Option<PathBuf>), Error> {
) -> Result<(Config, Option<Vec<PathBuf>>), Error> {
let over_ride = match options {
Some(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())))
Config::from_toml_path(over_ride.as_ref()).map(|p| (p, Some(vec![over_ride.to_owned()])))
} else if let Some(file_path) = file_path {
Config::from_resolved_toml_path(file_path)
} else {
Expand Down Expand Up @@ -417,6 +443,42 @@ mod test {
}
}

struct TempFile {
path: PathBuf,
}

fn make_temp_file(file_name: &'static str, content: &'static str) -> TempFile {
use std::env::var;

// Used in the Rust build system.
let target_dir = var("RUSTFMT_TEST_DIR").map_or_else(|_| env::temp_dir(), PathBuf::from);
let path = target_dir.join(file_name);

fs::create_dir_all(path.parent().unwrap()).expect("couldn't create temp file");
let mut file = File::create(&path).expect("couldn't create temp file");
file.write_all(content.as_bytes())
.expect("couldn't write temp file");
TempFile { path }
}

impl Drop for TempFile {
fn drop(&mut self) {
use std::fs::remove_file;
remove_file(&self.path).expect("couldn't delete temp file");
}
}

struct NullOptions;

impl CliOptions for NullOptions {
fn apply_to(&self, _: &mut Config) {
unreachable!();
}
fn config_path(&self) -> Option<&Path> {
unreachable!();
}
}

#[test]
fn test_config_set() {
let mut config = Config::default();
Expand Down Expand Up @@ -568,6 +630,44 @@ ignore = []
assert_eq!(&toml, &default_config);
}

#[test]
fn test_merged_config() {
match option_env!("CFG_RELEASE_CHANNEL") {
// this test requires nightly
None | Some("nightly") => {
let _outer_config = make_temp_file(
"a/rustfmt.toml",
r#"
tab_spaces = 2
fn_call_width = 50
ignore = ["b/main.rs", "util.rs"]
"#,
);

let inner_config = make_temp_file(
"a/b/rustfmt.toml",
r#"
version = "two"
tab_spaces = 3
ignore = []
"#,
);

let inner_dir = inner_config.path.parent().unwrap();
let (config, paths) = load_config::<NullOptions>(Some(inner_dir), None).unwrap();

assert_eq!(config.tab_spaces(), 3);
assert_eq!(config.fn_call_width(), 50);
assert_eq!(config.ignore().to_string(), r#"["main.rs"]"#);

let paths = paths.unwrap();
assert!(paths[0].ends_with("a/rustfmt.toml"));
assert!(paths[1].ends_with("a/b/rustfmt.toml"));
}
_ => (),
};
}

mod unstable_features {
use super::super::*;

Expand Down
Loading