Skip to content

Commit a49bf1e

Browse files
authored
Check ellipses in several places (#6031)
* helper function to throw error demoted to warning * check `fortify()` dots * check `get_alt_text()` dots * check `deprecated_guide_args()` dots * check for unknown labels * add news bullet
1 parent 2e08bba commit a49bf1e

File tree

13 files changed

+95
-8
lines changed

13 files changed

+95
-8
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@
168168
* `theme_classic()` now has black ticks and text instead of dark gray. In
169169
addition, `theme_classic()`'s axis line end is `"square"` (@teunbrand, #5978).
170170
* {tibble} is now suggested instead of imported (@teunbrand, #5986)
171+
* The ellipsis argument is now checked in `fortify()`, `get_alt_text()`,
172+
`labs()` and several guides (@teunbrand, #3196).
171173

172174
# ggplot2 3.5.1
173175

R/fortify.R

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@
88
#' @seealso [fortify.lm()]
99
#' @param model model or other R object to convert to data frame
1010
#' @param data original dataset, if needed
11-
#' @param ... other arguments passed to methods
11+
#' @inheritParams rlang::args_dots_used
1212
#' @export
13-
fortify <- function(model, data, ...) UseMethod("fortify")
13+
fortify <- function(model, data, ...) {
14+
warn_dots_used()
15+
UseMethod("fortify")
16+
}
1417

1518
#' @export
1619
fortify.data.frame <- function(model, data, ...) model

R/guide-legend.R

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,7 @@ deprecated_guide_args <- function(
688688
default.unit = "line",
689689
...,
690690
.call = caller_call()) {
691+
warn_dots_used(call = .call)
691692

692693
args <- names(formals(deprecated_guide_args))
693694
args <- setdiff(args, c("theme", "default.unit", "...", ".call"))

R/labels.R

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ update_labels <- function(p, labels) {
1818

1919
# Called in `ggplot_build()` to set default labels not specified by user.
2020
setup_plot_labels <- function(plot, layers, data) {
21-
# Initiate from user-defined labels
22-
labels <- plot$labels
21+
# Initiate empty labels
22+
labels <- list()
2323

2424
# Find labels from every layer
2525
for (i in seq_along(layers)) {
@@ -65,7 +65,26 @@ setup_plot_labels <- function(plot, layers, data) {
6565
labels <- defaults(labels, current)
6666
}
6767
}
68-
labels
68+
69+
# Warn for spurious labels that don't have a mapping.
70+
# Note: sometimes, 'x' and 'y' might not have a mapping, like in
71+
# `geom_function()`. We can display these labels anyway, so we include them.
72+
plot_labels <- plot$labels
73+
known_labels <- c(names(labels), fn_fmls_names(labs), "x", "y")
74+
extra_labels <- setdiff(names(plot_labels), known_labels)
75+
76+
if (length(extra_labels) > 0) {
77+
extra_labels <- paste0(
78+
"{.code ", extra_labels, " = \"", plot_labels[extra_labels], "\"}"
79+
)
80+
names(extra_labels) <- rep("*", length(extra_labels))
81+
cli::cli_warn(c(
82+
"Ignoring unknown labels:",
83+
extra_labels
84+
))
85+
}
86+
87+
defaults(plot_labels, labels)
6988
}
7089

7190
#' Modify axis, legend, and plot labels
@@ -168,7 +187,7 @@ ggtitle <- function(label, subtitle = waiver()) {
168187
#' text from the information stored in the plot.
169188
#'
170189
#' @param p a ggplot object
171-
#' @param ... Currently ignored
190+
#' @inheritParams rlang::args_dots_used
172191
#'
173192
#' @return A text string
174193
#'
@@ -191,6 +210,7 @@ ggtitle <- function(label, subtitle = waiver()) {
191210
#' get_alt_text(p)
192211
#'
193212
get_alt_text <- function(p, ...) {
213+
warn_dots_used()
194214
UseMethod("get_alt_text")
195215
}
196216
#' @export

R/utilities.R

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,18 @@ as_unordered_factor <- function(x) {
843843
x
844844
}
845845

846+
warn_dots_used <- function(env = caller_env(), call = caller_env()) {
847+
check_dots_used(
848+
env = env, call = call,
849+
# Demote from error to warning
850+
error = function(cnd) {
851+
# cli uses \f as newlines, not \n
852+
msg <- gsub("\n", "\f", cnd_message(cnd))
853+
cli::cli_warn(msg, call = call)
854+
}
855+
)
856+
}
857+
846858
# Shim for scales/#424
847859
col_mix <- function(a, b, amount = 0.5) {
848860
input <- vec_recycle_common(a = a, b = b, amount = amount)

man/fortify.Rd

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

man/get_alt_text.Rd

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testthat/_snaps/guides.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
# dots are checked when making guides
2+
3+
Ignoring unknown argument to `guide_axis()`: `foo`.
4+
5+
---
6+
7+
Arguments in `...` must be used.
8+
x Problematic argument:
9+
* foo = "bar"
10+
i Did you misspell an argument name?
11+
112
# Using non-position guides for position scales results in an informative error
213

314
`guide_legend()` cannot be used for x, xmin, xmax, or xend.

tests/testthat/_snaps/labels.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@
55
Output
66
[1] "A plot showing class on a discrete x-axis and count on a continuous y-axis using a bar layer."
77

8+
# get_alt_text checks dots
9+
10+
Arguments in `...` must be used.
11+
x Problematic argument:
12+
* foo = "bar"
13+
i Did you misspell an argument name?
14+
15+
# warnings are thrown for unknown labels
16+
17+
Ignoring unknown labels:
18+
* `foo = "bar"`
19+
820
# plot.tag.position rejects invalid input
921

1022
The `plot.tag.position` theme element must be a <character/numeric/integer> object.

tests/testthat/_snaps/plot.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88
`data` cannot be a function.
99
i Have you misspelled the `data` argument in `ggplot()`
1010

11+
---
12+
13+
Arguments in `...` must be used.
14+
x Problematic argument:
15+
* foobar = "nonsense"
16+
i Did you misspell an argument name?
17+
1118
# construction have user friendly errors
1219

1320
Cannot use `+` with a single argument.

tests/testthat/test-guides.R

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,15 @@ test_that("show.legend handles named vectors", {
8787
expect_equal(n_legends(p), 1)
8888
})
8989

90+
test_that("dots are checked when making guides", {
91+
expect_snapshot_warning(
92+
new_guide(foo = "bar", super = GuideAxis)
93+
)
94+
expect_snapshot_warning(
95+
guide_legend(foo = "bar")
96+
)
97+
})
98+
9099
test_that("axis_label_overlap_priority always returns the correct number of elements", {
91100
expect_identical(axis_label_priority(0), numeric(0))
92101
expect_setequal(axis_label_priority(1), seq_len(1))

tests/testthat/test-labels.R

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,15 @@ test_that("alt text can take a function", {
104104
expect_snapshot(get_alt_text(p))
105105
})
106106

107+
test_that("get_alt_text checks dots", {
108+
expect_snapshot_warning(get_alt_text(ggplot(), foo = "bar"))
109+
})
110+
111+
test_that("warnings are thrown for unknown labels", {
112+
p <- ggplot(mtcars, aes(mpg, disp)) + geom_point() + labs(foo = 'bar')
113+
expect_snapshot_warning(ggplot_build(p))
114+
})
115+
107116
test_that("plot.tag.position rejects invalid input", {
108117
p <- ggplot(mtcars, aes(mpg, disp)) + geom_point() + labs(tag = "Fig. A)")
109118

tests/testthat/test-plot.R

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
test_that("ggplot() throws informative errors", {
22
expect_snapshot_error(ggplot(mapping = letters))
33
expect_snapshot_error(ggplot(data))
4+
expect_snapshot_warning(ggplot(foobar = "nonsense"))
45
})
56

67
test_that("construction have user friendly errors", {

0 commit comments

Comments
 (0)