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

max_function_length counts multiple function clauses as a single function #335

Closed
kivra-pauoli opened this issue Feb 16, 2024 · 5 comments · Fixed by #365
Closed

max_function_length counts multiple function clauses as a single function #335

kivra-pauoli opened this issue Feb 16, 2024 · 5 comments · Fixed by #365
Labels
bug
Milestone
4.0.0

Comments

@kivra-pauoli
Copy link
Contributor

Bug Description

Note: might not be a bug.

max_function_length complains if a function definition spans over a given number of lines (by default: 30)...

... but it's also taking into account the multiple function clauses. So 15 clauses with 1 line each, in the body, would count for 30 lines. Is this expected?

To Reproduce

16 function clauses with 2 line (definition) each, 1 line for head, 1 line for body.

Expected Behavior

? no warning, maybe?

rebar3 Logs

Not applicable.

Additional Context

Not relevant.
@elbrujohalcon
Copy link
Member

Yeah, this rule was always a bit… blunt.
If we're going to adjust it, then it will need a rename as max_function_clause_length, I guess.

@kivra-pauoli
Copy link
Contributor Author

Or similar, yes. For the time being I'm Ok with just adding an exception in my rules, ofc. This was mostly just me wondering if we got (from the parsing) the size of each clause or not...

@elbrujohalcon elbrujohalcon added this to the 4.0.0 milestone Sep 16, 2024
@bormilan
Copy link
Contributor

I'm interested in working on this one.

Maybe the best is to rename the existing function to max_function_clause_length as @elbrujohalcon suggested and make another one with the name max_function_length.

Do you agree?

@elbrujohalcon
Copy link
Member

Yeah, go for it!!

@paulo-ferraz-oliveira
Copy link
Collaborator

Since this is a breaking change (not sure how you're gonna deal with that) you should remember to update the MIGRATION guide.

@bormilan bormilan mentioned this issue Oct 1, 2024
Merged
1 task
@paulo-ferraz-oliveira paulo-ferraz-oliveira changed the title max_function_length counts multiple function clauses as a single function max_function_length counts multiple function clauses as a single function Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
4.0.0
Development

Successfully merging a pull request may close this issue.

New elvis_style rule: max_function_clause_length bormilan/elvis_core
4 participants
@elbrujohalcon @paulo-ferraz-oliveira @bormilan @kivra-pauoli