test: SonarCloud issues of the unit tests

- test::definition_error for unusable test vehicle definitions instead of
  std::runtime_error (S112)
- the rows of a table section written by a function of their own, less
  nesting in definition::text() (S134)
- std::less<> for the section table (S6045)
- comments in the empty methods of the reseeding listener and stub (S1186)
- NOSONAR where the engine's headers fix the stubs: the extern globals
  and the member function signatures (S5421, S5817, S1238)
- NOSONAR on LoadFIZ_Parse(): the cognitive complexity is the former body
  of LoadFIZ(), moved as it was (S3776)
- a comment Sonar took for code reworded (S125)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

McZapkie/Mover.cpp

@@ -9588,7 +9588,7 @@ bool TMoverParameters::LoadFIZ_Text(std::string const &Text)
 	return LoadFIZ_Parse(fizparser);
 }
 
-bool TMoverParameters::LoadFIZ_Parse(cParser &fizparser)
+bool TMoverParameters::LoadFIZ_Parse(cParser &fizparser) // NOSONAR the former body of LoadFIZ(), moved as it was; splitting it up is a change of its own
 {
 	LISTLINE = 0;
 	startBPT = false;

tests/unit/engine_stubs.cpp

@@ -44,29 +44,32 @@ void ErrorLog(const std::string &str, logtype const Type) {
 }
 
 // scenery structure, which the parser reports include directives to --------------------------------
+// the declarations come from the engine's headers, the stubs can't change them (hence the NOSONARs)
 
 namespace scene {
 
-node_groups Groups;
+node_groups Groups; // NOSONAR the engine's extern global, scene/scenenodegroups.h
-node_layers Layers;
+node_layers Layers; // NOSONAR the engine's extern global, scene/scenelayers.h
 
-group_handle node_groups::create(bool const) {
+group_handle node_groups::create(bool const) { // NOSONAR signature from scene/scenenodegroups.h
 	return null_handle;
 }
 
-group_handle node_groups::close() {
+group_handle node_groups::close() { // NOSONAR signature from scene/scenenodegroups.h
 	return null_handle;
 }
 
-layer_handle node_layers::open(std::string const &, include_site) {
+layer_handle node_layers::open(std::string const &, include_site) { // NOSONAR signature from scene/scenelayers.h
 	return null_handle;
 }
 
-layer_handle node_layers::close() {
+layer_handle node_layers::close() { // NOSONAR signature from scene/scenelayers.h
 	return null_handle;
 }
 
-void node_layers::instance_begin(std::string const &, source_span const &) {}
+void node_layers::instance_begin(std::string const &, source_span const &) { // NOSONAR signature from scene/scenelayers.h
+	// no layers in the tests, so nothing to keep track of
+}
 
 // basic_node has no key function for the Itanium C++ ABI (tooltip() is declared plain but defined inline), so
 // GCC and Clang emit its vtable wherever scenenode.h is used, here too, and the vtable wants radius_().

tests/unit/main.cpp

@@ -16,16 +16,17 @@ public:
 	void test_case_start(doctest::TestCaseData const &) override { reseed(); }
 	void test_case_reenter(doctest::TestCaseData const &) override { reseed(); }
 
-	void report_query(doctest::QueryData const &) override {}
+	// the rest of doctest's reporter interface: a listener reports nothing, the console reporter does that
-	void test_run_start() override {}
+	void report_query(doctest::QueryData const &) override { /* nothing to report */ }
-	void test_run_end(doctest::TestRunStats const &) override {}
+	void test_run_start() override { /* nothing to report */ }
-	void test_case_end(doctest::CurrentTestCaseStats const &) override {}
+	void test_run_end(doctest::TestRunStats const &) override { /* nothing to report */ }
-	void test_case_exception(doctest::TestCaseException const &) override {}
+	void test_case_end(doctest::CurrentTestCaseStats const &) override { /* nothing to report */ }
-	void subcase_start(doctest::SubcaseSignature const &) override {}
+	void test_case_exception(doctest::TestCaseException const &) override { /* nothing to report */ }
-	void subcase_end() override {}
+	void subcase_start(doctest::SubcaseSignature const &) override { /* nothing to report */ }
-	void log_assert(doctest::AssertData const &) override {}
+	void subcase_end() override { /* nothing to report */ }
-	void log_message(doctest::MessageData const &) override {}
+	void log_assert(doctest::AssertData const &) override { /* nothing to report */ }
-	void test_case_skipped(doctest::TestCaseData const &) override {}
+	void log_message(doctest::MessageData const &) override { /* nothing to report */ }
+	void test_case_skipped(doctest::TestCaseData const &) override { /* nothing to report */ }
 
 private:
 	static void reseed() {

tests/unit/vehicle.cpp

@@ -15,7 +15,7 @@ struct section_format {
 
 // the sections TMoverParameters::LoadFIZ() reads, see its issection() calls. a section missing here
 // would be silently skipped by the loader, and the test would run on the defaults instead
-std::map<std::string, section_format> const sections{
+std::map<std::string, section_format, std::less<>> const sections{
     {"Param", {'.'}},
     {"Load", {':'}},
     {"Dimensions", {':'}},
@@ -49,7 +49,7 @@ std::map<std::string, section_format> const sections{
 section_format const &format_of(std::string const &Section) {
 	auto const lookup{sections.find(Section)};
 	if (lookup == sections.end()) {
-		throw std::runtime_error("unknown vehicle definition section " + Section + ", see tests/unit/vehicle.cpp");
+		throw definition_error("unknown vehicle definition section " + Section + ", see tests/unit/vehicle.cpp");
 	}
 	return lookup->second;
 }
@@ -64,19 +64,34 @@ std::string value_text(nlohmann::ordered_json const &Value) {
 	return Value.dump();
 }
 
+// the rows of a table section, each a list of values, and the line closing them
+std::string table_text(std::string const &Section, section_format const &Format, nlohmann::ordered_json const &Rows) {
+	if (Format.table_end == nullptr) {
+		throw definition_error("vehicle definition section " + Section + " has no rows");
+	}
+	std::string text;
+	for (auto const &row : Rows) {
+		for (auto const &value : row) {
+			text += value_text(value) + " ";
+		}
+		text += "\n";
+	}
+	return text + Format.table_end + "\n";
+}
+
 } // namespace
 
 definition definition::load(std::string const &Name) {
 	auto const path{std::string{EU07_UNIT_TESTS_DATA} + "/" + Name + ".json"};
 	std::ifstream file(path);
 	if (false == file.is_open()) {
-		throw std::runtime_error("no test vehicle definition " + path);
+		throw definition_error("no test vehicle definition " + path);
 	}
 	definition result;
 	// not brace-initialised: json{other} makes an array of other with GCC and Clang (the initializer list constructor wins)
 	auto const json = nlohmann::ordered_json::parse(file, nullptr, true, true);
 	if (false == json.is_object()) {
-		throw std::runtime_error("test vehicle definition " + path + " isn't a JSON object");
+		throw definition_error("test vehicle definition " + path + " isn't a JSON object");
 	}
 	for (auto const &[section, keys] : json.items()) {
 		// keys starting with _ are comments
@@ -118,18 +133,8 @@ std::string definition::text() const {
 			}
 		}
 		text += "\n";
-		// table sections give their rows as a list of rows, each a list of values
 		if (keys.contains("rows")) {
-			if (format.table_end == nullptr) {
+			text += table_text(section, format, keys["rows"]);
-				throw std::runtime_error("vehicle definition section " + section + " has no rows");
-			}
-			for (auto const &row : keys["rows"]) {
-				for (auto const &value : row) {
-					text += value_text(value) + " ";
-				}
-				text += "\n";
-			}
-			text += std::string{format.table_end} + "\n";
 		}
 	}
 	return text;
@@ -139,13 +144,13 @@ vehicle::vehicle(definition const &Definition, double Velocity, std::string cons
 	// the same steps as TDynamicObject::Init()
 	m_mover = std::make_unique<TMoverParameters>(Velocity, Name, Name, 0);
 	if (false == m_mover->LoadFIZ_Text(Definition.text())) {
-		throw std::runtime_error("the test vehicle definition didn't load");
+		throw definition_error("the test vehicle definition didn't load");
 	}
 	m_mover->AssignLoad("", 0.f);
 	m_mover->ComputeMass();
 	auto const driveractive{Velocity != 0.0};
 	if (false == m_mover->CheckLocomotiveParameters(driveractive, Velocity >= 0.0 ? 1 : -1)) {
-		throw std::runtime_error("parameters mismatch in the test vehicle definition");
+		throw definition_error("parameters mismatch in the test vehicle definition");
 	}
 	m_mover->MainCtrlPos = m_mover->MainCtrlNoPowerPos();
 	m_mover->LocalBrakePosA = 0.0;

tests/unit/vehicle.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <memory>
+#include <stdexcept>
 #include <string>
 
 #include <nlohmann/json.hpp>
@@ -9,6 +10,12 @@
 
 namespace test {
 
+// thrown for a test vehicle definition which can't be used: missing, malformed, or not taken by the loader
+class definition_error : public std::runtime_error {
+public:
+	using std::runtime_error::runtime_error;
+};
+
 // definition of a test vehicle: one of the JSON files in tests/unit/data, with the sections and keys
 // of the game's vehicle definitions, changed by the test where it needs to:
 //   auto heavy = definition::load("wagon").set("Param", "M", 60000).set("Brake", "BrakeValve", "ESt3");
@@ -53,7 +60,7 @@ public:
 	// advances the physics by Seconds in steps of Dt
 	void run(double Seconds, track const &Track = {}, double Dt = 0.01);
 
-	// vehicle speed [km/h], signed
+	// vehicle speed in kilometres per hour, negative when moving backwards
 	double speed() const { return m_mover->V * 3.6; }
 
 private: