diff --git a/R/implicit_integer_linter.R b/R/implicit_integer_linter.R index b41210fa0..49e57c52e 100644 --- a/R/implicit_integer_linter.R +++ b/R/implicit_integer_linter.R @@ -54,12 +54,20 @@ implicit_integer_linter <- function(allow_colon = FALSE) { Linter(linter_level = "file", function(source_expression) { xml <- source_expression$full_xml_parsed_content - numbers <- xml_find_all(xml, xpath) + number_expr <- xml_find_all(xml, xpath) + number <- xml_text(number_expr) + lint_idx <- is_implicit_integer(number) + number_expr <- number_expr[lint_idx] + number <- number[lint_idx] + is_negative <- !is.na(xml_find_first(number_expr, "parent::expr/preceding-sibling::OP-MINUS")) + + lint_message <- + sprintf("Use %1$dL or %1$d.0 to avoid implicit integers.", ((-1L) ^ is_negative) * as.integer(number)) xml_nodes_to_lints( - numbers[is_implicit_integer(xml_text(numbers))], + number_expr, source_expression = source_expression, - lint_message = "Avoid implicit integers. Use e.g. 1L for integers or 1.0 for doubles.", + lint_message = lint_message, type = "style", column_number_xpath = "number(./@col2 + 1)", # mark at end range_end_xpath = "number(./@col2 + 1)" # end after number for easy fixing (enter "L" or ".0") diff --git a/tests/testthat/test-implicit_integer_linter.R b/tests/testthat/test-implicit_integer_linter.R index 1aa57d146..12a94a9b6 100644 --- a/tests/testthat/test-implicit_integer_linter.R +++ b/tests/testthat/test-implicit_integer_linter.R @@ -4,51 +4,50 @@ local({ # Note: cases indicated by "*" are whole numbers, but don't lint because the user has # effectively declared "this is a double" much as adding '.0' is otherwise accepted. cases <- tibble::tribble( - ~num_value_str, ~should_lint, - "Inf", FALSE, - "NaN", FALSE, - "TRUE", FALSE, - "FALSE", FALSE, - "NA", FALSE, - "NA_character_", FALSE, - "2.000", FALSE, - "2.", FALSE, - "2L", FALSE, - "2.0", FALSE, - "2.1", FALSE, - "2", TRUE, - "1e3", TRUE, - "1e3L", FALSE, - "1.0e3L", FALSE, - "1.2e3", FALSE, # * ( = 1200) - "1.2e-3", FALSE, - "1e-3", FALSE, - "1e-33", FALSE, - "1.2e0", FALSE, - "0x1p+0", FALSE, # * ( = 1) - "0x1.ecp+6L", FALSE, - "0x1.ecp+6", FALSE, # * ( = 123) - "0x1.ec66666666666p+6", FALSE, - "8i", FALSE, - "8.0i", FALSE + ~num_value_str, ~lint_msg, + "Inf", "", + "NaN", "", + "TRUE", "", + "FALSE", "", + "NA", "", + "NA_character_", "", + "2.000", "", + "2.", "", + "2L", "", + "2.0", "", + "2.1", "", + "2", "2L or 2.0", + "1e3", "1000L or 1000.0", + "1e3L", "", + "1.0e3L", "", + "1.2e3", "", # * ( = 1200) + "1.2e-3", "", + "1e-3", "", + "1e-33", "", + "1.2e0", "", + "0x1p+0", "", # * ( = 1) + "0x1.ecp+6L", "", + "0x1.ecp+6", "", # * ( = 123) + "0x1.ec66666666666p+6", "", + "8i", "", + "8.0i", "" ) # for convenience of coercing these to string (since tribble doesn't support auto-conversion) int_max <- .Machine[["integer.max"]] # largest number that R can represent as an integer cases_int_max <- tibble::tribble( - ~num_value_str, ~should_lint, - -int_max - 1.0, FALSE, - -int_max, TRUE, - int_max, TRUE, - int_max + 1.0, FALSE + ~num_value_str, ~lint_msg, + -int_max - 1.0, "", + -int_max, sprintf("%1$dL or %1$d.0", -int_max), + int_max, sprintf("%1$dL or %1$d.0", int_max), + int_max + 1.0, "" ) cases_int_max$num_value_str <- as.character(cases_int_max$num_value_str) cases <- rbind(cases, cases_int_max) - cases$.test_name <- sprintf("num_value_str=%s, should_lint=%s", cases$num_value_str, cases$should_lint) linter <- implicit_integer_linter() patrick::with_parameters_test_that( "single numerical constants are properly identified ", - expect_lint(num_value_str, if (should_lint) "Avoid implicit integers", linter), + expect_lint(num_value_str, if (nzchar(lint_msg)) lint_msg, linter), .cases = cases ) }) @@ -56,7 +55,7 @@ local({ test_that("linter returns the correct linting", { linter <- implicit_integer_linter() - lint_msg <- rex::rex("Avoid implicit integers. Use e.g. 1L for integers or 1.0 for doubles.") + lint_msg <- rex::rex("Use 1L or 1.0 to avoid implicit integers.") expect_lint("x <<- 1L", NULL, linter) expect_lint("1.0/-Inf -> y", NULL, linter) @@ -67,7 +66,7 @@ test_that("linter returns the correct linting", { ) expect_lint( "z <- 1e5", - list(message = lint_msg, line_number = 1L, column_number = 9L), + list(message = rex::rex("100000L or 100000.0"), line_number = 1L, column_number = 9L), linter ) expect_lint( @@ -78,8 +77,8 @@ test_that("linter returns the correct linting", { expect_lint( "552^9", list( - list(message = lint_msg, line_number = 1L, column_number = 4L), - list(message = lint_msg, line_number = 1L, column_number = 6L) + list(message = rex::rex("552L or 552.0"), line_number = 1L, column_number = 4L), + list(message = rex::rex("9L or 9.0"), line_number = 1L, column_number = 6L) ), linter ) @@ -90,7 +89,7 @@ patrick::with_parameters_test_that( "numbers in a:b input are optionally not linted", expect_lint( paste0(left, ":", right), - if (n_lints > 0L) rep(list("Avoid implicit integers"), n_lints), + if (n_lints > 0L) rep(list(rex::rex("Use 1L or 1.0")), n_lints), implicit_integer_linter(allow_colon = allow_colon) ), .cases = tibble::tribble(