From dad775b79895df5167cfbc2c78549b8417af1c01 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 8 Dec 2016 10:14:11 -0500 Subject: [PATCH] Improve ObjC deprecated annotation support. - Check the parent file options for deprecation when deciding to tag Messages and Enums as deprecated. - Within the generated source push/pop the warning for implementing deprecated things around a deprecated class implementation. - Annotate the methods generated for extension fields as deprecated. - Add a testing .proto file that covers deprecated fields, messages, enums, enum values and compile it into the unittests to confirm things compile cleanly. - Add a testing .proto file that uses the file level option to make everything deprecated and compile it into the unittests to confirm things compile cleanly. --- Makefile.am | 2 + objectivec/DevTools/compile_testing_protos.sh | 2 + objectivec/Tests/GPBARCUnittestProtos.m | 2 + objectivec/Tests/GPBUnittestProtos.m | 2 + objectivec/Tests/unittest_deprecated.proto | 95 +++++++++++++++++++ .../Tests/unittest_deprecated_file.proto | 76 +++++++++++++++ .../compiler/objectivec/objectivec_enum.cc | 2 +- .../objectivec/objectivec_extension.cc | 5 +- .../compiler/objectivec/objectivec_helpers.h | 14 ++- .../compiler/objectivec/objectivec_message.cc | 21 +++- .../compiler/objectivec/objectivec_message.h | 1 + 11 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 objectivec/Tests/unittest_deprecated.proto create mode 100644 objectivec/Tests/unittest_deprecated_file.proto diff --git a/Makefile.am b/Makefile.am index 273437803a..bea48ca331 100644 --- a/Makefile.am +++ b/Makefile.am @@ -558,6 +558,8 @@ objectivec_EXTRA_DIST= \ objectivec/Tests/text_format_map_unittest_data.txt \ objectivec/Tests/text_format_unittest_data.txt \ objectivec/Tests/unittest_cycle.proto \ + objectivec/Tests/unittest_deprecated.proto \ + objectivec/Tests/unittest_deprecated_file.proto \ objectivec/Tests/unittest_extension_chain_a.proto \ objectivec/Tests/unittest_extension_chain_b.proto \ objectivec/Tests/unittest_extension_chain_c.proto \ diff --git a/objectivec/DevTools/compile_testing_protos.sh b/objectivec/DevTools/compile_testing_protos.sh index 6cc32da9b8..d7f3f60589 100755 --- a/objectivec/DevTools/compile_testing_protos.sh +++ b/objectivec/DevTools/compile_testing_protos.sh @@ -134,6 +134,8 @@ done compile_protos \ --proto_path="objectivec/Tests" \ objectivec/Tests/unittest_cycle.proto \ + objectivec/Tests/unittest_deprecated.proto \ + objectivec/Tests/unittest_deprecated_file.proto \ objectivec/Tests/unittest_extension_chain_a.proto \ objectivec/Tests/unittest_extension_chain_b.proto \ objectivec/Tests/unittest_extension_chain_c.proto \ diff --git a/objectivec/Tests/GPBARCUnittestProtos.m b/objectivec/Tests/GPBARCUnittestProtos.m index 28d2396c2a..29f6ccf69a 100644 --- a/objectivec/Tests/GPBARCUnittestProtos.m +++ b/objectivec/Tests/GPBARCUnittestProtos.m @@ -42,6 +42,8 @@ #import "google/protobuf/Unittest.pbobjc.h" #import "google/protobuf/UnittestCustomOptions.pbobjc.h" #import "google/protobuf/UnittestCycle.pbobjc.h" +#import "google/protobuf/UnittestDeprecated.pbobjc.h" +#import "google/protobuf/UnittestDeprecatedFile.pbobjc.h" #import "google/protobuf/UnittestDropUnknownFields.pbobjc.h" #import "google/protobuf/UnittestEmbedOptimizeFor.pbobjc.h" #import "google/protobuf/UnittestEmpty.pbobjc.h" diff --git a/objectivec/Tests/GPBUnittestProtos.m b/objectivec/Tests/GPBUnittestProtos.m index 8d2948bf63..756bd99ef7 100644 --- a/objectivec/Tests/GPBUnittestProtos.m +++ b/objectivec/Tests/GPBUnittestProtos.m @@ -43,6 +43,8 @@ #import "google/protobuf/UnittestArena.pbobjc.m" #import "google/protobuf/UnittestCustomOptions.pbobjc.m" #import "google/protobuf/UnittestCycle.pbobjc.m" +#import "google/protobuf/UnittestDeprecated.pbobjc.m" +#import "google/protobuf/UnittestDeprecatedFile.pbobjc.m" #import "google/protobuf/UnittestDropUnknownFields.pbobjc.m" #import "google/protobuf/UnittestEmbedOptimizeFor.pbobjc.m" #import "google/protobuf/UnittestEmpty.pbobjc.m" diff --git a/objectivec/Tests/unittest_deprecated.proto b/objectivec/Tests/unittest_deprecated.proto new file mode 100644 index 0000000000..96a52bbbe2 --- /dev/null +++ b/objectivec/Tests/unittest_deprecated.proto @@ -0,0 +1,95 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2016 Google Inc. All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto2"; + +package protobuf_deprecated; +option objc_class_prefix = "Dep"; + +// +// This file is like unittest_deprecated_file.proto, but uses message, enum, +// enum value, and field level deprecation. +// +// The source generated from this file needs to be inspect to confirm it has +// all of the expected annotations. It also will be compiled into the unittest +// and that compile should be clean without errors. +// + +// Mix of field types marked as deprecated. +message Msg1 { + extensions 100 to max; + + optional string string_field = 1 [deprecated=true]; + required int32 int_field = 2 [deprecated=true]; + repeated fixed32 fixed_field = 3 [deprecated=true]; + optional Msg1 msg_field = 4 [deprecated=true]; +} + +// Mix of extension field types marked as deprecated. +extend Msg1 { + optional string string_ext_field = 101 [deprecated=true]; + optional int32 int_ext_field = 102 [deprecated=true]; + repeated fixed32 fixed_ext_field = 103 [deprecated=true]; + optional Msg1 msg_ext_field = 104 [deprecated=true]; +} + +// Mix of extension field types (scoped to a message) marked as deprecated. +message Msg1A { + extend Msg1 { + optional string string_ext2_field = 201 [deprecated=true]; + optional int32 int_ext2_field = 202 [deprecated=true]; + repeated fixed32 fixed_ext2_field = 203 [deprecated=true]; + optional Msg1 msg_ext2_field = 204 [deprecated=true]; + } +} + +// Enum value marked as deprecated. +enum Enum1 { + ENUM1_ONE = 1; + ENUM1_TWO = 2; + ENUM1_THREE = 3 [deprecated=true]; +} + +// Message marked as deprecated. +message Msg2 { + option deprecated = true; + + optional string string_field = 1; + required int32 int_field = 2; + repeated fixed32 fixed_field = 3; +} + +// Enum marked as deprecated. +enum Enum2 { + option deprecated = true; + + ENUM2_ONE = 1; + ENUM2_TWO = 2; + ENUM2_THREE = 3; +} diff --git a/objectivec/Tests/unittest_deprecated_file.proto b/objectivec/Tests/unittest_deprecated_file.proto new file mode 100644 index 0000000000..ef92e7de1a --- /dev/null +++ b/objectivec/Tests/unittest_deprecated_file.proto @@ -0,0 +1,76 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2016 Google Inc. All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto2"; + +package protobuf_deprecated_file; +option objc_class_prefix = "FileDep"; + +// +// This file is like unittest_deprecated.proto, but does NOT use message, enum, +// enum value, or field level deprecation; instead it uses the file level option +// to mark everything. +// +// The source generated from this file needs to be inspect to confirm it has +// all of the expected annotations. It also will be compiled into the unittest +// and that compile should be clean without errors. +// +option deprecated = true; + +// Message to catch the deprecation. +message Msg1 { + extensions 100 to max; + + optional string string_field = 1; +} + +// Mix of extension field types to catch the deprecation. +extend Msg1 { + optional string string_ext_field = 101; + optional int32 int_ext_field = 102; + repeated fixed32 fixed_ext_field = 103; + optional Msg1 msg_ext_field = 104; +} + +// Mix of extension field types (scoped to a message) to catch the deprecation. +message Msg1A { + extend Msg1 { + optional string string_ext2_field = 201; + optional int32 int_ext2_field = 202; + repeated fixed32 fixed_ext2_field = 203; + optional Msg1 msg_ext2_field = 204; + } +} + +// Enum to catch the deprecation. +enum Enum1 { + ENUM1_ONE = 1; + ENUM1_TWO = 2; + ENUM1_THREE = 3; +} diff --git a/src/google/protobuf/compiler/objectivec/objectivec_enum.cc b/src/google/protobuf/compiler/objectivec/objectivec_enum.cc index 34e1782384..02d60b3e97 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_enum.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_enum.cc @@ -74,7 +74,7 @@ void EnumGenerator::GenerateHeader(io::Printer* printer) { printer->Print("$comments$typedef$deprecated_attribute$ GPB_ENUM($name$) {\n", "comments", enum_comments, - "deprecated_attribute", GetOptionalDeprecatedAttribute(descriptor_), + "deprecated_attribute", GetOptionalDeprecatedAttribute(descriptor_, descriptor_->file()), "name", name_); printer->Indent(); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_extension.cc b/src/google/protobuf/compiler/objectivec/objectivec_extension.cc index d0de1eca34..7073173c88 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_extension.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_extension.cc @@ -67,9 +67,12 @@ void ExtensionGenerator::GenerateMembersHeader(io::Printer* printer) { } else { vars["comments"] = ""; } + // Unlike normal message fields, check if the file for the extension was + // deprecated. + vars["deprecated_attribute"] = GetOptionalDeprecatedAttribute(descriptor_, descriptor_->file()); printer->Print(vars, "$comments$" - "+ (GPBExtensionDescriptor *)$method_name$;\n"); + "+ (GPBExtensionDescriptor *)$method_name$$deprecated_attribute$;\n"); } void ExtensionGenerator::GenerateStaticVariablesInitialization( diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index 316069e13c..a217944622 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -150,8 +150,18 @@ enum FlagType { }; template -string GetOptionalDeprecatedAttribute(const TDescriptor* descriptor, bool preSpace = true, bool postNewline = false) { - if (descriptor->options().deprecated()) { +string GetOptionalDeprecatedAttribute( + const TDescriptor* descriptor, + const FileDescriptor* file = NULL, + bool preSpace = true, bool postNewline = false) { + bool isDeprecated = descriptor->options().deprecated(); + // The file is only passed when checking Messages & Enums, so those types + // get tagged. At the moment, it doesn't seem to make sense to tag every + // field or enum value with when the file is deprecated. + if (!isDeprecated && file) { + isDeprecated = file->options().deprecated(); + } + if (isDeprecated) { string result = "DEPRECATED_ATTRIBUTE"; if (preSpace) { result.insert(0, " "); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message.cc b/src/google/protobuf/compiler/objectivec/objectivec_message.cc index 4c6e1b5562..e0bd3dac04 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_message.cc @@ -180,7 +180,10 @@ MessageGenerator::MessageGenerator(const string& root_classname, : root_classname_(root_classname), descriptor_(descriptor), field_generators_(descriptor, options), - class_name_(ClassName(descriptor_)) { + class_name_(ClassName(descriptor_)), + deprecated_attribute_( + GetOptionalDeprecatedAttribute(descriptor, descriptor->file(), false, true)) { + for (int i = 0; i < descriptor_->extension_count(); i++) { extension_generators_.push_back( new ExtensionGenerator(class_name_, descriptor_->extension(i))); @@ -339,7 +342,7 @@ void MessageGenerator::GenerateMessageHeader(io::Printer* printer) { printer->Print( "$comments$$deprecated_attribute$@interface $classname$ : GPBMessage\n\n", "classname", class_name_, - "deprecated_attribute", GetOptionalDeprecatedAttribute(descriptor_, false, true), + "deprecated_attribute", deprecated_attribute_, "comments", message_comments); vector seen_oneofs(descriptor_->oneof_decl_count(), 0); @@ -396,6 +399,14 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { "\n", "classname", class_name_); + if (!deprecated_attribute_.empty()) { + // No warnings when compiling the impl of this deprecated class. + printer->Print( + "#pragma clang diagnostic push\n" + "#pragma clang diagnostic ignored \"-Wdeprecated-implementations\"\n" + "\n"); + } + printer->Print("@implementation $classname$\n\n", "classname", class_name_); @@ -601,6 +612,12 @@ void MessageGenerator::GenerateSource(io::Printer* printer) { "}\n\n" "@end\n\n"); + if (!deprecated_attribute_.empty()) { + printer->Print( + "#pragma clang diagnostic pop\n" + "\n"); + } + for (int i = 0; i < descriptor_->field_count(); i++) { field_generators_.get(descriptor_->field(i)) .GenerateCFunctionImplementations(printer); diff --git a/src/google/protobuf/compiler/objectivec/objectivec_message.h b/src/google/protobuf/compiler/objectivec/objectivec_message.h index 910535ace3..0fb78bc024 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_message.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_message.h @@ -85,6 +85,7 @@ class MessageGenerator { const Descriptor* descriptor_; FieldGeneratorMap field_generators_; const string class_name_; + const string deprecated_attribute_; vector extension_generators_; vector enum_generators_; vector nested_message_generators_;