Skip to content

Commit

Permalink
allow ]}, update NEWS, incorporate feedback, fix lint
Browse files Browse the repository at this point in the history
  • Loading branch information
AshesITR committed Apr 26, 2022
1 parent a251c1e commit 733c4e1
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 5 deletions.
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* Removed deprecated functions `absolute_paths_linter`, `camel_case_linter`, `multiple_dots_linter`, `snake_case_linter`, and `trailing_semicolons_linter`. They have been marked as deprecated since v1.0.1, which was released in 2017.
* Rename `semicolon_terminator_linter` to `semicolon_linter` for better consistency. `semicolon_terminator_linter` survives but is marked for deprecation. The new linter also has a new signature, taking arguments `allow_compound` and `allow_trailing` to replace the old single argument `semicolon=`, again for signature consistency with other linters.
* Combined several curly brace related linters into a new `brace_linter`:
+ `closed_curly_linter()`
+ `closed_curly_linter()`, also allowing `}]` in addition to `})` and `},` as exceptions.
* The `...` arguments for `lint()`, `lint_dir()`, and `lint_package()` have promoted to an earlier position to better match the [Tidyverse design principal](https://design.tidyverse.org/args-data-details.html) of data->descriptor->details. This change enables passing objects to `...` without needing to specify non-required arguments, e.g. `lint_dir("/path/to/dir", linter())` now works without the need to specify `relative_path`. This affects some code that uses positional arguments. (#935, @michaelchirico)
+ For `lint()`, `...` is now the 3rd argument, where earlier this was `cache=`
+ For `lint_dir()` and `lint_package()`, `...` is now the 2nd argument, where earlier this was `relative_path=`
Expand Down
10 changes: 7 additions & 3 deletions R/brace_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ brace_linter <- function(allow_single_line = FALSE) {
if (isTRUE(allow_single_line)) {
"(@line1 != preceding-sibling::OP-LEFT-BRACE/@line1)"
},
# immediately followed by "," or ")"
"not(@line1 = ancestor::expr/following-sibling::*[1][self::OP-COMMA or self::OP-RIGHT-PAREN]/@line1)",
# immediately followed by ",", "]" or ")"
"not(
@line1 = ancestor::expr/following-sibling::*[1][
self::OP-COMMA or self::OP-RIGHT-BRACKET or self::OP-RIGHT-PAREN
]/@line1
)",
# double curly
"not(
(@line1 = parent::expr/following-sibling::OP-RIGHT-BRACE/@line1) or
Expand All @@ -33,7 +37,7 @@ brace_linter <- function(allow_single_line = FALSE) {

xp_closed_curly <- glue::glue("//OP-RIGHT-BRACE[
{ xp_cond_closed } and (
(@line1 = preceding-sibling::*/@line2) or
(@line1 = preceding-sibling::*[1]/@line2) or
(@line1 = parent::expr/following-sibling::*[1][not(self::ELSE)]/@line1)
)
]")
Expand Down
3 changes: 2 additions & 1 deletion R/closed_curly_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ closed_curly_linter <- function(allow_single_line = FALSE) {
"unless they are followed by an else."
),
line = source_file$lines[as.character(parsed$line1)]
)}
)
}
}
)
})
Expand Down
2 changes: 2 additions & 0 deletions tests/testthat/test-brace_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ test_that("brace_linter lints closed braces correctly", {

# }) is allowed
expect_lint("eval(bquote({...}))", NULL, linter)
# }] is too
expect_lint("df[, {...}]", NULL, linter)

# }, is allowed
expect_lint(
Expand Down

0 comments on commit 733c4e1

Please sign in to comment.