Validation improvements

R/class.R

@@ -50,10 +50,20 @@ is_class <- function(x) inherits(x, "R7_class")
 #'
 #' @param constructor The constructor function. This is optional, unless
 #'   you want to control which properties can be set on constructor.
-#' @param validator A function used to determine whether or not an object
-#'   is valid. This is called automatically after construction, and
-#'   whenever any property is set. It should return `NULL` if the object is
-#'   valid, and otherwise return a character vector of problems.
+#' @param validator A function taking a single argument, the object to validate.
+#'
+#'   The job of a validator is to determine whether the object is valid,
+#'   i.e. if the current property values form an allowed combination. The
+#'   types of the properties are always automatically validator so the job of
+#'   the validator is to verify that the value of individual properties is
+#'   ok (i.e. maybe a property should have length 1, or should always be
+#'   positive), or that the combination of values of multiple properties is ok.
+#'   It is called after construction and whenever any property is set.
+#'
+#'   The validator should return `NULL` if the object is valid. If not, it
+#'   should return a character vector where each element describes a single
+#'   problem. It's generally helpful to report as many problems at once
+#'   as possible.
 #' @param properties A list specifying the properties (data) that
 #'   every object of the class will possess. Each property can either be
 #'   a named string (specifying the class), or a call to [new_property()],

R/valid.R

@@ -1,13 +1,15 @@
 #' Validate an R7 object
 #'
 #' @description
-#' `validate()` calls the validator of an R7 object. This is done automatically
-#' when creating new objects (at the end of [new_object()]) and when setting
-#' any property with [prop<-].
+#' `validate()` ensures that an R7 object is valid by calling the `validator`
+#' provided in [new_class()]. This is done automatically when constructing new
+#' objects and when modifying properties.
 #'
 #' `valid_eventually()` disables validation, modifies the object, then
 #' revalidates. This is useful when a sequence of operations would otherwise
-#' lead an object to be temporarily invalid.
+#' lead an object to be temporarily invalid, or when repeated property
+#' modification causes a performance bottleneck because the validator is
+#' relatively expensive.
 #'
 #' `valid_implicitly()` does the same but does not validate the object at the
 #' end. It should only be used rarely, and in performance critical code where
@@ -68,22 +70,24 @@ validate <- function(object, properties = TRUE) {
   # is likely to return spurious errors
   if (properties) {
     errors <- validate_properties(object, class)
-  } else {
-    errors <- character()
+    if (length(errors) > 0) {
+      bullets <- paste0("- ", errors, collapse = "\n")
+      msg <- sprintf("%s object properties are invalid:\n%s", obj_desc(object), bullets)
+      stop(msg, call. = FALSE)
+    }
   }
 
   # Next, recursively validate the object
-  if (length(errors) == 0) {
-    while(!is.null(class) && is_class(class)) {
-      errors <- c(errors, class@validator(object))
-      class <- prop_safely(class, "parent")
-    }
+  errors <- character()
+  while(!is.null(class) && is_class(class)) {
+    errors <- c(errors, class@validator(object))
+    class <- prop_safely(class, "parent")
   }
 
   # If needed, report errors
   if (length(errors) > 0) {
     bullets <- paste0("- ", errors, collapse = "\n")
-    msg <- sprintf("Invalid %s object:\n%s", obj_desc(object), bullets)
+    msg <- sprintf("%s object is invalid:\n%s", obj_desc(object), bullets)
     stop(msg, call. = FALSE)
   }
 
@@ -128,5 +132,6 @@ valid_implicitly <- function(object, fun) {
   attr(object, ".should_validate") <- FALSE
   out <- fun(object)
   attr(out, ".should_validate") <- old
-  invisible(out)
+
+  out
 }

R/zzz.R

@@ -19,7 +19,7 @@ new_base_class <- function(name) {
     validator = function(object) {
       data <- unclass(object)
       if (!name %in% .class2(data)) {
-        sprintf("`.data` must be <%s> not %s", name, obj_desc(data))
+        sprintf("Underlying data must be <%s> not %s", name, obj_desc(data))
       }
     }
   )

tests/testthat/_snaps/class.md

@@ -39,6 +39,6 @@
 
 # constructor types check their values
 
-    Invalid <integer> object:
-    - `.data` must be <integer> not <character>
+    <integer> object is invalid:
+    - Underlying data must be <integer> not <character>
 

tests/testthat/_snaps/object.md

@@ -3,7 +3,7 @@
     Code
       range(start = "x", end = "y")
     Error <simpleError>
-      Invalid <range> object:
+      <range> object properties are invalid:
       - <range>@start must be of class <integer> or <double>, not <character>
       - <range>@end must be of class <integer> or <double>, not <character>
 

tests/testthat/_snaps/valid.md

@@ -5,13 +5,13 @@
       attr(obj, "x") <- -1
       validate(obj)
     Error <simpleError>
-      Invalid <klass> object:
+      <klass> object is invalid:
       - x must be positive
     Code
       attr(obj, "x") <- "y"
       validate(obj)
     Error <simpleError>
-      Invalid <klass> object:
+      <klass> object properties are invalid:
       - <klass>@x must be of class <double>, not <character>
 
 ---
@@ -21,14 +21,14 @@
       attr(obj, "x") <- -1
       validate(obj)
     Error <simpleError>
-      Invalid <klass2> object:
+      <klass2> object is invalid:
       - x must be positive
     Code
       attr(obj, "x") <- "y"
       attr(obj, "z") <- "y"
       validate(obj)
     Error <simpleError>
-      Invalid <klass2> object:
+      <klass2> object properties are invalid:
       - <klass2>@x must be of class <double>, not <character>
       - <klass2>@z must be of class <double>, not <character>
 
@@ -37,6 +37,6 @@
     Code
       validate(x)
     Error <simpleError>
-      Invalid <Double> object:
-      - `.data` must be <double> not <character>
+      <Double> object is invalid:
+      - Underlying data must be <double> not <character>