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

Add new CountAsOne option for code length related Metric cops #8159

Merged
merged 1 commit into from Jul 6, 2020

Conversation

fatkodima
Copy link
Contributor

Closes #7724

This new setting allows to specify which ruby construction (array, hash and heredoc are currently implemented) we want to treat as one line.

The algorithm from the high level:

  1. we calculate the total length for the whole code of specified node
  2. then, for each the most top level descendant of foldable type, we returning back its code length to the total length
  3. and so after steps 1 and 2 we get a resulted length

@Drenmi
Copy link
Collaborator

Drenmi commented Jun 22, 2020

Great start!

I have been thinking about a similar setting myself, but the point I kept getting stuck at is the fact that arrays and hashes can have arbitrary amounts of code in them.

In the test cases for this configuration option, all the arrays and hashes are assumed to be shallow collections of primitive values, which is the case where this makes sense. But I must assume this introduces some false negatives? 🤔

@fatkodima
Copy link
Contributor Author

fatkodima commented Jun 22, 2020

In the test cases for this configuration option, all the arrays and hashes are assumed to be shallow collections of primitive values

No, the code works for arbitrary complex arrays/hashes. For example, the multiline array in the original issue would be folded into 1 line.
Looks like I missed to add test cases for more complex examples.

But I must assume this introduces some false negatives? 🤔

Am I answered also this question or should you provide an example then, if not?

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 6, 2020

Yeah, it'd be nice if there are some examples in the cop's description about using this new config.

@fatkodima
Copy link
Contributor Author

Updated with suggestions.

@bbatsov bbatsov merged commit 127c411 into rubocop:master Jul 6, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Jul 6, 2020

Thanks!

@colinlieberman
Copy link

@fatkodima - I know this is 8 months old now, but I found it today, updated rubocop to the latest version, and am very, very happy. Thank you for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metrics/MethodLength should ignore/configure configuration lists
4 participants