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

WIP: New linter to block stop(paste(...)) calls #530

Closed
wants to merge 3 commits into from
Closed

WIP: New linter to block stop(paste(...)) calls #530

wants to merge 3 commits into from

Conversation

MichaelChirico
Copy link
Collaborator

Closes #527

Filing now for early feedback. If you're OK to add the linter here I will complete the PR by adding tests etc.

Informally there are these test cases thus far:

writeLines('
# bad
stop(paste("a", "b", "c"))
warning(paste0("hey", "you"))
message(paste("there", "we", "go"))
stop(paste("a", "b", "c", sep=""))

# good
stop("a", "b", "c")
paste0("a", "b", "c")
stop("go to", paste("jail", "now"))
stop(paste("a", "b"), "c")
stop(paste("a", "b", collapse=","))
stop(paste("a", "b", sep="|"))
', tmp <- tempfile())

lintr::lint(tmp, stop_paste_linter)
/tmp/RtmpGfxjfA/file38c5e31aefde27:3:1: warning: Avoid stop(paste(...)) expressions, just use stop(...). Disable this with a comment...
stop(paste("a", "b", "c"))
^~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/RtmpGfxjfA/file38c5e31aefde27:4:1: warning: Avoid warning(paste0(...)) expressions, just use warning(...). Disable this with a comment...
warning(paste0("hey", "you"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/RtmpGfxjfA/file38c5e31aefde27:5:1: warning: Avoid message(paste(...)) expressions, just use message(...). Disable this with a comment...
message(paste("there", "we", "go"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/RtmpGfxjfA/file38c5e31aefde27:6:1: warning: Avoid stop(paste(...)) expressions, just use stop(...). Disable this with a comment...
stop(paste("a", "b", "c", sep=""))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Closes #527

Filing now for early feedback. If you're OK to add the linter here I will complete the PR by adding tests etc.

Informally there are these test cases thus far:

```
writeLines('
# bad
stop(paste("a", "b", "c"))
warning(paste0("hey", "you"))
message(paste("there", "we", "go"))
stop(paste("a", "b", "c", sep=""))

# good
stop("a", "b", "c")
paste0("a", "b", "c")
stop("go to", paste("jail", "now"))
stop(paste("a", "b"), "c")
stop(paste("a", "b", collapse=","))
stop(paste("a", "b", sep="|"))
', tmp <- tempfile())

lintr::lint(tmp, stop_paste_linter)
/tmp/RtmpGfxjfA/file38c5e31aefde27:3:1: warning: Avoid stop(paste(...)) expressions, just use stop(...). Disable this with a comment...
stop(paste("a", "b", "c"))
^~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/RtmpGfxjfA/file38c5e31aefde27:4:1: warning: Avoid warning(paste0(...)) expressions, just use warning(...). Disable this with a comment...
warning(paste0("hey", "you"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/RtmpGfxjfA/file38c5e31aefde27:5:1: warning: Avoid message(paste(...)) expressions, just use message(...). Disable this with a comment...
message(paste("there", "we", "go"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/RtmpGfxjfA/file38c5e31aefde27:6:1: warning: Avoid stop(paste(...)) expressions, just use stop(...). Disable this with a comment...
stop(paste("a", "b", "c", sep=""))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
@AshesITR
Copy link
Collaborator

AshesITR commented Oct 7, 2020

You need to devtools::document() so the Collate field is properly re-generated in DESCRIPTION.
cf. the Travis error log:

https://travis-ci.org/github/jimhester/lintr/jobs/732649411#L932-L933

@MichaelChirico
Copy link
Collaborator Author

Thanks @AshesITR; done

@MichaelChirico
Copy link
Collaborator Author

Closing this -- it's not a tidyverse style issue (as of now) so it's more appropriate for a different package of add-on linters

@MichaelChirico MichaelChirico deleted the stop-paste-linter branch March 13, 2021 02:44
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.

Suggested linter: stop|warning|message(paste(...))
2 participants