mame	2018-10-26 12:08:10 +0900 (Fri, 26 Oct 2018)

  New Revision: 65368

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=65368

  Log:
    Revert "parse.y: remove "shadowing outer local variable" warning"
    
    I forgot to add the copyright of the patch...

  Modified files:
    trunk/NEWS
    trunk/parse.y
    trunk/test/lib/test/unit/assertions.rb
    trunk/test/ripper/test_parser_events.rb
    trunk/test/ruby/test_parse.rb
    trunk/test/ruby/test_rubyoptions.rb
Index: NEWS
===================================================================
--- NEWS	(revision 65367)
+++ NEWS	(revision 65368)
@@ -31,11 +31,6 @@ sufficient information, see the ChangeLo https://github.com/ruby/ruby/blob/trunk/NEWS#L31
 
 * Non-Symbol key in keyword arguments hash causes an exception.
 
-* "shadowing outer local variable" warning was removed.  [Feature #12490]
-  You can now write the following without warning.
-
-  user = User.all.find {|user| cond(user) }
-
 === Core classes updates (outstanding ones only)
 
 [Array]
Index: test/ruby/test_rubyoptions.rb
===================================================================
--- test/ruby/test_rubyoptions.rb	(revision 65367)
+++ test/ruby/test_rubyoptions.rb	(revision 65368)
@@ -782,6 +782,20 @@ class TestRubyOptions < Test::Unit::Test https://github.com/ruby/ruby/blob/trunk/test/ruby/test_rubyoptions.rb#L782
     assert_in_out_err(["-w", "-"], "eval('a=1')", [], [], feature7730)
   end
 
+  def test_shadowing_variable
+    bug4130 = '[ruby-dev:42718]'
+    assert_in_out_err(["-we", "def foo\n""  a=1\n""  1.times do |a| end\n""  a\n""end"],
+                      "", [], ["-e:3: warning: shadowing outer local variable - a"], bug4130)
+    assert_in_out_err(["-we", "def foo\n""  a=1\n""  1.times do |a| end\n""end"],
+                      "", [],
+                      ["-e:3: warning: shadowing outer local variable - a",
+                       "-e:2: warning: assigned but unused variable - a",
+                      ], bug4130)
+    feature6693 = '[ruby-core:46160]'
+    assert_in_out_err(["-we", "def foo\n""  _a=1\n""  1.times do |_a| end\n""end"],
+                      "", [], [], feature6693)
+  end
+
   def test_script_from_stdin
     begin
       require 'pty'
Index: test/ruby/test_parse.rb
===================================================================
--- test/ruby/test_parse.rb	(revision 65367)
+++ test/ruby/test_parse.rb	(revision 65368)
@@ -911,8 +911,10 @@ x = __ENCODING__ https://github.com/ruby/ruby/blob/trunk/test/ruby/test_parse.rb#L911
     assert_equal(expected, actual, bug5614)
   end
 
-  def test_no_shadowing_variable_warning
-    assert_no_warning(/shadowing outer local variable/) {eval("a=1; tap {|a|}")}
+  def test_shadowing_variable
+    assert_warning(/shadowing outer local variable/) {eval("a=1; tap {|a|}")}
+    a = "\u{3042}"
+    assert_warning(/#{a}/o) {eval("#{a}=1; tap {|#{a}|}")}
   end
 
   def test_unused_variable
Index: test/ripper/test_parser_events.rb
===================================================================
--- test/ripper/test_parser_events.rb	(revision 65367)
+++ test/ripper/test_parser_events.rb	(revision 65368)
@@ -1487,6 +1487,13 @@ class TestRipper::ParserEvents < Test::U https://github.com/ruby/ruby/blob/trunk/test/ripper/test_parser_events.rb#L1487
     assert_equal("`$' without identifiers is not allowed as a global variable name", compile_error('$'))
   end
 
+  def test_warning_shadowing
+    fmt, *args = warning("x = 1; tap {|;x|}")
+    assert_match(/shadowing outer local variable/, fmt)
+    assert_equal("x", args[0])
+    assert_match(/x/, fmt % args)
+  end
+
   def test_warning_ignored_magic_comment
     fmt, *args = warning("1; #-*- frozen-string-literal: true -*-")
     assert_match(/ignored after any tokens/, fmt)
Index: test/lib/test/unit/assertions.rb
===================================================================
--- test/lib/test/unit/assertions.rb	(revision 65367)
+++ test/lib/test/unit/assertions.rb	(revision 65368)
@@ -693,16 +693,6 @@ eom https://github.com/ruby/ruby/blob/trunk/test/lib/test/unit/assertions.rb#L693
         assert_warning(*args) {$VERBOSE = false; yield}
       end
 
-      def assert_no_warning(pat, msg = nil)
-        stderr = EnvUtil.verbose_warning {
-          EnvUtil.with_default_internal(pat.encoding) {
-            yield
-          }
-        }
-        msg = message(msg) {diff pat, stderr}
-        refute(pat === stderr, msg)
-      end
-
       def assert_no_memory_leak(args, prepare, code, message=nil, limit: 2.0, rss: false, **opt)
         # TODO: consider choosing some appropriate limit for MJIT and stop skipping this once it does not randomly fail
         skip 'assert_no_memory_leak may consider MJIT memory usage as leak' if defined?(RubyVM::MJIT) && RubyVM::MJIT.enabled?
Index: parse.y
===================================================================
--- parse.y	(revision 65367)
+++ parse.y	(revision 65368)
@@ -9220,6 +9220,7 @@ shadowing_lvar_0(struct parser_params *p https://github.com/ruby/ruby/blob/trunk/parse.y#L9220
 	    yyerror0("duplicated argument name");
 	}
 	else if (dvar_defined(p, name) || local_id(p, name)) {
+	    rb_warning1("shadowing outer local variable - %"PRIsWARN, rb_id2str(name));
 	    vtable_add(p->lvtbl->vars, name);
 	    if (p->lvtbl->used) {
 		vtable_add(p->lvtbl->used, (ID)p->ruby_sourceline | LVAR_USED);

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/