From 13f1465f8f8b055066a429067364e0b790f34c1d Mon Sep 17 00:00:00 2001 From: Alexander Larsen Date: Mon, 4 May 2026 09:42:13 +0200 Subject: [PATCH] Fix null asset binding instance validation - Fixed an issue where an asset binding specifying a null instance was considered invalid, causing an incompatible asset type error, instead of a missing dependency error. - Added a regression test for asset binding validation with null instance, ensuring null instance binding is not invalidated. --- CHANGELOG.md | 10 ++ .../Saneject/Editor/Core/BindingValidator.cs | 136 +++++++++++------- .../Plugins/Saneject/Editor/Core/Locator.cs | 2 +- .../Assets/Plugins/Saneject/package.json | 2 +- .../Validation/AssetBindingValidationTests.cs | 21 +++ 5 files changed, 119 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1baf3e95..c9c980ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## Version 1.1.1 + +### Fixes + +- Fixed an issue where an asset binding specifying a null instance was considered invalid, causing an incompatible asset type error, instead of a missing dependency error. + +### Tests + +- Added a regression test for asset binding validation with null instance, ensuring null instance binding is not invalidated. + ## Version 1.1.0 ### Changes diff --git a/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/BindingValidator.cs b/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/BindingValidator.cs index 91931927..a85e7326 100644 --- a/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/BindingValidator.cs +++ b/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/BindingValidator.cs @@ -40,22 +40,28 @@ private static void ValidateBinding( List errors = new(); if (!existingBindings.Add(bindingNode)) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: "Duplicate or ambiguous binding within same Scope detected" - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: "Duplicate or ambiguous binding within same Scope detected" + ) + ); switch (bindingNode) { case GlobalComponentBindingNode globalBinding: { if (existingGlobals.TryGetValue(globalBinding.ConcreteType, out GlobalComponentBindingNode existingGlobal)) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: $"Duplicate global binding '{globalBinding.ConcreteType.Name}' declared by '{globalBinding.ScopeNode.ScopeType.Name}'. Already owned by '{existingGlobal.ScopeNode.ScopeType.Name}'. Only one global per type is allowed." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: $"Duplicate global binding '{globalBinding.ConcreteType.Name}' declared by '{globalBinding.ScopeNode.ScopeType.Name}'. Already owned by '{existingGlobal.ScopeNode.ScopeType.Name}'. Only one global per type is allowed." + ) + ); else existingGlobals.Add(globalBinding.ConcreteType, globalBinding); @@ -67,33 +73,45 @@ private static void ValidateBinding( if (componentBinding.RuntimeProxyConfig != null) { if (componentBinding.InterfaceType == null) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: "RuntimeProxy bindings require an interface type. Use BindComponent().FromProxy()." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: "RuntimeProxy bindings require an interface type. Use BindComponent().FromProxy()." + ) + ); if (componentBinding.ConcreteType == null) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: "RuntimeProxy bindings require an interface type. Use BindComponent().FromProxy()." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: "RuntimeProxy bindings require an interface type. Use BindComponent().FromProxy()." + ) + ); if (componentBinding.IsCollectionBinding) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: "RuntimeProxy bindings must be single-value only. Collections cannot be resolved via a RuntimeProxy." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: "RuntimeProxy bindings must be single-value only. Collections cannot be resolved via a RuntimeProxy." + ) + ); } if (componentBinding.ConcreteType != null && !typeof(Component).IsAssignableFrom(componentBinding.ConcreteType)) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: $"Component binding type '{componentBinding.ConcreteType.Name}' is not a Unity Component. Component bindings must resolve UnityEngine.Component types." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: $"Component binding type '{componentBinding.ConcreteType.Name}' is not a Unity Component. Component bindings must resolve UnityEngine.Component types." + ) + ); break; } @@ -101,52 +119,70 @@ private static void ValidateBinding( case AssetBindingNode assetBinding: { if (assetBinding.ConcreteType != null && typeof(Component).IsAssignableFrom(assetBinding.ConcreteType)) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: $"Asset binding type '{assetBinding.ConcreteType.Name}' derives from Component. Assets must be ScriptableObjects, prefabs, or other UnityEngine.Object assets." - )); - else if (assetBinding.ResolveFromInstances != null && assetBinding.ResolveFromInstances.Any(x => !EditorUtility.IsPersistent(x))) - errors.Add(new InvalidBindingError + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: $"Asset binding type '{assetBinding.ConcreteType.Name}' derives from Component. Assets must be ScriptableObjects, prefabs, or other UnityEngine.Object assets." + ) + ); + else if (assetBinding.ResolveFromInstances != null && assetBinding.ResolveFromInstances.Any(x => x != null && !EditorUtility.IsPersistent(x))) + errors.Add ( - bindingNode: bindingNode, - reason: "Asset binding configured with non-asset objects." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: "Asset binding configured with non-asset objects." + ) + ); break; } } if (bindingNode.FromMethodException != null) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: "FromMethod(...) threw an exception.", - exception: bindingNode.FromMethodException - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: "FromMethod(...) threw an exception.", + exception: bindingNode.FromMethodException + ) + ); if (bindingNode.InterfaceType is { IsInterface: false }) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: $"Binding interface type '{bindingNode.InterfaceType.FullName}' is not an interface." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: $"Binding interface type '{bindingNode.InterfaceType.FullName}' is not an interface." + ) + ); if (bindingNode.InterfaceType != null && bindingNode.ConcreteType != null && bindingNode.InterfaceType.IsInterface && !bindingNode.InterfaceType.IsAssignableFrom(bindingNode.ConcreteType)) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: $"Concrete type '{bindingNode.ConcreteType.Name}' does not implement interface '{bindingNode.InterfaceType.Name}'." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: $"Concrete type '{bindingNode.ConcreteType.Name}' does not implement interface '{bindingNode.InterfaceType.Name}'." + ) + ); if (!bindingNode.LocatorStrategySpecified) - errors.Add(new InvalidBindingError + errors.Add ( - bindingNode: bindingNode, - reason: "Binding has no locator strategy (e.g. FromScopeSelf, FromAnywhere)." - )); + new InvalidBindingError + ( + bindingNode: bindingNode, + reason: "Binding has no locator strategy (e.g. FromScopeSelf, FromAnywhere)." + ) + ); context.RegisterErrors(errors); diff --git a/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/Locator.cs b/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/Locator.cs index 7cb93f6d..5fd79ec3 100644 --- a/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/Locator.cs +++ b/UnityProject/Saneject/Assets/Plugins/Saneject/Editor/Core/Locator.cs @@ -227,7 +227,7 @@ private static IEnumerable LocateAssetCandidates( .Select(assetPath => AssetDatabase.LoadAssetAtPath(assetPath, bindingNode.ConcreteType)) .Where(obj => obj != null), - AssetLoadType.Instance => bindingNode.ResolveFromInstances, + AssetLoadType.Instance => bindingNode.ResolveFromInstances.Where(x => x != null), _ => throw new ArgumentOutOfRangeException() }; diff --git a/UnityProject/Saneject/Assets/Plugins/Saneject/package.json b/UnityProject/Saneject/Assets/Plugins/Saneject/package.json index 40e07e04..eca48bcd 100644 --- a/UnityProject/Saneject/Assets/Plugins/Saneject/package.json +++ b/UnityProject/Saneject/Assets/Plugins/Saneject/package.json @@ -2,7 +2,7 @@ "name": "com.alexanderlarsen.saneject", "author": "Alexander Larsen", "displayName": "Saneject", - "version": "1.1.0", + "version": "1.1.1", "description": "Inject dependencies in the Unity Editor, not Play Mode, by writing them directly into serialized fields at edit-time using familiar DI APIs, so everything stays visible in the Inspector, including interfaces.\n\nNo runtime container. No startup cost. No hidden wiring. No weird lifecycles. Just simple, deterministic edit-time DI that works with Unity, not around it.", "documentationUrl": "https://github.com/alexanderlarsen/Saneject/blob/main/README.md", "changelogUrl": "https://github.com/alexanderlarsen/Saneject/blob/main/CHANGELOG.md", diff --git a/UnityProject/Saneject/Assets/Tests/Saneject/Editor/Binding/Validation/AssetBindingValidationTests.cs b/UnityProject/Saneject/Assets/Tests/Saneject/Editor/Binding/Validation/AssetBindingValidationTests.cs index f9836188..e9e2e444 100644 --- a/UnityProject/Saneject/Assets/Tests/Saneject/Editor/Binding/Validation/AssetBindingValidationTests.cs +++ b/UnityProject/Saneject/Assets/Tests/Saneject/Editor/Binding/Validation/AssetBindingValidationTests.cs @@ -121,5 +121,26 @@ public void BindAsset_TConcrete_DuplicateWithinSameScope_IsInvalid() // Inject InjectionRunner.Run(scene.Roots, ContextWalkFilter.SceneObjects); } + + [Test] + public void BindAsset_TConcrete_ToNullInstance_IsNotInvalid() + { + // Expect logs + LogAssert.Expect(LogType.Warning, new Regex("^Saneject: Unused binding")); + LogAssert.Expect(LogType.Warning, new Regex("^Saneject: Injection complete")); + + + // Set up scene + TestScene scene = TestScene.Create(roots: 1, width: 1, depth: 1); + TestScope scope = scene.Add("Root 1"); + + // Bind + scope.BindAsset().FromInstance(null); + + // Inject + InjectionRunner.Run(scene.Roots, ContextWalkFilter.SceneObjects); + + LogAssert.NoUnexpectedReceived(); + } } }