Skip to content

Commit

Permalink
Edits to docs for linters (#1480)
Browse files Browse the repository at this point in the history
* Edits to docs for linters

* unneeded_concatenation_linter docs

* replacement `c()` with single R6

* more edits

* More details for duplicate_argument_linter

* more edits

* more edits

* remaining

* a few more code elements

* minor

* address code review comments

* alternative phrasing for duplicated arg

* fix docs

* address more review comments
  • Loading branch information
IndrajeetPatil authored Sep 19, 2022
1 parent a32e85e commit b9f0123
Show file tree
Hide file tree
Showing 75 changed files with 139 additions and 119 deletions.
2 changes: 1 addition & 1 deletion R/aaa.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ NULL
#' Available linters
#' @name linters
#'
#' @description A variety of linters is available in \pkg{lintr}. The most popular ones are readily
#' @description A variety of linters are available in \pkg{lintr}. The most popular ones are readily
#' accessible through [default_linters()].
#'
#' Within a [lint()] function call, the linters in use are initialized with the provided
Expand Down
9 changes: 4 additions & 5 deletions R/any_duplicated_linter.R
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
#' Require usage of anyDuplicated() > 0 over any(duplicated(.))
#' Require usage of `anyDuplicated(x) > 0` over `any(duplicated(x))`
#'
#' [anyDuplicated()] exists as a replacement for `any(duplicated(.))` which is
#' more efficient for simple objects, and in the worst case is the same
#' efficiency. Therefore it should be used in all situations instead of the
#' latter.
#' [anyDuplicated()] exists as a replacement for `any(duplicated(.))`, which is
#' more efficient for simple objects, and is at worst equally efficient.
#' Therefore, it should be used in all situations instead of the latter.
#'
#' Also match usage like `length(unique(x$col)) == nrow(x)`, which can
#' be replaced by `anyDuplicated(x$col) == 0L`.
Expand Down
8 changes: 4 additions & 4 deletions R/any_is_na_linter.R
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#' Require usage of anyNA over any(is.na(.))
#' Require usage of `anyNA(x)` over `any(is.na(x))`
#'
#' [anyNA()] exists as a replacement for `any(is.na(.))` which is more efficient
#' for simple objects, and in the worst case is the same efficiency. Therefore
#' it should be used in all situations instead of the latter.
#' [anyNA()] exists as a replacement for `any(is.na(x))` which is more efficient
#' for simple objects, and is at worst equally efficient.
#' Therefore, it should be used in all situations instead of the latter.
#'
#' @evalRd rd_tags("any_is_na_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
Expand Down
2 changes: 1 addition & 1 deletion R/class_equals_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Block comparison of class with ==
#' Block comparison of class with `==`
#'
#' Usage like `class(x) == "character"` is prone to error since class in R
#' is in general a vector. The correct version for S3 classes is [inherits()]:
Expand Down
3 changes: 2 additions & 1 deletion R/closed_curly_linter.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#' Closed curly linter
#'
#' Check that closed curly braces are on their own line unless they follow an else, comma, or closing bracket.
#' Check that closed curly braces are on their own line unless they follow
#' an else, a comma, or a closing bracket.
#'
#' @param allow_single_line if `TRUE`, allow an open and closed curly pair on the same line.
#' @evalRd rd_tags("closed_curly_linter")
Expand Down
2 changes: 1 addition & 1 deletion R/condition_message_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Block usage of paste() and paste0() with messaging functions using ...
#' Block usage of `paste()` and `paste0()` with messaging functions using `...`
#'
#' `stop(paste0(...))` is strictly redundant -- `stop(...)` is equivalent.
#' `stop(...)` is also preferable to `stop(paste(...))`. The same applies to
Expand Down
2 changes: 1 addition & 1 deletion R/conjunct_test_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Force && conditions in expect_true(), expect_false() to be written separately
#' Force `&&` conditions in `expect_true()` and `expect_false()` to be written separately
#'
#' For readability of test outputs, testing only one thing per call to
#' [testthat::expect_true()] is preferable, i.e.,
Expand Down
4 changes: 2 additions & 2 deletions R/cyclocomp_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
#'
#' Check for overly complicated expressions. See [cyclocomp::cyclocomp()].
#'
#' @param complexity_limit expressions with a cyclomatic complexity higher than this are linted, defaults to 15.
#' See [cyclocomp::cyclocomp()].
#' @param complexity_limit Maximum cyclomatic complexity, default 15. Expressions more complex
#' than this are linted. See [cyclocomp::cyclocomp()].
#' @evalRd rd_tags("cyclocomp_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
#' @importFrom cyclocomp cyclocomp
Expand Down
6 changes: 5 additions & 1 deletion R/duplicate_argument_linter.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#' Duplicate argument linter
#'
#' Check for duplicate arguments in function calls.
#' Check for duplicate arguments in function calls. Some cases are run-time errors
#' (e.g. `mean(x = 1:5, x = 2:3)`), otherwise this linter is used to discourage
#' explicitly providing duplicate names to objects (e.g. `c(a = 1, a = 2)`).
#' Duplicate-named objects are hard to work with programmatically and
#' should typically be avoided.
#'
#' @param except a character vector of function names as exceptions.
#' @evalRd rd_tags("duplicate_argument_linter")
Expand Down
4 changes: 2 additions & 2 deletions R/equals_na_linter.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#' Equality check with NA linter
#'
#' Check for `x == NA` and `x != NA`
#'
#' Check for `x == NA` and `x != NA`. Such usage is almost surely incorrect --
#' checks for missing values should be done with [is.na()].
#' @evalRd rd_tags("equals_na_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
#' @export
Expand Down
2 changes: 1 addition & 1 deletion R/expect_comparison_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of expect_gt(x, y) over expect_true(x > y) (and similar)
#' Require usage of `expect_gt(x, y)` over `expect_true(x > y)` (and similar)
#'
#' [testthat::expect_gt()], [testthat::expect_gte()], [testthat::expect_lt()],
#' [testthat::expect_lte()], and [testthat::expect_equal()] exist specifically
Expand Down
8 changes: 4 additions & 4 deletions R/expect_identical_linter.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#' Require usage of expect_identical(x, y) where appropriate
#' Require usage of `expect_identical(x, y)` where appropriate
#'
#' At Google, [testthat::expect_identical()] should be the default/go-to function for
#' comparing an output to an expected value. `expect_true(identical(x, y))`
#' This linter enforces the usage of [testthat::expect_identical()] as the
#' default expectation for comparisons in a testthat suite. `expect_true(identical(x, y))`
#' is an equivalent but unadvised method of the same test. Further,
#' [testthat::expect_equal()] should only be used when `expect_identical()`
#' is inappropriate, i.e., when `x` and `y` need only be *numerically
Expand All @@ -17,7 +17,7 @@
#' 1. A named argument is set (e.g. `ignore_attr` or `tolerance`)
#' 2. Comparison is made to an explicit decimal, e.g.
#' `expect_equal(x, 1.0)` (implicitly setting `tolerance`)
#' 3. `...` is passed (wrapper functions whcih might set
#' 3. `...` is passed (wrapper functions which might set
#' arguments such as `ignore_attr` or `tolerance`)
#'
#' @evalRd rd_tags("expect_identical_linter")
Expand Down
2 changes: 1 addition & 1 deletion R/expect_length_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of expect_length(x, n) over expect_equal(length(x), n)
#' Require usage of `expect_length(x, n)` over `expect_equal(length(x), n)`
#'
#' [testthat::expect_length()] exists specifically for testing the [length()] of
#' an object. [testthat::expect_equal()] can also be used for such tests,
Expand Down
2 changes: 1 addition & 1 deletion R/expect_named_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of expect_named(x, n) over expect_equal(names(x), n)
#' Require usage of `expect_named(x, n)` over `expect_equal(names(x), n)`
#'
#' [testthat::expect_named()] exists specifically for testing the [names()] of
#' an object. [testthat::expect_equal()] can also be used for such tests,
Expand Down
2 changes: 1 addition & 1 deletion R/expect_not_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of expect_false(.) over expect_true(!.)
#' Require usage of `expect_false(x)` over `expect_true(!x)`
#'
#' [testthat::expect_false()] exists specifically for testing that an output is
#' `FALSE`. [testthat::expect_true()] can also be used for such tests by
Expand Down
2 changes: 1 addition & 1 deletion R/expect_null_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' expect_null Linter
#' Require usage of `expect_null` for checking `NULL`
#'
#' Require usage of `expect_null(x)` over `expect_equal(x, NULL)` and similar
#' usages.
Expand Down
4 changes: 2 additions & 2 deletions R/expect_s3_class_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of expect_s3_class()
#' Require usage of `expect_s3_class()`
#'
#' [testthat::expect_s3_class()] exists specifically for testing the class
#' of S3 objects. [testthat::expect_equal()], [testthat::expect_identical()],
Expand Down Expand Up @@ -69,7 +69,7 @@ is_s3_class_calls <- paste0("is.", c(
"mts", "stepfun", "ts", "tskernel"
))

#' Require usage of expect_s4_class(x, k) over expect_true(is(x, k))
#' Require usage of `expect_s4_class(x, k)` over `expect_true(is(x, k))`
#'
#' [testthat::expect_s4_class()] exists specifically for testing the class
#' of S4 objects. [testthat::expect_true()] can also be used for such tests,
Expand Down
2 changes: 1 addition & 1 deletion R/expect_true_false_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of expect_true(x) over expect_equal(x, TRUE)
#' Require usage of `expect_true(x)` over `expect_equal(x, TRUE)`
#'
#' [testthat::expect_true()] and [testthat::expect_false()] exist specifically
#' for testing the `TRUE`/`FALSE` value of an object.
Expand Down
2 changes: 1 addition & 1 deletion R/expect_type_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of expect_type(x, type) over expect_equal(typeof(x), type)
#' Require usage of `expect_type(x, type)` over `expect_equal(typeof(x), type)`
#'
#' [testthat::expect_type()] exists specifically for testing the storage type
#' of objects. [testthat::expect_equal()], [testthat::expect_identical()], and
Expand Down
2 changes: 1 addition & 1 deletion R/fixed_regex_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#'
#' NB: for `stringr` functions, that means wrapping the pattern in `stringr::fixed()`.
#'
#' NB: This linter is likely not able to distinguish every possible case when
#' NB: this linter is likely not able to distinguish every possible case when
#' a fixed regular expression is preferable, rather it seeks to identify
#' likely cases. It should _never_ report false positives, however; please
#' report false positives as an error.
Expand Down
4 changes: 3 additions & 1 deletion R/function_left_parentheses_linter.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#' Function left parentheses linter
#'
#' Check that all left parentheses in a function call do not have spaces before them.
#' Check that all left parentheses in a function call do not have spaces before them
#' (e.g. `mean (1:3)`). Although this is syntactically valid, it makes the code
#' difficult to read.
#'
#' @evalRd rd_tags("function_left_parentheses_linter")
#' @seealso
Expand Down
2 changes: 1 addition & 1 deletion R/ifelse_censor_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Block usage of ifelse where pmin or pmax is more appropriate
#' Block usage of `ifelse()` where `pmin()` or `pmax()` is more appropriate
#'
#' `ifelse(x > M, M, x)` is the same as `pmin(x, M)`, but harder
#' to read and requires several passes over the vector.
Expand Down
6 changes: 3 additions & 3 deletions R/inner_combine_linter.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#' Require c() to be applied before relatively expensive vectorized functions
#' Require `c()` to be applied before relatively expensive vectorized functions
#'
#' `as.Date(c(a, b))` is logically equivalent to `c(as.Date(a), as.Date(b))`;
#' ditto for the equivalence of several other vectorized functions like
#' `as.Date(c(a, b))` is logically equivalent to `c(as.Date(a), as.Date(b))`.
#' The same equivalence holds for several other vectorized functions like
#' [as.POSIXct()] and math functions like [sin()]. The former is to be
#' preferred so that the most expensive part of the operation ([as.Date()])
#' is applied only once.
Expand Down
3 changes: 2 additions & 1 deletion R/missing_argument_linter.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#' Missing argument linter
#'
#' Check for missing arguments in function calls.
#' Check for missing arguments in function calls (e.g. `stats::median(1:10, )`).
#'
#' @param except a character vector of function names as exceptions.
#' @param allow_trailing always allow trailing empty arguments?
#' @evalRd rd_tags("missing_argument_linter")
Expand Down
10 changes: 5 additions & 5 deletions R/nested_ifelse_linter.R
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
#' Block usage of nested ifelse() calls
#' Block usage of nested `ifelse()` calls
#'
#' Calling `ifelse` in nested calls is problematic for two main reasons:
#' Calling [ifelse()] in nested calls is problematic for two main reasons:
#' 1. It can be hard to read -- mapping the code to the expected output
#' for such code can be a messy task/require a lot of mental bandwidth,
#' especially for code that nests more than once
#' 2. It is inefficient -- `ifelse` can evaluate _all_ of its arguments at
#' both yes and no (see https://stackoverflow.com/q/16275149); this issue
#' 2. It is inefficient -- `ifelse()` can evaluate _all_ of its arguments at
#' both yes and no (see <https://stackoverflow.com/q/16275149>); this issue
#' is exacerbated for nested calls
#'
#' Users can instead rely on a more readable alternative modeled after SQL
#' CASE WHEN statements, such as `data.table::fcase` or `dplyr::case_when`,
#' CASE WHEN statements, such as `data.table::fcase()` or `dplyr::case_when()`,
#' or use a look-up-and-merge approach (build a mapping table between values
#' and outputs and merge this to the input).
#'
Expand Down
4 changes: 2 additions & 2 deletions R/object_usage_linter.R
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#' Object usage linter
#'
#' Check that closures have the proper usage using [codetools::checkUsage()].
#' Note that this runs [base::eval()] on the code, so do not use with untrusted code.
#' Note that this runs [base::eval()] on the code, so **do not use with untrusted code**.
#'
#' @param interpret_glue If TRUE, interpret [glue::glue()] calls to avoid false positives caused by local variables
#' @param interpret_glue If `TRUE`, interpret [glue::glue()] calls to avoid false positives caused by local variables
#' which are only used in a glue expression.
#'
#' @evalRd rd_tags("object_usage_linter")
Expand Down
2 changes: 1 addition & 1 deletion R/outer_negation_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of !any(.) over all(!.), !all(.) over any(!.)
#' Require usage of `!any(x)` over `all(!x)`, `!all(x)` over `any(!x)`
#'
#' `any(!x)` is logically equivalent to `!any(x)`; ditto for the equivalence of
#' `all(!x)` and `!any(x)`. Negating after aggregation only requires inverting
Expand Down
1 change: 1 addition & 0 deletions R/paren_brace_linter.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#' Parentheses before brace linter
#'
#' Check that there is a space between right parentheses and an opening curly brace.
#' For example, `function(){}` doesn't have a space, while `function() {}` does.
#'
#' @evalRd rd_tags("paren_brace_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
Expand Down
2 changes: 1 addition & 1 deletion R/paste_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Raise lints for several common poor usages of paste()
#' Raise lints for several common poor usages of `paste()`
#'
#' The following issues are linted by default by this linter
#' (and each can be turned off optionally):
Expand Down
4 changes: 2 additions & 2 deletions R/redundant_ifelse_linter.R
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#' Prevent ifelse() from being used to produce TRUE/FALSE or 1/0
#' Prevent `ifelse()` from being used to produce `TRUE`/`FALSE` or `1`/`0`
#'
#' Expressions like `ifelse(x, TRUE, FALSE)` and `ifelse(x, FALSE, TRUE)` are
#' redundant; just `x` or `!x` suffice in R code where logical vectors are a
#' core data structure. `ifelse(x, 1, 0)` is also `as.numeric(x)`, but even
#' this should only be needed rarely.
#' this should be needed only rarely.
#'
#' @evalRd rd_tags("redundant_ifelse_linter")
#' @param allow10 Logical, default `FALSE`. If `TRUE`, usage like
Expand Down
2 changes: 1 addition & 1 deletion R/regex_subset_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of direct methods for subsetting strings via regex.
#' Require usage of direct methods for subsetting strings via regex
#'
#' Using `value = TRUE` in [grep()] returns the subset of the input that matches
#' the pattern, e.g. `grep("[a-m]", letters, value = TRUE)` will return the
Expand Down
4 changes: 2 additions & 2 deletions R/sprintf_linter.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#' `sprintf` linter
#' Require correct `sprintf()` calls
#'
#' Check for an inconsistent number of arguments or arguments with incompatible types (for literal arguments) in
#' `sprintf` calls.
#' `sprintf()` calls.
#'
#' @evalRd rd_tags("sprintf_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
Expand Down
4 changes: 2 additions & 2 deletions R/string_boundary_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Require usage of startsWith() and endsWith() over grepl()/substr() versions
#' Require usage of `startsWith()` and `endsWith()` over `grepl()`/`substr()` versions
#'
#' [startsWith()] is used to detect fixed initial substrings; it is more
#' readable and more efficient than equivalents using [grepl()] or [substr()].
Expand All @@ -15,7 +15,7 @@
#'
#' We lint `grepl()` usages by default because the `!is.na()` version is more explicit
#' with respect to `NA` handling -- though documented, the way `grepl()` handles
#' missing inputs may be surprising to some readers.
#' missing inputs may be surprising to some users.
#'
#' @param allow_grepl Logical, default `FALSE`. If `TRUE`, usages with `grepl()`
#' are ignored. Some authors may prefer the `NA` input to `FALSE` output
Expand Down
2 changes: 1 addition & 1 deletion R/strings_as_factors_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Identify cases where stringsAsFactors should be supplied explicitly
#' Identify cases where `stringsAsFactors` should be supplied explicitly
#'
#' Designed for code bases written for versions of R before 4.0 seeking to upgrade to R >= 4.0, where
#' one of the biggest pain points will surely be the flipping of the
Expand Down
2 changes: 1 addition & 1 deletion R/system_file_linter.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#' Block usage of file.path() with system.file()
#' Block usage of `file.path()` with `system.file()`
#'
#' [system.file()] has a `...` argument which, internally, is passed to
#' [file.path()], so including it in user code is repetitive.
Expand Down
4 changes: 2 additions & 2 deletions R/undesirable_function_linter.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#' Undesirable function linter
#'
#' Report the use of undesirable functions, e.g. [base::return()], [base::options()], or
#' [base::sapply()] and suggest an alternative.
#' Report the use of undesirable functions (e.g. [base::return()], [base::options()], or
#' [base::sapply()]) and suggest an alternative.
#'
#' @param fun Named character vector. `names(fun)` correspond to undesirable functions,
#' while the values give a description of why the function is undesirable.
Expand Down
4 changes: 3 additions & 1 deletion R/unneeded_concatenation_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#' @param allow_single_expression Logical, default `TRUE`. If `FALSE`, one-expression
#' usages of `c()` are always linted, e.g. `c(x)` and `c(matrix(...))`. In some such
#' cases, `c()` is being used for its side-effect of stripping non-name attributes;
#' it is usually preferable to use [as.vector()] to accomplish the same more readably.
#' it is usually preferable to use the more readable [as.vector()] instead.
#' [as.vector()] is not always preferable, for example with environments
#' (especially, `R6` objects), in which case `list()` is the better alternative.
#'
#' @evalRd rd_tags("unneeded_concatenation_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
Expand Down
9 changes: 4 additions & 5 deletions man/any_duplicated_linter.Rd

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

8 changes: 4 additions & 4 deletions man/any_is_na_linter.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/class_equals_linter.Rd

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

Loading

0 comments on commit b9f0123

Please sign in to comment.