Make all uses of AidlErrorLog include a location
File names and directory names are locations with line = 0 and column =
0.
This allows AidlErrorLog to check for INTERNAL locations being emitted
upon destruction. If that is true, AidlErrorLog will abort after
emitting to original offending log and aditional log saying this should
not happen.
Test: atest aidl_unittests aidl_intergration_test
Change-Id: I0c11b66acb1d22e43f397f3ea9e4c4c14e9f46fc
diff --git a/aidl.cpp b/aidl.cpp
index ce8b877..5495410 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -429,12 +429,12 @@
for (AidlDefinedType* type : main_parser->GetDefinedTypes()) {
if (type->AsInterface() != nullptr || type->AsStructuredParcelable() != nullptr) {
num_interfaces_or_structured_parcelables++;
+ if (num_interfaces_or_structured_parcelables > 1) {
+ AIDL_ERROR(*type) << "You must declare only one type per file.";
+ return AidlError::BAD_TYPE;
+ }
}
}
- if (num_interfaces_or_structured_parcelables > 1) {
- AIDL_ERROR(input_file_name) << "You must declare only one type per a file.";
- return AidlError::BAD_TYPE;
- }
// Import the preprocessed file
for (const string& s : options.PreprocessedFiles()) {
@@ -492,7 +492,7 @@
if (std::find(type_from_import_statements.begin(), type_from_import_statements.end(),
import) != type_from_import_statements.end()) {
// Complain only when the import from the import statement has failed.
- AIDL_ERROR(import) << "couldn't find import for class " << import;
+ AIDL_ERROR(input_file_name) << "Couldn't find import for class " << import;
err = AidlError::BAD_IMPORT;
}
continue;
diff --git a/aidl_language.cpp b/aidl_language.cpp
index da80c38..8c8a70c 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -92,11 +92,14 @@
: file_(file), begin_(begin), end_(end), source_(source) {}
std::ostream& operator<<(std::ostream& os, const AidlLocation& l) {
- os << l.file_ << ":" << l.begin_.line << "." << l.begin_.column << "-";
- if (l.begin_.line != l.end_.line) {
- os << l.end_.line << ".";
+ os << l.file_;
+ if (l.LocationKnown()) {
+ os << ":" << l.begin_.line << "." << l.begin_.column << "-";
+ if (l.begin_.line != l.end_.line) {
+ os << l.end_.line << ".";
+ }
+ os << l.end_.column;
}
- os << l.end_.column;
return os;
}
@@ -115,15 +118,11 @@
return ss.str();
}
-AidlErrorLog::AidlErrorLog(bool fatal) : os_(std::cerr), fatal_(fatal) {
+AidlErrorLog::AidlErrorLog(bool fatal, const AidlLocation& location)
+ : os_(std::cerr), fatal_(fatal), location_(location) {
sHadError = true;
os_ << "ERROR: ";
-}
-
-AidlErrorLog::AidlErrorLog(bool fatal, const AidlLocation& location) : AidlErrorLog(fatal) {
- CHECK(!location.IsInternal())
- << "Logging an internal location should not happen. Offending location: " << location;
os_ << location << ": ";
}
diff --git a/aidl_language.h b/aidl_language.h
index 86640e9..5f08504 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -82,9 +82,14 @@
};
AidlLocation(const std::string& file, Point begin, Point end, Source source);
+ AidlLocation(const std::string& file, Source source)
+ : AidlLocation(file, {0, 0}, {0, 0}, source) {}
bool IsInternal() const { return source_ == Source::INTERNAL; }
+ // The first line of a file is line 1.
+ bool LocationKnown() const { return begin_.line != 0; }
+
friend std::ostream& operator<<(std::ostream& os, const AidlLocation& l);
friend class AidlNode;
@@ -127,9 +132,8 @@
// Generic point for printing any error in the AIDL compiler.
class AidlErrorLog {
public:
- AidlErrorLog(bool fatal, const std::string& filename) : AidlErrorLog(fatal) {
- os_ << filename << ": ";
- }
+ AidlErrorLog(bool fatal, const std::string& filename)
+ : AidlErrorLog(fatal, AidlLocation(filename, AidlLocation::Source::EXTERNAL)) {}
AidlErrorLog(bool fatal, const AidlLocation& location);
AidlErrorLog(bool fatal, const AidlNode& node) : AidlErrorLog(fatal, node.location_) {}
AidlErrorLog(bool fatal, const AidlNode* node) : AidlErrorLog(fatal, *node) {}
@@ -139,6 +143,11 @@
~AidlErrorLog() {
os_ << std::endl;
if (fatal_) abort();
+ if (location_.IsInternal()) {
+ os_ << "Logging an internal location should not happen. Offending location: " << location_
+ << std::endl;
+ abort();
+ }
}
std::ostream& os_;
@@ -146,10 +155,11 @@
static bool hadError() { return sHadError; }
private:
- AidlErrorLog(bool fatal);
bool fatal_;
+ const AidlLocation location_;
+
static bool sHadError;
DISALLOW_COPY_AND_ASSIGN(AidlErrorLog);
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 19e8d17..99f8cdc 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -669,7 +669,8 @@
TEST_P(AidlTest, FailOnManyDefinedTypes) {
AidlError error;
- const string expected_stderr = "ERROR: p/IFoo.aidl: You must declare only one type per a file.\n";
+ const string expected_stderr =
+ "ERROR: p/IFoo.aidl:3.33-38: You must declare only one type per file.\n";
CaptureStderr();
EXPECT_EQ(nullptr, Parse("p/IFoo.aidl",
R"(package p;
@@ -1129,9 +1130,9 @@
TEST_F(AidlTest, FailOnMultipleTypesInSingleFile) {
std::vector<std::string> rawOptions{"aidl --lang=java -o out foo/bar/Foo.aidl",
"aidl --lang=cpp -o out -h out/include foo/bar/Foo.aidl"};
- const string expected_stderr =
- "ERROR: foo/bar/Foo.aidl: You must declare only one type per a file.\n";
- for (auto& rawOption : rawOptions) {
+ for (const auto& rawOption : rawOptions) {
+ string expected_stderr =
+ "ERROR: foo/bar/Foo.aidl:3.1-10: You must declare only one type per file.\n";
Options options = Options::From(rawOption);
io_delegate_.SetFileContents(options.InputFiles().front(),
"package foo.bar;\n"
@@ -1151,6 +1152,7 @@
EXPECT_NE(0, ::android::aidl::compile_aidl(options, io_delegate_));
EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ expected_stderr = "ERROR: foo/bar/Foo.aidl:3.11-17: You must declare only one type per file.\n";
io_delegate_.SetFileContents(options.InputFiles().front(),
"package foo.bar;\n"
"parcelable Data1 { int a; int b;}\n"
@@ -1741,7 +1743,7 @@
"ERROR: p/IFoo.aidl: Duplicate files found for q.IBar from:\n"
"dir1/q/IBar.aidl\n"
"dir2/q/IBar.aidl\n"
- "ERROR: q.IBar: couldn't find import for class q.IBar\n";
+ "ERROR: p/IFoo.aidl: Couldn't find import for class q.IBar\n";
Options options = Options::From("aidl --lang=java -o out -I dir1 -I dir2 p/IFoo.aidl");
io_delegate_.SetFileContents("p/IFoo.aidl", "package p; import q.IBar; interface IFoo{}");
io_delegate_.SetFileContents("dir1/q/IBar.aidl", "package q; interface IBar{}");
@@ -1872,7 +1874,7 @@
"ERROR: IFoo.aidl: Duplicate files found for IBar from:\n"
"dir/IBar.aidl\n"
"dir2/IBar.aidl\n"
- "ERROR: IBar: couldn't find import for class IBar\n";
+ "ERROR: IFoo.aidl: Couldn't find import for class IBar\n";
Options options = Options::From("aidl --lang=java -I dir -I dir2 IFoo.aidl");
io_delegate_.SetFileContents("dir/IBar.aidl", "interface IBar{}");