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

[LINT_BUG]: box_default_linters$box_usage_linter errors when it shouldn't #146

Open
SaintRod opened this issue Sep 21, 2024 · 3 comments
Open

Comments

@SaintRod
Copy link

box.linters version

0.10.5

Sample source code to lint

box::use(
    here[here]
)

# fails
for (file in here()) {
  assign(
    x = paste0("path_", tolower(basename(file))),
    value = file,
    envir = .GlobalEnv
  )
  rm(file)
}

# succeeds
tmpFunc = function(name) paste0("path_", tolower(basename(name)))
for (file in here()) {
  assign(
    x = tmpFunc(file),
    value = file,
    envir = .GlobalEnv
  )
  rm(file)
}

Lint command used

box::use(
  box.linters[box_default_linters],
  here[here],
  lintr[lint],
)

lint(
  filename = here("test.r"),
  linters = box_default_linters$box_usage_linter
)

Lint result

Error: Linter 'box_usage_linter' failed in ./test.r: <text>:4:1: unexpected ','
3: "path_"
4: ,
   ^

Expected result

No error expected

@SaintRod
Copy link
Author

PS. I'm not using box.linters with rhino

@radbasa
Copy link
Collaborator

radbasa commented Sep 25, 2024

assign(
  x = paste0("path_", tolower(basename(file))),
  value = file,
  envir = .GlobalEnv
)

This fails linting because the linter expects the value of x to be a character string. Because linting does static code analysis, it cannot and does not process or execute the paste() command to build the character string.

The other way you presented won't trigger a linting error, but it may cause linting errors further down your code. box_usage_linter() may complain about:

x <- path_some_file

As path_some_file was not defined previously. A different way to accomplish this might be to do the following:

file_paths <- list()
assign(
  x = file_paths[[tolower(basename(file))]],
  value = file,
  envir = .GlobalEnv
)

# This should be ok
foo <- file_paths$some_filename

@SaintRod
Copy link
Author

SaintRod commented Sep 25, 2024

Thanks @radbasa

This fails linting because the linter expects the value of x to be a character string. Because linting does static code analysis, it cannot and does not process or execute the paste() command to build the character string.

Is it possible for the linter to know that the output of paste() and paste0() is of class character? This way no code needs to be executed and the type requirement for the x arg in assign() is satisfied.

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

No branches or pull requests

2 participants