Skip to content

too_many_lines: only highlight the function signature #15461

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
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

too_many_lines: only highlight the function signature #15461

wants to merge 4 commits into from
+138 −42

Conversation

ada4a
Copy link
Contributor

@ada4a ada4a commented Aug 11, 2025

resolves #15430

changelog: [too_many_lines]: only highlight the function signature

@rustbot
Copy link
Collaborator

rustbot commented Aug 11, 2025

r? @Alexendoo

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 11, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 11, 2025 •
edited
Loading

Lintcheck changes for 49666f6

Lint Added Removed Changed
clippy::too_many_lines 0 0 253

This comment will be updated if you push new changes

samueltardieu
samueltardieu reviewed Aug 11, 2025
View reviewed changes
Alexendoo
Alexendoo reviewed Aug 12, 2025
View reviewed changes
clippy_lints/src/functions/too_many_lines.rs Outdated
Comment on lines 77 to 78
span,
span.with_hi(decl.output.span().hi()),
Copy link
Member

Choose a reason for hiding this comment

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

You can pass in def_id and use cx.tcx.def_span() here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, thanks! I borrowed the decl thing from too_many_arguments, so I'll change that lint too then (in a separate PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though I don't quite understand why that span ends up being different from the one passed to check_fn (which spans the whole function)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at https://github.com/rust-lang/rust-clippy/actions/runs/16912149309#user-content-too-many-arguments (from a PR that changes too_many_arguments to use def_span as well), it looks like def_span also includes the where bounds -- and that's kind of unnecessary imo. What do you think?

ada4a added a commit to ada4a/rust-clippy that referenced this pull request Aug 12, 2025
@ada4a
use cx.tcx.def_span
31fc027 
as recommended in
rust-lang#15461 (comment)
@ada4a ada4a mentioned this pull request Aug 12, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samueltardieu samueltardieu samueltardieu left review comments

@Alexendoo Alexendoo Alexendoo left review comments

Assignees

@Alexendoo Alexendoo

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.

too_many_lines should only highlight the function signature, and not the entire function body
4 participants
@ada4a @rustbot @samueltardieu @Alexendoo