Add support to dump superclass nodes

[tahirmt]

Adding support for the following things

• By default includes all properties from the superclasses

Closes #57

[tahirmt]

@stephencelis This is a continuation of https://github.com/pointfreeco/swift-snapshot-testing/pull/597 here because I was setting up a plugin (https://github.com/tahirmt/swift-snapshot-testing-dump) to use this library for the dump but these features are still good to include since I want to have control over what gets dumped.

[stephencelis]

@tahirmt Thanks for moving things here, and for the issue! I think we should definitely include superclass properties by default in our dump. And folks that don't want them can use a custom mirror using the existing protocols.

Because of this I think we could simplify the implementation. Can we avoid the new protocols that you introduced for configuration and instead always use the superclass mirror? Do you foresee any issues with such a change?

[tahirmt]

@stephencelis I was noticing issues with system classes like CFNumber has a superclassMirror of type NSNumber and many of the system types have duplicate mirrors like that.

[stephencelis]

Interesting. For things like CFNumber and NSNumber we should be providing library-level conformances that convert to simple bools, numbers. Are there others that output strangely? Wanna share a few examples here?

[tahirmt]

Certainly

There's overall 25 failures. I agree we could provide an internal way of not logging superclass for these and provide an inverse protocol to hide superclass mirror someone wants to do it

Mirror: Mirror for KeyPath<UserClass, String>
Superclass Mirror: Mirror for PartialKeyPath<UserClass>
Mirror: Mirror for PartialKeyPath<UserClass>
Superclass Mirror: Mirror for AnyKeyPath

Mirror: Mirror for WritableKeyPath<(x: Double, y: Double), Double>
Superclass Mirror: Mirror for KeyPath<(x: Double, y: Double), Double>
Mirror: Mirror for KeyPath<(x: Double, y: Double), Double>
Superclass Mirror: Mirror for PartialKeyPath<(x: Double, y: Double)>
Mirror: Mirror for PartialKeyPath<(x: Double, y: Double)>
Superclass Mirror: Mirror for AnyKeyPath

Mirror: Mirror for __NSCFNumber
Superclass Mirror: Mirror for NSNumber
Mirror: Mirror for NSNumber
Superclass Mirror: Mirror for NSValue
Mirror: Mirror for NSValue
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for NSConcreteMutableAttributedString
Superclass Mirror: Mirror for NSMutableAttributedString
Mirror: Mirror for NSMutableAttributedString
Superclass Mirror: Mirror for NSAttributedString
Mirror: Mirror for NSAttributedString
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for _NSCopyOnWriteCalendarWrapper
Superclass Mirror: Mirror for NSCalendar
Mirror: Mirror for NSCalendar
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for _NSInlineData
Superclass Mirror: Mirror for NSData
Mirror: Mirror for NSData
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for __NSTaggedDate
Superclass Mirror: Mirror for NSDate
Mirror: Mirror for NSDate
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for NSTaggedPointerString
Superclass Mirror: Mirror for NSString
Mirror: Mirror for NSString
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for __NSCFString
Superclass Mirror: Mirror for NSMutableString
Mirror: Mirror for NSMutableString
Superclass Mirror: Mirror for NSString
Mirror: Mirror for NSString
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for NSFunctionExpression
Superclass Mirror: Mirror for NSExpression
Mirror: Mirror for NSExpression
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for __NSCFLocale
Superclass Mirror: Mirror for NSLocale
Mirror: Mirror for NSLocale
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for NSConcreteNotification
Superclass Mirror: Mirror for NSNotification
Mirror: Mirror for NSNotification
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for __NSTimeZone
Superclass Mirror: Mirror for NSTimeZone
Mirror: Mirror for NSTimeZone
Superclass Mirror: Mirror for NSObject

Mirror: Mirror for NSMutableURLRequest
Superclass Mirror: Mirror for NSURLRequest
Mirror: Mirror for NSURLRequest
Superclass Mirror: Mirror for NSObject

[stephencelis]

@tahirmt If you push a failing branch I'd love to take a look at the actual dump output to get a feel.

[iampatbrown]

@stephencelis This reminded me of a possible bug I might have mentioned to you a while back. The listed items would prevent items from dumping even if if they weren't recursive. Unsure if it's related. I'll be able to check it out on the weekend. Here's a branch with the issue I'm referring to https://github.com/pointfreeco/swift-custom-dump/compare/main...iampatbrown:repeated-types

[tahirmt]

@iampatbrown what issue is there on your branch? Your solution seems to be nicer than mine actually in that it combines the superclass properties with the subclass properties in the dump

[tahirmt]

Ohh I think I see now. You're talking about a scenario where the object is repeated but there is no recursion it still skips the object from the dump because it was already dumped. I still feel it doesn't really add value to dump it over and over but perhaps adding a way to identify which one it is because there could be multiple instances of the same type where only one is repeated.

What if we assign each instance of any class a unique identifier, e.g.

class User { 
  let name: String
  init(name: String) { self.name = name }
}

let user = User(name: "James")
let user2 = User(name: "John")

dump = ""
    customDump([
      user,
      user,
      user2,
      user2
    ], to: &dump)

This will dump

[
     [0]: User(name: "James"),
     [1]: User(↩︎),
     [2]: User(name: "John"),
     [3]: User(↩︎)
   ]

So perhaps we can assign an identifier to it instead so it prints something like this

[
     [0]: User(name: "James"),
     [1]: User(↩︎),
     [2]: User<1>(name: "John"),
     [3]: User<1>(↩︎)
   ]

And by not assigning a number to the first instance we will make sure most of the dumps won't include identifiers. This will help when we have different instances of the same type also getting repeated. I can work on that too but that would have to be a separate issue and PR. Thi

[iampatbrown]

You're talking about a scenario where the object is repeated but there is no recursion it still skips the object from the dump because it was already dumped.

Yep. That's it. I'm unsure if it's the desired behaviour or not. But it was problematic in my case.

It might be worth leveraging Mirror.AncestorRepresentation which could be used with CustomDumpReflectable for initialising the custom mirror. Would this provide the functionality you need without introducing a new protocol?

[tahirmt]

@iampatbrown I've never used ancestorRepresentation but from what I can see in the documentation it is possible although my new protocols provide a lot nicer for control and implementation. That being said perhaps it is better to split this PR into two and only have one to include superclass properties by default. And we can consider new protocols as a separate item on a different PR.

Curious what @stephencelis has to say about adding a way to identify the repeated objects in the dump without repeated dumps because that could end up being a lot of extra lines?

[tahirmt]

I have remove the additional protocols from this PR and only left the superclass properties in this PR for now.

[tahirmt]

@iampatbrown implemented my idea about assigning ids to track repeat occurrences https://github.com/pointfreeco/swift-custom-dump/pull/60

[tahirmt]

@stephencelis is there anything else I can do to get this moved forward?

[stephencelis]

Hi @tahirmt! Sorry for the delay! Just haven’t had time to catch up on this. Will take a look later today.

[stephencelis]

@tahirmt Thanks for providing this much simpler implementation! I think it looks good, but will chat with Brandon about it on Monday.