Skip to content

Suggest the extra comma for 1-ary tuple when warning about unnecessary parentheses around type #86019

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

Open
NicolasDP opened this issue Jun 5, 2021 · 4 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints L-unused_parens Lint: unused_parens T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@NicolasDP
Copy link

Given the following code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b6faae0b3f3e40ccb0c2158eb481b989

pub type U8 = (u8);

The current output is (1.52.1, 1.53.0-beta.3 and 1.54.0-nightly):

warning: unnecessary parentheses around type
 --> src/lib.rs:1:15
  |
1 | pub type U8 = (u8);
  |               ^^^^ help: remove these parentheses
  |
  = note: `#[warn(unused_parens)]` on by default

warning: 1 warning emitted

Ideally the output should look like:

warning: unnecessary parentheses around type
 --> src/lib.rs:1:15
  |
1 | pub type U8 = (u8);
  |               ^^^^ help: remove these parentheses
  |
  = note: `#[warn(unused_parens)]` on by default
  = note: if you meant to use 1-ary tuple, add an extra comma: `(T,)`
warning: 1 warning emitted

While I appreciate that using 1-ary tuple (T,) may seem odd in most cases it is still part of the language: rust reference. It is also something that the standard library leverages too (generalising trait implementations for 1-ary tuple). Providing this extra note could go a long way to help developers writing generalised implementation of their Trait for n-ary tuples.

@NicolasDP NicolasDP added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 5, 2021
@leonardo-m
Copy link

This is nice. But probably it needs to be smart otherwise you get that note every time there are two unused parentheses...

@jyn514
Copy link
Member

jyn514 commented Jun 5, 2021

"This is valid code" is not a very convincing argument that the compiler should suggest it. Using a tuple for a type named U8 seems odd. Just the warning itself should be a hint that something is wrong, I don't think new users of the language will be trying to work with unary tuples.

@NicolasDP
Copy link
Author

I would appreciate the benefit of the doubt here please. indeed type alias U8 does not make sense by itself. The example here is only the simplest example to reproduce the warning message.

I don't understand your point about the new users. Are you suggesting that new users should use only a subset of the language? Or are you suggesting the notes are intended only for the new users?

I think there's value in providing a bit more context to users of the language. Especially with this touching something ambiguous that the compiler is having issues with. One should be able to write a 1-ary Tuple without having to go through the hoop of an extra comma. Yet it's there and for good reason (the parentheses type needs to exist too). A little note with the warning could go a long way to help anyone (new or seasoned users).

@nkconnor
Copy link

Note, the same paren suggestion is used in an example like this:

fn test<T>() {}
    
test::<(String)>();

warning: unnecessary parentheses around type
  --> src/main.rs:72:12
   |
72 |     test::<(String)>();
   |            ^      ^
   |
   = note: `#[warn(unused_parens)]` on by default
help: remove these parentheses
   |
72 -     test::<(String)>();
72 +     test::<String>();

I was just tripped up by this trying to use a static function that inherits impl MyTrait for FnOnce<(Arg1,)> from another crate.

@jieyouxu jieyouxu added the L-unused_parens Lint: unused_parens label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints L-unused_parens Lint: unused_parens T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants