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

Tweak output on E0599 for assoc fn used as method #51135

Merged
merged 5 commits into from
Jun 2, 2018

Conversation

estebank
Copy link
Contributor

  • Use suggestion instead of help when possible
  • Add primary span label
  • Remove incorrect help suggestion using incorrect syntax
  • Do not refer to only one possible candidate as candidate #1, refer to it as the candidate

When encountering an unexisting method for a given trait where an
associated function has the same name, suggest using the appropriate
syntax, instead of using `help` text.

When only one candidate is found, do not call it "candidate rust-lang#1", just
call it "the candidate".
@rust-highfive
Copy link
Collaborator

r? @petrochenkov

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 28, 2018
@rust-highfive

This comment has been minimized.

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

Doesn't need to be acted upon in this PR, just some thoughts of what we should think about in the future

if static_sources.len() == 1 {
if let Some(expr) = rcvr_expr {
err.span_suggestion(expr.span.to(span),
"use associated function syntax intead",
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: intead

}
if static_sources.len() == 1 {
if let Some(expr) = rcvr_expr {
err.span_suggestion(expr.span.to(span),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is only the single source case a suggestion?

@@ -468,9 +495,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
} else {
let limit = if candidates.len() == 5 { 5 } else { 4 };
for (i, trait_did) in candidates.iter().take(limit).enumerate() {
msg.push_str(&format!("\ncandidate #{}: `use {};`",
Copy link
Contributor

Choose a reason for hiding this comment

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

These should probably become a real suggestion, in which case you don't need manual limiting anymore, because the renderer takes care of that

Copy link
Contributor

Choose a reason for hiding this comment

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

The suggestions for inserting use statements do that already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggestions currently do not preserve their position in the output, they always come at then end. I need to change that for the first requested fix.

@petrochenkov
Copy link
Contributor

r? @oli-obk

@estebank
Copy link
Contributor Author

estebank commented Jun 1, 2018

ping @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Jun 1, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Jun 1, 2018

📌 Commit 59b03b1 has been approved by oli-obk

@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 1, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 1, 2018
Tweak output on E0599 for assoc fn used as method

 - Use suggestion instead of `help` when possible
 - Add primary span label
 - Remove incorrect `help` suggestion using incorrect syntax
 - Do not refer to only one possible candidate as `candidate #1`, refer to it as `the candidate`
bors added a commit that referenced this pull request Jun 2, 2018
Rollup of 5 pull requests

Successful merges:

 - #51135 (Tweak output on E0599 for assoc fn used as method)
 - #51152 (Replace `if` with `if and only if` in the definition dox of `Sync`)
 - #51262 (Add missing whitespace in num example)
 - #51272 (Remove feature flag from fs::read_to_string example)
 - #51286 (Pull 1.26.2 release notes into master)

Failed merges:
@bors
Copy link
Contributor

bors commented Jun 2, 2018

⌛ Testing commit 59b03b1 with merge 5f7c9da...

@bors bors merged commit 59b03b1 into rust-lang:master Jun 2, 2018
@bors
Copy link
Contributor

bors commented Jun 2, 2018

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 2, 2018
@estebank estebank deleted the sugg-7575 branch November 9, 2023 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants