Skip to content

Commit

Permalink
New routine_registration_linter (#1669)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Oct 11, 2022
1 parent 444a2fb commit cee077a
Show file tree
Hide file tree
Showing 11 changed files with 127 additions and 3 deletions.
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ Collate:
'redundant_equals_linter.R'
'redundant_ifelse_linter.R'
'regex_subset_linter.R'
'routine_registration_linter.R'
'semicolon_linter.R'
'seq_linter.R'
'settings.R'
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export(pipe_continuation_linter)
export(redundant_equals_linter)
export(redundant_ifelse_linter)
export(regex_subset_linter)
export(routine_registration_linter)
export(sarif_output)
export(semicolon_linter)
export(semicolon_terminator_linter)
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@

* `unnecessary_placeholder_linter()` for identifying where usage of the {magrittr} placeholder `.` could be omitted (@MichaelChirico)

* `routine_registration_linter()` for identifying native routines that don't use registration (`useDynLib` in the `NAMESPACE`; @MichaelChirico)

## Notes

* `lint()` continues to support Rmarkdown documents. For users of custom .Rmd engines, e.g.
Expand Down
58 changes: 58 additions & 0 deletions R/routine_registration_linter.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
#' Identify unregistered native routines
#'
#' It is preferable to register routines for efficiency and safety.
#'
#' @examples
#' # will produce lints
#' lint(
#' text = '.Call("cpp_routine", PACKAGE = "mypkg")',
#' linters = routine_registration_linter()
#' )
#'
#' lint(
#' text = '.Fortran("f_routine", PACKAGE = "mypkg")',
#' linters = routine_registration_linter()
#' )
#'
#' # okay
#' lint(
#' text = ".Call(cpp_routine)",
#' linters = routine_registration_linter()
#' )
#'
#' lint(
#' text = ".Fortran(f_routine)",
#' linters = routine_registration_linter()
#' )
#'
#' @evalRd rd_tags("routine_registration_linter")
#' @seealso [linters] for a complete list of linters available in lintr. \cr
#' https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Registering-native-routines
#'
#' @export
routine_registration_linter <- function() {
native_routine_callers <- c(".C", ".Call", ".Fortran", ".External")
xpath <- glue::glue("
//SYMBOL_FUNCTION_CALL[ {xp_text_in_table(native_routine_callers)} ]
/parent::expr
/following-sibling::expr[1]/STR_CONST
/parent::expr
")

Linter(function(source_expression) {
if (!is_lint_level(source_expression, "expression")) {
return(list())
}

xml <- source_expression$xml_parsed_content

bad_expr <- xml2::xml_find_all(xml, xpath)

xml_nodes_to_lints(
bad_expr,
source_expression = source_expression,
lint_message = "Register your native code routines with useDynLib and R_registerRoutines().",
type = "warning"
)
})
}
1 change: 1 addition & 0 deletions inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pipe_continuation_linter,style readability default
redundant_equals_linter,best_practices readability efficiency common_mistakes
redundant_ifelse_linter,best_practices efficiency consistency
regex_subset_linter,best_practices efficiency
routine_registration_linter,best_practices efficiency robustness
semicolon_linter,style readability default configurable
semicolon_terminator_linter,style readability deprecated configurable
seq_linter,robustness efficiency consistency best_practices default
Expand Down
1 change: 1 addition & 0 deletions man/best_practices_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/efficiency_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions man/linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/robustness_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 42 additions & 0 deletions man/routine_registration_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions tests/testthat/test-routine_registration_linter.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
skip_if_not_installed("patrick")
patrick::with_parameters_test_that(
"lints correctly",
{
linter <- routine_registration_linter()
expect_lint(sprintf("%s(ROUTINE, 1)", caller), NULL, linter)
expect_lint(
sprintf("%s('ROUTINE', PACKAGE = 'foo')", caller),
"Register your native code routines with useDynLib",
linter
)
},
.test_name = c(".C", ".Call", ".External", ".Fortran"),
caller = c(".C", ".Call", ".External", ".Fortran")
)

0 comments on commit cee077a

Please sign in to comment.