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.

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

Add a suggestion to replace parentheses with angle brackets on associated trait constraint #97656

Merged

Conversation

EdwinRy
Copy link
Contributor

@EdwinRy EdwinRy commented Jun 2, 2022

This implements a requested suggestion FIXME in compiler/rustc_ast_lowering/src/lib.rs

The suggestion asks for the parentheses to either be replaced with angle brackets or removed completely depending on whether there are arguments provided within.

image

image

r? @oli-obk

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 2, 2022
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 2, 2022
@oli-obk oli-obk added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` labels Jun 2, 2022
&& snippet.ends_with(')')
&& let Some(split) = snippet.find("(") {

let trait_name = &snippet[0..split];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the trait name. According to the parser, split is always 0.


// Suggest removing empty parentheses: "Trait()" -> "Trait"
if data.inputs.is_empty() {
err.span_suggestion(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for this case?

gen_args.span(),
"parenthesized generic arguments cannot be used in associated type constraints",
);
if let Ok(snippet) = self.sess.source_map().span_to_snippet(data.span)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid re-parsing the source code, you have all the required information in the spans.
The span of the ( is data.span.shrink_to_lo().to(data.inputs.first().unwrap().span().shrink_to_lo()).
Similarly computation for the span for ).
You can use a multipart suggestion to suggest replacing ( by < and ) by >, without touching to the args' code.

@cjgillot
Copy link
Contributor

cjgillot commented Jun 2, 2022

Other nit: please avoid merge commits in your PR. In case of merge conflict, you can rebase your branch: https://rustc-dev-guide.rust-lang.org/git.html

@EdwinRy EdwinRy closed this Jun 2, 2022
@EdwinRy EdwinRy force-pushed the error_ast_low_type_contraint_parentheses branch from 5cbd004 to 9598b4b Compare June 2, 2022 17:20
@EdwinRy EdwinRy reopened this Jun 2, 2022
@cjgillot
Copy link
Contributor

cjgillot commented Jun 3, 2022

Thanks @EdwinRy! Could you squash the commits? I'll get them merged.

@EdwinRy EdwinRy force-pushed the error_ast_low_type_contraint_parentheses branch from c80cd7c to cd03fe1 Compare June 3, 2022 15:49
@cjgillot
Copy link
Contributor

cjgillot commented Jun 3, 2022

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 3, 2022

📌 Commit cd03fe1 has been approved by cjgillot

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2022
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#97446 (Make hir().get_generics and generics_of consistent.)
 - rust-lang#97656 (Add a suggestion to replace parentheses with angle brackets on associated trait constraint)
 - rust-lang#97692 (Add `#T-types/nominated` zulip notification)
 - rust-lang#97696 (Do not ICE when failing to normalize during inlining.)
 - rust-lang#97702 (Remove useless LocalDefId in ImplTraitContext::Universal.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 401be78 into rust-lang:master Jun 4, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 4, 2022
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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants