Skip to content

Commit 7f5cd16

Browse files
dsarnoclaude
andauthored
Fix create_script validator false-positive on constructor invocations (#1045)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 38292bd commit 7f5cd16

2 files changed

Lines changed: 64 additions & 4 deletions

File tree

MCPForUnity/Editor/Tools/ManageScript.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2744,16 +2744,18 @@ private static void CheckDuplicateMethodSignatures(string contents, System.Colle
27442744

27452745
// Step 3: Match method signatures on code-only text (includes => for expression-bodied)
27462746
var methodSigPattern = new Regex(
2747-
@"(?:(?: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*)?(?:[{;]|=>)",
2747+
@"(?:(?: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*)?(?:[{;]|=>)",
27482748
RegexOptions.Multiline | RegexOptions.CultureInvariant, TimeSpan.FromSeconds(2));
27492749
var sigMatches = methodSigPattern.Matches(codeOnly);
27502750
var seen = new System.Collections.Generic.Dictionary<string, int>(System.StringComparer.Ordinal);
27512751
foreach (Match sm in sigMatches)
27522752
{
2753-
string methodName = sm.Groups[1].Value;
2753+
string returnType = sm.Groups[1].Value;
2754+
string methodName = sm.Groups[2].Value;
2755+
if (string.Equals(returnType, "new", StringComparison.Ordinal)) continue; // constructor invocation, not a method declaration
27542756
if (IsCSharpKeyword(methodName)) continue;
2755-
int paramCount = CountTopLevelParams(sm.Groups[2].Value);
2756-
string paramTypes = ExtractParamTypes(sm.Groups[2].Value);
2757+
int paramCount = CountTopLevelParams(sm.Groups[3].Value);
2758+
string paramTypes = ExtractParamTypes(sm.Groups[3].Value);
27572759
string containingType = containingTypeArr[sm.Index];
27582760
string key = $"{containingType}/{methodName}/{paramCount}/{paramTypes}";
27592761
if (seen.TryGetValue(key, out _))

TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptValidationTests.cs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,64 @@ public void Update()
320320
"C# keywords (if, for, while, etc.) should not be matched as duplicate methods");
321321
}
322322

323+
[Test]
324+
public void DuplicateMethodCheck_ConstructorInvocations_NotFlagged()
325+
{
326+
string code = @"using UnityEngine;
327+
public class Test : MonoBehaviour
328+
{
329+
void Start()
330+
{
331+
GameObject a = new GameObject(""A"");
332+
GameObject b = new GameObject(""B"");
333+
}
334+
}";
335+
var errors = CallValidateScriptSyntaxUnity(code);
336+
Assert.IsFalse(HasDuplicateMethodError(errors),
337+
"Constructor invocations (new Type(...)) should not be flagged as duplicate methods");
338+
}
339+
340+
[Test]
341+
public void DuplicateMethodCheck_MultipleDistinctConstructors_NotFlagged()
342+
{
343+
string code = @"using UnityEngine;
344+
public class Test : MonoBehaviour
345+
{
346+
void Start()
347+
{
348+
var mpb1 = new MaterialPropertyBlock();
349+
var mpb2 = new MaterialPropertyBlock();
350+
var go1 = new GameObject(""A"");
351+
var go2 = new GameObject(""B"");
352+
}
353+
}";
354+
var errors = CallValidateScriptSyntaxUnity(code);
355+
Assert.IsFalse(HasDuplicateMethodError(errors),
356+
"Multiple constructor invocations of different types should not be flagged");
357+
}
358+
359+
[Test]
360+
public void DuplicateMethodCheck_NewModifierWithConstructors_CorrectBehavior()
361+
{
362+
string code = @"using UnityEngine;
363+
public class Base : MonoBehaviour
364+
{
365+
public virtual void Init() { }
366+
}
367+
public class Derived : Base
368+
{
369+
public new void Init() { }
370+
void Start()
371+
{
372+
var a = new GameObject(""A"");
373+
var b = new GameObject(""B"");
374+
}
375+
}";
376+
var errors = CallValidateScriptSyntaxUnity(code);
377+
Assert.IsFalse(HasDuplicateMethodError(errors),
378+
"new modifier on method should not interfere with constructor invocation filtering");
379+
}
380+
323381
[Test]
324382
public void HandleCommand_PathWithCsExtension_StripsFilename()
325383
{

0 commit comments

Comments
 (0)