diff --git a/src/Generators/AnalyzerReleases.Shipped.md b/src/Generators/AnalyzerReleases.Shipped.md new file mode 100644 index 00000000..d027c512 --- /dev/null +++ b/src/Generators/AnalyzerReleases.Shipped.md @@ -0,0 +1,3 @@ +; Shipped analyzer releases +; https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + diff --git a/src/Generators/AnalyzerReleases.Unshipped.md b/src/Generators/AnalyzerReleases.Unshipped.md new file mode 100644 index 00000000..bee547b6 --- /dev/null +++ b/src/Generators/AnalyzerReleases.Unshipped.md @@ -0,0 +1,9 @@ +; Unshipped analyzer release +; https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +DURABLE3001 | DurableTask.Design | Error | **DurableTaskSourceGenerator**: Reports when a task name in [DurableTask] attribute is not a valid C# identifier. Task names must start with a letter or underscore and contain only letters, digits, and underscores. +DURABLE3002 | DurableTask.Design | Error | **DurableTaskSourceGenerator**: Reports when an event name in [DurableEvent] attribute is not a valid C# identifier. Event names must start with a letter or underscore and contain only letters, digits, and underscores. diff --git a/src/Generators/DurableTaskSourceGenerator.cs b/src/Generators/DurableTaskSourceGenerator.cs index 2cc9e245..0116e568 100644 --- a/src/Generators/DurableTaskSourceGenerator.cs +++ b/src/Generators/DurableTaskSourceGenerator.cs @@ -3,6 +3,7 @@ using System.Collections.Immutable; using System.Diagnostics; +using System.Linq; using System.Text; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -39,6 +40,32 @@ public class DurableTaskSourceGenerator : IIncrementalGenerator * } */ + /// + /// Diagnostic ID for invalid task names. + /// + const string InvalidTaskNameDiagnosticId = "DURABLE3001"; + + /// + /// Diagnostic ID for invalid event names. + /// + const string InvalidEventNameDiagnosticId = "DURABLE3002"; + + static readonly DiagnosticDescriptor InvalidTaskNameRule = new( + InvalidTaskNameDiagnosticId, + title: "Invalid task name", + messageFormat: "The task name '{0}' is not a valid C# identifier. Task names must start with a letter or underscore and contain only letters, digits, and underscores.", + category: "DurableTask.Design", + DiagnosticSeverity.Error, + isEnabledByDefault: true); + + static readonly DiagnosticDescriptor InvalidEventNameRule = new( + InvalidEventNameDiagnosticId, + title: "Invalid event name", + messageFormat: "The event name '{0}' is not a valid C# identifier. Event names must start with a letter or underscore and contain only letters, digits, and underscores.", + category: "DurableTask.Design", + DiagnosticSeverity.Error, + isEnabledByDefault: true); + /// public void Initialize(IncrementalGeneratorInitializationContext context) { @@ -176,13 +203,15 @@ public void Initialize(IncrementalGeneratorInitializationContext context) ITypeSymbol? outputType = kind == DurableTaskKind.Entity ? null : taskType.TypeArguments.Last(); string taskName = classType.Name; + Location? taskNameLocation = null; if (attribute.ArgumentList?.Arguments.Count > 0) { ExpressionSyntax expression = attribute.ArgumentList.Arguments[0].Expression; taskName = context.SemanticModel.GetConstantValue(expression).ToString(); + taskNameLocation = expression.GetLocation(); } - return new DurableTaskTypeInfo(className, taskName, inputType, outputType, kind); + return new DurableTaskTypeInfo(className, taskName, inputType, outputType, kind, taskNameLocation); } static DurableEventTypeInfo? GetDurableEventTypeInfo(GeneratorSyntaxContext context) @@ -214,6 +243,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } string eventName = eventType.Name; + Location? eventNameLocation = null; if (attribute.ArgumentList?.Arguments.Count > 0) { @@ -222,10 +252,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context) if (constantValue.HasValue && constantValue.Value is string value) { eventName = value; + eventNameLocation = expression.GetLocation(); } } - return new DurableEventTypeInfo(eventName, eventType); + return new DurableEventTypeInfo(eventName, eventType, eventNameLocation); } static DurableFunction? GetDurableFunction(GeneratorSyntaxContext context) @@ -240,6 +271,22 @@ public void Initialize(IncrementalGeneratorInitializationContext context) return null; } + /// + /// Checks if a name is a valid C# identifier. + /// + /// The name to validate. + /// True if the name is a valid C# identifier, false otherwise. + static bool IsValidCSharpIdentifier(string name) + { + if (string.IsNullOrEmpty(name)) + { + return false; + } + + // Use Roslyn's built-in identifier validation + return SyntaxFacts.IsValidIdentifier(name); + } + static void Execute( SourceProductionContext context, Compilation compilation, @@ -253,15 +300,38 @@ static void Execute( return; } + // Validate task names and report diagnostics for invalid identifiers + foreach (DurableTaskTypeInfo task in allTasks) + { + if (!IsValidCSharpIdentifier(task.TaskName)) + { + Location location = task.TaskNameLocation ?? Location.None; + Diagnostic diagnostic = Diagnostic.Create(InvalidTaskNameRule, location, task.TaskName); + context.ReportDiagnostic(diagnostic); + } + } + + // Validate event names and report diagnostics for invalid identifiers + foreach (DurableEventTypeInfo eventInfo in allEvents.Where(e => !IsValidCSharpIdentifier(e.EventName))) + { + Location location = eventInfo.EventNameLocation ?? Location.None; + Diagnostic diagnostic = Diagnostic.Create(InvalidEventNameRule, location, eventInfo.EventName); + context.ReportDiagnostic(diagnostic); + } + // Determine if we should generate Durable Functions specific code bool isDurableFunctions = DetermineIsDurableFunctions(compilation, allFunctions, projectType); // Separate tasks into orchestrators, activities, and entities + // Skip tasks with invalid names to avoid generating invalid code List orchestrators = new(); List activities = new(); List entities = new(); - foreach (DurableTaskTypeInfo task in allTasks) + IEnumerable validTasks = allTasks + .Where(task => IsValidCSharpIdentifier(task.TaskName)); + + foreach (DurableTaskTypeInfo task in validTasks) { if (task.IsActivity) { @@ -277,7 +347,12 @@ static void Execute( } } - int found = activities.Count + orchestrators.Count + entities.Count + allEvents.Length + allFunctions.Length; + // Filter out events with invalid names + List validEvents = allEvents + .Where(eventInfo => IsValidCSharpIdentifier(eventInfo.EventName)) + .ToList(); + + int found = activities.Count + orchestrators.Count + entities.Count + validEvents.Count + allFunctions.Length; if (found == 0) { return; @@ -356,7 +431,7 @@ public static class GeneratedDurableTaskExtensions } // Generate WaitFor{EventName}Async methods for each event type - foreach (DurableEventTypeInfo eventInfo in allEvents) + foreach (DurableEventTypeInfo eventInfo in validEvents) { AddEventWaitMethod(sourceBuilder, eventInfo); AddEventSendMethod(sourceBuilder, eventInfo); @@ -640,11 +715,13 @@ public DurableTaskTypeInfo( string taskName, ITypeSymbol? inputType, ITypeSymbol? outputType, - DurableTaskKind kind) + DurableTaskKind kind, + Location? taskNameLocation = null) { this.TypeName = taskType; this.TaskName = taskName; this.Kind = kind; + this.TaskNameLocation = taskNameLocation; // Entities only have a state type parameter, not input/output if (kind == DurableTaskKind.Entity) @@ -672,6 +749,7 @@ public DurableTaskTypeInfo( public string InputParameter { get; } public string OutputType { get; } public DurableTaskKind Kind { get; } + public Location? TaskNameLocation { get; } public bool IsActivity => this.Kind == DurableTaskKind.Activity; @@ -699,14 +777,16 @@ static string GetRenderedTypeExpression(ITypeSymbol? symbol) class DurableEventTypeInfo { - public DurableEventTypeInfo(string eventName, ITypeSymbol eventType) + public DurableEventTypeInfo(string eventName, ITypeSymbol eventType, Location? eventNameLocation = null) { this.TypeName = GetRenderedTypeExpression(eventType); this.EventName = eventName; + this.EventNameLocation = eventNameLocation; } public string TypeName { get; } public string EventName { get; } + public Location? EventNameLocation { get; } static string GetRenderedTypeExpression(ITypeSymbol? symbol) { diff --git a/test/Generators.Tests/InvalidIdentifierTests.cs b/test/Generators.Tests/InvalidIdentifierTests.cs new file mode 100644 index 00000000..f0f07722 --- /dev/null +++ b/test/Generators.Tests/InvalidIdentifierTests.cs @@ -0,0 +1,238 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.DurableTask.Generators.Tests.Utils; + +namespace Microsoft.DurableTask.Generators.Tests; + +public class InvalidIdentifierTests +{ + const string GeneratedClassName = "GeneratedDurableTaskExtensions"; + const string GeneratedFileName = $"{GeneratedClassName}.cs"; + + [Theory] + [InlineData("Foo.Bar")] + [InlineData("Foo-Bar")] + [InlineData("Foo Bar")] + [InlineData("123Invalid")] + [InlineData("My-Task")] + [InlineData("Task.Name")] + [InlineData("@InvalidName")] + [InlineData("Task#Name")] + public Task Activity_InvalidName_ReportsDiagnostic(string invalidName) + { + string code = $@" +using System.Threading.Tasks; +using Microsoft.DurableTask; + +[DurableTask(""{invalidName}"")] +class MyActivity : TaskActivity +{{ + public override Task RunAsync(TaskActivityContext context, int input) => Task.FromResult(string.Empty); +}}"; + + // The test framework automatically verifies that the expected diagnostic is reported + DiagnosticResult expected = new DiagnosticResult("DURABLE3001", DiagnosticSeverity.Error) + .WithSpan("/0/Test0.cs", 5, 14, 5, 14 + invalidName.Length + 2) + .WithArguments(invalidName); + + CSharpSourceGeneratorVerifier.Test test = new() + { + TestState = + { + Sources = { code }, + ExpectedDiagnostics = { expected }, + AdditionalReferences = + { + typeof(TaskActivityContext).Assembly, + }, + }, + }; + + return test.RunAsync(); + } + + [Theory] + [InlineData("Foo.Bar")] + [InlineData("Foo-Bar")] + [InlineData("Foo Bar")] + [InlineData("123Invalid")] + public Task Orchestrator_InvalidName_ReportsDiagnostic(string invalidName) + { + string code = $@" +using System.Threading.Tasks; +using Microsoft.DurableTask; + +[DurableTask(""{invalidName}"")] +class MyOrchestrator : TaskOrchestrator +{{ + public override Task RunAsync(TaskOrchestrationContext ctx, int input) => Task.FromResult(string.Empty); +}}"; + + DiagnosticResult expected = new DiagnosticResult("DURABLE3001", DiagnosticSeverity.Error) + .WithSpan("/0/Test0.cs", 5, 14, 5, 14 + invalidName.Length + 2) + .WithArguments(invalidName); + + CSharpSourceGeneratorVerifier.Test test = new() + { + TestState = + { + Sources = { code }, + ExpectedDiagnostics = { expected }, + AdditionalReferences = + { + typeof(TaskActivityContext).Assembly, + }, + }, + }; + + return test.RunAsync(); + } + + [Theory] + [InlineData("Foo.Bar")] + [InlineData("Foo-Bar")] + [InlineData("Event Name")] + public Task Event_InvalidName_ReportsDiagnostic(string invalidName) + { + string code = $@" +using System.Threading.Tasks; +using Microsoft.DurableTask; + +[DurableEvent(""{invalidName}"")] +public sealed record MyEvent(bool Approved);"; + + DiagnosticResult expected = new DiagnosticResult("DURABLE3002", DiagnosticSeverity.Error) + .WithSpan("/0/Test0.cs", 5, 15, 5, 15 + invalidName.Length + 2) + .WithArguments(invalidName); + + CSharpSourceGeneratorVerifier.Test test = new() + { + TestState = + { + Sources = { code }, + ExpectedDiagnostics = { expected }, + AdditionalReferences = + { + typeof(TaskActivityContext).Assembly, + }, + }, + }; + + return test.RunAsync(); + } + + [Fact] + public Task Activity_ValidName_NoDiagnostic() + { + string code = @" +using System.Threading.Tasks; +using Microsoft.DurableTask; + +[DurableTask(""MyActivity"")] +class MyActivity : TaskActivity +{ + public override Task RunAsync(TaskActivityContext context, int input) => Task.FromResult(string.Empty); +}"; + + string expectedOutput = TestHelpers.WrapAndFormat( + GeneratedClassName, + methodList: @" +/// +/// Calls the activity. +/// +/// +public static Task CallMyActivityAsync(this TaskOrchestrationContext ctx, int input, TaskOptions? options = null) +{ + return ctx.CallActivityAsync(""MyActivity"", input, options); +} + +internal static DurableTaskRegistry AddAllGeneratedTasks(this DurableTaskRegistry builder) +{ + builder.AddActivity(); + return builder; +}"); + + return TestHelpers.RunTestAsync( + GeneratedFileName, + code, + expectedOutput, + isDurableFunctions: false); + } + + [Fact] + public Task Activity_ValidNameWithUnderscore_NoDiagnostic() + { + string code = @" +using System.Threading.Tasks; +using Microsoft.DurableTask; + +[DurableTask(""My_Activity"")] +class MyActivity : TaskActivity +{ + public override Task RunAsync(TaskActivityContext context, int input) => Task.FromResult(string.Empty); +}"; + + string expectedOutput = TestHelpers.WrapAndFormat( + GeneratedClassName, + methodList: @" +/// +/// Calls the activity. +/// +/// +public static Task CallMy_ActivityAsync(this TaskOrchestrationContext ctx, int input, TaskOptions? options = null) +{ + return ctx.CallActivityAsync(""My_Activity"", input, options); +} + +internal static DurableTaskRegistry AddAllGeneratedTasks(this DurableTaskRegistry builder) +{ + builder.AddActivity(); + return builder; +}"); + + return TestHelpers.RunTestAsync( + GeneratedFileName, + code, + expectedOutput, + isDurableFunctions: false); + } + + [Fact] + public Task Activity_InvalidName_NoCodeGenerated() + { + // When a task has an invalid name, we should report a diagnostic + // but NOT generate any code for it (to avoid compilation errors) + string code = @" +using System.Threading.Tasks; +using Microsoft.DurableTask; + +[DurableTask(""Foo.Bar"")] +class MyActivity : TaskActivity +{ + public override Task RunAsync(TaskActivityContext context, int input) => Task.FromResult(string.Empty); +}"; + + DiagnosticResult expected = new DiagnosticResult("DURABLE3001", DiagnosticSeverity.Error) + .WithSpan("/0/Test0.cs", 5, 14, 5, 23) + .WithArguments("Foo.Bar"); + + CSharpSourceGeneratorVerifier.Test test = new() + { + TestState = + { + Sources = { code }, + ExpectedDiagnostics = { expected }, + AdditionalReferences = + { + typeof(TaskActivityContext).Assembly, + }, + // Don't expect any generated sources since the name is invalid + }, + }; + + return test.RunAsync(); + } +}