Skip to content

Commit

Permalink
Warn non-nil $/ [Feature #14240]
Browse files Browse the repository at this point in the history
  • Loading branch information
nobu committed Feb 23, 2020
1 parent 6298ec2 commit 8a7e0aa
Show file tree
Hide file tree
Showing 18 changed files with 75 additions and 29 deletions.
4 changes: 2 additions & 2 deletions io.c
Expand Up @@ -13288,8 +13288,8 @@ Init_IO(void)
rb_gc_register_mark_object(rb_default_rs);
rb_rs = rb_default_rs;
rb_output_rs = Qnil;
rb_define_hooked_variable("$/", &rb_rs, 0, rb_str_setter);
rb_define_hooked_variable("$-0", &rb_rs, 0, rb_str_setter);
rb_define_hooked_variable("$/", &rb_rs, 0, deprecated_str_setter);
rb_define_hooked_variable("$-0", &rb_rs, 0, deprecated_str_setter);
rb_define_hooked_variable("$\\", &rb_output_rs, 0, deprecated_str_setter);

rb_define_virtual_variable("$_", get_LAST_READ_LINE, set_LAST_READ_LINE);
Expand Down
4 changes: 2 additions & 2 deletions spec/ruby/core/io/puts_spec.rb
Expand Up @@ -16,7 +16,7 @@ def @io.write(str)
ScratchPad.clear
@io.close if @io
rm_r @name
$/ = @before_separator
suppress_warning {$/ = @before_separator}
end

it "writes just a newline when given no args" do
Expand Down Expand Up @@ -105,7 +105,7 @@ def @io.write(str)
end

it "ignores the $/ separator global" do
$/ = ":"
suppress_warning {$/ = ":"}
@io.puts(5).should == nil
ScratchPad.recorded.should == "5\n"
end
Expand Down
8 changes: 4 additions & 4 deletions spec/ruby/core/io/readlines_spec.rb
Expand Up @@ -22,11 +22,11 @@

describe "when passed no arguments" do
before :each do
@sep, $/ = $/, " "
suppress_warning {@sep, $/ = $/, " "}
end

after :each do
$/ = @sep
suppress_warning {$/ = @sep}
end

it "returns an Array containing lines based on $/" do
Expand Down Expand Up @@ -184,7 +184,7 @@
after :each do
Encoding.default_external = @external
Encoding.default_internal = @internal
$/ = @dollar_slash
suppress_warning {$/ = @dollar_slash}
end

it "encodes lines using the default external encoding" do
Expand All @@ -196,7 +196,7 @@
it "encodes lines using the default internal encoding, when set" do
Encoding.default_external = Encoding::UTF_8
Encoding.default_internal = Encoding::UTF_16
$/ = $/.encode Encoding::UTF_16
suppress_warning {$/ = $/.encode Encoding::UTF_16}
lines = IO.readlines(@name)
lines.all? { |s| s.encoding == Encoding::UTF_16 }.should be_true
end
Expand Down
4 changes: 2 additions & 2 deletions spec/ruby/core/io/shared/each.rb
Expand Up @@ -168,12 +168,12 @@
before :each do
@io = IOSpecs.io_fixture "lines.txt"
ScratchPad.record []
@sep, $/ = $/, " "
suppress_warning {@sep, $/ = $/, " "}
end

after :each do
@io.close if @io
$/ = @sep
suppress_warning {$/ = @sep}
end

it "uses $/ as the default line separator" do
Expand Down
4 changes: 2 additions & 2 deletions spec/ruby/core/io/shared/readlines.rb
Expand Up @@ -60,11 +60,11 @@
end

after :each do
$/ = @sep
suppress_warning {$/ = @sep}
end

it "defaults to $/ as the separator" do
$/ = " "
suppress_warning {$/ = " "}
result = IO.send(@method, @name, 10, &@object)
(result ? result : ScratchPad.recorded).should == IOSpecs.lines_space_separator_limit
end
Expand Down
2 changes: 1 addition & 1 deletion spec/ruby/core/kernel/fixtures/classes.rb
Expand Up @@ -49,7 +49,7 @@ def self.chop(str, method)

def self.chomp(str, method, sep="\n")
code = "$_ = #{str.inspect}; $/ = #{sep.inspect}; #{method}; print $_"
IO.popen([*ruby_exe, "-n", "-e", code], "r+") do |io|
IO.popen([*ruby_exe, "-W0", "-n", "-e", code], "r+") do |io|
io.puts
io.close_write
io.read
Expand Down
3 changes: 2 additions & 1 deletion spec/ruby/core/kernel/warn_spec.rb
Expand Up @@ -8,8 +8,9 @@
end

after :each do
$VERBOSE = @before_verbose
$VERBOSE = nil
$/ = @before_separator
$VERBOSE = @before_verbose
end

it "is a private method" do
Expand Down
8 changes: 8 additions & 0 deletions spec/ruby/core/string/chomp_spec.rb
Expand Up @@ -6,11 +6,13 @@
describe "when passed no argument" do
before do
# Ensure that $/ is set to the default value
@verbose, $VERBOSE = $VERBOSE, nil
@dollar_slash, $/ = $/, "\n"
end

after do
$/ = @dollar_slash
$VERBOSE = @verbose
end

it "does not modify a String with no trailing carriage return or newline" do
Expand Down Expand Up @@ -179,11 +181,13 @@
describe "when passed no argument" do
before do
# Ensure that $/ is set to the default value
@verbose, $VERBOSE = $VERBOSE, nil
@dollar_slash, $/ = $/, "\n"
end

after do
$/ = @dollar_slash
$VERBOSE = @verbose
end

it "modifies self" do
Expand Down Expand Up @@ -350,11 +354,13 @@

describe "String#chomp" do
before :each do
@verbose, $VERBOSE = $VERBOSE, nil
@before_separator = $/
end

after :each do
$/ = @before_separator
$VERBOSE = @verbose
end

it "does not modify a multi-byte character" do
Expand All @@ -379,11 +385,13 @@

describe "String#chomp!" do
before :each do
@verbose, $VERBOSE = $VERBOSE, nil
@before_separator = $/
end

after :each do
$/ = @before_separator
$VERBOSE = @verbose
end

it "returns nil when the String is not modified" do
Expand Down
6 changes: 3 additions & 3 deletions spec/ruby/core/string/shared/each_line.rb
Expand Up @@ -84,7 +84,7 @@
end

after :each do
$/ = @before_separator
suppress_warning {$/ = @before_separator}
end

it "as the separator when none is given" do
Expand All @@ -96,10 +96,10 @@
expected = []
str.send(@method, sep) { |x| expected << x }

$/ = sep
suppress_warning {$/ = sep}

actual = []
str.send(@method) { |x| actual << x }
suppress_warning {str.send(@method) { |x| actual << x }}

actual.should == expected
end
Expand Down
4 changes: 4 additions & 0 deletions spec/ruby/language/predefined_spec.rb
Expand Up @@ -541,13 +541,15 @@ def bar

describe "Predefined global $/" do
before :each do
@verbose, $VERBOSE = $VERBOSE, nil
@dollar_slash = $/
@dollar_dash_zero = $-0
end

after :each do
$/ = @dollar_slash
$-0 = @dollar_dash_zero
$VERBOSE = @verbose
end

it "can be assigned a String" do
Expand Down Expand Up @@ -589,13 +591,15 @@ def bar

describe "Predefined global $-0" do
before :each do
@verbose, $VERBOSE = $VERBOSE, nil
@dollar_slash = $/
@dollar_dash_zero = $-0
end

after :each do
$/ = @dollar_slash
$-0 = @dollar_dash_zero
$VERBOSE = @verbose
end

it "can be assigned a String" do
Expand Down
5 changes: 3 additions & 2 deletions spec/ruby/library/stringio/gets_spec.rb
Expand Up @@ -76,12 +76,13 @@
@io.gets.should == "this is\n"

begin
old_sep, $/ = $/, " "
old_sep = $/
suppress_warning {$/ = " "}
@io.gets.should == "an "
@io.gets.should == "example\nfor "
@io.gets.should == "StringIO#gets"
ensure
$/ = old_sep
suppress_warning {$/ = old_sep}
end
end

Expand Down
5 changes: 3 additions & 2 deletions spec/ruby/library/stringio/readline_spec.rb
Expand Up @@ -64,12 +64,13 @@
@io.readline.should == "this is\n"

begin
old_sep, $/ = $/, " "
old_sep = $/
suppress_warning {$/ = " "}
@io.readline.should == "an "
@io.readline.should == "example\nfor "
@io.readline.should == "StringIO#readline"
ensure
$/ = old_sep
suppress_warning {$/ = old_sep}
end
end

Expand Down
5 changes: 3 additions & 2 deletions spec/ruby/library/stringio/readlines_spec.rb
Expand Up @@ -51,10 +51,11 @@

it "returns an Array containing lines based on $/" do
begin
old_sep, $/ = $/, " "
old_sep = $/;
suppress_warning {$/ = " "}
@io.readlines.should == ["this ", "is\nan ", "example\nfor ", "StringIO#readlines"]
ensure
$/ = old_sep
suppress_warning {$/ = old_sep}
end
end

Expand Down
5 changes: 3 additions & 2 deletions spec/ruby/library/stringio/shared/each.rb
Expand Up @@ -71,11 +71,12 @@
it "uses $/ as the default line separator" do
seen = []
begin
old_rs, $/ = $/, " "
old_rs = $/
suppress_warning {$/ = " "}
@io.send(@method) {|s| seen << s }
seen.should eql(["a ", "b ", "c ", "d ", "e\n1 ", "2 ", "3 ", "4 ", "5"])
ensure
$/ = old_rs
suppress_warning {$/ = old_rs}
end
end

Expand Down
4 changes: 2 additions & 2 deletions spec/ruby/optional/capi/globals_spec.rb
Expand Up @@ -59,15 +59,15 @@
end

after :each do
$/ = @dollar_slash
suppress_warning {$/ = @dollar_slash}
end

it "returns \\n by default" do
@f.rb_rs.should == "\n"
end

it "returns the value of $/" do
$/ = "foo"
suppress_warning {$/ = "foo"}
@f.rb_rs.should == "foo"
end
end
Expand Down
15 changes: 15 additions & 0 deletions string.c
Expand Up @@ -8245,6 +8245,21 @@ chomp_newline(const char *p, const char *e, rb_encoding *enc)
return e;
}

static VALUE
get_rs(void)
{
VALUE rs = rb_rs;
if (!NIL_P(rs) &&
(!RB_TYPE_P(rs, T_STRING) ||
RSTRING_LEN(rs) != 1 ||
RSTRING_PTR(rs)[0] != '\n')) {
rb_warn("$/ is set to non-default value");
}
return rs;
}

#define rb_rs get_rs()

static VALUE
rb_str_enumerate_lines(int argc, VALUE *argv, VALUE str, VALUE ary)
{
Expand Down
4 changes: 2 additions & 2 deletions test/openssl/test_pair.rb
Expand Up @@ -160,10 +160,10 @@ def test_puts_meta
ssl_pair {|s1, s2|
begin
old = $/
$/ = '*'
EnvUtil.suppress_warning {$/ = '*'}
s1.puts 'a'
ensure
$/ = old
EnvUtil.suppress_warning {$/ = old}
end
s1.close
assert_equal("a\n", s2.read)
Expand Down

0 comments on commit 8a7e0aa

Please sign in to comment.