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 space needed after right parenthesis of function symbol formals #809

Closed
renkun-ken opened this issue Jun 8, 2021 · 3 comments · Fixed by #830
Closed

Linter for space needed after right parenthesis of function symbol formals #809

renkun-ken opened this issue Jun 8, 2021 · 3 comments · Fixed by #830
Labels
feature a feature request or enhancement

Comments

@renkun-ken
Copy link
Collaborator

Just notice that there seems to be no linter that complains about the following code

function(x)x + 1

where a space is needed between function(x) and x + 1.

@kpagacz
Copy link
Contributor

kpagacz commented Jul 7, 2021

I don't know if there is any contribution guideline for lintr, therefore I am just going to ask - is it alright if I start working on this?

Do you think it should be a new linter or be a part of an already existing linter?

@renkun-ken
Copy link
Collaborator Author

The following linters might be related to this issue, but they are quite unique on their own.

$ <text>:List of 8
  ..$ filename     : chr "<text>"
  ..$ line_number  : int 1
  ..$ column_number: int 15
  ..$ type         : chr "style"
  ..$ message      : chr "Remove spaces before the left parenthesis in a function call."
  ..$ line         : Named chr "f <- function (x){x + 1}"
  .. ..- attr(*, "names")= chr "1"
  ..$ ranges       : NULL
  ..$ linter       : chr "function_left_parentheses_linter"
  ..- attr(*, "class")= chr "lint"
 $ <text>:List of 8
  ..$ filename     : chr "<text>"
  ..$ line_number  : int 1
  ..$ column_number: int 18
  ..$ type         : chr "style"
  ..$ message      : chr "Opening curly braces should never go on their own line and should always be followed by a new line."
  ..$ line         : Named chr "f <- function (x){x + 1}"
  .. ..- attr(*, "names")= chr "1"
  ..$ ranges       : NULL
  ..$ linter       : chr "open_curly_linter"
  ..- attr(*, "class")= chr "lint"
 $ <text>:List of 8
  ..$ filename     : chr "<text>"
  ..$ line_number  : int 1
  ..$ column_number: int 18
  ..$ type         : chr "style"
  ..$ message      : chr "There should be a space between right parenthesis and an opening curly brace."
  ..$ line         : chr "f <- function (x){x + 1}"
  ..$ ranges       :List of 1
  .. ..$ : num [1:2] 18 18
  ..$ linter       : chr "paren_brace_linter"
  ..- attr(*, "class")= chr "lint"
 $ <text>:List of 8
  ..$ filename     : chr "<text>"
  ..$ line_number  : int 1
  ..$ column_number: int 24
  ..$ type         : chr "style"
  ..$ message      : chr "Closing curly-braces should always be on their own line, unless they are followed by an else."
  ..$ line         : Named chr "f <- function (x){x + 1}"
  .. ..- attr(*, "names")= chr "1"
  ..$ ranges       : NULL
  ..$ linter       : chr "closed_curly_linter"
  ..- attr(*, "class")= chr "lint"

Looks like it enhances the paren_brace_linter so that it not only requires that there be a space between the right parenthesis and an opening curly brace, but also requires a space between the right parenthesis and the function body if they are in the same line. It is more like a paren_space_linter then.

If we are not to enhancing existing linters in this way, then we will need a new linter to specifically check this.

@kpagacz
Copy link
Contributor

kpagacz commented Jul 7, 2021

I have created a PR for this issue. Let me know what you think.
#830

@AshesITR AshesITR added the feature a feature request or enhancement label Jul 9, 2021
AshesITR added a commit that referenced this issue Jul 14, 2021
* feat(new linter): added a new linter

* Added a new linter that checks for a space between the right parenthesis and the function body in function definitions that span one line and don't use braces

Closes #809

* Added a new line at the end of paren_body_linter.R

* Updated after review no 1

* Added `\`

* Updated NEWS and README

* update roxygen and document()

* update roxygen and document()

* Added a skip condition to the test with syntax available since R 4.1.0

* Refactored comparing versions

* update NEWS to indicate paren_body_linter() is a default linter.

Co-authored-by: Konrad Pagacz <konrad.pagacz@contractors.roche.com>
Co-authored-by: Alexander Rosenstock <alexander.rosenstock@web.de>
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