Skip to content

Commit

Permalink
Deprecate and warn on skip-empty flag.
Browse files Browse the repository at this point in the history
This is now the default behavior.
  • Loading branch information
darronschall committed May 28, 2024
1 parent e4f8100 commit dd26ed5
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 22 deletions.
8 changes: 1 addition & 7 deletions lib/twirp/protoc_plugin/code_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ module ProtocPlugin
class CodeGenerator
# @param proto_file [Google::Protobuf::FileDescriptorProto]
# @param relative_ruby_protobuf [String] e.g. "example_rb.pb"
# @param options [Hash{Symbol => Boolean, Symbol}]
# * :skip_empty [Boolean] indicating whether generation should skip creating a twirp file
# for proto files that contain no services.
# @param options [Hash{Symbol => Symbol}]
# * :generate [Symbol] one of: :service, :client, or :both.
def initialize(proto_file, relative_ruby_protobuf, options)
@proto_file = proto_file
Expand Down Expand Up @@ -42,10 +40,6 @@ def generate
indent_level += 1
end

unless @proto_file.has_service?
output << line("# No services found; To skip generating this file, specify `--twirp_ruby_opt=skip-empty`.", indent_level)
end

@proto_file.service.each_with_index do |service, index| # service: <Google::Protobuf::ServiceDescriptorProto>
# Add newline between definitions when multiple are generated
output << "\n" if index > 0
Expand Down
38 changes: 23 additions & 15 deletions lib/twirp/protoc_plugin/process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,18 @@ class << self
def process(input)
request = Google::Protobuf::Compiler::CodeGeneratorRequest.decode(input)

options = extract_options(request.parameter)
options, warnings = extract_options(request.parameter)

warnings.each do |warning|
$stderr << "WARNING: #{warning}"
end

response = Google::Protobuf::Compiler::CodeGeneratorResponse.new
response.supported_features = Google::Protobuf::Compiler::CodeGeneratorResponse::Feature::FEATURE_PROTO3_OPTIONAL

request.proto_file.each do |proto_file| # proto_file: <Google::Protobuf::FileDescriptorProto>
next unless request.file_to_generate.include?(proto_file.name)
next if options[:skip_empty] && !proto_file.has_service?
next unless proto_file.has_service? # do not generate when no services defined

file = Google::Protobuf::Compiler::CodeGeneratorResponse::File.new
file.name = proto_file.twirp_output_filename
Expand All @@ -39,19 +43,24 @@ def process(input)

private

# @param params [String] the parameters from protoc command line in comma-separated stringified
# array format, e.g. "some-flag,key1=value1".
# @param params [String] the parameters from `protoc` command line in comma-separated stringified
# array format, e.g. "some-flag,key1=value1". For repeated command line options, `protoc` will
# add the option multiple times, e.g. "some-flag,key1=value1,key1=twice,key2=value2".
#
# The only valid parameter is currently the optional "generate" parameter.
#
# @return [Array(Hash{Symbol => Symbol}, Array<String>?)] the extracted options as the first
# element, and an array of any applicable warnings as the second element (or nil when no
# warnings are present).
#
# The first return value is a Hash that contains:
# * :generate [Symbol] one of: :service, :client, or :both. Default :both.
#
# The only valid parameter is currently the optional "skip-empty" flag.
# @return [Hash{Symbol => Boolean, Symbol}]
# * :skip_empty [Boolean] indicating whether generation should skip creating a twirp file
# for proto files that contain no services. Default false.
# * :generate [Symbol] one of: :service, :client, or :both. Default :both.
# @raise [ArgumentError] when a required parameter is missing, a parameter value is invalid, or
# an unrecognized parameter is present on the command line
def extract_options(params)
warnings = []
opts = {
skip_empty: false,
generate: :both
}

Expand All @@ -61,10 +70,9 @@ def extract_options(params)
# Limit the split to just separate the key out.
key, value = param.split("=", 2)
if key == "skip-empty"
unless value.nil? || value.empty?
raise ArgumentError, "Unexpected value passed to skip-empty flag: #{value}"
end
opts[:skip_empty] = true
# Ignore; this is now the default behavior. A future release will remove
# this flag entirely, and will raise an error instead of a warning.
warnings << "The `skip-empty` flag is deprecated and will be removed next release; it is now the default behavior."
elsif key == "generate"
if value.nil? || value.empty?
raise ArgumentError, "Unexpected missing value for generate option. Please supply one of: service, client, both."
Expand All @@ -81,7 +89,7 @@ def extract_options(params)
end
end

opts
[opts, warnings]
end
end
end
Expand Down

0 comments on commit dd26ed5

Please sign in to comment.