Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include file path and line number in parsing errors #512

Merged
merged 23 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
938edc0
Simple line number printout for element
aaronchongth Mar 5, 2021
9143ff9
Passing source into readXml, errors in include tags
aaronchongth Mar 8, 2021
efc97fe
Added helper function for creating trace strings
aaronchongth Mar 8, 2021
1e4d1a6
Fixing bad calls to elements in includes
aaronchongth Mar 8, 2021
fe0829c
Fixed bad logic in file exists check
aaronchongth Mar 8, 2021
38db358
Pimpl-ized Error, added API for file path and line number
aaronchongth Mar 12, 2021
618b576
Started using new Error API, removed previous helper functions
aaronchongth Mar 12, 2021
95470f3
Modified error stream to look like before, using << operator for cmdC…
aaronchongth Mar 15, 2021
09e3c2b
Added tests to check for file tracing and line number
aaronchongth Mar 15, 2021
afa7e5d
Added Error constructor without LineNumber for missing elements
aaronchongth Mar 15, 2021
b9605a1
Added tests for parsing errors includes, test sdfs and models
aaronchongth Mar 15, 2021
f273dbe
code_check fixes
aaronchongth Mar 15, 2021
8213b05
Removed accidental + char
aaronchongth Mar 15, 2021
1349ac9
Removed accidental + char
aaronchongth Mar 15, 2021
648d0dd
Merge branch 'aaron/trace_parsing_failure' of ssh://github.com/osrf/s…
aaronchongth Mar 15, 2021
5f4b61d
Remove file and line number reporting in warnings
aaronchongth Mar 15, 2021
bb56a0b
Removed checking for front Error: tag as ign.cc uses << now
aaronchongth Mar 16, 2021
1864331
Removed #ifdefs
aaronchongth Mar 19, 2021
3afb61a
Using std optional for getters
aaronchongth Mar 19, 2021
8905458
Added line number to Error for missing required attribute
aaronchongth Mar 19, 2021
6a0cfa3
Removed unused declaration
aaronchongth Mar 19, 2021
c0ddc7a
enforceConfigurablePolicyCondition accepts Error now, warn and log gr…
aaronchongth Mar 19, 2021
719b5fc
Merge branch 'master' into aaron/trace_parsing_failure
aaronchongth Mar 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 57 additions & 19 deletions include/sdf/Error.hh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <iostream>
#include <string>
#include <ignition/utils/ImplPtr.hh>
#include <sdf/sdf_config.h>
#include "sdf/system_util.hh"

Expand Down Expand Up @@ -145,15 +146,33 @@ namespace sdf

class SDFORMAT_VISIBLE Error
{
/// \brief default constructor
public: Error() = default;
/// \brief Default constructor
public: Error();

/// \brief Constructor.
/// \param[in] _code The error code.
/// \param[in] _message A description of the error.
/// \sa ErrorCode.
public: Error(const ErrorCode _code, const std::string &_message);

/// \brief Constructor.
/// \param[in] _code The error code.
/// \param[in] _message A description of the error.
/// \param[in] _filePath The file path that is related to this error.
/// \sa ErrorCode.
public: Error(const ErrorCode _code, const std::string &_message,
const std::string &_filePath);

/// \brief Constructor.
/// \param[in] _code The error code.
/// \param[in] _message A description of the error.
/// \param[in] _filePath The file path that is related to this error.
/// \param[in] _lineNumber The line number in the provided file path where
/// this error was raised.
/// \sa ErrorCode.
public: Error(const ErrorCode _code, const std::string &_message,
const std::string &_filePath, int _lineNumber);

/// \brief Get the error code.
/// \return An error code.
/// \sa ErrorCode.
Expand All @@ -163,6 +182,16 @@ namespace sdf
/// \return Error message.
public: std::string Message() const;

/// \brief Get the file path associated with this error.
/// \return Path of the file that this error is related to. Returns an empty
/// string if there is no related file.
public: std::string FilePath() const;
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved

/// \brief Get the line number associated with this error.
/// \return Line number. Returns -1 if there is no line number associated
/// with this error.
public: int LineNumber() const;

/// \brief Safe bool conversion.
/// \return True if this Error's Code() != NONE. In otherwords, this is
/// true when there is an error.
Expand All @@ -184,26 +213,35 @@ namespace sdf
public: friend std::ostream &operator<<(std::ostream &_out,
const sdf::Error &_err)
{
_out << "Error Code "
<< static_cast<std::underlying_type<sdf::ErrorCode>::type>(_err.Code())
<< " Msg: " << _err.Message();
if (_err.FilePath().empty())
{
_out << "Error Code "
<< static_cast<std::underlying_type<sdf::ErrorCode>::type>(
_err.Code())
<< " Msg: " << _err.Message();
}
else if (_err.LineNumber() < 1)
{
_out << "Error Code "
<< static_cast<std::underlying_type<sdf::ErrorCode>::type>(
_err.Code())
<< ": [" << _err.FilePath() << "]: "
<< " Msg: " << _err.Message();
}
else
{
_out << "Error Code "
<< static_cast<std::underlying_type<sdf::ErrorCode>::type>(
_err.Code())
<< ": [" << _err.FilePath() << ":L"
<< std::to_string(_err.LineNumber()) << "]: "
<< " Msg: " << _err.Message();
}
return _out;
}

/// \brief The error code value.
private: ErrorCode code = ErrorCode::NONE;

#ifdef _WIN32
// Disable warning C4251 which is triggered by
// std::string
#pragma warning(push)
#pragma warning(disable: 4251)
#endif
/// \brief Description of the error.
private: std::string message = "";
#ifdef _WIN32
#pragma warning(pop)
#endif
/// \brief Private data pointer.
IGN_UTILS_IMPL_PTR(dataPtr)
};
}
}
Expand Down
77 changes: 70 additions & 7 deletions src/Error.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,35 +19,98 @@

using namespace sdf;

class sdf::Error::Implementation
{
/// \brief The error code value.
public: ErrorCode code = ErrorCode::NONE;

#ifdef _WIN32
aaronchongth marked this conversation as resolved.
Show resolved Hide resolved
// Disable warning C4251 which is triggered by
// std::string
#pragma warning(push)
#pragma warning(disable: 4251)
#endif
/// \brief Description of the error.
public: std::string message = "";

/// \brief File path where the error was raised.
public: std::string filePath = "";
#ifdef _WIN32
#pragma warning(pop)
#endif

/// \brief Line number in the file path where the error was raised.
public: int lineNumber = -1;
};

/////////////////////////////////////////////////
Error::Error()
: dataPtr(ignition::utils::MakeImpl<Implementation>())
{}

/////////////////////////////////////////////////
Error::Error(const ErrorCode _code, const std::string &_message)
: dataPtr(ignition::utils::MakeImpl<Implementation>())
{
this->dataPtr->code = _code;
this->dataPtr->message = _message;
}

/////////////////////////////////////////////////
Error::Error(const ErrorCode _code, const std::string &_message,
const std::string &_filePath)
: dataPtr(ignition::utils::MakeImpl<Implementation>())
{
this->code = _code;
this->message = _message;
this->dataPtr->code = _code;
this->dataPtr->message = _message;
this->dataPtr->filePath = _filePath;
}

/////////////////////////////////////////////////
Error::Error(const ErrorCode _code, const std::string &_message,
const std::string &_filePath, int _lineNumber)
: dataPtr(ignition::utils::MakeImpl<Implementation>())
{
this->dataPtr->code = _code;
this->dataPtr->message = _message;
this->dataPtr->filePath = _filePath;
this->dataPtr->lineNumber = _lineNumber;
}

/////////////////////////////////////////////////
ErrorCode Error::Code() const
{
return this->code;
return this->dataPtr->code;
}

/////////////////////////////////////////////////
std::string Error::Message() const
{
return this->message;
return this->dataPtr->message;
}

/////////////////////////////////////////////////
std::string Error::FilePath() const
{
return this->dataPtr->filePath;
}

/////////////////////////////////////////////////
int Error::LineNumber() const
{
return this->dataPtr->lineNumber;
}

/////////////////////////////////////////////////
// cppcheck-suppress unusedFunction
Error::operator bool() const
{
return this->code != ErrorCode::NONE;
return this->dataPtr->code != ErrorCode::NONE;
}

/////////////////////////////////////////////////
bool Error::operator==(const bool _value) const
{
return ((this->code != ErrorCode::NONE) && _value) ||
((this->code == ErrorCode::NONE) && !_value);
return ((this->dataPtr->code != ErrorCode::NONE) && _value) ||
((this->dataPtr->code == ErrorCode::NONE) && !_value);
}
22 changes: 21 additions & 1 deletion src/Error_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,38 @@ TEST(Error, DefaultConstruction)
EXPECT_EQ(error, false);
EXPECT_EQ(error.Code(), sdf::ErrorCode::NONE);
EXPECT_TRUE(error.Message().empty());
EXPECT_TRUE(error.FilePath().empty());
EXPECT_LE(error.LineNumber(), 0);

if (error)
FAIL();
}

/////////////////////////////////////////////////
TEST(Error, ValueConstruction)
TEST(Error, ValueConstructionWithoutFile)
{
sdf::Error error(sdf::ErrorCode::FILE_READ, "Unable to read a file");
EXPECT_EQ(error, true);
EXPECT_EQ(error.Code(), sdf::ErrorCode::FILE_READ);
EXPECT_EQ(error.Message(), "Unable to read a file");
EXPECT_TRUE(error.FilePath().empty());
EXPECT_LE(error.LineNumber(), 0);

if (!error)
FAIL();
}

/////////////////////////////////////////////////
TEST(Error, ValueConstructionWithFile)
{
const std::string emptyFilePath = "Empty/file/path";
sdf::Error error(sdf::ErrorCode::FILE_READ, "Unable to read a file",
emptyFilePath, 10);
EXPECT_EQ(error, true);
EXPECT_EQ(error.Code(), sdf::ErrorCode::FILE_READ);
EXPECT_EQ(error.Message(), "Unable to read a file");
EXPECT_EQ(error.FilePath(), emptyFilePath);
EXPECT_EQ(error.LineNumber(), 10);

if (!error)
FAIL();
Expand Down
2 changes: 1 addition & 1 deletion src/ign.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ extern "C" SDFORMAT_VISIBLE int cmdCheck(const char *_path)
{
for (auto &error : errors)
{
std::cerr << "Error: " << error.Message() << std::endl;
std::cerr << error << std::endl;
}
return -1;
}
Expand Down
Loading