From b36483468b52cc59b6d60f8fc8bc6c6f05bfc8af Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 6 Apr 2026 20:29:23 -0700 Subject: [PATCH 1/2] Fix create_script validator false-positive on constructor invocations The regex-based duplicate method signature check misidentified `new Type(...)` constructor calls as method declarations, causing valid C# like `new GameObject("A"); new GameObject("B");` to be rejected. Capture the return-type token and skip when it is `new`. Addresses #1044 Co-Authored-By: Claude Opus 4.6 (1M context) --- MCPForUnity/Editor/Tools/ManageScript.cs | 10 +++--- .../Tools/ManageScriptValidationTests.cs | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/MCPForUnity/Editor/Tools/ManageScript.cs b/MCPForUnity/Editor/Tools/ManageScript.cs index ed345526c..7928ec600 100644 --- a/MCPForUnity/Editor/Tools/ManageScript.cs +++ b/MCPForUnity/Editor/Tools/ManageScript.cs @@ -2744,16 +2744,18 @@ private static void CheckDuplicateMethodSignatures(string contents, System.Colle // Step 3: Match method signatures on code-only text (includes => for expression-bodied) var methodSigPattern = new Regex( - @"(?:(?:public|private|protected|internal)\s+)?(?:(?:static|virtual|override|abstract|sealed|async|new)\s+)*\S+\s+(\w+)\s*\(([^)]*)\)\s*(?:where\s+\S+\s*:\s*\S+\s*)?(?:[{;]|=>)", + @"(?:(?:public|private|protected|internal)\s+)?(?:(?:static|virtual|override|abstract|sealed|async|new)\s+)*(\S+)\s+(\w+)\s*\(([^)]*)\)\s*(?:where\s+\S+\s*:\s*\S+\s*)?(?:[{;]|=>)", RegexOptions.Multiline | RegexOptions.CultureInvariant, TimeSpan.FromSeconds(2)); var sigMatches = methodSigPattern.Matches(codeOnly); var seen = new System.Collections.Generic.Dictionary(System.StringComparer.Ordinal); foreach (Match sm in sigMatches) { - string methodName = sm.Groups[1].Value; + string returnType = sm.Groups[1].Value; + string methodName = sm.Groups[2].Value; + if (returnType == "new") continue; // constructor invocation, not a method declaration if (IsCSharpKeyword(methodName)) continue; - int paramCount = CountTopLevelParams(sm.Groups[2].Value); - string paramTypes = ExtractParamTypes(sm.Groups[2].Value); + int paramCount = CountTopLevelParams(sm.Groups[3].Value); + string paramTypes = ExtractParamTypes(sm.Groups[3].Value); string containingType = containingTypeArr[sm.Index]; string key = $"{containingType}/{methodName}/{paramCount}/{paramTypes}"; if (seen.TryGetValue(key, out _)) diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs index b96c8cb7b..210c892af 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs @@ -320,6 +320,42 @@ public void Update() "C# keywords (if, for, while, etc.) should not be matched as duplicate methods"); } + [Test] + public void DuplicateMethodCheck_ConstructorInvocations_NotFlagged() + { + string code = @"using UnityEngine; +public class Test : MonoBehaviour +{ + void Start() + { + GameObject a = new GameObject(""A""); + GameObject b = new GameObject(""B""); + } +}"; + var errors = CallValidateScriptSyntaxUnity(code); + Assert.IsFalse(HasDuplicateMethodError(errors), + "Constructor invocations (new Type(...)) should not be flagged as duplicate methods"); + } + + [Test] + public void DuplicateMethodCheck_MultipleDistinctConstructors_NotFlagged() + { + string code = @"using UnityEngine; +public class Test : MonoBehaviour +{ + void Start() + { + var mpb1 = new MaterialPropertyBlock(); + var mpb2 = new MaterialPropertyBlock(); + var go1 = new GameObject(""A""); + var go2 = new GameObject(""B""); + } +}"; + var errors = CallValidateScriptSyntaxUnity(code); + Assert.IsFalse(HasDuplicateMethodError(errors), + "Multiple constructor invocations of different types should not be flagged"); + } + [Test] public void HandleCommand_PathWithCsExtension_StripsFilename() { From ed5323a974f9708013b1edf765c0ac1817866913 Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 6 Apr 2026 20:33:29 -0700 Subject: [PATCH 2/2] Address review: Ordinal comparison, add new-modifier + constructor test Use string.Equals with StringComparison.Ordinal for the "new" check. Add test combining `public new void Init()` with nearby constructor invocations to guard against modifier vs return-type parsing regressions. Co-Authored-By: Claude Opus 4.6 (1M context) --- MCPForUnity/Editor/Tools/ManageScript.cs | 2 +- .../Tools/ManageScriptValidationTests.cs | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/MCPForUnity/Editor/Tools/ManageScript.cs b/MCPForUnity/Editor/Tools/ManageScript.cs index 7928ec600..9905d671f 100644 --- a/MCPForUnity/Editor/Tools/ManageScript.cs +++ b/MCPForUnity/Editor/Tools/ManageScript.cs @@ -2752,7 +2752,7 @@ private static void CheckDuplicateMethodSignatures(string contents, System.Colle { string returnType = sm.Groups[1].Value; string methodName = sm.Groups[2].Value; - if (returnType == "new") continue; // constructor invocation, not a method declaration + if (string.Equals(returnType, "new", StringComparison.Ordinal)) continue; // constructor invocation, not a method declaration if (IsCSharpKeyword(methodName)) continue; int paramCount = CountTopLevelParams(sm.Groups[3].Value); string paramTypes = ExtractParamTypes(sm.Groups[3].Value); diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs index 210c892af..1bf058f3d 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs @@ -356,6 +356,28 @@ void Start() "Multiple constructor invocations of different types should not be flagged"); } + [Test] + public void DuplicateMethodCheck_NewModifierWithConstructors_CorrectBehavior() + { + string code = @"using UnityEngine; +public class Base : MonoBehaviour +{ + public virtual void Init() { } +} +public class Derived : Base +{ + public new void Init() { } + void Start() + { + var a = new GameObject(""A""); + var b = new GameObject(""B""); + } +}"; + var errors = CallValidateScriptSyntaxUnity(code); + Assert.IsFalse(HasDuplicateMethodError(errors), + "new modifier on method should not interfere with constructor invocation filtering"); + } + [Test] public void HandleCommand_PathWithCsExtension_StripsFilename() {