Skip to content

Instantly share code, notes, and snippets.

@MichaelChirico
Created November 10, 2023 18:18
Show Gist options
  • Save MichaelChirico/084bc163c01ad0debba8b1b945b75009 to your computer and use it in GitHub Desktop.
Save MichaelChirico/084bc163c01ad0debba8b1b945b75009 to your computer and use it in GitHub Desktop.
explicit_return_linter XPath
allowed_functions <- c(
# Normal calls
"return", "stop", "warning", "message", "stopifnot", "q", "quit",
"invokeRestart", "tryInvokeRestart",
# Normal calls from non-default libraries
"LOG", "abort",
# tests in the RUnit framework are functions ending with a call to one
# of the below. would rather users just use a different framework
# (e.g. testthat or tinytest), but already 250+ BUILD files depend
# on RUnit, so just cater to that. confirmed the efficiency impact
# of including these is minimal.
# RUnit tests look like 'TestInCamelCase <- function()'
# NB: check for starts-with(text(), 'Test') below is not sufficient, e.g.
# in cases of a "driver" test function taking arguments and the main unit
# test iterating over those.
"checkEquals", "checkEqualsNumeric", "checkException", "checkIdentical",
"checkStop", "checkTrue", "checkWarnings",
# Functions related to S3 methods
"UseMethod", "NextMethod",
# Functions related to S4 methods
"standardGeneric", "callNextMethod",
# Functions related to C interfaces
".C", ".Call", ".External", ".Fortran"
)
preceding_control_calls <-
.Paste0or("preceding-sibling::", c("IF", "FOR", "WHILE", "REPEAT"))
# from top, look for a FUNCTION definition that uses { (one-line
# function definitions are excepted), then look for failure to find
# return() on the last() expr of the function definition.
# exempt .onLoad which shows up in the tree like
# <expr><expr><SYMBOL>.onLoad</></><LEFT_ASSIGN></><expr><FUNCTION>...
# simple final expression (no control flow) must be
# <expr><expr> CALL( <expr> ) </expr></expr>
# NB: if this syntax _isn't_ used, the node may not be <expr>, hence
# the use of /*[...] below and self::expr here. position() = 1 is
# needed to guard against a few other cases.
# We also need to make sure that this expression isn't followed by a pipe
# symbol, which would indicate that we need to also check the last
# expression.
# pipe expressions are like
# ...
# <SPECIAL>%&gt;%</SPECIAL>
# <expr><expr><SYMBOL_FUNCTION_CALL>return</SYMBOL_FUNCTION_CALL>
# </expr></expr>
# Unlike the following case, the return should be the last expression in
# the sequence.
# conditional expressions are like
# <expr><IF> ( <expr> ) <expr> [ <ELSE> <expr>] </expr>
# we require _any_ call to return() in either of the latter two <expr>, i.e.,
# we don't apply recursive logic to check every branch, only that the
# two top level branches have at least two return()s
# because of special 'in' syntax for 'for' loops, the condition is
# tagged differently than for 'if'/'while' conditions (simple PAREN)
xpath <- glue("
//FUNCTION[parent::expr[not(
preceding-sibling::expr[SYMBOL[{ .XpTextInTable(.kReturnNotNeededFuns) }]]
or (
preceding-sibling::expr/SYMBOL[starts-with(text(), 'Test')]
and not(SYMBOL_FORMALS)
)
)]]
/following-sibling::expr[OP-LEFT-BRACE and expr[last()]/@line1 != @line1]
/expr[last()]
/*[
(
({ preceding_control_calls })
and (preceding-sibling::OP-RIGHT-PAREN or preceding-sibling::forcond)
and self::expr[not(
.//SYMBOL_FUNCTION_CALL[{ .XpTextInTable(allowed_functions) }]
)]
) or (
not({ preceding_control_calls })
and not(self::IF or self::FOR or self::WHILE or self::REPEAT)
and (
(
position() = last()
and (
preceding-sibling::PIPE
or preceding-sibling::SPECIAL[text() = '%>%']
)
and not(self::expr/expr/SYMBOL_FUNCTION_CALL[
{ .XpTextInTable(allowed_functions) }
])
) or (
position() = 1
and not(
following-sibling::PIPE
or following-sibling::SPECIAL[text() = '%>%']
)
and not(self::expr/SYMBOL_FUNCTION_CALL[
{ .XpTextInTable(allowed_functions) }
])
)
)
)
]
")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment