Skip to content

Commit c74faee

Browse files
hamirmahalalamb
hamirmahal
and
alamb
authored
style: simplify some strings for readability (#15999)
* style: simplify some strings for readability * fix: formatting in `datafusion/` directory * refactor: replace long `format!` string * refactor: replace `format!` with `assert_eq!` --------- Co-authored-by: Andrew Lamb <[email protected]>
1 parent 2f67a7e commit c74faee

File tree

13 files changed

+46
-57
lines changed

13 files changed

+46
-57
lines changed

‎datafusion-cli/src/catalog.rs

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,7 @@ mod tests {
337337
#[cfg(not(target_os = "windows"))]
338338
#[test]
339339
fn test_substitute_tilde() {
340-
use std::env;
341-
use std::path::MAIN_SEPARATOR;
340+
use std::{env, path::PathBuf};
342341
let original_home = home_dir();
343342
let test_home_path = if cfg!(windows) {
344343
"C:\\Users\\user"
@@ -350,17 +349,16 @@ mod tests {
350349
test_home_path,
351350
);
352351
let input = "~/Code/datafusion/benchmarks/data/tpch_sf1/part/part-0.parquet";
353-
let expected = format!(
354-
"{}{}Code{}datafusion{}benchmarks{}data{}tpch_sf1{}part{}part-0.parquet",
355-
test_home_path,
356-
MAIN_SEPARATOR,
357-
MAIN_SEPARATOR,
358-
MAIN_SEPARATOR,
359-
MAIN_SEPARATOR,
360-
MAIN_SEPARATOR,
361-
MAIN_SEPARATOR,
362-
MAIN_SEPARATOR
363-
);
352+
let expected = PathBuf::from(test_home_path)
353+
.join("Code")
354+
.join("datafusion")
355+
.join("benchmarks")
356+
.join("data")
357+
.join("tpch_sf1")
358+
.join("part")
359+
.join("part-0.parquet")
360+
.to_string_lossy()
361+
.to_string();
364362
let actual = substitute_tilde(input.to_string());
365363
assert_eq!(actual, expected);
366364
match original_home {

‎datafusion-cli/src/command.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,14 @@ impl Command {
7878
exec_and_print(ctx, print_options, "SHOW TABLES".into()).await
7979
}
8080
Self::DescribeTableStmt(name) => {
81-
exec_and_print(ctx, print_options, format!("SHOW COLUMNS FROM {}", name))
81+
exec_and_print(ctx, print_options, format!("SHOW COLUMNS FROM {name}"))
8282
.await
8383
}
8484
Self::Include(filename) => {
8585
if let Some(filename) = filename {
8686
let file = File::open(filename).map_err(|e| {
8787
DataFusionError::Execution(format!(
88-
"Error opening {:?} {}",
89-
filename, e
88+
"Error opening {filename:?} {e}"
9089
))
9190
})?;
9291
exec_from_lines(ctx, &mut BufReader::new(file), print_options)
@@ -116,7 +115,7 @@ impl Command {
116115
Self::SearchFunctions(function) => {
117116
if let Ok(func) = function.parse::<Function>() {
118117
let details = func.function_details()?;
119-
println!("{}", details);
118+
println!("{details}");
120119
Ok(())
121120
} else {
122121
exec_err!("{function} is not a supported function")

‎datafusion-cli/src/exec.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ pub async fn exec_from_repl(
200200
break;
201201
}
202202
Err(err) => {
203-
eprintln!("Unknown error happened {:?}", err);
203+
eprintln!("Unknown error happened {err:?}");
204204
break;
205205
}
206206
}
@@ -530,7 +530,7 @@ mod tests {
530530
)
531531
})?;
532532
for location in locations {
533-
let sql = format!("copy (values (1,2)) to '{}' STORED AS PARQUET;", location);
533+
let sql = format!("copy (values (1,2)) to '{location}' STORED AS PARQUET;");
534534
let statements = DFParser::parse_sql_with_dialect(&sql, dialect.as_ref())?;
535535
for statement in statements {
536536
//Should not fail

‎datafusion-cli/src/functions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ pub fn display_all_functions() -> Result<()> {
205205
let array = StringArray::from(
206206
ALL_FUNCTIONS
207207
.iter()
208-
.map(|f| format!("{}", f))
208+
.map(|f| format!("{f}"))
209209
.collect::<Vec<String>>(),
210210
);
211211
let schema = Schema::new(vec![Field::new("Function", DataType::Utf8, false)]);

‎datafusion-cli/src/main.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ async fn main_inner() -> Result<()> {
154154
let args = Args::parse();
155155

156156
if !args.quiet {
157-
println!("DataFusion CLI v{}", DATAFUSION_CLI_VERSION);
157+
println!("DataFusion CLI v{DATAFUSION_CLI_VERSION}");
158158
}
159159

160160
if let Some(ref path) = args.data_path {
@@ -280,22 +280,22 @@ fn parse_valid_file(dir: &str) -> Result<String, String> {
280280
if Path::new(dir).is_file() {
281281
Ok(dir.to_string())
282282
} else {
283-
Err(format!("Invalid file '{}'", dir))
283+
Err(format!("Invalid file '{dir}'"))
284284
}
285285
}
286286

287287
fn parse_valid_data_dir(dir: &str) -> Result<String, String> {
288288
if Path::new(dir).is_dir() {
289289
Ok(dir.to_string())
290290
} else {
291-
Err(format!("Invalid data directory '{}'", dir))
291+
Err(format!("Invalid data directory '{dir}'"))
292292
}
293293
}
294294

295295
fn parse_batch_size(size: &str) -> Result<usize, String> {
296296
match size.parse::<usize>() {
297297
Ok(size) if size > 0 => Ok(size),
298-
_ => Err(format!("Invalid batch size '{}'", size)),
298+
_ => Err(format!("Invalid batch size '{size}'")),
299299
}
300300
}
301301

@@ -352,20 +352,20 @@ fn parse_size_string(size: &str, label: &str) -> Result<usize, String> {
352352
let num_str = caps.get(1).unwrap().as_str();
353353
let num = num_str
354354
.parse::<usize>()
355-
.map_err(|_| format!("Invalid numeric value in {} '{}'", label, size))?;
355+
.map_err(|_| format!("Invalid numeric value in {label} '{size}'"))?;
356356

357357
let suffix = caps.get(2).map(|m| m.as_str()).unwrap_or("b");
358358
let unit = BYTE_SUFFIXES
359359
.get(suffix)
360-
.ok_or_else(|| format!("Invalid {} '{}'", label, size))?;
360+
.ok_or_else(|| format!("Invalid {label} '{size}'"))?;
361361
let total_bytes = usize::try_from(unit.multiplier())
362362
.ok()
363363
.and_then(|multiplier| num.checked_mul(multiplier))
364-
.ok_or_else(|| format!("{} '{}' is too large", label, size))?;
364+
.ok_or_else(|| format!("{label} '{size}' is too large"))?;
365365

366366
Ok(total_bytes)
367367
} else {
368-
Err(format!("Invalid {} '{}'", label, size))
368+
Err(format!("Invalid {label} '{size}'"))
369369
}
370370
}
371371

‎datafusion-cli/src/pool_type.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ impl FromStr for PoolType {
3333
match s {
3434
"Greedy" | "greedy" => Ok(PoolType::Greedy),
3535
"Fair" | "fair" => Ok(PoolType::Fair),
36-
_ => Err(format!("Invalid memory pool type '{}'", s)),
36+
_ => Err(format!("Invalid memory pool type '{s}'")),
3737
}
3838
}
3939
}

‎datafusion-cli/src/print_format.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,16 +137,16 @@ fn format_batches_with_maxrows<W: std::io::Write>(
137137
let formatted =
138138
pretty_format_batches_with_options(&filtered_batches, &options)?;
139139
if over_limit {
140-
let mut formatted_str = format!("{}", formatted);
140+
let mut formatted_str = format!("{formatted}");
141141
formatted_str = keep_only_maxrows(&formatted_str, maxrows);
142-
writeln!(writer, "{}", formatted_str)?;
142+
writeln!(writer, "{formatted_str}")?;
143143
} else {
144-
writeln!(writer, "{}", formatted)?;
144+
writeln!(writer, "{formatted}")?;
145145
}
146146
}
147147
MaxRows::Unlimited => {
148148
let formatted = pretty_format_batches_with_options(batches, &options)?;
149-
writeln!(writer, "{}", formatted)?;
149+
writeln!(writer, "{formatted}")?;
150150
}
151151
}
152152

@@ -206,7 +206,7 @@ impl PrintFormat {
206206
let empty_batch = RecordBatch::new_empty(schema);
207207
let formatted =
208208
pretty_format_batches_with_options(&[empty_batch], &format_options)?;
209-
writeln!(writer, "{}", formatted)?;
209+
writeln!(writer, "{formatted}")?;
210210
}
211211
_ => {}
212212
}

‎datafusion-cli/src/print_options.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ impl FromStr for MaxRows {
5252
} else {
5353
match maxrows.parse::<usize>() {
5454
Ok(nrows) => Ok(Self::Limited(nrows)),
55-
_ => Err(format!("Invalid maxrows {}. Valid inputs are natural numbers or \'none\', \'inf\', or \'infinite\' for no limit.", maxrows)),
55+
_ => Err(format!("Invalid maxrows {maxrows}. Valid inputs are natural numbers or \'none\', \'inf\', or \'infinite\' for no limit.")),
5656
}
5757
}
5858
}

‎datafusion-cli/tests/cli_integration.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,14 @@ async fn test_aws_options() {
161161
STORED AS CSV
162162
LOCATION 's3://data/cars.csv'
163163
OPTIONS(
164-
'aws.access_key_id' '{}',
165-
'aws.secret_access_key' '{}',
166-
'aws.endpoint' '{}',
164+
'aws.access_key_id' '{access_key_id}',
165+
'aws.secret_access_key' '{secret_access_key}',
166+
'aws.endpoint' '{endpoint_url}',
167167
'aws.allow_http' 'true'
168168
);
169169
170170
SELECT * FROM CARS limit 1;
171-
"#,
172-
access_key_id, secret_access_key, endpoint_url
171+
"#
173172
);
174173

175174
assert_cmd_snapshot!(cli().env_clear().pass_stdin(input));

‎datafusion/ffi/src/plan_properties.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ mod tests {
321321

322322
let foreign_props: PlanProperties = local_props_ptr.try_into()?;
323323

324-
assert!(format!("{foreign_props:?}") == format!("{original_props:?}"));
324+
assert_eq!(format!("{foreign_props:?}"), format!("{original_props:?}"));
325325

326326
Ok(())
327327
}

‎datafusion/proto/src/logical_plan/file_formats.rs

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,7 @@ impl LogicalExtensionCodec for CsvLogicalExtensionCodec {
205205
_ctx: &SessionContext,
206206
) -> datafusion_common::Result<Arc<dyn FileFormatFactory>> {
207207
let proto = CsvOptionsProto::decode(buf).map_err(|e| {
208-
DataFusionError::Execution(format!(
209-
"Failed to decode CsvOptionsProto: {:?}",
210-
e
211-
))
208+
DataFusionError::Execution(format!("Failed to decode CsvOptionsProto: {e:?}"))
212209
})?;
213210
let options: CsvOptions = (&proto).into();
214211
Ok(Arc::new(CsvFormatFactory {
@@ -233,7 +230,7 @@ impl LogicalExtensionCodec for CsvLogicalExtensionCodec {
233230
});
234231

235232
proto.encode(buf).map_err(|e| {
236-
DataFusionError::Execution(format!("Failed to encode CsvOptions: {:?}", e))
233+
DataFusionError::Execution(format!("Failed to encode CsvOptions: {e:?}"))
237234
})?;
238235

239236
Ok(())
@@ -316,8 +313,7 @@ impl LogicalExtensionCodec for JsonLogicalExtensionCodec {
316313
) -> datafusion_common::Result<Arc<dyn FileFormatFactory>> {
317314
let proto = JsonOptionsProto::decode(buf).map_err(|e| {
318315
DataFusionError::Execution(format!(
319-
"Failed to decode JsonOptionsProto: {:?}",
320-
e
316+
"Failed to decode JsonOptionsProto: {e:?}"
321317
))
322318
})?;
323319
let options: JsonOptions = (&proto).into();
@@ -346,7 +342,7 @@ impl LogicalExtensionCodec for JsonLogicalExtensionCodec {
346342
});
347343

348344
proto.encode(buf).map_err(|e| {
349-
DataFusionError::Execution(format!("Failed to encode JsonOptions: {:?}", e))
345+
DataFusionError::Execution(format!("Failed to encode JsonOptions: {e:?}"))
350346
})?;
351347

352348
Ok(())
@@ -632,8 +628,7 @@ impl LogicalExtensionCodec for ParquetLogicalExtensionCodec {
632628
) -> datafusion_common::Result<Arc<dyn FileFormatFactory>> {
633629
let proto = TableParquetOptionsProto::decode(buf).map_err(|e| {
634630
DataFusionError::Execution(format!(
635-
"Failed to decode TableParquetOptionsProto: {:?}",
636-
e
631+
"Failed to decode TableParquetOptionsProto: {e:?}"
637632
))
638633
})?;
639634
let options: TableParquetOptions = (&proto).into();
@@ -663,8 +658,7 @@ impl LogicalExtensionCodec for ParquetLogicalExtensionCodec {
663658

664659
proto.encode(buf).map_err(|e| {
665660
DataFusionError::Execution(format!(
666-
"Failed to encode TableParquetOptionsProto: {:?}",
667-
e
661+
"Failed to encode TableParquetOptionsProto: {e:?}"
668662
))
669663
})?;
670664

‎datafusion/proto/src/logical_plan/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ impl AsLogicalPlan for LogicalPlanNode {
465465
));
466466
};
467467
let arrow_type = DataType::try_from(arrow_type).map_err(|e| {
468-
proto_error(format!("Received an unknown ArrowType: {}", e))
468+
proto_error(format!("Received an unknown ArrowType: {e}"))
469469
})?;
470470
Ok((col.name.clone(), arrow_type))
471471
})
@@ -1127,8 +1127,7 @@ impl AsLogicalPlan for LogicalPlanNode {
11271127
let arrow_type = protobuf::ArrowType::try_from(arrow_type)
11281128
.map_err(|e| {
11291129
proto_error(format!(
1130-
"Received an unknown ArrowType: {}",
1131-
e
1130+
"Received an unknown ArrowType: {e}"
11321131
))
11331132
})?;
11341133
Ok(protobuf::PartitionColumn {

‎datafusion/proto/tests/cases/roundtrip_logical_plan.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ fn roundtrip_expr_test_with_codec(
126126
codec: &dyn LogicalExtensionCodec,
127127
) {
128128
let proto: protobuf::LogicalExprNode = serialize_expr(&initial_struct, codec)
129-
.unwrap_or_else(|e| panic!("Error serializing expression: {:?}", e));
129+
.unwrap_or_else(|e| panic!("Error serializing expression: {e:?}"));
130130
let round_trip: Expr = from_proto::parse_expr(&proto, &ctx, codec).unwrap();
131131

132132
assert_eq!(format!("{:?}", &initial_struct), format!("{round_trip:?}"));

0 commit comments

Comments
 (0)