Check that def, macro, and block parameters don't end with ? or !

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_warning "def foo(#{name}); end", "invalid parameter name: #{name}", 1, 14
+      assert_syntax_warning "def foo(foo #{name}); end", "invalid parameter name: #{name}", 1, 17
+      it_parses "def foo(#{name} foo); end", Def.new("foo", [Arg.new("foo", external_name: name.to_s)])
+
+      assert_syntax_warning "macro foo(#{name}); end", "invalid parameter name: #{name}", 1, 16
+      assert_syntax_warning "macro foo(foo #{name}); end", "invalid parameter name: #{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_warning "foo { |#{name}| }", "invalid parameter name: #{name}", 1, 12
+      assert_syntax_warning "foo { |(#{name})| }", "invalid parameter name: #{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)

spec/support/syntax.cr

@@ -158,6 +158,16 @@ def assert_syntax_error(str, message = nil, line = nil, column = nil, metafile =
   end
 end
 
+def assert_syntax_warning(str, message = nil, line = nil, column = nil, metafile = __FILE__, metaline = __LINE__, metaendline = __END_LINE__)
+  it "says syntax warning on #{str.inspect}", metafile, metaline, metaendline do
+    warnings = WarningCollection.new
+    parse str, false, nil, warnings
+    if warnings.infos.find { |info| info.ends_with?(message) }.nil?
+      fail "Expected warnings to include '#{message}'"
+    end
+  end
+end
+
 def parse(string, wants_doc = false, filename = nil, warnings = nil)
   parser = Parser.new(string, warnings: warnings)
   parser.warnings = warnings if warnings

src/compiler/crystal/syntax/parser.cr

@@ -3887,9 +3887,11 @@ module Crystal
       do_next_token = true
       found_string_literal = false
       invalid_internal_name = nil
+      external_name_token = nil
 
       if allow_external_name && (@token.type.ident? || string_literal_start?)
         name_location = @token.location
+        external_name_token = @token.dup
         if @token.type.ident?
           if @token.keyword? && invalid_internal_name?(@token.value)
             invalid_internal_name = @token.dup
@@ -3921,6 +3923,8 @@ module Crystal
           raise "when specified, external name must be different than internal name", @token
         end
 
+        check_valid_param_name
+
         uses_arg = false
         do_next_token = true
       when .instance_var?
@@ -3984,6 +3988,10 @@ module Crystal
             raise "cannot use '#{invalid_internal_name}' as a parameter name", invalid_internal_name
           end
           arg_name = external_name
+          if external_name_token.nil?
+            raise "missing external name token"
+          end
+          check_valid_param_name(external_name_token)
         else
           unexpected_token
         end
@@ -4033,6 +4041,13 @@ module Crystal
       end
     end
 
+    def check_valid_param_name(token : Token = @token)
+      param_name = token.value.to_s
+      if param_name[-1]?.in?('?', '!')
+        warnings.add_warning_at(token.location, "invalid parameter name: #{param_name}")
+      end
+    end
+
     def parse_if(check_end = true)
       location = @token.location
 
@@ -4351,6 +4366,7 @@ module Crystal
             end
 
             arg_name = @token.value.to_s
+            check_valid_param_name
 
             if all_names.includes?(arg_name)
               raise "duplicated block parameter name: #{arg_name}", @token
@@ -4372,6 +4388,7 @@ module Crystal
                 end
 
                 sub_arg_name = @token.value.to_s
+                check_valid_param_name
 
                 if all_names.includes?(sub_arg_name)
                   raise "duplicated block parameter name: #{sub_arg_name}", @token