lintr icon indicating copy to clipboard operation
lintr copied to clipboard

False positives with `nonportable_path_linter` for text that contains "/"?

Open dschlaep opened this issue 4 years ago • 2 comments

It seems that most/all text that uses the character "/" is considered a file path by nonportable_path_linter.

In cases when "/" is used for reasons other than as a file path separator and, consequently, "file.path" is not used, nonportable_path_linter marks the code as in need of linting. Is the intent of this linter that all such uses be removed or is the linter currently too greedy in determining what constitutes a file path? Thanks!

Example:

library("lintr")

# `nonportable_path_linter` was introduced in v2.0.0
stopifnot(packageVersion("lintr") >= "2.0.0")


# Example file
lint_tmpfile <- "lintr_testfile.R"
writeLines(
  c(
    'message("This is a message and/or a test.")',
    'format(as.Date(Sys.time(), format = "%Y/%j"))',
    'plot(1, ylab = "Rate (1/hour)")'
  ),
  con = lint_tmpfile
)


lint(
  lint_tmpfile,
  linters = with_defaults(nonportable_path_linter())
)

unlink(lint_tmpfile)

Output of example code:

lintr_testfile.R:1:10: warning: Use file.path() to construct portable file paths.
message("This is a message and/or a test.")
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lintr_testfile.R:2:38: warning: Use file.path() to construct portable file paths.
format(as.Date(Sys.time(), format = "%Y/%j"))
                                     ^~~~~
lintr_testfile.R:3:17: warning: Use file.path() to construct portable file paths.
plot(1, ylab = "Rate (1/hour)")
                ^~~~~~~~~~~~~

dschlaep avatar Mar 16 '20 13:03 dschlaep

I think a way out here is to add a parameter like alphanumeric_paths (name can be improved) that sees % and assume "not a file path".

Whether to allow spaces / parentheses is more of an edge case, it could be a second parameter? Otherwise it's hard to both (1) allow spaces/parentheses in paths and (2) skip edge cases like Rate (1/hour). Another idea is to skip linting if spaces are on either side of /, then Rate (1 / hour) wouldn't lint.

MichaelChirico avatar Oct 07 '22 17:10 MichaelChirico

it is pretty common to have / in quoted string such as time zones "America/New_York".

ssh352 avatar Jan 19 '24 08:01 ssh352