Skip to content

Add extensions to R hooks #280

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

Merged
merged 16 commits into from
Jun 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions .github/workflows/end-to-end.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
on:
push:
branches:
- master
pull_request:
branches:
- master

name: end-2-end

jobs:
end-2-end:
runs-on: macOS-latest
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: r-lib/actions/setup-r@v1

- uses: actions/checkout@v2

- name: Query dependencies
run: |
install.packages('remotes')
saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2)
writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version")
shell: Rscript {0}

- name: Cache R packages
uses: actions/cache@v2
with:
path: ${{ env.R_LIBS_USER }}
key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }}
restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-

- name: Install dependencies
run: |
remotes::install_deps(dependencies = TRUE)
remotes::install_cran('yaml')
shell: Rscript {0}

- name: Install package
run: R CMD INSTALL .

- name: Prepare pre-commit
run: |
config_name <- ".pre-commit-config.yaml"
ref_config <- file.path("tests", "testthat", "reference-objects", config_name)
len_declared <- length(yaml::read_yaml(".pre-commit-hooks.yaml"))
len_testing <- length(yaml::read_yaml(ref_config)$repos[[1]]$hooks)
if (len_declared != len_testing) {
rlang::abort("You don't test all hooks. Add them to `test_path('reference-objects/.pre-commit-config.yaml')`")
}
fs::file_delete(config_name)
fs::file_copy(ref_config, config_name)
shell: Rscript {0}
- name: Update Hook revision to current push
run: |
# hacky, maybe can use pre-commit try-repo?
# https://stackoverflow.com/questions/30871868/sed-replace-first-occurence-in-place-with-big-files
sed -i '' -e "1,/rev:.*/{s/rev:.*/rev: $GITHUB_SHA/;}" .pre-commit-config.yaml

- name: Run pre-commit
run: |
echo 'one' >> README.Rmd
echo 'one' >> codemeta.json
echo 'one' >> README.md
echo "#' some code\n#'\n#' @param here.\n#' @name somethings\nNULL" > R/test.R # overwrite if anything there
brew install pre-commit
pre-commit install
pre-commit run --files R/test.R
pre-commit run --files DESCRIPTION
git add README*
pre-commit run --files README.Rmd
git reset HEAD --hard # restore initial state
env:
SKIP: consistent-release-tag
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ repos:
(.*/|)WORDLIST|
\.github/workflows/.*|
data/.*|
inst/bin/.*|
inst/hooks/.*|
inst/pre-commit-.*|
)$
- id: readme-rmd-rendered
Expand All @@ -56,17 +56,17 @@ repos:
hooks:
- id: consistent-release-tag
name: consistent-release-tag
entry: inst/consistent-release-tag
entry: inst/hooks/local/consistent-release-tag.R
language: script
stages: [commit, push]
- id: hooks-config-to-inst
name: hooks-config-to-inst
entry: inst/hooks-config-to-inst
entry: inst/hooks/local/hooks-config-to-inst.R
language: script
stages: [commit, push]
- id: spell-check-exclude-to-config
name: spell-check-exclude-to-config
entry: inst/spell-check-exclude-to-config
entry: inst/hooks/local/spell-check-exclude-to-config.R
language: script
stages: [commit, push]
- id: forbid-to-commit
Expand All @@ -78,6 +78,6 @@ repos:
- id: spell-check-ordered-exclude
name: Ordered regex pattern for spell-check exclusion
description: Ensure alphabetical order in `exclude:` key of spell check.
entry: inst/spell-check-ordered-exclude.R
entry: inst/hooks/local/spell-check-ordered-exclude.R
language: script
files: '^(.*/|)\.?pre-commit-config.*\.yaml$'
20 changes: 10 additions & 10 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -1,58 +1,58 @@
- id: roxygenize
name: roxygenize
description: run roxygen2::roxygenize()
entry: inst/bin/roxygenize
entry: inst/hooks/exported/roxygenize.R
language: script
files: '^(R|man)/'
require_serial: true
minimum_pre_commit_version: "2.13.0"
- id: use-tidy-description
name: use-tidy-description
description: run usethis::use_tidy_description()
entry: inst/bin/use-tidy-description
entry: inst/hooks/exported/use-tidy-description.R
language: script
files: '^DESCRIPTION$'
minimum_pre_commit_version: "2.13.0"
- id: style-files
name: style-files
description: style files with styler
entry: inst/bin/style-files
entry: inst/hooks/exported/style-files.R
language: script
files: '(\.R|\.Rmd|\.Rnw|\.r|\.rmd|\.rnw)$'
exclude: 'renv/activate\.R'
minimum_pre_commit_version: "2.13.0"
- id: no-browser-statement
name: no-browser-statement
description: check if a .R file contains a `browser()` statement
entry: inst/bin/no-browser-statement
entry: inst/hooks/exported/no-browser-statement.R
language: script
files: '\.[rR]$'
minimum_pre_commit_version: "2.13.0"
- id: parsable-R
name: parsable-R
description: check if a .R file is parsable
entry: inst/bin/parsable-R
entry: inst/hooks/exported/parsable-R.R
language: script
files: '\.[rR]$'
minimum_pre_commit_version: "2.13.0"
- id: readme-rmd-rendered
name: readme-rmd-rendered
description: make sure README.Rmd hasn't been edited more recently than README.md
entry: inst/bin/readme-rmd-rendered
entry: inst/hooks/exported/readme-rmd-rendered
language: script
files: 'README\.[Rr]?md$'
minimum_pre_commit_version: "2.13.0"
- id: codemeta-description-updated
name: codemeta-description-updated
description: make sure codemeta.json is in sync with DESCRIPTION. It should be run after use-tidy-description.
entry: inst/bin/codemeta-description-updated
entry: inst/hooks/exported/codemeta-description-updated
language: script
files: '^DESCRIPTION$'
minimum_pre_commit_version: "2.13.0"
- id: spell-check
name: spell-check
description: perform a spell check with spelling::spell_check_files()
entry: inst/bin/spell-check
entry: inst/hooks/exported/spell-check.R
language: script
exclude: >
(?x)^(
Expand Down Expand Up @@ -86,15 +86,15 @@
- id: deps-in-desc
name: deps-in-desc
description: Check if dependencies that can be parsed from code are in DESCRIPTION.
entry: inst/bin/deps-in-desc
entry: inst/hooks/exported/deps-in-desc.R
language: script
files: '(\.[rR]profile|\.R|\.Rmd|\.Rnw|\.r|\.rmd|\.rnw)$'
exclude: 'renv/activate\.R'
minimum_pre_commit_version: "2.13.0"
- id: lintr
name: lintr
description: check if a .R file is lint free (using lintr)
entry: inst/bin/lintr
entry: inst/hooks/exported/lintr.R
language: script
files: '\.[rR]$'
exclude: 'renv/activate\.R'
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ This repo uses the [tic](https://github.com/ropenscilabs/tic) package for CI.

To create a new hook, have a look at the [official
documentation](https://pre-commit.com/#new-hooks) on creating new hooks, then have a look
at existing hooks in this repo. The actual executables are defined in [`inst/bin/`](https://github.com/lorenzwalthert/precommit/tree/master/inst/bin). In
at existing hooks in this repo. The actual executables are defined in [`inst/hooks/`](https://github.com/lorenzwalthert/precommit/tree/master/inst/hooks). In
the script, you can expect the passed command line arguments to be all options,
finally the files that should be processed with the hook.

Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ depending on whether or not you previously used pre-commit.
- The `deps-in-desc` hook now points to the hook argument
`--allow_private_imports` when the hook fails due to private imports (#254).
- roxygenize hook is now fully tested (#267).
- Hook scripts were relocated and R hooks now have a file extension (#280).

# precommit v0.1.3

Expand Down
12 changes: 8 additions & 4 deletions R/testing.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
#' the message we expect. To check changed file content, we set `error_msg` to
#' `NA`.
#'
#' @param hook_name The name of the hook in `bin/`.
#' @param hook_name The name of the hook in `inst/hooks/exported/`, without
#' file extension.
#' @param file_name The file to test in `tests/in` (without extension). Can be
#' a named vector of length one where the name is the target location relative
#' to the temporary location and the value is the source of the file.
Expand All @@ -41,10 +42,13 @@ run_test <- function(hook_name,
artifacts = NULL,
file_transformer = function(files) files,
env = character()) {
path_executable <- system.file(
fs::path("bin", hook_name),
path_executable <- fs::dir_ls(system.file(
fs::path("hooks", "exported"),
package = "precommit"
)
), regexp = paste0("/", hook_name))
if (length(path_executable) != 1) {
rlang::abort("Failed to derive hook path")
}
path_candidate <- paste0(testthat::test_path("in", file_name), suffix) %>%
ensure_named(names(file_name), fs::path_file)
run_test_impl(
Expand Down
1 change: 1 addition & 0 deletions inst/WORDLIST
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ RStudio
rstudioapi
saveCache
seealso
SHA
stderr
stdout
sterr
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash

# adapted from https://github.com/lorenzwalthert/precommit/blob/f4413cfe6282c84f7176160d06e1560860c8bd3d/inst/bin/readme-rmd-rendered
# adapted from https://github.com/lorenzwalthert/precommit/blob/f4413cfe6282c84f7176160d06e1560860c8bd3d/inst/hooks/exported/readme-rmd-rendered

if [[ DESCRIPTION -nt codemeta.json ]]
then
Expand Down
12 changes: 6 additions & 6 deletions inst/bin/deps-in-desc → inst/hooks/exported/deps-in-desc.R
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#!/usr/bin/env Rscript

'Ensure all dependencies of the form pkg::fun are in DESCRIPTION
"Ensure all dependencies of the form pkg::fun are in DESCRIPTION
Usage:
deps-in-desc [--allow_private_imports] <files>...

Options:
--allow_private_imports Whether or not to allow the use of ::: on imported functions.
' -> doc
" -> doc
pre_installed <- c(
"base", "boot", "class", "cluster", "codetools", "compiler",
"datasets", "foreign", "graphics", "grDevices", "grid", "KernSmooth",
Expand All @@ -32,8 +32,8 @@ deps_in_desc <- function(file, arguments) {
cat(
"Not all packages used in your code are listed in DESCRIPTION. ",
"The following are missing in file `", file, "`: ",
paste(unregistered, collapse = ", "), ".\n",
"You can add them with `usethis::use_package()` or ",
paste(unregistered, collapse = ", "), ".\n",
"You can add them with `usethis::use_package()` or ",
"`usethis::use_dev_package()`.\n",
sep = ""
)
Expand All @@ -53,10 +53,10 @@ deps_in_desc <- function(file, arguments) {
paste0(file, collapse = ", "), "\n\n",
"If you still think `:::` is a good idea, you can ",
"allow private imports in your `.pre-commit-config.yaml` like this:

- id: deps-in-desc
args: [--allow_private_imports]

",
sep = ""
)
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

files <- commandArgs(trailing = TRUE)
no_browser_statement <- function(path) {
pd <- getParseData(parse(path, keep.source = TRUE))
pd <- getParseData(parse(path, keep.source = TRUE))
if (any(pd$text[pd$token == "SYMBOL_FUNCTION_CALL"] == "browser")) {
stop("File `", path, "` contains a `browser()` statement.", call. = FALSE)
}
}
}

for (file in files) {
Expand Down
2 changes: 1 addition & 1 deletion inst/bin/parsable-R → inst/hooks/exported/parsable-R.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ files <- commandArgs(trailing = TRUE)

out <- lapply(files, function(path) {
tryCatch(
parse(path),
parse(path),
error = function(x) stop("File ", path, " is not parsable", call. = FALSE)
)
})
File renamed without changes.
File renamed without changes.
18 changes: 9 additions & 9 deletions inst/bin/spell-check → inst/hooks/exported/spell-check.R
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#!/usr/bin/env Rscript

'Spell check for files
"Spell check for files
Usage:
spell-check [--lang=<language>] <files>...

Options:
--lang=<language> Passed to `spelling::spell_check_files()` [default: en_US]

' -> doc
" -> doc

arguments <- docopt::docopt(doc)
path_wordlist <- file.path("inst", "WORDLIST")
Expand All @@ -26,23 +26,23 @@ if (file.exists(path_wordlist)) {


spelling_errors <- spelling::spell_check_files(
files,
ignore = ignore,
files,
ignore = ignore,
lang = arguments$lang
)

if (nrow(spelling_errors) > 0) {
cat("The following spelling errors were found:\n" )
cat("The following spelling errors were found:\n")
print(spelling_errors)
ignore <- sort(unique(c(ignore, spelling_errors$word)))
ignore <- ignore[ignore != ""] # drop blanks if any
writeLines(ignore, path_wordlist)
cat(
"All spelling errors found were copied to inst/WORDLIST assuming they were",
"not spelling errors and will be ignored in the future. Please ",
"review the above list and for each word that is an actual typo:\n",
"All spelling errors found were copied to inst/WORDLIST assuming they were",
"not spelling errors and will be ignored in the future. Please ",
"review the above list and for each word that is an actual typo:\n",
"- fix it in the source code.\n",
"- remove it again manually from inst/WORDLIST to make sure it's not\n",
"- remove it again manually from inst/WORDLIST to make sure it's not\n",
" ignored in the future.\n",
"Then, try committing again.\n"
)
Expand Down
2 changes: 1 addition & 1 deletion inst/bin/style-files → inst/hooks/exported/style-files.R
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ tryCatch(
},
warning = function(w) {
msg <- conditionMessage(w)
if (grepl('Unknown or uninitialised column', msg, ignore.case = TRUE)) {
if (grepl("Unknown or uninitialised column", msg, ignore.case = TRUE)) {
warning(msg, call. = FALSE)
} else {
stop(msg, call. = FALSE)
Expand Down
Loading