Exception error cleanup (#24)

* feat(error): Adding non dbc file format error (extension based), Missing version header error added

Fixed the tests so that they now make .dbc files since we are checking the extensions.

* feat(error): Adding a missing bit timing error class

* feat(error): Adding lines we attempted to parse and more error messaging for the Exceptions

* test: Updating tests names to be more clear at what is being validated.

* feat(parsing): Adding a way to push and read all of the unused lines since we don't error out on unused

* fix(format): Running code format

* fix: Fixing clang-tidy warnings

Author: LinuxDevon

include/libdbc/dbc.hpp

@@ -31,6 +31,8 @@ class DbcParser : public Parser {
 
 	Message::ParseSignalsStatus parse_message(uint32_t message_id, const std::vector<uint8_t>& data, std::vector<double>& out_values);
 
+	std::vector<std::string> unused_lines() const;
+
 private:
 	std::string version;
 	std::vector<std::string> nodes;
@@ -44,9 +46,13 @@ class DbcParser : public Parser {
 	std::regex value_re;
 	std::regex signal_re;
 
+	std::vector<std::string> missed_lines;
+
 	void parse_dbc_header(std::istream& file_stream);
 	void parse_dbc_nodes(std::istream& file_stream);
 	void parse_dbc_messages(const std::vector<std::string>& lines);
+
+	static std::string get_extension(const std::string& file_name);
 };
 
 }

include/libdbc/exceptions/error.hpp

@@ -2,6 +2,7 @@
 #define ERROR_HPP
 
 #include <exception>
+#include <string>
 
 namespace Libdbc {
 
@@ -19,6 +20,49 @@ class ValidityError : public Exception {
 	}
 };
 
+class NonDbcFileFormatError : public ValidityError {
+public:
+	NonDbcFileFormatError(const std::string& path, const std::string& extension) {
+		error_msg = {"File is not of DBC format. Expected a .dbc extension. Cannot read this type of file (" + path + "). Found the extension (" + extension
+					 + ")."};
+	}
+
+	const char* what() const throw() override {
+		return error_msg.c_str();
+	}
+
+private:
+	std::string error_msg;
+};
+
+class DbcFileIsMissingVersion : public ValidityError {
+public:
+	DbcFileIsMissingVersion(const std::string& line) {
+		error_msg = {"Invalid dbc file. Missing the required version header. Attempting to read line: (" + line + ")."};
+	}
+
+	const char* what() const throw() override {
+		return error_msg.c_str();
+	}
+
+private:
+	std::string error_msg;
+};
+
+class DbcFileIsMissingBitTiming : public ValidityError {
+public:
+	DbcFileIsMissingBitTiming(const std::string& line) {
+		error_msg = {"Invalid dbc file. Missing required bit timing in the header. Attempting to read line: (" + line + ")."};
+	}
+
+	const char* what() const throw() override {
+		return error_msg.c_str();
+	}
+
+private:
+	std::string error_msg;
+};
+
 } // libdbc
 
 #endif // ERROR_HPP

src/dbc.cpp

@@ -1,4 +1,5 @@
 #include <cstdint>
+#include <cstdio>
 #include <fstream>
 #include <istream>
 #include <libdbc/dbc.hpp>
@@ -76,8 +77,12 @@ void DbcParser::parse_file(const std::string& file) {
 
 	messages.clear();
 
-	parse_dbc_header(stream);
+	auto extension = get_extension(file);
+	if (extension != ".dbc") {
+		throw NonDbcFileFormatError(file, extension);
+	}
 
+	parse_dbc_header(stream);
 	parse_dbc_nodes(stream);
 
 	while (!stream.eof()) {
@@ -88,6 +93,15 @@ void DbcParser::parse_file(const std::string& file) {
 	parse_dbc_messages(lines);
 }
 
+std::string DbcParser::get_extension(const std::string& file_name) {
+	std::size_t dot = file_name.find_last_of(".");
+	if (dot != std::string::npos) {
+		return file_name.substr(dot, file_name.size() - dot);
+	}
+
+	return "";
+}
+
 std::string DbcParser::get_version() const {
 	return version;
 }
@@ -116,7 +130,7 @@ void DbcParser::parse_dbc_header(std::istream& file_stream) {
 	Utils::StreamHandler::get_line(file_stream, line);
 
 	if (!std::regex_search(line, match, version_re)) {
-		throw ValidityError();
+		throw DbcFileIsMissingVersion(line);
 	}
 
 	version = match.str(2);
@@ -126,7 +140,7 @@ void DbcParser::parse_dbc_header(std::istream& file_stream) {
 	Utils::StreamHandler::get_next_non_blank_line(file_stream, line);
 
 	if (!std::regex_search(line, match, bit_timing_re)) {
-		throw ValidityError();
+		throw DbcFileIsMissingBitTiming(line);
 	}
 }
 
@@ -136,9 +150,7 @@ void DbcParser::parse_dbc_nodes(std::istream& file_stream) {
 
 	Utils::StreamHandler::get_next_non_blank_line(file_stream, line);
 
-	if (!std::regex_search(line, match, node_re)) {
-		throw ValidityError();
-	}
+	std::regex_search(line, match, node_re);
 
 	if (match.length() > 2) {
 		std::string node = match.str(2);
@@ -213,6 +225,10 @@ void DbcParser::parse_dbc_messages(const std::vector<std::string>& lines) {
 			signal_value.push_back(val);
 			continue;
 		}
+
+		if (line.length() > 0) {
+			missed_lines.push_back(line);
+		}
 	}
 
 	for (const auto& signal : signal_value) {
@@ -225,4 +241,8 @@ void DbcParser::parse_dbc_messages(const std::vector<std::string>& lines) {
 	}
 }
 
+std::vector<std::string> DbcParser::unused_lines() const {
+	return missed_lines;
+}
+
 }

test/dbcs/MissingVersion.dbc

@@ -1,36 +1,36 @@
-NS_ :
-	BA_
-	BA_DEF_
-	BA_DEF_DEF_
-	BA_DEF_DEF_REL_
-	BA_DEF_REL_
-	BA_DEF_SGTYPE_
-	BA_REL_
-	BA_SGTYPE_
-	BO_TX_BU_
-	BU_BO_REL_
-	BU_EV_REL_
-	BU_SG_REL_
-	CAT_
-	CAT_DEF_
-	CM_
-	ENVVAR_DATA_
-	EV_DATA_
-	FILTER
-	NS_DESC_
-	SGTYPE_
-	SGTYPE_VAL_
-	SG_MUL_VAL_
-	SIGTYPE_VALTYPE_
-	SIG_GROUP_
-	SIG_TYPE_REF_
-	SIG_VALTYPE_
-	VAL_
-	VAL_TABLE_
-
-BS_:
-
-BU_: DBG DRIVER IO MOTOR SENSOR
-
-BO_ 500 IO_DEBUG: 4 IO
- SG_ IO_DEBUG_test_unsigned : 0|8@1+ (1,0) [0|0] "" DBG
+NS_ :
+	BA_
+	BA_DEF_
+	BA_DEF_DEF_
+	BA_DEF_DEF_REL_
+	BA_DEF_REL_
+	BA_DEF_SGTYPE_
+	BA_REL_
+	BA_SGTYPE_
+	BO_TX_BU_
+	BU_BO_REL_
+	BU_EV_REL_
+	BU_SG_REL_
+	CAT_
+	CAT_DEF_
+	CM_
+	ENVVAR_DATA_
+	EV_DATA_
+	FILTER
+	NS_DESC_
+	SGTYPE_
+	SGTYPE_VAL_
+	SG_MUL_VAL_
+	SIGTYPE_VALTYPE_
+	SIG_GROUP_
+	SIG_TYPE_REF_
+	SIG_VALTYPE_
+	VAL_
+	VAL_TABLE_
+
+BS_:
+
+BU_: DBG DRIVER IO MOTOR SENSOR
+
+BO_ 500 IO_DEBUG: 4 IO
+ SG_ IO_DEBUG_test_unsigned : 0|8@1+ (1,0) [0|0] "" DBG

test/test_dbc.cpp

@@ -3,29 +3,35 @@
 #include <catch2/catch_approx.hpp>
 #include <catch2/catch_test_macros.hpp>
 #include <catch2/matchers/catch_matchers.hpp>
+#include <catch2/matchers/catch_matchers_string.hpp>
 #include <libdbc/dbc.hpp>
 #include <libdbc/exceptions/error.hpp>
 #include <string>
 
+using Catch::Matchers::ContainsSubstring;
+
 TEST_CASE("Testing dbc file loading error issues", "[fileio][error]") {
 	auto parser = std::unique_ptr<Libdbc::DbcParser>(new Libdbc::DbcParser());
 
 	SECTION("Loading a non dbc file should throw an error", "[error]") {
-		REQUIRE_THROWS_AS(parser->parse_file(TEXT_FILE), Libdbc::ValidityError);
+		REQUIRE_THROWS_AS(parser->parse_file(TEXT_FILE), Libdbc::NonDbcFileFormatError);
+		REQUIRE_THROWS_WITH(parser->parse_file(TEXT_FILE), ContainsSubstring("TextFile.txt"));
 	}
 
-	SECTION("Loading a dbc with bad headers throws an error", "[error]") {
-		REQUIRE_THROWS_AS(parser->parse_file(MISSING_VERSION_DBC_FILE), Libdbc::ValidityError);
+	SECTION("Loading a dbc with missing version header throws an error (VERSION)", "[error]") {
+		REQUIRE_THROWS_AS(parser->parse_file(MISSING_VERSION_DBC_FILE), Libdbc::DbcFileIsMissingVersion);
+		REQUIRE_THROWS_WITH(parser->parse_file(MISSING_VERSION_DBC_FILE), ContainsSubstring("line: (NS_ :)"));
 	}
 
 	SECTION("Loading a dbc without the required bit timing section (BS_:)", "[error]") {
-		REQUIRE_THROWS_AS(parser->parse_file(MISSING_BIT_TIMING_DBC_FILE), Libdbc::ValidityError);
+		REQUIRE_THROWS_AS(parser->parse_file(MISSING_BIT_TIMING_DBC_FILE), Libdbc::DbcFileIsMissingBitTiming);
+		REQUIRE_THROWS_WITH(parser->parse_file(MISSING_BIT_TIMING_DBC_FILE), ContainsSubstring("BU_: DBG DRIVER IO MOTOR SENSOR"));
 	}
 
 	SECTION("Loading a dbc with some missing namespace section tags (NS_ :)", "[error]") {
 		// Confusion about this type of error. it appears that the header isn't
 		// very well standardized for now we ignore this type of error.
-		CHECK_NOTHROW(parser->parse_file(MISSING_NEW_SYMBOLS_DBC_FILE));
+		REQUIRE_NOTHROW(parser->parse_file(MISSING_NEW_SYMBOLS_DBC_FILE));
 	}
 
 	SECTION("Verify that what() method is accessible for all exceptions", "[error]") {
@@ -240,7 +246,7 @@ VAL_ 123 State1 123 "Description 3" 0 "Description 4" ;)";
 	REQUIRE(signal2.value_descriptions.at(1).description == "Description 4");
 }
 
-TEST_CASE("Should parse DBC with empty BU_") {
+TEST_CASE("Should parse DBC with empty BU_", "[error][optional]") {
 	std::string contents = R"(VERSION ""
 
 
@@ -254,15 +260,52 @@ NS_ :
 BO_ 293 Msg1: 2 Vector__XXX
  SG_ Wert7 : 0|16@1- (1,0) [0|0] "" Vector__XXX
 
-BO_ 292 Msg2: 8 Vector__XXX
+BO_ 292 Msg2: 1 Vector__XXX
  SG_ Wert8 : 56|8@1- (1,0) [0|0] "" Vector__XXX
 )";
 	const auto filename = create_temporary_dbc_with(contents.c_str());
 
 	auto parser = Libdbc::DbcParser();
-	parser.parse_file(filename.c_str());
+	REQUIRE_NOTHROW(parser.parse_file(filename.c_str()));
 
 	REQUIRE(parser.get_messages().size() == 2);
 	REQUIRE(parser.get_messages().at(0).name() == "Msg1");
 	REQUIRE(parser.get_messages().at(1).name() == "Msg2");
 }
+
+TEST_CASE("Should report unused lines since we don't have tracing.", "[parsing]") {
+	std::string contents = R"(VERSION ""
+
+NS_ :
+
+BS_:
+
+BU_:
+
+
+BO_ 293 Msg1: 2 Vector__XXX
+ SG_ Whitespace: | 0|16@1- (1,0) [0|0] "" Vector__XXX
+ SG_ Wert7 : 0|16@1- (1,0) [0|0] "" Vector__XXX
+ SG_ Wert8 : 0|16@1- (1,0) [0|0] "" Vector__XXX
+
+BO_ 292 Msg2: 1 Vector__XXX
+ SG_ Wert8 : 56|8@1- (1,0) [0|0] "" Vector__XXX
+ SB_ not a correct line
+
+BO_ have a issue here:
+)";
+
+	const auto filename = create_temporary_dbc_with(contents.c_str());
+
+	auto parser = Libdbc::DbcParser();
+	REQUIRE_NOTHROW(parser.parse_file(filename.c_str()));
+
+	REQUIRE(parser.get_messages().size() == 2);
+	REQUIRE(parser.get_messages()[0].size() == 2);
+	REQUIRE(parser.get_messages()[1].size() == 1);
+
+	auto unused = parser.unused_lines();
+
+	// We could match them all here but i think just a check that the size is sufficent.
+	REQUIRE(unused.size() == 3);
+}

test/testing_utils/common.cpp

@@ -23,7 +23,7 @@ std::string generate_unique_filename() {
 	int random_num = dis(gen);
 
 	// Concatenate time and random number to create a unique filename
-	return "temp_file_" + std::to_string(milliseconds) + "_" + std::to_string(random_num) + ".txt";
+	return "temp_file_" + std::to_string(milliseconds) + "_" + std::to_string(random_num) + ".dbc";
 }
 
 std::string create_temporary_dbc_with(const char* contents) {