Skip to content

Commit

Permalink
Inline format arguments for easier reading (rust-lang#5881)
Browse files Browse the repository at this point in the history
* Inline format arguments for easier reading

Code becomes shorter and often easier to read when format args are inlined.  Note that I skipped the mixed cases to make it more straightforward (could be done separatelly).

Also, there are two FIXME comments - for some reasons inlining makes format string exceed 100 char line width and crash.

```
cargo clippy --workspace --allow-dirty --fix --benches --tests --bins -- -A clippy::all -W clippy::uninlined_format_args
```

* address feedback
  • Loading branch information
nyurik authored Aug 13, 2023
1 parent e86c2ba commit b069aac
Show file tree
Hide file tree
Showing 38 changed files with 183 additions and 223 deletions.
8 changes: 4 additions & 4 deletions src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ impl Rewrite for ast::MetaItem {
// See #2479 for example.
let value = rewrite_literal(context, lit.as_token_lit(), lit.span, lit_shape)
.unwrap_or_else(|| context.snippet(lit.span).to_owned());
format!("{} = {}", path, value)
format!("{path} = {value}")
}
})
}
Expand Down Expand Up @@ -342,7 +342,7 @@ impl Rewrite for ast::Attribute {
let literal_str = literal.as_str();
let doc_comment_formatter =
DocCommentFormatter::new(literal_str, comment_style);
let doc_comment = format!("{}", doc_comment_formatter);
let doc_comment = format!("{doc_comment_formatter}");
return rewrite_doc_comment(
&doc_comment,
shape.comment(context.config),
Expand Down Expand Up @@ -406,9 +406,9 @@ impl Rewrite for [ast::Attribute] {
0,
)?;
let comment = if comment.is_empty() {
format!("\n{}", mlb)
format!("\n{mlb}")
} else {
format!("{}{}\n{}", mla, comment, mlb)
format!("{mla}{comment}\n{mlb}")
};
result.push_str(&comment);
result.push_str(&shape.indent.to_string(context.config));
Expand Down
6 changes: 3 additions & 3 deletions src/attr/doc_comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ impl Display for DocCommentFormatter<'_> {

// Handle `#[doc = ""]`.
if lines.peek().is_none() {
return write!(formatter, "{}", opener);
return write!(formatter, "{opener}");
}

while let Some(line) = lines.next() {
let is_last_line = lines.peek().is_none();
if is_last_line {
write!(formatter, "{}{}", opener, line)?;
write!(formatter, "{opener}{line}")?;
} else {
writeln!(formatter, "{}{}", opener, line)?;
writeln!(formatter, "{opener}{line}")?;
}
}
Ok(())
Expand Down
19 changes: 8 additions & 11 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn main() {
let exit_code = match execute(&opts) {
Ok(code) => code,
Err(e) => {
eprintln!("{:#}", e);
eprintln!("{e:#}");
1
}
};
Expand Down Expand Up @@ -284,7 +284,7 @@ fn format_string(input: String, options: GetOptsOptions) -> Result<i32> {
for f in config.file_lines().files() {
match *f {
FileName::Stdin => {}
_ => eprintln!("Warning: Extra file listed in file_lines option '{}'", f),
_ => eprintln!("Warning: Extra file listed in file_lines option '{f}'"),
}
}

Expand Down Expand Up @@ -380,7 +380,7 @@ fn format_and_emit_report<T: Write>(session: &mut Session<'_, T>, input: Input)
}
}
Err(msg) => {
eprintln!("Error writing files: {}", msg);
eprintln!("Error writing files: {msg}");
session.add_operational_error();
}
}
Expand All @@ -403,12 +403,9 @@ fn print_usage_to_stdout(opts: &Options, reason: &str) {
let sep = if reason.is_empty() {
String::new()
} else {
format!("{}\n\n", reason)
format!("{reason}\n\n")
};
let msg = format!(
"{}Format Rust code\n\nusage: rustfmt [options] <file>...",
sep
);
let msg = format!("{sep}Format Rust code\n\nusage: rustfmt [options] <file>...");
println!("{}", opts.usage(&msg));
}

Expand Down Expand Up @@ -442,7 +439,7 @@ fn print_version() {
include_str!(concat!(env!("OUT_DIR"), "/commit-info.txt"))
);

println!("rustfmt {}", version_info);
println!("rustfmt {version_info}");
}

fn determine_operation(matches: &Matches) -> Result<Operation, OperationError> {
Expand Down Expand Up @@ -647,9 +644,9 @@ impl GetOptsOptions {
match *f {
FileName::Real(ref f) if files.contains(f) => {}
FileName::Real(_) => {
eprintln!("Warning: Extra file listed in file_lines option '{}'", f)
eprintln!("Warning: Extra file listed in file_lines option '{f}'")
}
FileName::Stdin => eprintln!("Warning: Not a file '{}'", f),
FileName::Stdin => eprintln!("Warning: Not a file '{f}'"),
}
}
}
Expand Down
9 changes: 4 additions & 5 deletions src/cargo-fmt/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,13 @@ fn convert_message_format_to_rustfmt_args(
}
"human" => Ok(()),
_ => Err(format!(
"invalid --message-format value: {}. Allowed values are: short|json|human",
message_format
"invalid --message-format value: {message_format}. Allowed values are: short|json|human"
)),
}
}

fn print_usage_to_stderr(reason: &str) {
eprintln!("{}", reason);
eprintln!("{reason}");
let app = Opts::command();
app.after_help("")
.write_help(&mut io::stderr())
Expand Down Expand Up @@ -460,7 +459,7 @@ fn get_targets_with_hitlist(
let package = workspace_hitlist.iter().next().unwrap();
Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("package `{}` is not a member of the workspace", package),
format!("package `{package}` is not a member of the workspace"),
))
}
}
Expand Down Expand Up @@ -498,7 +497,7 @@ fn run_rustfmt(

if verbosity == Verbosity::Verbose {
print!("rustfmt");
print!(" --edition {}", edition);
print!(" --edition {edition}");
fmt_args.iter().for_each(|f| print!(" {}", f));
files.iter().for_each(|f| print!(" {}", f.display()));
println!();
Expand Down
2 changes: 1 addition & 1 deletion src/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl Rewrite for ChainItem {
rewrite_comment(comment, false, shape, context.config)?
}
};
Some(format!("{}{}", rewrite, "?".repeat(self.tries)))
Some(format!("{rewrite}{}", "?".repeat(self.tries)))
}
}

Expand Down
7 changes: 2 additions & 5 deletions src/closures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ fn rewrite_closure_with_block(
shape,
false,
)?;
Some(format!("{} {}", prefix, block))
Some(format!("{prefix} {block}"))
}

// Rewrite closure with a single expression without wrapping its body with block.
Expand Down Expand Up @@ -310,10 +310,7 @@ fn rewrite_closure_fn_decl(
.tactic(tactic)
.preserve_newline(true);
let list_str = write_list(&item_vec, &fmt)?;
let mut prefix = format!(
"{}{}{}{}{}|{}|",
binder, const_, immovable, is_async, mover, list_str
);
let mut prefix = format!("{binder}{const_}{immovable}{is_async}{mover}|{list_str}|");

if !ret_str.is_empty() {
if prefix.contains('\n') {
Expand Down
35 changes: 10 additions & 25 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ impl<'a> CommentRewrite<'a> {
is_prev_line_multi_line: false,
code_block_attr: None,
item_block: None,
comment_line_separator: format!("{}{}", indent_str, line_start),
comment_line_separator: format!("{indent_str}{line_start}"),
max_width,
indent_str,
fmt_indent: shape.indent,
Expand Down Expand Up @@ -951,7 +951,7 @@ const RUSTFMT_CUSTOM_COMMENT_PREFIX: &str = "//#### ";
fn hide_sharp_behind_comment(s: &str) -> Cow<'_, str> {
let s_trimmed = s.trim();
if s_trimmed.starts_with("# ") || s_trimmed == "#" {
Cow::from(format!("{}{}", RUSTFMT_CUSTOM_COMMENT_PREFIX, s))
Cow::from(format!("{RUSTFMT_CUSTOM_COMMENT_PREFIX}{s}"))
} else {
Cow::from(s)
}
Expand Down Expand Up @@ -1035,7 +1035,7 @@ pub(crate) fn recover_missing_comment_in_span(
} else {
Cow::from(" ")
};
Some(format!("{}{}", sep, missing_comment))
Some(format!("{sep}{missing_comment}"))
}
}

Expand Down Expand Up @@ -1832,8 +1832,7 @@ fn remove_comment_header(comment: &str) -> &str {
} else {
assert!(
comment.starts_with("/*"),
"string '{}' is not a comment",
comment
"string '{comment}' is not a comment"
);
&comment[2..comment.len() - 2]
}
Expand Down Expand Up @@ -2069,26 +2068,13 @@ fn main() {
expected_line_start: &str,
) {
let block = ItemizedBlock::new(test_input).unwrap();
assert_eq!(1, block.lines.len(), "test_input: {:?}", test_input);
assert_eq!(
expected_line, &block.lines[0],
"test_input: {:?}",
test_input
);
assert_eq!(
expected_indent, block.indent,
"test_input: {:?}",
test_input
);
assert_eq!(
expected_opener, &block.opener,
"test_input: {:?}",
test_input
);
assert_eq!(1, block.lines.len(), "test_input: {test_input:?}");
assert_eq!(expected_line, &block.lines[0], "test_input: {test_input:?}");
assert_eq!(expected_indent, block.indent, "test_input: {test_input:?}");
assert_eq!(expected_opener, &block.opener, "test_input: {test_input:?}");
assert_eq!(
expected_line_start, &block.line_start,
"test_input: {:?}",
test_input
"test_input: {test_input:?}"
);
}

Expand Down Expand Up @@ -2145,8 +2131,7 @@ fn main() {
let maybe_block = ItemizedBlock::new(line);
assert!(
maybe_block.is_none(),
"The following line shouldn't be classified as a list item: {}",
line
"The following line shouldn't be classified as a list item: {line}"
);
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/config/config_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,18 +500,16 @@ where
// Stable with an unstable option
(false, false, _) => {
eprintln!(
"Warning: can't set `{} = {:?}`, unstable features are only \
available in nightly channel.",
option_name, option_value
"Warning: can't set `{option_name} = {option_value:?}`, unstable features are only \
available in nightly channel."
);
false
}
// Stable with a stable option, but an unstable variant
(false, true, false) => {
eprintln!(
"Warning: can't set `{} = {:?}`, unstable variants are only \
available in nightly channel.",
option_name, option_value
"Warning: can't set `{option_name} = {option_value:?}`, unstable variants are only \
available in nightly channel."
);
false
}
Expand Down
2 changes: 1 addition & 1 deletion src/config/file_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl fmt::Display for FileLines {
None => write!(f, "None")?,
Some(map) => {
for (file_name, ranges) in map.iter() {
write!(f, "{}: ", file_name)?;
write!(f, "{file_name}: ")?;
write!(f, "{}\n", ranges.iter().format(", "))?;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/config/macro_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,6 @@ mod test {
#[test]
fn macro_names_display() {
let macro_names = MacroSelectors::from_str(r#"["foo", "*", "bar"]"#).unwrap();
assert_eq!(format!("{}", macro_names), "foo, *, bar");
assert_eq!(format!("{macro_names}"), "foo, *, bar");
}
}
15 changes: 6 additions & 9 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ impl Config {
let required_version = self.required_version();
if version != required_version {
println!(
"Error: rustfmt version ({}) doesn't match the required version ({})",
version, required_version,
"Error: rustfmt version ({version}) doesn't match the required version \
({required_version})"
);
return false;
}
Expand Down Expand Up @@ -310,20 +310,20 @@ impl Config {
.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);
let msg = &format!("Warning: Unknown configuration option `{key}`\n");
err.push_str(msg)
}
}
match parsed.try_into() {
Ok(parsed_config) => {
if !err.is_empty() {
eprint!("{}", err);
eprint!("{err}");
}
Ok(Config::default().fill_from_parsed_config(parsed_config, dir))
}
Err(e) => {
err.push_str("Error: Decoding config file failed:\n");
err.push_str(format!("{}\n", e).as_str());
err.push_str(format!("{e}\n").as_str());
err.push_str("Please check your config file.");
Err(err)
}
Expand Down Expand Up @@ -563,10 +563,7 @@ mod test {
let toml = used_options.to_toml().unwrap();
assert_eq!(
toml,
format!(
"merge_derives = {}\nskip_children = {}\n",
merge_derives, skip_children,
)
format!("merge_derives = {merge_derives}\nskip_children = {skip_children}\n",)
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/config/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ pub struct WidthHeuristics {

impl fmt::Display for WidthHeuristics {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}", self)
write!(f, "{self:?}")
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ pub(crate) trait Emitter {
fn ensure_real_path(filename: &FileName) -> &Path {
match *filename {
FileName::Real(ref path) => path,
_ => panic!("cannot format `{}` and emit to files", filename),
_ => panic!("cannot format `{filename}` and emit to files"),
}
}
4 changes: 2 additions & 2 deletions src/emitter/checkstyle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub(crate) fn output_checkstyle_file<T>(
where
T: Write,
{
write!(writer, r#"<file name="{}">"#, filename)?;
write!(writer, r#"<file name="{filename}">"#)?;
for mismatch in diff {
let begin_line = mismatch.line_number;
let mut current_line;
Expand Down Expand Up @@ -82,7 +82,7 @@ mod tests {
);
assert_eq!(
&writer[..],
format!(r#"<file name="{}"></file>"#, file_name).as_bytes()
format!(r#"<file name="{file_name}"></file>"#).as_bytes()
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/emitter/checkstyle/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ impl<'a> Display for XmlEscaped<'a> {
'"' => write!(formatter, "&quot;"),
'\'' => write!(formatter, "&apos;"),
'&' => write!(formatter, "&amp;"),
_ => write!(formatter, "{}", char),
_ => write!(formatter, "{char}"),
}?;
}

Expand Down
Loading

0 comments on commit b069aac

Please sign in to comment.