Skip to content

Commit

Permalink
Use linting to detect undesired function usage (#2541)
Browse files Browse the repository at this point in the history
Co-authored-by: Maëlle Salmon <[email protected]>
  • Loading branch information
hadley and maelle authored May 16, 2024
1 parent f42932a commit 350854a
Show file tree
Hide file tree
Showing 24 changed files with 350 additions and 44 deletions.
1 change: 1 addition & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@
^fake-index.html$
^CRAN-SUBMISSION$
^tools$
^\.lintr.R$
34 changes: 34 additions & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
on:
push:
branches: [main, master]
pull_request:
branches: [main, master]

name: lint

permissions: read-all

jobs:
lint:
runs-on: ubuntu-latest
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v4

- uses: r-lib/actions/setup-r@v2
with:
use-public-rspm: true

- uses: r-lib/actions/setup-r-dependencies@v2
with:
extra-packages: any::lintr, local::.
needs: lint

- name: Lint
run: lintr::lint_package()
shell: Rscript {0}
env:
LINTR_ERROR_ON_LINT: true
40 changes: 40 additions & 0 deletions .lintr.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
linters <- list(lintr::undesirable_function_linter(
fun = c(
# Base messaging
"message" = "use cli::cli_inform()",
"warning" = "use cli::cli_warn()",
"stop" = "use cli::cli_abort()",
# rlang messaging
"inform" = "use cli::cli_inform()",
"warn" = "use cli::cli_warn()",
"abort" = "use cli::cli_abort()",
# older cli
"cli_alert_danger" = "use cli::cli_inform()",
"cli_alert_info" = "use cli::cli_inform()",
"cli_alert_success" = "use cli::cli_inform()",
"cli_alert_warning" = "use cli::cli_inform()",
# fs
"file.path" = "use path()",
"dir" = "use dir_ls()",
"dir.create" = "use dir_create()",
"file.copy" = "use file_copy()",
"file.create" = "use file_create()",
"file.exists" = "use file_exists()",
"file.info" = "use file_info()",
"normalizePath" = "use path_real()",
"unlink" = "use file_delete()",
"basename" = "use path_file()",
"dirname" = "use path_dir()",
# i/o
"readLines" = "use read_lines()",
"writeLines" = "use write_lines()"
),
symbol_is_undesirable = FALSE
))

exclusions <- list(
"R/import-standalone-obj-type.R",
"R/import-standalone-types-check.R",
"vignettes",
"tests/testthat/assets"
)
5 changes: 2 additions & 3 deletions R/build-favicons.R
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ build_favicons <- function(pkg = ".", overwrite = FALSE) {
cli::cli_abort("API request failed.", .internal = TRUE)
}

tmp <- tempfile()
on.exit(unlink(tmp))
tmp <- withr::local_tempdir()
result <- httr::RETRY(
"GET",
result$favicon$package_url,
Expand Down Expand Up @@ -120,5 +119,5 @@ copy_favicons <- function(pkg = ".") {
has_favicons <- function(pkg = ".") {
pkg <- as_pkgdown(pkg)

file.exists(path(pkg$src_path, "pkgdown", "favicon"))
unname(file_exists(path(pkg$src_path, "pkgdown", "favicon")))
}
2 changes: 1 addition & 1 deletion R/build-home-index.R
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ data_home_sidebar <- function(pkg = ".", call = caller_env()) {
html_path <- path(pkg$src_path, pkg$meta$home$sidebar$html)

if (length(html_path)) {
if (!file.exists(html_path)) {
if (!file_exists(html_path)) {
rel_html_path <- path_rel(html_path, pkg$src_path)
config_abort(
pkg,
Expand Down
2 changes: 1 addition & 1 deletion R/build-home-md.R
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ render_md <- function(pkg, filename) {
cli::cli_inform("Reading {src_path(path_rel(filename, pkg$src_path))}")

body <- markdown_body(filename, strip_header = TRUE)
path <- path_ext_set(basename(filename), "html")
path <- path_ext_set(path_file(filename), "html")

render_page(pkg, "title-body",
data = list(
Expand Down
7 changes: 3 additions & 4 deletions R/deploy-site.R
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ deploy_site_github <- function(
...
)

cli::cli_alert_success("Deploy completed")
cli::cli_inform(c(v = "Deploy completed"))
}

#' Build and deploy a site locally
Expand Down Expand Up @@ -199,10 +199,9 @@ git <- function(..., echo_cmd = TRUE, echo = TRUE, error_on_status = TRUE) {
callr::run("git", c(...), echo_cmd = echo_cmd, echo = echo, error_on_status = error_on_status)
}

construct_commit_message <- function(pkg, commit = ci_commit_sha()) {
construct_commit_message <- function(pkg = ".", commit = ci_commit_sha()) {
pkg <- as_pkgdown(pkg)
commit <- sprintf("%s@%s", pkg$version, substr(commit, 1, 7))
cli::cli_alert_success("Built site for {cli::col_yellow(pkg$package)}: {.emph {cli::col_green(commit)}}")
cli::format_inline("Built site for {pkg$package}@{pkg$version}: {substr(commit, 1, 7)}")
}

ci_commit_sha <- function() {
Expand Down
2 changes: 1 addition & 1 deletion R/package.R
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ package_vignettes <- function(path = ".") {
description = desc,
depth = dir_depth(file_out)
)
out[order(basename(out$file_out)), ]
out[order(path_file(out$file_out)), ]
}

find_template_config <- function(package,
Expand Down
3 changes: 0 additions & 3 deletions R/pkgdown.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ local_pkgdown_site <- function(path = NULL, meta = NULL, clone = FALSE, env = pa
}
pkg <- as_pkgdown(path, meta)
pkg$dst_path <- withr::local_tempdir(.local_envir = env)

withr::defer(unlink(pkg$dst_path, recursive = TRUE), envir = env)

pkg
}

Expand Down
2 changes: 1 addition & 1 deletion R/preview.R
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ preview_site <- function(pkg = ".", path = ".", preview = NA) {
}

if (preview) {
cli::cli_alert_info("Previewing site")
cli::cli_inform(c(i = "Previewing site"))
utils::browseURL(path(pkg$dst_path, path, "index.html"))
}

Expand Down
8 changes: 4 additions & 4 deletions R/rmarkdown.R
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ render_rmarkdown <- function(pkg, input, output, ..., seed = NULL, copy_images =

# temporarily copy the rendered html into the input path directory and scan
# again for additional external resources that may be been included by R code
tempfile_in_input_dir <- file_temp(ext = "html", tmp_dir = path_dir(input_path))
file_copy(path, tempfile_in_input_dir)
withr::defer(unlink(tempfile_in_input_dir))
ext_post <- rmarkdown::find_external_resources(tempfile_in_input_dir)
tempfile <- path(path_dir(input_path), "--find-assets.html")
withr::defer(try(file_delete(tempfile)))
file_copy(path, tempfile)
ext_post <- rmarkdown::find_external_resources(tempfile)

ext <- rbind(ext_src, ext_post)
ext <- ext[!duplicated(ext$path), ]
Expand Down
2 changes: 1 addition & 1 deletion R/theme.R
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ build_bslib <- function(pkg = ".") {
}

data_deps <- function(pkg, depth) {
if (!file.exists(data_deps_path(pkg))) {
if (!file_exists(data_deps_path(pkg))) {
cli::cli_abort(
"Run {.fn pkgdown::init_site} first.",
call = caller_env()
Expand Down
4 changes: 1 addition & 3 deletions R/utils-fs.R
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,11 @@ out_of_date <- function(source, target) {
)
}

file.info(source)$mtime > file.info(target)$mtime
file_info(source)$mtime > file_info(target)$mtime
}

# Path helpers ------------------------------------------------------------

file.path <- function(...) stop("Use path!")

path_abs <- function(path, start = ".") {
is_abs <- is_absolute_path(path)

Expand Down
7 changes: 2 additions & 5 deletions R/utils-io.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,12 @@ read_file <- function(path) {

# Inspired by roxygen2 utils-io.R (https://github.com/klutometis/roxygen/) -----------

readLines <- function(...) stop("Use read_lines!")
writeLines <- function(...) stop("Use write_lines!")

read_lines <- function(path, n = -1L) {
base::readLines(path, n = n, encoding = "UTF-8", warn = FALSE)
base::readLines(path, n = n, encoding = "UTF-8", warn = FALSE) # nolint
}

write_lines <- function(text, path) {
base::writeLines(enc2utf8(text), path, useBytes = TRUE)
base::writeLines(enc2utf8(text), path, useBytes = TRUE) # nolint
}

# Other -------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 350854a

Please sign in to comment.