Skip to content
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

Linter for partially matched arguments #2562

Open
MichaelChirico opened this issue Apr 22, 2024 · 2 comments
Open

Linter for partially matched arguments #2562

MichaelChirico opened this issue Apr 22, 2024 · 2 comments

Comments

@MichaelChirico
Copy link
Collaborator

In topepo/caret#1361, I came across a lot of partially-matched argument calls, especially seq(along= and seq(length= (should be seq(along.with= and seq(length.out=), respectively).

It would be good to have a linter for this. This will require having function definitions loaded, in which case we can use match.call() with options(warnPartialMatchArgs=TRUE) set to check for issues:

c <- substitute(seq(along = x))
options(warnPartialMatchArgs = TRUE, warn=2L)
tryCatch(math.call(seq, c), error=\(c) "lint")
#  [1] "lint"

The key is resolving seq generically for any call name in the linted code.

@MichaelChirico
Copy link
Collaborator Author

Some exploratory code for potential implementation:

xml = source_expression$full_xml_parsed_content
all_call_expr = xml_find_all(xml, "//SYMBOL_FUNCTION_CALL/parent::expr/parent::expr")
# xml2lang() still not robust enough to generically convert <expr>...</expr> --> lang
all_call_lang = Filter(length, lapply(all_call_expr, \(x) tryCatch(xml2lang(x), error=\(c) NULL))
# drop primitives, e.g. match.call(c, quote(c(1))) fails; also no hope to resolve all calls generally,
#   e.g. nested / anonymous functions, so just skip those
all_calls=Filter(\(e) tryCatch(!is.primitive(eval(e[[1]])), error=\(c) FALSE), all_calls)
lapply(all_calls, \(e) match.call(eval(e[[1]]), e))

@MichaelChirico
Copy link
Collaborator Author

MichaelChirico commented Apr 23, 2024

codetools::checkUsage() can be used for partially matched arguments at least:

foo <- function() lm(Sepal.Width ~ Sepal.Length, d=iris)
codetools::checkUsage(foo, suppressPartialMatchArgs=FALSE)
# <anonymous>: warning in lm(Sepal.Width ~ Sepal.Length, d = iris): partial argument match of 'd' to 'data'

R CMD check already does so, so this won't help to find issues in CRAN packages, but can still help for real-time linting during development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant