Skip to content

Commit ce5bb3c

Browse files
jongallowayjongalloway
authored
Fix XML generator partial method issues (#1059)
1 parent db0f4d1 commit ce5bb3c

File tree

2 files changed

+268
-2
lines changed

2 files changed

+268
-2
lines changed

‎src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,20 +321,25 @@ private static void AppendMethodDeclaration(
321321
writer.WriteLine($"[return: Description(\"{EscapeString(xmlDocs.Returns)}\")]");
322322
}
323323

324-
// Copy modifiers from original method syntax.
324+
// Copy modifiers from original method syntax, excluding 'async' which is invalid on partial declarations (CS1994).
325325
// Add return type (without nullable annotations).
326326
// Add method name.
327-
writer.Write(string.Join(" ", methodDeclaration.Modifiers.Select(m => m.Text)));
327+
var modifiers = methodDeclaration.Modifiers
328+
.Where(m => !m.IsKind(SyntaxKind.AsyncKeyword))
329+
.Select(m => m.Text);
330+
writer.Write(string.Join(" ", modifiers));
328331
writer.Write(' ');
329332
writer.Write(methodSymbol.ReturnType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat));
330333
writer.Write(' ');
331334
writer.Write(methodSymbol.Name);
332335

333336
// Add parameters with their Description attributes.
334337
writer.Write("(");
338+
var parameterSyntaxList = methodDeclaration.ParameterList.Parameters;
335339
for (int i = 0; i < methodSymbol.Parameters.Length; i++)
336340
{
337341
IParameterSymbol param = methodSymbol.Parameters[i];
342+
ParameterSyntax? paramSyntax = i < parameterSyntaxList.Count ? parameterSyntaxList[i] : null;
338343

339344
if (i > 0)
340345
{
@@ -352,6 +357,13 @@ private static void AppendMethodDeclaration(
352357
writer.Write(param.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat));
353358
writer.Write(' ');
354359
writer.Write(param.Name);
360+
361+
// Preserve default parameter values from the original syntax.
362+
if (paramSyntax?.Default is { } defaultValue)
363+
{
364+
writer.Write(' ');
365+
writer.Write(defaultValue.ToFullString().Trim());
366+
}
355367
}
356368
writer.WriteLine(");");
357369
}

‎tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs‎

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,6 +1463,260 @@ partial class GlobalTools
14631463
AssertGeneratedSourceEquals(expected, result.GeneratedSources[0].SourceText.ToString());
14641464
}
14651465

1466+
[Fact]
1467+
public void Generator_WithDefaultParameterValues_PreservesDefaults()
1468+
{
1469+
var result = RunGenerator("""
1470+
using ModelContextProtocol.Server;
1471+
using System.ComponentModel;
1472+
1473+
namespace Test;
1474+
1475+
[McpServerToolType]
1476+
public partial class TestTools
1477+
{
1478+
/// <summary>
1479+
/// Test tool with defaults
1480+
/// </summary>
1481+
/// <param name="project">The project name</param>
1482+
/// <param name="flag">Enable flag</param>
1483+
/// <param name="count">Item count</param>
1484+
[McpServerTool]
1485+
public static partial string TestMethod(
1486+
string? project = null,
1487+
bool flag = false,
1488+
int count = 42)
1489+
{
1490+
return project ?? "default";
1491+
}
1492+
}
1493+
""");
1494+
1495+
Assert.True(result.Success);
1496+
Assert.Single(result.GeneratedSources);
1497+
1498+
var expected = $$"""
1499+
// <auto-generated/>
1500+
// ModelContextProtocol.Analyzers {{typeof(XmlToDescriptionGenerator).Assembly.GetName().Version}}
1501+
1502+
#pragma warning disable
1503+
1504+
using System.ComponentModel;
1505+
using ModelContextProtocol.Server;
1506+
1507+
namespace Test
1508+
{
1509+
partial class TestTools
1510+
{
1511+
[Description("Test tool with defaults")]
1512+
public static partial string TestMethod([Description("The project name")] string? project = null, [Description("Enable flag")] bool flag = false, [Description("Item count")] int count = 42);
1513+
}
1514+
}
1515+
""";
1516+
1517+
AssertGeneratedSourceEquals(expected, result.GeneratedSources[0].SourceText.ToString());
1518+
}
1519+
1520+
[Fact]
1521+
public void Generator_WithAsyncMethod_ExcludesAsyncModifier()
1522+
{
1523+
var result = RunGenerator("""
1524+
using ModelContextProtocol.Server;
1525+
using System.ComponentModel;
1526+
using System.Threading.Tasks;
1527+
1528+
namespace Test;
1529+
1530+
[McpServerToolType]
1531+
public partial class TestTools
1532+
{
1533+
/// <summary>
1534+
/// Async tool
1535+
/// </summary>
1536+
[McpServerTool]
1537+
public async partial Task<string> DoWorkAsync(string input)
1538+
{
1539+
await Task.Delay(100);
1540+
return input;
1541+
}
1542+
}
1543+
""");
1544+
1545+
Assert.True(result.Success);
1546+
Assert.Single(result.GeneratedSources);
1547+
1548+
var expected = $$"""
1549+
// <auto-generated/>
1550+
// ModelContextProtocol.Analyzers {{typeof(XmlToDescriptionGenerator).Assembly.GetName().Version}}
1551+
1552+
#pragma warning disable
1553+
1554+
using System.ComponentModel;
1555+
using ModelContextProtocol.Server;
1556+
1557+
namespace Test
1558+
{
1559+
partial class TestTools
1560+
{
1561+
[Description("Async tool")]
1562+
public partial Task<string> DoWorkAsync(string input);
1563+
}
1564+
}
1565+
""";
1566+
1567+
AssertGeneratedSourceEquals(expected, result.GeneratedSources[0].SourceText.ToString());
1568+
}
1569+
1570+
[Fact]
1571+
public void Generator_WithAsyncStaticMethod_ExcludesAsyncModifier()
1572+
{
1573+
var result = RunGenerator("""
1574+
using ModelContextProtocol.Server;
1575+
using System.ComponentModel;
1576+
using System.Threading.Tasks;
1577+
1578+
namespace Test;
1579+
1580+
[McpServerToolType]
1581+
public partial class TestTools
1582+
{
1583+
/// <summary>
1584+
/// Static async tool
1585+
/// </summary>
1586+
[McpServerTool]
1587+
public static async partial Task<string> StaticAsyncMethod(string input)
1588+
{
1589+
await Task.Delay(100);
1590+
return input;
1591+
}
1592+
}
1593+
""");
1594+
1595+
Assert.True(result.Success);
1596+
Assert.Single(result.GeneratedSources);
1597+
1598+
var expected = $$"""
1599+
// <auto-generated/>
1600+
// ModelContextProtocol.Analyzers {{typeof(XmlToDescriptionGenerator).Assembly.GetName().Version}}
1601+
1602+
#pragma warning disable
1603+
1604+
using System.ComponentModel;
1605+
using ModelContextProtocol.Server;
1606+
1607+
namespace Test
1608+
{
1609+
partial class TestTools
1610+
{
1611+
[Description("Static async tool")]
1612+
public static partial Task<string> StaticAsyncMethod(string input);
1613+
}
1614+
}
1615+
""";
1616+
1617+
AssertGeneratedSourceEquals(expected, result.GeneratedSources[0].SourceText.ToString());
1618+
}
1619+
1620+
[Fact]
1621+
public void Generator_WithDefaultParameterValuesAndAsync_HandlesBothCorrectly()
1622+
{
1623+
var result = RunGenerator("""
1624+
using ModelContextProtocol.Server;
1625+
using System.ComponentModel;
1626+
using System.Threading.Tasks;
1627+
1628+
namespace Test;
1629+
1630+
[McpServerToolType]
1631+
public partial class TestTools
1632+
{
1633+
/// <summary>
1634+
/// Async tool with defaults
1635+
/// </summary>
1636+
/// <param name="input">The input</param>
1637+
/// <param name="timeout">Timeout in ms</param>
1638+
[McpServerTool]
1639+
public static async partial Task<string> AsyncWithDefaults(string input, int timeout = 1000)
1640+
{
1641+
await Task.Delay(timeout);
1642+
return input;
1643+
}
1644+
}
1645+
""");
1646+
1647+
Assert.True(result.Success);
1648+
Assert.Single(result.GeneratedSources);
1649+
1650+
var expected = $$"""
1651+
// <auto-generated/>
1652+
// ModelContextProtocol.Analyzers {{typeof(XmlToDescriptionGenerator).Assembly.GetName().Version}}
1653+
1654+
#pragma warning disable
1655+
1656+
using System.ComponentModel;
1657+
using ModelContextProtocol.Server;
1658+
1659+
namespace Test
1660+
{
1661+
partial class TestTools
1662+
{
1663+
[Description("Async tool with defaults")]
1664+
public static partial Task<string> AsyncWithDefaults([Description("The input")] string input, [Description("Timeout in ms")] int timeout = 1000);
1665+
}
1666+
}
1667+
""";
1668+
1669+
AssertGeneratedSourceEquals(expected, result.GeneratedSources[0].SourceText.ToString());
1670+
}
1671+
1672+
[Fact]
1673+
public void Generator_WithStringDefaultValue_PreservesQuotedDefault()
1674+
{
1675+
var result = RunGenerator("""
1676+
using ModelContextProtocol.Server;
1677+
using System.ComponentModel;
1678+
1679+
namespace Test;
1680+
1681+
[McpServerToolType]
1682+
public partial class TestTools
1683+
{
1684+
/// <summary>
1685+
/// Test tool with string default
1686+
/// </summary>
1687+
[McpServerTool]
1688+
public static partial string TestMethod(string name = "default value")
1689+
{
1690+
return name;
1691+
}
1692+
}
1693+
""");
1694+
1695+
Assert.True(result.Success);
1696+
Assert.Single(result.GeneratedSources);
1697+
1698+
var expected = $$"""
1699+
// <auto-generated/>
1700+
// ModelContextProtocol.Analyzers {{typeof(XmlToDescriptionGenerator).Assembly.GetName().Version}}
1701+
1702+
#pragma warning disable
1703+
1704+
using System.ComponentModel;
1705+
using ModelContextProtocol.Server;
1706+
1707+
namespace Test
1708+
{
1709+
partial class TestTools
1710+
{
1711+
[Description("Test tool with string default")]
1712+
public static partial string TestMethod(string name = "default value");
1713+
}
1714+
}
1715+
""";
1716+
1717+
AssertGeneratedSourceEquals(expected, result.GeneratedSources[0].SourceText.ToString());
1718+
}
1719+
14661720
private GeneratorRunResult RunGenerator([StringSyntax("C#-test")] string source)
14671721
{
14681722
var syntaxTree = CSharpSyntaxTree.ParseText(source);

0 commit comments

Comments
 (0)