Skip to content
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.

Sign up for GitHub

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

Jump to bottom

No unnecessary parentheses warning in types #64169

Closed
oli-cosmian opened this issue Sep 5, 2019 · 5 comments · Fixed by #65112
Closed

No unnecessary parentheses warning in types #64169

oli-cosmian opened this issue Sep 5, 2019 · 5 comments · Fixed by #65112
Assignees
@rustbot
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-cosmian
Copy link

oli-cosmian commented Sep 5, 2019 •
edited by rustbot
Loading

Types can be parenthesized, which is useful for e.g. dyn (Foo + 'static) or similar, but often these parentheses are unnecessary. Whenever a trivial type is parenthesized the compiler should suggest to remove the parentheses

pub fn foo() -> (i32) { 42 }

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 0.64s

This issue has been assigned to @sjmann via this comment.
@csmoe csmoe added the A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` label Sep 5, 2019
@Centril Centril added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` labels Sep 5, 2019
@Centril
Copy link
Contributor

Centril commented Sep 5, 2019

I think this falls under the spirit of the unused_parens lint and the language team does not need to approve this as I think it is obviously right.

Mentoring instructions:

See #64128 for a PR that recently changed the semantics of the unused_parens lint. You'll want to introduce fn check_ty and then match on the ty.node: TyKind while keeping in mind to avoid suggesting the removal of parenthesis that would result in a compilation error.

@Centril Centril added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Sep 5, 2019
@sjmann
Copy link

sjmann commented Sep 5, 2019

Would it be ok if I try to tackle this as a first time contributor? Seems like a nice starter issue.

@Centril
Copy link
Contributor

Centril commented Sep 5, 2019

@sjmann sure, go right ahead!

@rustbot assign @sjmann

@rustbot rustbot self-assigned this Sep 5, 2019
@ijt
Copy link

ijt commented Sep 30, 2019

@sjmann, are you still working on this?

@jack-t
Copy link
Contributor

jack-t commented Oct 4, 2019

Hi —

I've got what I believe is a working PR over here. There's a kind of chicken-and-egg problem with building core with the lint, though. I'm not totally sure how to solve it.

Thanks!

@jack-t jack-t mentioned this issue Oct 25, 2019
Merged
tmandry added a commit to tmandry/rust that referenced this issue Oct 30, 2019
@tmandry
Rollup merge of rust-lang#65112 - jack-t:type-parens-lint, r=varkor
da5d7a7 
Add lint and tests for unnecessary parens around types

This is my first contribution to the Rust project, so I apologize if I'm not doing things the right way.

The PR fixes rust-lang#64169. It adds a lint and tests for unnecessary parentheses around types. I've run `tidy` and `rustfmt` — I'm not totally sure it worked right, though — and I've tried to follow the instructions linked in the readme.

I tried to think through all the variants of `ast::TyKind` to find exceptions to this lint, and I could only find the one mentioned in the original issue, which concerns types with `dyn`. I'm not a Rust expert, thought, so I may well be missing something.

There's also a problem with getting this to build. The new lint catches several things in the, e.g., `core`. Because `x.py` seems to build with an equivalent of `-Werror`, what would have been warnings cause the build to break. I got it to build and the tests to pass with `--warnings warn` on my `x.py build` and `x.py test` commands.
Centril added a commit to Centril/rust that referenced this issue Oct 30, 2019
@Centril
Rollup merge of rust-lang#65112 - jack-t:type-parens-lint, r=varkor
5272df0 
Add lint and tests for unnecessary parens around types

This is my first contribution to the Rust project, so I apologize if I'm not doing things the right way.

The PR fixes rust-lang#64169. It adds a lint and tests for unnecessary parentheses around types. I've run `tidy` and `rustfmt` — I'm not totally sure it worked right, though — and I've tried to follow the instructions linked in the readme.

I tried to think through all the variants of `ast::TyKind` to find exceptions to this lint, and I could only find the one mentioned in the original issue, which concerns types with `dyn`. I'm not a Rust expert, thought, so I may well be missing something.

There's also a problem with getting this to build. The new lint catches several things in the, e.g., `core`. Because `x.py` seems to build with an equivalent of `-Werror`, what would have been warnings cause the build to break. I got it to build and the tests to pass with `--warnings warn` on my `x.py build` and `x.py test` commands.
tmandry added a commit to tmandry/rust that referenced this issue Oct 31, 2019
@tmandry
Rollup merge of rust-lang#65112 - jack-t:type-parens-lint, r=varkor
255a713 
Add lint and tests for unnecessary parens around types

This is my first contribution to the Rust project, so I apologize if I'm not doing things the right way.

The PR fixes rust-lang#64169. It adds a lint and tests for unnecessary parentheses around types. I've run `tidy` and `rustfmt` — I'm not totally sure it worked right, though — and I've tried to follow the instructions linked in the readme.

I tried to think through all the variants of `ast::TyKind` to find exceptions to this lint, and I could only find the one mentioned in the original issue, which concerns types with `dyn`. I'm not a Rust expert, thought, so I may well be missing something.

There's also a problem with getting this to build. The new lint catches several things in the, e.g., `core`. Because `x.py` seems to build with an equivalent of `-Werror`, what would have been warnings cause the build to break. I got it to build and the tests to pass with `--warnings warn` on my `x.py build` and `x.py test` commands.
@bors bors closed this as completed in 9175247 Nov 1, 2019
@viniciusd viniciusd mentioned this issue Nov 10, 2019
Merged
LukeMathWalker pushed a commit to rust-ndarray/ndarray that referenced this issue Nov 11, 2019
@viniciusd @LukeMathWalker
Remove unnecessary parens around types (#750)
3b9a8b6 
[LINT]: Address rust-lang/rust#64169
This was referenced Jan 4, 2020
Closed
Merged
@davidMcneil davidMcneil mentioned this issue Jan 13, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rustbot rustbot

Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add lint and tests for unnecessary parens around types jack-t/rust
7 participants
@ijt @Centril @jack-t @sjmann @csmoe @rustbot @oli-cosmian