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.

Sign up for GitHub

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

Jump to bottom

Remove match_path and match_qpath #7049

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Remove match_path and match_qpath #7049

merged 1 commit into from
Apr 16, 2021

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Apr 7, 2021

The only remaining usage is the author lint, so the functions are left in for now. The test result for repl_uninit changed, the lint was broken before.

The collapsible_span_lint_calls and match_type_on_diag_item tests have been changed. Both lints were broken when utils was extracted into it's own crate. match_type_on_diag_item isn't quite fixed, but it's at least less broken.

changelog: None

@rust-highfive
Copy link

r? @flip1995

(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 7, 2021
@camsteffen
Copy link
Contributor

Thanks for jumping on this! I think there is some overlapping work here with #7047. Also I think we should jump straight to diagnostic items where possible.

@Jarcho Jarcho force-pushed the remove_match_path branch from b632c92 to db40bd4 Compare April 7, 2021 22:26
@flip1995
Copy link
Member

flip1995 commented Apr 8, 2021

r? @camsteffen You've done more work in this direction recently, so I leave reviewing this PR to you.

@rust-highfive rust-highfive assigned camsteffen and unassigned flip1995 Apr 8, 2021
camsteffen
camsteffen reviewed Apr 8, 2021
View reviewed changes
clippy_utils/src/lib.rs Show resolved Hide resolved
clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
tests/ui-internal/collapsible_span_lint_calls.rs Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Apr 8, 2021

☔ The latest upstream changes (presumably #7023) made this pull request unmergeable. Please resolve the merge conflicts.

@boxdot boxdot mentioned this pull request Apr 12, 2021
Merged
@Jarcho Jarcho force-pushed the remove_match_path branch 2 times, most recently from 252bc07 to 56e6bb8 Compare April 13, 2021 03:07
@Jarcho
Copy link
Contributor Author

Jarcho commented Apr 13, 2021

Remaining util functions:

  • expr_path_res
  • is_qpath_def_path
  • is_expr_path_def_path
  • match_any_def_paths

@Jarcho Jarcho force-pushed the remove_match_path branch 2 times, most recently from d99bb68 to a408d4d Compare April 13, 2021 11:26
@bors
Copy link
Contributor

bors commented Apr 13, 2021

☔ The latest upstream changes (presumably #7074) made this pull request unmergeable. Please resolve the merge conflicts.

camsteffen
camsteffen suggested changes Apr 14, 2021
View reviewed changes
clippy_lints/src/implicit_saturating_sub.rs Outdated Show resolved Hide resolved
clippy_lints/src/map_identity.rs Outdated Show resolved Hide resolved
clippy_lints/src/implicit_saturating_sub.rs Outdated Show resolved Hide resolved
clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
clippy_lints/src/question_mark.rs Outdated Show resolved Hide resolved
clippy_lints/src/slow_vector_initialization.rs Outdated Show resolved Hide resolved
camsteffen
camsteffen reviewed Apr 14, 2021
View reviewed changes
clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
@Jarcho Jarcho force-pushed the remove_match_path branch 2 times, most recently from cb19326 to 4929caa Compare April 15, 2021 01:15
@Jarcho Jarcho force-pushed the remove_match_path branch 5 times, most recently from 5764a13 to 2205c55 Compare April 15, 2021 11:45
@camsteffen
Copy link
Contributor

Let me know when you're ready for another review in a comment or in the github UI. After the next review, please push any additional changes as separate commits so that I can see what changed, and then squash at the end.

@Jarcho
Copy link
Contributor Author

Jarcho commented Apr 15, 2021

Good to keep going.

camsteffen
camsteffen suggested changes Apr 15, 2021
View reviewed changes
clippy_lints/src/slow_vector_initialization.rs Show resolved Hide resolved
clippy_utils/src/paths.rs Outdated Show resolved Hide resolved
tests/ui-internal/match_type_on_diag_item.stderr Show resolved Hide resolved
tests/ui/repl_uninit.stderr Outdated Show resolved Hide resolved
@Jarcho Jarcho force-pushed the remove_match_path branch from 76dcd0d to ba9e155 Compare April 15, 2021 20:33
camsteffen
camsteffen approved these changes Apr 15, 2021
View reviewed changes
Copy link
Contributor

@camsteffen camsteffen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, thanks! Squash and then r=me

@Jarcho
Remove all usages of match_path, match_qpath and match_path_ast…
f6c5d8d 
… except the `author` lint.

Add note to fix `MATCH_TYPE_ON_DIAG_ITEM`
Add false negative test for `uninit_assumed_init`
@Jarcho Jarcho force-pushed the remove_match_path branch from ba9e155 to f6c5d8d Compare April 15, 2021 23:28
@camsteffen
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 15, 2021

📌 Commit f6c5d8d has been approved by camsteffen

@bors
Copy link
Contributor

bors commented Apr 15, 2021

⌛ Testing commit f6c5d8d with merge 586a993...

@bors
Copy link
Contributor

bors commented Apr 16, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: camsteffen
Pushing 586a993 to master...

@bors bors merged commit 586a993 into rust-lang:master Apr 16, 2021
This was referenced Apr 16, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camsteffen camsteffen camsteffen approved these changes

Assignees

@camsteffen camsteffen

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Jarcho @rust-highfive @camsteffen @flip1995 @bors