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 positive/negative lints with "natural" comments #2827

Open
MichaelChirico opened this issue Mar 10, 2025 · 0 comments
Open

False positive/negative lints with "natural" comments #2827

MichaelChirico opened this issue Mar 10, 2025 · 0 comments
Labels
false-negative code that should lint, but doesn't false-positive code that shouldn't lint, but does

Comments

@MichaelChirico
Copy link
Collaborator

As fixed in #2822, here are some linters that generate incorrect linting due to comments in a "natural" position (as opposed to some cases which generate incorrect lints only when comments are placed in ways not likely to be observed in practice):

# When expect_no_lint() is used, it's a false negative, otherwise, a false positive

expect_no_lint(
  trim_some("
    stopifnot('x must be a logical scalar' = # comment
      length(x) == 1 && is.logical(x) && !is.na(x)
    )
  "),
  conjunct_test_linter()
)
expect_lint("x <- { # comment\n}", ".", empty_assignment_linter())
expect_lint(
  trim_some("
    x %>%   grepl(pattern = # comment
    'a')
  "),
  "This regular expression is static",
  fixed_regex_linter()
)

# (ditto object_length_linter)
expect_lint(
  trim_some("
    assign(envir = # comment
    'good_env_name', 'badName', 2)
  "),
  ".",
  object_name_linter()
)

expect_lint(
  trim_some("
    any(!x, na.rm = # comment
    TRUE)
  "),
  ".",
  outer_negation_linter()
)

expect_no_lint(
  trim_some("
    x |> # comment
    sprintf(fmt = '%s')
  "),
  sprintf_linter()
)

expect_no_lint(
  trim_some('
    "%s" %>% # comment
    sprintf("%s%s")
  '),
  sprintf_linter()
)

expect_no_lint(
  trim_some('
    "a" %T>% # comment
      c("b")
  '),
  unnecessary_concatenation_linter()
)

expect_no_lint(
  trim_some("
    lapply(x, function(xi) data.frame(nm = # comment
    xi))
  "),
  unnecessary_lambda_linter()
)

expect_no_lint(
  trim_some("
    map_dbl(x, ~foo(bar = # comment
    .x))
  "),
  unnecessary_lambda_linter()
)

expect_lint(
  trim_some("
    lapply(y, function(yi) {
      print(yi) # comment
    })
  "),
  ".",
  unnecessary_lambda_linter()
)

expect_lint(
  trim_some("
    if (x && a) {
      # comment1
      if (y || b) {
        1L
      }
      # comment2
    }
  "),
  "Combine this `if` statement with the one found at line 1",
  unnecessary_nesting_linter()
)

expect_lint(
  trim_some("
    foo <- function(bar) {
      if (bar) {
        return(bar); # comment
        x <- 2
      } else {
        return(bar); # comment
        x <- 3
      }
      while (bar) {
        return(bar); # comment
        5 + 3
      }
      repeat {
        return(bar); # comment
        test()
      }
      for(i in 1:3) {
        return(bar); # comment
        5 + 4
      }
    }
  "),
  list(
    list(line_number = 4L, message = "."),
    list(line_number = 7L, message = "."),
    list(line_number = 11L, message = "."),
    list(line_number = 15L, message = "."),
    list(line_number = 19L, message = ".")
  ),
  unreachable_code_linter()
)

expect_lint(
  trim_some("
    foo <- function(bar) {
      if (bar) {
        next; # comment
        x <- 2
      } else {
        break; # comment
        x <- 3
      }
      while (bar) {
        break; # comment
        5 + 3
      }
      repeat {
        next; # comment
        test()
      }
      for(i in 1:3) {
        break; # comment
        5 + 4
      }
    }
  "),
  list(
    list(line_number = 4L, message = "."),
    list(line_number = 7L, message = "."),
    list(line_number = 11L, message = "."),
    list(line_number = 15L, message = "."),
    list(line_number = 19L, message = ".")
  ),
  unreachable_code_linter()
)
@MichaelChirico MichaelChirico added false-negative code that should lint, but doesn't false-positive code that shouldn't lint, but does labels Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-negative code that should lint, but doesn't false-positive code that shouldn't lint, but does
Projects
None yet
Development

No branches or pull requests

1 participant