Skip to content

Commit

Permalink
[csharp proto plugin] Apply Obsolete attribute to deprecated services…
Browse files Browse the repository at this point in the history
… and methods in C# generated code (grpc#32414)

Apply Obsolete attribute to deprecated services and methods in C#
generated code

Fix for grpc#28597

- Deprecated support for enums and enum values is already fixed by
protocolbuffers/protobuf#10520 but this is not
yet released. It is fixed in Protocol Buffers v22.0-rc1 but the gRPC
repo currently has 21.12 as the protocol buffers submodule.

- Deprecated support for messages and fields already exists in the
protocol buffers compiler.

The fix in this PR adds `Obsolete` attribute to classes and methods for
deprecated services and methods within services. e.g.
```
service Greeter {
  option deprecated=true; // service level deprecated
  // Sends a greeting
  rpc SayHello (HelloRequest) returns (HelloReply) {
    option deprecated=true; // method level deprecated
  }
}
```

I couldn't find any protocol buffers plugin tests to update. Tested
locally.
  • Loading branch information
tonydnewell authored and XuanWang-Amos committed May 1, 2023
1 parent 198d54e commit 9114942
Show file tree
Hide file tree
Showing 7 changed files with 267 additions and 2 deletions.
19 changes: 18 additions & 1 deletion src/compiler/csharp_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ void GenerateGeneratedCodeAttribute(grpc::protobuf::io::Printer* printer) {
"null)]\n");
}

void GenerateObsoleteAttribute(grpc::protobuf::io::Printer* printer,
bool is_deprecated) {
// Mark the code deprecated using the [ObsoleteAttribute] attribute.
if (is_deprecated) {
printer->Print("[global::System.ObsoleteAttribute]\n");
}
}

template <typename DescriptorType>
bool GenerateDocCommentBody(grpc::protobuf::io::Printer* printer,
const DescriptorType* descriptor) {
Expand Down Expand Up @@ -439,6 +447,7 @@ void GenerateServerClass(Printer* out, const ServiceDescriptor* service) {
"/// <summary>Base class for server-side implementations of "
"$servicename$</summary>\n",
"servicename", GetServiceClassName(service));
GenerateObsoleteAttribute(out, service->options().deprecated());
out->Print(
"[grpc::BindServiceMethod(typeof($classname$), "
"\"BindService\")]\n",
Expand All @@ -450,6 +459,7 @@ void GenerateServerClass(Printer* out, const ServiceDescriptor* service) {
for (int i = 0; i < service->method_count(); i++) {
const MethodDescriptor* method = service->method(i);
GenerateDocCommentServerMethod(out, method);
GenerateObsoleteAttribute(out, method->options().deprecated());
GenerateGeneratedCodeAttribute(out);
out->Print(
"public virtual $returntype$ "
Expand All @@ -475,6 +485,7 @@ void GenerateServerClass(Printer* out, const ServiceDescriptor* service) {
void GenerateClientStub(Printer* out, const ServiceDescriptor* service) {
out->Print("/// <summary>Client for $servicename$</summary>\n", "servicename",
GetServiceClassName(service));
GenerateObsoleteAttribute(out, service->options().deprecated());
out->Print("public partial class $name$ : grpc::ClientBase<$name$>\n", "name",
GetClientClassName(service));
out->Print("{\n");
Expand Down Expand Up @@ -525,9 +536,11 @@ void GenerateClientStub(Printer* out, const ServiceDescriptor* service) {

for (int i = 0; i < service->method_count(); i++) {
const MethodDescriptor* method = service->method(i);
const bool is_deprecated = method->options().deprecated();
if (!method->client_streaming() && !method->server_streaming()) {
// unary calls have an extra synchronous stub method
GenerateDocCommentClientMethod(out, method, true, false);
GenerateObsoleteAttribute(out, is_deprecated);
GenerateGeneratedCodeAttribute(out);
out->Print(
"public virtual $response$ $methodname$($request$ request, "
Expand All @@ -551,6 +564,7 @@ void GenerateClientStub(Printer* out, const ServiceDescriptor* service) {

// overload taking CallOptions as a param
GenerateDocCommentClientMethod(out, method, true, true);
GenerateObsoleteAttribute(out, is_deprecated);
GenerateGeneratedCodeAttribute(out);
out->Print(
"public virtual $response$ $methodname$($request$ request, "
Expand All @@ -573,6 +587,7 @@ void GenerateClientStub(Printer* out, const ServiceDescriptor* service) {
method_name += "Async"; // prevent name clash with synchronous method.
}
GenerateDocCommentClientMethod(out, method, false, false);
GenerateObsoleteAttribute(out, is_deprecated);
GenerateGeneratedCodeAttribute(out);
out->Print(
"public virtual $returntype$ "
Expand All @@ -598,6 +613,7 @@ void GenerateClientStub(Printer* out, const ServiceDescriptor* service) {

// overload taking CallOptions as a param
GenerateDocCommentClientMethod(out, method, false, true);
GenerateObsoleteAttribute(out, is_deprecated);
GenerateGeneratedCodeAttribute(out);
out->Print(
"public virtual $returntype$ "
Expand Down Expand Up @@ -739,6 +755,7 @@ void GenerateService(Printer* out, const ServiceDescriptor* service,
bool internal_access) {
GenerateDocCommentBody(out, service);

GenerateObsoleteAttribute(out, service->options().deprecated());
out->Print("$access_level$ static partial class $classname$\n",
"access_level", GetAccessLevel(internal_access), "classname",
GetServiceClassName(service));
Expand Down Expand Up @@ -801,7 +818,7 @@ std::string GetServices(const FileDescriptor* file, bool generate_client,
out.PrintRaw(leading_comments.c_str());
}

out.Print("#pragma warning disable 0414, 1591, 8981\n");
out.Print("#pragma warning disable 0414, 1591, 8981, 0612\n");

out.Print("#region Designer generated code\n");
out.Print("\n");
Expand Down
22 changes: 22 additions & 0 deletions test/csharp/codegen/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ licenses(["notice"])

grpc_sh_test(
name = "csharp_codegen_simple_test",
size = "small",
srcs = ["csharp_codegen_simple_test.sh"],
data = [
"simple/expected/HelloworldGrpc.cs",
Expand All @@ -34,3 +35,24 @@ grpc_sh_test(
],
uses_polling = False,
)

grpc_sh_test(
name = "csharp_codegen_deprecated_test",
size = "small",
srcs = ["csharp_codegen_deprecated_test.sh"],
data = [
"deprecated/proto/depmethod.proto",
"deprecated/proto/depnothing.proto",
"deprecated/proto/depservice.proto",
"//src/compiler:grpc_csharp_plugin",
"@com_google_protobuf//:protoc",
],
tags = [
"no_windows",
"noasan",
"nomsan",
"notsan",
"noubsan",
],
uses_polling = False,
)
115 changes: 115 additions & 0 deletions test/csharp/codegen/csharp_codegen_deprecated_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
#!/bin/bash
# Copyright 2023 gRPC authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Run this script via bazel test
# It expects that protoc and grpc_csharp_plugin have already been built.

# Simple test - compare generated output to expected files

set -x

TESTNAME=deprecated

# protoc and grpc_csharp_plugin binaries are supplied as "data" in bazel
PROTOC=./external/com_google_protobuf/protoc
PLUGIN=./src/compiler/grpc_csharp_plugin

# where to find the test data
DATA_DIR=./test/csharp/codegen/${TESTNAME}

# output directory for the generated files
PROTO_OUT=./proto_out
rm -rf ${PROTO_OUT}
mkdir -p ${PROTO_OUT}

# run protoc and the plugin
$PROTOC \
--plugin=protoc-gen-grpc=$PLUGIN \
--csharp_out=${PROTO_OUT} \
--grpc_out=${PROTO_OUT} \
-I ${DATA_DIR}/proto \
${DATA_DIR}/proto/depnothing.proto \
${DATA_DIR}/proto/depservice.proto \
${DATA_DIR}/proto/depmethod.proto

# log the files generated
ls -l ./proto_out

# Rather than doing a diff against a known file, just using grep to
# check for some of the code changes when "deprecated" is specified.
# This isn't bullet proof but does avoid tests breaking when other
# codegen changes are made.

# For depnothing.proto there should zero "ObsoleteAttribute" found
nmatches=$(grep -c ObsoleteAttribute ${PROTO_OUT}/DepnothingGrpc.cs)
if [ "$nmatches" -ne 0 ]
then
echo >&2 "Unexpected ObsoleteAttribute in DepnothingGrpc.cs"
exit 1
fi

# For depservice.proto need to check ObsoleteAttribute added to three classes.
# First check ObsoleteAttribute exists in output
nmatches=$(grep -c ObsoleteAttribute ${PROTO_OUT}/DepserviceGrpc.cs)
if [ "$nmatches" -eq 0 ]
then
echo >&2 "Missing ObsoleteAttribute in DepserviceGrpc.cs"
exit 1
fi

# capture context after ObsoleteAttribute for further checking
CTX=$(grep -A 2 ObsoleteAttribute ${PROTO_OUT}/DepserviceGrpc.cs)

# Check ObsoleteAttribute before class GreeterServiceLevelDep
nmatches=$(echo "$CTX" | grep -c "class GreeterServiceLevelDep$" )
if [ "$nmatches" -ne 1 ]
then
echo >&2 "Missing ObsoleteAttribute on class GreeterServiceLevelDep"
exit 1
fi
# Check ObsoleteAttribute before class GreeterServiceLevelDepBase
nmatches=$(echo "$CTX" | grep -c "class GreeterServiceLevelDepBase$" )
if [ "$nmatches" -ne 1 ]
then
echo >&2 "Missing ObsoleteAttribute on class GreeterServiceLevelDepBase"
exit 1
fi
# Check ObsoleteAttribute before class GreeterServiceLevelDepClient
nmatches=$(echo "$CTX" | grep -c "class GreeterServiceLevelDepClient" )
if [ "$nmatches" -ne 1 ]
then
echo >&2 "Missing ObsoleteAttribute on class GreeterServiceLevelDepClient"
exit 1
fi

# For depmethod.proto need to check ObsoleteAttribute added in five places for SayHello method.
# Check ObsoleteAttribute exists in output
nmatches=$(grep -c ObsoleteAttribute ${PROTO_OUT}/DepmethodGrpc.cs)
if [ "$nmatches" -eq 0 ]
then
echo >&2 "Missing ObsoleteAttribute in DepmethodGrpc.cs"
exit 1
fi
# Check ObsoleteAttribute before SayHello methods
nmatches=$(grep -A 2 ObsoleteAttribute ${PROTO_OUT}/DepmethodGrpc.cs | grep -c SayHello)
if [ "$nmatches" -ne 5 ]
then
echo >&2 "Missing ObsoleteAttribute on SayHello method"
exit 1
fi

# Run one extra command to clear $? before exiting the script to prevent
# failing even when tests pass.
echo "Plugin test: ${TESTNAME}: passed."
38 changes: 38 additions & 0 deletions test/csharp/codegen/deprecated/proto/depmethod.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2023 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

package test.csharp.codegen.simple.proto.depmethod;

// Service with method deprecation
service GreeterMethodLevelDep {
// Sends a greeting - method is deprecated
rpc SayHello (HelloRequest) returns (HelloReply) {
option deprecated = true; // method level option
}

rpc SayHelloStreamReply (HelloRequest) returns (stream HelloReply) {}
}

// The request message containing the user's name.
message HelloRequest {
string name = 1;
}

// The response message containing the greetings
message HelloReply {
string message = 1;
}

36 changes: 36 additions & 0 deletions test/csharp/codegen/deprecated/proto/depnothing.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2023 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

package test.csharp.codegen.simple.proto.depnothing;

// The greeting service definition - with nothing deprecated
service Greeter {
// Sends a greeting
rpc SayHello (HelloRequest) returns (HelloReply) {}

rpc SayHelloStreamReply (HelloRequest) returns (stream HelloReply) {}
}

// The request message containing the user's name.
message HelloRequest {
string name = 1;
}

// The response message containing the greetings
message HelloReply {
string message = 1;
}

37 changes: 37 additions & 0 deletions test/csharp/codegen/deprecated/proto/depservice.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2023 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

package test.csharp.codegen.simple.proto.depservice;

// Service with service-level deprecation
service GreeterServiceLevelDep {
option deprecated = true; // service level option
// Sends a greeting
rpc SayHello (HelloRequest) returns (HelloReply) {}

rpc SayHelloStreamReply (HelloRequest) returns (stream HelloReply) {}
}

// The request message containing the user's name.
message HelloRequest {
string name = 1;
}

// The response message containing the greetings
message HelloReply {
string message = 1;
}

2 changes: 1 addition & 1 deletion test/csharp/codegen/simple/expected/HelloworldGrpc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//
#pragma warning disable 0414, 1591, 8981
#pragma warning disable 0414, 1591, 8981, 0612
#region Designer generated code

using grpc = global::Grpc.Core;
Expand Down

0 comments on commit 9114942

Please sign in to comment.