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

hi-rustin
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!

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>
@hi-rustin hi-rustin 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
@hi-rustin hi-rustin 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
@hi-rustin hi-rustin 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
@hi-rustin hi-rustin 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
@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.

None yet

5 participants