[ruby/net-smtp] Net::SMTP.start arguments are keyword arguments

The helo argument is not important, but the helo argument must be
specified to specify the user and secret arguments.
If helo, user, secret, and authtype arguments are keyword arguments,
it is not necessary to specify the helo argument.

269774deac

lib/net/smtp.rb

@@ -146,8 +146,8 @@ module Net
   # The SMTP server will judge whether it should send or reject
   # the SMTP session by inspecting the HELO domain.
   #
-  #     Net::SMTP.start('your.smtp.server', 25,
+  #     Net::SMTP.start('your.smtp.server', 25
-  #                     'mail.from.domain') { |smtp| ... }
+  #                     helo: 'mail.from.domain') { |smtp| ... }
   #
   # === SMTP Authentication
   #
@@ -157,15 +157,15 @@ module Net
   # SMTP.start/SMTP#start.
   #
   #     # PLAIN
-  #     Net::SMTP.start('your.smtp.server', 25, 'mail.from.domain',
+  #     Net::SMTP.start('your.smtp.server', 25
-  #                     'Your Account', 'Your Password', :plain)
+  #                     user: 'Your Account', secret: 'Your Password', authtype: :plain)
   #     # LOGIN
-  #     Net::SMTP.start('your.smtp.server', 25, 'mail.from.domain',
+  #     Net::SMTP.start('your.smtp.server', 25
-  #                     'Your Account', 'Your Password', :login)
+  #                     user: 'Your Account', secret: 'Your Password', authtype: :login)
   #
   #     # CRAM MD5
-  #     Net::SMTP.start('your.smtp.server', 25, 'mail.from.domain',
+  #     Net::SMTP.start('your.smtp.server', 25
-  #                     'Your Account', 'Your Password', :cram_md5)
+  #                     user: 'Your Account', secret: 'Your Password', authtype: :cram_md5)
   #
   class SMTP < Protocol
     VERSION = "0.1.0"
@@ -401,12 +401,16 @@ module Net
     # SMTP session control
     #
 
+    #
+    # :call-seq:
+    #  start(address, port = nil, helo: 'localhost', user: nil, secret: nil, authtype: nil) { |smtp| ... }
+    #  start(address, port = nil, helo = 'localhost', user = nil, secret = nil, authtype = nil) { |smtp| ... }
     #
     # Creates a new Net::SMTP object and connects to the server.
     #
     # This method is equivalent to:
     #
-    #   Net::SMTP.new(address, port).start(helo_domain, account, password, authtype)
+    #   Net::SMTP.new(address, port).start(helo: helo_domain, user: account, secret: password, authtype: authtype)
     #
     # === Example
     #
@@ -450,10 +454,15 @@ module Net
     # * Net::ReadTimeout
     # * IOError
     #
-    def SMTP.start(address, port = nil, helo = 'localhost',
+    def SMTP.start(address, port = nil, *args, helo: nil,
-                   user = nil, secret = nil, authtype = nil,
+                   user: nil, secret: nil, password: nil, authtype: nil,
-                   &block)   # :yield: smtp
+                   &block)
-      new(address, port).start(helo, user, secret, authtype, &block)
+      raise ArgumentError, "wrong number of arguments (given #{args.size + 2}, expected 1..6)" if args.size > 4
+      helo ||= args[0] || 'localhost'
+      user ||= args[1]
+      secret ||= password || args[2]
+      authtype ||= args[3]
+      new(address, port).start(helo: helo, user: user, secret: secret, authtype: authtype, &block)
     end
 
     # +true+ if the SMTP session has been started.
@@ -461,6 +470,10 @@ module Net
       @started
     end
 
+    #
+    # :call-seq:
+    #  start(helo: 'localhost', user: nil, secret: nil, authtype: nil) { |smtp| ... }
+    #  start(helo = 'localhost', user = nil, secret = nil, authtype = nil) { |smtp| ... }
     #
     # Opens a TCP connection and starts the SMTP session.
     #
@@ -488,7 +501,7 @@ module Net
     #
     #     require 'net/smtp'
     #     smtp = Net::SMTP.new('smtp.mail.server', 25)
-    #     smtp.start(helo_domain, account, password, authtype) do |smtp|
+    #     smtp.start(helo: helo_domain, user: account, secret: password, authtype: authtype) do |smtp|
     #       smtp.send_message msgstr, 'from@example.com', ['dest@example.com']
     #     end
     #
@@ -512,8 +525,13 @@ module Net
     # * Net::ReadTimeout
     # * IOError
     #
-    def start(helo = 'localhost',
+    def start(*args, helo: nil,
-              user = nil, secret = nil, authtype = nil)   # :yield: smtp
+              user: nil, secret: nil, password: nil, authtype: nil)
+      raise ArgumentError, "wrong number of arguments (given #{args.size}, expected 0..4)" if args.size > 4
+      helo ||= args[0] || 'localhost'
+      user ||= args[1]
+      secret ||= password || args[2]
+      authtype ||= args[3]
       if block_given?
         begin
           do_start helo, user, secret, authtype

test/net/smtp/test_smtp.rb

@@ -184,17 +184,99 @@ module Net
       end
     end
 
+    def test_start
+      port = fake_server_start
+      smtp = Net::SMTP.start('localhost', port)
+      smtp.quit
+    end
+
+    def test_start_with_position_argument
+      port = fake_server_start(helo: 'myname', user: 'account', password: 'password')
+      smtp = Net::SMTP.start('localhost', port, 'myname', 'account', 'password', :plain)
+      smtp.quit
+    end
+
+    def test_start_with_keyword_argument
+      port = fake_server_start(helo: 'myname', user: 'account', password: 'password')
+      smtp = Net::SMTP.start('localhost', port, helo: 'myname', user: 'account', secret: 'password', authtype: :plain)
+      smtp.quit
+    end
+
+    def test_start_password_is_secret
+      port = fake_server_start(helo: 'myname', user: 'account', password: 'password')
+      smtp = Net::SMTP.start('localhost', port, helo: 'myname', user: 'account', password: 'password', authtype: :plain)
+      smtp.quit
+    end
+
+    def test_start_invalid_number_of_arguments
+      err = assert_raise ArgumentError do
+        Net::SMTP.start('localhost', 25, 'myname', 'account', 'password', :plain, :invalid_arg)
+      end
+      assert_equal('wrong number of arguments (given 7, expected 1..6)', err.message)
+    end
+
+    def test_start_instance
+      port = fake_server_start
+      smtp = Net::SMTP.new('localhost', port)
+      smtp.start
+      smtp.quit
+    end
+
+    def test_start_instance_with_position_argument
+      port = fake_server_start(helo: 'myname', user: 'account', password: 'password')
+      smtp = Net::SMTP.new('localhost', port)
+      smtp.start('myname', 'account', 'password', :plain)
+      smtp.quit
+    end
+
+    def test_start_instance_with_keyword_argument
+      port = fake_server_start(helo: 'myname', user: 'account', password: 'password')
+      smtp = Net::SMTP.new('localhost', port)
+      smtp.start(helo: 'myname', user: 'account', secret: 'password', authtype: :plain)
+      smtp.quit
+    end
+
+    def test_start_instance_password_is_secret
+      port = fake_server_start(helo: 'myname', user: 'account', password: 'password')
+      smtp = Net::SMTP.new('localhost', port)
+      smtp.start(helo: 'myname', user: 'account', password: 'password', authtype: :plain)
+      smtp.quit
+    end
+
+    def test_start_instance_invalid_number_of_arguments
+      smtp = Net::SMTP.new('localhost')
+      err = assert_raise ArgumentError do
+        smtp.start('myname', 'account', 'password', :plain, :invalid_arg)
+      end
+      assert_equal('wrong number of arguments (given 5, expected 0..4)', err.message)
+    end
+
     private
 
     def accept(servers)
-      loop do
+      Socket.accept_loop(servers) { |s, _| break s }
-        readable, = IO.select(servers.map(&:to_io))
-        readable.each do |r|
-          sock, = r.accept_nonblock(exception: false)
-          next if sock == :wait_readable
-          return sock
     end
+
+    def fake_server_start(helo: 'localhost', user: nil, password: nil)
+      servers = Socket.tcp_server_sockets('localhost', 0)
+      Thread.start do
+        Thread.current.abort_on_exception = true
+        sock = accept(servers)
+        sock.puts "220 ready\r\n"
+        assert_equal("EHLO #{helo}\r\n", sock.gets)
+        sock.puts "220-servername\r\n220 AUTH PLAIN\r\n"
+        if user
+          credential = ["\0#{user}\0#{password}"].pack('m0')
+          assert_equal("AUTH PLAIN #{credential}\r\n", sock.gets)
+          sock.puts "235 2.7.0 Authentication successful\r\n"
         end
+        assert_equal("QUIT\r\n", sock.gets)
+        sock.puts "221 2.0.0 Bye\r\n"
+        sock.close
+        servers.each(&:close)
+      end
+      port = servers[0].local_address.ip_port
+      return port
     end
   end
 end