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

Fix has_no_input_arg check and rename it to has_only_self_parameter #71270

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

Rustin170506
Copy link
Member

Signed-off-by: Rustin-Liu rustin.liu@gmail.com

Fixes #70643 (comment).

@rust-highfive
Copy link
Collaborator

r? @eddyb

(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 Apr 18, 2020
@@ -246,7 +246,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// This function checks if the method isn't static and takes other arguments than `self`.
Copy link
Member Author

Choose a reason for hiding this comment

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

I am confusing about this comment, is this correct?

Copy link
Member

Choose a reason for hiding this comment

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

It's a confusing comment. And the name of the function is also probably wrong. cc @estebank

Copy link
Member Author

Choose a reason for hiding this comment

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

I think maybe the function actually wants to do is to check that the function only has a self arg and no other args.

Copy link
Contributor

@estebank estebank Apr 19, 2020

Choose a reason for hiding this comment

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

This code seems to only be used for suggestions to convert a type in a whitelist, so I think the "has no input arg" should be read/rewritten as "check that this associated item can be called without any additional arguments, like <&str>::to_string(self)". Feel free to rewrite the docstring and method name.

The code change seems correct, r=me on that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@estebank @eddyb Updated, PTAL~ Thanks!

@Rustin170506 Rustin170506 force-pushed the rustin-patch-has-self branch from 61ac0dc to 8a32e51 Compare April 19, 2020 05:31
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>

rename has_no_input_arg to has_only_self_parameter

Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@Rustin170506 Rustin170506 force-pushed the rustin-patch-has-self branch from 8a32e51 to 7c28dfe Compare April 19, 2020 05:39
@Rustin170506 Rustin170506 changed the title Fix has_no_input_arg function has self check Rename has_no_input_arg to has_only_self_parameter Apr 19, 2020
@Rustin170506 Rustin170506 changed the title Rename has_no_input_arg to has_only_self_parameter Rename function has_no_input_arg to has_only_self_parameter Apr 19, 2020
@Rustin170506 Rustin170506 changed the title Rename function has_no_input_arg to has_only_self_parameter Fix has_no_input_arg check and rename it to has_only_self_parameter Apr 19, 2020
@Rustin170506 Rustin170506 changed the title Fix has_no_input_arg check and rename it to has_only_self_parameter Fix has_no_input_arg check and rename it to has_only_self_parameter Apr 19, 2020
@Rustin170506 Rustin170506 requested review from eddyb and estebank April 20, 2020 13:39
@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 20, 2020

📌 Commit 7c28dfe has been approved by estebank

@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 Apr 20, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#71250 (Replace big JS dict with JSON parsing)
 - rust-lang#71270 (Fix `has_no_input_arg` check and rename it to `has_only_self_parameter`)
 - rust-lang#71284 (fix -Zast-json to output correct JSON form)
 - rust-lang#71328 (Stabilize PathBuf capacity methods)
 - rust-lang#71334 (Update pattern docs.)

Failed merges:

r? @ghost
@bors bors merged commit e80d303 into rust-lang:master Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants