Add lint and tests for unnecessary parens around types #65112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @zackmdavis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@jack-t Thanks! You can edit libcore/ and libstd/ as part of this PR. (It should be fine to remove those parens in |
|
(And, of course, if you find any cases that aren't fine to remove; that would be a bug in your lint change; I'll do my own audit of the |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
JohnCSimon
added
S-waiting-on-author
|
Ping from triage |
|
Thanks for following up on this, @JohnCSimon -- I had a few papers to write this week, which really bogged me down. I'm hoping the commit I just added to this PR will get the lint right. The last version incorrectly warned that the parentheses in, e.g., I've gone through all the variants in Separately, I found that this syntax is pretty much undocumented: |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
r? @varkor |
varkor
approved these changes
Oct 25, 2019
src/librustc_lint/unused.rs
Outdated
| fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) { | ||
| if let &ast::TyKind::Paren(ref r) = &ty.kind { | ||
| match &r.kind { | ||
| &ast::TyKind::TraitObject(..) => {}, |
There was a problem hiding this comment.
Suggested change
| &ast::TyKind::TraitObject(..) => {}, | |
| &ast::TyKind::TraitObject(..) => {} |
src/librustc_lint/unused.rs
Outdated
| if let &ast::TyKind::Paren(ref r) = &ty.kind { | ||
| match &r.kind { | ||
| &ast::TyKind::TraitObject(..) => {}, | ||
| &ast::TyKind::ImplTrait(_, ref bounds) if bounds.len() > 1 => {}, |
There was a problem hiding this comment.
Suggested change
| &ast::TyKind::ImplTrait(_, ref bounds) if bounds.len() > 1 => {}, | |
| &ast::TyKind::ImplTrait(_, ref bounds) if bounds.len() > 1 => {} |
src/librustc_lint/unused.rs
Outdated
| }; | ||
|
|
||
| Self::remove_outer_parens(cx, ty.span, &pattern_text, "type", (false, false)); | ||
| }, |
jack-t
force-pushed
the
type-parens-lint
branch
from
283f591
to
08ca236
Compare
|
Thanks, @varkor! The last commit in this PR is a squashed version of the previous commits, plus changes to address the changes you suggested above. Let me know if there's anything else I need to do! Jack |
|
📌 Commit 08ca236 has been approved by |
bors
added
S-waiting-on-bors
tmandry
added a commit
to tmandry/rust
that referenced
this pull request
Oct 30, 2019
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 pull request
Oct 30, 2019
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 pull request
Oct 31, 2019
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
added a commit
that referenced
this pull request
Oct 31, 2019
Rollup of 14 pull requests Successful merges: - #65112 (Add lint and tests for unnecessary parens around types) - #65459 (Fix `-Zunpretty=mir-cfg` to render multiple items) - #65471 (Add long error explanation for E0578) - #65857 (rustdoc: Resolve module-level doc references more locally) - #65914 (Use structured suggestion for unnecessary bounds in type aliases) - #65945 (Optimize long-linker-command-line test) - #65946 (Make `promote_consts` emit the errors when required promotion fails) - #65960 (doc: reword iter module example and mention other methods) - #65963 (update submodules to rust-lang) - #65972 (Fix libunwind build: Define __LITTLE_ENDIAN__ for LE targets) - #65977 (Fix incorrect diagnostics for expected type in E0271 with an associated type) - #65995 (Add error code E0743 for "C-variadic has been used on a non-foreign function") - #65997 (Fix outdated rustdoc of Once::init_locking function) - #66005 (vxWorks: remove code related unix socket) Failed merges: r? @ghost
tmandry
added a commit
to tmandry/rust
that referenced
this pull request
Nov 1, 2019
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
added a commit
that referenced
this pull request
Nov 1, 2019
Rollup of 16 pull requests Successful merges: - #65112 (Add lint and tests for unnecessary parens around types) - #65470 (Don't hide ICEs from previous incremental compiles) - #65471 (Add long error explanation for E0578) - #65857 (rustdoc: Resolve module-level doc references more locally) - #65902 (Make ItemContext available for better diagnositcs) - #65914 (Use structured suggestion for unnecessary bounds in type aliases) - #65946 (Make `promote_consts` emit the errors when required promotion fails) - #65960 (doc: reword iter module example and mention other methods) - #65963 (update submodules to rust-lang) - #65972 (Fix libunwind build: Define __LITTLE_ENDIAN__ for LE targets) - #65977 (Fix incorrect diagnostics for expected type in E0271 with an associated type) - #65995 (Add error code E0743 for "C-variadic has been used on a non-foreign function") - #65997 (Fix outdated rustdoc of Once::init_locking function) - #66002 (Stabilize float_to_from_bytes feature) - #66005 (vxWorks: remove code related unix socket) - #66018 (Revert PR 64324: dylibs export generics again (for now)) Failed merges: r? @ghost
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is my first contribution to the Rust project, so I apologize if I'm not doing things the right way.
The PR fixes #64169. It adds a lint and tests for unnecessary parentheses around types. I've run
tidyandrustfmt— 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::TyKindto find exceptions to this lint, and I could only find the one mentioned in the original issue, which concerns types withdyn. 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. Becausex.pyseems 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 warnon myx.py buildandx.py testcommands.