Skip to content

Commit

Permalink
Improve ObjC deprecated annotation support.
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
thomasvl committed Dec 8, 2016
1 parent c836ad4 commit dad775b
Show file tree
Hide file tree
Showing 11 changed files with 216 additions and 6 deletions.
2 changes: 2 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
2 changes: 2 additions & 0 deletions objectivec/DevTools/compile_testing_protos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
2 changes: 2 additions & 0 deletions objectivec/Tests/GPBARCUnittestProtos.m
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions objectivec/Tests/GPBUnittestProtos.m
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
95 changes: 95 additions & 0 deletions objectivec/Tests/unittest_deprecated.proto
Original file line number Diff line number Diff line change
@@ -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;
}
76 changes: 76 additions & 0 deletions objectivec/Tests/unittest_deprecated_file.proto
Original file line number Diff line number Diff line change
@@ -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;
}
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/objectivec/objectivec_enum.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
14 changes: 12 additions & 2 deletions src/google/protobuf/compiler/objectivec/objectivec_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,18 @@ enum FlagType {
};

template<class TDescriptor>
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, " ");
Expand Down
21 changes: 19 additions & 2 deletions src/google/protobuf/compiler/objectivec/objectivec_message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down Expand Up @@ -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<char> seen_oneofs(descriptor_->oneof_decl_count(), 0);
Expand Down Expand Up @@ -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_);

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class MessageGenerator {
const Descriptor* descriptor_;
FieldGeneratorMap field_generators_;
const string class_name_;
const string deprecated_attribute_;
vector<ExtensionGenerator*> extension_generators_;
vector<EnumGenerator*> enum_generators_;
vector<MessageGenerator*> nested_message_generators_;
Expand Down

0 comments on commit dad775b

Please sign in to comment.