From f0c228635e874e39eb53db0bf091c4925e53cbb1 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Fri, 27 Mar 2026 11:45:42 -0500 Subject: [PATCH] Remove AllowCaseFunction feature flag (#4316) --- src/Runner.Worker/ActionManifestManager.cs | 1 - .../ActionManifestManagerLegacy.cs | 3 +- .../Expressions2/ExpressionParser.cs | 16 ++------ .../ObjectTemplating/TemplateContext.cs | 6 --- .../ObjectTemplating/Tokens/TemplateToken.cs | 8 ++-- .../PipelineTemplateConverter.cs | 2 +- src/Sdk/Expressions/ExpressionParser.cs | 20 +++------ .../Conversion/WorkflowTemplateConverter.cs | 2 +- .../ObjectTemplating/TemplateContext.cs | 8 +--- .../ObjectTemplating/Tokens/TemplateToken.cs | 12 +++--- src/Test/L0/Sdk/ExpressionParserL0.cs | 32 ++++----------- src/Test/L0/Worker/ActionManifestManagerL0.cs | 41 ++++++++++++++++++- 12 files changed, 69 insertions(+), 82 deletions(-) diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index a70592381..014c053aa 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -316,7 +316,6 @@ namespace GitHub.Runner.Worker Schema = _actionManifestSchema, // TODO: Switch to real tracewriter for cutover TraceWriter = new GitHub.Actions.WorkflowParser.ObjectTemplating.EmptyTraceWriter(), - AllowCaseFunction = false, }; // Expression values from execution context diff --git a/src/Runner.Worker/ActionManifestManagerLegacy.cs b/src/Runner.Worker/ActionManifestManagerLegacy.cs index c332efd2e..d423aff86 100644 --- a/src/Runner.Worker/ActionManifestManagerLegacy.cs +++ b/src/Runner.Worker/ActionManifestManagerLegacy.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Threading; @@ -315,7 +315,6 @@ namespace GitHub.Runner.Worker maxBytes: 10 * 1024 * 1024), Schema = _actionManifestSchema, TraceWriter = executionContext.ToTemplateTraceWriter(), - AllowCaseFunction = false, }; // Expression values from execution context diff --git a/src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs b/src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs index f0dac3d24..95c6698c0 100644 --- a/src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs +++ b/src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs @@ -17,10 +17,9 @@ namespace GitHub.DistributedTask.Expressions2 String expression, ITraceWriter trace, IEnumerable namedValues, - IEnumerable functions, - Boolean allowCaseFunction = true) + IEnumerable functions) { - var context = new ParseContext(expression, trace, namedValues, functions, allowCaseFunction: allowCaseFunction); + var context = new ParseContext(expression, trace, namedValues, functions); context.Trace.Info($"Parsing expression: <{expression}>"); return CreateTree(context); } @@ -416,12 +415,6 @@ namespace GitHub.DistributedTask.Expressions2 String name, out IFunctionInfo functionInfo) { - if (String.Equals(name, "case", StringComparison.OrdinalIgnoreCase) && !context.AllowCaseFunction) - { - functionInfo = null; - return false; - } - return ExpressionConstants.WellKnownFunctions.TryGetValue(name, out functionInfo) || context.ExtensionFunctions.TryGetValue(name, out functionInfo); } @@ -429,7 +422,6 @@ namespace GitHub.DistributedTask.Expressions2 private sealed class ParseContext { public Boolean AllowUnknownKeywords; - public Boolean AllowCaseFunction; public readonly String Expression; public readonly Dictionary ExtensionFunctions = new Dictionary(StringComparer.OrdinalIgnoreCase); public readonly Dictionary ExtensionNamedValues = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -445,8 +437,7 @@ namespace GitHub.DistributedTask.Expressions2 ITraceWriter trace, IEnumerable namedValues, IEnumerable functions, - Boolean allowUnknownKeywords = false, - Boolean allowCaseFunction = true) + Boolean allowUnknownKeywords = false) { Expression = expression ?? String.Empty; if (Expression.Length > ExpressionConstants.MaxLength) @@ -467,7 +458,6 @@ namespace GitHub.DistributedTask.Expressions2 LexicalAnalyzer = new LexicalAnalyzer(Expression); AllowUnknownKeywords = allowUnknownKeywords; - AllowCaseFunction = allowCaseFunction; } private class NoOperationTraceWriter : ITraceWriter diff --git a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs index d93eda398..af8cf76ec 100644 --- a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs +++ b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs @@ -86,12 +86,6 @@ namespace GitHub.DistributedTask.ObjectTemplating internal ITraceWriter TraceWriter { get; set; } - /// - /// Gets or sets a value indicating whether the case expression function is allowed. - /// Defaults to true. Set to false to disable the case function. - /// - internal Boolean AllowCaseFunction { get; set; } = true; - private IDictionary FileIds { get diff --git a/src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateToken.cs b/src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateToken.cs index 870163b76..21a8aab78 100644 --- a/src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateToken.cs +++ b/src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateToken.cs @@ -57,7 +57,7 @@ namespace GitHub.DistributedTask.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, @@ -94,7 +94,7 @@ namespace GitHub.DistributedTask.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, @@ -123,7 +123,7 @@ namespace GitHub.DistributedTask.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, @@ -152,7 +152,7 @@ namespace GitHub.DistributedTask.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, diff --git a/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs b/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs index 87bb00bae..38176eb36 100644 --- a/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs +++ b/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs @@ -681,7 +681,7 @@ namespace GitHub.DistributedTask.Pipelines.ObjectTemplating var node = default(ExpressionNode); try { - node = expressionParser.CreateTree(condition, null, namedValues, functions, allowCaseFunction: context.AllowCaseFunction) as ExpressionNode; + node = expressionParser.CreateTree(condition, null, namedValues, functions) as ExpressionNode; } catch (Exception ex) { diff --git a/src/Sdk/Expressions/ExpressionParser.cs b/src/Sdk/Expressions/ExpressionParser.cs index fd7dd1b45..3c4ad104f 100644 --- a/src/Sdk/Expressions/ExpressionParser.cs +++ b/src/Sdk/Expressions/ExpressionParser.cs @@ -1,4 +1,4 @@ -#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references +#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references using System; using System.Collections.Generic; @@ -17,10 +17,9 @@ namespace GitHub.Actions.Expressions String expression, ITraceWriter trace, IEnumerable namedValues, - IEnumerable functions, - Boolean allowCaseFunction = true) + IEnumerable functions) { - var context = new ParseContext(expression, trace, namedValues, functions, allowCaseFunction: allowCaseFunction); + var context = new ParseContext(expression, trace, namedValues, functions); context.Trace.Info($"Parsing expression: <{expression}>"); return CreateTree(context); } @@ -322,7 +321,7 @@ namespace GitHub.Actions.Expressions context.Operators.Pop(); } var functionOperands = PopOperands(context, parameterCount); - + // Node already exists on the operand stack function = (Function)context.Operands.Peek(); @@ -416,12 +415,6 @@ namespace GitHub.Actions.Expressions String name, out IFunctionInfo functionInfo) { - if (String.Equals(name, "case", StringComparison.OrdinalIgnoreCase) && !context.AllowCaseFunction) - { - functionInfo = null; - return false; - } - return ExpressionConstants.WellKnownFunctions.TryGetValue(name, out functionInfo) || context.ExtensionFunctions.TryGetValue(name, out functionInfo); } @@ -429,7 +422,6 @@ namespace GitHub.Actions.Expressions private sealed class ParseContext { public Boolean AllowUnknownKeywords; - public Boolean AllowCaseFunction; public readonly String Expression; public readonly Dictionary ExtensionFunctions = new Dictionary(StringComparer.OrdinalIgnoreCase); public readonly Dictionary ExtensionNamedValues = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -445,8 +437,7 @@ namespace GitHub.Actions.Expressions ITraceWriter trace, IEnumerable namedValues, IEnumerable functions, - Boolean allowUnknownKeywords = false, - Boolean allowCaseFunction = true) + Boolean allowUnknownKeywords = false) { Expression = expression ?? String.Empty; if (Expression.Length > ExpressionConstants.MaxLength) @@ -467,7 +458,6 @@ namespace GitHub.Actions.Expressions LexicalAnalyzer = new LexicalAnalyzer(Expression); AllowUnknownKeywords = allowUnknownKeywords; - AllowCaseFunction = allowCaseFunction; } private class NoOperationTraceWriter : ITraceWriter diff --git a/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs b/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs index 8ae6ea0c9..0c53f87fc 100644 --- a/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs +++ b/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs @@ -1828,7 +1828,7 @@ namespace GitHub.Actions.WorkflowParser.Conversion var node = default(ExpressionNode); try { - node = expressionParser.CreateTree(condition, null, namedValues, functions, allowCaseFunction: context.AllowCaseFunction) as ExpressionNode; + node = expressionParser.CreateTree(condition, null, namedValues, functions) as ExpressionNode; } catch (Exception ex) { diff --git a/src/Sdk/WorkflowParser/ObjectTemplating/TemplateContext.cs b/src/Sdk/WorkflowParser/ObjectTemplating/TemplateContext.cs index 6e35e850b..13a11ca2c 100644 --- a/src/Sdk/WorkflowParser/ObjectTemplating/TemplateContext.cs +++ b/src/Sdk/WorkflowParser/ObjectTemplating/TemplateContext.cs @@ -1,4 +1,4 @@ -#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references +#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references using System; using System.Collections.Generic; @@ -113,12 +113,6 @@ namespace GitHub.Actions.WorkflowParser.ObjectTemplating /// internal Boolean StrictJsonParsing { get; set; } - /// - /// Gets or sets a value indicating whether the case expression function is allowed. - /// Defaults to true. Set to false to disable the case function. - /// - internal Boolean AllowCaseFunction { get; set; } = true; - internal ITraceWriter TraceWriter { get; set; } private IDictionary FileIds diff --git a/src/Sdk/WorkflowParser/ObjectTemplating/Tokens/TemplateToken.cs b/src/Sdk/WorkflowParser/ObjectTemplating/Tokens/TemplateToken.cs index 0c1295ccb..fe90b4957 100644 --- a/src/Sdk/WorkflowParser/ObjectTemplating/Tokens/TemplateToken.cs +++ b/src/Sdk/WorkflowParser/ObjectTemplating/Tokens/TemplateToken.cs @@ -1,4 +1,4 @@ -#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references +#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references using System; using System.Collections.Generic; @@ -55,7 +55,7 @@ namespace GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, @@ -93,7 +93,7 @@ namespace GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, @@ -123,7 +123,7 @@ namespace GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, @@ -153,7 +153,7 @@ namespace GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens var originalBytes = context.Memory.CurrentBytes; try { - var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions, allowCaseFunction: context.AllowCaseFunction); + var tree = new ExpressionParser().CreateTree(expression, null, context.GetExpressionNamedValues(), context.ExpressionFunctions); var options = new EvaluationOptions { MaxMemory = context.Memory.MaxBytes, @@ -289,4 +289,4 @@ namespace GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens return result; } } -} \ No newline at end of file +} diff --git a/src/Test/L0/Sdk/ExpressionParserL0.cs b/src/Test/L0/Sdk/ExpressionParserL0.cs index 6512ec9e7..313d038cd 100644 --- a/src/Test/L0/Sdk/ExpressionParserL0.cs +++ b/src/Test/L0/Sdk/ExpressionParserL0.cs @@ -1,4 +1,4 @@ -using GitHub.DistributedTask.Expressions2; +using GitHub.DistributedTask.Expressions2; using GitHub.DistributedTask.Expressions2.Sdk; using GitHub.DistributedTask.ObjectTemplating; using System; @@ -9,7 +9,7 @@ namespace GitHub.Runner.Common.Tests.Sdk { /// /// Regression tests for ExpressionParser.CreateTree to verify that - /// allowCaseFunction does not accidentally set allowUnknownKeywords. + /// the case function does not accidentally set allowUnknownKeywords. /// public sealed class ExpressionParserL0 { @@ -18,7 +18,7 @@ namespace GitHub.Runner.Common.Tests.Sdk [Trait("Category", "Sdk")] public void CreateTree_RejectsUnrecognizedNamedValue() { - // Regression: allowCaseFunction was passed positionally into + // Regression: the case function parameter was passed positionally into // the allowUnknownKeywords parameter, causing all named values // to be silently accepted. var parser = new ExpressionParser(); @@ -52,7 +52,7 @@ namespace GitHub.Runner.Common.Tests.Sdk [Fact] [Trait("Level", "L0")] [Trait("Category", "Sdk")] - public void CreateTree_CaseFunctionWorks_WhenAllowed() + public void CreateTree_CaseFunctionWorks() { var parser = new ExpressionParser(); var namedValues = new List @@ -60,35 +60,17 @@ namespace GitHub.Runner.Common.Tests.Sdk new NamedValueInfo("github"), }; - var node = parser.CreateTree("case(github.event_name, 'push', 'Push Event')", null, namedValues, null, allowCaseFunction: true); + var node = parser.CreateTree("case(github.event_name, 'push', 'Push Event')", null, namedValues, null); Assert.NotNull(node); } - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Sdk")] - public void CreateTree_CaseFunctionRejected_WhenDisallowed() - { - var parser = new ExpressionParser(); - var namedValues = new List - { - new NamedValueInfo("github"), - }; - - var ex = Assert.Throws(() => - parser.CreateTree("case(github.event_name, 'push', 'Push Event')", null, namedValues, null, allowCaseFunction: false)); - - Assert.Contains("Unrecognized function", ex.Message); - } - [Fact] [Trait("Level", "L0")] [Trait("Category", "Sdk")] public void CreateTree_CaseFunctionDoesNotAffectUnknownKeywords() { - // The key regression test: with allowCaseFunction=true (default), - // unrecognized named values must still be rejected. + // The key regression test: unrecognized named values must still be rejected. var parser = new ExpressionParser(); var namedValues = new List { @@ -96,7 +78,7 @@ namespace GitHub.Runner.Common.Tests.Sdk }; var ex = Assert.Throws(() => - parser.CreateTree("github.ref", null, namedValues, null, allowCaseFunction: true)); + parser.CreateTree("github.ref", null, namedValues, null)); Assert.Contains("Unrecognized named-value", ex.Message); } diff --git a/src/Test/L0/Worker/ActionManifestManagerL0.cs b/src/Test/L0/Worker/ActionManifestManagerL0.cs index 9d707c3da..6a3da0c72 100644 --- a/src/Test/L0/Worker/ActionManifestManagerL0.cs +++ b/src/Test/L0/Worker/ActionManifestManagerL0.cs @@ -504,7 +504,7 @@ namespace GitHub.Runner.Common.Tests.Worker } } - [Fact] + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] public void Load_Node24Action() @@ -1006,6 +1006,45 @@ namespace GitHub.Runner.Common.Tests.Worker _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())).Callback((Issue issue, ExecutionContextLogOptions logOptions) => { _hc.GetTrace().Info($"[{issue.Type}]{logOptions.LogMessageOverride ?? issue.Message}"); }); } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Evaluate_Default_Input_Case_Function() + { + try + { + //Arrange + Setup(); + + var actionManifest = new ActionManifestManager(); + actionManifest.Initialize(_hc); + + _ec.Object.ExpressionValues["github"] = new LegacyContextData.DictionaryContextData + { + { "ref", new LegacyContextData.StringContextData("refs/heads/main") }, + }; + _ec.Object.ExpressionValues["strategy"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["matrix"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["steps"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["job"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["runner"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["env"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionFunctions.Add(new LegacyExpressions.FunctionInfo("hashFiles", 1, 255)); + + // Act — evaluate a case() expression as a default input value. + // The feature flag is set, so this should succeed. + var token = new BasicExpressionToken(null, null, null, "case(true, 'matched', 'default')"); + var result = actionManifest.EvaluateDefaultInput(_ec.Object, "testInput", token); + + // Assert — case() should evaluate successfully + Assert.Equal("matched", result); + } + finally + { + Teardown(); + } + } + private void Teardown() { _hc?.Dispose();