Skip to content

improve error message when two-arg assert_eq! receives a trailing comma #39554

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

Conversation

zackmdavis
Copy link
Member

Previously, assert_eq!(left, right,) (respectively, assert_ne!(left, right,); note the trailing comma) would result in a confusing "requires
at least a format string argument" error. In reality, a format string is
optional, but the trailing comma puts us into the "match a token tree of
zero or more tokens" branch of the macro (in order to support the
optional format string), and passing the empty token tree into
format_args! results in the confusing error. If instead we match a
token tree of one or more tokens, we get a much more sensible
"unexpected end of macro invocation" error.

While we're here, fix up a stray space before a comma in the match
guards.

Resolves #39369.


Before:

$ rustc scratch.rs 
error: requires at least a format string argument
 --> scratch.rs:2:5
  |
2 |     assert_eq!(1, 2,);
  |     ^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in a macro outside of the current crate

error: aborting due to previous error

After:

$ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc scratch.rs 
error: unexpected end of macro invocation
 --> scratch.rs:2:20
  |
2 |     assert_eq!(1, 2,);
  |                    ^

@rust-highfive
Copy link
Contributor

r? @BurntSushi

(rust_highfive has picked a reviewer for you, use r? to override)

@BurntSushi
Copy link
Member

@zackmdavis Thanks! Is there a way to write a compile-fail test for this?

@arielb1
Copy link
Contributor

arielb1 commented Feb 6, 2017

@BurntSushi

This would be a UI test, I imagine (see the src/test/ui tests, and the update-references.sh script which is supposed to be creating the output).

Previously, `assert_eq!(left, right,)` (respectively, `assert_ne!(left,
right,)`; note the trailing comma) would result in a confusing "requires
at least a format string argument" error. In reality, a format string is
optional, but the trailing comma puts us into the "match a token tree of
zero or more tokens" branch of the macro (in order to support the
optional format string), and passing the empty token tree into
`format_args!` results in the confusing error. If instead we match a
token tree of one or more tokens, we get a much more sensible
"unexpected end of macro invocation" error.

While we're here, fix up a stray space before a comma in the match
guards.

Resolves rust-lang#39369.
@zackmdavis zackmdavis force-pushed the assert_eq_has_a_terrible_error_message_when_given_a_trailing_comma branch from 8ebbe16 to 65435e1 Compare February 7, 2017 06:33
@zackmdavis
Copy link
Member Author

(force-pushed)

@zackmdavis
Copy link
Member Author

@BurntSushi ping?

@BurntSushi
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 11, 2017

📌 Commit 65435e1 has been approved by BurntSushi

@BurntSushi
Copy link
Member

@zackmdavis Sorry about that! Thank you! :-)

@bors
Copy link
Collaborator

bors commented Feb 12, 2017

⌛ Testing commit 65435e1 with merge 410d807...

bors added a commit that referenced this pull request Feb 12, 2017
…age_when_given_a_trailing_comma, r=BurntSushi

improve error message when two-arg assert_eq! receives a trailing comma

Previously, `assert_eq!(left, right,)` (respectively, `assert_ne!(left,
right,)`; note the trailing comma) would result in a confusing "requires
at least a format string argument" error. In reality, a format string is
optional, but the trailing comma puts us into the "match a token tree of
zero or more tokens" branch of the macro (in order to support the
optional format string), and passing the empty token tree into
`format_args!` results in the confusing error. If instead we match a
token tree of one or more tokens, we get a much more sensible
"unexpected end of macro invocation" error.

While we're here, fix up a stray space before a comma in the match
guards.

Resolves #39369.

-----

**Before:**
```
$ rustc scratch.rs
error: requires at least a format string argument
 --> scratch.rs:2:5
  |
2 |     assert_eq!(1, 2,);
  |     ^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in a macro outside of the current crate

error: aborting due to previous error
```

**After:**
```
$ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc scratch.rs
error: unexpected end of macro invocation
 --> scratch.rs:2:20
  |
2 |     assert_eq!(1, 2,);
  |                    ^
```
@bors
Copy link
Collaborator

bors commented Feb 12, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: BurntSushi
Pushing 410d807 to master...

@bors bors merged commit 65435e1 into rust-lang:master Feb 12, 2017
@zackmdavis zackmdavis deleted the assert_eq_has_a_terrible_error_message_when_given_a_trailing_comma branch January 13, 2018 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assert_eq! has a terrible error message when given a trailing comma
5 participants