Skip to content

Commit

Permalink
support for # nolint next (#2090)
Browse files Browse the repository at this point in the history
* support for # nolint next

* review

---------

Co-authored-by: AshesITR <[email protected]>
  • Loading branch information
MichaelChirico and AshesITR authored Aug 17, 2023
1 parent 8613153 commit 6713862
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 22 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

## New and improved features

* New exclusion sentinel `# nolint next` to signify the next line should skip linting (#1791, @MichaelChirico). The usual rules apply for excluding specific linters, e.g. `# nolint next: assignment_linter.`. The exact string used to match a subsequent-line exclusion is controlled by the `exclude_next` config entry or R option `"lintr.exclude_next"`.
* Linters with logic around the magrittr pipe `%>%` consistently apply it to the other pipes `%!>%`, `%T>%`, `%<>%` (and possibly `%$%`) where appropriate (#2008, @MichaelChirico).
+ `brace_linter()`
+ `pipe_call_linter()`
Expand Down
43 changes: 29 additions & 14 deletions R/exclude.R
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,20 @@ line_info <- function(line_numbers, type = c("start", "end")) {
#' read a source file and parse all the excluded lines from it
#'
#' @param file R source file
#' @param exclude regular expression used to mark lines to exclude
#' @param exclude_start regular expression used to mark the start of an excluded range
#' @param exclude_end regular expression used to mark the end of an excluded range
#' @param exclude_linter regular expression used to capture a list of to-be-excluded linters immediately following a
#' @param exclude Regular expression used to mark lines to exclude.
#' @param exclude_next Regular expression used to mark lines immediately preceding excluded lines.
#' @param exclude_start Regular expression used to mark the start of an excluded range.
#' @param exclude_end Regular expression used to mark the end of an excluded range.
#' @param exclude_linter Regular expression used to capture a list of to-be-excluded linters immediately following a
#' `exclude` or `exclude_start` marker.
#' @param exclude_linter_sep regular expression used to split a linter list into individual linter names for exclusion.
#' @param lines a character vector of the content lines of `file`
#' @param linter_names Names of active linters
#' @param exclude_linter_sep Regular expression used to split a linter list into individual linter names for exclusion.
#' @param lines A character vector of the content lines of `file`.
#' @param linter_names Names of active linters.
#'
#' @return A possibly named list of excluded lines, possibly for specific linters.
parse_exclusions <- function(file, exclude = settings$exclude,
parse_exclusions <- function(file,
exclude = settings$exclude,
exclude_next = settings$exclude_next,
exclude_start = settings$exclude_start,
exclude_end = settings$exclude_end,
exclude_linter = settings$exclude_linter,
Expand Down Expand Up @@ -131,22 +134,34 @@ parse_exclusions <- function(file, exclude = settings$exclude,
}
}

next_locations <- re_matches(lines, exclude_next, locations = TRUE)[, "end"] + 1L
nexts <- which(!is.na(next_locations))

nolint_locations <- re_matches(lines, exclude, locations = TRUE)[, "end"] + 1L
nolints <- which(!is.na(nolint_locations))
# Disregard nolint tags if they also match nolint start / end
nolints <- setdiff(nolints, c(starts, ends))

for (i in seq_along(nolints)) {
linters_string <- substring(lines[nolints[i]], nolint_locations[nolints[i]])
linters_string <- re_matches(linters_string, exclude_linter)[, 1L]
exclusions <- add_exclusions(exclusions, nolints[i], linters_string, exclude_linter_sep, linter_names)
# Disregard nolint tags if they also match nolint next / start / end
nolints <- setdiff(nolints, c(nexts, starts, ends))

for (nolint in nolints) {
linters_string <- get_linters_string(lines[nolint], nolint_locations[nolint], exclude_linter)
exclusions <- add_exclusions(exclusions, nolint, linters_string, exclude_linter_sep, linter_names)
}
for (nextt in nexts) {
linters_string <- get_linters_string(lines[nextt], next_locations[nextt], exclude_linter)
exclusions <- add_exclusions(exclusions, nextt + 1L, linters_string, exclude_linter_sep, linter_names)
}

exclusions[] <- lapply(exclusions, function(lines) sort(unique(lines)))

exclusions
}

get_linters_string <- function(line, loc, exclude_linter) {
linters_string <- substring(line, loc)
re_matches(linters_string, exclude_linter)[, 1L]
}

add_excluded_lines <- function(exclusions, excluded_lines, excluded_linters) {
for (linter in excluded_linters) {
if (linter %in% names2(exclusions)) {
Expand Down
1 change: 1 addition & 0 deletions R/zzz.R
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ settings <- NULL
linters = default_linters,
encoding = "UTF-8",
exclude = rex("#", any_spaces, "nolint"),
exclude_next = rex("#", any_spaces, "nolint next"),
exclude_start = rex("#", any_spaces, "nolint start"),
exclude_end = rex("#", any_spaces, "nolint end"),
exclude_linter = rex(
Expand Down
2 changes: 1 addition & 1 deletion man/default_settings.Rd

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

17 changes: 10 additions & 7 deletions man/parse_exclusions.Rd

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

56 changes: 56 additions & 0 deletions tests/testthat/test-exclusions.R
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,59 @@ test_that("#1442: is_excluded_files works if no global exclusions are specified"
)
expect_length(lint_dir(tmp), 3L)
})

test_that("next-line exclusion works", {
withr::local_options(
lintr.exclude = "# NL",
lintr.exclude_next = "# NLN",
lintr.exlcude_linter = default_settings$exclude_linter
)

linter <- assignment_linter()

# blanket exclusion works
expect_lint(
trim_some("
# NLN
x = 1
"),
NULL,
linter
)

# specific exclusion works
expect_lint(
trim_some("
# NLN: assignment_linter.
x = 1
"),
NULL,
linter
)
expect_lint(
trim_some("
# NLN: assignment.
x = 1
"),
NULL,
linter
)
expect_lint(
trim_some("
# NLN: line_length_linter.
x = 1
"),
rex::rex("Use <-, not =, for assignment."),
list(linter, line_length_linter())
)

# interaction with plain nolint
expect_lint(
trim_some("
x = 1 # NLN: assignment_linter.
x = 2
"),
list(rex::rex("Use <-, not =, for assignment."), line_number = 1L),
linter
)
})

0 comments on commit 6713862

Please sign in to comment.