Merge branch 'main' into exec

Author: IndrajeetPatil

DESCRIPTION

@@ -127,7 +127,6 @@ Collate:
     'namespace.R'
     'namespace_linter.R'
     'nested_ifelse_linter.R'
-    'no_tab_linter.R'
     'nonportable_path_linter.R'
     'numeric_leading_zero_linter.R'
     'object_length_linter.R'
@@ -169,6 +168,7 @@ Collate:
     'unused_import_linter.R'
     'use_lintr.R'
     'vector_logic_linter.R'
+    'whitespace_linter.R'
     'with.R'
     'with_id.R'
     'xml_nodes_to_lints.R'

NAMESPACE

@@ -136,6 +136,7 @@ export(unreachable_code_linter)
 export(unused_import_linter)
 export(use_lintr)
 export(vector_logic_linter)
+export(whitespace_linter)
 export(with_defaults)
 export(with_id)
 export(xml_nodes_to_lints)

NEWS.md

@@ -1,10 +1,12 @@
 # lintr (development version)
 
-## Deprecations
+## Deprecations & Breaking Changes
 
 * `single_quotes_linter()` is deprecated in favor of the more generalizable `quotes_linter()` (#1729, @MichaelChirico).
 * `unneeded_concatentation_linter()` is deprecated in favor of `unnecessary_concatenation_linter()` for naming consistency (#1707, @IndrajeetPatil).
 * `consecutive_stopifnot_linter()` is deprecated in favor of the more general (see below) `consecutive_assertion_linter()` (#1604, @MichaelChirico).
+* `no_tab_linter()` is deprecated in favor of `whitespace_linter()` for naming consistency and future generalization (#1954, @MichaelChirico).
+* `available_linters()` prioritizes `tags` over `exclude_tags` in the case of overlap, i.e., tags listed in both arguments are included, not excluded. We don't expect many people to be affected by this, and the old behavior was not made explicit in the documentation, but make note of it here since it required changing a test in lintr's own suite where `linters_with_tags()` implicitly assumed this behavior.
 
 ## Bug fixes
 
@@ -37,6 +39,9 @@
 
 * `object_name_linter()` allows all S3 group Generics (see `?base::groupGeneric`) and S3 generics defined in a different file in the same package (#1808, #1841, @AshesITR)
 
+* `object_usage_linter()` improves identification of the exact source of a lint for undefined variables in expressions with where the variable is used as a symbol in a usual way, for example in a formula or in an extraction with `$` (#1914, @MichaelChirico).
+
+* `function_left_parentheses_linter()` produces a more specific lint (and no longer fails) when the opening parenthesis is on a different line than `function` or the call name (#1953, @MichaelChirico).
 ## Changes to defaults
 
 * Set the default for the `except` argument in `duplicate_argument_linter()` to `c("mutate", "transmute")`.
@@ -63,6 +68,7 @@
    + `indentation_linter()`
    + `quotes_linter()`
    + `unnecessary_concatenation_linter()`
+   + `whitespace_linter()`
 
 * `lint_package()` also looks for files in `exec/` (#1950, @jmaspons).
 
@@ -111,6 +117,10 @@
 
 * `missing_argument_linter()` allows missing arguments in `quote()` calls (#1889, @IndrajeetPatil). 
 
+* `get_source_expressions()` correctly extracts indented code chunks from R Markdown documents, which helps avoid spurious lints related to whitespace (#1945, @MichaelChirico). The convention taken is that, within each chunk, all code is anchored relative to the leftmost non-whitespace column.
+
+* `available_linters()` gives priority to `tags` over `exclude_tags` in the case of overlap. In particular, this means that `available_linters(tags = "deprecated")` will work to return deprecated linters without needing to specify `exclude_tags` (#1959, @MichaelChirico).
+
 ### New linters
 
 * `matrix_apply_linter()` recommends use of dedicated `rowSums()`, `colSums()`, `colMeans()`, `rowMeans()` over `apply(., MARGIN, sum)` or `apply(., MARGIN, mean)`. The recommended alternative is much more efficient and more readable (#1869, @Bisaloo).
@@ -152,6 +162,8 @@
 
 * `consecutive_assertion_linter()` (f.k.a. `consecutive_stopifnot_linter()`) now lints for consecutive calls to `assertthat::assert_that()` (as long as the `msg=` argument is not used; #1604, @MichaelChirico).
 
+* `whitespace_linter()` is simply `no_tab_linter()`, renamed. In the future, we plan to extend it to work for different whitespace preferences.
+
 ## Notes
 
 * {lintr} now depends on R version 3.5.0, in line with the tidyverse policy for R version compatibility.

R/extract.R

@@ -22,12 +22,14 @@ extract_r_source <- function(filename, lines, error = identity) {
 
   output_env <- environment() # nolint: object_usage_linter. False positive-ish -- used below.
   Map(
-    function(start, end) {
+    function(start, end, indent) {
       line_seq <- seq(start + 1L, end - 1L)
-      output_env$output[line_seq] <- lines[line_seq]
+      chunk_code <- lines[line_seq]
+      output_env$output[line_seq] <- if (indent > 0L) substr(chunk_code, indent + 1L, nchar(chunk_code)) else chunk_code
     },
     chunks[["starts"]],
-    chunks[["ends"]]
+    chunks[["ends"]],
+    chunks[["indents"]]
   )
   # drop <<chunk>> references, too
   is.na(output) <- grep(pattern$ref.chunk, output)
@@ -73,7 +75,18 @@ get_chunk_positions <- function(pattern, lines) {
   # only keep those blocks that contain at least one line of code
   keep <- which(ends - starts > 1L)
 
-  list(starts = starts[keep], ends = ends[keep])
+  starts <- starts[keep]
+  ends <- ends[keep]
+
+  # Check indent on all lines in the chunk to allow for staggered indentation within a chunk;
+  #   set the initial column to the leftmost one within each chunk (including the start+end gates). See tests.
+  # use 'ws_re' to make clear that we're matching knitr's definition of initial whitespace.
+  ws_re <- sub("```.*", "", pattern$chunk.begin)
+  indents <- mapply(
+    function(start, end) min(vapply(gregexpr(ws_re, lines[start:end], perl = TRUE), attr, integer(1L), "match.length")),
+    starts, ends
+  )
+  list(starts = starts, ends = ends, indents = indents)
 }
 
 filter_chunk_start_positions <- function(starts, lines) {

R/function_left_parentheses_linter.R

@@ -41,26 +41,37 @@
 #' - [spaces_left_parentheses_linter()]
 #' @export
 function_left_parentheses_linter <- function() { # nolint: object_length.
-  xpath <- "
-  //FUNCTION[@col2 != following-sibling::OP-LEFT-PAREN/@col1 - 1]
-  |
-  //SYMBOL_FUNCTION_CALL/parent::expr[@col2 != following-sibling::OP-LEFT-PAREN/@col1 - 1]
-  "
+  xpath_fmt <- "//FUNCTION[ {cond} ] | //SYMBOL_FUNCTION_CALL/parent::expr[ {cond} ]"
+  bad_line_cond <- "@line1 != following-sibling::OP-LEFT-PAREN/@line1"
+  bad_col_cond <- xp_and(
+    "@line1 = following-sibling::OP-LEFT-PAREN/@line1",
+    "@col2 != following-sibling::OP-LEFT-PAREN/@col1 - 1"
+  )
+  bad_line_xpath <- glue::glue(xpath_fmt, cond = bad_line_cond)
+  bad_col_xpath <- glue::glue(xpath_fmt, cond = bad_col_cond)
 
   Linter(function(source_expression) {
     if (!is_lint_level(source_expression, "expression")) {
       return(list())
     }
 
     xml <- source_expression$xml_parsed_content
-    bad_exprs <- xml2::xml_find_all(xml, xpath)
 
-    xml_nodes_to_lints(
-      bad_exprs,
+    bad_line_exprs <- xml2::xml_find_all(xml, bad_line_xpath)
+    bad_line_lints <- xml_nodes_to_lints(
+      bad_line_exprs,
+      source_expression = source_expression,
+      lint_message = "Left parenthesis should be on the same line as the function's symbol."
+    )
+
+    bad_col_exprs <- xml2::xml_find_all(xml, bad_col_xpath)
+    bad_col_lints <- xml_nodes_to_lints(
+      bad_col_exprs,
       source_expression = source_expression,
       lint_message = "Remove spaces before the left parenthesis in a function call.",
       range_start_xpath = "number(./@col2 + 1)", # start after function / fun
       range_end_xpath = "number(./following-sibling::OP-LEFT-PAREN/@col1 - 1)" # end before (
     )
+    c(bad_line_lints, bad_col_lints)
   })
 }

R/linter_tags.R

@@ -7,7 +7,8 @@
 #' returned. If `tags` is `NULL`, all linters will be returned. See `available_tags("lintr")` to find out what
 #' tags are already used by lintr.
 #' @param exclude_tags Tags to exclude from the results. Linters with at least one matching tag will not be returned.
-#' If `except_tags` is `NULL`, no linters will be excluded.
+#' If `except_tags` is `NULL`, no linters will be excluded. Note that `tags` takes priority, meaning that any
+#' tag found in both `tags` and `exclude_tags` will be included, not excluded.
 #'
 #' @section Package Authors:
 #'
@@ -41,7 +42,9 @@
 #'
 #' lintr_linters2 <- available_linters(c("lintr", "does-not-exist"))
 #' identical(lintr_linters, lintr_linters2)
-#' @seealso [linters] for a complete list of linters available in lintr.
+#' @seealso
+#'  - [linters] for a complete list of linters available in lintr.
+#'  - [available_tags()] to retrieve the set of valid tags.
 #' @export
 available_linters <- function(packages = "lintr", tags = NULL, exclude_tags = "deprecated") {
   if (!is.character(packages)) {
@@ -54,6 +57,9 @@ available_linters <- function(packages = "lintr", tags = NULL, exclude_tags = "d
     stop("`exclude_tags` must be a character vector.")
   }
 
+  # any tags specified explicitly will not be excluded (#1959)
+  exclude_tags <- setdiff(exclude_tags, tags)
+
   # Handle multiple packages
   if (length(packages) > 1L) {
     return(do.call(rbind, lapply(packages, available_linters, tags = tags, exclude_tags = exclude_tags)))
@@ -160,7 +166,7 @@ rd_tags <- function(linter_name) {
 #'
 #' @noRd
 rd_linters <- function(tag_name) {
-  linters <- available_linters(tags = tag_name, exclude_tags = NULL)
+  linters <- available_linters(tags = tag_name)
   tagged <- platform_independent_sort(linters[["linter"]])
   if (length(tagged) == 0L) {
     stop("No linters found associated with tag ", tag_name)
@@ -181,9 +187,11 @@ rd_linters <- function(tag_name) {
 #' @noRd
 rd_taglist <- function() {
   linters <- available_linters(exclude_tags = NULL)
+  # don't count tags on deprecated linters to the counts of other tags
+  linters$tags <- lapply(linters$tags, function(x) if ("deprecated" %in% x) "deprecated" else x)
 
   tag_table <- table(unlist(linters[["tags"]]))
-  tags <- platform_independent_sort(unique(unlist(linters[["tags"]])))
+  tags <- platform_independent_sort(names(tag_table))
   # re-order
   tag_table <- tag_table[tags]
 
@@ -203,7 +211,7 @@ rd_taglist <- function() {
 #'
 #' @noRd
 rd_linterlist <- function() {
-  linters <- available_linters(exclude_tags = NULL)
+  linters <- available_linters()
   linter_names <- platform_independent_sort(linters[["linter"]])
 
   c(

R/lintr-deprecated.R

@@ -239,3 +239,16 @@ consecutive_stopifnot_linter <- function() {
   )
   consecutive_assertion_linter()
 }
+
+#' No tabs linter
+#' @rdname lintr-deprecated
+#' @export
+no_tab_linter <- function() {
+  lintr_deprecated(
+    old = "no_tab_linter",
+    new = "whitespace_linter",
+    version = "3.1.0",
+    type = "Linter"
+  )
+  whitespace_linter()
+}

R/object_usage_linter.R

@@ -46,6 +46,15 @@ object_usage_linter <- function(interpret_glue = TRUE, skip_with = TRUE) {
     sep = " | "
   )
 
+  # not all instances of linted symbols are potential sources for the observed violations -- see #1914
+  symbol_exclude_cond <- "preceding-sibling::OP-DOLLAR or preceding-sibling::OP-AT or ancestor::expr[OP-TILDE]"
+  xpath_culprit_symbol <- glue::glue("
+    descendant::SYMBOL[not( {symbol_exclude_cond} )]
+    | descendant::SYMBOL_FUNCTION_CALL[not( {symbol_exclude_cond} )]
+    | descendant::SPECIAL
+    | descendant::LEFT_ASSIGN[text() = ':=']
+  ")
+
   Linter(function(source_expression) {
     if (!is_lint_level(source_expression, "file")) {
       return(list())
@@ -97,15 +106,7 @@ object_usage_linter <- function(interpret_glue = TRUE, skip_with = TRUE) {
       # e.g. `not_existing<-`(a, b)
       res$name <- rex::re_substitutes(res$name, rex::rex("<-"), "")
 
-      lintable_symbols <- xml2::xml_find_all(
-        fun_assignment,
-        "
-        descendant::SYMBOL
-        | descendant::SYMBOL_FUNCTION_CALL
-        | descendant::SPECIAL
-        | descendant::LEFT_ASSIGN[text() = ':=']
-        "
-      )
+      lintable_symbols <- xml2::xml_find_all(fun_assignment, xpath_culprit_symbol)
 
       lintable_symbol_names <- gsub("^`|`$", "", xml2::xml_text(lintable_symbols))
       lintable_symbol_lines <- as.integer(xml2::xml_attr(lintable_symbols, "line1"))

R/no_tab_linter.R renamed to R/whitespace_linter.R

@@ -1,32 +1,35 @@
-#' No tab linter
+#' Whitespace linter
 #'
-#' Check that only spaces are used for indentation, not tabs. Much ink has been
-#' spilled on this topic, and we encourage you to check out references for more
-#' information.
+#' Check that the correct character is used for indentation.
+#'
+#' Currently, only supports linting in the presence of tabs.
+#'
+#' Much ink has been spilled on this topic, and we encourage you to check
+#'   out references for more information.
 #'
 #' @include make_linter_from_regex.R
 #'
 #' @examples
 #' # will produce lints
 #' lint(
 #'   text = "\tx",
-#'   linters = no_tab_linter()
+#'   linters = whitespace_linter()
 #' )
 #'
 #' # okay
 #' lint(
 #'   text = "  x",
-#'   linters = no_tab_linter()
+#'   linters = whitespace_linter()
 #' )
 #'
-#' @evalRd rd_tags("no_tab_linter")
+#' @evalRd rd_tags("whitespace_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #'
 #' @references
 #' - https://www.jwz.org/doc/tabs-vs-spaces.html
 #' - https://blog.codinghorror.com/death-to-the-space-infidels/
 #' @export
-no_tab_linter <- make_linter_from_regex(
+whitespace_linter <- make_linter_from_regex(
   regex = rex(start, zero_or_more(regex("\\s")), one_or_more("\t")),
   lint_type = "style",
   lint_msg = "Use spaces to indent, not tabs."

R/zzz.R

@@ -23,7 +23,6 @@ default_linters <- modify_defaults(
   indentation_linter(),
   infix_spaces_linter(),
   line_length_linter(),
-  no_tab_linter(),
   object_length_linter(),
   object_name_linter(),
   object_usage_linter(),
@@ -37,7 +36,8 @@ default_linters <- modify_defaults(
   T_and_F_symbol_linter(),
   trailing_blank_lines_linter(),
   trailing_whitespace_linter(),
-  vector_logic_linter()
+  vector_logic_linter(),
+  whitespace_linter()
 )
 
 #' Default undesirable functions and operators

inst/lintr/linters.csv

@@ -49,7 +49,7 @@ missing_argument_linter,correctness common_mistakes configurable
 missing_package_linter,robustness common_mistakes
 namespace_linter,correctness robustness configurable executing
 nested_ifelse_linter,efficiency readability
-no_tab_linter,style consistency default
+no_tab_linter,style consistency deprecated
 nonportable_path_linter,robustness best_practices configurable
 numeric_leading_zero_linter,style consistency readability
 object_length_linter,style readability default configurable executing
@@ -93,4 +93,5 @@ unneeded_concatenation_linter,style readability efficiency configurable deprecat
 unreachable_code_linter,best_practices readability
 unused_import_linter,best_practices common_mistakes configurable executing
 vector_logic_linter,default efficiency best_practices
+whitespace_linter,style consistency default
 yoda_test_linter,package_development best_practices readability