From f4ceedb8057ac6f72e98e1b1ba5f004999e10d2c Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sun, 4 Aug 2024 11:46:37 +0200 Subject: [PATCH 1/8] Print a message when no lints found closes #2640 --- R/methods.R | 8 ++++++-- tests/testthat/test-methods.R | 7 +++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/R/methods.R b/R/methods.R index 29a974d48..c1162feac 100644 --- a/R/methods.R +++ b/R/methods.R @@ -101,10 +101,14 @@ print.lints <- function(x, ...) { if (isTRUE(settings$error_on_lint)) { quit("no", 31L, FALSE) # nocov } - } else if (use_rstudio_source_markers) { + } + + if (length(x) == 0L) { + cli_inform("No lints found.") # Empty lints: clear RStudio source markers - rstudio_source_markers(x) + if (use_rstudio_source_markers) rstudio_source_markers(x) } + invisible(x) } diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index 172b9692d..b337169f1 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -96,6 +96,13 @@ test_that("print.lint works", { expect_output(print(l), " 1:length(x)", fixed = TRUE) }) +test_that("print.lint works with empty lints", { + withr::local_options(list(lintr.rstudio_source_markers = FALSE)) + l <- lint(text = "1L") + + expect_message(print(l), "No lints found", fixed = TRUE) +}) + test_that("print.lint works for inline data, even in RStudio", { l <- lint("x = 1\n") From 223b98e411bde2dde7b5619ba72ce098cda0be7f Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sun, 4 Aug 2024 11:54:35 +0200 Subject: [PATCH 2/8] use info markup --- R/methods.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/methods.R b/R/methods.R index c1162feac..d37090b48 100644 --- a/R/methods.R +++ b/R/methods.R @@ -104,7 +104,7 @@ print.lints <- function(x, ...) { } if (length(x) == 0L) { - cli_inform("No lints found.") + cli_inform(c("i" = "No lints found.")) # Empty lints: clear RStudio source markers if (use_rstudio_source_markers) rstudio_source_markers(x) } From 24f461b7619380b1193a0d8fbf34060bf2079afb Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sun, 4 Aug 2024 11:57:17 +0200 Subject: [PATCH 3/8] Update NEWS.md --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 140ee54ea..5ba695f5c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -86,7 +86,7 @@ ## Notes -* All user-facing messages are now prepared using the `{cli}` package (#2418, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent. +* All user-facing messages are now prepared using the `{cli}` package (#2418, #2643, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent. * {lintr} now depends on R version 4.0.0. It already does so implicitly due to recursive upstream dependencies requiring this version; we've simply made that dependency explicit and up-front (#2569, @MichaelChirico). # lintr 3.1.2 From 78939d2afff423427619f18b83563648dcd5b717 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sun, 4 Aug 2024 12:03:12 +0200 Subject: [PATCH 4/8] delint --- R/methods.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/methods.R b/R/methods.R index d37090b48..a5f466691 100644 --- a/R/methods.R +++ b/R/methods.R @@ -104,7 +104,7 @@ print.lints <- function(x, ...) { } if (length(x) == 0L) { - cli_inform(c("i" = "No lints found.")) + cli_inform(c(i = "No lints found.")) # Empty lints: clear RStudio source markers if (use_rstudio_source_markers) rstudio_source_markers(x) } From b0a24238de9c1593db333652aec9b6c9d17693cf Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sun, 4 Aug 2024 23:32:48 +0200 Subject: [PATCH 5/8] more inlining --- R/methods.R | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/R/methods.R b/R/methods.R index a5f466691..4baf92781 100644 --- a/R/methods.R +++ b/R/methods.R @@ -98,15 +98,12 @@ print.lints <- function(x, ...) { lapply(x, print, ...) } - if (isTRUE(settings$error_on_lint)) { - quit("no", 31L, FALSE) # nocov - } + if (isTRUE(settings$error_on_lint)) quit("no", 31L, FALSE) # nocov } if (length(x) == 0L) { - cli_inform(c(i = "No lints found.")) - # Empty lints: clear RStudio source markers - if (use_rstudio_source_markers) rstudio_source_markers(x) + cli_inform(c("i" = "No lints found.")) + if (use_rstudio_source_markers) rstudio_source_markers(x) # clear RStudio source markers } invisible(x) From 63b00176034cf5fb7ecd943ef1b18fd5770155d4 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Sun, 4 Aug 2024 23:36:42 +0200 Subject: [PATCH 6/8] use single if-else --- R/methods.R | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/R/methods.R b/R/methods.R index 4baf92781..869c17ef1 100644 --- a/R/methods.R +++ b/R/methods.R @@ -98,12 +98,15 @@ print.lints <- function(x, ...) { lapply(x, print, ...) } - if (isTRUE(settings$error_on_lint)) quit("no", 31L, FALSE) # nocov - } - - if (length(x) == 0L) { - cli_inform(c("i" = "No lints found.")) - if (use_rstudio_source_markers) rstudio_source_markers(x) # clear RStudio source markers + if (isTRUE(settings$error_on_lint)) { + quit("no", 31L, FALSE) # nocov + } + } else { + # Empty lints + cli_inform(c(i = "No lints found.")) + if (use_rstudio_source_markers) { + rstudio_source_markers(x) # clear RStudio source markers + } } invisible(x) From de94d8fd64d522d998400e714f7038dde18717d1 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Mon, 5 Aug 2024 07:20:14 +0200 Subject: [PATCH 7/8] Create a separate NEWS entry --- NEWS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 5ba695f5c..68ef13f8d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -58,6 +58,7 @@ * `make_linter_from_xpath()` errors up front when `lint_message` is missing (instead of delaying this error until the linter is used, #2541, @MichaelChirico). * `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico). * `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle). +* `lint()` and friends emit a message if no lints are found (#2643, @IndrajeetPatil). ### New linters @@ -86,7 +87,7 @@ ## Notes -* All user-facing messages are now prepared using the `{cli}` package (#2418, #2643, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent. +* All user-facing messages are now prepared using the `{cli}` package (#2418, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent. * {lintr} now depends on R version 4.0.0. It already does so implicitly due to recursive upstream dependencies requiring this version; we've simply made that dependency explicit and up-front (#2569, @MichaelChirico). # lintr 3.1.2 From b8dfccad364b4eabdbd20f7c48bf6ba85b0aa8d4 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Mon, 5 Aug 2024 08:47:29 +0200 Subject: [PATCH 8/8] don't install quarto to avoid NOTE --- .github/workflows/R-CMD-check-hard.yaml | 1 + .github/workflows/R-CMD-check.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/R-CMD-check-hard.yaml b/.github/workflows/R-CMD-check-hard.yaml index 54db1e064..977807d3c 100644 --- a/.github/workflows/R-CMD-check-hard.yaml +++ b/.github/workflows/R-CMD-check-hard.yaml @@ -48,6 +48,7 @@ jobs: - uses: r-lib/actions/setup-r-dependencies@v2 with: + install-quarto: false pak-version: devel dependencies: '"hard"' cache: false diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 110d148cb..f0848b43b 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -70,6 +70,7 @@ jobs: - uses: r-lib/actions/setup-r-dependencies@v2 with: + install-quarto: false extra-packages: | any::rcmdcheck needs: check