parameters on yarn functions and commands are now validated.

McJones · McJones · commit 007bd39833f6 · 2026-01-23T15:19:37.000+11:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   - has one field which is the name the enum as declared in your Yarn
   - Intended to allow you to hint to the VSCode extension to offer enum based suggestions
   - this is temporary until forward declaration of Yarn enums into C# works
+- Incorrectly defined parameters for commands and functions will now generate diagnostics on the C# implementation, warnings exist for:
+  - invalid types
+  - incorrectly attributed parameters
 
 ### Changed
 
diff --git a/Editor/Analysis/Action.cs b/Editor/Analysis/Action.cs
@@ -303,17 +303,20 @@ public string ToJSON()
                     }
                     else if (isAnEnum)
                     {
-                        paramObject["type"] = "enum";
-
+                        var subtype = "any";
                         var attribute = p.Attributes?.Where(a => a.AttributeClass?.Name == "YarnEnumParameterAttribute").First();
                         if (attribute != null && attribute.ConstructorArguments.Count() > 0)
                         {
                             var enumType = attribute.ConstructorArguments[0];
                             if (enumType.Type?.SpecialType == SpecialType.System_String)
                             {
-                                paramObject["subtype"] = enumType.Value as string ?? p.YarnTypeString;
+                                subtype = enumType.Value as string ?? p.YarnTypeString;
                             }
                         }
+
+                        paramObject["type"] = "enum";
+                        paramObject["subtype"] = subtype;
+
                     }
                     else
                     {
@@ -339,8 +342,9 @@ public string ToJSON()
             return Yarn.Unity.Editor.Json.Serialize(result);
         }
 
-        public List<Microsoft.CodeAnalysis.Diagnostic> Validate(Compilation compilation)
+        public List<Microsoft.CodeAnalysis.Diagnostic> Validate(Compilation compilation, ILogger? logger)
         {
+            logger?.WriteLine($"Beginning validation");
             var diagnostics = new List<Microsoft.CodeAnalysis.Diagnostic>();
             if (this.MethodDeclarationSyntax == null)
             {
@@ -432,11 +436,11 @@ public string ToJSON()
                     break;
 
                 case ActionType.Command:
-                    diagnostics.AddRange(ValidateCommand(compilation));
+                    diagnostics.AddRange(ValidateCommand(compilation, logger));
                     break;
 
                 case ActionType.Function:
-                    diagnostics.AddRange(ValidateFunction(compilation));
+                    diagnostics.AddRange(ValidateFunction(compilation, logger));
                     break;
 
                 default:
@@ -447,9 +451,8 @@ public string ToJSON()
             return diagnostics;
         }
 
-        private IEnumerable<Diagnostic> ValidateFunction(Compilation compilation)
+        private IEnumerable<Diagnostic> ValidateFunction(Compilation compilation, ILogger? logger)
         {
-
             string identifier;
             Location returnTypeLocation;
             Location identifierLocation;
@@ -484,6 +487,12 @@ private IEnumerable<Diagnostic> ValidateFunction(Compilation compilation)
                 yield return Diagnostic.Create(Diagnostics.YS1006YarnFunctionsMustBeStatic, identifierLocation);
             }
 
+            var paramDiags = ValidateParameters(compilation, logger);
+            foreach (var p in paramDiags)
+            {
+                yield return p;
+            }
+
             // Functions must return a number, string, or bool
             var returnTypeSymbol = this.MethodSymbol.ReturnType;
 
@@ -510,7 +519,135 @@ private IEnumerable<Diagnostic> ValidateFunction(Compilation compilation)
             }
         }
 
-        private IEnumerable<Diagnostic> ValidateCommand(Compilation compilation)
+        // validates the parameters are correct
+        private List<Diagnostic> ValidateParameters(Compilation compilation, ILogger? logger)
+        {
+            List<Diagnostic> diagnostics = new List<Diagnostic>();
+            ParameterListSyntax? parameterList = null;
+            Location? actionLocation = null;
+            string? identifier = null;
+            
+            if (this.MethodDeclarationSyntax is MethodDeclarationSyntax methodDeclaration)
+            {
+                identifier = methodDeclaration.Identifier.ToString();
+                logger?.WriteLine($"Validating {identifier} as a method");
+                parameterList = methodDeclaration.ParameterList;
+                actionLocation = methodDeclaration.GetLocation();
+            }
+            else if (this.MethodDeclarationSyntax is LocalFunctionStatementSyntax localFunctionStatement)
+            {
+                identifier = localFunctionStatement.Identifier.ToString();
+                logger?.WriteLine($"Validating {identifier} as a local function");
+                parameterList = localFunctionStatement.ParameterList;
+                actionLocation = localFunctionStatement.GetLocation();
+            }
+            else if(this.MethodDeclarationSyntax is LambdaExpressionSyntax lambdaExpression)
+            {
+                logger?.WriteLine("The action is a lambda.");
+                actionLocation = lambdaExpression.GetLocation();
+
+                if (lambdaExpression is SimpleLambdaExpressionSyntax)
+                {
+                    logger?.WriteLine("The action is a simple lambda, validations do not apply here, skipping this action.");
+                    diagnostics.Add(Diagnostic.Create(Diagnostics.YS1012ActionIsALambda, actionLocation));
+                    return diagnostics;
+                }
+
+                if (lambdaExpression is ParenthesizedLambdaExpressionSyntax pls)
+                {
+                    logger?.WriteLine("The action is a parenthesized lambda, can perform some validation.");
+
+                    identifier = "(lambda expression)";
+                    parameterList = pls.ParameterList;
+
+                    diagnostics.Add(Diagnostic.Create(Diagnostics.YS1012ActionIsALambda, actionLocation));
+                }
+            }
+            
+            if (parameterList == null || parameterList.Parameters.Count() == 0)
+            {
+                logger?.WriteLine($"{identifier} has no parameters, ignoring");
+                return diagnostics;
+            }
+
+            logger?.WriteLine($"Will be checking {parameterList.Parameters.Count()} parameters");
+            foreach (var parameter in parameterList.Parameters)
+            {
+                if (parameter.Type == null)
+                {
+                    logger?.WriteLine($"{parameter.ToFullString()} has no type, ignoring validation?");
+                    continue;
+                }
+
+                var model = compilation.GetSemanticModel(parameter.SyntaxTree);
+                var typeInfo = model.GetTypeInfo(parameter.Type).Type;
+
+                var parameterName = model.GetDeclaredSymbol(parameter)?.Name ?? "(UNKNOWN)";
+                logger?.WriteLine($"Validating {parameterName}");
+
+                if (typeInfo == null)
+                {
+                    logger?.WriteLine($"Unable to determine typeinfo of {parameterName} ignoring validation?");
+                    continue;
+                }
+
+                var symbol = model.GetDeclaredSymbol(parameter);
+                if (symbol == null)
+                {
+                    logger?.WriteLine($"Unable to determine the declared symbol for {parameterName}, skipping validation");
+                    continue;
+                }
+
+                if (symbol.IsParams)
+                {
+                    if (symbol.Type is IArrayTypeSymbol arrayTypeSymbol)
+                    {
+                        var subtype = arrayTypeSymbol.ElementType;
+                        logger?.WriteLine($"{parameterName} is a params array made up of {subtype}:_{subtype.Name}_:-{subtype.GetYarnTypeString()}-");
+                        if (subtype.GetYarnTypeString() == "any")
+                        {
+                            logger?.WriteLine($"{parameterName} is a parameter array of non Yarn compatible types!");
+                            diagnostics.Add(Diagnostic.Create(Diagnostics.YS1008ActionsParamsArraysMustBeOfYarnTypes, parameter.GetLocation(), parameterName, subtype.Name));
+                        }
+                    }
+                }
+                else
+                {    
+                    if (typeInfo.GetYarnTypeString() == "any" && typeInfo.BaseType?.Name != "Component")
+                    {
+                        // we have an invalid type
+                        logger?.WriteLine($"{parameterName} is an invalid type for use in a Yarn action");
+                        diagnostics.Add(Diagnostic.Create(Diagnostics.YS1011ActionsParameterIsAnIncompatibleType, parameter.GetLocation(), parameterName, typeInfo.Name));
+                    }
+                }
+
+
+                foreach (var attribute in symbol.GetAttributes())
+                {
+                    // this attribute is an enum parameter
+                    if (attribute.AttributeClass?.Name == "YarnEnumParameterAttribute")
+                    {
+                        if (typeInfo.GetYarnTypeString() == "any")
+                        {
+                            logger?.WriteLine($"{parameterName} is attributed as an enum but isn't a Yarn compatible type!");
+                            diagnostics.Add(Diagnostic.Create(Diagnostics.YS1009ActionsEnumAttributedParameterIsOfIncompatibleType, parameter.GetLocation(), parameterName, typeInfo.Name));
+                        }
+                    }
+                    if (attribute.AttributeClass?.Name == "YarnNodeParameterAttribute")
+                    {
+                        if (typeInfo.GetYarnTypeString() != "string")
+                        {
+                            logger?.WriteLine($"{parameterName} is attributed as a node but isn't a string!");
+                            diagnostics.Add(Diagnostic.Create(Diagnostics.YS1010ActionsNodeAttributedParameterIsOfIncompatibleType, parameter.GetLocation(), parameterName, typeInfo.Name));
+                        }
+                    }
+                }
+            }
+
+            return diagnostics;
+        }
+
+        private IEnumerable<Diagnostic> ValidateCommand(Compilation compilation, ILogger? logger)
         {
             if (MethodSymbol == null)
             {
@@ -571,6 +708,11 @@ private IEnumerable<Diagnostic> ValidateCommand(Compilation compilation)
                 throw new InvalidOperationException($"Expected decl for {this.Name} ({this.SourceFileName}) was of unexpected type {this.MethodDeclarationSyntax?.GetType().Name ?? "null"}");
             }
 
+            var paramDiags = ValidateParameters(compilation, logger);
+            foreach (var p in paramDiags)
+            {
+                yield return p;
+            }
 
             var typeIsKnownValid = validCommandReturnTypes.Contains(returnTypeSymbol)
                 || validTaskTypes.Contains(returnTypeSymbol);
diff --git a/Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs b/Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs
@@ -220,7 +220,7 @@ public void Execute(GeneratorExecutionContext context)
                     continue;
                 }
 
-                var diagnostics = action.Validate(compilation);
+                var diagnostics = action.Validate(compilation, output);
                 foreach (var diagnostic in diagnostics)
                 {
                     context.ReportDiagnostic(diagnostic);
diff --git a/Editor/Analysis/Analyser.cs b/Editor/Analysis/Analyser.cs
@@ -58,7 +58,7 @@ public Analyser(string sourcePath)
         public IEnumerable<string> SourceFiles => GetSourceFiles(SourcePath);
         public string SourcePath { get; set; }
 
-        public IEnumerable<Action> GetActions(IEnumerable<string>? assemblyPaths = null, bool onlyValid = false)
+        public IEnumerable<Action> GetActions(IEnumerable<string>? assemblyPaths = null, ILogger? logger = null)
         {
             var trees = SourceFiles
                 .Select(path => CSharpSyntaxTree.ParseText(File.ReadAllText(path), path: path))
@@ -111,17 +111,17 @@ static string GetLocationOfAssemblyWithType(string typeName)
             {
                 foreach (var tree in trees)
                 {
-                    output.AddRange(GetActions(compilation, tree));
+                    output.AddRange(GetActions(compilation, tree, logger));
                 }
             }
             catch (Exception e)
             {
                 throw new AnalyserException(e.Message, e, diagnostics);
             }
 
-            if (onlyValid)
+            foreach (var action in output)
             {
-                output = output.Where(a => a.Validate(compilation).Count == 0).ToList();
+                action.Validate(compilation, logger);
             }
 
             return output;
diff --git a/Editor/Analysis/Diagnostics.cs b/Editor/Analysis/Diagnostics.cs
@@ -64,12 +64,57 @@ public static class Diagnostics
                         helpLinkUri: "https://docs.yarnspinner.dev/using-yarnspinner-with-unity/creating-commands-functions");
 
     public static readonly DiagnosticDescriptor YS1007ActionsMustBeInPublicTypes = new DiagnosticDescriptor(
-                        "YS1006",
+                        "YS1007",
                         title: $"Yarn action methods must be in a public type",
                         messageFormat: "Yarn actions must be in a publicly accessible type. {0}'s containing type, {1}, is {2}.",
                         category: "Yarn Spinner",
                         defaultSeverity: DiagnosticSeverity.Warning,
                         isEnabledByDefault: true,
                         helpLinkUri: "https://docs.yarnspinner.dev/using-yarnspinner-with-unity/creating-commands-functions");
+    
+    public static readonly DiagnosticDescriptor YS1008ActionsParamsArraysMustBeOfYarnTypes = new DiagnosticDescriptor(
+                        "YS1008",
+                        title: "Params arrays must be of a Yarn compatible type",
+                        messageFormat: "Params arrays must be of a Yarn compatible type, but {0} is of type \"{1}\"",
+                        category: "Yarn Spinner",
+                        defaultSeverity: DiagnosticSeverity.Warning,
+                        isEnabledByDefault: true,
+                        helpLinkUri: "https://docs.yarnspinner.dev/yarn-spinner-for-unity/creating-commands-functions");
+    
+    public static readonly DiagnosticDescriptor YS1009ActionsEnumAttributedParameterIsOfIncompatibleType = new DiagnosticDescriptor(
+                        "YS1009",
+                        title: "Yarn Enum attributed parameters must be of a Yarn compatible type",
+                        messageFormat: "Yarn Enum attributed parameters must be of a Yarn compatible type, but {0} is of type \"{1}\"",
+                        category: "Yarn Spinner",
+                        defaultSeverity: DiagnosticSeverity.Warning,
+                        isEnabledByDefault: true,
+                        helpLinkUri: "https://docs.yarnspinner.dev/yarn-spinner-for-unity/creating-commands-functions");
+    
+    public static readonly DiagnosticDescriptor YS1010ActionsNodeAttributedParameterIsOfIncompatibleType = new DiagnosticDescriptor(
+                        "YS1010",
+                        title: "Yarn Node attributed parameters must be a string",
+                        messageFormat: "Yarn Node attributed parameters must be a string, but {0} is of type \"{1}\"",
+                        category: "Yarn Spinner",
+                        defaultSeverity: DiagnosticSeverity.Warning,
+                        isEnabledByDefault: true,
+                        helpLinkUri: "https://docs.yarnspinner.dev/yarn-spinner-for-unity/creating-commands-functions");
+    
+    public static readonly DiagnosticDescriptor YS1011ActionsParameterIsAnIncompatibleType = new DiagnosticDescriptor(
+                        "YS1011",
+                        title: "Yarn action parameters must be of a Yarn compatible type",
+                        messageFormat: "Yarn action parameters must be of a Yarn compatible type, but {0} is of type \"{1}\"",
+                        category: "Yarn Spinner",
+                        defaultSeverity: DiagnosticSeverity.Warning,
+                        isEnabledByDefault: true,
+                        helpLinkUri: "https://docs.yarnspinner.dev/yarn-spinner-for-unity/creating-commands-functions");
+    
+    public static readonly DiagnosticDescriptor YS1012ActionIsALambda = new DiagnosticDescriptor(
+                        "YS1012",
+                        title: "Yarn actions can be lambdas but this generally isn't recommended.",
+                        messageFormat: "Yarn actions can be lambdas but this generally isn't recommended.",
+                        category: "Yarn Spinner",
+                        defaultSeverity: DiagnosticSeverity.Info,
+                        isEnabledByDefault: true,
+                        helpLinkUri: "https://docs.yarnspinner.dev/yarn-spinner-for-unity/creating-commands-functions");
 
 }
diff --git a/SourceGenerator/YarnSpinner.Unity.SourceCodeGenerator.dll b/SourceGenerator/YarnSpinner.Unity.SourceCodeGenerator.dll

Original file line number	Diff line number	Diff line change
`@@ -220,7 +220,7 @@ public void Execute(GeneratorExecutionContext context)`
`220`	`220`	`continue;`
`221`	`221`	`}`
`222`	`222`
`223`		`- var diagnostics = action.Validate(compilation);`
	`223`	`+ var diagnostics = action.Validate(compilation, output);`
`224`	`224`	`foreach (var diagnostic in diagnostics)`
`225`	`225`	`{`
`226`	`226`	`context.ReportDiagnostic(diagnostic);`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ public Analyser(string sourcePath)`
`58`	`58`	`public IEnumerable<string> SourceFiles => GetSourceFiles(SourcePath);`
`59`	`59`	`public string SourcePath { get; set; }`
`60`	`60`
`61`		`- public IEnumerable<Action> GetActions(IEnumerable<string>? assemblyPaths = null, bool onlyValid = false)`
	`61`	`+ public IEnumerable<Action> GetActions(IEnumerable<string>? assemblyPaths = null, ILogger? logger = null)`
`62`	`62`	`{`
`63`	`63`	`var trees = SourceFiles`
`64`	`64`	`.Select(path => CSharpSyntaxTree.ParseText(File.ReadAllText(path), path: path))`
`@@ -111,17 +111,17 @@ static string GetLocationOfAssemblyWithType(string typeName)`
`111`	`111`	`{`
`112`	`112`	`foreach (var tree in trees)`
`113`	`113`	`{`
`114`		`- output.AddRange(GetActions(compilation, tree));`
	`114`	`+ output.AddRange(GetActions(compilation, tree, logger));`
`115`	`115`	`}`
`116`	`116`	`}`
`117`	`117`	`catch (Exception e)`
`118`	`118`	`{`
`119`	`119`	`throw new AnalyserException(e.Message, e, diagnostics);`
`120`	`120`	`}`
`121`	`121`
`122`		`- if (onlyValid)`
	`122`	`+ foreach (var action in output)`
`123`	`123`	`{`
`124`		`- output = output.Where(a => a.Validate(compilation).Count == 0).ToList();`
	`124`	`+ action.Validate(compilation, logger);`
`125`	`125`	`}`
`126`	`126`
`127`	`127`	`return output;`