test_that("if_not_else_linter skips allowed usages", { linter <- if_not_else_linter() # simple if/else statement is fine expect_lint("if (A) x else y", NULL, linter) # not plain negation --> OK expect_lint("if (!A || B) x else y", NULL, linter) # no else clause --> OK expect_lint("if (!A) x", NULL, linter) # nested statements are also OK expect_lint("if (!A) x else if (B) y", NULL, linter) expect_lint("if (!A) x else if (B) y else z", NULL, linter) expect_lint("if (A) x else if (B) y else if (!C) z", NULL, linter) # ! picked up in the evaluation statements is skipped expect_lint("if (A) !x else y", NULL, linter) expect_lint("if (A) x else !y", NULL, linter) }) test_that("if_not_else_linter blocks simple disallowed usages", { linter <- if_not_else_linter() lint_msg <- rex::rex("In a simple if/else statement, prefer `if (A) x else y`") expect_lint("if (!A) x else y", lint_msg, linter) expect_lint("if (!A) x else if (!B) y else z", lint_msg, linter) # ditto for more complicated expressions where ! is still the outer operator expect_lint("if (!x %in% 1:10) y else z", lint_msg, linter) }) patrick::with_parameters_test_that( "if_not_else_linter blocks usages in ifelse() and friends as well", { linter <- if_not_else_linter() expect_lint(sprintf("%s(!A | B, x, y)", ifelse_fun), NULL, linter) expect_lint(sprintf("%s(A, !x, y)", ifelse_fun), NULL, linter) expect_lint( sprintf("%s(!A, x, y)", ifelse_fun), sprintf("Prefer `%s[(]A, x, y[)]` to the less-readable", ifelse_fun), linter ) # particularly relevant for if_else() expect_lint(sprintf("%s(!!A, x, y)", ifelse_fun), NULL, linter) }, .test_name = c("ifelse", "fifelse", "if_else"), ifelse_fun = c("ifelse", "fifelse", "if_else") ) test_that("if_not_else_linter skips negated calls to is.null & similar", { linter <- if_not_else_linter() expect_lint("if (!is.null(x)) x else y", NULL, linter) expect_lint("if (!is.na(x)) x else y", NULL, linter) expect_lint("if (!missing(x)) x else y", NULL, linter) expect_lint("ifelse(!is.na(x), x, y)", NULL, linter) }) test_that("multiple lints are generated correctly", { expect_lint( trim_some("{ if (!A) x else B ifelse(!A, x, y) fifelse(!A, x, y) if_else(!A, x, y) }"), list( "In a simple if/else statement", "Prefer `ifelse", "Prefer `fifelse", "Prefer `if_else" ), if_not_else_linter() ) }) test_that("exceptions= argument works", { expect_lint( "if (!is.null(x)) x else y", "In a simple if/else statement", if_not_else_linter(exceptions = character()) ) expect_lint("if (!foo(x)) y else z", NULL, if_not_else_linter(exceptions = "foo")) }) # TODO(michaelchirico): should if (A != B) be considered as well?