Skip to content

Commit

Permalink
first pass done
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Dec 7, 2023
1 parent e3b18e1 commit 05cd63f
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 63 deletions.
2 changes: 1 addition & 1 deletion tests/testthat/test-lengths_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ test_that("lints vectorize", {
expect_lint(
trim_some("{
sapply(x, length)
ma_int(x, length)
map_int(x, length)
}"),
list(
list(lint_msg, line_number = 2L),
Expand Down
6 changes: 5 additions & 1 deletion tests/testthat/test-routine_registration_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ patrick::with_parameters_test_that(
)

test_that("lints vectorize", {
lint_msg <- "Register your native code routines with useDynLib"

expect_lint(
trim_some("{
.C('ROUTINE', PACKAGE = 'foo')
.External('POUTINE', PACKAGE = 'bar')
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
routine_registration_linter()
)
})
13 changes: 2 additions & 11 deletions tests/testthat/test-scalar_in_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,9 @@ test_that("multiple lints are generated correctly", {
x %in% 1
y %chin% "a"
}'),
list("%in%", "%chin%"),
linter
)
})

test_that("lints vectorize", {
expect_lint(
trim_some("{
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
list("%in%", line_number = 2L),
list("%chin%", line_number = 3L)
),
linter
)
Expand Down
8 changes: 5 additions & 3 deletions tests/testthat/test-sort_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,13 @@ test_that("sort_linter blocks simple disallowed usages", {
test_that("lints vectorize", {
expect_lint(
trim_some("{
x == sort(x)
x != sort(x)
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
list(rex::rex("is.unsorted(x)"), line_number = 2L),
list(rex::rex("!is.unsorted(x)"), line_number = 3L)
),
linter
sort_linter()
)
})
6 changes: 5 additions & 1 deletion tests/testthat/test-spaces_left_parentheses_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,17 @@ test_that("doesn't produce a warning", {
})

test_that("lints vectorize", {
lint_msg <- rex::rex("Place a space before left parenthesis, except in a function call.")

expect_lint(
trim_some("{
y1<-(abs(yn)>90)*1
for(i in j) { }
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
spaces_left_parentheses_linter()
)
})
10 changes: 7 additions & 3 deletions tests/testthat/test-sprintf_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,17 @@ local({
})

test_that("lints vectorize", {
skip_if_not_r_version("4.1.0")

expect_lint(
trim_some("{
sprintf('%s', a, b)
sprintf('%s%s', a)
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
list("one argument not used by format", line_number = 2L),
list("too few arguments", line_number = 3L)
),
linter
linter <- sprintf_linter()
)
})
6 changes: 5 additions & 1 deletion tests/testthat/test-strings_as_factors_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,17 @@ test_that("strings_as_factors_linter catches more functions with string output",
})

test_that("lints vectorize", {
lint_msg <- "Supply an explicit value for stringsAsFactors for this code"

expect_lint(
trim_some("{
data.frame('a')
data.frame('b')
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
strings_as_factors_linter()
)
})
26 changes: 13 additions & 13 deletions tests/testthat/test-system_file_linter.R
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
test_that("system_file_linter skips allowed usages", {
expect_lint("system.file('a', 'b', 'c')", NULL, system_file_linter())
expect_lint("file.path('a', 'b', 'c')", NULL, system_file_linter())
linter <- system_file_linter()

expect_lint("system.file('a', 'b', 'c')", NULL, linter)
expect_lint("file.path('a', 'b', 'c')", NULL, linter)
})

test_that("system_file_linter blocks simple disallowed usages", {
expect_lint(
"system.file(file.path('path', 'to', 'data'), package = 'foo')",
rex::rex("Use the `...` argument of system.file() to expand paths"),
system_file_linter()
)
linter <- system_file_linter()
lint_msg <- rex::rex("Use the `...` argument of system.file() to expand paths")

expect_lint(
"file.path(system.file(package = 'foo'), 'path', 'to', 'data')",
rex::rex("Use the `...` argument of system.file() to expand paths"),
system_file_linter()
)
expect_lint("system.file(file.path('path', 'to', 'data'), package = 'foo')", lint_msg, linter)
expect_lint("file.path(system.file(package = 'foo'), 'path', 'to', 'data')", lint_msg, linter)
})

test_that("lints vectorize", {
lint_msg <- rex::rex("Use the `...` argument of system.file() to expand paths")

expect_lint(
trim_some("{
file.path(system.file(package = 'foo'), 'bar')
system.file(file.path('bar', data'), package = 'foo')
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
system_file_linter()
)
})
10 changes: 7 additions & 3 deletions tests/testthat/test-unnecessary_lambda_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,15 @@ test_that("function shorthand is handled", {
test_that("lints vectorize", {
expect_lint(
trim_some("{
sapply(x, function(xi) sd(xi))
lapply(y, function(yi) {
sum(yi)
})
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
list("sd", line_number = 2L),
list("sum", line_number = 3L)
),
linter
unnecessary_lambda_linter()
)
})
51 changes: 34 additions & 17 deletions tests/testthat/test-unnecessary_nesting_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,29 @@ test_that("unnecessary_nesting_linter skips allowed usages", {
linter <- unnecessary_nesting_linter()

# parallel stops() and return()s are OK
double_stop_lines <- c(
"if (A) {",
" stop()",
"} else {",
" stop()",
"}"
expect_lint(
trim_some("
if (A) {
stop()
} else {
stop()
}
"),
NULL,
linter
)
expect_lint(double_stop_lines, NULL, linter)

double_return_lines <- c(
"if (A) {",
" return()",
"} else {",
" return()",
"}"
expect_lint(
trim_some("
if (A) {
return()
} else {
return()
}
"),
NULL,
linter
)
expect_lint(double_return_lines, NULL, linter)
})

# TODO(michaelchirico): consider if there's a nice easy pattern to enforce for
Expand Down Expand Up @@ -317,13 +323,24 @@ test_that("unnecessary_nesting_linter allow_assignment= argument works", {
})

test_that("lints vectorize", {
lint_msg <- rex::rex("Reduce the nesting of this if/else")

expect_lint(
trim_some("{
}"),
trim_some("
if (A) {
stop('no')
} else {
if (B) {
stop('really no')
} else {
1
}
}
"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
unnecessary_nesting_linter()
)
})
6 changes: 5 additions & 1 deletion tests/testthat/test-unnecessary_placeholder_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,17 @@ patrick::with_parameters_test_that(
)

test_that("lints vectorize", {
lint_msg <- rex::rex("Don't use the placeholder (`.`) when it's not needed")

expect_lint(
trim_some("{
x %>% foo(.)
y %T>% bar(.)
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
),
linter
unnecessary_placeholder_linter()
)
})
4 changes: 2 additions & 2 deletions tests/testthat/test-unused_import_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ test_that("unused_import_linter handles message vectorization", {
xmlparsedata::xml_parse_data(parse(text = 'a'))
"),
list(
rex::rex("Package 'crayon' is attached but never used."),
rex::rex("Don't attach package 'xmlparsedata', which is only used by namespace")
list(rex::rex("Package 'crayon' is attached but never used."), line_number = 1L),
list(rex::rex("Don't attach package 'xmlparsedata', which is only used by namespace"), line_number = 2L)
),
unused_import_linter()
)
Expand Down
8 changes: 5 additions & 3 deletions tests/testthat/test-vector_logic_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,13 @@ test_that("vector_logic_linter recognizes some false positves around bitwise &/|
test_that("lints vectorize", {
expect_lint(
trim_some("{
if (AA & BB) {}
if (CC | DD) {}
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
list(rex::rex("`&&`"), line_number = 2L),
list(rex::rex("`||`"), line_number = 3L)
),
linter
vector_logic_linter()
)
})
8 changes: 5 additions & 3 deletions tests/testthat/test-yoda_test_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,14 @@ test_that("yoda_test_linter throws a special message for placeholder tests", {
test_that("lints vectorize", {
expect_lint(
trim_some("{
expect_equal(1, 1)
expect_equal(2, foo(x))
}"),
list(
list(lint_msg, line_number = 2L),
list(lint_msg, line_number = 3L)
list("Avoid storing placeholder tests", line_number = 2L),
list("Compare objects in tests in the order 'actual', 'expected'", line_number = 3L)
),
linter
yoda_test_linter()
)
})

Expand Down

0 comments on commit 05cd63f

Please sign in to comment.