From b08fbd3cbaefd9918d20619e21f268f812a56884 Mon Sep 17 00:00:00 2001
From: Giovanni Cappellotto <potomak84@gmail.com>
Date: Mon, 4 Jul 2022 13:13:19 -0400
Subject: [PATCH] Check that def, macro, and block parameters don't end with ?
 or !

Fix for #10917
---
 spec/compiler/parser/parser_spec.cr   | 19 +++++++++++++++++++
 src/compiler/crystal/syntax/parser.cr | 11 +++++++++++
 2 files changed, 30 insertions(+)

diff --git a/spec/compiler/parser/parser_spec.cr b/spec/compiler/parser/parser_spec.cr
index d454d213da23..8cef168f4453 100644
--- a/spec/compiler/parser/parser_spec.cr
+++ b/spec/compiler/parser/parser_spec.cr
@@ -253,6 +253,25 @@ module Crystal
       assert_syntax_error "foo { |(#{kw}))| }", "cannot use '#{kw}' as a block parameter name", 1, 9
     end
 
+    # #10917
+    %w(
+      bar? bar!
+    ).each do |name|
+      assert_syntax_error "def foo(#{name}); end", "invalid param name", 1, 14
+      assert_syntax_error "def foo(foo #{name}); end", "invalid param name", 1, 17
+      it_parses "def foo(#{name} foo); end", Def.new("foo", [Arg.new("foo", external_name: name.to_s)])
+
+      assert_syntax_error "macro foo(#{name}); end", "invalid param name", 1, 16
+      assert_syntax_error "macro foo(foo #{name}); end", "invalid param name", 1, 19
+      it_parses "macro foo(#{name} foo); end", Macro.new("foo", [Arg.new("foo", external_name: name.to_s)], body: MacroLiteral.new(" "))
+
+      it_parses "foo(#{name})", Call.new(nil, "foo", [name.call] of ASTNode)
+      it_parses "foo #{name}", Call.new(nil, "foo", [name.call] of ASTNode)
+
+      assert_syntax_error "foo { |#{name})| }", "invalid param name", 1, 12
+      assert_syntax_error "foo { |(#{name}))| }", "invalid param name", 1, 13
+    end
+
     it_parses "def self.foo\n1\nend", Def.new("foo", body: 1.int32, receiver: "self".var)
     it_parses "def self.foo()\n1\nend", Def.new("foo", body: 1.int32, receiver: "self".var)
     it_parses "def self.foo=\n1\nend", Def.new("foo=", body: 1.int32, receiver: "self".var)
diff --git a/src/compiler/crystal/syntax/parser.cr b/src/compiler/crystal/syntax/parser.cr
index ead4e2c9ed5d..1ebfa28fe09d 100644
--- a/src/compiler/crystal/syntax/parser.cr
+++ b/src/compiler/crystal/syntax/parser.cr
@@ -3921,6 +3921,8 @@ module Crystal
           raise "when specified, external name must be different than internal name", @token
         end
 
+        check_valid_param_name(arg_name)
+
         uses_arg = false
         do_next_token = true
       when .instance_var?
@@ -3984,6 +3986,7 @@ module Crystal
             raise "cannot use '#{invalid_internal_name}' as a parameter name", invalid_internal_name
           end
           arg_name = external_name
+          check_valid_param_name(arg_name)
         else
           unexpected_token
         end
@@ -4033,6 +4036,12 @@ module Crystal
       end
     end
 
+    def check_valid_param_name(param_name)
+      if param_name.ends_with?(/\?|!/)
+        raise "invalid param name"
+      end
+    end
+
     def parse_if(check_end = true)
       location = @token.location
 
@@ -4351,6 +4360,7 @@ module Crystal
             end
 
             arg_name = @token.value.to_s
+            check_valid_param_name(arg_name)
 
             if all_names.includes?(arg_name)
               raise "duplicated block parameter name: #{arg_name}", @token
@@ -4372,6 +4382,7 @@ module Crystal
                 end
 
                 sub_arg_name = @token.value.to_s
+                check_valid_param_name(sub_arg_name)
 
                 if all_names.includes?(sub_arg_name)
                   raise "duplicated block parameter name: #{sub_arg_name}", @token