diff --git a/NEWS.md b/NEWS.md index aede5c723..7dfc590a5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -19,6 +19,7 @@ * `object_name_linter()` now excludes special R hook functions such as `.onLoad` (#500, #614, @AshesITR) * `equals_na_linter()` now lints `x != NA` and `NA == x`, and skips usages in comments (#545, @michaelchirico) * Malformed Rmd files now cause a lint instead of an error (#571, #575, @AshesITR) +* `object_name_linter()` gains a new default style, `"symbols"`, which won't lint all-symbol object names (in particular, that means operator names like `%+%` are skipped; #615, @michaelchirico) # lintr 2.0.1 diff --git a/R/object_name_linters.R b/R/object_name_linters.R index 79bc847fd..17a7211a2 100644 --- a/R/object_name_linters.R +++ b/R/object_name_linters.R @@ -3,7 +3,7 @@ #' \Sexpr[stage=render, results=rd]{lintr:::regexes_rd}. A name should #' match at least one of these styles. #' @export -object_name_linter <- function(styles = "snake_case") { +object_name_linter <- function(styles = c("snake_case", "symbols")) { .or_string <- function(xs) { # returns " or " @@ -13,7 +13,7 @@ object_name_linter <- function(styles = "snake_case") { if (len <= 1) { return(xs) } - comma_sepd_prefix <- paste(xs[-len], collapse = ", ") + comma_sepd_prefix <- toString(xs[-len]) paste(comma_sepd_prefix, "or", xs[len]) } @@ -24,10 +24,9 @@ object_name_linter <- function(styles = "snake_case") { ) function(source_file) { - x <- global_xml_parsed_content(source_file) - if (is.null(x)) { - return() - } + if (is.null(source_file$full_xml_parsed_content)) return(list()) + + xml <- source_file$full_xml_parsed_content xpath <- paste0( # Left hand assignments @@ -46,14 +45,14 @@ object_name_linter <- function(styles = "snake_case") { "//SYMBOL_FORMALS" ) - assignments <- xml2::xml_find_all(x, xpath) + assignments <- xml2::xml_find_all(xml, xpath) # Retrieve assigned name nms <- strip_names( as.character(xml2::xml_find_first(assignments, "./text()"))) generics <- c( - declared_s3_generics(x), + declared_s3_generics(xml), namespace_imports()$fun, names(.knownS3Generics), .S3PrimitiveGenerics, ls(baseenv())) @@ -102,7 +101,6 @@ strip_names <- function(x) { x } - object_lint2 <- function(expr, source_file, message, type) { symbol <- xml2::as_list(expr) Lint( @@ -272,53 +270,21 @@ object_lint <- function(source_file, token, message, type) { } -object_name_linter_old <- function(style = "snake_case") { - make_object_linter( - function(source_file, token) { - name <- unquote(token[["text"]]) - if (!any(matches_styles(name, style))) { - object_lint( - source_file, - token, - sprintf("Variable and function name style should be %s.", paste(style, collapse = " or ")), - "object_name_linter" - ) - } - } - ) -} - - loweralnum <- rex(one_of(lower, digit)) upperalnum <- rex(one_of(upper, digit)) style_regexes <- list( - "CamelCase" = rex(start, maybe("."), upper, zero_or_more(alnum), end), - "camelCase" = rex(start, maybe("."), lower, zero_or_more(alnum), end), - "snake_case" = rex(start, maybe("."), some_of(lower, digit), any_of("_", lower, digit), end), - "SNAKE_CASE" = rex(start, maybe("."), some_of(upper, digit), any_of("_", upper, digit), end), - "dotted.case" = rex(start, maybe("."), one_or_more(loweralnum), zero_or_more(dot, one_or_more(loweralnum)), end), + "symbols" = rex(start, maybe("."), zero_or_more(none_of(alnum)), end), + "CamelCase" = rex(start, maybe("."), upper, zero_or_more(alnum), end), + "camelCase" = rex(start, maybe("."), lower, zero_or_more(alnum), end), + "snake_case" = rex(start, maybe("."), some_of(lower, digit), any_of("_", lower, digit), end), + "SNAKE_CASE" = rex(start, maybe("."), some_of(upper, digit), any_of("_", upper, digit), end), + "dotted.case" = rex(start, maybe("."), one_or_more(loweralnum), zero_or_more(dot, one_or_more(loweralnum)), end), "lowercase" = rex(start, maybe("."), one_or_more(loweralnum), end), "UPPERCASE" = rex(start, maybe("."), one_or_more(upperalnum), end) ) -regexes_rd <- paste0(collapse = ", ", paste0("\\sQuote{", names(style_regexes), "}")) - -matches_styles <- function(name, styles=names(style_regexes)) { - invalids <- paste(styles[!styles %in% names(style_regexes)], collapse=", ") - if (nzchar(invalids)) { - valids <- paste(names(style_regexes), collapse=", ") - stop(sprintf("Invalid style(s) requested: %s\nValid styles are: %s\n", invalids, valids)) - } - name <- re_substitutes(name, rex(start, one_or_more(dot)), "") # remove leading dots - vapply( - style_regexes[styles], - re_matches, - logical(1L), - data=name - ) -} - +regexes_rd <- toString(paste0("\\sQuote{", names(style_regexes), "}")) #' @describeIn linters check that object names are not too long. #' @export diff --git a/man/get_source_expressions.Rd b/man/get_source_expressions.Rd index b940874f1..706119959 100644 --- a/man/get_source_expressions.Rd +++ b/man/get_source_expressions.Rd @@ -37,7 +37,7 @@ consisting of 6 elements: for .Rmd scripts, this is the extracted R source code (as text)} \item{\code{full_parsed_content} (\code{data.frame}) as given by \code{\link[utils:getParseData]{utils::getParseData()}} for the full content} -\item{\code{xml_parsed_content} (\code{xml_document}) the XML parse tree of all +\item{\code{full_xml_parsed_content} (\code{xml_document}) the XML parse tree of all expressions as given by \code{\link[xmlparsedata:xml_parse_data]{xmlparsedata::xml_parse_data()}}} \item{\code{terminal_newline} (\code{logical}) records whether \code{filename} has a terminal newline (as determined by \code{\link[=readLines]{readLines()}} producing a corresponding warning)} diff --git a/man/linters.Rd b/man/linters.Rd index 7fee6467f..802943d01 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -73,7 +73,7 @@ todo_comment_linter(todo = c("todo", "fixme")) cyclocomp_linter(complexity_limit = 25) -object_name_linter(styles = "snake_case") +object_name_linter(styles = c("snake_case", "symbols")) object_length_linter(length = 30L) diff --git a/tests/testthat/test-object_name_linter.R b/tests/testthat/test-object_name_linter.R index 7eba9667b..4fa5f0c59 100644 --- a/tests/testthat/test-object_name_linter.R +++ b/tests/testthat/test-object_name_linter.R @@ -3,40 +3,40 @@ context("object_name_linter") test_that("styles are correctly identified", { styles <- names(style_regexes) - do_style_check <- function(nms) lapply(styles, check_style, nms = nms) - # UpC lowC snake SNAKE dot alllow ALLUP - expect_equivalent(do_style_check("x" ), list(FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) - expect_equivalent(do_style_check(".x" ), list(FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) - expect_equivalent(do_style_check("X" ), list( TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) - expect_equivalent(do_style_check("x." ), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("X." ), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("x_" ), list(FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("X_" ), list(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("xy" ), list(FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) - expect_equivalent(do_style_check("xY" ), list(FALSE, TRUE, FALSE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("Xy" ), list( TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("XY" ), list( TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) - expect_equivalent(do_style_check("x1" ), list(FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) - expect_equivalent(do_style_check("X1" ), list( TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) - expect_equivalent(do_style_check("x_y"), list(FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("X_Y"), list(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("X.Y"), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("x_2"), list(FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("X_2"), list(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("x.2"), list(FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE)) - expect_equivalent(do_style_check("X.2"), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) - + # symbl UpC lowC snake SNAKE dot alllow ALLUP + expect_equivalent(do_style_check("x" ), list(FALSE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) + expect_equivalent(do_style_check(".x" ), list(FALSE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) + expect_equivalent(do_style_check("X" ), list(FALSE, TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) + expect_equivalent(do_style_check("x." ), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("X." ), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("x_" ), list(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("X_" ), list(FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("xy" ), list(FALSE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) + expect_equivalent(do_style_check("xY" ), list(FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("Xy" ), list(FALSE, TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("XY" ), list(FALSE, TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) + expect_equivalent(do_style_check("x1" ), list(FALSE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) + expect_equivalent(do_style_check("X1" ), list(FALSE, TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) + expect_equivalent(do_style_check("x_y"), list(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("X_Y"), list(FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("X.Y"), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("x_2"), list(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("X_2"), list(FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("x.2"), list(FALSE, FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE)) + expect_equivalent(do_style_check("X.2"), list(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) - # UpC lowC snake SNAKE dot alllow ALLUP - expect_equivalent(do_style_check("IHave1Cat" ), c( TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("iHave1Cat" ), c(FALSE, TRUE, FALSE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("i_have_1_cat" ), c(FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("I_HAVE_1_CAT" ), c(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) - expect_equivalent(do_style_check("i.have.1.cat" ), c(FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE)) - expect_equivalent(do_style_check("ihave1cat" ), c(FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) - expect_equivalent(do_style_check("IHAVE1CAT" ), c( TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) - expect_equivalent(do_style_check("I.HAVE_ONECAT"), c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + # symbl UpC lowC snake SNAKE dot alllow ALLUP + expect_equivalent(do_style_check("IHave1Cat" ), c(FALSE, TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("iHave1Cat" ), c(FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("i_have_1_cat" ), c(FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("I_HAVE_1_CAT" ), c(FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("i.have.1.cat" ), c(FALSE, FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE)) + expect_equivalent(do_style_check("ihave1cat" ), c(FALSE, FALSE, TRUE, TRUE, FALSE, TRUE, TRUE, FALSE)) + expect_equivalent(do_style_check("IHAVE1CAT" ), c(FALSE, TRUE, FALSE, FALSE, TRUE, FALSE, FALSE, TRUE)) + expect_equivalent(do_style_check("I.HAVE_ONECAT"), c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("." ), c( TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equivalent(do_style_check("%^%" ), c( TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)) }) test_that("linter ignores some objects", { @@ -48,6 +48,7 @@ test_that("linter ignores some objects", { expect_lint("print.foo <- t", NULL, object_name_linter("CamelCase")) # S3 generic expect_lint("names.foo <- t", NULL, object_name_linter("CamelCase")) # int generic expect_lint("sapply(x,f,USE.NAMES=T)", NULL, object_name_linter("snake_case")) # defined elsewhere + expect_lint("`%++%` <- `+`", NULL, object_name_linter("symbols")) # all-symbol operator }) test_that("linter returns correct linting", {