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

False positives with nonportable_path_linter for text that contains "/"? #468

Open
dschlaep opened this issue Mar 16, 2020 · 2 comments
Open
Labels
false-positive code that shouldn't lint, but does

Comments

@dschlaep
Copy link

dschlaep commented Mar 16, 2020

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)")
                ^~~~~~~~~~~~~
@russHyde russHyde added the bug an unexpected problem or unintended behavior label Mar 30, 2020
dschlaep added a commit to DrylandEcology/rSOILWAT2 that referenced this issue Apr 21, 2020
dschlaep added a commit to DrylandEcology/rSFSW2 that referenced this issue Apr 24, 2020
@AshesITR AshesITR added the false-positive code that shouldn't lint, but does label Jun 19, 2022
@MichaelChirico
Copy link
Collaborator

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.

@ssh352
Copy link

ssh352 commented Jan 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive code that shouldn't lint, but does
Projects
None yet
Development

No branches or pull requests

5 participants