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

Linter for function length #361

Closed
kenahoo opened this issue Dec 18, 2018 · 2 comments · Fixed by #396
Closed

Linter for function length #361

kenahoo opened this issue Dec 18, 2018 · 2 comments · Fixed by #396
Labels
feature a feature request or enhancement

Comments

@kenahoo
Copy link

kenahoo commented Dec 18, 2018

It looks like there's no existing linter for function length (number of lines, or number of lines of code). Would that be a valuable addition?

I ask because I've been asked to code review a package that other people wrote, and there are several extremely long functions, and it would be nice to get a linter to report these facts for me. =)

Obviously I am aware that there's no agreed-upon max length for functions, that number of lines of code is a poor indicator of complexity, etc. - would like to get something that at least gets in the right ballpark, though.

@jimhester
Copy link
Member

I believe the good practice has this, among other things. Or if not literally a check for line length it has a cyclomatic complexity check, which should usually flag the same functions

@kenahoo
Copy link
Author

kenahoo commented Dec 19, 2018

Looks like it only has cyclomatic complexity. Complexity is sometimes related to length, but often a bit orthogonal:

> library(cyclocomp)
> cyclocomp( function(arg) { do(this); and(that) } )
[1] 1
> cyclocomp( function(arg) { do(this); and(that); and(the - other - thing) } )
[1] 1

Some of the functions I'm looking at are hundreds of lines long without much branching complexity at all.

Would line length be a more appropriate addition here, or in Good Practice? Do you imagine this package accumulating lots of different linters that people contribute, or farming that out to other derived packages? Or something in the middle where people can declare linter dependencies in their .lintr file, maybe?

@russHyde russHyde added the feature a feature request or enhancement label Jan 12, 2019
jimhester pushed a commit that referenced this issue Aug 23, 2019
* [WIP] linter works

* adds linting for cyclomatic complexity.

incurs dependency on cyclocomp.
closes #361.

* updates NEWS for cyclocomp_linter

* less verbose message for cyclocomp_linter

* update tests to new message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants