-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apparent regression with NameError in 9.1.0.0 #3934
Comments
Just quickly tried this. The error is something wrong with warnings on via '-v' as you can see it works as expected without -v:
In 9.1.2.0 I get slightly different output than reporter:
So it looks like the NameError is occuring and whatever is messing up in the warning is now not masking over the NameError. It is still clearly wrong though. |
Hmm. NameError uses sprintf to format the message. Somewhere along the way we're passing too many args in for the format string. I'll have a look. |
I'm not sure why you didn't get a filename, but I did:
The warning still appears to be ours, though, since I don't see DidYouMean doing any string formatting at that line. |
Hmm well I am using 9.1.0.0 and I do have debug mode enabled in my project via My
If I upgrade to 9.1.2.0 will that help? Thanks! |
You don't need I think you should be ok with just |
I have a fix for the warning. MRI uses positional arguments in its I will audit other places we do NameError and try to ensure we're always using the formatting logic as in MRI. |
Thanks for the tip! But when I comment out
|
Wow, that was fast! Impressive! |
Hmm...let me double check on the debug thing. I thought |
Some of these use the format string, some don't. See #3934.
Several cases were formatting the string ahead of time rather than allowing the formatting mechanism to work. Relates to #3934.
I found a few more cases that had this problem:
I've audited all consumers of the formatted @eregon I'm not sure if there's a good way to test these. We'd have to test that verbose mode does not display anything on MRI, and of course it does not do a bunch of things. For now I'm planning to just add some JRuby regression specs that check this issue. |
See #3934 for some cases that were either warning or erroring incorrectly.
@headius just curious if you maybe had a chance to look into this? I’d love to be able to run my tests with accurate coverage but without all the extra output in my console. Thanks! |
@donv great, thank you!! |
Environment
Expected Behavior
The 9.0.5.0 behavior when referencing an undefined name:
I can see that a
NameError
occurs, which symbol is uninitialized, and the specific location where it was referenced.Actual Behavior
The actual problem has been masked by this
ArgumentError
.The text was updated successfully, but these errors were encountered: