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

Improving Code Idiomacy #268

Merged
merged 1 commit into from
Nov 30, 2023
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
3 changes: 2 additions & 1 deletion AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@

Ahmed Ibrahim <[email protected]>
Joshua Thijssen <[email protected]>
Zach Weaver <[email protected]>
Niklas Scheerhoorn <[email protected]>
Zach Weaver <[email protected]>
19 changes: 10 additions & 9 deletions src/api/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub enum LogLevel {
impl fmt::Display for LogLevel {
// When displaying the enum, make sure it is in lowercase
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", format!("{:?}", self).to_lowercase())
write!(f, "{}", format!("{self:?}").to_lowercase())
}
}

Expand Down Expand Up @@ -72,8 +72,9 @@ impl Console {
/// # Returns
///
/// A new Console struct
pub fn new(printer: Box<dyn Printer>) -> Console {
Console {
#[must_use]
pub fn new(printer: Box<dyn Printer>) -> Self {
Self {
timer_map: HashMap::new(),
count_map: HashMap::new(),
group_stacks: vec![],
Expand Down Expand Up @@ -132,7 +133,7 @@ impl Console {
/// Emit table if tabular data is supported
pub fn table(&mut self, tabular_data: String, _properties: &[&str]) {
// TODO: needs implementation
self.printer.print(LogLevel::Log, &[&tabular_data], &[])
self.printer.print(LogLevel::Log, &[&tabular_data], &[]);
}

/// Emit a trace message
Expand Down Expand Up @@ -224,7 +225,7 @@ impl Console {
/// Create a timer with given label
pub fn time(&mut self, label: &str) {
if self.timer_map.contains_key(&label.to_owned()) {
let warning = format!("Timer '{}' already started", label);
let warning = format!("Timer '{label}' already started");
self.logger(LogLevel::Warn, &[&warning]);
return;
}
Expand All @@ -248,7 +249,7 @@ impl Console {
pub fn time_log(&mut self, label: &str, data: &[&dyn fmt::Display]) {
let mut message = String::from(" ");
for arg in data {
message.push_str(&format!("{} ", arg));
message.push_str(&format!("{arg} "));
}
let message = message.trim_end();

Expand Down Expand Up @@ -325,7 +326,7 @@ mod tests {
c.log(&[&"some", &"data", &12i32]);
c.warn(&[&"Hello", &"World"]);

let out = buffer.borrow().to_string().unwrap();
let out = buffer.borrow().try_to_string().unwrap();
assert_eq!(
out,
"\
Expand Down Expand Up @@ -357,7 +358,7 @@ mod tests {
c.group_end();
c.warn(&[&"Back", &"To root"]);

let out = buffer.borrow().to_string().unwrap();
let out = buffer.borrow().try_to_string().unwrap();
let re = Regex::new(
r"^--- Clear ---
> Expanded group: foo
Expand Down Expand Up @@ -385,7 +386,7 @@ $",
c.assert(true, &[]);
c.assert(false, &[]);

let out = buffer.borrow().to_string().unwrap();
let out = buffer.borrow().try_to_string().unwrap();
assert_eq!(
out,
"\
Expand Down
6 changes: 4 additions & 2 deletions src/api/console/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@ use crate::types::Result;
use std::io::Write;

/// A buffer that can be written to and then converted to a string
#[derive(Default)]
pub struct Buffer {
/// Internal buffer
buf: Vec<u8>,
}

impl Buffer {
/// Creates a new buffer
#[must_use]
pub fn new() -> Self {
Self { buf: Vec::new() }
Self::default()
}

/// Converts the buffer to a String
pub fn to_string(&self) -> Result<String> {
pub fn try_to_string(&self) -> Result<String> {
Ok(String::from_utf8(self.buf.clone())?)
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/api/console/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@ pub struct Formatter;

impl Formatter {
/// Returns a new formatter
pub fn new() -> Formatter {
Formatter {}
#[must_use]
pub fn new() -> Self {
Self {}
}

/// Formats the given string based on the formatting arguments and data provided
pub fn format(&self, args: &[&dyn fmt::Display]) -> String {
let mut s = String::from("");
let mut s = String::new();
for arg in args {
s.push_str(&format!("{} ", arg));
s.push_str(&format!("{arg} "));
}

s.trim_end().to_string()
s.trim_end().to_owned()
}
}
28 changes: 17 additions & 11 deletions src/api/console/writable_printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ pub(crate) struct WritablePrinter<W: Write> {

impl<W: Write> WritablePrinter<W> {
/// Creates a new writable printer
pub fn new(writer: Rc<RefCell<W>>) -> WritablePrinter<W> {
WritablePrinter {
pub fn new(writer: Rc<RefCell<W>>) -> Self {
Self {
writer,
groups: vec![],
}
Expand Down Expand Up @@ -58,11 +58,11 @@ impl<W: Write> Printer for WritablePrinter<W> {

let group_prefix = " > ".repeat(self.groups.len());

let mut data = String::from("");
let mut data = String::new();
for arg in args {
data.push_str(format!("{} ", arg).as_str());
data.push_str(format!("{arg} ").as_str());
}
data = data.trim_end().to_string();
data = data.trim_end().to_owned();
let mut writer = self.writer.borrow_mut();

let _ = match log_level {
Expand All @@ -71,13 +71,13 @@ impl<W: Write> Printer for WritablePrinter<W> {
| LogLevel::Error
| LogLevel::Log
| LogLevel::Assert => {
writeln!(writer, "{}[{}] {}", group_prefix, log_level, data)
writeln!(writer, "{group_prefix}[{log_level}] {data}")
}
LogLevel::Group => writeln!(writer, "{}Expanded group: {}", group_prefix, data),
LogLevel::Group => writeln!(writer, "{group_prefix}Expanded group: {data}"),
LogLevel::GroupCollapsed => {
writeln!(writer, "{}Collapsed group: {}", group_prefix, data)
writeln!(writer, "{group_prefix}Collapsed group: {data}")
}
LogLevel::TimeEnd => writeln!(writer, "{}{} - timer ended", group_prefix, data),
LogLevel::TimeEnd => writeln!(writer, "{group_prefix}{data} - timer ended"),
_ => Ok(()),
};
}
Expand Down Expand Up @@ -106,7 +106,10 @@ mod tests {

printer.print(LogLevel::Log, &[&"Hello", &"World"], &[]);
assert_eq!(
buffer.borrow_mut().to_string().expect("failed to convert"),
buffer
.borrow_mut()
.try_to_string()
.expect("failed to convert"),
"[log] Hello World\n"
);

Expand All @@ -116,7 +119,10 @@ mod tests {
printer.print(LogLevel::Warn, &[&"a", &"b"], &[]);
printer.print(LogLevel::Error, &[], &[]);
assert_eq!(
buffer.borrow_mut().to_string().expect("failed to convert"),
buffer
.borrow_mut()
.try_to_string()
.expect("failed to convert"),
"[info] Foo 2 false\n[warn] a b\n"
);
}
Expand Down
10 changes: 5 additions & 5 deletions src/bin/config-store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ enum Commands {
},
}

#[derive(Debug, Clone, Copy, clap::ValueEnum, Display)]
#[derive(Clone, Copy, Debug, Display, clap::ValueEnum)]
enum Engine {
Sqlite,
Json,
Expand Down Expand Up @@ -91,15 +91,15 @@ fn main() -> anyhow::Result<()> {
let info = config::config_store().get_info(&key).unwrap();
let value = config::config_store().get(&key).unwrap();

println!("Key : {}", key);
println!("Current Value : {}", value);
println!("Key : {key}");
println!("Current Value : {value}");
println!("Default Value : {}", info.default);
println!("Description : {}", info.description);
}
Commands::List => {
for key in config::config_store().find("*") {
let value = config::config_store().get(&key).unwrap();
println!("{:40}: {}", key, value);
println!("{key:40}: {value}");
}
}
Commands::Set { key, value } => {
Expand All @@ -108,7 +108,7 @@ fn main() -> anyhow::Result<()> {
Commands::Search { key } => {
for key in config::config_store().find(&key) {
let value = config::config_store().get(&key).unwrap();
println!("{:40}: {}", key, value);
println!("{key:40}: {value}");
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/bin/gosub-parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::fs;
use std::process::exit;

fn bail(message: &str) -> ! {
println!("{}", message);
println!("{message}");
exit(1);
}

Expand Down Expand Up @@ -38,16 +38,16 @@ fn main() -> Result<()> {

// If the encoding confidence is not Confidence::Certain, we should detect the encoding.
if !chars.is_certain_encoding() {
chars.detect_encoding()
chars.detect_encoding();
}

let document = DocumentBuilder::new_document();
let parse_errors = Html5Parser::parse_document(&mut chars, Document::clone(&document), None)?;

println!("Generated tree: \n\n {}", document);
println!("Generated tree: \n\n {document}");

for e in parse_errors {
println!("Parse Error: {}", e.message)
println!("Parse Error: {}", e.message);
}

Ok(())
Expand Down
4 changes: 2 additions & 2 deletions src/bin/html5-parser-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main() -> Result<()> {
let mut total = 0;
let mut failed = 0;

for file in files.iter() {
for file in &files {
// if file != "math.dat" {
// continue;
// }
Expand All @@ -26,7 +26,7 @@ fn main() -> Result<()> {
let mut harness = Harness::new();

// Run tests
for test in fixture.tests.iter() {
for test in &fixture.tests {
for &scripting_enabled in test.script_modes() {
let result = harness
.run_test(test.clone(), scripting_enabled)
Expand Down
16 changes: 5 additions & 11 deletions src/bin/parser-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use gosub_engine::testing::tree_construction::Harness;
use gosub_engine::testing::tree_construction::Test;

/// Holds the results from all tests that are executed
#[derive(Default)]
pub struct TotalTestResults {
/// Number of tests (as defined in the suite)
tests: usize,
Expand All @@ -20,14 +21,7 @@ pub struct TotalTestResults {
}

fn main() {
let mut results = TotalTestResults {
tests: 0,
assertions: 0,
succeeded: 0,
failed: 0,
failed_position: 0,
tests_failed: Vec::new(),
};
let mut results = TotalTestResults::default();

let filenames = Some(&["tests15.dat"][..]);
let fixtures = read_fixtures(filenames).expect("fixtures");
Expand Down Expand Up @@ -61,8 +55,8 @@ fn main() {
if results.failed > 0 {
println!("❌ Failed tests:");
for (test_idx, line, data) in results.tests_failed {
println!(" * Test #{} at line {}:", test_idx, line);
println!(" {}", data);
println!(" * Test #{test_idx} at line {line}:");
println!(" {data}");
}
}
}
Expand Down Expand Up @@ -112,7 +106,7 @@ fn run_test(test_idx: usize, test: Test, all_results: &mut TotalTestResults) {
#[cfg(feature = "debug_parser")]
println!("❌ {} (wanted: {})", entry.actual, entry.expected);
}
_ => {}
ResultStatus::IncorrectPosition => {}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/bin/test-user-agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ fn main() -> Result<()> {
}

for e in parse_errors {
println!("Parse Error: {}", e.message)
println!("Parse Error: {}", e.message);
}

Ok(())
Expand Down
Loading