Skip to content

Commit e32824e

Browse files
committed
Gate complex default warning on class check
`new_property()` warned for any non-scalar `default` before checking whether the value matched the declared property class. With an invalid non-scalar default such as `new_property(class_integer, default = c("x", "y"))`, this warning was emitted before the class check, so callers saw both the new deprecation warning and the existing type error. Under `options(warn = 2)`, they only saw the warning-as-error and lost the real validation message. Move this warning after confirming `class_inherits(default, class)` so invalid defaults keep the existing clear error path. Add regression coverage for the invalid complex default path with warnings converted to errors.
1 parent 8fc2a79 commit e32824e

3 files changed

Lines changed: 14 additions & 4 deletions

File tree

DESCRIPTION

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ Suggests:
3737
methods,
3838
rmarkdown,
3939
testthat (>= 3.2.0),
40-
tibble
40+
tibble,
41+
withr
4142
VignetteBuilder:
4243
knitr
4344
Config/build/compilation-database: true

R/property.R

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,10 @@ check_prop_default <- function(
168168
return()
169169
}
170170

171-
if (!is_scalar(default)) {
172-
warn_prop_default_complex(default, default_expr, call)
173-
}
174171
if (class_inherits(default, class)) {
172+
if (!is_scalar(default)) {
173+
warn_prop_default_complex(default, default_expr, call)
174+
}
175175
return()
176176
}
177177

tests/testthat/test-property.R

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,15 @@ describe("new_property()", {
434434
})
435435
})
436436

437+
it("rejects invalid complex defaults without warning first", {
438+
withr::local_options(list(warn = 2))
439+
440+
expect_error(
441+
new_property(class_integer, default = c("x", "y")),
442+
"`default` must be an instance of <integer>"
443+
)
444+
})
445+
437446
it("warns if default is not a scalar or quoted call", {
438447
expect_snapshot({
439448
. <- new_property(class_integer, default = c(any = 1L))

0 commit comments

Comments
 (0)