Depricate IO operation with |

Author: nobu

io.c

@@ -8223,21 +8223,6 @@ rb_io_s_sysopen(int argc, VALUE *argv, VALUE _)
     return INT2NUM(fd);
 }
 
-static VALUE
-check_pipe_command(VALUE filename_or_command)
-{
-    char *s = RSTRING_PTR(filename_or_command);
-    long l = RSTRING_LEN(filename_or_command);
-    char *e = s + l;
-    int chlen;
-
-    if (rb_enc_ascget(s, e, &chlen, rb_enc_get(filename_or_command)) == '|') {
-        VALUE cmd = rb_str_new(s+chlen, l-chlen);
-        return cmd;
-    }
-    return Qnil;
-}
-
 /*
  *  call-seq:
  *    open(path, mode = 'r', perm = 0666, **opts)             -> io or nil
@@ -8283,13 +8268,7 @@ rb_f_open(int argc, VALUE *argv, VALUE _)
                 redirect = TRUE;
             }
             else {
-                VALUE cmd = check_pipe_command(tmp);
-                if (!NIL_P(cmd)) {
-                    // TODO: when removed in 4.0, update command_injection.rdoc
-                    rb_warn_deprecated_to_remove_at(4.0, "Calling Kernel#open with a leading '|'", "IO.popen");
-                    argv[0] = cmd;
-                    return rb_io_s_popen(argc, argv, rb_cIO);
-                }
+                argv[0] = tmp;
             }
         }
     }
@@ -8308,16 +8287,8 @@ static VALUE
 rb_io_open_generic(VALUE klass, VALUE filename, int oflags, enum rb_io_mode fmode,
                    const struct rb_io_encoding *convconfig, mode_t perm)
 {
-    VALUE cmd;
-    if (klass == rb_cIO && !NIL_P(cmd = check_pipe_command(filename))) {
-        // TODO: when removed in 4.0, update command_injection.rdoc
-        rb_warn_deprecated_to_remove_at(4.0, "IO process creation with a leading '|'", "IO.popen");
-        return pipe_open_s(cmd, rb_io_oflags_modestr(oflags), fmode, convconfig);
-    }
-    else {
-        return rb_file_open_generic(io_alloc(klass), filename,
-                                    oflags, fmode, convconfig, perm);
-    }
+    return rb_file_open_generic(io_alloc(klass), filename,
+                                oflags, fmode, convconfig, perm);
 }
 
 static VALUE

test/ruby/test_io.rb

@@ -2619,36 +2619,15 @@ def o.to_open(kw); kw; end
     assert_equal({:a=>1}, open(o, {a: 1}))
   end
 
-  def test_open_pipe
-    assert_deprecated_warning(/Kernel#open with a leading '\|'/) do # https://bugs.ruby-lang.org/issues/19630
-      open("|" + EnvUtil.rubybin, "r+") do |f|
-        f.puts "puts 'foo'"
-        f.close_write
-        assert_equal("foo\n", f.read)
-      end
-    end
-  end
+  def test_path_with_pipe
+    mkcdtmpdir do
+      cmd = "|echo foo"
+      assert_file.not_exist?(cmd)
 
-  def test_read_command
-    assert_deprecated_warning(/IO process creation with a leading '\|'/) do # https://bugs.ruby-lang.org/issues/19630
-      assert_equal("foo\n", IO.read("|echo foo"))
-    end
-    assert_raise(Errno::ENOENT, Errno::EINVAL) do
-      File.read("|#{EnvUtil.rubybin} -e puts")
-    end
-    assert_raise(Errno::ENOENT, Errno::EINVAL) do
-      File.binread("|#{EnvUtil.rubybin} -e puts")
-    end
-    assert_raise(Errno::ENOENT, Errno::EINVAL) do
-      Class.new(IO).read("|#{EnvUtil.rubybin} -e puts")
-    end
-    assert_raise(Errno::ENOENT, Errno::EINVAL) do
-      Class.new(IO).binread("|#{EnvUtil.rubybin} -e puts")
-    end
-    assert_raise(Errno::ESPIPE) do
-      assert_deprecated_warning(/IO process creation with a leading '\|'/) do # https://bugs.ruby-lang.org/issues/19630
-        IO.read("|#{EnvUtil.rubybin} -e 'puts :foo'", 1, 1)
-      end
+      pipe_errors = [Errno::ENOENT, Errno::EINVAL, Errno::EACCES, Errno::EPERM]
+      assert_raise(*pipe_errors) { open(cmd, "r+") }
+      assert_raise(*pipe_errors) { IO.read(cmd) }
+      assert_raise(*pipe_errors) { IO.foreach(cmd) {|x| assert false } }
     end
   end
 
@@ -2853,19 +2832,6 @@ def test_reopen_ivar
   end
 
   def test_foreach
-    a = []
-
-    assert_deprecated_warning(/IO process creation with a leading '\|'/) do # https://bugs.ruby-lang.org/issues/19630
-      IO.foreach("|" + EnvUtil.rubybin + " -e 'puts :foo; puts :bar; puts :baz'") {|x| a << x }
-    end
-    assert_equal(["foo\n", "bar\n", "baz\n"], a)
-
-    a = []
-    assert_deprecated_warning(/IO process creation with a leading '\|'/) do # https://bugs.ruby-lang.org/issues/19630
-      IO.foreach("|" + EnvUtil.rubybin + " -e 'puts :zot'", :open_args => ["r"]) {|x| a << x }
-    end
-    assert_equal(["zot\n"], a)
-
     make_tempfile {|t|
       a = []
       IO.foreach(t.path) {|x| a << x }

test/ruby/test_io_m17n.rb

@@ -1395,30 +1395,6 @@ def test_popenv_r_enc_enc_in_opt2
     }
   end
 
-  def test_open_pipe_r_enc
-    EnvUtil.suppress_warning do # https://bugs.ruby-lang.org/issues/19630
-      open("|#{EnvUtil.rubybin} -e 'putc 255'", "r:ascii-8bit") {|f|
-        assert_equal(Encoding::ASCII_8BIT, f.external_encoding)
-        assert_equal(nil, f.internal_encoding)
-        s = f.read
-        assert_equal(Encoding::ASCII_8BIT, s.encoding)
-        assert_equal("\xff".force_encoding("ascii-8bit"), s)
-      }
-    end
-  end
-
-  def test_open_pipe_r_enc2
-    EnvUtil.suppress_warning do # https://bugs.ruby-lang.org/issues/19630
-      open("|#{EnvUtil.rubybin} -e 'putc \"\\u3042\"'", "r:UTF-8") {|f|
-        assert_equal(Encoding::UTF_8, f.external_encoding)
-        assert_equal(nil, f.internal_encoding)
-        s = f.read
-        assert_equal(Encoding::UTF_8, s.encoding)
-        assert_equal("\u3042", s)
-      }
-    end
-  end
-
   def test_s_foreach_enc
     with_tmpdir {
       generate_file("t", "\xff")