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

feat: add new lint seek_from_current #9681

Merged
merged 1 commit into from
Oct 26, 2022
Merged

feat: add new lint seek_from_current #9681

merged 1 commit into from
Oct 26, 2022

Conversation

koka831
Copy link
Contributor

@koka831 koka831 commented Oct 20, 2022

changelog: seek_from_current: new lint to suggest using stream_position instead of seek from current position with SeekFrom::Current(0)

addresses #7886.

This PR is related to #9667, so I will update methods/mod.rs if it get conflicted.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @giraffate (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 20, 2022
giraffate
giraffate reviewed Oct 21, 2022
View reviewed changes
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

This needs to implement msrv because stream_position was implemented at 1.51.0: https://doc.rust-lang.org/stable/std/io/trait.Seek.html#method.stream_position

This comment is helpful for implementing msrv: #9667 (review)

clippy_lints/src/methods/mod.rs Outdated
/// Ok(())
/// }
/// ```
#[clippy::version = "1.65.0"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[clippy::version = "1.65.0"]
#[clippy::version = "1.66.0"]

@koka831
Copy link
Contributor Author

koka831 commented Oct 21, 2022

@giraffate Thank you for review, fixed in 42a0c72

@bors
Copy link
Collaborator

bors commented Oct 23, 2022

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

@koka831 koka831 force-pushed the feat/add-seek-from-current-lint branch from 42a0c72 to 84d08f4 Compare October 23, 2022 23:10
giraffate
giraffate reviewed Oct 24, 2022
View reviewed changes
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

clippy_lints/src/lib.register_complexity.rs and src/docs.rs seem to include the unrelated changes, so can these be addressed?

giraffate
giraffate reviewed Oct 24, 2022
View reviewed changes
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

Can the test for msrv be added?
ref: https://github.com/rust-lang/rust-clippy/pull/8692/files#diff-451ecfe3eb1cef4b5a0b556b3c1f66eea9fc2fa0c48ceb40110efc0d45d58215

@koka831 koka831 force-pushed the feat/add-seek-from-current-lint branch 4 times, most recently from 34e6bd5 to b2e0526 Compare October 24, 2022 10:12
@koka831
Copy link
Contributor Author

koka831 commented Oct 24, 2022 •
edited
Loading

@giraffate fixed!
I also removed document txt file in https://github.com/rust-lang/rust-clippy/compare/34e6bd5e544f5bfae0c93cd55c8c665e3f44d79a..b2e0526039132a86e4d8535d565b34b37015f61e, according to #9541

@bors
Copy link
Collaborator

bors commented Oct 25, 2022

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

giraffate
giraffate approved these changes Oct 25, 2022
View reviewed changes
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

Overall looks good, thanks!

I'll merge this once the conflicts are resolved.

@koka831
feat: add new lint seek_from_current
6efb3a2 
addresses rust-lang#7886
added `seek_from_current` complexity lint.
it checks use of `Seek#seek` with `SeekFrom::Current(0)` and
suggests `Seek#stream_position` method

fix: add msrv

fix: register LintInfo

fix: remove unnecessary files

fix: add test for msrv

fix: remove

fix

fix: remove docs
@koka831 koka831 force-pushed the feat/add-seek-from-current-lint branch from b2e0526 to 6efb3a2 Compare October 25, 2022 03:27
@koka831
Copy link
Contributor Author

koka831 commented Oct 25, 2022 •
edited
Loading

Thanks!
rebased on #9667 and tweak some order alphabetically during resolving conflicts.

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Oct 26, 2022

📌 Commit 6efb3a2 has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 26, 2022

⌛ Testing commit 6efb3a2 with merge 7182a6b...

@bors
Copy link
Collaborator

bors commented Oct 26, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 7182a6b to master...

@bors bors merged commit 7182a6b into rust-lang:master Oct 26, 2022
@koka831 koka831 deleted the feat/add-seek-from-current-lint branch October 26, 2022 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giraffate giraffate giraffate approved these changes

Assignees

@giraffate giraffate

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.
4 participants
@koka831 @rust-highfive @bors @giraffate