Clean up provider parsing, CLI validation, and duplicate tests

docs/code-cleanup-review.md

@@ -0,0 +1,31 @@
+# Code cleanup review — 2026-09-22
+
+Reviewed from freshly fetched `origin/master` (`ed162ba`). This is a focused source and build review, not certification of every database provider.
+
+## Changes in this cleanup
+
+- **Factory resolution:** stop swallowing exceptions thrown by registered factory delegates. Missing registrations still fall through to the system registry and reflection; actual initialization failures retain their original exception. Use a concurrent registry for registration, lookup and provider-name enumeration.
+- **Migration names:** preserve the first character of lowercase names and handle empty or numeric-only names without substring exceptions. These names are used in loader tracing and migration logging.
+- **SQL type names:** compare SQL names independently of the current culture, avoid repeated enumeration, and expand length/precision/scale placeholders when falling back to the default type mapping.
+- **Duplication:** share loader trace output, delegate the `DbType` registration overloads to the `MigratorDbType` implementation, and use a hash set for duplicate migration-version detection.
+- **Unnecessary reflection:** instantiate known dialect classes directly. The existing null result for unknown provider values remains unchanged.
+- **Dead code:** remove three files containing only commented-out test code: the old MySQL provider fixture, SQLite PRAGMA fixture and schema-dumper fixture. None contributed executable tests.
+- **Small hygiene fixes:** remove a duplicate MySQL import, dispose the generic test provider and directly index the built fluent-operation list.
+
+## Follow-up changes
+
+- **Provider default parsing:** moved SQL Server, Oracle and PostgreSQL default interpretation into separate internal parsers. Shared text classification, numeric conversions and hexadecimal decoding remove duplicated conversion logic while retaining dialect-specific date, interval, cast and GUID handling. Removed unreachable string branches and PostgreSQL's empty primary-key branch.
+- **Parser corrections:** numeric SQL `NULL` defaults remain null; malformed odd-length binary literals now fail instead of silently losing their last digit. PostgreSQL numeric literals no longer depend on whether their text contains the table name; SQL expressions still pass through as `RawSql`.
+- **Fixture inheritance:** removed an empty SQLite fixture with a hidden setup method; renamed the table-plus-primary-key helper so it no longer hides the base helper; removed SQL Server's identical copy of the inherited compound-primary-key test.
+- **Duplicate scenarios:** retained one identity/primary-key metadata test and removed two identical copies in the generic fixture. This removes two repeated cases from each of four derived provider suites. No distinct assertions were removed.
+- **CLI structure:** extracted typed options and command handlers from the entry point. Driver names and factories now use one mapping. Options are validated before assembly loading or opening a database, including options passed to offline/list commands. Exit codes and credential-safe errors remain covered.
+- **Warning noise:** deliberate calls to three obsolete APIs now go through the test-only `LegacyMetadata` helper, with suppression scoped to those calls. Production obsolete-API warnings and complexity diagnostics remain enabled.
+
+## Validation and remaining limits
+
+- Solution rebuild passes. The final build retains production complexity and compatibility warnings; the CLI complexity and hidden-test-member warnings are gone.
+- Unit suite: 175 passed, zero skipped, including 53 catalog parser cases and CLI argument/filter/offline-output coverage.
+- SQLite suite: 203 passed, zero skipped. The previous 205 included two duplicate generic cases.
+- Parser cases check CLR types as well as values under a non-English culture, SQL expressions, nulls, SQL Server datetime conversion, Oracle GUID byte order, PostgreSQL casts/timezones/intervals, and invalid binary input.
+- SQL Server, PostgreSQL and Oracle catalog queries still need their live CI suites after this refactor. Docker is unavailable locally, so no server-backed matrix result is claimed. Test discovery/count reporting is dynamic and does not require hardcoded count updates.
+- The provider catalog/type-mapping methods remain substantial; this change separates and tests default parsing without rewriting their database queries. Other existing complexity warnings in the migration runner and SQLite reconstruction/parser code remain future cleanup work.

src/Migrator.Tests/CatalogDefaultParsingTests.cs

@@ -0,0 +1,114 @@
+using System;
+using System.Collections.Generic;
+using System.Data;
+using DotNetProjects.Migrator.Framework;
+using DotNetProjects.Migrator.Providers.Impl.Oracle;
+using DotNetProjects.Migrator.Providers.Impl.PostgreSQL;
+using DotNetProjects.Migrator.Providers.Impl.SqlServer;
+using NUnit.Framework;
+
+namespace Migrator.Tests;
+
+public class CatalogDefaultParsingTests
+{
+    private static IEnumerable<TestCaseData> Literals()
+    {
+        foreach (var dialect in new[] { "SqlServer", "Oracle", "PostgreSQL" })
+        {
+            yield return new TestCaseData(dialect, DbType.String, "'O''Brien (x)'", "O'Brien (x)");
+            yield return new TestCaseData(dialect, DbType.Int32, "42", 42L);
+            yield return new TestCaseData(dialect, DbType.Int16, "'-42'", -42L);
+            yield return new TestCaseData(dialect, DbType.Decimal, "'12.50'", 12.50m);
+            yield return new TestCaseData(dialect, DbType.Double, "1.25", 1.25d);
+            yield return new TestCaseData(dialect, DbType.Single, "1.25", dialect == "Oracle" ? (object)1.25f : 1.25d);
+            yield return new TestCaseData(dialect, DbType.Guid, "'00112233-4455-6677-8899-aabbccddeeff'", Guid.Parse("00112233-4455-6677-8899-aabbccddeeff"));
+        }
+        yield return new TestCaseData("SqlServer", DbType.Int64, "(('42'))", 42L);
+        yield return new TestCaseData("SqlServer", DbType.Byte, "((255))", (byte)255);
+        yield return new TestCaseData("SqlServer", DbType.UInt64, "((18446744073709551615))", ulong.MaxValue);
+        yield return new TestCaseData("SqlServer", DbType.Boolean, "((1))", true);
+        yield return new TestCaseData("SqlServer", DbType.Boolean, "('FALSE')", false);
+        yield return new TestCaseData("SqlServer", DbType.Binary, "(0x00A1ff)", new byte[] { 0, 161, 255 });
+        yield return new TestCaseData("SqlServer", DbType.DateTime, "(CONVERT([datetime],'2000-01-02 03:04:05.123',(121)))", new DateTime(2000, 1, 2, 3, 4, 5, 123, DateTimeKind.Utc));
+        yield return new TestCaseData("SqlServer", DbType.Time, "'12:34:56.1234567'", new TimeOnly(12, 34, 56).Add(TimeSpan.FromTicks(1234567)));
+        yield return new TestCaseData("Oracle", DbType.Binary, "HEXTORAW('00A1ff')", new byte[] { 0, 161, 255 });
+        yield return new TestCaseData("Oracle", DbType.Guid, "HEXTORAW('00112233445566778899AABBCCDDEEFF')", Guid.Parse("00112233-4455-6677-8899-aabbccddeeff"));
+        yield return new TestCaseData("Oracle", DbType.DateTime, "TO_TIMESTAMP('2000-01-02 03:04:05.123','YYYY-MM-DD HH24:MI:SS.FF')", new DateTime(2000, 1, 2, 3, 4, 5, 123, DateTimeKind.Utc));
+        yield return new TestCaseData("Oracle", DbType.Boolean, "TRUE", true);
+        yield return new TestCaseData("PostgreSQL", DbType.Int32, "'42'::integer", 42L);
+        yield return new TestCaseData("PostgreSQL", DbType.String, "'O''Brien'::text", "O'Brien");
+        yield return new TestCaseData("PostgreSQL", DbType.Binary, "'\\x00a1ff'::bytea", new byte[] { 0, 161, 255 });
+        yield return new TestCaseData("PostgreSQL", DbType.Time, "'12:34:56.1234567'::time without time zone", new TimeOnly(12, 34, 56).Add(TimeSpan.FromTicks(1234567)));
+        yield return new TestCaseData("PostgreSQL", DbType.DateTime, "'2000-01-02 03:04:05'::timestamp without time zone", new DateTime(2000, 1, 2, 3, 4, 5, DateTimeKind.Utc));
+        yield return new TestCaseData("PostgreSQL", DbType.DateTimeOffset, "'2000-01-02 03:04:05+02'::timestamp with time zone", new DateTimeOffset(2000, 1, 2, 3, 4, 5, TimeSpan.FromHours(2)));
+        yield return new TestCaseData("PostgreSQL", DbType.Boolean, "false", false);
+    }
+
+    [TestCaseSource(nameof(Literals))]
+    [SetCulture("de-DE")]
+    public void CatalogLiteralsRetainTheirValuesAndClrTypes(string dialect, DbType type, string sql, object expected)
+    {
+        var column = new Column("Value", type);
+        Apply(dialect, column, sql);
+        Assert.That(column.DefaultValue, Is.EqualTo(expected));
+        Assert.That(column.DefaultValue.GetType(), Is.EqualTo(expected.GetType()));
+    }
+
+    [TestCase("SqlServer", "(newid())")]
+    [TestCase("Oracle", "SYS_GUID()")]
+    [TestCase("PostgreSQL", "gen_random_uuid()")]
+    public void ExpressionsRemainSql(string dialect, string sql)
+    {
+        var column = new Column("Value", DbType.Guid);
+        Apply(dialect, column, sql);
+        Assert.That(column.DefaultValue, Is.TypeOf<RawSql>());
+        Assert.That(column.DefaultValue.ToString(), Is.EqualTo(sql));
+    }
+
+    [TestCase("SqlServer", "(0xABC)")]
+    [TestCase("Oracle", "HEXTORAW('ABC')")]
+    [TestCase("PostgreSQL", "'\\xabc'::bytea")]
+    public void OddLengthBinaryDefaultsAreRejectedInsteadOfTruncated(string dialect, string sql)
+    {
+        Assert.Throws<FormatException>(() => Apply(dialect, new Column("Value", DbType.Binary), sql));
+    }
+
+    [TestCase("schema.sequence.nextval")]
+    [TestCase("\"ISEQ$$123\".nextval")]
+    [TestCase("NULL")]
+    public void OracleIdentityExpressionsAndNullDoNotBecomeDefaults(string sql)
+    {
+        var column = new Column("Value", DbType.Int64);
+        OracleColumnDefault.Apply(column, sql);
+        Assert.That(column.DefaultValue, Is.Null);
+    }
+
+    private static void Apply(string dialect, Column column, string sql)
+    {
+        switch (dialect)
+        {
+            case "SqlServer": SqlServerColumnDefault.Apply(column, sql); break;
+            case "Oracle": OracleColumnDefault.Apply(column, sql); break;
+            case "PostgreSQL": PostgreSqlColumnDefault.Apply(column, sql); break;
+            default: throw new ArgumentOutOfRangeException(nameof(dialect));
+        }
+    }
+
+    [TestCase("SqlServer", "((NULL))")]
+    [TestCase("PostgreSQL", "NULL")]
+    public void NumericNullDefaultsRemainNull(string dialect, string sql)
+    {
+        var column = new Column("Value", DbType.Int32);
+        Apply(dialect, column, sql);
+        Assert.That(column.DefaultValue, Is.Null);
+    }
+
+    [TestCase("-51:02:03.1234567", -1837231234567L)]
+    [TestCase("51:02:03.1234567", 1837231234567L)]
+    public void PostgreSqlIntervalsRetainSignDaysAndFractionalTicks(string literal, long ticks)
+    {
+        var column = new Column("Value", MigratorDbType.Interval);
+        PostgreSqlColumnDefault.Apply(column, "'" + literal + "'::interval");
+        Assert.That(column.DefaultValue, Is.EqualTo(TimeSpan.FromTicks(ticks)));
+    }
+}

src/Migrator.Tests/CoreCleanupRegressionTests.cs

@@ -0,0 +1,87 @@
+using System;
+using System.Data;
+using System.Data.Common;
+using DotNetProjects.Migrator.Framework;
+using DotNetProjects.Migrator.Providers;
+using Microsoft.Data.Sqlite;
+using NUnit.Framework;
+using ProviderFactories = DotNetProjects.Migrator.Providers.DbProviderFactories;
+
+namespace Migrator.Tests;
+
+public class CoreCleanupRegressionTests
+{
+    [TestCase("CreateATable", "Create a table")]
+    [TestCase("createTable", "Create table")]
+    [TestCase("x", "X")]
+    [TestCase("_123", "")]
+    [TestCase("", "")]
+    [TestCase("001_CreateATable_123", "Create a table")]
+    public void HumanNamesHandleShortAndLowercaseNames(string name, string expected)
+    {
+        Assert.That(StringUtils.ToHumanName(name), Is.EqualTo(expected));
+    }
+
+    [Test]
+    public void RegisteredFactoryErrorsArePreserved()
+    {
+        var name = "Cleanup.FailingFactory";
+        var failure = new InvalidOperationException("Factory configuration failed.");
+        ProviderFactories.RegisterFactory(name, () => throw failure);
+
+        var actual = Assert.Throws<InvalidOperationException>(() =>
+            DbProviderFactoriesHelper.GetFactory(name, "Missing.Assembly", "Missing.Factory"));
+
+        Assert.That(actual, Is.SameAs(failure));
+    }
+
+    [Test]
+    public void FactoryResolutionUsesCustomThenSystemThenReflection()
+    {
+        var name = "Cleanup.FactoryPrecedence." + Guid.NewGuid();
+        var assembly = typeof(SqliteFactory).Assembly.GetName().Name;
+        var type = typeof(SqliteFactory).FullName;
+        Assert.That(DbProviderFactoriesHelper.GetFactory(name, assembly, type), Is.SameAs(SqliteFactory.Instance));
+
+        System.Data.Common.DbProviderFactories.RegisterFactory(name, SqliteFactory.Instance);
+        try
+        {
+            Assert.That(DbProviderFactoriesHelper.GetFactory(name, "Missing.Assembly", "Missing.Factory"), Is.SameAs(SqliteFactory.Instance));
+            var custom = new TestFactory();
+            ProviderFactories.RegisterFactory(name, () => custom);
+            Assert.That(DbProviderFactoriesHelper.GetFactory(name, assembly, type), Is.SameAs(custom));
+        }
+        finally
+        {
+            System.Data.Common.DbProviderFactories.UnregisterFactory(name);
+        }
+    }
+
+    [Test]
+    [SetCulture("tr-TR")]
+    public void SqlTypeLookupDoesNotDependOnCurrentCulture()
+    {
+        var names = new TypeNames();
+        names.Put(DbType.Int32, "INTEGER");
+        names.Put(DbType.Int64, 20, "BIGINT");
+        names.PutAlias(DbType.Int16, "SMALLINT");
+        Assert.That(names.GetDbType("integer"), Is.EqualTo(DbType.Int32));
+        Assert.That(names.GetDbType("bigint"), Is.EqualTo(DbType.Int64));
+        Assert.That(names.GetDbType("smallint"), Is.EqualTo(DbType.Int16));
+    }
+
+    [Test]
+    public void DefaultTypeTemplatesExpandSizePrecisionAndScale()
+    {
+        var names = new TypeNames();
+        names.Put(DbType.String, "VARCHAR($l)");
+        names.Put(DbType.Decimal, "DECIMAL($p,$s)");
+        names.Put(DbType.Decimal, 5, "SMALLDECIMAL($p,$s)");
+
+        Assert.That(names.Get(DbType.String, 80, 0, 0), Is.EqualTo("VARCHAR(80)"));
+        Assert.That(names.Get(DbType.Decimal, 10, 18, 4), Is.EqualTo("DECIMAL(18,4)"));
+        Assert.That(names.Get(DbType.Decimal, 5, 8, 2), Is.EqualTo("SMALLDECIMAL(8,2)"));
+    }
+
+    private sealed class TestFactory : DbProviderFactory { }
+}

src/Migrator.Tests/FluentOperationsTests.cs

@@ -21,7 +21,7 @@ [Test] public void FileAndEmbeddedScriptsUseScriptOperations()
             builder.Execute.Script(path);
             builder.Execute.EmbeddedScript(typeof(ScriptTests).Assembly, "Migrator.Tests.ScriptResource.sql");
             Assert.That(builder.Build().All(x => x is ScriptOperation), Is.True);
-            var script = builder.Build().First().ToSql(new SqlGenerationContext(ProviderTypes.SqlServer));
+            var script = builder.Build()[0].ToSql(new SqlGenerationContext(ProviderTypes.SqlServer));
             Assert.That(script, Does.Contain("GO"));
             Assert.That(script.TrimEnd(), Does.EndWith("SELECT 2;"));
         }

src/Migrator.Tests/IdentifierAndTimeRegressionTests.cs

@@ -64,7 +64,7 @@ public void TimeDefaultsAndValuesSurviveSQLiteReconstruction()
         using var provider = ProviderFactory.Create(ProviderTypes.SQLite, "Data Source=:memory:", null);
         var time = new TimeOnly(12, 34, 56).Add(TimeSpan.FromTicks(1234560));
         provider.AddTable("Times", new Column("Id", DbType.Int32), new Column("Value", DbType.Time) { DefaultValue = time });
-        var column = provider.GetColumns("Times").Single(c => c.Name == "Value");
+        var column = provider.ReadLegacyColumns("Times").Single(c => c.Name == "Value");
         Assert.That(column.Type, Is.EqualTo(DbType.Time));
         Assert.That(column.DefaultValue, Is.EqualTo(time));
         provider.ChangeColumn("Times", new Column("Id", DbType.Int64));

src/Migrator.Tests/LegacyMetadata.cs

@@ -0,0 +1,19 @@
+using DotNetProjects.Migrator.Framework;
+
+namespace Migrator.Tests;
+
+// These tests deliberately verify the compatibility metadata API. Keep the
+// obsolete calls here so other accidental obsolete API usage still warns.
+internal static class LegacyMetadata
+{
+#pragma warning disable CS0618 // Explicit compatibility coverage of these three APIs.
+    internal static Column[] ReadLegacyColumns(this ITransformationProvider provider, string table) =>
+        provider.GetColumns(table);
+
+    internal static Column ReadLegacyColumn(this ITransformationProvider provider, string table, string column) =>
+        provider.GetColumnByName(table, column);
+
+    internal static void RemoveLegacyConstraints(this ITransformationProvider provider, string table) =>
+        provider.RemoveAllConstraints(table);
+#pragma warning restore CS0618
+}

src/Migrator.Tests/Providers/Generic/Generic_AddPrimaryKey.cs

@@ -61,8 +61,8 @@ public void AddPrimaryKey_AddPrimaryKey_ShouldStillBeNotNull()
         Provider.AddPrimaryKey(name: "MyPkName", table: tableName, columnName1);
 
         // Assert
-        var column1 = Provider.GetColumnByName(table: tableName, column: columnName1);
-        var column2 = Provider.GetColumnByName(table: tableName, column: columnName2);
+        var column1 = Provider.ReadLegacyColumn(table: tableName, column: columnName1);
+        var column2 = Provider.ReadLegacyColumn(table: tableName, column: columnName2);
 
         Assert.That(column1.IsNullable, Is.False);
         Assert.That(column2.IsNullable, Is.False);