[Bug #20468] More strictly check loop variable

parse.y

@@ -1343,6 +1343,16 @@ anddot_multiple_assignment_check(struct parser_params* p, const YYLTYPE *loc, ID
  }
 }
 
+static bool
+anddot_loop_variable_check(struct parser_params* p, const YYLTYPE *loc, ID id)
+{
+ if (id == tANDDOT) {
+ yyerror1(loc, "&. in loop variable");
+ return false;
+ }
+ return true;
+}
+
 static inline void
 set_line_body(NODE *body, int line)
 {
@@ -2694,6 +2704,19 @@ rb_parser_ary_free(rb_parser_t *p, rb_parser_ary_t *ary)
 }
 
 #endif /* !RIPPER */
+
+static NODE *
+loop_variable_error(struct parser_params *p, const YYLTYPE *loc, int idx)
+{
+ static const char mesg[] = "loop variable cannot be a constant";
+#ifdef RIPPER
+ set_value(dispatch2(param_error, STR_NEW2(mesg), get_value(idx)));
+ p->error_p = 1;
+#else
+ yyerror1(loc, mesg);
+#endif
+ return NEW_ERROR(loc);
+}
 %}
 
 %expect 0
@@ -2878,7 +2901,7 @@ rb_parser_ary_free(rb_parser_t *p, rb_parser_ary_t *ary)
 %type <node> p_args p_args_head p_args_tail p_args_post p_arg p_rest
 %type <node> p_value p_primitive p_variable p_var_ref p_expr_ref p_const
 %type <node> p_kwargs p_kwarg p_kw
-%type <id> keyword_variable user_variable sym operation operation2 operation3
+%type <id> keyword_variable user_variable sym operation operation2 operation3 for_variable
 %type <id> cname fname op f_rest_arg f_block_arg opt_f_block_arg f_norm_arg f_bad_arg
 %type <id> f_kwrest f_label f_arg_asgn call_op call_op2 reswords relop dot_or_colon
 %type <id> p_kwrest p_kwnorest p_any_kwrest p_kw_label
@@ -5024,7 +5047,64 @@ opt_else : none
  }
  ;
 
-for_var : lhs
+for_variable : tIDENTIFIER
+ | nonlocal_var
+ | keyword_variable
+ ;
+
+for_var : for_variable
+ {
+ $$ = assignable(p, $1, 0, &@$);
+ /*% ripper: ripper_assignable(p, $1, var_field(p, get_value($:1))) %*/
+ }
+ | primary_value '[' opt_call_args rbracket
+ {
+ $$ = aryset(p, $1, $3, &@$);
+ /*% ripper: aref_field!($:1, $:3) %*/
+ }
+ | primary_value call_op tIDENTIFIER
+ {
+ if (anddot_loop_variable_check(p, &@2, $2)) {
+ $$ = attrset(p, $1, $2, $3, &@$);
+ /*% ripper: field!($:1, $:2, $:3) %*/
+ }
+ else {
+ $$ = NEW_ERROR(&@$);
+ }
+ }
+ | primary_value tCOLON2 tIDENTIFIER
+ {
+ $$ = attrset(p, $1, idCOLON2, $3, &@$);
+ /*% ripper: field!($:1, $:2, $:3) %*/
+ }
+ | primary_value call_op tCONSTANT
+ {
+ anddot_loop_variable_check(p, &@2, $2);
+ /*% ripper[error]: field!($:1, $:2, $:3) %*/
+ }
+ | primary_value tCOLON2 tCONSTANT
+ {
+ $$ = loop_variable_error(p, &@$, $:3);
+ /*% ripper: ripper_const_decl(p, const_path_field!($:1, $:3)) %*/
+ }
+ | tCOLON3 tCONSTANT
+ {
+ $$ = loop_variable_error(p, &@$, $:2);
+ /*% ripper: ripper_const_decl(p, top_const_field!($:2)) %*/
+ }
+ | tCONSTANT
+ {
+ $$ = loop_variable_error(p, &@$, $:1);
+ /*% ripper: ripper_const_decl(p, var_field!($:1)) %*/
+ }
+ | backref
+ {
+ /*%%%*/
+ rb_backref_error(p, $1);
+ /*% %*/
+ $$ = NEW_ERROR(&@$);
+ /*% ripper[error]: backref_error(p, $1, var_field(p, get_value($:1))) %*/
+ }
  | mlhs
  ;
 

prism/prism.c

@@ -18559,6 +18559,14 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
  index = (pm_node_t *) pm_splat_node_create(parser, &star_operator, name);
  } else if (token_begins_expression_p(parser->current.type)) {
  index = parse_expression(parser, PM_BINDING_POWER_INDEX, false, PM_ERR_EXPECT_EXPRESSION_AFTER_COMMA);
+ switch (index->type) {
+ case PM_CALL_NODE:
+ if (!(index->flags & PM_CALL_NODE_FLAGS_SAFE_NAVIGATION)) break;
+ case PM_CONSTANT_READ_NODE:
+ case PM_CONSTANT_PATH_NODE:
+ pm_parser_err_node(parser, index, PM_ERR_FOR_INDEX);
+ break;
+ }
  } else {
  pm_parser_err_token(parser, &for_keyword, PM_ERR_FOR_INDEX);
  index = (pm_node_t *) pm_missing_node_create(parser, for_keyword.start, for_keyword.end);

spec/ruby/language/for_spec.rb

@@ -67,17 +67,16 @@ class OFor
  end
  end
 
- it "allows a constant as an iterator name" do
- class OFor
- m = [1,2,3]
- n = 0
- -> {
- for CONST in m
- n += 1
- end
- }.should complain(/already initialized constant/)
- CONST.should == 3
- n.should == 3
+ ruby_bug("#20468", ""..."3.4") do
+ it "disallows a constant as an iterator name" do
+ class OFor
+ -> {
+ eval(<<-RUBY)
+ for CONST in [1,2,3]
+ end
+ RUBY
+ }.should raise_error(SyntaxError)
+ end
  end
  end
 

test/.excludes-prism/TestSyntax.rb

@@ -11,6 +11,7 @@
 exclude(:test_error_message_encoding, "unknown")
 exclude(:test_heredoc_cr, "unknown")
 exclude(:test_heredoc_no_terminator, "unknown")
+exclude(:test_invalid_loop_var, "[Bug #20468]")
 exclude(:test_it, "https://github.com/ruby/prism/issues/2323")
 exclude(:test_keyword_invalid_name, "unknown")
 exclude(:test_keyword_self_reference, "unknown")

test/ripper/test_parser_events.rb

@@ -1061,6 +1061,9 @@ def test_param_error
  thru_param_error = false
  parse('def foo(@@a) end', :on_param_error) {thru_param_error = true}
  assert_equal true, thru_param_error
+ thru_param_error = false
+ parse('for Foo in []; end', :on_param_error) {thru_param_error = true}
+ assert_equal true, thru_param_error
  end
 
  def test_params

test/ruby/test_syntax.rb

@@ -1238,6 +1238,13 @@ def test_safe_call_in_massign_lhs
  assert_syntax_error("a&.x,=0", /multiple assignment destination/)
  end
 
+ def test_invalid_loop_var
+ assert_syntax_error("for X in []; end", /loop variable cannot be a constant/)
+ assert_syntax_error("for ::X in []; end", /loop variable cannot be a constant/)
+ assert_syntax_error("for self::X in []; end", /loop variable cannot be a constant/)
+ assert_syntax_error("for a&.x in []; end", /in loop variable/)
+ end
+
  def test_no_warning_logop_literal
  assert_warning("") do
  eval("true||raise;nil")