From a8349ae5514f6b2e78d2ae902618f7838f91aa2a Mon Sep 17 00:00:00 2001 From: mpadge Date: Wed, 25 Sep 2024 13:44:58 +0200 Subject: [PATCH 1/3] replace internal 'trailing_semicolon_linter' with lintr version for #171 --- R/chk_lintr.R | 4 ++-- R/my_linters.R | 37 ------------------------------------- R/prep_lintr.R | 2 +- tests/testthat/test-lintr.R | 10 +++++----- 4 files changed, 8 insertions(+), 45 deletions(-) diff --git a/R/chk_lintr.R b/R/chk_lintr.R index 4f2abbc..8c25a50 100644 --- a/R/chk_lintr.R +++ b/R/chk_lintr.R @@ -52,7 +52,7 @@ CHECKS$lintr_line_length_linter <- make_check( } ) -CHECKS$lintr_trailing_semicolon_linter <- make_check( +CHECKS$lintr_semicolon_linter <- make_check( description = "No trailing semicolons", tags = c("style", "lintr"), @@ -63,7 +63,7 @@ CHECKS$lintr_trailing_semicolon_linter <- make_check( forbid them", check = function(state) { - get_lintr_state(state, "trailing_semicolon_linter") + get_lintr_state(state, "semicolon_linter") } ) diff --git a/R/my_linters.R b/R/my_linters.R index 75e39fa..f9d87b6 100644 --- a/R/my_linters.R +++ b/R/my_linters.R @@ -1,43 +1,6 @@ #' @importFrom lintr Lint -trailing_semicolon_linter <- function() lintr::Linter(function(source_file) { - - allsc <- which(source_file$parsed_content$token == "';'") - - if (!length(allsc)) return(list()) - - ## Check that it is the last token in the line - ## Note that we need to restrict the search to terminal - ## nodes, because parse is buggy and sometimes reports false, - ## too large end positions for non-terminal nodes. - badsc <- Filter( - x = allsc, - f = function(line) { - with( - source_file$parsed_content, - all(! terminal | line1 != line1[line] | col2 <= col2[line]) - ) - } - ) - - lapply( - badsc, - function(line) { - parsed <- source_file$parsed_content[line, ] - Lint( - filename = source_file$filename, - line_number = parsed$line1, - column_number = parsed$col1, - type = "style", - message = "Avoid trailing semicolons, they are not needed.", - line = source_file$lines[as.character(parsed$line1)], - ranges = list(c(parsed$col1, parsed$col2)) - ) - } - ) -}) - #' Find dangerous 1:x expressions #' #' Find occurrences of \code{1:length(x)}, \code{1:nrow(x)}, diff --git a/R/prep_lintr.R b/R/prep_lintr.R index 3a9ded6..9d2fc8d 100644 --- a/R/prep_lintr.R +++ b/R/prep_lintr.R @@ -4,7 +4,7 @@ linters_to_lint <- list( assignment_linter = lintr::assignment_linter(), line_length_linter = lintr::line_length_linter(80), - trailing_semicolon_linter = trailing_semicolon_linter(), + semicolon_linter = lintr::semicolon_linter(allow_compound = TRUE), attach_detach_linter = attach_detach_linter(), setwd_linter = setwd_linter(), sapply_linter = sapply_linter(), diff --git a/tests/testthat/test-lintr.R b/tests/testthat/test-lintr.R index 0c0b9e2..c06be90 100644 --- a/tests/testthat/test-lintr.R +++ b/tests/testthat/test-lintr.R @@ -1,5 +1,5 @@ -# "lintr_trailing_semicolon_linter" fails in bad1 +# "lintr_semicolon_linter" fails in bad1 # "lintr_attach_detach_linter" fails in bad2 # "lintr_setwd_linter" fails in bad2 # "lintr_sapply_linter" fails in bad2 @@ -10,7 +10,7 @@ # "lintr_line_length_linter" gp_lintrs <- c("lintr_assignment_linter", "lintr_line_length_linter", - "lintr_trailing_semicolon_linter", + "lintr_semicolon_linter", "lintr_attach_detach_linter", "lintr_setwd_linter", "lintr_sapply_linter", "lintr_library_require_linter", "lintr_seq_linter") @@ -41,10 +41,10 @@ test_that("lintr_line_length_linter", { # TODO expectation/example where the check fails }) -test_that("lintr_trailing_semicolon_linter", { +test_that("lintr_semicolon_linter", { - expect_true(get_result(res_bad2, "lintr_trailing_semicolon_linter")) - expect_false(get_result(res_bad1, "lintr_trailing_semicolon_linter")) + expect_true(get_result(res_bad2, "lintr_semicolon_linter")) + expect_false(get_result(res_bad1, "lintr_semicolon_linter")) }) test_that("lintr_attach_detach_linter", { From df587bcb56355b9fa96154a110eb45babf7109aa Mon Sep 17 00:00:00 2001 From: mpadge Date: Wed, 25 Sep 2024 13:45:26 +0200 Subject: [PATCH 2/3] update readme for #171 --- README.md | 73 ++++++++++++++++++++++++------------------------------- 1 file changed, 32 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index e66e9a7..bf4724f 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,3 @@ - # goodpractice @@ -28,7 +27,7 @@ install.packages("goodpractice") and the development version from GitHub ``` r -remotes::install_github("ropensci-review-tools/goodpractice") +pak::pak("ropensci-review-tools/goodpractice") ``` ## Usage @@ -48,59 +47,52 @@ g <- gp(pkg_path) ``` #> ── R CMD build ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── - #> checking for file ‘/tmp/RtmpjAXJO4/remotes2649f3077b5d9/badpackage/DESCRIPTION’ ... ✔ - #> ─ i Preparing ‘badpackage’: - #> ─ checking DESCRIPTION meta-information ... ✔ - #> ─ checking vignette meta-information ... ✔ - #> ─ checking for LF line-endings in source and make files and shell scripts (362ms) + #> checking for file ‘/tmp/Rtmpoq5BBi/remotes9dcd65cdcc55/badpackage/DESCRIPTION’ ... ✔ checking for file ‘/tmp/Rtmpoq5BBi/remotes9dcd65cdcc55/badpackage/DESCRIPTION’ + #> ─ preparing ‘badpackage’: + #> checking DESCRIPTION meta-information ... ✔ checking DESCRIPTION meta-information + #> checking vignette meta-information ... ✔ checking vignette meta-information + #> ─ checking for LF line-endings in source and make files and shell scripts (400ms) #> ─ checking for empty or unneeded directories - #> ─ building ‘badpackage_1.0.0.tar.gz’ + #> ─ building ‘badpackage_1.0.0.tar.gz’ + #> + #> ``` r g ``` - #> ── GP badpackage ─────────────────────────────────────────────────────────────── + #> ── GP badpackage ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── #> #> It is good practice to #> - #> ✖ not use "Depends" in DESCRIPTION, as it can cause name clashes, and - #> poor interaction with other packages. Use "Imports" instead. - #> ✖ omit "Date" in DESCRIPTION. It is not required and it gets invalid - #> quite often. A build date will be added to the package when you + #> ✖ not use "Depends" in DESCRIPTION, as it can cause name clashes, and poor interaction with other packages. Use "Imports" instead. + #> ✖ omit "Date" in DESCRIPTION. It is not required and it gets invalid quite often. A build date will be added to the package when you #> perform `R CMD build` on it. - #> ✖ add a "URL" field to DESCRIPTION. It helps users find information - #> about your package online. If your package does not have a + #> ✖ add a "URL" field to DESCRIPTION. It helps users find information about your package online. If your package does not have a #> homepage, add an URL to GitHub, or the CRAN package package page. - #> ✖ add a "BugReports" field to DESCRIPTION, and point it to a bug - #> tracker. Many online code hosting services provide bug trackers for + #> ✖ add a "BugReports" field to DESCRIPTION, and point it to a bug tracker. Many online code hosting services provide bug trackers for #> free, https://github.com, https://gitlab.com, etc. - #> ✖ omit trailing semicolons from code lines. They are not needed and - #> most R coding standards forbid them + #> ✖ omit trailing semicolons from code lines. They are not needed and most R coding standards forbid them #> - #> R/semicolons.R:4:30 - #> R/semicolons.R:5:29 - #> R/semicolons.R:9:38 + #> 'R/semicolons.R:4:30' + #> 'R/semicolons.R:5:29' + #> 'R/semicolons.R:9:38' #> - #> ✖ not import packages as a whole, as this can cause name clashes - #> between the imported packages. Instead, import only the specific + #> ✖ not import packages as a whole, as this can cause name clashes between the imported packages. Instead, import only the specific #> functions you need. - #> ✖ fix this R CMD check ERROR: VignetteBuilder package not declared: - #> ‘knitr’ See section ‘The DESCRIPTION file’ in the ‘Writing R + #> ✖ fix this R CMD check ERROR: VignetteBuilder package not declared: ‘knitr’ See section ‘The DESCRIPTION file’ in the ‘Writing R #> Extensions’ manual. - #> ✖ avoid 'T' and 'F', as they are just variables which are set to the - #> logicals 'TRUE' and 'FALSE' by default, but are not reserved words - #> and hence can be overwritten by the user. Hence, one should always - #> use 'TRUE' and 'FALSE' for the logicals. + #> ✖ avoid 'T' and 'F', as they are just variables which are set to the logicals 'TRUE' and 'FALSE' by default, but are not reserved + #> words and hence can be overwritten by the user. Hence, one should always use 'TRUE' and 'FALSE' for the logicals. #> - #> R/tf.R:NA:NA - #> R/tf.R:NA:NA - #> R/tf.R:NA:NA - #> R/tf.R:NA:NA - #> R/tf.R:NA:NA + #> 'R/tf.R' + #> 'R/tf.R' + #> 'R/tf.R' + #> 'R/tf.R' + #> 'R/tf.R' #> ... and 4 more lines #> - #> ──────────────────────────────────────────────────────────────────────────────── + #> ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ``` r # show all available checks @@ -111,14 +103,13 @@ g_url <- gp(pkg_path, checks = "description_url") g_url ``` - #> ── GP badpackage ─────────────────────────────────────────────────────────────── + #> ── GP badpackage ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── #> #> It is good practice to #> - #> ✖ add a "URL" field to DESCRIPTION. It helps users find information - #> about your package online. If your package does not have a + #> ✖ add a "URL" field to DESCRIPTION. It helps users find information about your package online. If your package does not have a #> homepage, add an URL to GitHub, or the CRAN package package page. - #> ──────────────────────────────────────────────────────────────────────────────── + #> ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ``` r # which checks were carried out? @@ -136,7 +127,7 @@ failed_checks(g) #> [2] "no_description_date" #> [3] "description_url" #> [4] "description_bugreports" - #> [5] "lintr_trailing_semicolon_linter" + #> [5] "lintr_semicolon_linter" #> [6] "no_import_package_as_a_whole" #> [7] "rcmdcheck_package_dependencies_present" #> [8] "truefalse_not_tf" From ac1f1dbcee438d975789d7d2af7956e0aebae348 Mon Sep 17 00:00:00 2001 From: mpadge Date: Wed, 25 Sep 2024 13:46:01 +0200 Subject: [PATCH 3/3] update copyright notice on readme --- README.Rmd | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.Rmd b/README.Rmd index 153565d..5e84bf3 100644 --- a/README.Rmd +++ b/README.Rmd @@ -78,4 +78,4 @@ results(g)[1:5,] ## License -MIT © 2022 Ascent Digital Services UK Limited +MIT © 2024 rOpenSci diff --git a/README.md b/README.md index bf4724f..bec43d3 100644 --- a/README.md +++ b/README.md @@ -146,4 +146,4 @@ results(g)[1:5,] ## License -MIT © 2022 Ascent Digital Services UK Limited +MIT © 2024 rOpenSci