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

Use | more judiciously #2028

Merged
merged 8 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
6 changes: 3 additions & 3 deletions R/function_argument_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@
#' - <https://design.tidyverse.org/required-no-defaults.html>
#' @export
function_argument_linter <- function() {
xpath <- paste(collapse = " | ", glue::glue("
//{c('FUNCTION', 'OP-LAMBDA')}
xpath <- "
(//FUNCTION | //OP-LAMBDA)
/following-sibling::EQ_FORMALS[1]
/following-sibling::SYMBOL_FORMALS[
text() != '...'
and not(following-sibling::*[not(self::COMMENT)][1][self::EQ_FORMALS])
]
"))
"

Linter(function(source_expression) {
if (!is_lint_level(source_expression, "expression")) {
Expand Down
7 changes: 4 additions & 3 deletions R/indentation_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,16 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al
")"
)

global_nodes <- function(nodes) paste0("//", nodes, collapse = "|") # nolint: object_usage. TODO(#2029): Fix this.
xp_indent_changes <- paste(
c(
glue::glue("//{paren_tokens_left}[not(@line1 = following-sibling::expr[
@line2 > @line1 and
({xp_or(paste0('descendant::', paren_tokens_left, '[', xp_last_on_line, ']'))})
]/@line1)]"),
glue::glue("//{infix_tokens}[{xp_last_on_line}{infix_condition}]"),
glue::glue("//{no_paren_keywords}[{xp_last_on_line}]"),
glue::glue("//{keyword_tokens}/following-sibling::OP-RIGHT-PAREN[
glue::glue("({ global_nodes(infix_tokens) })[{xp_last_on_line}{infix_condition}]"),
glue::glue("({ global_nodes(no_paren_keywords) })[{xp_last_on_line}]"),
glue::glue("({ global_nodes(keyword_tokens) })/following-sibling::OP-RIGHT-PAREN[
{xp_last_on_line} and
not(following-sibling::expr[1][OP-LEFT-BRACE])
]")
Expand Down
5 changes: 3 additions & 2 deletions R/infix_spaces_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@
# NB: parent::*[count(expr | SYMBOL_SUB)) > 1] for the unary case, e.g. x[-1]
# SYMBOL_SUB for case with missing argument like alist(a =)
# NB: the last not() disables lints inside box::use() declarations
xpath <- paste(collapse = "|", glue::glue("//{infix_tokens}[
global_xpath <- paste0('//', infix_tokens, collapse = '|')

Check warning on line 150 in R/infix_spaces_linter.R

View workflow job for this annotation

GitHub Actions / lint

file=R/infix_spaces_linter.R,line=150,col=26,[quotes_linter] Only use double-quotes.

Check warning on line 150 in R/infix_spaces_linter.R

View workflow job for this annotation

GitHub Actions / lint

file=R/infix_spaces_linter.R,line=150,col=57,[quotes_linter] Only use double-quotes.

Check warning on line 150 in R/infix_spaces_linter.R

View workflow job for this annotation

GitHub Actions / lint-changed-files

file=R/infix_spaces_linter.R,line=150,col=26,[quotes_linter] Only use double-quotes.

Check warning on line 150 in R/infix_spaces_linter.R

View workflow job for this annotation

GitHub Actions / lint-changed-files

file=R/infix_spaces_linter.R,line=150,col=57,[quotes_linter] Only use double-quotes.
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
xpath <- glue::glue("({global_xpath})[
parent::*[count(expr | SYMBOL_SUB) > 1]
and (
(
Expand All @@ -166,7 +167,7 @@
]
]
)
]"))
]")

Linter(function(source_expression) {
if (!is_lint_level(source_expression, "expression")) {
Expand Down
18 changes: 8 additions & 10 deletions R/pipe_continuation_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,17 @@ pipe_continuation_linter <- function() {
# Where a single-line pipeline is nested inside a larger expression
# e.g. inside a function definition), the outer expression can span multiple lines
# without throwing a lint.

pipe_conditions <- "
preceding_pipe <- "preceding-sibling::expr[1]/descendant::*[self::SPECIAL[text() = '%>%'] or self::PIPE]"
xpath <- glue::glue("
(//PIPE | //SPECIAL[text() = '%>%'])[
parent::expr[@line1 < @line2]
and preceding-sibling::expr/descendant-or-self::*[self::SPECIAL[text() = '%>%'] or self::PIPE]
and {preceding_pipe}
and (
preceding-sibling::expr/descendant-or-self::expr/@line2
= following-sibling::expr/descendant-or-self::expr/@line1
or @line1 = preceding-sibling::expr/descendant-or-self::*[self::SPECIAL[text() = '%>%'] or self::PIPE]/@line1
preceding-sibling::expr[1]/descendant-or-self::expr/@line2
= following-sibling::expr[1]/descendant-or-self::expr/@line1
or @line1 = {preceding_pipe}/@line1
)
"
xpath <- glue::glue("
//SPECIAL[text() = '%>%' and {pipe_conditions} ]
| //PIPE[ {pipe_conditions} ]
]
")

Linter(function(source_expression) {
Expand Down
11 changes: 6 additions & 5 deletions R/redundant_equals_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@
#' - [outer_negation_linter()]
#' @export
redundant_equals_linter <- function() {
xpath <- paste0(
c("//EQ", "//NE"),
"/parent::expr/expr[NUM_CONST[text() = 'TRUE' or text() = 'FALSE']]/parent::expr",
collapse = " | "
)
xpath <- "
(//EQ | //NE)
/parent::expr
/expr[NUM_CONST[text() = 'TRUE' or text() = 'FALSE']]
/parent::expr
"

Linter(function(source_expression) {
if (!is_lint_level(source_expression, "expression")) {
Expand Down
7 changes: 3 additions & 4 deletions R/vector_logic_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ vector_logic_linter <- function() {
# <expr> ... </expr>
# </expr>
# we _don't_ want to match anything on the second expr, hence this
xpath_parts <- glue::glue("
//{ c('AND', 'OR') }[
xpath <- "
(//AND | //OR)[
ancestor::expr[
not(preceding-sibling::OP-RIGHT-PAREN)
and preceding-sibling::*[
Expand All @@ -75,8 +75,7 @@ vector_logic_linter <- function() {
or preceding-sibling::OP-LEFT-BRACKET
])
]
")
xpath <- paste(xpath_parts, collapse = " | ")
"

Linter(function(source_expression) {
if (!is_lint_level(source_expression, "expression")) {
Expand Down
Loading