Mutating & erring S3-dispatch attempts

[mmuurr]

structure() modifies the object (unexpectedly)

This is somewhat related to #62, but only that it was discovered via the conversation and thoughts around that issue.
I think independent of that issue there's a small change that could prevent a hard-to-track-down problem (for some).
The issue is if toJSON() is ever called -- even when it shouldn't be -- on a reference-semantics object, like an R6 object, simply calling toJSON() will 'damage' the original object.
Here's a trivial example:

x <- R6::R6Class("Klass")$new()  ## do-nothing Klass

class(x)
# [1] "Klass" "R6"

x
# <Klass>
#   Public:
#     clone: function (deep = FALSE)

jsonlite::toJSON(x)  ## won't work, because no methods for Klass nor R6
# Error: No method asJSON S3 class: R6

class(x)  ## but now `x` has been stripped of Klass! this is (probably) unexpected
# [1] "R6"

x  ## indeed, `x` has been modified _in-place_
# <R6>
#   Public:
#     clone: function (deep = FALSE)

The culprit is this line, where structure() modifies reference-semantics objects in-place:

jsonlite/R/asJSON.ANY.R

Line 13 in 0134cba

return(asJSON(structure(x, class = class(x)[-1]), force = force, ...))

I'm not aware of an obvious way to deal with this directly via the object x, and attempting to clone the object (to then recurse on the cloned version) seems both inefficient and way of out scope for {jsonlite}.

BUT, since at this point in the dispatch code we'd be dealing with S3 objects (well, not S4 objects), and the existing code effectively replicates the NextMethod() procedure, we can conceivably look for the first non-NULL getMethod() result, like so:

...
} else if (length(class(x)) > 1) {
  for (cls in class(x)[-1]) {  ## this should always fail for the first el of class(), else we'd already have dispatched to its method
    f <- getMethod("asJSON", cls, optional = TRUE)
    if (!is.null(f)) f(x, ...)  ## note that the `force` arg is no longer needed since we found our method
  }
} else if ...

toJSON() errs via unclass(), even when force = TRUE is used.

A second unexpected error can happen when unclass(x) fails, even with force = TRUE, here:

jsonlite/R/asJSON.ANY.R

Lines 14 to 16 in 0134cba

	} else if (isTRUE(force) && existsMethod("asJSON", class(unclass(x))[1])) {
	# As a last resort we can force encoding using the unclassed object
	return(asJSON(unclass(x), force = force, ...))

thing <- R6::R6Class("Thing")$new()
jsonlite::toJSON(thing, force = TRUE)

# Error in unclass(x) : cannot unclass an environment

This one is easy-enough to handle, with some try() or tryCatch() logic, I think.

PR

I've opened a PR that helps to deal with these cases; all tests pass and I think there's only minimal additional overhead (versus the structure() & unclass() recursive approach). I haven't added tests yet for preventing either the mutation of x or erring (when unclass(x) fails), but if this PR has any potential future of being merged, I'm obviously happy to write those test cases, too.

Side-note, is it possible these two lines are out-of-order?

jsonlite/R/asJSON.ANY.R

Lines 18 to 19 in 0134cba

	return(asJSON(NULL))
	warning("No method asJSON S3 class: ", class(x))