From 8007312889994db046d000393b982c50c77212ff Mon Sep 17 00:00:00 2001 From: Forest Fang Date: Mon, 28 Dec 2015 23:23:39 -0500 Subject: [PATCH 1/4] make spaces_left_parentheses_linter more robust it has been made smarter in determing whether ( belongs to a function call --- R/spaces_left_parentheses_linter.R | 17 +++++++++-------- .../test-spaces_left_parentheses_linter.R | 6 ++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/R/spaces_left_parentheses_linter.R b/R/spaces_left_parentheses_linter.R index e7090d6ee6..35ed108603 100644 --- a/R/spaces_left_parentheses_linter.R +++ b/R/spaces_left_parentheses_linter.R @@ -7,14 +7,15 @@ spaces_left_parentheses_linter <- function(source_file) { parsed <- source_file$parsed_content[id, ] - family_ids <- family(source_file$parsed_content, parsed$id) - - types <- source_file$parsed_content[ - source_file$parsed_content$id %in% family_ids, - "token"] - - is_function <- length(family_ids) %!=% 0L && - any(types %in% c("SYMBOL_FUNCTION_CALL", "FUNCTION")) + # content before parsed parentheses + before_parsed <- source_file$parsed_content[1:(id - 1), ] + # only look terminal tokens + terminal_types <- before_parsed[before_parsed$terminal, "token"] + # find the type of last such token + type <- terminal_types[length(terminal_types)] + + is_function <- length(type) %!=% 0L && + (type %in% c("SYMBOL_FUNCTION_CALL", "FUNCTION", "'}'", "')'")) if (!is_function) { diff --git a/tests/testthat/test-spaces_left_parentheses_linter.R b/tests/testthat/test-spaces_left_parentheses_linter.R index 306ec37dd3..76eeecee3f 100644 --- a/tests/testthat/test-spaces_left_parentheses_linter.R +++ b/tests/testthat/test-spaces_left_parentheses_linter.R @@ -35,6 +35,8 @@ test_that("returns the correct linting", { expect_lint("range(10)[(2 - 1):(10 - 1)]", NULL, spaces_left_parentheses_linter) + expect_lint("function(){function(){}}()()", NULL, spaces_left_parentheses_linter) + expect_lint("((1 + 1))", rex("Place a space before left parenthesis, except in a function call."), spaces_left_parentheses_linter) @@ -55,6 +57,10 @@ test_that("returns the correct linting", { rex("Place a space before left parenthesis, except in a function call."), spaces_left_parentheses_linter) + expect_lint("test <- function(x) { if(`+`(1, 1)) 'hi' }", + rex("Place a space before left parenthesis, except in a function call."), + spaces_left_parentheses_linter) + expect_lint("\"test <- function(x) { if(1 + 1) 'hi' }\"", NULL, spaces_left_parentheses_linter) From 1d56f06e90805e639bc5bf95ea2253c026a5735a Mon Sep 17 00:00:00 2001 From: Forest Fang Date: Mon, 28 Dec 2015 23:25:47 -0500 Subject: [PATCH 2/4] incorporate spaces_left_parentheses_linter suggestions --- R/cache.R | 2 +- R/exclude.R | 6 +++--- R/get_source_expressions.R | 2 +- R/methods.R | 2 +- R/trailing_blank_lines_linter.R | 2 +- R/zzz.R | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/R/cache.R b/R/cache.R index ead2d7c8cd..ee22b94908 100644 --- a/R/cache.R +++ b/R/cache.R @@ -105,7 +105,7 @@ find_new_line <- function(line_number, line, lines) { width <- 1L - while(width <= length(lines)) { + while (width <= length(lines)) { low <- line_number - width if (low > 0L) { if (lines[low] %==% line) { diff --git a/R/exclude.R b/R/exclude.R index bdbae6b00a..5cad9d9726 100644 --- a/R/exclude.R +++ b/R/exclude.R @@ -1,5 +1,5 @@ #' Exclude lines or files from linting -#' +#' #' @param lints that need to be filetered. #' @param exclusions manually specified exclusions #' @param ... additional arguments passed to \code{\link{parse_exclusions}} @@ -8,7 +8,7 @@ #' \enumerate{ #' \item{single line in the source file. default: \code{# nolint}} #' \item{line range in the source file. default: \code{# nolint start}, \code{# nolint end}} -#' \item{exclusions parameter, a named list of the files and lines to exclude, or just the filenames +#' \item{exclusions parameter, a named list of the files and lines to exclude, or just the filenames #' if you want to exclude the entire file.} #' } exclude <- function(lints, exclusions = settings$exclusions, ...) { @@ -63,7 +63,7 @@ parse_exclusions <- function(file, exclude = settings$exclude, stop(file, " has ", length(starts), " starts but only ", length(ends), " ends!") } - for(i in seq_along(starts)) { + for (i in seq_along(starts)) { exclusions <- c(exclusions, seq(starts[i], ends[i])) } } diff --git a/R/get_source_expressions.R b/R/get_source_expressions.R index cef9b3aa44..888f6163f5 100644 --- a/R/get_source_expressions.R +++ b/R/get_source_expressions.R @@ -249,7 +249,7 @@ fix_eq_assign <- function(pc) { end <- true_locs[i] j <- end + 1L - while(j <= length(expr_locs) && expr_locs[j] == FALSE) { + while (j <= length(expr_locs) && expr_locs[j] == FALSE) { end <- j j <- j + 1L } diff --git a/R/methods.R b/R/methods.R index ffdb38ae1e..5a17a6594b 100644 --- a/R/methods.R +++ b/R/methods.R @@ -116,7 +116,7 @@ names.lints <- function(x, ...) { split.lints <- function(x, f=NULL, ...) { if (is.null(f)) f <- names(x) splt <- split.default(x, f) - for(i in names(splt)) class(splt[[i]]) <- "lints" + for (i in names(splt)) class(splt[[i]]) <- "lints" return(splt) } diff --git a/R/trailing_blank_lines_linter.R b/R/trailing_blank_lines_linter.R index 537e35d447..88c57ebc56 100644 --- a/R/trailing_blank_lines_linter.R +++ b/R/trailing_blank_lines_linter.R @@ -6,7 +6,7 @@ trailing_blank_lines_linter <- function(source_file) { line_number <- length(source_file$file_lines) lints <- list() - while(line_number > 0L && (is.na(blanks[[line_number]]) || isTRUE(blanks[[line_number]]))) { + while (line_number > 0L && (is.na(blanks[[line_number]]) || isTRUE(blanks[[line_number]]))) { if (!is.na(blanks[[line_number]])) { lints[[length(lints) + 1L]] <- Lint( diff --git a/R/zzz.R b/R/zzz.R index 6f8bdce0c9..57b798324d 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -28,7 +28,7 @@ named_list <- function(...) { #' # you can also omit the argument name if you are just using different #' # arguments. #' with_defaults(line_length_linter(120)) -#' +#' #' # enforce camelCase rather than snake_case #' with_defaults(camel_case_linter = NULL, #' snake_case_linter) @@ -110,7 +110,7 @@ settings <- list2env(default_settings, parent = emptyenv()) lintr.linter_file = ".lintr" ) toset <- ! (names(op.lintr) %in% names(op)) - if(any(toset)) options(op.lintr[toset]) + if (any(toset)) options(op.lintr[toset]) invisible() } From abda27f526ec671eb7cc2597c74be731107a422c Mon Sep 17 00:00:00 2001 From: Forest Fang Date: Tue, 29 Dec 2015 10:15:23 -0500 Subject: [PATCH 3/4] make it more readable per @jimhester --- R/spaces_left_parentheses_linter.R | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/R/spaces_left_parentheses_linter.R b/R/spaces_left_parentheses_linter.R index 35ed108603..ef43137e2e 100644 --- a/R/spaces_left_parentheses_linter.R +++ b/R/spaces_left_parentheses_linter.R @@ -7,15 +7,15 @@ spaces_left_parentheses_linter <- function(source_file) { parsed <- source_file$parsed_content[id, ] - # content before parsed parentheses - before_parsed <- source_file$parsed_content[1:(id - 1), ] - # only look terminal tokens - terminal_types <- before_parsed[before_parsed$terminal, "token"] - # find the type of last such token - type <- terminal_types[length(terminal_types)] - - is_function <- length(type) %!=% 0L && - (type %in% c("SYMBOL_FUNCTION_CALL", "FUNCTION", "'}'", "')'")) + terminal_tokens_before <- + source_file$parsed_content$token[ + source_file$parsed_content$line1 == parsed$line1 & + source_file$parsed_content$col1 < parsed$col1 & + source_file$parsed_content$terminal] + last_type <- tail(terminal_tokens_before, n = 1) + + is_function <- length(last_type) %!=% 0L && + (last_type %in% c("SYMBOL_FUNCTION_CALL", "FUNCTION", "'}'", "')'")) if (!is_function) { From 62eadcb5623dd55036bb52871a03f2ac22373e51 Mon Sep 17 00:00:00 2001 From: Forest Fang Date: Tue, 29 Dec 2015 10:15:49 -0500 Subject: [PATCH 4/4] add ']' to the whitelist --- R/spaces_left_parentheses_linter.R | 2 +- tests/testthat/test-spaces_left_parentheses_linter.R | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/R/spaces_left_parentheses_linter.R b/R/spaces_left_parentheses_linter.R index ef43137e2e..d91eb2aa6a 100644 --- a/R/spaces_left_parentheses_linter.R +++ b/R/spaces_left_parentheses_linter.R @@ -15,7 +15,7 @@ spaces_left_parentheses_linter <- function(source_file) { last_type <- tail(terminal_tokens_before, n = 1) is_function <- length(last_type) %!=% 0L && - (last_type %in% c("SYMBOL_FUNCTION_CALL", "FUNCTION", "'}'", "')'")) + (last_type %in% c("SYMBOL_FUNCTION_CALL", "FUNCTION", "'}'", "')'", "']'")) if (!is_function) { diff --git a/tests/testthat/test-spaces_left_parentheses_linter.R b/tests/testthat/test-spaces_left_parentheses_linter.R index 76eeecee3f..d29a734cc8 100644 --- a/tests/testthat/test-spaces_left_parentheses_linter.R +++ b/tests/testthat/test-spaces_left_parentheses_linter.R @@ -37,6 +37,8 @@ test_that("returns the correct linting", { expect_lint("function(){function(){}}()()", NULL, spaces_left_parentheses_linter) + expect_lint("c(function(){})[1]()", NULL, spaces_left_parentheses_linter) + expect_lint("((1 + 1))", rex("Place a space before left parenthesis, except in a function call."), spaces_left_parentheses_linter)