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.

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

max_function_length counts multiple function clauses as a single function #335

Open
kivra-pauoli opened this issue Feb 16, 2024 · 5 comments · May be fixed by #365
Open

max_function_length counts multiple function clauses as a single function #335

kivra-pauoli opened this issue Feb 16, 2024 · 5 comments · May be fixed by #365
Labels
Milestone

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

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 linked a pull request Oct 1, 2024 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants