Extending a parent class with inconsistent properties

[hadley]

Currently it's possible to define a class whose properties are inconsistent with its parent:

library(S7)

foo1 <- new_class("foo1", properties = list(x = class_integer))
foo2 <- new_class("foo2", parent = foo1, properties = list(x = class_character))

This doesn't error when you create the class, but it's not possible to construct it because there's no value that will satisfy the validator of both foo1 and foo2:

foo2(x = 1L)
#> Error: <foo2> object properties are invalid:
#> - @x must be <character>, not <integer>
foo2(x = "x")
#> Error: <foo1> object properties are invalid:
#> - @x must be <integer>, not <character>

Should we make this a define-time error? In principle, I think it's ok for a child class to make a property more restrictive than the parent class, but there's no way to test for test for this in general (since the restriction may occur at the validator level).

[hadley]

I think this should probably be a warning, not an error. For example, imagine that pkg A defines class A and pkg B defines class B which extends class A. If pkg A introduces a new field that conflicts with pkg B, you can continue to somewhat use pkg B (i.e. you only error where there are direct conflicts with the semantics, not at every instantiation).

Similarly if you accidentally introduce a conflict in your own package, it's generally easy to debug if you get a clear warning rather than an error which causes all code loading to fail.

[hadley]

This is a more subtle example of the issue:

library(S7)

foo1 <- new_class("foo1", properties = list(x = class_numeric)
foo2 <- new_class("foo2", parent = foo1, properties = list(x = class_integer))

bar <- new_generic("bar", "x")
method(bar, foo1) <- function(x) {
  x@x <- 1.5
  x
}

bar(foo2(10L))

Narrowing might also happen through the validator:

foo1 <- new_class("foo1", properties = list(x = class_numeric)
foo2 <- new_class("foo2", parent = foo1, properties = list(x = new_property(class_numeric, validator = is_positive))

[hadley]

WG call suggested that it should be an error if the child property class is not a subclass of parent property class, i.e. we only allow narrowing of types. (This only works if the property is largely read only, but that's a common case) Otherwise, it's silent.

[hadley]

Something like this:

  overridden <- intersect(names(all_props), names(new_props))
  if (length(overridden) > 0) {
    for (i in seq_along(overridden)) {
      new <- new_props[[i]]
      old <- all_props[[i]]

      if (!class_extends(new$class, old$class)) {
        msg <- sprintf(
          "%s@%s must extend %s@%2$s.\n* %3$s@%2$s is %s.\n* %1$s@%2$s is %s.",
          name,
          overridden[[i]],
          parent@name,
          class_desc(old$class),
          class_desc(new$class)
        )
        stop(msg, call. = FALSE)
      }
    }
  }

But need to start with definition of class_extends()