refactor: remove dead code and simplify trivial abstractions

Author: sj-robert

src/config.rs

@@ -19,7 +19,7 @@ pub enum ConfigError {
 }
 
 /// Main configuration structure
-#[derive(Debug, Clone, Deserialize, Serialize)]
+#[derive(Debug, Clone, Default, Deserialize, Serialize)]
 pub struct Config {
     #[serde(default)]
     pub migrations: MigrationsConfig,
@@ -157,20 +157,4 @@ impl Config {
         let config: Config = toml::from_str(&contents)?;
         Ok(config)
     }
-
-    /// Create a default configuration
-    pub fn default_config() -> Self {
-        Self {
-            migrations: MigrationsConfig::default(),
-            liquibase: LiquibaseConfig::default(),
-            output: OutputConfig::default(),
-            cli: CliConfig::default(),
-        }
-    }
-}
-
-impl Default for Config {
-    fn default() -> Self {
-        Self::default_config()
-    }
 }

src/input/liquibase_bridge.rs

@@ -26,9 +26,6 @@ pub struct BridgeLoader {
 #[derive(Debug, Deserialize)]
 struct BridgeChangeset {
     changeset_id: String,
-    #[serde(default)]
-    #[allow(dead_code)]
-    author: String,
     sql: String,
     xml_file: String,
     #[serde(default = "default_xml_line")]

src/input/liquibase_xml.rs

@@ -78,8 +78,6 @@ enum ParseState {
 #[derive(Debug, Clone)]
 struct ChangeSetInfo {
     id: String,
-    #[allow(dead_code)]
-    author: String,
     run_in_transaction: bool,
     line: usize,
     sql_parts: Vec<String>,
@@ -89,14 +87,12 @@ impl ChangeSetInfo {
     /// Create a new ChangeSetInfo from attributes of a <changeSet> element.
     fn from_attributes(attrs: &[(String, String)], line: usize) -> Self {
         let id = get_attr(attrs, "id").unwrap_or_default();
-        let author = get_attr(attrs, "author").unwrap_or_default();
         let run_in_transaction = get_attr(attrs, "runInTransaction")
             .map(|v| v != "false")
             .unwrap_or(true);
 
         Self {
             id,
-            author,
             run_in_transaction,
             line,
             sql_parts: Vec::new(),

src/input/sql.rs

@@ -14,14 +14,10 @@ use std::path::{Path, PathBuf};
 /// lexicographically by filename. Each file becomes one `MigrationUnit`.
 ///
 /// Down migrations are detected by filename patterns: `.down.sql` or `_down.sql`.
+#[derive(Default)]
 pub struct SqlLoader;
 
 impl SqlLoader {
-    /// Create a new `SqlLoader`.
-    pub fn new() -> Self {
-        Self
-    }
-
     /// Load a single SQL file and parse it into a `MigrationUnit`.
     ///
     /// The file is read entirely into memory, parsed into IR nodes, and
@@ -52,12 +48,6 @@ impl SqlLoader {
     }
 }
 
-impl Default for SqlLoader {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
 impl MigrationLoader for SqlLoader {
     /// Load migrations from the given paths.
     ///
@@ -108,8 +98,6 @@ impl MigrationLoader for SqlLoader {
 }
 
 /// Collect all `.sql` files from a directory (non-recursive).
-///
-/// Returns the files sorted lexicographically by filename.
 fn collect_sql_files(dir: &Path) -> Result<Vec<PathBuf>, LoadError> {
     let entries = std::fs::read_dir(dir).map_err(|e| LoadError::Io {
         path: dir.to_path_buf(),
@@ -130,7 +118,6 @@ fn collect_sql_files(dir: &Path) -> Result<Vec<PathBuf>, LoadError> {
         }
     }
 
-    files.sort();
     Ok(files)
 }
 
@@ -255,7 +242,7 @@ mod tests {
         // Also create a non-SQL file that should be ignored
         fs::write(dir.path().join("README.md"), "# Migrations").expect("write");
 
-        let loader = SqlLoader::new();
+        let loader = SqlLoader;
         let history = loader
             .load(&[dir.path().to_path_buf()])
             .expect("Failed to load migrations");
@@ -274,21 +261,21 @@ mod tests {
         let file_path = dir.path().join("migration.sql");
         fs::write(&file_path, "CREATE TABLE t (id int);").expect("write");
 
-        let loader = SqlLoader::new();
+        let loader = SqlLoader;
         let history = loader.load(&[file_path]).expect("Failed to load migration");
         assert_eq!(history.units.len(), 1);
     }
 
     #[test]
     fn test_loader_nonexistent_path() {
-        let loader = SqlLoader::new();
+        let loader = SqlLoader;
         let result = loader.load(&[PathBuf::from("/nonexistent/path")]);
         assert!(result.is_err());
     }
 
     #[test]
     fn test_loader_empty_paths() {
-        let loader = SqlLoader::new();
+        let loader = SqlLoader;
         let history = loader.load(&[]).expect("Empty paths should succeed");
         assert!(history.units.is_empty());
     }
@@ -309,7 +296,7 @@ mod tests {
         )
         .expect("write");
 
-        let loader = SqlLoader::new();
+        let loader = SqlLoader;
         let history = loader
             .load(&[dir1.path().to_path_buf(), dir2.path().to_path_buf()])
             .expect("Failed to load migrations");

src/main.rs

@@ -357,7 +357,7 @@ fn load_migrations(config: &Config) -> Result<MigrationHistory> {
         }
         "filename_lexicographic" => {
             eprintln!("pg-migration-lint: using filename_lexicographic strategy");
-            let loader = SqlLoader::new();
+            let loader = SqlLoader;
             let history = loader
                 .load(&config.migrations.paths)
                 .context("Failed to load migrations")?;
@@ -368,7 +368,7 @@ fn load_migrations(config: &Config) -> Result<MigrationHistory> {
                 "pg-migration-lint: unknown strategy '{}', falling back to filename_lexicographic",
                 other
             );
-            let loader = SqlLoader::new();
+            let loader = SqlLoader;
             let history = loader
                 .load(&config.migrations.paths)
                 .context("Failed to load migrations")?;

src/parser/pg_query.rs

@@ -116,7 +116,7 @@ fn convert_node(node: &NodeEnum, raw_sql: &str) -> IrNode {
 // ---------------------------------------------------------------------------
 
 /// Convert a pg_query `CreateStmt` to `IrNode::CreateTable`.
-fn convert_create_table(create: &pg_query::protobuf::CreateStmt, raw_sql: &str) -> IrNode {
+fn convert_create_table(create: &pg_query::protobuf::CreateStmt, _raw_sql: &str) -> IrNode {
     let name = relation_to_qualified_name(create.relation.as_ref());
 
     let temporary = matches!(
@@ -155,7 +155,6 @@ fn convert_create_table(create: &pg_query::protobuf::CreateStmt, raw_sql: &str)
         constraints,
         temporary,
     })
-    .or_unparseable(raw_sql)
 }
 
 /// Convert a pg_query `ColumnDef` into an IR `ColumnDef` plus any inline
@@ -661,14 +660,11 @@ fn extract_qualified_name_from_drop_objects(
                 1 => Some(QualifiedName::unqualified(&strings[0])),
                 2 => Some(QualifiedName::qualified(&strings[0], &strings[1])),
                 _ if !strings.is_empty() => {
-                    // Take last two as schema.name
+                    // Take last two as schema.name (len >= 3 guaranteed here
+                    // since len == 1 and len == 2 are handled above)
                     let name = strings.last().cloned().unwrap_or_default();
-                    if strings.len() >= 2 {
-                        let schema = strings[strings.len() - 2].clone();
-                        Some(QualifiedName::qualified(schema, name))
-                    } else {
-                        Some(QualifiedName::unqualified(name))
-                    }
+                    let schema = strings[strings.len() - 2].clone();
+                    Some(QualifiedName::qualified(schema, name))
                 }
                 _ => None,
             };
@@ -805,18 +801,6 @@ fn extract_first_identifier(s: &str) -> Option<String> {
     }
 }
 
-/// Extension trait to allow fallback to `Unparseable` if conversion panics.
-/// This is used internally as a safe guard.
-trait OrUnparseable {
-    fn or_unparseable(self, raw_sql: &str) -> IrNode;
-}
-
-impl OrUnparseable for IrNode {
-    fn or_unparseable(self, _raw_sql: &str) -> IrNode {
-        self
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;