Skip to content

Commit 7798431

Browse files
yjunechoeteunbrand
andauthored
Ensure ggproto methods start with { (#6460)
* add curly braces * add test * spell out identity() * bracket `Scale$palette()` * report potential test failures * add positive/negative control to test * fix test --------- Co-authored-by: Teun van den Brand <[email protected]>
1 parent 5ad8736 commit 7798431

File tree

11 files changed

+127
-26
lines changed

11 files changed

+127
-26
lines changed

R/coord-cartesian-.R

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,14 @@ coord_cartesian <- function(xlim = NULL, ylim = NULL, expand = TRUE,
9797
#' @export
9898
CoordCartesian <- ggproto("CoordCartesian", Coord,
9999

100-
is_linear = function() TRUE,
101-
is_free = function(self) is.null(self$ratio),
100+
is_linear = function() {
101+
TRUE
102+
},
103+
104+
is_free = function(self) {
105+
is.null(self$ratio)
106+
},
107+
102108
aspect = function(self, ranges) {
103109
if (is.null(self$ratio)) {
104110
return(NULL)

R/coord-fixed.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ coord_equal <- coord_fixed
3737
#' @usage NULL
3838
#' @export
3939
CoordFixed <- ggproto("CoordFixed", CoordCartesian,
40-
is_free = function() FALSE,
40+
is_free = function() {
41+
FALSE
42+
},
4143

4244
aspect = function(self, ranges) {
4345
diff(ranges$y.range) / diff(ranges$x.range) * self$ratio

R/coord-polar.R

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,13 @@ coord_polar <- function(theta = "x", start = 0, direction = 1, clip = "on") {
2020
#' @export
2121
CoordPolar <- ggproto("CoordPolar", Coord,
2222

23-
aspect = function(details) 1,
23+
aspect = function(details) {
24+
1
25+
},
2426

25-
is_free = function() TRUE,
27+
is_free = function() {
28+
TRUE
29+
},
2630

2731
distance = function(self, x, y, details, boost = 0.75) {
2832
arc <- self$start + c(0, 2 * pi)

R/coord-radial.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,9 @@ CoordRadial <- ggproto("CoordRadial", Coord,
199199
diff(details$bbox$y) / diff(details$bbox$x)
200200
},
201201

202-
is_free = function() TRUE,
202+
is_free = function() {
203+
TRUE
204+
},
203205

204206
distance = function(self, x, y, details, boost = 0.75) {
205207
arc <- details$arc %||% c(0, 2 * pi)

R/coord-sf.R

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,11 +303,15 @@ CoordSf <- ggproto("CoordSf", CoordCartesian,
303303
},
304304

305305
# CoordSf enforces a fixed aspect ratio -> axes cannot be changed freely under faceting
306-
is_free = function() FALSE,
306+
is_free = function() {
307+
FALSE
308+
},
307309

308310
# for regular geoms (such as geom_path, geom_polygon, etc.), CoordSf is non-linear
309311
# if the default_crs option is being used, i.e., not set to NULL
310-
is_linear = function(self) is.null(self$get_default_crs()),
312+
is_linear = function(self) {
313+
is.null(self$get_default_crs())
314+
},
311315

312316
distance = function(self, x, y, panel_params) {
313317
d <- self$backtransform_range(panel_params)
@@ -328,7 +332,9 @@ CoordSf <- ggproto("CoordSf", CoordCartesian,
328332
diff(panel_params$y_range) / diff(panel_params$x_range) / ratio
329333
},
330334

331-
labels = function(labels, panel_params) labels,
335+
labels = function(labels, panel_params) {
336+
labels
337+
},
332338

333339
render_bg = function(self, panel_params, theme) {
334340
el <- calc_element("panel.grid.major", theme)

R/coord-transform.R

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,11 @@ coord_trans <- function(...) {
126126
#' @export
127127
CoordTransform <- ggproto(
128128
"CoordTransform", Coord,
129-
is_free = function() TRUE,
129+
130+
is_free = function() {
131+
TRUE
132+
},
133+
130134
distance = function(self, x, y, panel_params) {
131135
max_dist <- dist_euclidean(panel_params$x.range, panel_params$y.range)
132136
dist_euclidean(self$trans$x$transform(x), self$trans$y$transform(y)) / max_dist

R/geom-blank.R

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ geom_blank <- function(mapping = NULL, data = NULL,
3535
#' @export
3636
GeomBlank <- ggproto("GeomBlank", Geom,
3737
default_aes = aes(),
38-
handle_na = function(data, params) data,
38+
handle_na = function(data, params) {
39+
data
40+
},
3941
check_constant_aes = FALSE,
40-
draw_panel = function(...) nullGrob()
42+
draw_panel = function(...) {
43+
nullGrob()
44+
}
4145
)

R/scale-.R

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,9 @@ Scale <- ggproto("Scale", NULL,
458458
#' as described in e.g. [`?continuous_scale`][continuous_scale].
459459
#' Note that `limits` is expected in transformed space.
460460
aesthetics = character(),
461-
palette = function() cli::cli_abort("Not implemented."),
461+
palette = function() {
462+
cli::cli_abort("Not implemented.")
463+
},
462464

463465
limits = NULL,
464466
na.value = NA,
@@ -998,7 +1000,9 @@ ScaleContinuous <- ggproto("ScaleContinuous", Scale,
9981000
n.breaks = NULL,
9991001
trans = transform_identity(),
10001002

1001-
is_discrete = function() FALSE,
1003+
is_discrete = function() {
1004+
FALSE
1005+
},
10021006

10031007
train = function(self, x) {
10041008
if (length(x) == 0) {
@@ -1257,7 +1261,9 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale,
12571261
n.breaks.cache = NULL,
12581262
palette.cache = NULL,
12591263

1260-
is_discrete = function() TRUE,
1264+
is_discrete = function() {
1265+
TRUE
1266+
},
12611267

12621268
train = function(self, x) {
12631269
if (length(x) == 0) {
@@ -1271,7 +1277,9 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale,
12711277
)
12721278
},
12731279

1274-
transform = identity,
1280+
transform = function(self, x) {
1281+
x
1282+
},
12751283

12761284
map = function(self, x, limits = self$get_limits()) {
12771285
limits <- vec_slice(limits, !is.na(limits))
@@ -1483,7 +1491,9 @@ ScaleBinned <- ggproto("ScaleBinned", Scale,
14831491
after.stat = FALSE,
14841492
show.limits = FALSE,
14851493

1486-
is_discrete = function() FALSE,
1494+
is_discrete = function() {
1495+
FALSE
1496+
},
14871497

14881498
train = function(self, x) {
14891499
if (length(x) == 0) {
@@ -1644,7 +1654,9 @@ ScaleBinned <- ggproto("ScaleBinned", Scale,
16441654
transformation$transform(breaks)
16451655
},
16461656

1647-
get_breaks_minor = function(...) NULL,
1657+
get_breaks_minor = function(...) {
1658+
NULL
1659+
},
16481660

16491661
get_labels = function(self, breaks = self$get_breaks()) {
16501662
if (is.null(breaks)) return(NULL)

R/scale-view.R

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,27 @@ ViewScale <- ggproto("ViewScale", NULL,
107107
is_empty = function(self) {
108108
is.null(self$get_breaks()) && is.null(self$get_breaks_minor())
109109
},
110-
is_discrete = function(self) self$scale_is_discrete,
111-
dimension = function(self) self$continuous_range,
112-
get_limits = function(self) self$limits,
113-
get_breaks = function(self) self$breaks,
114-
get_breaks_minor = function(self) self$minor_breaks,
115-
get_labels = function(self, breaks = self$get_breaks()) self$scale$get_labels(breaks),
116-
get_transformation = function(self) self$scale$get_transformation(),
110+
is_discrete = function(self) {
111+
self$scale_is_discrete
112+
},
113+
dimension = function(self) {
114+
self$continuous_range
115+
},
116+
get_limits = function(self) {
117+
self$limits
118+
},
119+
get_breaks = function(self) {
120+
self$breaks
121+
},
122+
get_breaks_minor = function(self) {
123+
self$minor_breaks
124+
},
125+
get_labels = function(self, breaks = self$get_breaks()) {
126+
self$scale$get_labels(breaks)
127+
},
128+
get_transformation = function(self) {
129+
self$scale$get_transformation()
130+
},
117131
rescale = function(self, x) {
118132
self$scale$rescale(x, self$limits, self$continuous_range)
119133
},

R/stat-unique.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
#' @export
55
StatUnique <- ggproto(
66
"StatUnique", Stat,
7-
compute_panel = function(data, scales) unique0(data)
7+
compute_panel = function(data, scales) {
8+
unique0(data)
9+
}
810
)
911

1012
#' Remove duplicates

tests/testthat/test-ggproto.R

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,48 @@ test_that("construction checks input", {
1010
expect_snapshot_error(ggproto("Test", NULL, a <- function(self, a) a))
1111
expect_snapshot_error(ggproto("Test", mtcars, a = function(self, a) a))
1212
})
13+
14+
test_that("all ggproto methods start with `{` (#6459)", {
15+
16+
ggprotos <- Filter(
17+
function(x) inherits(x, "ggproto"),
18+
mget(ls("package:ggplot2"), asNamespace("ggplot2"), ifnotfound = list(NULL))
19+
)
20+
21+
lacks_brackets <- function(method) {
22+
if (!inherits(method, "ggproto_method")) {
23+
return(FALSE)
24+
}
25+
body <- as.list(body(environment(method)$f))
26+
if (length(body) == 0 || body[[1]] != quote(`{`)) {
27+
return(TRUE)
28+
}
29+
return(FALSE)
30+
}
31+
32+
report_no_bracket <- function(ggproto_class) {
33+
unlist(lapply(
34+
ls(envir = ggproto_class),
35+
function(method) {
36+
has_brackets <- !lacks_brackets(ggproto_class[[method]])
37+
if (has_brackets) {
38+
return(character())
39+
}
40+
return(method)
41+
}
42+
))
43+
}
44+
45+
# Test to make sure we're testing correctly
46+
ctrl <- list(
47+
foo = ggproto("Dummy", dummy = function(x) x + 10),
48+
bar = ggproto("Dummy", dummy = function(x) {x + 10})
49+
)
50+
ctrl <- lapply(ctrl, report_no_bracket)
51+
expect_equal(ctrl, list(foo = "dummy", bar = character()))
52+
53+
# Actual relevant test
54+
failures <- lapply(ggprotos, report_no_bracket)
55+
failures <- failures[lengths(failures) > 0]
56+
expect_equal(names(failures), character())
57+
})

0 commit comments

Comments
 (0)