[fidlc] Sharing error formatting in ErrorReporter
Test: unit tests, no expected behavior change
Change-Id: I2cfd3488ed56e0e887d125b46b545dca26bee17f
diff --git a/system/host/fidl/include/fidl/error_reporter.h b/system/host/fidl/include/fidl/error_reporter.h
index c323ce2..7816742 100644
--- a/system/host/fidl/include/fidl/error_reporter.h
+++ b/system/host/fidl/include/fidl/error_reporter.h
@@ -8,11 +8,17 @@
#include <string>
#include <vector>
+#include "source_location.h"
+#include "string_view.h"
+#include "token.h"
+
namespace fidl {
class ErrorReporter {
public:
- void ReportError(std::string error);
+ void ReportError(const SourceLocation& location, StringView message);
+ void ReportError(const Token& token, StringView message);
+ void ReportError(StringView message);
void PrintReports();
const std::vector<std::string>& errors() const { return errors_; };
diff --git a/system/host/fidl/lib/error_reporter.cpp b/system/host/fidl/lib/error_reporter.cpp
index 1779088..72df693 100644
--- a/system/host/fidl/lib/error_reporter.cpp
+++ b/system/host/fidl/lib/error_reporter.cpp
@@ -3,16 +3,90 @@
// found in the LICENSE file.
#include "fidl/error_reporter.h"
+#include "fidl/source_location.h"
+#include "fidl/string_view.h"
+#include "fidl/token.h"
namespace fidl {
-void ErrorReporter::ReportError(std::string error) {
+std::string MakeSquiggle(const std::string& surrounding_line, int column) {
+ std::string squiggle;
+ for (int i = 0; i < column; i++) {
+ switch (surrounding_line[i]) {
+ case '\t':
+ squiggle.push_back('\t');
+ default:
+ squiggle.push_back(' ');
+ }
+ }
+ squiggle.push_back('^');
+ return squiggle;
+}
+
+std::string FormatError(const SourceLocation& location, StringView message, size_t squiggle_size = 0u) {
+ SourceFile::Position position;
+ std::string surrounding_line = location.SourceLine(&position);
+
+ std::string squiggle = MakeSquiggle(surrounding_line, position.column);
+ if (squiggle_size != 0u) {
+ --squiggle_size;
+ }
+ squiggle += std::string(squiggle_size, '~');
+ // Some tokens (like string literals) can span multiple
+ // lines. Truncate the string to just one line at most. The
+ // containing line contains a newline, so drop it when
+ // comparing sizes.
+ size_t line_size = surrounding_line.size() - 1;
+ if (squiggle.size() > line_size) {
+ squiggle.resize(line_size);
+ }
+
+ // Many editors and IDEs recognize errors in the form of
+ // filename:linenumber:column: error: descriptive-test-here\n
+ std::string error = location.position();
+ error.append(": error: ");
+ error.append(message);
+ error.push_back('\n');
+ error.append(surrounding_line);
+ error.append(squiggle);
+
+ return error;
+}
+
+// ReportError records an error with the location, message, source line, and
+// position indicator.
+//
+// filename:line:col: error: message
+// sourceline
+// ^
+void ErrorReporter::ReportError(const SourceLocation& location, StringView message) {
+ auto error = FormatError(location, message);
+ errors_.push_back(std::move(error));
+}
+
+// ReportError records an error with the location, message, source line,
+// position indicator, and tildes under the token reported.
+//
+// filename:line:col: error: message
+// sourceline
+// ^~~~
+void ErrorReporter::ReportError(const Token& token, StringView message) {
+ auto token_location = token.location();
+ auto token_data = token_location.data();
+ auto error = FormatError(token_location, message, token_data.size());
+ errors_.push_back(std::move(error));
+}
+
+// ReportError records the provided message.
+void ErrorReporter::ReportError(StringView message) {
+ std::string error("error: ");
+ error.append(message);
errors_.push_back(std::move(error));
}
void ErrorReporter::PrintReports() {
for (const auto& error : errors_) {
- fprintf(stderr, "%s", error.data());
+ fprintf(stderr, "%s\n", error.data());
}
}
diff --git a/system/host/fidl/lib/flat_ast.cpp b/system/host/fidl/lib/flat_ast.cpp
index 369cb5f..f36ccd4 100644
--- a/system/host/fidl/lib/flat_ast.cpp
+++ b/system/host/fidl/lib/flat_ast.cpp
@@ -365,16 +365,12 @@
}
bool Library::Fail(StringView message) {
- auto formatted_message = std::string(message) + "\n";
- error_reporter_->ReportError(std::move(formatted_message));
+ error_reporter_->ReportError(message);
return false;
}
bool Library::Fail(const SourceLocation& location, StringView message) {
- // Many editors and IDEs recognize errors in the form of
- // filename:linenumber:column: error: descriptive-test-here\n
- auto formatted_message = location.position() + ": error: " + std::string(message) + "\n";
- error_reporter_->ReportError(std::move(formatted_message));
+ error_reporter_->ReportError(location, message);
return false;
}
diff --git a/system/host/fidl/lib/parser.cpp b/system/host/fidl/lib/parser.cpp
index 6d2ba77..e7a39fd 100644
--- a/system/host/fidl/lib/parser.cpp
+++ b/system/host/fidl/lib/parser.cpp
@@ -79,59 +79,13 @@
return true;
}
-std::string MakeSquiggle(const std::string& surrounding_line, int column) {
- std::string squiggle;
- for (int i = 0; i < column - 1; i++) {
- switch (surrounding_line[i]) {
- case '\t':
- squiggle += "\t";
- default:
- squiggle += " ";
- }
- }
- squiggle += "^";
- return squiggle;
-}
-
decltype(nullptr) Parser::Fail() {
return Fail("found unexpected token");
}
decltype(nullptr) Parser::Fail(StringView message) {
if (ok_) {
- auto token_location = last_token_.location();
- auto token_data = token_location.data();
-
- SourceFile::Position position;
- std::string surrounding_line = token_location.SourceLine(&position);
- auto line_number = std::to_string(position.line);
- auto column_number = std::to_string(position.column);
-
- std::string squiggle = MakeSquiggle(surrounding_line, position.column);
- size_t squiggle_size = token_data.size();
- if (squiggle_size != 0u) {
- --squiggle_size;
- }
- squiggle += std::string(squiggle_size, '~');
- // Some tokens (like string literals) can span multiple
- // lines. Truncate the string to just one line at most. The
- // containing line contains a newline, so drop it when
- // comparing sizes.
- size_t line_size = surrounding_line.size() - 1;
- if (squiggle.size() > line_size) {
- squiggle.resize(line_size);
- }
-
- // Many editors and IDEs recognize errors in the form of
- // filename:linenumber:column: error: descriptive-test-here\n
- std::string error = token_location.position();
- error += ": error: ";
- error += message;
- error += "\n";
- error += surrounding_line;
- error += squiggle + "\n";
-
- error_reporter_->ReportError(std::move(error));
+ error_reporter_->ReportError(last_token_, std::move(message));
ok_ = false;
}
return nullptr;
diff --git a/system/utest/fidl-compiler/using_tests.cpp b/system/utest/fidl-compiler/using_tests.cpp
index e6d4a37..5b8c6c7 100644
--- a/system/utest/fidl-compiler/using_tests.cpp
+++ b/system/utest/fidl-compiler/using_tests.cpp
@@ -93,11 +93,8 @@
ASSERT_FALSE(library.Compile());
const auto& errors = library.errors();
ASSERT_EQ(1, errors.size());
- // TODO(pascal): Refactor. We should check the error message, not
- // necessarily the formatted message with filename:linenum:colnum. Consider
- // introducing ASSERT_STR_CONTAINS.
- ASSERT_STR_EQ(errors[0].c_str(),
- "uzing.fidl:7:2: error: Unknown dependent library fidl.test.uzing.dependent. Did you require it with `using`?\n");
+ ASSERT_STR_STR(errors[0].c_str(),
+ "Unknown dependent library fidl.test.uzing.dependent. Did you require it with `using`?");
END_TEST;
}
@@ -124,11 +121,8 @@
ASSERT_FALSE(library.Compile());
const auto& errors = library.errors();
ASSERT_EQ(1, errors.size());
- // TODO(pascal): Refactor. We should check the error message, not
- // necessarily the formatted message with filename:linenum:colnum. Consider
- // introducing ASSERT_STR_CONTAINS.
- ASSERT_STR_EQ(errors[0].c_str(),
- "Library fidl.test.uzing.dependent already imported. Did you require it twice?\n");
+ ASSERT_STR_STR(errors[0].c_str(),
+ "Library fidl.test.uzing.dependent already imported. Did you require it twice?");
END_TEST;
}
