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.

Sign up for GitHub

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

fix nested tidy function call indentation #1948

Merged
merged 4 commits into from
Apr 4, 2023
Merged

fix nested tidy function call indentation #1948

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f5997ba
fix nested tidy function call indentation
AshesITR Apr 3, 2023
37d6bb3
restrict suppression to function calls for now
AshesITR Apr 3, 2023
0bf4029
more thorough tests
AshesITR Apr 4, 2023
9f252da
Merge branch 'main' into fix/1898-multiline-hanging
AshesITR Apr 4, 2023
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 NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@

* `routine_registration_linter()` for identifying native routines that don't use registration (`useDynLib` in the `NAMESPACE`; @MichaelChirico)

* `indentation_linter()` for checking that the indentation conforms to 2-space Tidyverse-style (@AshesITR and @dgkf, #1411, #1792).
* `indentation_linter()` for checking that the indentation conforms to 2-space Tidyverse-style (@AshesITR and @dgkf, #1411, #1792, #1898).

* `unnecessary_nested_if_linter()` for checking unnecessary nested `if` statements where a single
`if` statement with appropriate conditional expression would suffice (@IndrajeetPatil and @AshesITR, #1778).
Expand Down
19 changes: 18 additions & 1 deletion R/indentation_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,9 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al
}

find_new_indent <- function(current_indent, change_type, indent, hanging_indent) {
if (change_type == "hanging") {
if (change_type == "suppress") {
current_indent
} else if (change_type == "hanging") {
hanging_indent
} else if (change_type == "double") {
current_indent + 2L * indent
Expand All @@ -311,6 +313,7 @@ build_indentation_style_tidy <- function() {
paren_tokens_left <- c("OP-LEFT-BRACE", "OP-LEFT-PAREN", "OP-LEFT-BRACKET", "LBB")
paren_tokens_right <- c("OP-RIGHT-BRACE", "OP-RIGHT-PAREN", "OP-RIGHT-BRACKET", "OP-RIGHT-BRACKET")
xp_last_on_line <- "@line1 != following-sibling::*[not(self::COMMENT)][1]/@line1"
xp_inner_expr <- "preceding-sibling::*[1][self::expr and expr[SYMBOL_FUNCTION_CALL]]/*[not(self::COMMENT)]"

# double indent is tidyverse style for function definitions
# triggered only if the closing parenthesis of the function definition is not on its own line and the opening
Expand All @@ -333,6 +336,18 @@ build_indentation_style_tidy <- function() {
parent::expr[FUNCTION and not(@line1 = SYMBOL_FORMALS/@line1)]
/OP-RIGHT-PAREN[@line1 = preceding-sibling::*[not(self::COMMENT)][1]/@line2]
"

xp_suppress <- paste(
glue::glue("
self::{paren_tokens_left}[
@line1 = following-sibling::{paren_tokens_right}/{xp_inner_expr}[position() = 1]/@line1
]/following-sibling::{paren_tokens_right}[
@line1 > {xp_inner_expr}[position() = last() - 1]/@line2
]"
),
collapse = " | "
)

xp_is_not_hanging <- paste(
c(
glue::glue(
Expand All @@ -346,6 +361,8 @@ build_indentation_style_tidy <- function() {
function(change) {
if (length(xml2::xml_find_first(change, xp_is_double_indent)) > 0L) {
"double"
} else if (length(xml2::xml_find_first(change, xp_suppress)) > 0L) {
"suppress"
} else if (length(xml2::xml_find_first(change, xp_is_not_hanging)) == 0L) {
"hanging"
} else {
Expand Down
11 changes: 11 additions & 0 deletions tests/testthat/test-indentation_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,17 @@ test_that("hanging_indent_stlye works", {
expect_lint(code_hanging_same_line, NULL, tidy_linter)
expect_lint(code_hanging_same_line, NULL, hanging_linter)
expect_lint(code_hanging_same_line, "Indent", non_hanging_linter)

# regression test for #1898
expect_lint(
trim_some("
outer_fun(inner_fun(x,
one_indent = 42L
))
"),
NULL,
tidy_linter
)
IndrajeetPatil marked this conversation as resolved.
Show resolved Hide resolved
})

test_that("assignment_as_infix works", {
Expand Down