Remove all `:destroy()` methods from Fusion

[dphfox]

With the introduction of scopes (#292) it became possible for objects to declare cleanup tasks at construction time. This has turned out to be a massively successful method for preventing memory leaks, and is starting to see wider adoption within Fusion itself for managing memory.

In particular, scopes are fantastic for dealing with the destruction of composed objects. If an object is constructed as part of another object's constructor, historically it would have to also be added to the :destroy() procedure for that composed object. Since developers are fallible (not least myself), this could easily be overlooked.

The :destroy() method presents an additional hazard to end users, in that they may attempt to use it for destroying objects individually, without realising that this is an antipattern and that objects with independent lifetimes should be registered to scopes with independent lifetimes, so as to avoid double-destruction or other issues where destruction tasks can end up scattered across unrelated scopes.

As an alternative, if we're comfortable fossilising ScopedObject.scope, there are two recommendations we can make to end users with regards to avoiding :destroy():

• independent objects can be constructed in inner scopes, e.g. scope:innerScope():Value(...)
• independent objects can be destroyed via object.scope:doCleanup() as an alternative to object:destroy() to ensure that all related cleanup tasks run, not just those explicitly defined in :destroy().

And more generally, as a matter of soundness, I propose that all of Fusion's objects should fully switch to using scopes for registering cleanup tasks, rather than depending on a separate destroy method. This would eliminate the possibility of double destroys, strongly push users towards using scopes, and improve the maintainability and consistency of internal code.

[dphfox]

For a concrete example of what this would look like, take Value.luau (from push-pull-execution):

local function Value<T>(
    scope: Types.Scope<unknown>,
    initialValue: T
): Self<T, any>
    if initialValue == nil and (typeof(scope) ~= "table" or (scope[1] == nil and next(scope) ~= nil)) then
        External.logError("scopeMissing", nil, "Value", "myScope:Value(initialValue)")
    end
    local self: Self<T, any> = setmetatable(
        {
            dependencySet = {},
            dependentSet = {},
            lastChange = os.clock(),
            scope = scope,
            validity = "valid",
            _EXTREMELY_DANGEROUS_usedAsValue = initialValue
        }, 
        METATABLE
    ) :: any
    table.insert(scope, self)
    return self
end

function class.destroy<T, S>(
    self: Self<T, S>
): ()
    if self.scope == nil then
        External.logError("destroyedTwice", nil, "Value")
    end
    self.scope = nil
end

After this change, the contents of :destroy() would move to replace the table.insert(scope, self):

local function Value<T>(
    scope: Types.Scope<unknown>,
    initialValue: T
): Self<T, any>
    if initialValue == nil and (typeof(scope) ~= "table" or (scope[1] == nil and next(scope) ~= nil)) then
        External.logError("scopeMissing", nil, "Value", "myScope:Value(initialValue)")
    end
    local self: Self<T, any> = setmetatable(
        {
            dependencySet = {},
            dependentSet = {},
            lastChange = os.clock(),
            scope = scope,
            validity = "valid",
            _EXTREMELY_DANGEROUS_usedAsValue = initialValue
        }, 
        METATABLE
    ) :: any
    table.insert(scope, function()
        if self.scope == nil then
            External.logError("destroyedTwice", nil, "Value")
        end
        self.scope = nil
    end)
    return self
end

[dphfox]

Are scopes well-defined enough that we no longer need to check for double destruction? Or alternatively, should we be checking double destruction at the scope level, since objects are now inseparably bound to scopes as a part of this change?

In what scenarios may a scope be double destroyed? Is this easy to do?

[dphfox]

ScopedObject.scope should probably remain typed as Scope<unknown>? - while it might be much harder to double-destroy a scope, it's still easy to try and use destroyed objects after their scope has cleaned up.

However, it will need to lose ScopedObject:destroy() from the type signature.

[dphfox]

Are scopes well-defined enough that we no longer need to check for double destruction? Or alternatively, should we be checking double destruction at the scope level, since objects are now inseparably bound to scopes as a part of this change?

In what scenarios may a scope be double destroyed? Is this easy to do?

This should be fine actually. doCleanup clears out scopes after destruction, so there shouldn't be any interference.

I think this should be a fine change.

[dphfox]

Actually, let's do this on main.