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

Adapt to R devel parser #441

Merged
merged 6 commits into from
Nov 18, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Collate:
'communicate.R'
'compat-dplyr.R'
'compat-tidyr.R'
'environments.R'
'expr-is.R'
'indent.R'
'initialize.R'
Expand Down
50 changes: 50 additions & 0 deletions R/environments.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#' Work with parser versions
#'
#' The structure of the parse data affects many operations in styler. There was
#' unexpected behaviour of the parser that styler was initially designed to work
#' around. Examples are [#187](https://github.com/r-lib/styler/issues/187),
#' [#216](https://github.com/r-lib/styler/issues/216),
#' [#100](https://github.com/r-lib/styler/issues/100) and others. With
#' [#419](https://github.com/r-lib/styler/issues/419), the structrure of the parse
#' data changes and we need to dispatch for older versions. As it is inconvenient
#' to pass a parser version down in the call stack in various places, the
#' environment `env_current` is used to store the current version *globally*
#' but internally.
#'
#' We version the parser as follows:
#'
#' * version 1: Before fix mentioned in #419.
#' * version 2: After #419.
#'
#'The following utilities are available:
#'
#' * `parser_version_set()` sets the parser version in the environment
#' `env_current`.
#' * `parser_version_get()` retrieves the parser version from the
#' environment `env_current`.
#' * `parser_version_find()` determines the version of the parser from parse
#' data. This does not necessarily mean that the version found is the
#' actual version, but it *behaves* like it. For example, code that does not
#' contain `EQ_ASSIGN` is parsed the same way with version 1 and 2. If the
#' behaviour is identical, the version is set to 1.
#' @param version The version of the parser to be used.
#' @param pd A parse table such as the output from
#' `utils::getParseData(parse(text = text))`.
#' @keywords internal
parser_version_set <- function(version) {
env_current$parser_version <- version
}

#' @rdname parser_version_set
parser_version_get <- function() {
env_current$parser_version
}

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



env_current <- rlang::new_environment(parent = rlang::empty_env())
8 changes: 7 additions & 1 deletion R/nested-to-tree.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@
create_tree <- function(text, structure_only = FALSE) {
compute_parse_data_nested(text) %>%
pre_visit(c(default_style_guide_attributes)) %>%
create_node_from_nested_root(structure_only) %>%
create_tree_from_pd_with_default_style_attributes(structure_only)
}

create_tree_from_pd_with_default_style_attributes <- function(pd, structure_only = FALSE) {
pd %>%
create_node_from_nested_root(structure_only) %>%
as.data.frame()
}


#' Convert a nested tibble into a node tree
#'
#' This function is convenient to display all nesting levels of a nested tibble
Expand Down
4 changes: 3 additions & 1 deletion R/parse.R
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ get_parse_data <- function(text, include_text = TRUE, ...) {
# avoid https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=16041
parse_safely(text, keep.source = TRUE)
parsed <- parse_safely(text, keep.source = TRUE)
as_tibble(utils::getParseData(parsed, includeText = include_text)) %>%
pd <- as_tibble(utils::getParseData(parsed, includeText = include_text)) %>%
add_id_and_short()
parser_version_set(parser_version_find(pd))
pd
}

#' Add column `pos_id` and `short`
Expand Down
15 changes: 11 additions & 4 deletions R/relevel.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ flatten_operators <- function(pd_nested) {
#' @keywords internal
flatten_operators_one <- function(pd_nested) {
pd_token_left <- c(special_token, math_token, "'$'")
pd_token_right <- c(special_token, "LEFT_ASSIGN", "'+'", "'-'")
pd_token_right <- c(
special_token, "LEFT_ASSIGN", if (parser_version_get() > 1) "EQ_ASSIGN",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we leave "EQ_ASSIGN" here unconditionally?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. A few changes in #441 did not have any known effects on the styling, it's just about making the AST look the same. But probably it would break something because there was some reason why it was not included in the list I guess.

"'+'", "'-'"
)
bound <- pd_nested %>%
flatten_pd(pd_token_left, left = TRUE) %>%
flatten_pd(pd_token_right, left = FALSE)
Expand Down Expand Up @@ -125,8 +128,13 @@ wrap_expr_in_expr <- function(pd) {
#' )
#' @keywords internal
relocate_eq_assign <- function(pd) {
pd %>%
post_visit(c(relocate_eq_assign_nest))
if (parser_version_get() < 2) {
pd %>%
post_visit(c(relocate_eq_assign_nest))
} else {
pd
}

}


Expand Down Expand Up @@ -161,7 +169,6 @@ relocate_eq_assign_nest <- function(pd) {
pd
}


#' Find the block to which a token belongs
#'
#' Two assignment tokens `EQ_ASSIGN` belong to the same block if they are not
Expand Down
6 changes: 4 additions & 2 deletions R/roxygen-examples-parse.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
#' ))
#' @keywords internal
parse_roxygen <- function(roxygen) {
parsed <- remove_roxygen_mask(roxygen) %>%
textConnection() %>%
connection <- remove_roxygen_mask(roxygen) %>%
textConnection()
parsed <- connection %>%
tools::parse_Rd(fragment = TRUE) %>%
as.character()
is_line_break <- parsed[1] == "\n"
close(connection)
c(parsed[1][!is_line_break], parsed[-1])
}

Expand Down
53 changes: 53 additions & 0 deletions man/parser_version_set.Rd

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