Skip to content

treewide: tighten clippy lints#7483

Merged
rbradford merged 6 commits intocloud-hypervisor:mainfrom
phip1611:upstream-clippy2
Nov 21, 2025
Merged

treewide: tighten clippy lints#7483
rbradford merged 6 commits intocloud-hypervisor:mainfrom
phip1611:upstream-clippy2

Conversation

@phip1611
Copy link
Member

@phip1611 phip1611 commented Nov 17, 2025 •
edited
Loading

Follow-up of #7482 to improve the code quality of Cloud Hypervisor in a sustainable way.
Most of the new individual lints are carefully selected from clippy::pedantic. The current PR reflects the number of lints Rob approved for the first lint tightening iteration.

Hints for Reviewers

  • I'd suggest we discuss and decide for each lint separately whether we want them. Therefore, I created one commit per lint for helping you finding a decision Rob already answered this
  • Please review commit by commit

Steps to Undraft

@phip1611 phip1611 requested a review from a team as a code owner November 17, 2025 15:21
@phip1611 phip1611 self-assigned this Nov 17, 2025
@phip1611 phip1611 marked this pull request as draft November 17, 2025 15:22
@phip1611 phip1611 changed the title add lint clippy::must_use_candidate to workspace [DRAFT, BLOCKED] add lint clippy::must_use_candidate to workspace Nov 17, 2025
@phip1611 phip1611 force-pushed the upstream-clippy2 branch 2 times, most recently from c506844 to a1371c9 Compare November 18, 2025 08:55
@phip1611 phip1611 changed the title [DRAFT, BLOCKED] add lint clippy::must_use_candidate to workspace treewide: tighten clippy lints Nov 18, 2025
@phip1611 phip1611 marked this pull request as ready for review November 18, 2025 08:56
@phip1611 phip1611 mentioned this pull request Nov 18, 2025
Closed
3 tasks
@rbradford
Copy link
Member

rbradford commented Nov 18, 2025

@phip1611 Everything up to "misc: clippy: add manual_string_new" seems very non controversial to me - can you split that into a separate PR that we can land. The rest I think we might need to think about some more.

@phip1611 phip1611 force-pushed the upstream-clippy2 branch 4 times, most recently from b215dfc to 8911af4 Compare November 20, 2025 08:50
rbradford
rbradford approved these changes Nov 20, 2025
View reviewed changes
Copy link
Member

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

Thanks. lgtm to me - does anyone else have an opinion on this?

@phip1611 phip1611 force-pushed the upstream-clippy2 branch 3 times, most recently from 861cb59 to 9ea9ce4 Compare November 20, 2025 17:59
@phip1611
Copy link
Member Author

phip1611 commented Nov 20, 2025

I removed missing_const_for_fn for now from this PR as I'm unsure about the benefit in Cloud Hypervisor. It is very invasive. We should discuss this in a dedicated place.

@phip1611 phip1611 mentioned this pull request Nov 20, 2025
Closed
13 tasks
@phip1611 phip1611 force-pushed the upstream-clippy2 branch 2 times, most recently from 80abb66 to d78fae0 Compare November 20, 2025 18:48
likebreath
likebreath approved these changes Nov 20, 2025
View reviewed changes
Copy link
Member

@likebreath likebreath 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. Not very sure about unnecessary_semicolon and semicolon_if_nothing_returned, but no strong opinions here.

@phip1611
Copy link
Member Author

phip1611 commented Nov 20, 2025 •
edited
Loading

Overall looks good. Not very sure about unnecessary_semicolon and semicolon_if_nothing_returned, but no strong opinions here.

Thanks! I don't have a strong opinion either. But if I read the lint descriptions, I have a tendency that it is beneficial.

@up2wing
Copy link
Contributor

up2wing commented Nov 21, 2025

LGTM.

I suppose this serial is good for the code quality, thx.

@phip1611
misc: clippy: add default clippy lint groups
d4fdda1 
This is the first commit in a series of commits to improve the Code
Quality in Cloud Hypervisor in a sustainable way. These are the
default rules from `clippy::all` but written here to be more explicit.
`clippy::all` refers to all "default sensible" lints, not all
existing lints.

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
@phip1611
misc: clippy: add unnecessary_semicolon
5df3d0e 
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
@phip1611
misc: clippy: add uninlined_format_args
497ba7b 
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
@phip1611
misc: clippy: add semicolon_if_nothing_returned
25e9935 
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
@phip1611
misc: clippy: add manual_string_new
4002841 
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
@phip1611
vmm: unrelated small code improvements
edfc787 
Unfortunately, there is no lint for that.

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
@rbradford rbradford enabled auto-merge November 21, 2025 08:46
@phip1611
Copy link
Member Author

phip1611 commented Nov 21, 2025

I don't quite see how this suddenly fails but never failed before? https://github.com/cloud-hypervisor/cloud-hypervisor/actions/runs/19564834479/job/56024328565?pr=7483

@rbradford rbradford added this pull request to the merge queue Nov 21, 2025
Merged via the queue into cloud-hypervisor:main with commit f02745a Nov 21, 2025
43 of 45 checks passed
@phip1611 phip1611 mentioned this pull request Nov 21, 2025
Merged
@phip1611 phip1611 deleted the upstream-clippy2 branch November 21, 2025 15:20
@phip1611 phip1611 mentioned this pull request Nov 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rbradford rbradford rbradford approved these changes

@up2wing up2wing up2wing approved these changes

@alyssais alyssais alyssais approved these changes

@likebreath likebreath likebreath approved these changes

Assignees

@phip1611 phip1611

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@phip1611 @rbradford @up2wing @alyssais @likebreath