Limit comment tags placement
Documentation comments are recognized only when placed immediately
before entities.
--dumpapi now generates a single block comment when an entity is
hidden/deprecated to comply with the change.
/* @hide
@deprecated */
AIDL entity;
Bug: 177276893
Bug: 177616426
Test: aidl_unittests
Change-Id: I2c00d8e234d1a1ec3fbbcf34f290e0b163d508c0
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 8d9a3fe..f06b8a4 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -766,16 +766,20 @@
return android::aidl::FindDeprecated(GetComments()).has_value();
}
-// Dumps comment only if its has meaningful tags.
+// Dumps comment only if its has meaningful tags.
void AidlCommentable::DumpComments(CodeWriter& out) const {
using namespace android::aidl;
- if (IsHidden()) {
- out << "/* @hide */\n";
- }
- if (auto deprecated = FindDeprecated(GetComments()); deprecated) {
- out << "/* @deprecated ";
- if (!deprecated->note.empty()) out << deprecated->note << " ";
- out << "*/\n";
+ const auto hidden = IsHidden();
+ const auto deprecated = FindDeprecated(GetComments());
+ if (hidden || deprecated) {
+ out << "/**\n";
+ if (hidden) {
+ out << " * @hide\n";
+ }
+ if (deprecated) {
+ out << " * @deprecated " << deprecated->note << "\n";
+ }
+ out << " */\n";
}
}
diff --git a/aidl_language_l.ll b/aidl_language_l.ll
index 3ff7c2e..e43ef45 100644
--- a/aidl_language_l.ll
+++ b/aidl_language_l.ll
@@ -52,7 +52,7 @@
\/\* { extra_text += yytext; BEGIN(LONG_COMMENT); }
<LONG_COMMENT>\*+\/ { extra_text += yytext; yylloc->step(); BEGIN(INITIAL);
- comments.push_back({Comment::Type::BLOCK, extra_text});
+ comments.push_back({extra_text});
extra_text.clear(); }
<LONG_COMMENT>\*+ { extra_text += yytext; }
<LONG_COMMENT>\n+ { extra_text += yytext; yylloc->lines(yyleng); }
@@ -62,7 +62,7 @@
return yy::parser::token::C_STR; }
\/\/.* { extra_text += yytext; extra_text += "\n";
- comments.push_back({Comment::Type::LINE, extra_text});
+ comments.push_back({extra_text});
extra_text.clear(); }
\n+ { yylloc->lines(yyleng); yylloc->step(); }
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 408bee8..7d8b1c8 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -31,6 +31,7 @@
#include "aidl_language.h"
#include "aidl_to_cpp.h"
#include "aidl_to_java.h"
+#include "comments.h"
#include "logging.h"
#include "options.h"
#include "tests/fake_io_delegate.h"
@@ -636,14 +637,12 @@
EXPECT_NE(nullptr, parse_result);
EXPECT_EQ("", GetCapturedStderr());
- EXPECT_EQ((Comments{{Comment::Type::BLOCK, "/* foo */"}}), parse_result->GetComments());
+ EXPECT_EQ((Comments{{"/* foo */"}}), parse_result->GetComments());
const AidlInterface* interface = parse_result->AsInterface();
- EXPECT_EQ((Comments{{Comment::Type::BLOCK, "/* i */"}}),
- interface->GetMethods()[0]->GetComments());
- EXPECT_EQ((Comments{{Comment::Type::LINE, "// j\n"}}), interface->GetMethods()[1]->GetComments());
- EXPECT_EQ((Comments{{Comment::Type::LINE, "// k1\n"}, {Comment::Type::BLOCK, "/* k2 */"}}),
- interface->GetMethods()[2]->GetComments());
+ EXPECT_EQ((Comments{{"/* i */"}}), interface->GetMethods()[0]->GetComments());
+ EXPECT_EQ((Comments{{"// j\n"}}), interface->GetMethods()[1]->GetComments());
+ EXPECT_EQ((Comments{{"// k1\n"}, {"/* k2 */"}}), interface->GetMethods()[2]->GetComments());
}
TEST_P(AidlTest, CppHeaderCanBeIdentifierAsWell) {
@@ -662,8 +661,7 @@
auto parse_result = Parse(input_path, input, typenames_, GetLanguage());
EXPECT_NE(nullptr, parse_result);
const AidlInterface* interface = parse_result->AsInterface();
- EXPECT_EQ((Comments{{Comment::Type::LINE, "// get bar\n"}}),
- interface->GetMethods()[0]->GetComments());
+ EXPECT_EQ((Comments{{"// get bar\n"}}), interface->GetMethods()[0]->GetComments());
}
TEST_F(AidlTest, ParsesPreprocessedFile) {
@@ -1439,12 +1437,13 @@
"import foo.bar.Data;\n"
"// commented /* @hide */\n"
"interface IFoo {\n"
- " /* @hide */\n"
- " int foo(out int[] a, String b, boolean c, inout List<String> d);\n"
- " int foo2(@utf8InCpp String x, inout List<String> y);\n"
+ " /* @hide applied \n"
+ " @deprecated use foo2 */\n"
+ " int foo(out int[] a, String b, boolean c, inout List<String> d);\n"
+ " int foo2(@utf8InCpp String x, inout List<String> y);\n"
" IFoo foo3(IFoo foo);\n"
" Data getData();\n"
- " // @hide\n"
+ " // @hide not applied\n"
" /** blahblah\n"
" @deprecated\n"
" reason why... */\n"
@@ -1474,29 +1473,38 @@
ASSERT_TRUE(result);
string actual;
EXPECT_TRUE(io_delegate_.GetWrittenContents("dump/foo/bar/IFoo.aidl", &actual));
- EXPECT_EQ(actual, string(kPreamble).append(R"(package foo.bar;
+ EXPECT_EQ(string(kPreamble).append(R"(package foo.bar;
interface IFoo {
- /* @hide */
+ /**
+ * @hide
+ * @deprecated use foo2
+ */
int foo(out int[] a, String b, boolean c, inout List<String> d);
int foo2(@utf8InCpp String x, inout List<String> y);
foo.bar.IFoo foo3(foo.bar.IFoo foo);
foo.bar.Data getData();
- /* @deprecated reason why... */
+ /**
+ * @deprecated reason why...
+ */
const int A = 1;
const String STR = "Hello";
}
-)"));
+)"),
+ actual);
EXPECT_TRUE(io_delegate_.GetWrittenContents("dump/foo/bar/Data.aidl", &actual));
- EXPECT_EQ(actual, string(kPreamble).append(R"(package foo.bar;
-/* @hide */
+ EXPECT_EQ(string(kPreamble).append(R"(package foo.bar;
+/**
+ * @hide
+ */
parcelable Data {
int x = 10;
int y;
foo.bar.IFoo foo;
@nullable String[] c;
}
-)"));
+)"),
+ actual);
}
TEST_F(AidlTest, ApiDumpWithManualIds) {
diff --git a/comments.cpp b/comments.cpp
index 36f37e8..4b724b2 100644
--- a/comments.cpp
+++ b/comments.cpp
@@ -45,7 +45,8 @@
static const std::regex kTagHideRegex{"@hide\\b"};
std::string ConsumePrefix(const std::string& s, std::string_view prefix) {
- AIDL_FATAL_IF(!StartsWith(s, prefix), AIDL_LOCATION_HERE);
+ AIDL_FATAL_IF(!StartsWith(s, prefix), AIDL_LOCATION_HERE)
+ << "'" << s << "' has no prefix '" << prefix << "'";
return s.substr(prefix.size());
}
@@ -63,7 +64,9 @@
// - keeps leading spaces, but trims trailing spaces
// - keeps empty lines
std::vector<std::string> TrimmedLines(const Comment& c) {
- if (c.type == Comment::Type::LINE) return std::vector{ConsumePrefix(c.body, kLineCommentBegin)};
+ if (c.type == Comment::Type::LINE) {
+ return std::vector{ConsumePrefix(c.body, kLineCommentBegin)};
+ }
std::string stripped = ConsumeSuffix(ConsumePrefix(c.body, kBlockCommentBegin), kBlockCommentEnd);
@@ -99,7 +102,10 @@
return lines;
}
+// Parses a block comment and returns block tags in the comment.
std::vector<BlockTag> BlockTags(const Comment& c) {
+ AIDL_FATAL_IF(c.type != Comment::Type::BLOCK, AIDL_LOCATION_HERE);
+
std::vector<BlockTag> tags;
// current tag and paragraph
@@ -150,23 +156,55 @@
return tags;
}
-}
-bool HasHideInComments(const Comments& comments) {
- for (const auto& c : comments) {
- if (c.type == Comment::Type::LINE) continue;
- if (std::regex_search(c.body, kTagHideRegex)) {
- return true;
- }
+} // namespace
+
+Comment::Comment(const std::string& body) : body(body) {
+ if (StartsWith(body, kLineCommentBegin)) {
+ type = Type::LINE;
+ } else if (StartsWith(body, kBlockCommentBegin) && EndsWith(body, kBlockCommentEnd)) {
+ type = Type::BLOCK;
+ } else {
+ AIDL_FATAL(AIDL_LOCATION_HERE) << "invalid comments body:" << body;
}
- return false;
}
-// Finds @deprecated tag and returns it with optional note which follows the tag.
+// Returns the immediate block comment from the list of comments.
+// Only the last/block comment can have the tag.
+//
+// /* @hide */
+// int x;
+//
+// But tags in line or distant comments don't count. In the following,
+// the variable 'x' is not hidden.
+//
+// // @hide
+// int x;
+//
+// /* @hide */
+// /* this is the immemediate comment to 'x' */
+// int x;
+//
+static std::optional<Comment> GetValidComment(const Comments& comments) {
+ if (!comments.empty() && comments.back().type == Comment::Type::BLOCK) {
+ return comments.back();
+ }
+ return std::nullopt;
+}
+
+// Sees if comments have the @hide tag.
+// Example: /** @hide */
+bool HasHideInComments(const Comments& comments) {
+ const auto valid_comment = GetValidComment(comments);
+ return valid_comment && std::regex_search(valid_comment->body, kTagHideRegex);
+}
+
+// Finds the @deprecated tag in comments and returns it with optional note which
+// follows the tag.
+// Example: /** @deprecated reason */
std::optional<Deprecated> FindDeprecated(const Comments& comments) {
- for (const auto& c : comments) {
- if (c.type == Comment::Type::LINE) continue;
- for (const auto& [name, description] : BlockTags(c)) {
+ if (const auto valid_comment = GetValidComment(comments); valid_comment) {
+ for (const auto& [name, description] : BlockTags(comments.back())) {
// take the first @deprecated
if (kTagDeprecated == name) {
return Deprecated{description};
diff --git a/comments.h b/comments.h
index 339885a..6814e60 100644
--- a/comments.h
+++ b/comments.h
@@ -29,6 +29,8 @@
Type type;
std::string body;
+ Comment(const std::string& body);
+
// for GTest assertions
friend inline bool operator==(const Comment& lhs, const Comment& rhs) {
return lhs.body == rhs.body;