Type checking for dynamic properties

[t-kalinowski]

Dynamic properties currently lack type checking:

Foo <- new_class("Foo", properties = list(
  double = new_property(
    class_double,
    getter = function(self) "not a double"
  )
))

Foo()@double
#> [1] "not a double"

Options:

1. No change: class remains informative in printouts but is not enforced.
2. Require that class == class_any if a getter is supplied to new_property().
3. Validate property value from getter before returning from prop().

[t-kalinowski]

This overlaps with the print methods for S7_class as well as S7_object, both of which may be displaying incorrect information for dynamic properties.

library(S7)
Foo <- new_class("Foo", properties = list(
  double = new_property(
    class_double,
    getter = function(self) "not a double"
  )
))

Foo
#> <Foo> class
#> @ parent     : <S7_object>
#> @ constructor: function() {...}
#> @ validator  : <NULL>
#> @ properties :
#>  $ double: <double>

Foo()
#> <Foo>
#>  @ double: chr "not a double"

[lawremi]

I guess (4) would be to not skip dynamic properties during object validation. They're probably skipped for performance reasons, but that might not be a good enough reason. It's likely that checking a property on every access, option (3), would have a bigger impact than just checking them during validation. Of course, checking them on every access is the only way to guarantee that they are valid.

[hadley]

I think we can make a variation on (1) — that a conflict between the declared type and the provided type is undefined behaviour that the class developer needs to fix.

[t-kalinowski]

The conclusion we reached in #451 means that getters are expected to be safe and inexpensive to call. In light of that, I'm starting to think we should run validators on dynamic properties as well.

[hadley]

Hmmmm, I wonder if we should check them at development-time but not use-time? (i.e. because only the class developer can fix such a problem). But I don't know how we could make that distinction.

[lawremi]

I would stick with (1) for now, assuming they are correct. Dynamic properties are essentially sugar for an accessor. We've never considered checking the return value on those. The class developer can always check them in the class-level validator.