New object_overwrite_linter

[MichaelChirico]

Part of #884

Here's another one we didn't end up using internally, so it hasn't gotten all too much scrutiny -- it might benefit from some rethinking (e.g. is exclusion of top-level assignments and function formals really necessary? Or even worth putting behind an argument?)

Originally, we also used a fixed set of functions to check for duplicates based on the observed frequency of overwrites in our code base. Presenting that list for posterity, but I'm not sure there's a "principled" way to include it in the general package:

kCommonBaseOverwrites <- unname(unlist(list(
  base = c(
    "all", "args", "beta", "c", "col", "date", "diff",
    "dir", "file", "labels", "mean", "message", "names",
    "q", "row", "scale", "summary", "t", "table", "units"
  ),
  utils = c("data", "history"),
  stats = c(
    "coef", "coefficients", "cov", "df", "dt", "fitted",
    "formula", "sd", "sigma", "start", "time", "var",
    "variable.names", "weights"
  ),
  graphics = c("lines", "par", "plot", "text", "title"),
  grDevices = "colors"
)))

[MichaelChirico]

There are a couple dozen hits on {lintr}:

                      filename line_number                                                                                  line
1                  R/exclude.R          38                                                            df <- as.data.frame(lints)
2                  R/exclude.R          49                                                                file <- df$filename[i]
3                  R/exclude.R         378                                             lines <- unlist(ex[names2(ex) == linter])
4              R/expect_lint.R         102                                                      exp <- if (field == "message") {
5                  R/extract.R          96                                           drop <- defines_knitr_engine(lines[starts])
6   R/get_source_expressions.R         205                            line <- fixup_line(source_expression$lines[[line_number]])
7   R/get_source_expressions.R         236                            line <- fixup_line(source_expression$lines[[line_number]])
8   R/get_source_expressions.R         239                            line <- fixup_line(source_expression$lines[[line_number]])
9   R/get_source_expressions.R         564                                                  lines <- as.integer(names(tab_cols))
10  R/get_source_expressions.R         587       offset <- 7L - (tab_idx + cum_offset) %% 8L # using a tab width of 8 characters
11  R/get_source_expressions.R         633                                                                 start <- true_locs[i]
12  R/get_source_expressions.R         638                                                                   end <- true_locs[i]
13  R/get_source_expressions.R         642                                                                              end <- j
14                    R/lint.R          47                                                    lines <- get_lines(filename, text)
15                    R/lint.R         342                                         new <- "a call to the linters (see ?linters)"
16                    R/lint.R         350                                    new <- "linters classed as 'linter' (see ?Linter)"
17                    R/lint.R         378                              lines <- vapply(lints, `[[`, integer(1L), "line_number")
18                    R/lint.R         665                                                       line <- fill_with(" ", maximum)
19     R/matrix_apply_linter.R          87                                    var <- xml_text(xml_find_all(bad_expr, var_xpath))
20        R/namespace_linter.R         100                                                 symbols <- get_r_string(symbol_nodes)
21     R/object_usage_linter.R         141                                                                         symbols <- c(
22     R/object_usage_linter.R         216                                                         missing <- is.na(res$message)
23            R/paste_linter.R         261                                    args <- xml_find_all(expr, "expr[position() > 1]")
24              R/path_utils.R         146                                                         start <- token[["col1"]] + 1L
25              R/path_utils.R         147                                                           end <- token[["col2"]] - 1L
26        R/pipe_call_linter.R          37           pipe <- xml_text(xml_find_first(bad_expr, "preceding-sibling::SPECIAL[1]"))
27        R/shared_constants.R         294                                                                  symbols <- tryCatch(
28             R/sort_linter.R         111                                                       var <- xml_text(xml_find_first(
29             R/sort_linter.R         119                                              args <- vapply(order_expr, function(e) {
30          R/sprintf_linter.R         116              message <- vapply(sprintf_calls, capture_sprintf_warning, character(1L))
31                   R/utils.R          53                                                       call <- sys.call(which = which)
32                   R/utils.R          57                                      match <- re_matches(nm, regex, locations = TRUE)
33                   R/utils.R          66                                                                  missing <- nms == ""
34                   R/utils.R         175                                                         lines <- withCallingHandlers(
35                    R/with.R         223                     args <- as.character(eval(substitute(alist(...)[missing_index])))
36                    R/with.R         229                   args <- re_substitutes(args, rex("(", anything), "", options = "s")
37                    R/with.R         231                args <- re_substitutes(args, rex(start, anything, '["' %or% "::"), "")
38      R/xml_nodes_to_lints.R          69                                                 lines <- source_expression[["lines"]]
39      R/xml_nodes_to_lints.R          70                        if (is.null(lines)) lines <- source_expression[["file_lines"]]
40 tests/testthat/test-cache.R           7                                           lines <- c("foobar1", "foobar2", "foobar3")
41         vignettes/lintr.Rmd         184                                              args <- linters_with_args[[linter_name]]

[codecov-commenter]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7d20334) 99.40% compared to head (a3c0535) 99.38%.

❗ Current head a3c0535 differs from pull request most recent head 1347524. Consider uploading reports for the commit 1347524 to get more accurate results

Files	Patch %	Lines
R/object_overwrite_linter.R	97.29%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2307      +/-   ##
==========================================
- Coverage   99.40%   99.38%   -0.02%     
==========================================
  Files         118      119       +1     
  Lines        5355     5392      +37     
==========================================
+ Hits         5323     5359      +36     
- Misses         32       33       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MichaelChirico]

Another idea could be to have another argument like allow_names to supply names to ignore, e.g. we might put in "line" for cases like in get_source_expressions():

line <- fixup_line(...)

[AshesITR]

Another idea could be to have another argument like allow_names to supply names to ignore, e.g. we might put in "line" for cases like in get_source_expressions():

line <- fixup_line(...)

That sounds like a great idea.

[MichaelChirico]

Another idea could be to have another argument like allow_names to supply names to ignore, e.g. we might put in "line" for cases like in get_source_expressions():

line <- fixup_line(...)

That sounds like a great idea.

done

[AshesITR]

Metadata + vectorization tests are missing.