Skip to content

Remove some tests of internal API #2801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
215e2e0
remove internal API test
MichaelChirico Feb 27, 2025
15cde56
remove more tests of internal helpers
MichaelChirico Feb 27, 2025
786e804
nocov
MichaelChirico Feb 27, 2025
0d0d051
unneeded read_settings() calls
MichaelChirico Feb 27, 2025
fafa8da
trim_output is not used
MichaelChirico Feb 27, 2025
ab86a5b
rm unused is_relative_path
MichaelChirico Feb 27, 2025
d165068
remove now-dead test
MichaelChirico Feb 27, 2025
9541270
Merge branch 'main' into rm-make-linter-test
MichaelChirico Feb 27, 2025
036bdd7
use public API in linter_tags
MichaelChirico Feb 27, 2025
aaf4620
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Feb 27, 2025
5789d8a
progress removing from object_name_linter tests
MichaelChirico Feb 27, 2025
11ea6fe
missing ','
MichaelChirico Feb 27, 2025
2d41ad5
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Feb 27, 2025
73c06aa
placehold ignore_order=TRUE for now
MichaelChirico Feb 27, 2025
8a7f524
Add ignore_order= argument
MichaelChirico Feb 27, 2025
ce923f9
remove file of all ::: tests
MichaelChirico Feb 28, 2025
ce7b2f2
better "wrong columns" test input
MichaelChirico Feb 28, 2025
e80a561
delint
MichaelChirico Mar 3, 2025
5717cb5
delete ::: tests to see what needs coverage
MichaelChirico Mar 3, 2025
58e830b
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Mar 3, 2025
c49ec12
Remove ':::' usage
MichaelChirico Mar 3, 2025
3cef71f
Remove mostly-internal test file
MichaelChirico Mar 3, 2025
360901a
Just delete more tests for now
MichaelChirico Mar 3, 2025
28bacc7
Merge branch 'main' into rm-make-linter-test
MichaelChirico Mar 3, 2025
6a30a9e
delint
MichaelChirico Mar 3, 2025
83ee84b
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Mar 3, 2025
23818cb
Just delete ::: tests
MichaelChirico Mar 3, 2025
24e6e54
test was picking up global settings (?)
MichaelChirico Mar 3, 2025
78c35ca
Merge remote-tracking branch 'origin/main' into rm-make-linter-test
MichaelChirico May 8, 2025
cb67e96
Merge branch 'main' into rm-make-linter-test
MichaelChirico May 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ bad.R
script.R
*~
\#*\#
*.swp

*.o
*.so
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@
* Setting `exclusions` supports globs like `knitr*` to exclude files/directories with a pattern (#1554, @MichaelChirico).
* `get_source_expression()` captures warnings emitted by the R parser (currently always for mis-specified literal integers like `1.1L`) and `lint()` returns them as lints (#2065, @MichaelChirico).
* `object_name_linter()` and `object_length_linter()` apply to objects assigned with `assign()` or generics created with `setGeneric()` (#1665, @MichaelChirico).
* `expect_lint()` has a new argument `ignore_order` (default `FALSE`), which, if `TRUE`, allows the `checks=` to be provided in arbitary order vs. how `lint()` produces them (@MichaelChirico).
* `object_usage_linter()` gains argument `interpret_extensions` to govern which false positive-prone common syntaxes should be checked for used objects (#1472, @MichaelChirico). Currently `"glue"` (renamed from earlier argument `interpret_glue`) and `"rlang"` are supported. The latter newly covers usage of the `.env` pronoun like `.env$key`, where `key` was previously missed as being a used variable.
* `boolean_arithmetic_linter()` finds many more cases like `sum(x | y) == 0` where the total of a known-logical vector is compared to 0 (#1580, @MichaelChirico).
* `expect_lint()` has a new argument `ignore_order` (default `FALSE`), which, if `TRUE`, allows the `checks=` to be provided in arbitary order vs. how `lint()` produces them (@MichaelChirico).
6 changes: 3 additions & 3 deletions R/expect_lint.R
Original file line number Diff line number Diff line change
@@ -62,11 +62,11 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en", igno

lints <- lint(file, ...)
n_lints <- length(lints)
lint_str <- if (n_lints) paste(c("", lints), collapse = "\n") else ""
lint_fmt <- function() if (n_lints > 0L) paste(c("", lints), collapse = "\n") else ""

wrong_number_fmt <- "got %d lints instead of %d%s"
if (is.null(checks)) {
msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str)
msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_fmt())
return(testthat::expect(n_lints %==% 0L, msg))
}

@@ -76,7 +76,7 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en", igno
checks[] <- lapply(checks, fix_names, "message")

if (n_lints != length(checks)) {
msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_str)
msg <- sprintf(wrong_number_fmt, n_lints, length(checks), lint_fmt())
return(testthat::expect(FALSE, msg))
}

4 changes: 2 additions & 2 deletions R/make_linter_from_regex.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
make_linter_from_regex <- function(regex,
lint_type,
lint_msg) {
function() {
function() { # nocov: only run at namespace load time

Check warning on line 4 in R/make_linter_from_regex.R

Codecov / codecov/patch

R/make_linter_from_regex.R#L4

Added line #L4 was not covered by tests
Linter(linter_level = "file", function(source_expression) {
all_matches <- re_matches(
source_expression[["file_lines"]],
@@ -29,7 +29,7 @@
})
lints[lengths(lints) > 0L]
})
}
} # nocov: ditto opening brace

Check warning on line 32 in R/make_linter_from_regex.R

Codecov / codecov/patch

R/make_linter_from_regex.R#L32

Added line #L32 was not covered by tests
}

#' Determine if a regex match is covered by an expression in a source_expression
33 changes: 0 additions & 33 deletions R/methods.R
Original file line number Diff line number Diff line change
@@ -85,39 +85,6 @@ print.lints <- function(x, ...) {
invisible(x)
}

trim_output <- function(x, max = 65535L) {
# if x is less than the max, just return it
if (length(x) <= 0L || nchar(x) <= max) {
return(x)
}

# otherwise trim x to the max, then search for the lint starts
x <- substr(x, 1L, max)

re <- rex(
"[", except_some_of(":"), ":", numbers, ":", numbers, ":", "]",
"(", except_some_of(")"), ")",
space,
"*", or("style", "warning", "error"), ":", "*",
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline
)

lint_starts <- re_matches(x, re, global = TRUE, locations = TRUE)[[1L]]

# if at least one lint ends before the cutoff, cutoff there, else just use
# the cutoff
last_end <- lint_starts[NROW(lint_starts), "end"]
if (!is.na(last_end)) {
substr(x, 1L, last_end)
} else {
x
}
}

#' @export
names.lints <- function(x, ...) {
vapply(x, `[[`, character(1L), "filename")
4 changes: 0 additions & 4 deletions R/path_utils.R
Original file line number Diff line number Diff line change
@@ -57,10 +57,6 @@ is_root_path <- function(path) {
re_matches(path, root_path_regex)
}

is_relative_path <- function(path) {
re_matches(path, relative_path_regex)
}

is_path <- function(path) {
re_matches(path, path_regex)
}
10 changes: 5 additions & 5 deletions tests/testthat/dummy_projects/project/mismatched_starts_ends.R
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#nolint end
#TeSt_NoLiNt_EnD

#nolint start
#TeSt_NoLiNt_StArT

c(1,2)

#nolint start
#TeSt_NoLiNt_StArT

#nolint end
#TeSt_NoLiNt_EnD

#nolint start
#TeSt_NoLiNt_StArT
2 changes: 1 addition & 1 deletion tests/testthat/dummy_projects/project/one_start_no_end.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@


#nolint start
#TeSt_NoLiNt_StArT

c(1,2)
131 changes: 0 additions & 131 deletions tests/testthat/test-absolute_path_linter.R
Original file line number Diff line number Diff line change
@@ -1,134 +1,3 @@
# styler: off
test_that("is_root_path", {
f <- lintr:::is_root_path

x <- character()
y <- logical()
expect_identical(f(x), y)

x <- c("", "foo", "http://rseek.org/", "./", " /", "/foo", "'/'")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_identical(f(x), y)

x <- c("/", "//")
y <- c(TRUE, FALSE)
expect_identical(f(x), y)

x <- c("~", "~/", "~//", "~bob2", "~foo_bar/")
y <- c(TRUE, TRUE, TRUE, TRUE, TRUE)
expect_identical(f(x), y)

x <- c("c:", "C:\\", "D:/", "C:\\\\", "D://")
y <- c(TRUE, TRUE, TRUE, FALSE, FALSE)
expect_identical(f(x), y)

x <- c("\\\\", "\\\\localhost", "\\\\localhost\\")
y <- c(TRUE, TRUE, TRUE)
expect_identical(f(x), y)
})


test_that("is_absolute_path", {
f <- lintr:::is_absolute_path

x <- character()
y <- logical()
expect_identical(f(x), y)

x <- c("/", "//", "/foo", "/foo/")
y <- c(TRUE, FALSE, TRUE, TRUE)
expect_identical(f(x), y)

x <- c("~", "~/foo", "~/foo/", "~'")
y <- c(TRUE, TRUE, TRUE, FALSE)
expect_identical(f(x), y)

x <- c("c:", "C:\\foo\\", "C:/foo/")
y <- c(TRUE, TRUE, TRUE)
expect_identical(f(x), y)

x <- c("\\\\", "\\\\localhost", "\\\\localhost\\c$", "\\\\localhost\\c$\\foo")
y <- c(TRUE, TRUE, TRUE, TRUE)
expect_identical(f(x), y)
})


test_that("is_relative_path", {
f <- lintr:::is_relative_path

x <- character()
y <- logical()
expect_identical(f(x), y)

x <- c("/", "c:\\", "~/", "foo", "http://rseek.org/", "'./'")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_identical(f(x), y)

x <- c("/foo", "foo/", "foo/bar", "foo//bar", "./foo", "../foo")
y <- c(FALSE, TRUE, TRUE, TRUE, TRUE, TRUE)
expect_identical(f(x), y)

x <- c("\\\\", "\\foo", "foo\\", "foo\\bar", ".\\foo", "..\\foo", ".", "..", "../")
y <- c(FALSE, FALSE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE)
expect_identical(f(x), y)
})


test_that("is_path", {
f <- lintr:::is_path

x <- character()
y <- logical()
expect_identical(f(x), y)

x <- c("", "foo", "http://rseek.org/", "foo\nbar", "'foo/bar'", "'/'")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_identical(f(x), y)

x <- c("c:", "..", "foo/bar", "foo\\bar", "~", "\\\\localhost")
y <- c(TRUE, TRUE, TRUE, TRUE, TRUE, TRUE)
expect_identical(f(x), y)
})


test_that("is_valid_path", {
f <- lintr:::is_valid_path

x <- character()
y <- logical()
expect_identical(f(x), y)

x <- c("C:/asdf", "C:/asd*f", "a\\s:df", "a\\\nsdf")
y <- c(TRUE, FALSE, FALSE, FALSE)
expect_identical(f(x), y)

x <- c("C:/asdf", "C:/asd*f", "a\\s:df", "a\\\nsdf")
y <- c(TRUE, FALSE, FALSE, FALSE)
expect_identical(f(x, lax = TRUE), y)

x <- c("/asdf", "/asd*f", "/as:df", "/a\nsdf")
y <- c(TRUE, TRUE, TRUE, TRUE)
expect_identical(f(x), y)

x <- c("/asdf", "/asd*f", "/as:df", "/a\nsdf")
y <- c(TRUE, FALSE, FALSE, FALSE)
expect_identical(f(x, lax = TRUE), y)
})


test_that("is_long_path", {
f <- lintr:::is_long_path

x <- character()
y <- logical()
expect_identical(f(x), y)

x <- c("foo/", "/foo", "n/a", "Z:\\foo", "foo/bar", "~/foo", "../foo")
y <- c(FALSE, FALSE, FALSE, TRUE, TRUE, TRUE, TRUE)
expect_identical(f(x), y)
})
# styler: on

test_that("returns the correct linting", {
lint_msg <- rex::escape("Do not use absolute paths.")

Loading
Loading