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

WIP: Bump minimum needed R version to 3.6 (after R 4.3 release) #985

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
46b8f0c
Also test with parser version 1
IndrajeetPatil Aug 23, 2022
b87f626
Bump minimum needed R version
IndrajeetPatil Aug 23, 2022
bd564c7
Get rid of logic related to parser version 1
IndrajeetPatil Aug 23, 2022
f75a565
Don't update NEWS for now
IndrajeetPatil Aug 23, 2022
7c76c69
Merge branch 'r-lib:main' into test_old_supported_version
IndrajeetPatil Aug 23, 2022
f3c9c09
get rid of more logic for R < 3.6
IndrajeetPatil Aug 23, 2022
64c8701
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Aug 23, 2022
2931c64
Merge branch 'r-lib:main' into test_old_supported_version
IndrajeetPatil Aug 24, 2022
7b60219
Merge branch 'r-lib:main' into test_old_supported_version
IndrajeetPatil Aug 26, 2022
083aaba
Merge branch 'r-lib:main' into test_old_supported_version
IndrajeetPatil Aug 26, 2022
900b780
some more R 3.6 clean up
IndrajeetPatil Sep 9, 2022
9525190
pre-commit
github-actions[bot] Sep 9, 2022
3739ddf
turn off oldrel-4 for now
IndrajeetPatil Sep 9, 2022
5a12099
`relocate_eq_assign()` is no longer needed
IndrajeetPatil Sep 9, 2022
a7fa554
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 11, 2022
5fc30da
always define global variables
lorenzwalthert Sep 11, 2022
475c836
remove its usage, but retain the logic for future
IndrajeetPatil Sep 11, 2022
4f2711b
Merge branch 'r-lib:main' into test_old_supported_version
IndrajeetPatil Sep 11, 2022
979f98a
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 11, 2022
dd9e255
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 13, 2022
0008ec8
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 13, 2022
5263223
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 14, 2022
6cc7b69
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 14, 2022
7beb072
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 20, 2022
3b6f236
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 21, 2022
04dcc78
Update environments.R
IndrajeetPatil Sep 21, 2022
442f1ee
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Sep 28, 2022
583991e
Merge branch 'r-lib:main' into test_old_supported_version
IndrajeetPatil Oct 1, 2022
2eb53d8
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Oct 8, 2022
a94384e
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Oct 17, 2022
5b049c7
Merge branch 'main' into test_old_supported_version
IndrajeetPatil Oct 25, 2022
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 DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ License: MIT + file LICENSE
URL: https://github.com/r-lib/styler, https://styler.r-lib.org
BugReports: https://github.com/r-lib/styler/issues
Depends:
R (>= 3.5.0)
R (>= 3.6.0)
Imports:
cli (>= 3.1.1),
magrittr (>= 2.0.0),
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ editor_options:
formats (`.R`, `.Rmd`, `.Rmarkdown`, `.Rnw`, and `.qmd`) in the (package)
directory (\@IndrajeetPatil, #965).
- `style_pkg()` now excludes the auto-generated `R/cpp11.R` file (#977).

- Minimum needed R version is now bumped to `3.5` (\@IndrajeetPatil, #986).

**Features**
Expand Down
8 changes: 1 addition & 7 deletions R/environments.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,7 @@ parser_version_get <- function() {

#' @rdname parser_version_set
parser_version_find <- function(pd) {
ifelse(any(pd$token == "equal_assign"),
2,
ifelse(any(pd$token == "expr_or_assign_or_help"),
3,
1
)
)
ifelse(any(pd$token == "equal_assign"), 2, 3)
}


Expand Down
2 changes: 1 addition & 1 deletion R/parse.R
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ get_parse_data <- function(text, include_text = TRUE, ...) {
pd <- pd %>%
add_id_and_short()

parser_version_set(parser_version_find(pd))
# parser_version_set(parser_version_find(pd))
IndrajeetPatil marked this conversation as resolved.
Show resolved Hide resolved
pd
}

Expand Down
123 changes: 6 additions & 117 deletions R/relevel.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ flatten_operators <- function(pd_nested) {
#' @keywords internal
flatten_operators_one <- function(pd_nested) {
pd_token_left <- c(special_token, "PIPE", math_token, "'$'")
pd_token_right <- c(
special_token, "PIPE", "LEFT_ASSIGN",
if (parser_version_get() > 1) "EQ_ASSIGN",
"'+'", "'-'", "'~'"
)
pd_token_right <- c(special_token, "PIPE", "LEFT_ASSIGN", "EQ_ASSIGN", "'+'", "'-'", "'~'")
pd_nested %>%
flatten_pd(pd_token_left, left = TRUE) %>%
flatten_pd(pd_token_right, left = FALSE)
Expand Down Expand Up @@ -111,19 +107,11 @@ wrap_expr_in_expr <- function(pd) {
#' Relocate the expressions containing the token `EQ_ASSIGN` within the nested
#' parse table
#'
#' Although syntactically identical, [utils::getParseData()] does not produce
#' the same hierarchy of the parse table (parent and id relationship) for `<-`
#' and `=` (See 'Examples').
#' This is considered to be a bug and causes problems because the
#' nested parse table constructed with [compute_parse_data_nested()] is not
#' consistent if `EQ_ASSIGN` occurs in the expression to style. In particular,
#' `EQ_ASSIGN` and the tokens to its left and right are located too high up in
#' the hierarchy of the nested parse data. Hence, this function wraps the
#' sub-expression into an expression, similar to [wrap_expr_in_curly()].
#' Since `wrap_expr_in_curly()` is called from within a visitor
#' (and `relocate_eq_assign()` not), we need to
#' wrap the the implementation [relocate_eq_assign_nest()] that operates on
#' *nests* into a visitor call.
#' This used to be relevant when `{styler}` supported R versions `< 3.5`, and,
#' therefore, parser version `1`, which had a bug that did not produce the same
#' hierarchy of the parse table (parent and id relationship) for `<-` and `=`
#' (See 'Examples').
#'
#' @param pd A parse table.
#' @examples
#' styler:::get_parse_data("a <- b <- 3")
Expand All @@ -142,104 +130,5 @@ wrap_expr_in_expr <- function(pd) {
#' )
#' @keywords internal
relocate_eq_assign <- function(pd) {
if (parser_version_get() < 2) {
post_visit_one(pd, relocate_eq_assign_nest)
} else {
pd
}
}


#' Relocate all assignment expressions that contain `EQ_ASSIGN` within a *nest*
#'
#' Implements the relocation of an `EQ_ASSIGN` and associated tokens
#' within a *nest* (nested parse table at one level of nesting).
#' Note that one assignment expression (such as "a = b = c") can include
#' multiple assignment operators, an assignment involves just one assignment
#' operator.
#' For the relocation of assignment expressions that contain `EQ_ASSIGN` within
#' a *nest*, we need to first find the expressions that contain `=` and then
#' split the *nest* into parse tables each containing one such assignment
#' expression and then relocate each of them separately.
#' We can't do all of them together because:
#'
#' * An assignment can contain more than just three tokens, e.g. (a <- b <- c).
#' * Two assignments can be in the same nest although they don't belong to the
#' same assignment (if-else statement).
#'
#' Please refer to the section 'Examples' in [relocate_eq_assign()] for details.
#' @param pd A parse table.
#' @importFrom rlang seq2
#' @keywords internal
relocate_eq_assign_nest <- function(pd) {
idx_eq_assign <- which(pd$token == "EQ_ASSIGN")
if (length(idx_eq_assign) > 0) {
block_id <- find_block_id(pd)
blocks <- split(pd, block_id)
pd <- map_dfr(blocks, relocate_eq_assign_one)
}
pd
}

#' Find the block to which a token belongs
#'
#' Two assignment tokens `EQ_ASSIGN` belong to the same block if they are not
#' separated by more than one token. Token between `EQ_ASSIGN` tokens belong
#' to the `EQ_ASSIGN` token occurring before them, except the token right before
#' `EQ_ASSING` already belongs to the `EQ_ASSING` after it. Note that this
#' notion is unrelated to the column *block* in the parse table, which is used
#' to [parse_transform_serialize_r()] code blocks and leave out the ones that
#' are cached.
#' @param pd A parse table.
#' @keywords internal
find_block_id <- function(pd) {
idx_eq_assign <- which(pd$token == "EQ_ASSIGN")
eq_belongs_to_block <- c(0, diff(idx_eq_assign) > 2)

empty_seq <- rep(0, nrow(pd))
empty_seq[idx_eq_assign - 1] <- eq_belongs_to_block
block_id <- cumsum(empty_seq)
block_id
}

#' Relocate an assignment expression
#'
#' Relocates an assignment expression within a parse table containing one
#' assignment expression. Note that one assignment can include multiple
#' assignment operators such as "a = b = c".
#' @param pd A parse table with one assignment expression to relocate.
#' @keywords internal
relocate_eq_assign_one <- function(pd) {
idx_eq_assign <- which(pd$token == "EQ_ASSIGN")
eq_ind <- seq2(idx_eq_assign[1] - 1L, last(idx_eq_assign) + 1L)
# initialize because wrap_expr_in_expr -> create_tokens -> requires it
pd$indent <- 0
eq_expr <- pd[eq_ind, ] %>%
wrap_expr_in_expr() %>%
add_line_col_to_wrapped_expr() %>%
remove_attributes(c(
"multi_line", "indention_ref_pos_id",
"newlines", "indent", "spaces", "lag_newlines"
))
eq_expr$id <- NA
eq_expr$parent <- NA
pd$indent <- NULL
non_eq_expr <- pd[-eq_ind, ]
pd <- bind_rows(eq_expr, non_eq_expr) %>%
arrange_pos_id()
pd
}

#' Adds line and col information to an expression from its child
#'
#' @param pd A parse table.
#' @importFrom rlang abort
#' @keywords internal
add_line_col_to_wrapped_expr <- function(pd) {
if (nrow(pd) > 1) abort("pd must be a wrapped expression that has one row.")
pd$line1 <- pd$child[[1]]$line1[1]
pd$line2 <- last(pd$child[[1]]$line2)
pd$col1 <- pd$child[[1]]$col1[1]
pd$col2 <- last(pd$child[[1]]$col2)
pd
}
4 changes: 0 additions & 4 deletions R/style-guides.R
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,6 @@ tidyverse_style <- function(scope = "tokens",
)
)

if (getRversion() < "3.6") {
transformers_drop$token$force_assignment_op <- NULL
}

style_guide_name <- "styler::tidyverse_style@https://github.com/r-lib"
create_style_guide(
# transformer functions
Expand Down
15 changes: 0 additions & 15 deletions man/add_line_col_to_wrapped_expr.Rd

This file was deleted.

21 changes: 0 additions & 21 deletions man/find_block_id.Rd

This file was deleted.

17 changes: 4 additions & 13 deletions man/relocate_eq_assign.Rd

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

33 changes: 0 additions & 33 deletions man/relocate_eq_assign_nest.Rd

This file was deleted.

17 changes: 0 additions & 17 deletions man/relocate_eq_assign_one.Rd

This file was deleted.

5 changes: 1 addition & 4 deletions tests/testthat/test-transformers-drop.R
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ test_that("tidyverse transformers are correctly dropped", {
names_indention <- c("indent_braces", "indent_op", "indent_without_paren")
expect_setequal(names(t_fun$indention), names_indention)

names_tokens <- c(
"fix_quotes",
if (getRversion() < "3.6") "force_assignment_op"
)
names_tokens <- "fix_quotes"
expect_setequal(names(t_fun$token), names_tokens)
})

Expand Down