Skip to content

Commit da47294

Browse files
Address CI feedback: docs, contains(), designated initializers
- Add Doxygen comments for new public members (HardResetResult, UnicodeUpgradeResult, ColumnUpgradeEntry, PlanFoldingResult inner structs, ColumnDiff, TableDiff, RowDiff, TableDataDiff, DiffProgressEvent, DataSourceInfo, DriverInfo, SecretResolver move ops, MigrationRenderContext::ColumnKey/TableKey, SplitFileWriter CodeBlock). - Replace `\ref` with backticks in SqlSchemaDiff/SqlDataDiff comments so doxygen stops complaining about unresolved references. - Reword `@overload` directive in MigrationPlan.hpp so doxygen does not interpret the trailing words as a symbol name. - Move `@copydoc` recursion in SqlStatement.hpp by giving the primary declarations their own brief docstrings. - Exclude `src/Lightweight/tui/` from doxygen — vendored code that follows the upstream comment style. - Migrate `.find(x) != std::string::npos` to `.contains(x)` (and the inverse) across all PR-introduced code, satisfying clang-tidy 22's `readability-container-contains` check. - Convert positional aggregate initializers to designated form for `StatementWithComments` and `MigrationRenderContext::ColumnKey`, satisfying `modernize-use-designated-initializers`. No behavioral changes — all transformations are mechanical. Signed-off-by: Christian Parpart <christian@parpart.family>
1 parent fafbe3d commit da47294

13 files changed

Lines changed: 101 additions & 60 deletions

src/Lightweight/CodeGen/SplitFileWriter.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ namespace Lightweight::CodeGen
2222
/// migrations.
2323
struct CodeBlock
2424
{
25+
/// Rendered text of the block, ready to be written verbatim into an output file.
2526
std::string content;
27+
/// Cached newline count for `content`; reused across packing passes.
2628
std::size_t lineCount = 0;
2729
};
2830

src/Lightweight/Odbc/DataSourceEnumerator.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@ struct DataSourceInfo
3939
User,
4040
System,
4141
};
42+
/// Whether the DSN is user-private or registered system-wide.
4243
Scope scope = Scope::User;
4344

45+
/// Total ordering on (name, description, scope) — defaulted three-way comparison.
4446
auto operator<=>(DataSourceInfo const&) const noexcept = default;
4547
};
4648

@@ -54,6 +56,7 @@ struct DriverInfo
5456
/// Typical keys include "FileUsage", "APILevel", "ConnectFunctions".
5557
std::vector<std::pair<std::string, std::string>> attributes;
5658

59+
/// Total ordering on (name, attributes) — defaulted three-way comparison.
5760
auto operator<=>(DriverInfo const&) const noexcept = default;
5861
};
5962

src/Lightweight/Secrets/SecretResolver.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ class LIGHTWEIGHT_API SecretResolver
4646
SecretResolver() = default;
4747
~SecretResolver() = default;
4848
SecretResolver(SecretResolver const&) = delete;
49+
/// Move-construct from another resolver, taking ownership of its backend chain.
4950
SecretResolver(SecretResolver&&) = default;
5051
SecretResolver& operator=(SecretResolver const&) = delete;
52+
/// Move-assign from another resolver, taking ownership of its backend chain.
5153
SecretResolver& operator=(SecretResolver&&) = default;
5254

5355
/// Appends a backend to the chain. The backend's `Name()` is used both as

src/Lightweight/SqlMigration.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,8 @@ namespace
960960
auto const width = CharacterWidthFromDataType(dataType, static_cast<std::size_t>(*maxLengthOpt));
961961
if (width.value == 0)
962962
continue;
963-
ctx.columnWidths[{ std::string(schema), std::string(table), columnName }] = width;
963+
ctx.columnWidths[MigrationRenderContext::ColumnKey {
964+
.schema = std::string(schema), .table = std::string(table), .column = columnName }] = width;
964965
}
965966
}
966967
catch (SqlException const&) // NOLINT(bugprone-empty-catch) — see function comment

src/Lightweight/SqlMigration.hpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,13 @@ namespace SqlMigration
398398
/// Per-table state: ordered column declarations + per-table FK list.
399399
struct TableState
400400
{
401+
/// Ordered column declarations as they would be emitted in CREATE TABLE.
401402
std::vector<SqlColumnDeclaration> columns;
403+
/// Composite foreign keys declared on this table (single-column FKs are
404+
/// carried inline on the corresponding `SqlColumnDeclaration::foreignKey`).
402405
std::vector<SqlCompositeForeignKeyConstraint> compositeForeignKeys;
406+
/// Whether the original CREATE TABLE used IF NOT EXISTS (carried through
407+
/// to emitters so they can replay it verbatim).
403408
bool ifNotExists = false;
404409
};
405410

@@ -417,8 +422,11 @@ namespace SqlMigration
417422
/// One data step (INSERT/UPDATE/DELETE/RawSql) tagged with its source migration.
418423
struct DataStep
419424
{
425+
/// Timestamp of the migration that contributed this data step.
420426
MigrationTimestamp sourceTimestamp;
427+
/// Title of the migration that contributed this data step.
421428
std::string sourceTitle;
429+
/// The plan element itself (INSERT, UPDATE, DELETE, or RawSql).
422430
SqlMigrationPlanElement element;
423431
};
424432

@@ -455,6 +463,8 @@ namespace SqlMigration
455463
/// @brief Result of a `HardReset` call.
456464
struct HardResetResult
457465
{
466+
/// True when the populating call was a dry-run; the result describes the
467+
/// would-be plan and no DDL was issued.
458468
bool wasDryRun = false;
459469
/// Tables the registered migrations *would* have created and were also present
460470
/// in the live DB — these were dropped (or would be dropped on a real run).
@@ -488,18 +498,23 @@ namespace SqlMigration
488498
/// @brief One column upgrade entry in `UnicodeUpgradeResult`.
489499
struct ColumnUpgradeEntry
490500
{
501+
/// Fully-qualified name of the table that owns the column.
491502
SqlSchema::FullyQualifiedTableName table;
503+
/// Name of the column being upgraded.
492504
std::string column;
493505
/// Live byte-counted type the column currently has.
494506
SqlColumnTypeDefinition liveType;
495507
/// Char-counted type the migrations now declare for this column.
496508
SqlColumnTypeDefinition intendedType;
509+
/// Whether the column is nullable (preserved across the type rewrite).
497510
bool nullable = true;
498511
};
499512

500513
/// @brief Result of an `UnicodeUpgradeTables` call.
501514
struct UnicodeUpgradeResult
502515
{
516+
/// True when the populating call was a dry-run; the result describes the
517+
/// would-be diff and no DDL was issued.
503518
bool wasDryRun = false;
504519
/// Columns whose live type drifted from the intended type and were upgraded
505520
/// (or would be on a real run).

src/Lightweight/SqlQuery/MigrationPlan.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,8 @@ namespace
386386

387387
for (auto& [columnName, value]: columns)
388388
{
389-
auto const it = context.columnWidths.find({ schemaName, tableName, columnName });
389+
auto const it = context.columnWidths.find(
390+
MigrationRenderContext::ColumnKey { .schema = schemaName, .table = tableName, .column = columnName });
390391
if (it == context.columnWidths.end())
391392
continue;
392393
std::size_t originalSize = 0;

src/Lightweight/SqlQuery/MigrationPlan.hpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -584,9 +584,10 @@ struct [[nodiscard]] MigrationRenderContext
584584
/// truncation layer leaves the value alone.
585585
struct ColumnKey
586586
{
587-
std::string schema;
588-
std::string table;
589-
std::string column;
587+
std::string schema; ///< Schema label (empty for engines without schemas, e.g. SQLite).
588+
std::string table; ///< Table name.
589+
std::string column; ///< Column name.
590+
/// Total ordering on (schema, table, column) — defaulted three-way comparison.
590591
auto operator<=>(ColumnKey const&) const = default;
591592
};
592593

@@ -606,6 +607,7 @@ struct [[nodiscard]] MigrationRenderContext
606607
WidthUnit unit { WidthUnit::Characters };
607608
};
608609

610+
/// Cache of declared character widths, populated lazily as plan elements render.
609611
std::map<ColumnKey, ColumnWidth> columnWidths;
610612

611613
/// @brief Optional fallback that fetches column widths for a `(schema, table)` from
@@ -624,10 +626,12 @@ struct [[nodiscard]] MigrationRenderContext
624626
/// re-query for every INSERT against the same table.
625627
struct TableKey
626628
{
627-
std::string schema;
628-
std::string table;
629+
std::string schema; ///< Schema label (empty when the engine reports none).
630+
std::string table; ///< Table name.
631+
/// Total ordering on (schema, table) — defaulted three-way comparison.
629632
auto operator<=>(TableKey const&) const = default;
630633
};
634+
/// Tables for which `widthLookup` has already been called; prevents repeat queries.
631635
std::set<TableKey> lookupAttempted;
632636

633637
/// @brief Identity of the migration currently being rendered. Set by
@@ -636,6 +640,7 @@ struct [[nodiscard]] MigrationRenderContext
636640
/// offending migration. Empty / zero when rendering happens outside a migration
637641
/// run (e.g. unit tests calling `ToSql` directly).
638642
std::string activeMigrationTitle;
643+
/// Timestamp of the migration currently being rendered (0 outside a migration run).
639644
std::uint64_t activeMigrationTimestamp = 0;
640645
};
641646

@@ -650,7 +655,8 @@ struct [[nodiscard]] MigrationRenderContext
650655
[[nodiscard]] LIGHTWEIGHT_API std::vector<std::string> ToSql(SqlQueryFormatter const& formatter,
651656
SqlMigrationPlanElement const& element);
652657

653-
/// @overload Rendering variant that consults a `MigrationRenderContext` for compat flags
658+
/// @overload
659+
/// Rendering variant that consults a `MigrationRenderContext` for compat flags
654660
/// and column-width tracking. Updates `context` in place when the rendered step declares
655661
/// new columns.
656662
[[nodiscard]] LIGHTWEIGHT_API std::vector<std::string> ToSql(SqlQueryFormatter const& formatter,

src/Lightweight/SqlStatement.hpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,24 @@ class [[nodiscard]] SqlStatement final: public SqlDataBinderCallback
224224
std::source_location location = std::source_location::current()) noexcept;
225225
void CloseCursor() noexcept;
226226

227+
/// @brief Binds the given output column variables to the result columns of this statement.
228+
/// @tparam Args ODBC-bindable output column types.
229+
/// @param args Pointers to caller-owned storage for each result column, in order.
227230
template <SqlOutputColumnBinder... Args>
228231
void BindOutputColumns(Args*... args);
229232

233+
/// @brief Binds the members of @p records to the result columns of this statement
234+
/// in declaration order, via reflection.
235+
/// @tparam Records Aggregate record types whose members map to result columns.
236+
/// @param records Pointers to caller-owned record instances.
230237
template <typename... Records>
231238
requires(((std::is_class_v<Records> && std::is_aggregate_v<Records>) && ...))
232239
void BindOutputColumnsToRecord(Records*... records);
233240

241+
/// @brief Binds a single output column variable to the result column at @p columnIndex.
242+
/// @tparam T An ODBC-bindable output column type.
243+
/// @param columnIndex 1-based result column index.
244+
/// @param arg Pointer to caller-owned storage for the column value.
234245
template <SqlOutputColumnBinder T>
235246
void BindOutputColumn(SQLUSMALLINT columnIndex, T* arg);
236247

@@ -637,7 +648,7 @@ inline LIGHTWEIGHT_FORCE_INLINE std::string const& SqlStatement::PreparedQuery()
637648
return m_preparedQuery;
638649
}
639650

640-
/// @copydoc SqlStatement::BindOutputColumns
651+
/// @brief Out-of-line definition of `SqlStatement::BindOutputColumns`.
641652
template <SqlOutputColumnBinder... Args>
642653
inline LIGHTWEIGHT_FORCE_INLINE void SqlStatement::BindOutputColumns(Args*... args)
643654
{
@@ -663,7 +674,7 @@ void SqlStatement::BindOutputColumnsToRecord(Records*... records)
663674
...);
664675
}
665676

666-
/// @copydoc SqlStatement::BindOutputColumn
677+
/// @brief Out-of-line definition of `SqlStatement::BindOutputColumn`.
667678
template <SqlOutputColumnBinder T>
668679
inline LIGHTWEIGHT_FORCE_INLINE void SqlStatement::BindOutputColumn(SQLUSMALLINT columnIndex, T* arg)
669680
{

src/tests/ConfigProfileStoreTests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ TEST_CASE("ProfileStore — malformed YAML returns a readable error", "[ProfileS
178178
ScopedTempYaml const yaml("profiles:\n broken: [not-a-map]\n");
179179
auto const result = Lightweight::Config::ProfileStore::LoadOrDefault(yaml.Path());
180180
REQUIRE_FALSE(result.has_value());
181-
CHECK(result.error().find("broken") != std::string::npos);
181+
CHECK(result.error().contains("broken"));
182182
}
183183

184184
TEST_CASE("ProfileStore — dsn and connectionString are mutually exclusive", "[ProfileStore]")
@@ -190,8 +190,8 @@ TEST_CASE("ProfileStore — dsn and connectionString are mutually exclusive", "[
190190
)");
191191
auto const result = Lightweight::Config::ProfileStore::LoadOrDefault(yaml.Path());
192192
REQUIRE_FALSE(result.has_value());
193-
CHECK(result.error().find("dsn") != std::string::npos);
194-
CHECK(result.error().find("connectionString") != std::string::npos);
193+
CHECK(result.error().contains("dsn"));
194+
CHECK(result.error().contains("connectionString"));
195195
}
196196

197197
TEST_CASE("ProfileStore — Remove clears default when it points at the removed profile", "[ProfileStore]")

src/tests/Lup2DbtoolTests.cpp

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ TEST_CASE("ResolvePositionalInserts substitutes real column names", "[lup2dbtool
295295
c.tableName = "cfg";
296296
c.columns.push_back({ .name = "nr", .type = "integer" });
297297
c.columns.push_back({ .name = "val", .type = "integer" });
298-
m1.statements.push_back({ {}, c });
298+
m1.statements.push_back({ .comments = {}, .statement = c });
299299
}
300300

301301
ParsedMigration m2;
@@ -306,7 +306,7 @@ TEST_CASE("ResolvePositionalInserts substitutes real column names", "[lup2dbtool
306306
i.tableName = "cfg";
307307
i.columnValues.emplace_back("0", "4");
308308
i.columnValues.emplace_back("1", "215");
309-
m2.statements.push_back({ {}, i });
309+
m2.statements.push_back({ .comments = {}, .statement = i });
310310
}
311311

312312
std::vector migrations { m1, m2 };
@@ -328,21 +328,21 @@ TEST_CASE("ResolvePositionalInserts tracks ALTER TABLE ADD COLUMN", "[lup2dbtool
328328
CreateTableStmt c;
329329
c.tableName = "t";
330330
c.columns.push_back({ .name = "id", .type = "integer" });
331-
m.statements.push_back({ {}, c });
331+
m.statements.push_back({ .comments = {}, .statement = c });
332332
}
333333
{
334334
AlterTableAddColumnStmt a;
335335
a.tableName = "t";
336336
a.column.name = "extra";
337337
a.column.type = "integer";
338-
m.statements.push_back({ {}, a });
338+
m.statements.push_back({ .comments = {}, .statement = a });
339339
}
340340
{
341341
InsertStmt i;
342342
i.tableName = "t";
343343
i.columnValues.emplace_back("0", "1");
344344
i.columnValues.emplace_back("1", "99");
345-
m.statements.push_back({ {}, i });
345+
m.statements.push_back({ .comments = {}, .statement = i });
346346
}
347347

348348
std::vector migrations { m };
@@ -361,14 +361,14 @@ TEST_CASE("ResolvePositionalInserts leaves out-of-bounds indices untouched", "[l
361361
CreateTableStmt c;
362362
c.tableName = "t";
363363
c.columns.push_back({ .name = "id", .type = "integer" });
364-
m.statements.push_back({ {}, c });
364+
m.statements.push_back({ .comments = {}, .statement = c });
365365
}
366366
{
367367
InsertStmt i;
368368
i.tableName = "t";
369369
i.columnValues.emplace_back("0", "1");
370370
i.columnValues.emplace_back("1", "bogus"); // Table has only 1 column.
371-
m.statements.push_back({ {}, i });
371+
m.statements.push_back({ .comments = {}, .statement = i });
372372
}
373373

374374
std::vector migrations { m };
@@ -525,8 +525,8 @@ TEST_CASE("ParseSqlStatement.Update.ExistsSubqueryIsStructured", "[lup2dbtool]")
525525
REQUIRE(std::holds_alternative<UpdateStmt>(stmt));
526526
auto const& u = std::get<UpdateStmt>(stmt);
527527
CHECK(u.tableName == "docs");
528-
CHECK(u.whereExpression.find("EXISTS (") != std::string::npos);
529-
CHECK(u.whereExpression.find(R"("FOLDER" IS NULL)") != std::string::npos);
528+
CHECK(u.whereExpression.contains("EXISTS ("));
529+
CHECK(u.whereExpression.contains(R"("FOLDER" IS NULL)"));
530530
}
531531

532532
TEST_CASE("ParseSqlStatement.Update.WhereIsNull", "[lup2dbtool]")
@@ -913,24 +913,24 @@ TEST_CASE("GeneratePluginCMake.UsesPluginName", "[lup2dbtool]")
913913
auto const script = out.str();
914914

915915
// Plugin name appears everywhere a target name would: add_library, GLOB variable, link, properties.
916-
CHECK(script.find("add_library(MyMigrations MODULE") != std::string::npos);
917-
CHECK(script.find("target_link_libraries(MyMigrations PRIVATE Lightweight::Lightweight") != std::string::npos);
918-
CHECK(script.find("${MyMigrations_MIGRATIONS}") != std::string::npos);
916+
CHECK(script.contains("add_library(MyMigrations MODULE"));
917+
CHECK(script.contains("target_link_libraries(MyMigrations PRIVATE Lightweight::Lightweight"));
918+
CHECK(script.contains("${MyMigrations_MIGRATIONS}"));
919919
// Plugin.cpp is the entry point the script must compile alongside the generated sources.
920-
CHECK(script.find("Plugin.cpp") != std::string::npos);
920+
CHECK(script.contains("Plugin.cpp"));
921921
// CONFIGURE_DEPENDS glob keeps lup_*.cpp additions cheap.
922-
CHECK(script.find("CONFIGURE_DEPENDS") != std::string::npos);
923-
CHECK(script.find("lup_*.cpp") != std::string::npos);
922+
CHECK(script.contains("CONFIGURE_DEPENDS"));
923+
CHECK(script.contains("lup_*.cpp"));
924924
// Output destination matches the existing plugin convention.
925-
CHECK(script.find("${CMAKE_BINARY_DIR}/plugins") != std::string::npos);
925+
CHECK(script.contains("${CMAKE_BINARY_DIR}/plugins"));
926926
}
927927

928928
TEST_CASE("GeneratePluginCMake.DefaultPluginName", "[lup2dbtool]")
929929
{
930930
std::ostringstream out;
931931
CodeGenerator::GeneratePluginCMake(out, "LupMigrations");
932932
auto const script = out.str();
933-
CHECK(script.find("add_library(LupMigrations MODULE") != std::string::npos);
933+
CHECK(script.contains("add_library(LupMigrations MODULE"));
934934
}
935935

936936
TEST_CASE("GeneratePluginEntryPoint.DeclaresPluginMacro", "[lup2dbtool]")
@@ -939,8 +939,8 @@ TEST_CASE("GeneratePluginEntryPoint.DeclaresPluginMacro", "[lup2dbtool]")
939939
CodeGenerator::GeneratePluginEntryPoint(out);
940940
auto const src = out.str();
941941

942-
CHECK(src.find("#include <Lightweight/SqlMigration.hpp>") != std::string::npos);
943-
CHECK(src.find("LIGHTWEIGHT_MIGRATION_PLUGIN()") != std::string::npos);
942+
CHECK(src.contains("#include <Lightweight/SqlMigration.hpp>"));
943+
CHECK(src.contains("LIGHTWEIGHT_MIGRATION_PLUGIN()"));
944944
}
945945

946946
// ================================================================================================
@@ -1019,7 +1019,7 @@ TEST_CASE("ParseSqlFile.AutoMode.PassesThroughUtf8", "[lup2dbtool]")
10191019
// The byte payload should still contain the original UTF-8 bytes, untouched.
10201020
bool foundUtf8 = false;
10211021
for (auto const& [_, value]: stmt.columnValues)
1022-
if (value.find("\xC3\xA4") != std::string::npos)
1022+
if (value.contains("\xC3\xA4"))
10231023
foundUtf8 = true;
10241024
CHECK(foundUtf8);
10251025
RemoveQuiet(path);
@@ -1040,9 +1040,9 @@ TEST_CASE("ParseSqlFile.AutoMode.ConvertsWindows1252", "[lup2dbtool]")
10401040
bool foundLatin1 = false;
10411041
for (auto const& [_, value]: stmt.columnValues)
10421042
{
1043-
if (value.find("\xC3\xA4") != std::string::npos)
1043+
if (value.contains("\xC3\xA4"))
10441044
foundUtf8 = true;
1045-
if (value.find('\xE4') != std::string::npos && value.find("\xC3\xA4") == std::string::npos)
1045+
if (value.contains('\xE4') && !value.contains("\xC3\xA4"))
10461046
foundLatin1 = true;
10471047
}
10481048
CHECK(foundUtf8);
@@ -1072,8 +1072,8 @@ TEST_CASE("ParseSqlFile.ExplicitUtf8RejectsWindows1252", "[lup2dbtool]")
10721072
auto const result = ParseSqlFile(path, ParserConfig { .inputEncoding = "utf-8" });
10731073
REQUIRE_FALSE(result.has_value());
10741074
INFO("Error message: " << result.error());
1075-
CHECK(result.error().find("invalid UTF-8") != std::string::npos);
1076-
CHECK(result.error().find("utf-8") != std::string::npos);
1075+
CHECK(result.error().contains("invalid UTF-8"));
1076+
CHECK(result.error().contains("utf-8"));
10771077
RemoveQuiet(path);
10781078
}
10791079

@@ -1085,7 +1085,7 @@ TEST_CASE("ParseSqlFile.ExplicitWindows1252RejectsUtf8", "[lup2dbtool]")
10851085
auto const path = WriteTempSqlFile("9_99_05", "INSERT INTO T VALUES (1, 'M\xC3\xA4ller')\n");
10861086
auto const result = ParseSqlFile(path, ParserConfig { .inputEncoding = "windows-1252" });
10871087
REQUIRE_FALSE(result.has_value());
1088-
CHECK(result.error().find("validates as UTF-8") != std::string::npos);
1088+
CHECK(result.error().contains("validates as UTF-8"));
10891089
RemoveQuiet(path);
10901090
}
10911091

0 commit comments

Comments
 (0)