Merge pull request #40 from collectiveidea/make-skip-empty-the-defaul…

…t-and-remove-flag

Make `skip-empty` the default behavior and deprecate option

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## [Unreleased]
 
+- Make `skip-empty` the default behavior; remove recognizing the option flag - [#40](https://github.com/collectiveidea/protoc-gen-twirp_ruby/pull/40) 
 - Update GitHub action to run specs on all supported Ruby versions - [#37](https://github.com/collectiveidea/protoc-gen-twirp_ruby/pull/37)
 
 ## [1.1.1] - 2024-05-22

README.md

@@ -53,18 +53,16 @@ protoc --proto_path=. --ruby_out=. --twirp_ruby_out=. ./path/to/service.proto
 
 You can configure the code generation. Pass options by specifying `--twirp_ruby_opt=<option>` on the `protoc` command line.
 
- * `skip-empty`: Avoid generating a `_twirp.rb` for a `.proto` with no service definitions. By default, a `_twirp.rb`
-   file is generated for every proto file listed on the command line, even if the file is empty scaffolding. 
- * `generate=<service|client|both>`: Customize generated output to include generated services, clients, or both.
-   * `generate=service` - only generate `::Twirp::Service` subclass(es).
-   * `generate=client` - only generate `::Twirp::Client` subclass(es).
-   * `generate=both` - generate both services and clients. This is the default option to preserve
-     backwards compatibility.
+ * `generate=<service|client|both>`: Customize generated output to include generated services, clients, or both. Optional,
+    defaults to both to preserve backwards compatibility.
+   * `generate=service` - only generate `::Twirp::Service`s.
+   * `generate=client` - only generate `::Twirp::Client`s.
+   * `generate=both` - default; generate both services and clients. 
 
-Example (with two options): 
+Example : 
 
 ```bash
-protoc --proto_path=. --ruby_out=. --twirp_ruby_out=. --twirp_ruby_opt=generate=client --twirp_ruby_opt=skip-empty ./path/to/service.proto
+protoc --proto_path=. --ruby_out=. --twirp_ruby_out=. --twirp_ruby_opt=generate=client ./path/to/service.proto
 ```
 
 ## Migrating from the Go module
@@ -84,7 +82,7 @@ that might affect migration include:
  * Generated output code is in [standardrb style](https://github.com/standardrb/standard).
  * Generated service and client class names are improved for well-named protobuf services. See [#6](https://github.com/collectiveidea/protoc-gen-twirp_ruby/pull/6).
  * Supports `ruby_package` in `.proto` files
- * Supports various protoc command line [configuration options](#options).
+ * Supports protoc command line [configuration options](#options).
 
 ## Development
 

lib/twirp/protoc_plugin/code_generator.rb

@@ -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
@@ -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

lib/twirp/protoc_plugin/process.rb

@@ -25,7 +25,7 @@ def process(input)
 
         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
@@ -39,19 +39,19 @@ 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 "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.
+      #   The only valid parameter is currently the optional "generate" parameter.
+      #
+      # @return Hash{Symbol => Symbol} the extracted options, a Hash that contains:
       #   * :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)
         opts = {
-          skip_empty: false,
           generate: :both
         }
 
@@ -60,12 +60,7 @@ def extract_options(params)
           # In the event value contains an =, we want to leave that intact.
           # 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
-          elsif key == "generate"
+          if key == "generate"
             if value.nil? || value.empty?
               raise ArgumentError, "Unexpected missing value for generate option. Please supply one of: service, client, both."
             end

spec/spec_helper.rb

@@ -9,7 +9,6 @@
 
 require "rspec/file_fixtures"
 require "twirp/protoc_plugin"
-require "support/matchers/be_empty_scaffolding_matcher"
 
 RSpec.configure do |config|
   # Enable flags like --only-failures and --next-failure

spec/twirp/protoc_plugin/process_spec.rb

@@ -15,18 +15,6 @@
         end
       end
 
-      context "when passing an invalid value for the skip-empty flag" do
-        # Generate code gen request fixture:
-        #   `./spec/support/create_fixture -b -p skip-empty=true -f service_code_gen_request_invalid_skip_empty_value_param_pb.bin ./spec/fixtures/service.proto`
-        let(:request_pb) { fixture("service_code_gen_request_invalid_skip_empty_value_param_pb.bin").read }
-
-        it "raises an argument error" do
-          expect {
-            Twirp::ProtocPlugin.process(request_pb)
-          }.to raise_error(ArgumentError, "Unexpected value passed to skip-empty flag: true")
-        end
-      end
-
       context "when passing an empty value for the generate flag" do
         # Generate code gen request fixture:
         #   `./spec/support/create_fixture -b -p generate= -f service_code_gen_request_generate_param_missing_value_pb.bin ./spec/fixtures/service.proto`
@@ -137,76 +125,13 @@ class HelloWorldClient < ::Twirp::Client
     end
 
     context "when using a complex example with imported types and multiple services" do
-      # Generate code gen request fixture:
-      #   `./spec/support/create_fixture -b -f api_code_gen_request_pb.bin -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
-      let(:api_code_gen_request_pb) { fixture("complex_example/api_code_gen_request_pb.bin").read }
-
-      context "when omitting the `skip-empty` option", :aggregate_failures do
-        it "generates expected output" do
-          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_pb)
-          response = Google::Protobuf::Compiler::CodeGeneratorResponse.decode(response_pb)
-
-          expect(response.supported_features).to eq(Google::Protobuf::Compiler::CodeGeneratorResponse::Feature::FEATURE_PROTO3_OPTIONAL)
-          expect(response.file.size).to eq(4)
-
-          first_file = response.file[0]
-          expect(first_file.name).to eq("common/rpc/status_twirp.rb")
-          expect(first_file.content).to be_empty_scaffolding("common/rpc/status.proto", "status_pb", %w[Common RPC])
-
-          second_file = response.file[1]
-          expect(second_file.name).to eq("common/type/color_twirp.rb")
-          expect(second_file.content).to be_empty_scaffolding("common/type/color.proto", "color_pb", %w[Common Type])
-
-          third_file = response.file[2]
-          expect(third_file.name).to eq("common/type/time_of_day_twirp.rb")
-          expect(third_file.content).to be_empty_scaffolding("common/type/time_of_day.proto", "time_of_day_pb", %w[Common Type])
-
-          fourth_file = response.file[3]
-          expect(fourth_file.name).to eq("api_twirp.rb")
-          expect(fourth_file.content).to eq <<~EOF
-            # frozen_string_literal: true
-
-            # Generated by the protoc-gen-twirp_ruby gem v#{Twirp::ProtocPlugin::VERSION}. DO NOT EDIT!
-            # source: api.proto
-
-            require "twirp"
-            require_relative "api_pb"
-
-            module API
-              class GreetService < ::Twirp::Service
-                package "api"
-                service "GreetService"
-                rpc :SayHello, HelloRequest, HelloResponse, ruby_method: :say_hello
-                rpc :SayGoodbye, GoodbyeRequest, GoodbyeResponse, ruby_method: :say_goodbye
-                rpc :ChangeColor, ::Common::Type::Color, ChangeColorWrapper::Response, ruby_method: :change_color
-              end
-            
-              class GreetClient < ::Twirp::Client
-                client_for GreetService
-              end
-
-              class StatusService < ::Twirp::Service
-                package "api"
-                service "StatusService"
-                rpc :GetStatus, ::Google::Protobuf::Empty, ::Common::RPC::Status, ruby_method: :get_status
-                rpc :GetTimeOfDay, TimeOfDayRequest, ::Common::Type::TimeOfDay, ruby_method: :get_time_of_day
-              end
-            
-              class StatusClient < ::Twirp::Client
-                client_for StatusService
-              end
-            end
-          EOF
-        end
-      end
-
-      context "when specifying the `skip-empty` option" do
+      context "when run without any options" do
         # Generate code gen request fixture:
-        #   `./spec/support/create_fixture -b -f api_code_gen_request_skip_empty_pb.bin -p skip-empty -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
-        let(:api_code_gen_request_skip_empty_pb) { fixture("complex_example/api_code_gen_request_skip_empty_pb.bin").read }
+        #   `./spec/support/create_fixture -b -f api_code_gen_request_pb.bin -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
+        let(:api_code_gen_request_pb) { fixture("complex_example/api_code_gen_request_pb.bin").read }
 
         it "generates a single file containing two services and two clients" do
-          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_skip_empty_pb)
+          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_pb)
           response = Google::Protobuf::Compiler::CodeGeneratorResponse.decode(response_pb)
 
           expect(response.supported_features).to eq(Google::Protobuf::Compiler::CodeGeneratorResponse::Feature::FEATURE_PROTO3_OPTIONAL)
@@ -251,13 +176,13 @@ class StatusClient < ::Twirp::Client
         end
       end
 
-      context "when specifying the `skip-empty` and `generate=both` options" do
+      context "when specifying the `generate=both` option" do
         # Generate code gen request fixture:
-        #   `./spec/support/create_fixture -b -f api_code_gen_request_skip_empty_generate_both_pb.bin -p skip-empty,generate=both -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
-        let(:api_code_gen_request_skip_empty_generate_both_pb) { fixture("complex_example/api_code_gen_request_skip_empty_generate_both_pb.bin").read }
+        #   `./spec/support/create_fixture -b -f api_code_gen_request_generate_both_pb.bin -p generate=both -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
+        let(:api_code_gen_request_generate_both_pb) { fixture("complex_example/api_code_gen_request_generate_both_pb.bin").read }
 
         it "generates a single file containing two services and two clients" do
-          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_skip_empty_generate_both_pb)
+          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_generate_both_pb)
           response = Google::Protobuf::Compiler::CodeGeneratorResponse.decode(response_pb)
 
           expect(response.supported_features).to eq(Google::Protobuf::Compiler::CodeGeneratorResponse::Feature::FEATURE_PROTO3_OPTIONAL)
@@ -302,13 +227,13 @@ class StatusClient < ::Twirp::Client
         end
       end
 
-      context "when specifying the `skip-empty` and `generate=service` options" do
+      context "when specifying the `generate=service` option" do
         # Generate code gen request fixture:
-        #   `./spec/support/create_fixture -b -f api_code_gen_request_skip_empty_generate_service_pb.bin -p skip-empty,generate=service -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
-        let(:api_code_gen_request_skip_empty_generate_service_pb) { fixture("complex_example/api_code_gen_request_skip_empty_generate_service_pb.bin").read }
+        #   `./spec/support/create_fixture -b -f api_code_gen_request_generate_service_pb.bin -p generate=service -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
+        let(:api_code_gen_request_generate_service_pb) { fixture("complex_example/api_code_gen_request_generate_service_pb.bin").read }
 
         it "generates a single file with only two services" do
-          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_skip_empty_generate_service_pb)
+          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_generate_service_pb)
           response = Google::Protobuf::Compiler::CodeGeneratorResponse.decode(response_pb)
 
           expect(response.supported_features).to eq(Google::Protobuf::Compiler::CodeGeneratorResponse::Feature::FEATURE_PROTO3_OPTIONAL)
@@ -345,13 +270,13 @@ class StatusService < ::Twirp::Service
         end
       end
 
-      context "when specifying the `skip-empty` and `generate=client` options" do
+      context "when specifying the `generate=client` option" do
         # Generate code gen request fixture:
-        #   `./spec/support/create_fixture -b -f api_code_gen_request_skip_empty_generate_client_pb.bin -p skip-empty,generate=client -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
-        let(:api_code_gen_request_skip_empty_generate_client_pb) { fixture("complex_example/api_code_gen_request_skip_empty_generate_client_pb.bin").read }
+        #   `./spec/support/create_fixture -b -f api_code_gen_request_generate_client_pb.bin -p generate=client -o ./spec/fixtures/complex_example -I ./spec/fixtures/complex_example api.proto common/rpc/status.proto common/type/color.proto common/type/time_of_day.proto`
+        let(:api_code_gen_request_generate_client_pb) { fixture("complex_example/api_code_gen_request_generate_client_pb.bin").read }
 
         it "generates a single file with only two clients without the `client_for` DSL" do
-          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_skip_empty_generate_client_pb)
+          response_pb = Twirp::ProtocPlugin.process(api_code_gen_request_generate_client_pb)
           response = Google::Protobuf::Compiler::CodeGeneratorResponse.decode(response_pb)
 
           expect(response.supported_features).to eq(Google::Protobuf::Compiler::CodeGeneratorResponse::Feature::FEATURE_PROTO3_OPTIONAL)