Skip to content

Commit

Permalink
merge with main
Browse files Browse the repository at this point in the history
  • Loading branch information
IndrajeetPatil committed Mar 23, 2024
2 parents d3c53f5 + dd63d9c commit 807f41e
Show file tree
Hide file tree
Showing 18 changed files with 175 additions and 313 deletions.
2 changes: 1 addition & 1 deletion .github/CODE_OF_CONDUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ representative at an online or offline event.
## Enforcement

Instances of abusive, harassing, or otherwise unacceptable behavior may be
reported to the community leaders responsible for enforcement at james.f.hester@gmail.com.
reported to the community leaders responsible for enforcement at michaelchirico4@gmail.com.
All complaints will be reviewed and investigated promptly and fairly.

All community leaders are obligated to respect the privacy and security of the
Expand Down
9 changes: 6 additions & 3 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ Package: lintr
Title: A 'Linter' for R Code
Version: 3.1.1.9000
Authors@R: c(
person("Jim", "Hester", , "[email protected]", role = c("aut", "cre")),
person("Jim", "Hester", , role = "aut"),
person("Florent", "Angly", role = "aut",
comment = "fangly"),
person("Russ", "Hyde", role = "aut"),
person("Michael", "Chirico", role = "aut"),
person("Michael", "Chirico", email = "[email protected]", role = c("aut", "cre")),
person("Kun", "Ren", role = "aut"),
person("Alexander", "Rosenstock", role = "aut",
comment = "AshesITR"),
Expand Down Expand Up @@ -37,7 +37,11 @@ Imports:
xmlparsedata (>= 1.0.5)
Suggests:
bookdown,
<<<<<<< HEAD
httr (>= 1.2.1),
=======
cli,
>>>>>>> main
jsonlite,
patrick (>= 0.2.0),
rlang,
Expand Down Expand Up @@ -76,7 +80,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'
Expand Down
4 changes: 3 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
* 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

* `object_name_linter()` no longer errors when user-supplied `regexes=` have capture groups (#2188, @MichaelChirico).
* `.lintr` config validation correctly accepts regular expressions which only compile under `perl = TRUE` (#2375, @MichaelChirico). These have always been valid (since `rex::re_matches()`, which powers the lint exclusion logic, also uses this setting), but the new up-front validation in v3.1.1 incorrectly used `perl = FALSE`.
* `.lintr` configs set by option `lintr.linter_file` or environment variable `R_LINTR_LINTER_FILE` can point to subdirectories (#2512, @MichaelChirico).

## Changes to default linters

Expand Down Expand Up @@ -60,7 +62,7 @@
* `which_grepl_linter()` for discouraging `which(grepl(ptn, x))` in favor of directly using `grep(ptn, x)` (part of #884, @MichaelChirico).
* `list_comparison_linter()` for discouraging comparisons on the output of `lapply()`, e.g. `lapply(x, sum) > 10` (part of #884, @MichaelChirico).
* `print_linter()` for discouraging usage of `print()` on string literals like `print("Reached here")` or `print(paste("Found", nrow(DF), "rows."))` (#1894, @MichaelChirico).
* `unnecessary_nesting_linter()` for discouraging overly-nested code where an early return or eliminated sub-expression (inside '{') is preferable (#2317 and part of #884, @MichaelChirico).
* `unnecessary_nesting_linter()` for discouraging overly-nested code where an early return or eliminated sub-expression (inside '{') is preferable (#2317, #2334 and part of #884, @MichaelChirico).
* `consecutive_mutate_linter()` for encouraging consecutive calls to `dplyr::mutate()` to be combined (part of #884, @MichaelChirico).
* `if_switch_linter()` for encouraging `switch()` over repeated `if`/`else` tests (#2322 and part of #884, @MichaelChirico).
* `nested_pipe_linter()` for discouraging pipes within pipes, e.g. `df1 %>% inner_join(df2 %>% select(a, b))` (part of #884, @MichaelChirico).
Expand Down
119 changes: 0 additions & 119 deletions R/comments.R

This file was deleted.

12 changes: 0 additions & 12 deletions R/methods.R
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...)
}

Expand Down
2 changes: 1 addition & 1 deletion R/settings_utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ find_config <- function(filename) {
# may exist in subsequent directories are ignored
file_locations <- c(
# Local (incl. parent) directories
find_local_config(path, basename(linter_file)),
find_local_config(path, linter_file),
# User directory
# cf: rstudio@bc9b6a5 SessionRSConnect.R#L32
file.path(Sys.getenv("HOME", unset = "~"), linter_file),
Expand Down
29 changes: 28 additions & 1 deletion R/unnecessary_nesting_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
#' The `TRUE` case facilitates interaction with [implicit_assignment_linter()]
#' for certain cases where an implicit assignment is necessary, so a braced
#' assignment is used to further distinguish the assignment. See examples.
#' @param allow_functions Character vector of functions which always allow
#' one-child braced expressions. `testthat::test_that()` is always allowed because
#' testthat requires a braced expression in its `code` argument. The other defaults
#' similarly compute on expressions in a way which is worth highlighting by
#' em-bracing them, even if there is only one expression, while [switch()] is allowed
#' for its use as a control flow analogous to `if`/`else`.
#'
#' @examples
#' # will produce lints
Expand Down Expand Up @@ -39,6 +45,11 @@
#' linters = unnecessary_nesting_linter()
#' )
#'
#' lint(
#' text = "my_quote({x})",
#' linters = unnecessary_nesting_linter()
#' )
#'
#' # okay
#' code <- "if (A) {\n stop('A is bad because a.')\n} else {\n stop('!A is bad too.')\n}"
#' writeLines(code)
Expand Down Expand Up @@ -73,12 +84,27 @@
#' linters = unnecessary_nesting_linter()
#' )
#'
#' lint(
#' text = "my_quote({x})",
#' linters = unnecessary_nesting_linter(allow_functions = "my_quote")
#' )
#'
#' @evalRd rd_tags("unnecessary_nesting_linter")
#' @seealso
#' - [cyclocomp_linter()] for another linter that penalizes overly complexcode.
#' - [linters] for a complete list of linters available in lintr.
#' @export
unnecessary_nesting_linter <- function(allow_assignment = TRUE) {
unnecessary_nesting_linter <- function(
allow_assignment = TRUE,
allow_functions = c(
"switch",
"try", "tryCatch", "withCallingHandlers",
"quote", "expression", "bquote", "substitute",
"with_parameters_test_that",
"reactive", "observe", "observeEvent",
"renderCachedPlot", "renderDataTable", "renderImage", "renderPlot",
"renderPrint", "renderTable", "renderText", "renderUI"
)) {
exit_calls <- c("stop", "return", "abort", "quit", "q")
exit_call_expr <- glue("
expr[SYMBOL_FUNCTION_CALL[{xp_text_in_table(exit_calls)}]]
Expand Down Expand Up @@ -145,6 +171,7 @@ unnecessary_nesting_linter <- function(allow_assignment = TRUE) {
or self::IF
or self::WHILE
or self::REPEAT
or self::expr/SYMBOL_FUNCTION_CALL[{ xp_text_in_table(c('test_that', allow_functions)) }]
or self::expr/expr/SYMBOL_FUNCTION_CALL[text() = 'foreach']
or self::OP-TILDE
or self::LEFT_ASSIGN[text() = ':=']
Expand Down
2 changes: 1 addition & 1 deletion inst/syntastic/lintr.vim
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"============================================================================
"File: lintr.vim
"Description: Syntax checking plugin for syntastic.vim
"Maintainer: Jim Hester <james.f.hester at gmail dot com>
"Maintainer: Michael Chirico <michaelchirico4 at gmail dot com>
"License: This program is free software. It comes without any warranty,
" to the extent permitted by applicable law. You can redistribute
" it and/or modify it under the terms of the Do What The Fuck You
Expand Down
4 changes: 2 additions & 2 deletions man/lintr-package.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 24 additions & 1 deletion man/unnecessary_nesting_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 0 additions & 22 deletions tests/testthat/test-ci.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Loading

0 comments on commit 807f41e

Please sign in to comment.