Merge "Don't add API annotations in the internal R.java" into rvc-dev-plus-aosp am: 7c72b7f45e am: 780aeb116a am: 47d47a8ac5
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11955280 Change-Id: I602f8515391d93daf1d538f965659dfdca62e550
This commit is contained in:
@@ -1272,7 +1272,8 @@ class Linker {
|
||||
return false;
|
||||
}
|
||||
|
||||
ClassDefinition::WriteJavaFile(manifest_class.get(), package_utf8, true, &fout);
|
||||
ClassDefinition::WriteJavaFile(manifest_class.get(), package_utf8, true,
|
||||
false /* strip_api_annotations */, &fout);
|
||||
fout.Flush();
|
||||
|
||||
if (fout.HadError()) {
|
||||
|
||||
@@ -123,7 +123,7 @@ void AnnotationProcessor::AppendNewLine() {
|
||||
}
|
||||
}
|
||||
|
||||
void AnnotationProcessor::Print(Printer* printer) const {
|
||||
void AnnotationProcessor::Print(Printer* printer, bool strip_api_annotations) const {
|
||||
if (has_comments_) {
|
||||
std::string result = comment_.str();
|
||||
for (const StringPiece& line : util::Tokenize(result, '\n')) {
|
||||
@@ -137,6 +137,9 @@ void AnnotationProcessor::Print(Printer* printer) const {
|
||||
printer->Println("@Deprecated");
|
||||
}
|
||||
|
||||
if (strip_api_annotations) {
|
||||
return;
|
||||
}
|
||||
for (const AnnotationRule& rule : sAnnotationRules) {
|
||||
const auto& it = annotation_parameter_map_.find(rule.bit_mask);
|
||||
if (it != annotation_parameter_map_.end()) {
|
||||
|
||||
@@ -65,7 +65,7 @@ class AnnotationProcessor {
|
||||
void AppendNewLine();
|
||||
|
||||
// Writes the comments and annotations to the Printer.
|
||||
void Print(text::Printer* printer) const;
|
||||
void Print(text::Printer* printer, bool strip_api_annotations = false) const;
|
||||
|
||||
private:
|
||||
std::stringstream comment_;
|
||||
|
||||
@@ -91,6 +91,21 @@ TEST(AnnotationProcessorTest, EmitsTestApiAnnotationAndRemovesFromComment) {
|
||||
EXPECT_THAT(annotations, HasSubstr("This is a test API"));
|
||||
}
|
||||
|
||||
TEST(AnnotationProcessorTest, NotEmitSystemApiAnnotation) {
|
||||
AnnotationProcessor processor;
|
||||
processor.AppendComment("@SystemApi This is a system API");
|
||||
|
||||
std::string annotations;
|
||||
StringOutputStream out(&annotations);
|
||||
Printer printer(&out);
|
||||
processor.Print(&printer, true /* strip_api_annotations */);
|
||||
out.Flush();
|
||||
|
||||
EXPECT_THAT(annotations, Not(HasSubstr("@android.annotation.SystemApi")));
|
||||
EXPECT_THAT(annotations, Not(HasSubstr("@SystemApi")));
|
||||
EXPECT_THAT(annotations, HasSubstr("This is a system API"));
|
||||
}
|
||||
|
||||
TEST(AnnotationProcessor, ExtractsFirstSentence) {
|
||||
EXPECT_THAT(AnnotationProcessor::ExtractFirstSentence("This is the only sentence"),
|
||||
Eq("This is the only sentence"));
|
||||
|
||||
@@ -23,15 +23,15 @@ using ::android::StringPiece;
|
||||
|
||||
namespace aapt {
|
||||
|
||||
void ClassMember::Print(bool /*final*/, Printer* printer) const {
|
||||
processor_.Print(printer);
|
||||
void ClassMember::Print(bool /*final*/, Printer* printer, bool strip_api_annotations) const {
|
||||
processor_.Print(printer, strip_api_annotations);
|
||||
}
|
||||
|
||||
void MethodDefinition::AppendStatement(const StringPiece& statement) {
|
||||
statements_.push_back(statement.to_string());
|
||||
}
|
||||
|
||||
void MethodDefinition::Print(bool final, Printer* printer) const {
|
||||
void MethodDefinition::Print(bool final, Printer* printer, bool) const {
|
||||
printer->Print(signature_).Println(" {");
|
||||
printer->Indent();
|
||||
for (const auto& statement : statements_) {
|
||||
@@ -74,12 +74,12 @@ bool ClassDefinition::empty() const {
|
||||
return true;
|
||||
}
|
||||
|
||||
void ClassDefinition::Print(bool final, Printer* printer) const {
|
||||
void ClassDefinition::Print(bool final, Printer* printer, bool strip_api_annotations) const {
|
||||
if (empty() && !create_if_empty_) {
|
||||
return;
|
||||
}
|
||||
|
||||
ClassMember::Print(final, printer);
|
||||
ClassMember::Print(final, printer, strip_api_annotations);
|
||||
|
||||
printer->Print("public ");
|
||||
if (qualifier_ == ClassQualifier::kStatic) {
|
||||
@@ -93,7 +93,7 @@ void ClassDefinition::Print(bool final, Printer* printer) const {
|
||||
// and takes precedence over a previous member with the same name. The overridden member is
|
||||
// set to nullptr.
|
||||
if (member != nullptr) {
|
||||
member->Print(final, printer);
|
||||
member->Print(final, printer, strip_api_annotations);
|
||||
printer->Println();
|
||||
}
|
||||
}
|
||||
@@ -111,11 +111,11 @@ constexpr static const char* sWarningHeader =
|
||||
" */\n\n";
|
||||
|
||||
void ClassDefinition::WriteJavaFile(const ClassDefinition* def, const StringPiece& package,
|
||||
bool final, io::OutputStream* out) {
|
||||
bool final, bool strip_api_annotations, io::OutputStream* out) {
|
||||
Printer printer(out);
|
||||
printer.Print(sWarningHeader).Print("package ").Print(package).Println(";");
|
||||
printer.Println();
|
||||
def->Print(final, &printer);
|
||||
def->Print(final, &printer, strip_api_annotations);
|
||||
}
|
||||
|
||||
} // namespace aapt
|
||||
|
||||
@@ -50,7 +50,7 @@ class ClassMember {
|
||||
// Writes the class member to the Printer. Subclasses should derive this method
|
||||
// to write their own data. Call this base method from the subclass to write out
|
||||
// this member's comments/annotations.
|
||||
virtual void Print(bool final, text::Printer* printer) const;
|
||||
virtual void Print(bool final, text::Printer* printer, bool strip_api_annotations = false) const;
|
||||
|
||||
private:
|
||||
AnnotationProcessor processor_;
|
||||
@@ -70,10 +70,11 @@ class PrimitiveMember : public ClassMember {
|
||||
return name_;
|
||||
}
|
||||
|
||||
void Print(bool final, text::Printer* printer) const override {
|
||||
void Print(bool final, text::Printer* printer, bool strip_api_annotations = false)
|
||||
const override {
|
||||
using std::to_string;
|
||||
|
||||
ClassMember::Print(final, printer);
|
||||
ClassMember::Print(final, printer, strip_api_annotations);
|
||||
|
||||
printer->Print("public static ");
|
||||
if (final) {
|
||||
@@ -104,8 +105,9 @@ class PrimitiveMember<std::string> : public ClassMember {
|
||||
return name_;
|
||||
}
|
||||
|
||||
void Print(bool final, text::Printer* printer) const override {
|
||||
ClassMember::Print(final, printer);
|
||||
void Print(bool final, text::Printer* printer, bool strip_api_annotations = false)
|
||||
const override {
|
||||
ClassMember::Print(final, printer, strip_api_annotations);
|
||||
|
||||
printer->Print("public static ");
|
||||
if (final) {
|
||||
@@ -142,8 +144,9 @@ class PrimitiveArrayMember : public ClassMember {
|
||||
return name_;
|
||||
}
|
||||
|
||||
void Print(bool final, text::Printer* printer) const override {
|
||||
ClassMember::Print(final, printer);
|
||||
void Print(bool final, text::Printer* printer, bool strip_api_annotations = false)
|
||||
const override {
|
||||
ClassMember::Print(final, printer, strip_api_annotations);
|
||||
|
||||
printer->Print("public static final int[] ").Print(name_).Print("={");
|
||||
printer->Indent();
|
||||
@@ -195,7 +198,7 @@ class MethodDefinition : public ClassMember {
|
||||
return false;
|
||||
}
|
||||
|
||||
void Print(bool final, text::Printer* printer) const override;
|
||||
void Print(bool final, text::Printer* printer, bool strip_api_annotations = false) const override;
|
||||
|
||||
private:
|
||||
DISALLOW_COPY_AND_ASSIGN(MethodDefinition);
|
||||
@@ -209,7 +212,7 @@ enum class ClassQualifier { kNone, kStatic };
|
||||
class ClassDefinition : public ClassMember {
|
||||
public:
|
||||
static void WriteJavaFile(const ClassDefinition* def, const android::StringPiece& package,
|
||||
bool final, io::OutputStream* out);
|
||||
bool final, bool strip_api_annotations, io::OutputStream* out);
|
||||
|
||||
ClassDefinition(const android::StringPiece& name, ClassQualifier qualifier, bool createIfEmpty)
|
||||
: name_(name.to_string()), qualifier_(qualifier), create_if_empty_(createIfEmpty) {}
|
||||
@@ -227,7 +230,7 @@ class ClassDefinition : public ClassMember {
|
||||
return name_;
|
||||
}
|
||||
|
||||
void Print(bool final, text::Printer* printer) const override;
|
||||
void Print(bool final, text::Printer* printer, bool strip_api_annotations = false) const override;
|
||||
|
||||
private:
|
||||
DISALLOW_COPY_AND_ASSIGN(ClassDefinition);
|
||||
|
||||
@@ -601,6 +601,8 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate,
|
||||
rewrite_method->AppendStatement("final int packageIdBits = p << 24;");
|
||||
}
|
||||
|
||||
const bool is_public = (options_.types == JavaClassGeneratorOptions::SymbolTypes::kPublic);
|
||||
|
||||
for (const auto& package : table_->packages) {
|
||||
for (const auto& type : package->types) {
|
||||
if (type->type == ResourceType::kAttrPrivate) {
|
||||
@@ -609,8 +611,7 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate,
|
||||
}
|
||||
|
||||
// Stay consistent with AAPT and generate an empty type class if the R class is public.
|
||||
const bool force_creation_if_empty =
|
||||
(options_.types == JavaClassGeneratorOptions::SymbolTypes::kPublic);
|
||||
const bool force_creation_if_empty = is_public;
|
||||
|
||||
std::unique_ptr<ClassDefinition> class_def;
|
||||
if (out != nullptr) {
|
||||
@@ -634,8 +635,7 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate,
|
||||
}
|
||||
}
|
||||
|
||||
if (out != nullptr && type->type == ResourceType::kStyleable &&
|
||||
options_.types == JavaClassGeneratorOptions::SymbolTypes::kPublic) {
|
||||
if (out != nullptr && type->type == ResourceType::kStyleable && is_public) {
|
||||
// When generating a public R class, we don't want Styleable to be part
|
||||
// of the API. It is only emitted for documentation purposes.
|
||||
class_def->GetCommentBuilder()->AppendComment("@doconly");
|
||||
@@ -654,7 +654,7 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate,
|
||||
|
||||
if (out != nullptr) {
|
||||
AppendJavaDocAnnotations(options_.javadoc_annotations, r_class.GetCommentBuilder());
|
||||
ClassDefinition::WriteJavaFile(&r_class, out_package_name, options_.use_final, out);
|
||||
ClassDefinition::WriteJavaFile(&r_class, out_package_name, options_.use_final, !is_public, out);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -26,6 +26,7 @@ using ::testing::Not;
|
||||
namespace aapt {
|
||||
|
||||
static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,
|
||||
bool strip_api_annotations,
|
||||
std::string* out_str);
|
||||
|
||||
TEST(ManifestClassGeneratorTest, NameIsProperlyGeneratedFromSymbol) {
|
||||
@@ -39,7 +40,8 @@ TEST(ManifestClassGeneratorTest, NameIsProperlyGeneratedFromSymbol) {
|
||||
</manifest>)");
|
||||
|
||||
std::string actual;
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(),
|
||||
false /* strip_api_annotations */, &actual));
|
||||
|
||||
ASSERT_THAT(actual, HasSubstr("public static final class permission {"));
|
||||
ASSERT_THAT(actual, HasSubstr("public static final class permission_group {"));
|
||||
@@ -91,7 +93,8 @@ TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) {
|
||||
</manifest>)");
|
||||
|
||||
std::string actual;
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(),
|
||||
false /* strip_api_annotations */, &actual));
|
||||
|
||||
const char* expected_access_internet = R"( /**
|
||||
* Required to access the internet.
|
||||
@@ -123,6 +126,55 @@ TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) {
|
||||
EXPECT_THAT(actual, HasSubstr(expected_test));
|
||||
}
|
||||
|
||||
TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresentButNoApiAnnotations) {
|
||||
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
|
||||
std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"(
|
||||
<manifest xmlns:android="http://schemas.android.com/apk/res/android">
|
||||
<!-- Required to access the internet.
|
||||
Added in API 1. -->
|
||||
<permission android:name="android.permission.ACCESS_INTERNET" />
|
||||
<!-- @deprecated This permission is for playing outside. -->
|
||||
<permission android:name="android.permission.PLAY_OUTSIDE" />
|
||||
<!-- This is a private permission for system only!
|
||||
@hide
|
||||
@SystemApi -->
|
||||
<permission android:name="android.permission.SECRET" />
|
||||
<!-- @TestApi This is a test only permission. -->
|
||||
<permission android:name="android.permission.TEST_ONLY" />
|
||||
</manifest>)");
|
||||
|
||||
std::string actual;
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(),
|
||||
true /* strip_api_annotations */, &actual));
|
||||
|
||||
const char* expected_access_internet = R"( /**
|
||||
* Required to access the internet.
|
||||
* Added in API 1.
|
||||
*/
|
||||
public static final String ACCESS_INTERNET="android.permission.ACCESS_INTERNET";)";
|
||||
EXPECT_THAT(actual, HasSubstr(expected_access_internet));
|
||||
|
||||
const char* expected_play_outside = R"( /**
|
||||
* @deprecated This permission is for playing outside.
|
||||
*/
|
||||
@Deprecated
|
||||
public static final String PLAY_OUTSIDE="android.permission.PLAY_OUTSIDE";)";
|
||||
EXPECT_THAT(actual, HasSubstr(expected_play_outside));
|
||||
|
||||
const char* expected_secret = R"( /**
|
||||
* This is a private permission for system only!
|
||||
* @hide
|
||||
*/
|
||||
public static final String SECRET="android.permission.SECRET";)";
|
||||
EXPECT_THAT(actual, HasSubstr(expected_secret));
|
||||
|
||||
const char* expected_test = R"( /**
|
||||
* This is a test only permission.
|
||||
*/
|
||||
public static final String TEST_ONLY="android.permission.TEST_ONLY";)";
|
||||
EXPECT_THAT(actual, HasSubstr(expected_test));
|
||||
}
|
||||
|
||||
// This is bad but part of public API behaviour so we need to preserve it.
|
||||
TEST(ManifestClassGeneratorTest, LastSeenPermissionWithSameLeafNameTakesPrecedence) {
|
||||
std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
|
||||
@@ -135,7 +187,8 @@ TEST(ManifestClassGeneratorTest, LastSeenPermissionWithSameLeafNameTakesPreceden
|
||||
</manifest>)");
|
||||
|
||||
std::string actual;
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(),
|
||||
false /* strip_api_annotations */, &actual));
|
||||
EXPECT_THAT(actual, HasSubstr("ACCESS_INTERNET=\"com.android.aapt.test.ACCESS_INTERNET\";"));
|
||||
EXPECT_THAT(actual, Not(HasSubstr("ACCESS_INTERNET=\"android.permission.ACCESS_INTERNET\";")));
|
||||
EXPECT_THAT(actual, Not(HasSubstr("ACCESS_INTERNET=\"com.android.sample.ACCESS_INTERNET\";")));
|
||||
@@ -149,11 +202,13 @@ TEST(ManifestClassGeneratorTest, NormalizePermissionNames) {
|
||||
</manifest>)");
|
||||
|
||||
std::string actual;
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
|
||||
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(),
|
||||
false /* strip_api_annotations */, &actual));
|
||||
EXPECT_THAT(actual, HasSubstr("access_internet=\"android.permission.access-internet\";"));
|
||||
}
|
||||
|
||||
static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,
|
||||
bool strip_api_annotations,
|
||||
std::string* out_str) {
|
||||
std::unique_ptr<ClassDefinition> manifest_class =
|
||||
GenerateManifestClass(context->GetDiagnostics(), res);
|
||||
@@ -162,7 +217,7 @@ static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xm
|
||||
}
|
||||
|
||||
StringOutputStream out(out_str);
|
||||
manifest_class->WriteJavaFile(manifest_class.get(), "android", true, &out);
|
||||
manifest_class->WriteJavaFile(manifest_class.get(), "android", true, strip_api_annotations, &out);
|
||||
out.Flush();
|
||||
return ::testing::AssertionSuccess();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user