Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename no_tab_linter to whitespace_linter #1955

Merged
merged 7 commits into from
Apr 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* `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).

## Bug fixes

Expand Down Expand Up @@ -65,6 +66,7 @@
+ `indentation_linter()`
+ `quotes_linter()`
+ `unnecessary_concatenation_linter()`
+ `whitespace_linter()`

## New and improved features

Expand Down Expand Up @@ -152,6 +154,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.
Expand Down
13 changes: 13 additions & 0 deletions R/lintr-deprecated.R
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
19 changes: 11 additions & 8 deletions R/no_tab_linter.R → R/whitespace_linter.R
Original file line number Diff line number Diff line change
@@ -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."
Expand Down
4 changes: 2 additions & 2 deletions R/zzz.R
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions man/consistency_linters.Rd

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

2 changes: 1 addition & 1 deletion man/default_linters.Rd

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

1 change: 1 addition & 0 deletions man/deprecated_linters.Rd

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

9 changes: 5 additions & 4 deletions man/linters.Rd

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

3 changes: 3 additions & 0 deletions man/lintr-deprecated.Rd

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

1 change: 1 addition & 0 deletions man/style_linters.Rd

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

24 changes: 14 additions & 10 deletions man/no_tab_linter.Rd → man/whitespace_linter.Rd

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

4 changes: 2 additions & 2 deletions tests/testthat/test-expect_lint.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ test_that("single check", {
expect_error(expect_lint("a=1", c(message = lint_msg, lineXXX = 1L), linter), "invalid field")

expect_failure(expect_lint("foo ()", list(ranges = list(c(2L, 2L))), function_left_parentheses_linter()))
expect_success(expect_lint("\t1", list(ranges = list(c(1L, 1L))), no_tab_linter()))
expect_success(expect_lint("\t1", list(ranges = list(c(1L, 1L))), whitespace_linter()))
expect_success(expect_lint("a=1", list(message = lint_msg, line_number = 1L), linter))
expect_failure(expect_lint("a=1", list(2L, lint_msg), linter))

Expand All @@ -48,7 +48,7 @@ test_that("multiple checks", {
expect_success(expect_lint("a=1; b=2", list(list(line_number = 1L), list(line_number = 2L)), linter))
expect_failure(expect_lint("a=1; b=2", list(list(line_number = 2L), list(line_number = 2L)), linter))
expect_success(
expect_lint("\t1\n\t2", list("tabs", list(column_number = 1L, ranges = list(c(1L, 1L)))), no_tab_linter())
expect_lint("\t1\n\t2", list("tabs", list(column_number = 1L, ranges = list(c(1L, 1L)))), whitespace_linter())
)
})

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
test_that("no_tab_linter skips allowed usages", {
linter <- no_tab_linter()
test_that("whitespace_linter skips allowed usages", {
linter <- whitespace_linter()

expect_lint("blah", NULL, linter)
expect_lint(" blah", NULL, linter)
expect_lint(" blah", NULL, linter)
expect_lint("#\tblah", NULL, linter)
})

test_that("no_tab_linter skips allowed tab usages inside strings", {
linter <- no_tab_linter()
test_that("whitespace_linter skips allowed tab usages inside strings", {
linter <- whitespace_linter()

expect_lint(
'lint_msg <- "dont flag tabs if\tthey are inside a string."',
Expand All @@ -23,8 +23,8 @@ test_that("no_tab_linter skips allowed tab usages inside strings", {
)
})

test_that("no_tab_linter blocks disallowed usages", {
linter <- no_tab_linter()
test_that("whitespace_linter blocks disallowed usages", {
linter <- whitespace_linter()
lint_msg <- rex::rex("Use spaces to indent, not tabs.")

expect_lint(
Expand All @@ -40,10 +40,10 @@ test_that("no_tab_linter blocks disallowed usages", {
)
})

test_that("no_tab_linter blocks disallowed usages with a pipe", {
test_that("whitespace_linter blocks disallowed usages with a pipe", {
skip_if_not_r_version("4.1.0")

linter <- no_tab_linter()
linter <- whitespace_linter()
lint_msg <- rex::rex("Use spaces to indent, not tabs.")

expect_lint(
Expand All @@ -58,3 +58,15 @@ test_that("no_tab_linter blocks disallowed usages with a pipe", {
linter
)
})

test_that("no_tab_linter id deprecated", {
expect_warning(
{
old_linter <- no_tab_linter()
},
"Use whitespace_linter instead",
fixed = TRUE
)
expect_lint(" f(a, b, c)", NULL, old_linter)
expect_lint("\tf(a, b, c)", "not tabs", old_linter)
})
12 changes: 6 additions & 6 deletions tests/testthat/test-with.R
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ test_that("with_defaults is supported with a deprecation warning", {
expect_identical(defaults, old_defaults)

# linters_with_defaults only accepts `defaults = list()` to start from blank
defaults <- linters_with_defaults(defaults = list(), no_tab_linter())
defaults <- linters_with_defaults(defaults = list(), whitespace_linter())
expect_warning(
{
old_defaults <- with_defaults(default = NULL, no_tab_linter())
old_defaults <- with_defaults(default = NULL, whitespace_linter())
},
rex::rex("Use linters_with_defaults or modify_defaults instead.")
)
Expand All @@ -99,17 +99,17 @@ test_that("modify_defaults works", {
test_that("linters_with_defaults(default = .) is supported with a deprecation warning", {
expect_warning(
{
linters <- linters_with_defaults(default = list(), no_tab_linter())
linters <- linters_with_defaults(default = list(), whitespace_linter())
},
"'default'"
)
expect_named(linters, "no_tab_linter")
expect_named(linters, "whitespace_linter")

# the same warning is not triggered in modify_defaults
expect_silent({
linters <- modify_defaults(defaults = list(), default = list(), no_tab_linter())
linters <- modify_defaults(defaults = list(), default = list(), whitespace_linter())
})
expect_named(linters, c("default", "no_tab_linter"))
expect_named(linters, c("default", "whitespace_linter"))

# if default= is explicitly provided alongside defaults=, assume that was intentional
default <- Linter(function(.) list())
Expand Down
Loading