Reserve some property names (#484)

* Reserve some property names

closes #425

* allow property name `levels`

* Also, disallow `...` as a prop name.

R/class.R

@@ -130,6 +130,7 @@ new_class <- function(
   # Combine properties from parent, overriding as needed
   all_props <- attr(parent, "properties", exact = TRUE) %||% list()
   new_props <- as_properties(properties)
+  check_prop_names(new_props)
   all_props[names(new_props)] <- new_props
 
   if (is.null(constructor)) {
@@ -327,3 +328,16 @@ str.S7_object <- function(object, ..., nest.lev = 0) {
 S7_class <- function(object) {
   attr(object, "S7_class", exact = TRUE)
 }
+
+
+check_prop_names <- function(properties, error_call = sys.call(-1L)) {
+  # these attributes have special C handlers in base R
+  forbidden <- c("names", "dim", "dimnames", "class",
+                 "tsp", "comment", "row.names", "...")
+  forbidden <- intersect(forbidden, names(properties))
+  if (length(forbidden)) {
+    msg <- paste0("property can't be named: ",
+                  paste0(forbidden, collapse = ", "))
+    stop(simpleError(msg, error_call))
+  }
+}

tests/testthat/_snaps/class.md

@@ -225,3 +225,21 @@
       Error:
       ! Can not combine S7 class objects
 
+# can't create class with reserved property names
+
+    Code
+      new_class("foo", properties = list(names = class_character))
+    Condition
+      Error in `new_class()`:
+      ! property can't be named: names
+    Code
+      new_class("foo", properties = list(dim = NULL | class_integer))
+    Condition
+      Error in `new_class()`:
+      ! property can't be named: dim
+    Code
+      new_class("foo", properties = list(dim = NULL | class_integer, dimnames = class_list))
+    Condition
+      Error in `new_class()`:
+      ! property can't be named: dim, dimnames
+

tests/testthat/test-class.R

@@ -243,3 +243,13 @@ test_that("can round trip to disk and back", {
 
   expect_equal(f2, f)
 })
+
+
+test_that("can't create class with reserved property names", {
+  expect_snapshot(error = TRUE, {
+    new_class("foo", properties = list(names = class_character))
+    new_class("foo", properties = list(dim = NULL | class_integer))
+    new_class("foo", properties = list(dim = NULL | class_integer,
+                                       dimnames = class_list))
+  })
+})