Skip to content

Conversation

TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Jul 10, 2022

This patch remove a string matching about methods and adds some rustfix tests.

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

r? @jackh726

(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 Jul 10, 2022
@TaKO8Ki TaKO8Ki force-pushed the remove-string-matching-about-methods branch from 24b0b0d to 1599c5a Compare July 10, 2022 17:23
@TaKO8Ki TaKO8Ki changed the title Remove a string matching about methods Refactor: remove a string matching about methods Jul 13, 2022
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

Thanks @TaKO8Ki. Can we take this opportunity to simplify the code for this suggestion?

if receiver.ends_with(&method_call) {
None // do not suggest code that is already there (#53348)
let mut suggestions = iter::zip(iter::repeat(&expr), &methods)
.filter_map(|(receiver_expr, method)| {
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be simpler to bind receiver_expr as a variable above and use it as the closure environment.

&& receiver_method.ident.name == sym::clone
&& method_call_list.contains(&method_call.as_str())
{
vec![(
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 comment to explain what is the purpose of this branch?

@TaKO8Ki TaKO8Ki requested a review from cjgillot July 14, 2022 06:31
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

Thanks, that is much clearer.
A couple more nits then r=me.

@TaKO8Ki TaKO8Ki force-pushed the remove-string-matching-about-methods branch from 4a11f33 to 45b88af Compare July 15, 2022 05:31
@cjgillot
Copy link
Contributor

Thanks !
@bors r+

@bors
Copy link
Collaborator

bors commented Jul 15, 2022

📌 Commit 45b88af has been approved by cjgillot

It is now in the queue for this repository.

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

Successful merges:

 - rust-lang#88991 (Add Nintendo Switch as tier 3 target)
 - rust-lang#98869 (Remove some usages of `guess_head_span`)
 - rust-lang#99119 (Refactor: remove a string matching about methods)
 - rust-lang#99209 (Correctly handle crate level page on docs.rs as well)
 - rust-lang#99246 (Update RLS)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 24f0e14 into rust-lang:master Jul 15, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 15, 2022
@TaKO8Ki TaKO8Ki deleted the remove-string-matching-about-methods branch July 16, 2022 03:32
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. 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