Merge branch 'main' into skip-basename

DESCRIPTION

@@ -37,7 +37,6 @@ Imports:
 Suggests:
     bookdown,
     cli,
-    httr (>= 1.2.1),
     jsonlite,
     patrick (>= 0.2.0),
     rlang,
@@ -76,7 +75,6 @@ Collate:
     'class_equals_linter.R'
     'commas_linter.R'
     'commented_code_linter.R'
-    'comments.R'
     'comparison_negation_linter.R'
     'condition_call_linter.R'
     'condition_message_linter.R'

NEWS.md

@@ -14,6 +14,7 @@
 * Adjusted various lint messages for consistency in readability (#1330, @MichaelChirico). In general, we favor lint messages to be phrased like "Action, reason" to but the "what" piece of the message front-and-center. This may be a breaking change for code that tests the specific phrasing of lints.
 * `extraction_operator_linter()` is deprecated. Although switching from `$` to `[[` has some robustness benefits for package code, it can lead to non-idiomatic code in many contexts (e.g. R6 classes, Shiny applications, etc.) (#2409, @IndrajeetPatil). To enable the detection of the `$` operator for extraction through partial matching, use `options(warnPartialMatchDollar = TRUE)`.
 * `unnecessary_nested_if_linter()` is deprecated and subsumed into the new/more general `unnecessary_nesting_linter()`.
+* Drop support for posting GitHub comments from inside Travis, Wercker, and Jenkins CI tools (spurred by #2148, @MichaelChirico). We rely on GitHub Actions for linting in CI, and don't see any active users relying on these alternatives. We welcome and encourage community contributions to get support for different CI system going again.
 
 ## Bug fixes
 

R/methods.R

@@ -95,18 +95,6 @@ print.lints <- function(x, ...) {
     } else if (in_github_actions()) {
       github_actions_log_lints(x, project_dir = github_annotation_project_dir)
     } else {
-      if (in_ci() && settings$comment_bot) {
-        info <- ci_build_info()
-
-        lint_output <- trim_output(
-          paste0(
-            collapse = "\n",
-            capture.output(invisible(lapply(x, markdown, info, ...)))
-          )
-        )
-
-        github_comment(lint_output, info, ...)
-      }
       lapply(x, print, ...)
     }
 

tests/testthat/test-ci.R

@@ -37,25 +37,3 @@ test_that("GitHub Actions - linting on error works", {
   local_mocked_bindings(quit = function(...) cat("Tried to quit.\n"))
   expect_output(print(l), "::warning file", fixed = TRUE)
 })
-
-test_that("Printing works for Travis", {
-  withr::local_envvar(list(GITHUB_ACTIONS = "false", TRAVIS_REPO_SLUG = "test/repo", LINTR_COMMENT_BOT = "true"))
-  withr::local_options(lintr.rstudio_source_markers = FALSE)
-  tmp <- withr::local_tempfile(lines = "x <- 1:nrow(y)")
-
-  l <- lint(tmp)
-
-  local_mocked_bindings(github_comment = function(x, ...) cat(x, "\n"))
-  expect_output(print(l), "*warning:*", fixed = TRUE)
-})
-
-test_that("Printing works for Wercker", {
-  withr::local_envvar(list(GITHUB_ACTIONS = "false", WERCKER_GIT_BRANCH = "test/repo", LINTR_COMMENT_BOT = "true"))
-  withr::local_options(lintr.rstudio_source_markers = FALSE)
-  tmp <- withr::local_tempfile(lines = "x <- 1:nrow(y)")
-
-  l <- lint(tmp)
-
-  local_mocked_bindings(github_comment = function(x, ...) cat(x, "\n"))
-  expect_output(print(l), "*warning:*", fixed = TRUE)
-})

tests/testthat/test-fixed_regex_linter.R

@@ -287,51 +287,67 @@ robust_non_printable_unicode <- function() {
 }
 
 # styler: off
-patrick::with_parameters_test_that("fixed replacements are correct", {
-  skip_if(
-    regex_expr %in% c("abc\\U{A0DEF}ghi", "[\\U1d4d7]", "[\\U{1D4D7}]", "\\u{A0}\\U{0001d4d7}") &&
-      .Platform$OS.type == "windows" &&
-      !hasName(R.Version(), "crt"),
-    message = "UTF-8 support is required"
+local({
+  .cases <- tibble::tribble(
+    ~.test_name,            ~regex_expr,            ~fixed_expr,
+    "[.]",                  "[.]",                  ".",
+    '[\\\"]',               '[\\\"]',               '\\"',
+    "[]]",                  "[]]",                  "]",
+    "\\\\.",                "\\\\.",                ".",
+    "\\\\:",                "\\\\:",                ":",
+    "\\\\<",                "\\\\<",                "<",
+    "\\\\$",                "\\\\$",                "$",
+    "[\\1]",                "[\\1]",                "\\001",
+    "\\1",                  "\\1",                  "\\001",
+    "[\\12]",               "[\\12]",               "\\n",
+    "[\\123]",              "[\\123]",              "S",
+    "a[*]b",                "a[*]b",                "a*b",
+    "abcdefg",              "abcdefg",              "abcdefg",
+    "abc\\U{A0DEF}ghi",     "abc\\U{A0DEF}ghi",     robust_non_printable_unicode(),
+    "a-z",                  "a-z",                  "a-z",
+    "[\\n]",                "[\\n]",                "\\n",
+    "\\n",                  "\n",                   "\\n",
+    "[\\u01]",              "[\\u01]",              "\\001",
+    "[\\u012]",             "[\\u012]",             "\\022",
+    "[\\u0123]",            "[\\u0123]",            "\u0123",
+    "[\\u{1}]",             "[\\u{1}]",             "\\001",
+    "[\\U1d4d7]",           "[\\U1d4d7]",           "\U1D4D7",
+    "[\\U{1D4D7}]",         "[\\U{1D4D7}]",         "\U1D4D7",
+    "[\\U8]",               "[\\U8]",               "\\b",
+    "\\u{A0}",              "\\u{A0}",              "\uA0",
+    "\\u{A0}\\U{0001d4d7}", "\\u{A0}\\U{0001d4d7}", "\uA0\U1D4D7",
+    "[\\uF]",               "[\\uF]",               "\\017",
+    "[\\U{F7D5}]",          "[\\U{F7D5}]",          "\UF7D5",
+    "[\\x32]",              "[\\x32]",              "2",
+    "[\\xa]",               "[\\xa]",               "\\n"
   )
-  expect_lint(
-    sprintf("grepl('%s', x)", regex_expr),
-    rex::rex(sprintf('Use "%s" with fixed = TRUE', fixed_expr)),
-    fixed_regex_linter()
+  if (.Platform$OS.type == "windows" && !hasName(R.Version(), "crt")) {
+    skip_cases <- c(
+      # These require UTF-8 support
+      "abc\\U{A0DEF}ghi", "[\\U1d4d7]", "[\\U{1D4D7}]", "\\u{A0}\\U{0001d4d7}",
+      # R version-specific difference in output message on Windows (probably r80051)
+      if (getRversion() == "4.0.4") "[\\U{F7D5}]"
+    )
+  } else {
+    skip_cases <- character()
+  }
+  patrick::with_parameters_test_that(
+    "fixed replacements are correct",
+    {
+      # TODO(google/patrick#19): handle this more cleanly by skipping up-front
+      skip_if(
+        regex_expr %in% skip_cases,
+        sprintf("regex '%s' is not supported on this system", regex_expr)
+      )
+      expect_lint(
+        sprintf("grepl('%s', x)", regex_expr),
+        rex::rex(sprintf('Use "%s" with fixed = TRUE', fixed_expr)),
+        fixed_regex_linter()
+      )
+    },
+    .cases = .cases
   )
-}, .cases = tibble::tribble(
-  ~.test_name,            ~regex_expr,            ~fixed_expr,
-  "[.]",                  "[.]",                  ".",
-  '[\\\"]',               '[\\\"]',               '\\"',
-  "[]]",                  "[]]",                  "]",
-  "\\\\.",                "\\\\.",                ".",
-  "\\\\:",                "\\\\:",                ":",
-  "\\\\<",                "\\\\<",                "<",
-  "\\\\$",                "\\\\$",                "$",
-  "[\\1]",                "[\\1]",                "\\001",
-  "\\1",                  "\\1",                  "\\001",
-  "[\\12]",               "[\\12]",               "\\n",
-  "[\\123]",              "[\\123]",              "S",
-  "a[*]b",                "a[*]b",                "a*b",
-  "abcdefg",              "abcdefg",              "abcdefg",
-  "abc\\U{A0DEF}ghi",     "abc\\U{A0DEF}ghi",     robust_non_printable_unicode(),
-  "a-z",                  "a-z",                  "a-z",
-  "[\\n]",                "[\\n]",                "\\n",
-  "\\n",                  "\n",                   "\\n",
-  "[\\u01]",              "[\\u01]",              "\\001",
-  "[\\u012]",             "[\\u012]",             "\\022",
-  "[\\u0123]",            "[\\u0123]",            "\u0123",
-  "[\\u{1}]",             "[\\u{1}]",             "\\001",
-  "[\\U1d4d7]",           "[\\U1d4d7]",           "\U1D4D7",
-  "[\\U{1D4D7}]",         "[\\U{1D4D7}]",         "\U1D4D7",
-  "[\\U8]",               "[\\U8]",               "\\b",
-  "\\u{A0}",              "\\u{A0}",              "\uA0",
-  "\\u{A0}\\U{0001d4d7}", "\\u{A0}\\U{0001d4d7}", "\uA0\U1D4D7",
-  "[\\uF]",               "[\\uF]",               "\\017",
-  "[\\U{F7D5}]",          "[\\U{F7D5}]",          "\UF7D5",
-  "[\\x32]",              "[\\x32]",              "2",
-  "[\\xa]",               "[\\xa]",               "\\n"
-))
+})
 # styler: on
 
 test_that("'unescaped' regex can optionally be skipped", {

tests/testthat/test-lint_package.R

@@ -231,6 +231,9 @@ test_that("package using .lintr.R config lints correctly", {
 })
 
 test_that("lintr need not be attached for .lintr.R configs to use lintr functions", {
+  # For some obscure reason, running in the subprocess on this specific version of R
+  #   on Windows stopped working after PR #2446 with 'Package lintr not found'.
+  if (getRversion() == "3.6.3") skip_on_os("windows")
   exprs <- paste(
     'options(lintr.linter_file = "lintr_test_config")',
     sprintf('lints <- lintr::lint_package("%s")', test_path("dummy_packages", "RConfig")),