Skip to content

Commit

Permalink
global refactorings
Browse files Browse the repository at this point in the history
- use mod.rs instead of empty named module file
- replace emtpy new constructor with defaulf implementations
- sorting derives
- replaced ElementClass::from_string method with From trait impl
- rename `Buffer::to_string` to `try_to_string` to indicate possible failure
- default impl for `TotalTestResutls`
- make function arguments more readable/flexible
- better usage of iterators and options, as well as minor improvements
- remove iter calls in for loops
- asserts, matches and if let bindings
- must use attributes
- strings and closures
- debug and default impls
- borrows in arguments
- filter iterators
- make use of `Self` keyword in impl blocks
  • Loading branch information
koopa1338 committed Nov 29, 2023
1 parent 507b1ea commit 37584b2
Show file tree
Hide file tree
Showing 57 changed files with 763 additions and 796 deletions.
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

0 comments on commit 37584b2

Please sign in to comment.