feat(uair): warn on unknown config values (#25)

* feat: add deny_unknown_fields to deserializable structs

This prevents configs with unknown fields from deserializing, letting
the user know immediately that the field is invalid.

Closes #22

* chore: add serde_ignored to dependencies

* chore(uair): remove '#[serde(deny_unknown_fields)]'

* feat(uair): warn on unknown config values

* refactor: move logger initialization to main

This allows for mock logger setup in tests

* test: assert warning is printed on unknown config fields

* chore: update CHANGELOG

* chore: remove commented code

---------

Co-authored-by: Rishabh Das <[email protected]>

CHANGELOG.md

@@ -1,3 +1,7 @@
+## Unreleased
+
+- Log warnings about unknown fields when deserializing the config, letting the user know immediately that the field is invalid. (@0xangelo)
+
 ### v0.6.2
 
 ### Added

Cargo.toml

@@ -19,9 +19,13 @@ humantime = "2.1.0"
 humantime-serde = "1.1.1"
 log = "0.4.22"
 serde = { version = "1.0.214", features = ["derive"] }
+serde_ignored = "0.1.10"
 signal-hook = "0.3.17"
 signal-hook-async-std = "0.2.2"
 simplelog = "0.12.2"
 thiserror = "2.0.3"
 toml = "0.8.19"
 winnow = "0.6.20"
+
+[dev-dependencies]
+testing_logger = "0.1"

src/bin/uair/app.rs

@@ -4,9 +4,8 @@ use crate::socket::{Listener, Stream};
 use crate::timer::{State, UairTimer};
 use crate::{Args, Error};
 use futures_lite::FutureExt;
-use log::{error, LevelFilter};
-use simplelog::{ColorChoice, Config as LogConfig, TermLogger, TerminalMode, WriteLogger};
-use std::fs::{self, File};
+use log::error;
+use std::fs;
 use std::io::{self, Error as IoError, ErrorKind, Write};
 use std::time::{Duration, Instant};
 use uair::{Command, FetchArgs, JumpArgs, ListenArgs, PauseArgs, ResumeArgs};
@@ -18,20 +17,6 @@ pub struct App {
 
 impl App {
 	pub fn new(args: Args) -> Result<Self, Error> {
-		if args.log == "-" {
-			TermLogger::init(
-				LevelFilter::Info,
-				LogConfig::default(),
-				TerminalMode::Stderr,
-				ColorChoice::Auto,
-			)?;
-		} else {
-			WriteLogger::init(
-				LevelFilter::Info,
-				LogConfig::default(),
-				File::create(&args.log)?,
-			)?;
-		}
 		let timer = UairTimer::new(Duration::from_secs(1), args.quiet);
 		let data = AppData::new(args)?;
 		Ok(App { data, timer })

src/bin/uair/config.rs

@@ -1,4 +1,5 @@
 use crate::session::{Color, Overridables, Session, TimeFormatToken, Token};
+use log::warn;
 use serde::de::Error as _;
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
@@ -31,7 +32,10 @@ pub struct ConfigBuilder {
 
 impl ConfigBuilder {
 	pub fn deserialize(conf: &str) -> Result<Self, Error> {
-		toml::from_str(conf)
+		let deserializer = toml::Deserializer::new(conf);
+		serde_ignored::deserialize(deserializer, |path| {
+			warn!("{path} is not a valid config and will be ignored.")
+		})
 	}
 
 	pub fn build(self) -> Result<Config, Error> {
@@ -239,3 +243,35 @@ impl OverridablesBuilder {
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use toml::de::Error;
+
+	use log::Level;
+
+	const TOML_CFG: &str = r#"
+loop-on-end = true
+
+[defaults]
+format = "{time}\n"
+time_format = "%M:%S"
+
+[[sessions]]
+"#;
+
+	#[test]
+	fn unknown_config_field_warning() -> Result<(), Error> {
+		testing_logger::setup();
+		ConfigBuilder::deserialize(TOML_CFG)?;
+		testing_logger::validate(|captured_logs| {
+			assert_eq!(captured_logs.len(), 1);
+			assert!(captured_logs[0]
+				.body
+				.contains("is not a valid config and will be ignored"));
+			assert_eq!(captured_logs[0].level, Level::Warn);
+		});
+		Ok(())
+	}
+}

src/bin/uair/main.rs

@@ -7,10 +7,13 @@ mod timer;
 use crate::app::App;
 use argh::FromArgs;
 use futures_lite::{FutureExt, StreamExt};
-use log::error;
+use log::{error, LevelFilter};
 use signal_hook::consts::signal::*;
 use signal_hook_async_std::Signals;
+use simplelog::{ColorChoice, Config as LogConfig, TermLogger, TerminalMode, WriteLogger};
 use std::env;
+use std::fmt::Display;
+use std::fs::File;
 use std::io::{self, Write};
 use std::process::ExitCode;
 use uair::get_socket_path;
@@ -29,23 +32,19 @@ fn main() -> ExitCode {
 
 	let enable_stderr = args.log != "-";
 
+	if let Err(err) = init_logger(&args) {
+		return raise_err(err, enable_stderr);
+	}
+
 	let app = match App::new(args) {
 		Ok(app) => app,
 		Err(err) => {
-			error!("{}", err);
-			if enable_stderr {
-				eprintln!("{}", err)
-			}
-			return ExitCode::FAILURE;
+			return raise_err(err, enable_stderr);
 		}
 	};
 
 	if let Err(err) = async_io::block_on(app.run().or(catch_term_signals())) {
-		error!("{}", err);
-		if enable_stderr {
-			eprintln!("{}", err);
-			return ExitCode::FAILURE;
-		}
+		return raise_err(err, enable_stderr);
 	}
 
 	ExitCode::SUCCESS
@@ -91,6 +90,32 @@ async fn catch_term_signals() -> Result<(), Error> {
 	Ok(())
 }
 
+fn init_logger(args: &Args) -> Result<(), Error> {
+	if args.log == "-" {
+		TermLogger::init(
+			LevelFilter::Info,
+			LogConfig::default(),
+			TerminalMode::Stderr,
+			ColorChoice::Auto,
+		)?;
+	} else {
+		WriteLogger::init(
+			LevelFilter::Info,
+			LogConfig::default(),
+			File::create(&args.log)?,
+		)?;
+	}
+	Ok(())
+}
+
+fn raise_err(err: impl Display, enable_stderr: bool) -> ExitCode {
+	error!("{}", err);
+	if enable_stderr {
+		eprintln!("{}", err)
+	}
+	ExitCode::FAILURE
+}
+
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
 	#[error("Log Error: {0}")]