Fix issues in AttributeStatement (#4526)

Fixes #4525
microsoft · Sep 26, 2024 · fa68969 · fa68969
1 parent 3fdc695
commit fa68969
Show file tree

Hide file tree

Showing 11 changed files with 215 additions and 12 deletions.
diff --git a/...p-client-csharp/generator/Microsoft.Generator.CSharp/src/Statements/AttributeStatement.cs b/...p-client-csharp/generator/Microsoft.Generator.CSharp/src/Statements/AttributeStatement.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 using System.Collections.Generic;
-using System.Linq;
 using Microsoft.Generator.CSharp.Expressions;
 using Microsoft.Generator.CSharp.Primitives;
 
@@ -12,30 +11,56 @@ public sealed class AttributeStatement : MethodBodyStatement
     {
         public CSharpType Type { get; }
         public IReadOnlyList<ValueExpression> Arguments { get; }
+        public IReadOnlyList<KeyValuePair<string, ValueExpression>> PositionalArguments { get; }
 
-        public AttributeStatement(CSharpType type, IReadOnlyList<ValueExpression> arguments)
+        public AttributeStatement(CSharpType type, IReadOnlyList<ValueExpression> arguments, IReadOnlyList<KeyValuePair<string, ValueExpression>> positionalArguments)
         {
             Type = type;
             Arguments = arguments;
+            PositionalArguments = positionalArguments;
         }
 
-        public AttributeStatement(CSharpType type, params ValueExpression[] arguments) : this(type, (IReadOnlyList<ValueExpression>)arguments) { }
+        public AttributeStatement(CSharpType type, IReadOnlyList<ValueExpression> arguments) : this(type, arguments, []) { }
+
+        public AttributeStatement(CSharpType type, IReadOnlyList<KeyValuePair<string, ValueExpression>> positionalArguments) : this(type, [], positionalArguments) { }
+
+        public AttributeStatement(CSharpType type, params ValueExpression[] arguments) : this(type, arguments, []) { }
 
         internal override void Write(CodeWriter writer)
         {
-            if (Arguments.Any())
+            writer.Append($"[{Type}");
+            var hasArguments = Arguments.Count > 0 || PositionalArguments.Count > 0;
+            if (hasArguments)
+            {
+                writer.AppendRaw("(");
+            }
+            for (int i = 0; i < Arguments.Count; i++)
+            {
+                Arguments[i].Write(writer);
+                if (i != Arguments.Count - 1)
+                {
+                    writer.AppendRaw(", ");
+                }
+            }
+            if (Arguments.Count > 0 && PositionalArguments.Count > 0)
+            {
+                writer.AppendRaw(", ");
+            }
+            for (int i = 0; i < PositionalArguments.Count; i++)
             {
-                writer.Append($"[{Type}(");
-                foreach (var argument in Arguments)
+                var (key, value) = PositionalArguments[i];
+                writer.Append($"{key} = ");
+                value.Write(writer);
+                if (i != PositionalArguments.Count - 1)
                 {
-                    argument.Write(writer);
+                    writer.AppendRaw(", ");
                 }
-                writer.WriteRawLine(")]");
             }
-            else
+            if (hasArguments)
             {
-                writer.WriteLine($"[{Type}]");
+                writer.AppendRaw(")");
             }
+            writer.WriteRawLine("]");
         }
     }
 }
diff --git a/...ient-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ParameterProviderTests.cs b/...ient-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ParameterProviderTests.cs
@@ -78,8 +78,8 @@ private static IEnumerable<TestCaseData> NotEqualsTestCases()
                new ParameterProvider("name1", $"Description", new CSharpType(typeof(string))),
                false);
             yield return new TestCaseData(
-               new ParameterProvider("name", $"Description", new CSharpType(typeof(string)), attributes: [new(new CSharpType(typeof(int)), [])]),
-               new ParameterProvider("name1", $"Description", new CSharpType(typeof(string)), attributes: [new(new CSharpType(typeof(string)), [])]),
+               new ParameterProvider("name", $"Description", new CSharpType(typeof(string)), attributes: [new(new CSharpType(typeof(int)))]),
+               new ParameterProvider("name1", $"Description", new CSharpType(typeof(string)), attributes: [new(new CSharpType(typeof(string)))]),
                false);
         }
 

diff --git a/...csharp/generator/Microsoft.Generator.CSharp/test/Providers/ProviderWithAttributesTests.cs b/...csharp/generator/Microsoft.Generator.CSharp/test/Providers/ProviderWithAttributesTests.cs
@@ -0,0 +1,55 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using Microsoft.Generator.CSharp.Expressions;
+using Microsoft.Generator.CSharp.Providers;
+using Microsoft.Generator.CSharp.Statements;
+using Microsoft.Generator.CSharp.Tests.Common;
+using NUnit.Framework;
+using static Microsoft.Generator.CSharp.Snippets.Snippet;
+
+namespace Microsoft.Generator.CSharp.Tests.Providers
+{
+    public class ProviderWithAttributesTests
+    {
+        public ProviderWithAttributesTests()
+        {
+            MockHelpers.LoadMockPlugin();
+        }
+
+        private class ProviderWithAttribute : TypeProvider
+        {
+            public ProviderWithAttribute()
+            {
+            }
+
+            protected override string BuildName() => "ProviderWithAttributes";
+
+            protected override string BuildRelativeFilePath() => ".";
+
+            protected override IReadOnlyList<AttributeStatement> BuildAttributes()
+            {
+                return [
+                    new(typeof(ObsoleteAttribute)),
+                    new(typeof(ObsoleteAttribute), Literal("This is obsolete")),
+                    new(typeof(ObsoleteAttribute), Literal("This is obsolete"), Literal(true)),
+                    new(typeof(ObsoleteAttribute), [new KeyValuePair<string, ValueExpression>("DiagnosticId", Literal("TypeSpecGenerator001"))]),
+                    new(typeof(ObsoleteAttribute), [new KeyValuePair<string, ValueExpression>("DiagnosticId", Literal("TypeSpecGenerator001")), new KeyValuePair<string, ValueExpression>("UrlFormat", Literal("my-format"))]),
+                    new(typeof(ObsoleteAttribute), [Literal("This is obsolete"), Literal(true)],
+                        [new KeyValuePair<string, ValueExpression>("DiagnosticId", Literal("TypeSpecGenerator001")), new KeyValuePair<string, ValueExpression>("UrlFormat", Literal("my-format"))]),
+                    ];
+            }
+        }
+
+        [Test]
+        public void ValidateAttributes()
+        {
+            var provider = new ProviderWithAttribute();
+            var writer = CodeModelPlugin.Instance.GetWriter(provider);
+            var content = writer.Write();
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), content.Content);
+        }
+    }
+}
diff --git a/...enerator.CSharp/test/Providers/TestData/ProviderWithAttributesTests/ValidateAttributes.cs b/...enerator.CSharp/test/Providers/TestData/ProviderWithAttributesTests/ValidateAttributes.cs
@@ -0,0 +1,19 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System;
+
+namespace Sample
+{
+    /// <summary></summary>
+    [global::System.ObsoleteAttribute]
+    [global::System.ObsoleteAttribute("This is obsolete")]
+    [global::System.ObsoleteAttribute("This is obsolete", true)]
+    [global::System.ObsoleteAttribute(DiagnosticId = "TypeSpecGenerator001")]
+    [global::System.ObsoleteAttribute(DiagnosticId = "TypeSpecGenerator001", UrlFormat = "my-format")]
+    [global::System.ObsoleteAttribute("This is obsolete", true, DiagnosticId = "TypeSpecGenerator001", UrlFormat = "my-format")]
+    public partial class ProviderWithAttributes
+    {
+    }
+}
diff --git a/...nt-csharp/generator/Microsoft.Generator.CSharp/test/Statements/AttributeStatementTests.cs b/...nt-csharp/generator/Microsoft.Generator.CSharp/test/Statements/AttributeStatementTests.cs
@@ -0,0 +1,98 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using Microsoft.Generator.CSharp.Expressions;
+using Microsoft.Generator.CSharp.Statements;
+using Microsoft.Generator.CSharp.Tests.Common;
+using NUnit.Framework;
+using static Microsoft.Generator.CSharp.Snippets.Snippet;
+
+namespace Microsoft.Generator.CSharp.Tests.Statements
+{
+    public class AttributeStatementTests
+    {
+        public AttributeStatementTests()
+        {
+            MockHelpers.LoadMockPlugin();
+        }
+
+        [Test]
+        public void AttributeStatementWithNoArgument()
+        {
+            var attributeStatement = new AttributeStatement(typeof(ObsoleteAttribute));
+
+            using var writer = new CodeWriter();
+            attributeStatement.Write(writer);
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false));
+        }
+
+        [Test]
+        public void AttributeStatementWithOneArgument()
+        {
+            var attributeStatement = new AttributeStatement(typeof(ObsoleteAttribute), Literal("This is obsolete"));
+
+            using var writer = new CodeWriter();
+            attributeStatement.Write(writer);
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false));
+        }
+
+        [Test]
+        public void AttributeStatementWithMultipleArguments()
+        {
+            var attributeStatement = new AttributeStatement(typeof(ObsoleteAttribute), Literal("This is obsolete"), Literal(true));
+
+            using var writer = new CodeWriter();
+            attributeStatement.Write(writer);
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false));
+        }
+
+        [Test]
+        public void AttributeStatementWithOneNamedArgument()
+        {
+            var attributeStatement = new AttributeStatement(typeof(ObsoleteAttribute),
+                [
+                    new KeyValuePair<string, ValueExpression>("DiagnosticId", Literal("TypeSpecGenerator001"))
+                ]);
+
+            using var writer = new CodeWriter();
+            attributeStatement.Write(writer);
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false));
+        }
+
+        [Test]
+        public void AttributeStatementWithNamedArguments()
+        {
+            var attributeStatement = new AttributeStatement(typeof(ObsoleteAttribute),
+                [
+                    new KeyValuePair<string, ValueExpression>("DiagnosticId", Literal("TypeSpecGenerator001")),
+                    new KeyValuePair<string, ValueExpression>("UrlFormat", Literal("my-format"))
+                ]);
+
+            using var writer = new CodeWriter();
+            attributeStatement.Write(writer);
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false));
+        }
+
+        [Test]
+        public void AttributeStatementWithArgumentsAndNamedArguments()
+        {
+            var attributeStatement = new AttributeStatement(typeof(ObsoleteAttribute), [Literal("This is obsolete"), Literal(true)],
+                [
+                    new KeyValuePair<string, ValueExpression>("DiagnosticId", Literal("TypeSpecGenerator001")),
+                    new KeyValuePair<string, ValueExpression>("UrlFormat", Literal("my-format"))
+                ]);
+
+            using var writer = new CodeWriter();
+            attributeStatement.Write(writer);
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false));
+        }
+    }
+}
diff --git a/...ents/TestData/AttributeStatementTests/AttributeStatementWithArgumentsAndNamedArguments.cs b/...ents/TestData/AttributeStatementTests/AttributeStatementWithArgumentsAndNamedArguments.cs
@@ -0,0 +1 @@
+[global::System.ObsoleteAttribute("This is obsolete", true, DiagnosticId = "TypeSpecGenerator001", UrlFormat = "my-format")]
diff --git a/...st/Statements/TestData/AttributeStatementTests/AttributeStatementWithMultipleArguments.cs b/...st/Statements/TestData/AttributeStatementTests/AttributeStatementWithMultipleArguments.cs
@@ -0,0 +1 @@
+[global::System.ObsoleteAttribute("This is obsolete", true)]
diff --git a/.../test/Statements/TestData/AttributeStatementTests/AttributeStatementWithNamedArguments.cs b/.../test/Statements/TestData/AttributeStatementTests/AttributeStatementWithNamedArguments.cs
@@ -0,0 +1 @@
+[global::System.ObsoleteAttribute(DiagnosticId = "TypeSpecGenerator001", UrlFormat = "my-format")]
diff --git a/...harp/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithNoArgument.cs b/...harp/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithNoArgument.cs
@@ -0,0 +1 @@
+[global::System.ObsoleteAttribute]
diff --git a/...arp/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithOneArgument.cs b/...arp/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithOneArgument.cs
@@ -0,0 +1 @@
+[global::System.ObsoleteAttribute("This is obsolete")]
diff --git a/...est/Statements/TestData/AttributeStatementTests/AttributeStatementWithOneNamedArgument.cs b/...est/Statements/TestData/AttributeStatementTests/AttributeStatementWithOneNamedArgument.cs
@@ -0,0 +1 @@
+[global::System.ObsoleteAttribute(DiagnosticId = "TypeSpecGenerator001")]