Skip to content

Commit

Permalink
Merge branch 'main' into chore-parallel-testing
Browse files Browse the repository at this point in the history
  • Loading branch information
IndrajeetPatil committed Dec 3, 2024
2 parents 4c6b048 + 0103bad commit e7d20f2
Show file tree
Hide file tree
Showing 80 changed files with 3,265 additions and 435 deletions.
2 changes: 2 additions & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
^bench$
^tests/testthat/dummy_packages/package/[.]Rbuildignore$
^tests/testthat/dummy_packages/cp1252/[.]Rbuildignore$
testthat-problems[.]rds$
^_pkgdown\.yaml$
^docs$
^pkgdown$
^vignettes/[^-]+.gif$
^CRAN-SUBMISSION$
^CODE_OF_CONDUCT\.md$
^paper$
39 changes: 39 additions & 0 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Contributing to `{lintr}`

This outlines how to propose a change to `{lintr}`. For a detailed discussion on contributing to this, r-lib, and other tidyverse packages, please see the [development contributing guide](https://rstd.io/tidy-contrib) and our [code review principles](https://code-review.tidyverse.org/).

## Fixing typos

You can fix typos, spelling mistakes, or grammatical errors in the documentation directly using the GitHub web interface, as long as the changes are made in the _source_ file. This generally means you'll need to edit [roxygen2 comments](https://roxygen2.r-lib.org/articles/roxygen2.html) in an `.R`, not a `.Rd` file. You can find the `.R` file that generates the `.Rd` by reading the comment in the first line.

## Bigger changes

If you want to make a bigger change, it's a good idea to first file an issue and make sure someone from the team agrees that it’s needed. If you’ve found a bug, please file an issue that illustrates the bug with a minimal [reprex](https://www.tidyverse.org/help/#reprex) (this will also help you write a unit test, if needed). See the tidyverse guide on [how to create a great issue](https://code-review.tidyverse.org/issues/) for more advice.

### Adding a new linter

If you wish to contribute a new linter, the [Creating new linters](https://lintr.r-lib.org/articles/creating_linters.html) article serves as a comprehensive guide.

### Pull request process

* Fork the package and clone onto your computer. If you haven't done this before, we recommend using `usethis::create_from_github("r-lib/lintr", fork = TRUE)`.

* Install all development dependencies with `devtools::install_dev_deps()`, and then make sure the package passes R CMD check by running `devtools::check()`. If R CMD check doesn't pass cleanly, it's a good idea to ask for help before continuing.

* Create a Git branch for your pull request (PR). We recommend using `usethis::pr_init("brief-description-of-change")`. At a minimum, please avoid submitting PRs from your fork's `main` branch` as this can make the review process more complicated.

* Make your changes, commit them to Git, and create a PR using `usethis::pr_push()`. Follow the prompts in your browser to complete the process. Use a concise title for your PR that summarizes the change, and include `Fixes #issue-number` in the PR _description_. Doing so will automatically close the linked issue when the PR is merged. For complicated changes, add a textual overview of what your PR does in the description. Consider breaking up large PRs into a chain of more digestible+focused smaller PRs.

* For user-facing changes, add a bullet appropriately in the top section of `NEWS.md` (i.e. below the first header). Follow the style described in <https://style.tidyverse.org/news.html>. Most importantly, your audience for NEWS items is a package user, i.e., _not_ a package developer.

### Code style

* New code should follow the tidyverse [style guide](https://style.tidyverse.org). You can use the [styler](https://CRAN.R-project.org/package=styler) package to apply these styles.

* We use [roxygen2](https://cran.r-project.org/package=roxygen2), with [Markdown syntax](https://cran.r-project.org/web/packages/roxygen2/vignettes/rd-formatting.html), for documentation.

* We use [testthat](https://cran.r-project.org/package=testthat) for unit tests. Contributions with test cases included are easier to accept.

## Code of Conduct

Please note that the lintr project is released with a [Contributor Code of Conduct](CODE_OF_CONDUCT.md). By contributing to this project you agree to abide by its terms.
1 change: 1 addition & 0 deletions .github/workflows/R-CMD-check-hard.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ jobs:

- uses: r-lib/actions/setup-r-dependencies@v2
with:
install-quarto: false
pak-version: devel
dependencies: '"hard"'
cache: false
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/R-CMD-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ jobs:

- uses: r-lib/actions/setup-r-dependencies@v2
with:
install-quarto: false
extra-packages: |
any::rcmdcheck
needs: check
Expand Down
30 changes: 30 additions & 0 deletions .github/workflows/draft-pdf.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# TODO: delete this file once the paper is published
on:
push:
branches: [main]
pull_request:
branches: [main]

jobs:
paper:
runs-on: ubuntu-latest
name: Paper Draft
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Build draft PDF
uses: openjournals/openjournals-draft-action@master
with:
journal: joss
# This should be the path to the paper within your repo.
paper-path: paper/paper.md

- name: Upload
uses: actions/upload-artifact@v4
with:
name: paper
# This is the output path where Pandoc will write the compiled
# PDF. Note, this should be the same directory as the input
# paper.md
path: paper/paper.pdf
2 changes: 1 addition & 1 deletion .github/workflows/pkgdown.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:

- name: Deploy to GitHub pages 🚀
if: github.event_name != 'pull_request'
uses: JamesIves/github-pages-deploy-action@v4.6.1
uses: JamesIves/github-pages-deploy-action@v4.7.1
with:
clean: false
branch: gh-pages
Expand Down
5 changes: 4 additions & 1 deletion .lintr
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ linters: all_linters(
options = NULL,
message = "use cli::cli_inform()",
warning = "use cli::cli_warn()",
stop = "use cli::cli_abort()"
stop = "use cli::cli_abort()",
normalizePath = "use normalize_path()"
)),
undesirable_operator_linter(modify_defaults(
defaults = default_undesirable_operators,
Expand All @@ -28,6 +29,8 @@ linters: all_linters(
absolute_path_linter = NULL,
library_call_linter = NULL,
nonportable_path_linter = NULL,
# We now require R>=4.0.0
strings_as_factors_linter = NULL,
todo_comment_linter = NULL,
# TODO(#2327): Enable this.
unreachable_code_linter = NULL
Expand Down
4 changes: 2 additions & 2 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ License: MIT + file LICENSE
URL: https://lintr.r-lib.org, https://github.com/r-lib/lintr
BugReports: https://github.com/r-lib/lintr/issues
Depends:
R (>= 3.6)
R (>= 4.0)
Imports:
backports (>= 1.1.7),
cli (>= 3.4.0),
Expand Down Expand Up @@ -56,7 +56,7 @@ Config/testthat/edition: 3
Config/testthat/parallel: true
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
RoxygenNote: 7.3.2
Collate:
'make_linter_from_xpath.R'
'xp_utils.R'
Expand Down
8 changes: 2 additions & 6 deletions NAMESPACE
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
# Generated by roxygen2: do not edit by hand


if (getRversion() >= "4.0.0") {
importFrom(tools, R_user_dir)
} else {
importFrom(backports, R_user_dir)
}
S3method("[",lints)
S3method(as.data.frame,lints)
S3method(format,lint)
Expand Down Expand Up @@ -182,7 +176,9 @@ importFrom(rex,re_matches)
importFrom(rex,re_substitutes)
importFrom(rex,regex)
importFrom(rex,rex)
importFrom(stats,complete.cases)
importFrom(stats,na.omit)
importFrom(tools,R_user_dir)
importFrom(utils,capture.output)
importFrom(utils,getParseData)
importFrom(utils,getTxtProgressBar)
Expand Down
9 changes: 9 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
* Drop support for posting GitHub comments from inside GitHub comment bot, 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.
* `cyclocomp_linter()` is no longer part of the default linters (#2555, @IndrajeetPatil) because the tidyverse style guide doesn't contain any guidelines on meeting certain complexity requirements. Note that users with `cyclocomp_linter()` in their configs may now need to install {cyclocomp} intentionally, in particular in CI/CD pipelines.
* `scalar_in_linter` is now configurable to allow other `%in%` like operators to be linted. The data.table operator `%chin%` is no longer linted by default; use `in_operators = "%chin%"` to continue linting it. (@F-Noelle)
* `lint()` and friends now normalize paths to forward slashes on Windows (@olivroy, #2613).
* `undesirable_function_linter()`, `undesirable_operator_linter()`, and `list_comparison_linter()` were removed from the tag `efficiency` (@IndrajeetPatil, #2655). If you use `linters_with_tags("efficiency")` to include these linters, you'll need to adjust your config to keep linting your code against them. We did not find any such users on GitHub.


## Bug fixes

Expand All @@ -26,6 +29,7 @@
* `.lintr` configs set by option `lintr.linter_file` or environment variable `R_LINTR_LINTER_FILE` can point to subdirectories (#2512, @MichaelChirico).
* `indentation_linter()` returns `ranges[1L]==1L` when the offending line has 0 spaces (#2550, @MichaelChirico).
* `literal_coercion_linter()` doesn't surface a warning about NAs during coercion for code like `as.integer("a")` (#2566, @MichaelChirico).
* `commented_code_linter()` can detect commented code that ends with a pipe (#2671, @jcken95)

## Changes to default linters

Expand Down Expand Up @@ -57,6 +61,7 @@
* `make_linter_from_xpath()` errors up front when `lint_message` is missing (instead of delaying this error until the linter is used, #2541, @MichaelChirico).
* `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico).
* `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle).
* `lint()` and friends emit a message if no lints are found (#2643, @IndrajeetPatil).

### New linters

Expand All @@ -82,10 +87,14 @@
### Lint accuracy fixes: removing false positives

* `object_name_linter()` and `object_length_linter()` ignore {rlang} name injection like `x |> mutate("{new_name}" := foo(col))` (#1926, @MichaelChirico). No checking is applied in such cases. {data.table} in-place assignments like `DT[, "sPoNGeBob" := "friend"]` are still eligible for lints.
* `object_usage_linter()` finds global variables assigned with `=` or `->`, which avoids some issues around "undefined global variables" in scripts (#2654, @MichaelChirico).

## Notes

* All user-facing messages are now prepared using the `{cli}` package (#2418, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent.
* File locations in lints and error messages contain clickable hyperlinks to improve code navigation (#2645, #2588, @olivroy).
* {lintr} now depends on R version 4.0.0. It already does so implicitly due to recursive upstream dependencies requiring this version; we've simply made that dependency explicit and up-front (#2569, @MichaelChirico).
* Some code with parameters accepting regular expressions is less strict about whether there are capture groups (#2678, @MichaelChirico). In particular, this affects `unreachable_code_linter(allow_comment_regex=)` and `expect_lint(checks=)`.

# lintr 3.1.2

Expand Down
4 changes: 1 addition & 3 deletions R/absolute_path_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
#' * contain at least two path elements, with one having at least two characters and
#' * contain only alphanumeric chars (including UTF-8), spaces, and win32-allowed punctuation
#'
#' @examplesIf getRversion() >= "4.0"
#' # Following examples use raw character constant syntax introduced in R 4.0.
#'
#' @examples
#' # will produce lints
#' lint(
#' text = 'R"--[/blah/file.txt]--"',
Expand Down
4 changes: 4 additions & 0 deletions R/actions.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ in_github_actions <- function() {
identical(Sys.getenv("GITHUB_ACTIONS"), "true")
}

in_pkgdown <- function() {
identical(Sys.getenv("IN_PKGDOWN"), "true")
}

# Output logging commands for any lints found
github_actions_log_lints <- function(lints, project_dir = "") {
for (x in lints) {
Expand Down
8 changes: 4 additions & 4 deletions R/cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ load_cache <- function(file, path = NULL) {
invokeRestart("muffleWarning")
},
error = function(e) {
cli_warn(c(
x = "Could not load cache file {.file {file}}:",
i = conditionMessage(e)
))
cli_warn(
"Could not load cache file {.file {file}}:",
parent = e
)
}
)
} # else nothing to do for source file that has no cache
Expand Down
7 changes: 4 additions & 3 deletions R/class_equals_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ class_equals_linter <- function() {
bad_expr <- xml_find_all(xml_calls, xpath)

operator <- xml_find_chr(bad_expr, "string(*[2])")
lint_message <- sprintf(
"Use inherits(x, 'class-name'), is.<class> or is(x, 'class') instead of comparing class(x) with %s.",
operator
lint_message <- paste0(
"Use inherits(x, 'class-name'), is.<class> for S3 classes, ",
"or is(x, 'S4Class') for S4 classes, ",
"instead of comparing class(x) with ", operator, "."
)
xml_nodes_to_lints(
bad_expr,
Expand Down
5 changes: 3 additions & 2 deletions R/commented_code_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,10 @@ commented_code_linter <- function() {
all_comments <- xml_text(all_comment_nodes)
code_candidates <- re_matches(all_comments, code_candidate_regex, global = FALSE, locations = TRUE)
extracted_code <- code_candidates[, "code"]
# ignore trailing ',' when testing for parsability
extracted_code <- re_substitutes(extracted_code, rex(",", any_spaces, end), "")
# ignore trailing ',' or pipes ('|>', '%>%') when testing for parsability
extracted_code <- re_substitutes(extracted_code, rex(or(",", "|>", "%>%"), any_spaces, end), "")
extracted_code <- re_substitutes(extracted_code, rex(start, any_spaces, ","), "")

is_parsable <- which(vapply(extracted_code, parsable, logical(1L)))

lint_list <- xml_nodes_to_lints(
Expand Down
2 changes: 1 addition & 1 deletion R/exclude.R
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ normalize_exclusions <- function(x, normalize_path = TRUE,
paths[rel_path] <- file.path(root, paths[rel_path])
names(x) <- paths
x <- x[file.exists(paths)] # remove exclusions for non-existing files
names(x) <- normalizePath(names(x)) # get full path for remaining files
names(x) <- normalize_path(names(x)) # get full path for remaining files
}

remove_line_duplicates(
Expand Down
8 changes: 1 addition & 7 deletions R/expect_lint.R
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,10 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
)
# deparse ensures that NULL, list(), etc are handled gracefully
ok <- if (field == "message") {
re_matches(value, check)
re_matches_logical(value, check)
} else {
isTRUE(all.equal(value, check))
}
if (!is.logical(ok)) {
cli_abort(c(
x = "Invalid regex result. Did you mistakenly have a capture group in the regex?",
i = "You can match parentheses with a character class, i.e. inside `[]`."
))
}
testthat::expect(ok, msg)
})
},
Expand Down
2 changes: 1 addition & 1 deletion R/extract.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ extract_r_source <- function(filename, lines, error = identity) {
output <- rep.int(NA_character_, length(lines))

chunks <- tryCatch(get_chunk_positions(pattern = pattern, lines = lines), error = error)
if (inherits(chunks, "error") || inherits(chunks, "lint")) {
if (is_error(chunks) || is_lint(chunks)) {
assign("e", chunks, envir = parent.frame())
# error, so return empty code
return(output)
Expand Down
9 changes: 4 additions & 5 deletions R/get_source_expressions.R
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ get_source_expressions <- function(filename, lines = NULL) {
source_expression$content <- get_content(source_expression$lines)
parsed_content <- get_source_expression(source_expression, error = function(e) lint_parse_error(e, source_expression))

if (inherits(e, "lint") && (is.na(e$line) || !nzchar(e$line) || e$message == "unexpected end of input")) {
if (is_lint(e) && (is.na(e$line) || !nzchar(e$line) || e$message == "unexpected end of input")) {
# Don't create expression list if it's unreliable (invalid encoding or unhandled parse error)
expressions <- list()
} else {
Expand Down Expand Up @@ -502,7 +502,7 @@ get_source_expression <- function(source_expression, error = identity) {
error = error
)

if (inherits(parsed_content, c("error", "lint"))) {
if (is_error(parsed_content) || is_lint(parsed_content)) {
assign("e", parsed_content, envir = parent.frame())
parse_error <- TRUE
}
Expand All @@ -513,7 +513,7 @@ get_source_expression <- function(source_expression, error = identity) {
error = error
)

if (inherits(parsed_content, c("error", "lint"))) {
if (is_error(parsed_content) || is_lint(parsed_content)) {
# Let parse errors take precedence over encoding problems
if (!parse_error) assign("e", parsed_content, envir = parent.frame())
return() # parsed_content is unreliable if encoding is invalid
Expand Down Expand Up @@ -636,8 +636,7 @@ fix_eq_assigns <- function(pc) {
parent = integer(n_expr),
token = character(n_expr),
terminal = logical(n_expr),
text = character(n_expr),
stringsAsFactors = FALSE
text = character(n_expr)
)

for (i in seq_len(n_expr)) {
Expand Down
Loading

0 comments on commit e7d20f2

Please sign in to comment.