"duplicated argument name" vs "repeated parameter name"

[eregon]

$ ruby -e 'def m(a,a); end'
-e:1: duplicated argument name
def m(a,a); end
-e: compile error (SyntaxError)

vs

$ bin/parse -e 'def m(a,a); end' 
Errors:
[#<Prism::ParseError @message="repeated parameter name" @location=#<Prism::Location @start_offset=8 @length=1 start_line=1> @level=:error_default>]
...

I think either is fine, but if keeping repeated parameter name then there are quite a few occurrences to update in https://github.com/ruby/ruby/blob/master/test/ruby/test_syntax.rb

[kddnewton]

I don't really like this error message from CRuby at all, but I'd rather not fight the backward compatibility fight. I'll change it to match.

[eregon]

FWIW I've noticed there are quite a few messages different in Prism.
I personally think it's fine.
For now I worked around by changing specs & tests to allow both, specs they will get upstreamed but changes in MRI tests they won't.
Anyway you will see when trying to pass test-all & test-spec with Prism on CRuby.

I just filed this one because it was a lot of occurrences to change.

[kddnewton]

@eregon do you think this is something we should try to push on? I do think it's a parameter and not an argument, but I don't want to push for too many changes all at the same time.

[eregon]

I think anyway you will need to adapt many MRI tests so I think it's fine to have the more precise message here.
See test/mri/tests/ruby/test_parse.rb and following files at https://github.com/oracle/truffleruby/pull/3406/files#diff-db521a46faacd55c802b27d84034971359cfa4d41b2e5cfc2a2d36cd232334a6 for example changes due to different SyntaxError messages with Prism.

The term parameter started to be used (much more) in Ruby around Ruby 2.7/3.0 as kwargs warnings used that term, so that's probably why the term argument was used in the first place.

[eregon]

In terms of discussion on the CRuby tracker I think it makes more sense to discuss it once there is a good overview of the changes (or to use my link above but that's rather messy since it's a huge PR). In general Prism SyntaxError messages are much clearer, but in some cases it might be best to stay compatible if the existing message is not too confusing, something to discuss there.

I wish gems wouldn't depend on exact exception messages but in practice they often do (a recent example: https://github.com/oracle/truffleruby/issues/3398). Which is kind of natural for integration-like test which might just compare the expected output as an exact string.

[kddnewton]

I just got to the point where this is now failing in the CRuby test suite, so this will be fixed soon.