Skip to content

Commit 9b5e4ff

Browse files
refactor(util): use 'using' statements instead of try-finally
Convert try-finally patterns to 'using' declarations for cleaner resource management as suggested by CodeQL static analysis. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
1 parent e877460 commit 9b5e4ff

2 files changed

Lines changed: 88 additions & 137 deletions

File tree

UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -626,65 +626,47 @@ public void DelayFrame_NegativeValue_SkipsDelay()
626626
[Test]
627627
public void TaskCapacity_ThrowsWhenExceeded()
628628
{
629-
var action = JAction.Create();
630-
try
631-
{
632-
// Add 256 tasks (the max capacity)
633-
for (int i = 0; i < 256; i++)
634-
{
635-
action.Do(() => { });
636-
}
629+
using var action = JAction.Create();
637630

638-
// The 257th task should throw
639-
Assert.Throws<InvalidOperationException>(() => action.Do(() => { }));
640-
}
641-
finally
631+
// Add 256 tasks (the max capacity)
632+
for (int i = 0; i < 256; i++)
642633
{
643-
action.Dispose();
634+
action.Do(() => { });
644635
}
636+
637+
// The 257th task should throw
638+
Assert.Throws<InvalidOperationException>(() => action.Do(() => { }));
645639
}
646640

647641
[Test]
648642
public void Reset_ClearsTasks()
649643
{
650644
int counter = 0;
651645

652-
var action = JAction.Create()
646+
using var action = JAction.Create()
653647
.Do(() => counter++)
654648
.Do(() => counter++);
655-
try
656-
{
657-
// Reset should clear the tasks
658-
action.Reset();
659649

660-
// Execute after reset - should do nothing (no tasks)
661-
action.Execute();
650+
// Reset should clear the tasks
651+
action.Reset();
662652

663-
Assert.AreEqual(0, counter);
664-
}
665-
finally
666-
{
667-
action.Dispose();
668-
}
653+
// Execute after reset - should do nothing (no tasks)
654+
action.Execute();
655+
656+
Assert.AreEqual(0, counter);
669657
}
670658

671659
[Test]
672660
public void Reset_ClearsParallelMode()
673661
{
674-
var action = JAction.Create()
662+
using var action = JAction.Create()
675663
.Parallel();
676-
try
677-
{
678-
Assert.IsTrue(action.IsParallel);
679664

680-
action.Reset();
665+
Assert.IsTrue(action.IsParallel);
681666

682-
Assert.IsFalse(action.IsParallel);
683-
}
684-
finally
685-
{
686-
action.Dispose();
687-
}
667+
action.Reset();
668+
669+
Assert.IsFalse(action.IsParallel);
688670
}
689671

690672
#endregion

UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs

Lines changed: 69 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -434,27 +434,20 @@ public IEnumerator Do_AsyncFunc_WaitsForCompletion()
434434
return RunAsync(async () =>
435435
{
436436
// Create an inner action that we'll wait for
437-
var innerAction = JAction.Create()
437+
using var innerAction = JAction.Create()
438438
.Do(static o => o.Add(1), order)
439439
.DelayFrame(2)
440440
.Do(static o => o.Add(2), order);
441-
try
442-
{
443-
using var action = await JAction.Create()
444-
.Do(static o => o.Add(0), order)
445-
.Do(static act => { _ = act.ExecuteAsync(); }, innerAction) // Fire and forget
446-
.Do(static o => o.Add(3), order)
447-
.ExecuteAsync();
448-
449-
// The inner action executes asynchronously, so order depends on timing
450-
Assert.Contains(0, order);
451-
Assert.Contains(3, order);
452-
}
453-
finally
454-
{
455-
// Clean up inner action
456-
innerAction.Dispose();
457-
}
441+
442+
using var action = await JAction.Create()
443+
.Do(static o => o.Add(0), order)
444+
.Do(static act => { _ = act.ExecuteAsync(); }, innerAction) // Fire and forget
445+
.Do(static o => o.Add(3), order)
446+
.ExecuteAsync();
447+
448+
// The inner action executes asynchronously, so order depends on timing
449+
Assert.Contains(0, order);
450+
Assert.Contains(3, order);
458451
});
459452
}
460453

@@ -497,7 +490,7 @@ public IEnumerator Parallel_AllowsConcurrentExecution()
497490
return RunAsync(async () =>
498491
{
499492
// Create a parallel action
500-
var action = JAction.Create()
493+
using var action = JAction.Create()
501494
.Parallel()
502495
.Do(() =>
503496
{
@@ -506,23 +499,17 @@ public IEnumerator Parallel_AllowsConcurrentExecution()
506499
})
507500
.DelayFrame(2)
508501
.Do(() => concurrentCount--);
509-
try
510-
{
511-
// Start multiple executions concurrently
512-
var task1 = action.ExecuteAsync();
513-
var task2 = action.ExecuteAsync();
514502

515-
await task1;
516-
await task2;
503+
// Start multiple executions concurrently
504+
var task1 = action.ExecuteAsync();
505+
var task2 = action.ExecuteAsync();
517506

518-
// With parallel mode, both should have run
519-
// Note: maxConcurrent may be 1 or 2 depending on timing
520-
Assert.GreaterOrEqual(maxConcurrent, 1);
521-
}
522-
finally
523-
{
524-
action.Dispose();
525-
}
507+
await task1;
508+
await task2;
509+
510+
// With parallel mode, both should have run
511+
// Note: maxConcurrent may be 1 or 2 depending on timing
512+
Assert.GreaterOrEqual(maxConcurrent, 1);
526513
});
527514
}
528515

@@ -534,36 +521,30 @@ public IEnumerator NonParallel_BlocksConcurrentExecution()
534521
return RunAsync(async () =>
535522
{
536523
// Create a non-parallel action (default)
537-
var action = JAction.Create()
524+
using var action = JAction.Create()
538525
.Do(() => executionCount++)
539526
.DelayFrame(2);
540-
try
541-
{
542-
// Start first execution
543-
var task1 = action.ExecuteAsync();
544527

545-
// Verify action is executing
546-
Assert.IsTrue(action.Executing);
528+
// Start first execution
529+
var task1 = action.ExecuteAsync();
547530

548-
// Try to start second execution while first is running
549-
// This should log a warning and return immediately
550-
LogAssert.Expect(LogType.Warning,
551-
"[JAction] Already executing. Enable Parallel() for concurrent execution.");
552-
var task2 = action.ExecuteAsync();
531+
// Verify action is executing
532+
Assert.IsTrue(action.Executing);
553533

554-
// task2 should complete immediately (returns early)
555-
await task2;
534+
// Try to start second execution while first is running
535+
// This should log a warning and return immediately
536+
LogAssert.Expect(LogType.Warning,
537+
"[JAction] Already executing. Enable Parallel() for concurrent execution.");
538+
var task2 = action.ExecuteAsync();
556539

557-
// Wait for first execution to complete
558-
await task1;
540+
// task2 should complete immediately (returns early)
541+
await task2;
559542

560-
// Only the first execution should have incremented
561-
Assert.AreEqual(1, executionCount);
562-
}
563-
finally
564-
{
565-
action.Dispose();
566-
}
543+
// Wait for first execution to complete
544+
await task1;
545+
546+
// Only the first execution should have incremented
547+
Assert.AreEqual(1, executionCount);
567548
});
568549
}
569550

@@ -711,32 +692,26 @@ public IEnumerator Cancel_DuringAsyncExecution_StopsExecution()
711692

712693
return RunAsync(async () =>
713694
{
714-
var action = JAction.Create()
695+
using var action = JAction.Create()
715696
.Do(() => step1 = true)
716697
.Delay(1f)
717698
.Do(() => step2 = true)
718699
.OnCancel(() => cancelled = true);
719-
try
720-
{
721-
// Start execution (don't await yet)
722-
var handle = action.ExecuteAsync();
723700

724-
// Wait a bit then cancel via handle (per-execution cancellation)
725-
await UniTask.Delay(50);
726-
handle.Cancel();
701+
// Start execution (don't await yet)
702+
var handle = action.ExecuteAsync();
727703

728-
// Now await the handle to get the result
729-
var result = await handle;
704+
// Wait a bit then cancel via handle (per-execution cancellation)
705+
await UniTask.Delay(50);
706+
handle.Cancel();
730707

731-
Assert.IsTrue(step1);
732-
Assert.IsFalse(step2);
733-
Assert.IsTrue(cancelled);
734-
Assert.IsTrue(result.Cancelled);
735-
}
736-
finally
737-
{
738-
action.Dispose();
739-
}
708+
// Now await the handle to get the result
709+
var result = await handle;
710+
711+
Assert.IsTrue(step1);
712+
Assert.IsFalse(step2);
713+
Assert.IsTrue(cancelled);
714+
Assert.IsTrue(result.Cancelled);
740715
});
741716
}
742717

@@ -747,33 +722,27 @@ public IEnumerator Cancel_ParallelExecution_CancelsOnlySpecificHandle()
747722

748723
return RunAsync(async () =>
749724
{
750-
var action = JAction.Create()
725+
using var action = JAction.Create()
751726
.Parallel()
752727
.Delay(1f)
753728
.OnCancel(() => cancelCount++);
754-
try
755-
{
756-
// Start two parallel executions
757-
var handle1 = action.ExecuteAsync();
758-
var handle2 = action.ExecuteAsync();
759-
760-
// Wait a bit then cancel only handle1
761-
await UniTask.Delay(50);
762-
handle1.Cancel();
763-
764-
// Await both
765-
var result1 = await handle1;
766-
var result2 = await handle2;
767-
768-
// Only handle1 should be cancelled
769-
Assert.IsTrue(result1.Cancelled);
770-
Assert.IsFalse(result2.Cancelled);
771-
Assert.AreEqual(1, cancelCount);
772-
}
773-
finally
774-
{
775-
action.Dispose();
776-
}
729+
730+
// Start two parallel executions
731+
var handle1 = action.ExecuteAsync();
732+
var handle2 = action.ExecuteAsync();
733+
734+
// Wait a bit then cancel only handle1
735+
await UniTask.Delay(50);
736+
handle1.Cancel();
737+
738+
// Await both
739+
var result1 = await handle1;
740+
var result2 = await handle2;
741+
742+
// Only handle1 should be cancelled
743+
Assert.IsTrue(result1.Cancelled);
744+
Assert.IsFalse(result2.Cancelled);
745+
Assert.AreEqual(1, cancelCount);
777746
});
778747
}
779748

0 commit comments

Comments
 (0)