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

suggest semicolon on double-call that doesn't typecheck as callable #51098

Conversation

zackmdavis
Copy link
Member

This provides the desired suggestion for #51055.

The suggestion in changed UI test output for test/ui/block-result/issue-20862 doesn't actually solve that test file's problems, but you can at least see how it's a plausible guess, and arguably less confusing than the "not a function" label in that context.

r? @estebank

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

@estebank estebank left a comment

Choose a reason for hiding this comment

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

I'll write something more in depth later, but what do you think about the feedback?

| ^^^^^^^^^ not a function
| ^^^^^^-^^
| |
| help: try adding a semicolon: `;`
Copy link
Contributor

Choose a reason for hiding this comment

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

You can check wether the spans are in different lines and only suggest then.

error[E0618]: expected function, found `bool`
--> $DIR/issue-51055-missing-semicolon-between-call-and-tuple.rs:14:5
|
LL | vindictive() //~ ERROR expected function, found `bool`
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be nice if the main span here pointed at only vindictive(), given that that's where a function was expected (while also pointing at the entire span or the arguments, to make it clear what is happening.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, although I think this will require a bit of restructuring (need to assert that the expression is an ExprCall before you can get the callee span).

(regretfully, I'm out of rustc-dev time for at least a few days)

@shepmaster shepmaster added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 2, 2018
@TimNN
Copy link
Contributor

TimNN commented Jun 12, 2018

@zackmdavis: Could you give us an update on the status of this PR? If you don't have the time right now to work on this, that is fine, we just like to hear from PR authors every once in a while.

@zackmdavis
Copy link
Member Author

Regrets: #51149 (comment)

I don't think this will make the 1.28 cutoff, but I can reopen later

@zackmdavis zackmdavis closed this Jun 12, 2018
@estebank
Copy link
Contributor

@zackmdavis priorities. Deal with whatever is happening to you. Don't hesitate to reach out if there's anything I could do to help.

@estebank
Copy link
Contributor

@zackmdavis just wanted to know if you could have any time to get back to this. I feel that the changes needed to merge this should be relatively small.

@zackmdavis
Copy link
Member Author

Oh, right; rust-lang/rustfix#141 (blocking edition-relevant #53013) and #52537 (false-positive I introduced) are higher-priority for me, then this

@zackmdavis
Copy link
Member Author

Surprisingly, GitHub does not let you reopen pull requests after force-pushing?!

cannot_reopen

@estebank
Copy link
Contributor

@zackmdavis yes, I've been bitten by this too in the past 🤷‍♀️

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Nov 18, 2018
…you_call_them, r=estebank

in which the E0618 "expected function" diagnostic gets a makeover

A woman of wisdom once told me, "Better late than never." (Can't reopen the previously-closed pull request from six months ago [due to GitHub limitations](rust-lang#51098 (comment)).)

Now the main span focuses on the erroneous not-a-function callee, while showing the entire call expression is relegated to a secondary span. In the case where the erroneous callee is itself a call, we
point out the definition, and, if the call expression spans multiple lines, tentatively suggest a semicolon (because we suspect that the "outer" call is actually supposed to be a tuple).

![not_a_fn_1](https://user-images.githubusercontent.com/1076988/48309935-96755000-e538-11e8-9390-02a048abb0c2.png)

![not_a_fn_2](https://user-images.githubusercontent.com/1076988/48309936-98d7aa00-e538-11e8-8b9b-257bc77d6261.png)

The new `bug!` assertion is, in fact, safe (`confirm_builtin_call` is only called by `check_call`, which is only called with a first arg of kind `ExprKind::Call` in `check_expr_kind`).

Resolves rust-lang#51055.

r? @estebank
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants