Adds rubocop and fixes most style violations it detected

- add rubocop as a dev dependency
- fixed many style violations it reported, often using --auto-correct
- add a rubocop config
 - .rubocop.yml shows the exceptions
 - .rubocopy_todo.yml tracks outstanding style issues
- adds a rake task to allow rubocop styling checks to be automated

6 files changed, 93 insertions, 96 deletions

diff --git a/src/ruby/bin/interop/interop_client.rb b/src/ruby/bin/interop/interop_client.rb
index 718b0fdb83..0ce10d9e30 100755
--- a/src/ruby/bin/interop/interop_client.rb
+++ b/src/ruby/bin/interop/interop_client.rb
@@ -65,7 +65,7 @@ end
 # creates a Credentials from the test certificates.
 def test_creds
   certs = load_test_certs
-  creds = GRPC::Core::Credentials.new(certs[0])
+  GRPC::Core::Credentials.new(certs[0])
 end
 
 # creates a test stub that accesses host:port securely.
@@ -73,15 +73,15 @@ def create_stub(host, port)
   address = "#{host}:#{port}"
   stub_opts = {
     :creds => test_creds,
-    GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com',
+    GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com'
   }
   logger.info("... connecting securely to #{address}")
-  stub = Grpc::Testing::TestService::Stub.new(address, **stub_opts)
+  Grpc::Testing::TestService::Stub.new(address, **stub_opts)
 end
 
 # produces a string of null chars (\0) of length l.
 def nulls(l)
-  raise 'requires #{l} to be +ve' if l < 0
+  fail 'requires #{l} to be +ve' if l < 0
   [].pack('x' * l).force_encoding('utf-8')
 end
 
@@ -102,13 +102,13 @@ class PingPongPlayer
 
   def each_item
     return enum_for(:each_item) unless block_given?
-    req_cls, p_cls= StreamingOutputCallRequest, ResponseParameters  # short
+    req_cls, p_cls = StreamingOutputCallRequest, ResponseParameters  # short
     count = 0
     @msg_sizes.each do |m|
       req_size, resp_size = m
-      req = req_cls.new(:payload => Payload.new(:body => nulls(req_size)),
-                        :response_type => COMPRESSABLE,
-                        :response_parameters => [p_cls.new(:size => resp_size)])
+      req = req_cls.new(payload: Payload.new(body: nulls(req_size)),
+                        response_type: COMPRESSABLE,
+                        response_parameters: [p_cls.new(size: resp_size)])
       yield req
       resp = @queue.pop
       assert_equal(PayloadType.lookup(COMPRESSABLE), resp.payload.type,
@@ -148,11 +148,11 @@ class NamedTests
   #   ruby server
   # FAILED
   def large_unary
-    req_size, wanted_response_size = 271828, 314159
-    payload = Payload.new(:type => COMPRESSABLE, :body => nulls(req_size))
-    req = SimpleRequest.new(:response_type => COMPRESSABLE,
-                            :response_size => wanted_response_size,
-                            :payload => payload)
+    req_size, wanted_response_size = 271_828, 314_159
+    payload = Payload.new(type: COMPRESSABLE, body: nulls(req_size))
+    req = SimpleRequest.new(response_type: COMPRESSABLE,
+                            response_size: wanted_response_size,
+                            payload: payload)
     resp = @stub.unary_call(req)
     assert_equal(wanted_response_size, resp.payload.body.length,
                  'large_unary: payload had the wrong length')
@@ -166,27 +166,27 @@ class NamedTests
   #   ruby server
   # FAILED
   def client_streaming
-    msg_sizes = [27182, 8, 1828, 45904]
-    wanted_aggregate_size = 74922
+    msg_sizes = [27_182, 8, 1828, 45_904]
+    wanted_aggregate_size = 74_922
     reqs = msg_sizes.map do |x|
-      req = Payload.new(:body => nulls(x))
-      StreamingInputCallRequest.new(:payload => req)
+      req = Payload.new(body: nulls(x))
+      StreamingInputCallRequest.new(payload: req)
     end
     resp = @stub.streaming_input_call(reqs)
     assert_equal(wanted_aggregate_size, resp.aggregated_payload_size,
                  'client_streaming: aggregate payload size is incorrect')
     p 'OK: client_streaming'
-   end
+  end
 
   # TESTING:
   # PASSED
   #   ruby server
   # FAILED
   def server_streaming
-    msg_sizes = [31415, 9, 2653, 58979]
-    response_spec = msg_sizes.map { |s| ResponseParameters.new(:size => s) }
-    req = StreamingOutputCallRequest.new(:response_type => COMPRESSABLE,
-                                         :response_parameters => response_spec)
+    msg_sizes = [31_415, 9, 2653, 58_979]
+    response_spec = msg_sizes.map { |s| ResponseParameters.new(size: s) }
+    req = StreamingOutputCallRequest.new(response_type: COMPRESSABLE,
+                                         response_parameters: response_spec)
     resps = @stub.streaming_output_call(req)
     resps.each_with_index do |r, i|
       assert i < msg_sizes.length, 'too many responses'
@@ -203,13 +203,12 @@ class NamedTests
   #   ruby server
   # FAILED
   def ping_pong
-    msg_sizes = [[27182, 31415], [8, 9], [1828, 2653], [45904, 58979]]
+    msg_sizes = [[27_182, 31_415], [8, 9], [1828, 2653], [45_904, 58_979]]
     ppp = PingPongPlayer.new(msg_sizes)
     resps = @stub.full_duplex_call(ppp.each_item)
     resps.each { |r| ppp.queue.push(r) }
     p 'OK: ping_pong'
   end
-
 end
 
 # validates the the command line options, returning them as a Hash.
@@ -217,7 +216,7 @@ def parse_options
   options = {
     'server_host' => nil,
     'server_port' => nil,
-    'test_case' => nil,
+    'test_case' => nil
   }
   OptionParser.new do |opts|
     opts.banner = 'Usage: --server_host <server_host> --server_port server_port'
@@ -228,17 +227,17 @@ def parse_options
       options['server_port'] = v
     end
     # instance_methods(false) gives only the methods defined in that class
-    test_cases = NamedTests.instance_methods(false).map { |t| t.to_s }
+    test_cases = NamedTests.instance_methods(false).map(&:to_s)
     test_case_list = test_cases.join(',')
-    opts.on("--test_case CODE", test_cases, {}, "select a test_case",
+    opts.on('--test_case CODE', test_cases, {}, 'select a test_case',
             "  (#{test_case_list})") do |v|
       options['test_case'] = v
     end
   end.parse!
 
-  ['server_host', 'server_port', 'test_case'].each do |arg|
+  %w(server_host, server_port, test_case).each do |arg|
     if options[arg].nil?
-      raise OptionParser::MissingArgument.new("please specify --#{arg}")
+      fail(OptionParser::MissingArgument, "please specify --#{arg}")
     end
   end
   options
diff --git a/src/ruby/bin/interop/interop_server.rb b/src/ruby/bin/interop/interop_server.rb
index 63071f3ec2..9273dcdf91 100755
--- a/src/ruby/bin/interop/interop_server.rb
+++ b/src/ruby/bin/interop/interop_server.rb
@@ -62,12 +62,12 @@ end
 # creates a ServerCredentials from the test certificates.
 def test_server_creds
   certs = load_test_certs
-  server_creds = GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
+  GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
 end
 
 # produces a string of null chars (\0) of length l.
 def nulls(l)
-  raise 'requires #{l} to be +ve' if l < 0
+  fail 'requires #{l} to be +ve' if l < 0
   [].pack('x' * l).force_encoding('utf-8')
 end
 
@@ -86,7 +86,7 @@ class EnumeratorQueue
     loop do
       r = @q.pop
       break if r.equal?(@sentinel)
-      raise r if r.is_a?Exception
+      fail r if r.is_a? Exception
       yield r
     end
   end
@@ -98,27 +98,27 @@ class TestTarget < Grpc::Testing::TestService::Service
   include Grpc::Testing
   include Grpc::Testing::PayloadType
 
-  def empty_call(empty, call)
+  def empty_call(_empty, _call)
     Empty.new
   end
 
-  def unary_call(simple_req, call)
+  def unary_call(simple_req, _call)
     req_size = simple_req.response_size
-    SimpleResponse.new(:payload => Payload.new(:type => COMPRESSABLE,
-                                               :body => nulls(req_size)))
+    SimpleResponse.new(payload: Payload.new(type: COMPRESSABLE,
+                                            body: nulls(req_size)))
   end
 
   def streaming_input_call(call)
     sizes = call.each_remote_read.map { |x| x.payload.body.length }
-    sum = sizes.inject { |sum,x| sum + x }
-    StreamingInputCallResponse.new(:aggregated_payload_size => sum)
+    sum = sizes.inject { |s, x| s + x }
+    StreamingInputCallResponse.new(aggregated_payload_size: sum)
   end
 
-  def streaming_output_call(req, call)
+  def streaming_output_call(req, _call)
     cls = StreamingOutputCallResponse
     req.response_parameters.map do |p|
-      cls.new(:payload => Payload.new(:type => req.response_type,
-                                      :body => nulls(p.size)))
+      cls.new(payload: Payload.new(type: req.response_type,
+                                   body: nulls(p.size)))
     end
   end
 
@@ -126,13 +126,13 @@ class TestTarget < Grpc::Testing::TestService::Service
     # reqs is a lazy Enumerator of the requests sent by the client.
     q = EnumeratorQueue.new(self)
     cls = StreamingOutputCallResponse
-    t = Thread.new do
+    Thread.new do
       begin
         reqs.each do |req|
           logger.info("read #{req.inspect}")
           resp_size = req.response_parameters[0].size
-          resp = cls.new(:payload => Payload.new(:type => req.response_type,
-                                                 :body => nulls(resp_size)))
+          resp = cls.new(payload: Payload.new(type: req.response_type,
+                                              body: nulls(resp_size)))
           q.push(resp)
         end
         logger.info('finished reads')
@@ -149,13 +149,12 @@ class TestTarget < Grpc::Testing::TestService::Service
     # currently used in any tests
     full_duplex_call(reqs)
   end
-
 end
 
 # validates the the command line options, returning them as a Hash.
 def parse_options
   options = {
-    'port' => nil,
+    'port' => nil
   }
   OptionParser.new do |opts|
     opts.banner = 'Usage: --port port'
@@ -165,7 +164,7 @@ def parse_options
   end.parse!
 
   if options['port'].nil?
-    raise OptionParser::MissingArgument.new("please specify --port")
+    fail(OptionParser::MissingArgument, 'please specify --port')
   end
   options
 end
diff --git a/src/ruby/bin/math_client.rb b/src/ruby/bin/math_client.rb
index 4df333d085..195406c8b3 100755
--- a/src/ruby/bin/math_client.rb
+++ b/src/ruby/bin/math_client.rb
@@ -29,7 +29,6 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-
 # Sample app that accesses a Calc service running on a Ruby gRPC server and
 # helps validate RpcServer as a gRPC server using proto2 serialization.
 #
@@ -49,9 +48,9 @@ include GRPC::Core::TimeConsts
 def do_div(stub)
   logger.info('request_response')
   logger.info('----------------')
-  req = Math::DivArgs.new(:dividend => 7, :divisor => 3)
+  req = Math::DivArgs.new(dividend: 7, divisor: 3)
   logger.info("div(7/3): req=#{req.inspect}")
-  resp = stub.div(req, deadline=INFINITE_FUTURE)
+  resp = stub.div(req, INFINITE_FUTURE)
   logger.info("Answer: #{resp.inspect}")
   logger.info('----------------')
 end
@@ -60,7 +59,7 @@ def do_sum(stub)
   # to make client streaming requests, pass an enumerable of the inputs
   logger.info('client_streamer')
   logger.info('---------------')
-  reqs = [1, 2, 3, 4, 5].map { |x| Math::Num.new(:num => x) }
+  reqs = [1, 2, 3, 4, 5].map { |x| Math::Num.new(num: x) }
   logger.info("sum(1, 2, 3, 4, 5): reqs=#{reqs.inspect}")
   resp = stub.sum(reqs)  # reqs.is_a?(Enumerable)
   logger.info("Answer: #{resp.inspect}")
@@ -70,9 +69,9 @@ end
 def do_fib(stub)
   logger.info('server_streamer')
   logger.info('----------------')
-  req = Math::FibArgs.new(:limit => 11)
+  req = Math::FibArgs.new(limit: 11)
   logger.info("fib(11): req=#{req.inspect}")
-  resp = stub.fib(req, deadline=INFINITE_FUTURE)
+  resp = stub.fib(req, INFINITE_FUTURE)
   resp.each do |r|
     logger.info("Answer: #{r.inspect}")
   end
@@ -83,11 +82,11 @@ def do_div_many(stub)
   logger.info('bidi_streamer')
   logger.info('-------------')
   reqs = []
-  reqs << Math::DivArgs.new(:dividend => 7, :divisor => 3)
-  reqs << Math::DivArgs.new(:dividend => 5, :divisor => 2)
-  reqs << Math::DivArgs.new(:dividend => 7, :divisor => 2)
+  reqs << Math::DivArgs.new(dividend: 7, divisor: 3)
+  reqs << Math::Di5AvArgs.new(dividend: 5, divisor: 2)
+  reqs << Math::DivArgs.new(dividend: 7, divisor: 2)
   logger.info("div(7/3), div(5/2), div(7/2): reqs=#{reqs.inspect}")
-  resp = stub.div_many(reqs, deadline=10)
+  resp = stub.div_many(reqs, 10)
   resp.each do |r|
     logger.info("Answer: #{r.inspect}")
   end
@@ -103,7 +102,7 @@ end
 
 def test_creds
   certs = load_test_certs
-  creds = GRPC::Core::Credentials.new(certs[0])
+  GRPC::Core::Credentials.new(certs[0])
 end
 
 def main
@@ -117,7 +116,7 @@ def main
       options['host'] = v
     end
     opts.on('-s', '--secure', 'access using test creds') do |v|
-      options['secure'] = true
+      options['secure'] = v
     end
   end.parse!
 
@@ -128,7 +127,7 @@ def main
   if options['secure']
     stub_opts = {
       :creds => test_creds,
-      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com',
+      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com'
     }
     p stub_opts
     p options['host']
diff --git a/src/ruby/bin/math_server.rb b/src/ruby/bin/math_server.rb
index 0e47f71e66..55ee1d3314 100755
--- a/src/ruby/bin/math_server.rb
+++ b/src/ruby/bin/math_server.rb
@@ -46,9 +46,8 @@ require 'optparse'
 
 # Holds state for a fibonacci series
 class Fibber
-
   def initialize(limit)
-    raise "bad limit: got #{limit}, want limit > 0" if limit < 1
+    fail "bad limit: got #{limit}, want limit > 0" if limit < 1
     @limit = limit
   end
 
@@ -57,14 +56,14 @@ class Fibber
     idx, current, previous = 0, 1, 1
     until idx == @limit
       if idx == 0 || idx == 1
-        yield Math::Num.new(:num => 1)
+        yield Math::Num.new(num: 1)
         idx += 1
         next
       end
       tmp = current
       current = previous + current
       previous = tmp
-      yield Math::Num.new(:num => current)
+      yield Math::Num.new(num: current)
       idx += 1
     end
   end
@@ -85,43 +84,41 @@ class EnumeratorQueue
     loop do
       r = @q.pop
       break if r.equal?(@sentinel)
-      raise r if r.is_a?Exception
+      fail r if r.is_a? Exception
       yield r
     end
   end
-
 end
 
 # The Math::Math:: module occurs because the service has the same name as its
 # package. That practice should be avoided by defining real services.
 class Calculator < Math::Math::Service
-
-  def div(div_args, call)
+  def div(div_args, _call)
     if div_args.divisor == 0
       # To send non-OK status handlers raise a StatusError with the code and
       # and detail they want sent as a Status.
-      raise GRPC::StatusError.new(GRPC::Status::INVALID_ARGUMENT,
-                                  'divisor cannot be 0')
+      fail GRPC::StatusError.new(GRPC::Status::INVALID_ARGUMENT,
+                                 'divisor cannot be 0')
     end
 
-    Math::DivReply.new(:quotient => div_args.dividend/div_args.divisor,
-                       :remainder => div_args.dividend % div_args.divisor)
+    Math::DivReply.new(quotient: div_args.dividend / div_args.divisor,
+                       remainder: div_args.dividend % div_args.divisor)
   end
 
   def sum(call)
     # the requests are accesible as the Enumerator call#each_request
-    nums = call.each_remote_read.collect { |x| x.num }
-    sum = nums.inject { |sum,x| sum + x }
-    Math::Num.new(:num => sum)
+    nums = call.each_remote_read.collect(&:num)
+    sum = nums.inject { |s, x| s + x }
+    Math::Num.new(num: sum)
   end
 
-  def fib(fib_args, call)
+  def fib(fib_args, _call)
     if fib_args.limit < 1
-      raise StatusError.new(Status::INVALID_ARGUMENT, 'limit must be >= 0')
+      fail StatusError.new(Status::INVALID_ARGUMENT, 'limit must be >= 0')
     end
 
     # return an Enumerator of Nums
-    Fibber.new(fib_args.limit).generator()
+    Fibber.new(fib_args.limit).generator
     # just return the generator, GRPC::GenericServer sends each actual response
   end
 
@@ -132,10 +129,10 @@ class Calculator < Math::Math::Service
       begin
         requests.each do |req|
           logger.info("read #{req.inspect}")
-          resp = Math::DivReply.new(:quotient => req.dividend/req.divisor,
-                                    :remainder => req.dividend % req.divisor)
+          resp = Math::DivReply.new(quotient: req.dividend / req.divisor,
+                                    remainder: req.dividend % req.divisor)
           q.push(resp)
-          Thread::pass  # let the internal Bidi threads run
+          Thread.pass  # let the internal Bidi threads run
         end
         logger.info('finished reads')
         q.push(self)
@@ -147,7 +144,6 @@ class Calculator < Math::Math::Service
     t.priority = -2  # hint that the div_many thread should not be favoured
     q.each_item
   end
-
 end
 
 def load_test_certs
@@ -159,7 +155,7 @@ end
 
 def test_server_creds
   certs = load_test_certs
-  server_creds = GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
+  GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
 end
 
 def main
@@ -173,7 +169,7 @@ def main
       options['host'] = v
     end
     opts.on('-s', '--secure', 'access using test creds') do |v|
-      options['secure'] = true
+      options['secure'] = v
     end
   end.parse!
 
diff --git a/src/ruby/bin/noproto_client.rb b/src/ruby/bin/noproto_client.rb
index 34bdf545ee..74bdfbb93a 100755
--- a/src/ruby/bin/noproto_client.rb
+++ b/src/ruby/bin/noproto_client.rb
@@ -40,16 +40,18 @@ $LOAD_PATH.unshift(lib_dir) unless $LOAD_PATH.include?(lib_dir)
 require 'grpc'
 require 'optparse'
 
+# a simple non-protobuf message class.
 class NoProtoMsg
-  def self.marshal(o)
+  def self.marshal(_o)
     ''
   end
 
-  def self.unmarshal(o)
+  def self.unmarshal(_o)
     NoProtoMsg.new
   end
 end
 
+# service the uses the non-protobuf message class.
 class NoProtoService
   include GRPC::GenericService
   rpc :AnRPC, NoProtoMsg, NoProtoMsg
@@ -66,7 +68,7 @@ end
 
 def test_creds
   certs = load_test_certs
-  creds = GRPC::Core::Credentials.new(certs[0])
+  GRPC::Core::Credentials.new(certs[0])
 end
 
 def main
@@ -80,14 +82,14 @@ def main
       options['host'] = v
     end
     opts.on('-s', '--secure', 'access using test creds') do |v|
-      options['secure'] = true
+      options['secure'] = v
     end
   end.parse!
 
   if options['secure']
     stub_opts = {
       :creds => test_creds,
-      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com',
+      GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.com'
     }
     p stub_opts
     p options['host']
diff --git a/src/ruby/bin/noproto_server.rb b/src/ruby/bin/noproto_server.rb
index 1bdc192f02..e34075c1f0 100755
--- a/src/ruby/bin/noproto_server.rb
+++ b/src/ruby/bin/noproto_server.rb
@@ -40,26 +40,29 @@ $LOAD_PATH.unshift(lib_dir) unless $LOAD_PATH.include?(lib_dir)
 require 'grpc'
 require 'optparse'
 
+# a simple non-protobuf message class.
 class NoProtoMsg
-  def self.marshal(o)
+  def self.marshal(_o)
     ''
   end
 
-  def self.unmarshal(o)
+  def self.unmarshal(_o)
     NoProtoMsg.new
   end
 end
 
+# service the uses the non-protobuf message class.
 class NoProtoService
   include GRPC::GenericService
   rpc :AnRPC, NoProtoMsg, NoProtoMsg
 end
 
+# an implementation of the non-protobuf service.
 class NoProto < NoProtoService
-  def initialize(default_var='ignored')
+  def initialize(_default_var = 'ignored')
   end
 
-  def an_rpc(req, call)
+  def an_rpc(req, _call)
     logger.info('echo service received a request')
     req
   end
@@ -74,7 +77,7 @@ end
 
 def test_server_creds
   certs = load_test_certs
-  server_creds = GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
+  GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
 end
 
 def main
@@ -88,7 +91,7 @@ def main
       options['host'] = v
     end
     opts.on('-s', '--secure', 'access using test creds') do |v|
-      options['secure'] = true
+      options['secure'] = v
     end
   end.parse!
 
@@ -106,5 +109,4 @@ def main
   s.run
 end
 
-
 main