Skip to content

fix print-config minimal option #3687

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

Merged
merged 2 commits into from
Jul 15, 2019
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
109 changes: 64 additions & 45 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use env_logger;
#[macro_use]
extern crate failure;
use failure::{err_msg, format_err, Error as FailureError, Fail};
use io::Error as IoError;

use rustfmt_nightly as rustfmt;

Expand All @@ -10,12 +10,10 @@ use std::io::{self, stdout, Read, Write};
use std::path::{Path, PathBuf};
use std::str::FromStr;

use failure::err_msg;

use getopts::{Matches, Options};

use crate::rustfmt::{
load_config, CliOptions, Color, Config, Edition, EmitMode, ErrorKind, FileLines, FileName,
load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName,
FormatReportFormatterBuilder, Input, Session, Verbosity,
};

Expand Down Expand Up @@ -49,20 +47,37 @@ enum Operation {
},
/// Print the help message.
Help(HelpOp),
// Print version information
/// Print version information
Version,
/// Output default config to a file, or stdout if None
ConfigOutputDefault {
path: Option<String>,
},
ConfigOutputDefault { path: Option<String> },
/// Output current config (as if formatting to a file) to stdout
ConfigOutputCurrent {
path: Option<String>,
},
ConfigOutputCurrent { path: Option<String> },
/// No file specified, read from stdin
Stdin {
input: String,
},
Stdin { input: String },
}

/// Rustfmt operations errors.
#[derive(Fail, Debug)]
pub enum OperationError {
/// An unknown help topic was requested.
#[fail(display = "Unknown help topic: `{}`.", _0)]
UnknownHelpTopic(String),
/// An unknown print-config option was requested.
#[fail(display = "Unknown print-config option: `{}`.", _0)]
UnknownPrintConfigTopic(String),
/// Attempt to generate a minimal config from standard input.
#[fail(display = "The `--print-config=minimal` option doesn't work with standard input.")]
MinimalPathWithStdin,
/// An io error during reading or writing.
#[fail(display = "io error: {}", _0)]
IoError(IoError),
}

impl From<IoError> for OperationError {
fn from(e: IoError) -> OperationError {
OperationError::IoError(e)
}
}

/// Arguments to `--help`
Expand Down Expand Up @@ -156,7 +171,7 @@ fn is_nightly() -> bool {
}

// Returned i32 is an exit code
fn execute(opts: &Options) -> Result<i32, failure::Error> {
fn execute(opts: &Options) -> Result<i32, FailureError> {
let matches = opts.parse(env::args().skip(1))?;
let options = GetOptsOptions::from_matches(&matches)?;

Expand Down Expand Up @@ -210,7 +225,7 @@ fn execute(opts: &Options) -> Result<i32, failure::Error> {
}
}

fn format_string(input: String, options: GetOptsOptions) -> Result<i32, failure::Error> {
fn format_string(input: String, options: GetOptsOptions) -> Result<i32, FailureError> {
// try to read config from local directory
let (mut config, _) = load_config(Some(Path::new(".")), Some(options.clone()))?;

Expand Down Expand Up @@ -243,7 +258,7 @@ fn format(
files: Vec<PathBuf>,
minimal_config_path: Option<String>,
options: &GetOptsOptions,
) -> Result<i32, failure::Error> {
) -> Result<i32, FailureError> {
options.verify_file_lines(&files);
let (config, config_path) = load_config(None, Some(options.clone()))?;

Expand Down Expand Up @@ -385,7 +400,7 @@ fn print_version() {
println!("rustfmt {}", version_info);
}

fn determine_operation(matches: &Matches) -> Result<Operation, ErrorKind> {
fn determine_operation(matches: &Matches) -> Result<Operation, OperationError> {
if matches.opt_present("h") {
let topic = matches.opt_str("h");
if topic == None {
Expand All @@ -395,22 +410,25 @@ fn determine_operation(matches: &Matches) -> Result<Operation, ErrorKind> {
} else if topic == Some("file-lines".to_owned()) {
return Ok(Operation::Help(HelpOp::FileLines));
} else {
println!("Unknown help topic: `{}`\n", topic.unwrap());
return Ok(Operation::Help(HelpOp::None));
return Err(OperationError::UnknownHelpTopic(topic.unwrap()));
}
}
let mut free_matches = matches.free.iter();

let mut minimal_config_path = None;
if let Some(ref kind) = matches.opt_str("print-config") {
let path = matches.free.get(0).cloned();
if kind == "default" {
return Ok(Operation::ConfigOutputDefault { path });
} else if kind == "current" {
return Ok(Operation::ConfigOutputCurrent { path });
} else if kind == "minimal" {
minimal_config_path = path;
if minimal_config_path.is_none() {
println!("WARNING: PATH required for `--print-config minimal`");
if let Some(kind) = matches.opt_str("print-config") {
let path = free_matches.next().cloned();
match kind.as_str() {
"default" => return Ok(Operation::ConfigOutputDefault { path }),
"current" => return Ok(Operation::ConfigOutputCurrent { path }),
"minimal" => {
minimal_config_path = path;
if minimal_config_path.is_none() {
eprintln!("WARNING: PATH required for `--print-config minimal`.");
}
}
_ => {
return Err(OperationError::UnknownPrintConfigTopic(kind));
}
}
}
Expand All @@ -419,17 +437,7 @@ fn determine_operation(matches: &Matches) -> Result<Operation, ErrorKind> {
return Ok(Operation::Version);
}

// if no file argument is supplied, read from stdin
if matches.free.is_empty() {
let mut buffer = String::new();
io::stdin().read_to_string(&mut buffer)?;

return Ok(Operation::Stdin { input: buffer });
}

let files: Vec<_> = matches
.free
.iter()
let files: Vec<_> = free_matches
.map(|s| {
let p = PathBuf::from(s);
// we will do comparison later, so here tries to canonicalize first
Expand All @@ -438,6 +446,17 @@ fn determine_operation(matches: &Matches) -> Result<Operation, ErrorKind> {
})
.collect();

// if no file argument is supplied, read from stdin
if files.is_empty() {
if minimal_config_path.is_some() {
return Err(OperationError::MinimalPathWithStdin);
}
let mut buffer = String::new();
io::stdin().read_to_string(&mut buffer)?;

return Ok(Operation::Stdin { input: buffer });
}

Ok(Operation::Format {
files,
minimal_config_path,
Expand All @@ -464,7 +483,7 @@ struct GetOptsOptions {
}

impl GetOptsOptions {
pub fn from_matches(matches: &Matches) -> Result<GetOptsOptions, failure::Error> {
pub fn from_matches(matches: &Matches) -> Result<GetOptsOptions, FailureError> {
let mut options = GetOptsOptions::default();
options.verbose = matches.opt_present("verbose");
options.quiet = matches.opt_present("quiet");
Expand Down Expand Up @@ -598,15 +617,15 @@ impl CliOptions for GetOptsOptions {
}
}

fn edition_from_edition_str(edition_str: &str) -> Result<Edition, failure::Error> {
fn edition_from_edition_str(edition_str: &str) -> Result<Edition, FailureError> {
match edition_str {
"2015" => Ok(Edition::Edition2015),
"2018" => Ok(Edition::Edition2018),
_ => Err(format_err!("Invalid value for `--edition`")),
}
}

fn emit_mode_from_emit_str(emit_str: &str) -> Result<EmitMode, failure::Error> {
fn emit_mode_from_emit_str(emit_str: &str) -> Result<EmitMode, FailureError> {
match emit_str {
"files" => Ok(EmitMode::Files),
"stdout" => Ok(EmitMode::Stdout),
Expand Down
77 changes: 77 additions & 0 deletions tests/rustfmt/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//! Integration tests for rustfmt.
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome 👍


use std::env;
use std::fs::remove_file;
use std::path::Path;
use std::process::Command;

/// Run the rustfmt executable and return its output.
fn rustfmt(args: &[&str]) -> (String, String) {
let mut bin_dir = env::current_exe().unwrap();
bin_dir.pop(); // chop off test exe name
if bin_dir.ends_with("deps") {
bin_dir.pop();
}
let cmd = bin_dir.join(format!("rustfmt{}", env::consts::EXE_SUFFIX));

// Ensure the rustfmt binary runs from the local target dir.
let path = env::var_os("PATH").unwrap_or_default();
let mut paths = env::split_paths(&path).collect::<Vec<_>>();
paths.insert(0, bin_dir);
let new_path = env::join_paths(paths).unwrap();

match Command::new(&cmd).args(args).env("PATH", new_path).output() {
Ok(output) => (
String::from_utf8(output.stdout).expect("utf-8"),
String::from_utf8(output.stderr).expect("utf-8"),
),
Err(e) => panic!("failed to run `{:?} {:?}`: {}", cmd, args, e),
}
}

macro_rules! assert_that {
($args:expr, $check:ident $check_args:tt) => {
let (stdout, stderr) = rustfmt($args);
if !stdout.$check$check_args && !stderr.$check$check_args {
panic!(
"Output not expected for rustfmt {:?}\n\
expected: {}{}\n\
actual stdout:\n{}\n\
actual stderr:\n{}",
$args,
stringify!($check),
stringify!($check_args),
stdout,
stderr
);
}
};
}

#[test]
fn print_config() {
assert_that!(
&["--print-config", "unknown"],
starts_with("Unknown print-config option")
);
assert_that!(&["--print-config", "default"], contains("max_width = 100"));
assert_that!(&["--print-config", "minimal"], contains("PATH required"));
assert_that!(
&["--print-config", "minimal", "minimal-config"],
contains("doesn't work with standard input.")
);

let (stdout, stderr) = rustfmt(&[
"--print-config",
"minimal",
"minimal-config",
"src/shape.rs",
]);
assert!(
Path::new("minimal-config").exists(),
"stdout:\n{}\nstderr:\n{}",
stdout,
stderr
);
remove_file("minimal-config").unwrap();
}