Skip to content

Commit 5bd2b12

Browse files
committed
Reject implicit base property narrowing
1 parent cd561d4 commit 5bd2b12

2 files changed

Lines changed: 28 additions & 95 deletions

File tree

R/class-spec.R

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,6 @@ class_extends <- function(child, parent) {
319319
} else if (is_union(child)) {
320320
# A union child extends `parent` only if every one of its members does.
321321
all(vlapply(child$classes, class_extends, parent = parent))
322-
} else if (class_extends_implicit_base(child, parent)) {
323-
TRUE
324322
} else if (is_union(parent)) {
325323
# A non-union child extends a union parent if it extends any of its members.
326324
any(vlapply(parent$classes, class_extends, child = child))
@@ -397,37 +395,6 @@ union_contains_any <- function(x) {
397395
is_union(x) && any(vlapply(x$classes, is_class_any))
398396
}
399397

400-
class_extends_implicit_base <- function(child, parent) {
401-
base <- class_implicit_base(child)
402-
!is.null(base) && class_extends(base, parent)
403-
}
404-
405-
class_implicit_base <- function(x) {
406-
switch(
407-
class_type(x),
408-
S4 = S4_implicit_base(x),
409-
S7 = class_implicit_base(x@parent),
410-
S7_S3 = bundled_S3_implicit_base(x),
411-
NULL
412-
)
413-
}
414-
415-
S4_implicit_base <- function(x) {
416-
extensions <- methods::extends(x, fullInfo = TRUE)
417-
basic_classes <- S4_basic_base_classes()
418-
419-
for (class in names(extensions)) {
420-
if (
421-
hasName(basic_classes, class) &&
422-
S4_extension_is_data_part(extensions[[class]])
423-
) {
424-
return(basic_classes[[class]])
425-
}
426-
}
427-
428-
NULL
429-
}
430-
431398
S4_extends_unconditionally <- function(child, parent) {
432399
extension <- methods::extends(
433400
child@className,
@@ -438,45 +405,5 @@ S4_extends_unconditionally <- function(child, parent) {
438405
(isS4(extension) && !methods::is(extension, "conditionalExtension"))
439406
}
440407

441-
S4_extension_is_data_part <- function(extension) {
442-
isS4(extension) &&
443-
isTRUE(methods::slot(extension, "simple")) &&
444-
isTRUE(methods::slot(extension, "dataPart"))
445-
}
446-
447-
S4_basic_base_classes <- function() {
448-
list(
449-
logical = class_logical,
450-
integer = class_integer,
451-
double = class_double,
452-
numeric = class_numeric,
453-
character = class_character,
454-
complex = class_complex,
455-
raw = class_raw,
456-
list = class_list,
457-
expression = class_expression,
458-
vector = class_vector,
459-
`function` = class_function,
460-
environment = class_environment,
461-
name = class_name,
462-
call = class_call
463-
)
464-
}
465-
466-
bundled_S3_implicit_base <- function(x) {
467-
# Arbitrary S3 classes do not declare a base contract; bundled classes do.
468-
if (identical(x, class_factor)) {
469-
class_integer
470-
} else if (identical(x, class_Date) || identical(x, class_POSIXct)) {
471-
class_numeric
472-
} else if (identical(x, class_POSIXlt) || identical(x, class_data.frame)) {
473-
class_list
474-
} else if (identical(x, class_matrix) || identical(x, class_array)) {
475-
class_vector
476-
} else {
477-
NULL
478-
}
479-
}
480-
481408
# Suppress @className false positive
482409
globalVariables("className")

tests/testthat/test-class.R

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -95,30 +95,36 @@ describe("inheritance", {
9595
properties = list(x = new_property(Child, default = quote(Child())))
9696
))
9797
})
98-
it("child properties can narrow base properties with bundled S3 classes", {
98+
it("child properties can't narrow base properties with bundled S3 classes", {
9999
integer_parent <- new_class(
100100
"integer_parent",
101101
properties = list(x = class_integer),
102102
package = NULL
103103
)
104-
expect_no_error(new_class(
105-
"factor_child",
106-
integer_parent,
107-
properties = list(x = class_factor),
108-
package = NULL
109-
))
104+
expect_error(
105+
new_class(
106+
"factor_child",
107+
integer_parent,
108+
properties = list(x = class_factor),
109+
package = NULL
110+
),
111+
"must narrow"
112+
)
110113

111114
numeric_parent <- new_class(
112115
"numeric_parent",
113116
properties = list(x = class_numeric),
114117
package = NULL
115118
)
116-
expect_no_error(new_class(
117-
"date_child",
118-
numeric_parent,
119-
properties = list(x = class_Date),
120-
package = NULL
121-
))
119+
expect_error(
120+
new_class(
121+
"date_child",
122+
numeric_parent,
123+
properties = list(x = class_Date),
124+
package = NULL
125+
),
126+
"must narrow"
127+
)
122128
})
123129
it("child properties can't narrow S7_object with base or S3 classes", {
124130
Parent <- new_class(
@@ -180,7 +186,7 @@ describe("inheritance", {
180186
"must narrow"
181187
)
182188
})
183-
it("child properties can narrow base properties with S4 subclasses", {
189+
it("child properties can't narrow base properties with S4 subclasses", {
184190
on.exit(S4_remove_classes("S4PropertyNum"))
185191
S4PropertyNum <- methods::setClass("S4PropertyNum", contains = "numeric")
186192

@@ -189,15 +195,15 @@ describe("inheritance", {
189195
properties = list(x = class_numeric),
190196
package = NULL
191197
)
192-
Child <- new_class(
193-
"Child",
194-
Parent,
195-
properties = list(x = S4PropertyNum),
196-
package = NULL
198+
expect_error(
199+
new_class(
200+
"Child",
201+
Parent,
202+
properties = list(x = S4PropertyNum),
203+
package = NULL
204+
),
205+
"must narrow"
197206
)
198-
199-
x <- methods::new("S4PropertyNum", 1)
200-
expect_s4_class(Child(x = x)@x, "S4PropertyNum")
201207
})
202208
it("child properties can't narrow with conditional S4 extensions", {
203209
on.exit(S4_remove_classes(c(

0 commit comments

Comments
 (0)