From 8626056d22128b6410ce2a84d74b40c6e2a377e9 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Fri, 27 Mar 2026 15:16:20 +0000 Subject: [PATCH] Remove AllowCaseFunction feature flag The case() expression function is now unconditionally available. Remove all gating logic, the feature flag constant, and related test scaffolding across both SDK copies and the Runner layer. --- 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 a70592381c7..014c053aa59 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -316,7 +316,6 @@ private TemplateContext CreateTemplateContext( 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 c332efd2eb7..d423aff8603 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 @@ private TemplateContext CreateTemplateContext( 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 f0dac3d24aa..95c6698c0d7 100644 --- a/src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs +++ b/src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs @@ -17,10 +17,9 @@ public IExpressionNode CreateTree( 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 @@ private static Boolean TryGetFunctionInfo( 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 @@ private static Boolean TryGetFunctionInfo( 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 @@ public ParseContext( 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 @@ public ParseContext( 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 d93eda398ac..af8cf76eca6 100644 --- a/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs +++ b/src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs @@ -86,12 +86,6 @@ internal IDictionary State 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 870163b7605..21a8aab7899 100644 --- a/src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateToken.cs +++ b/src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateToken.cs @@ -57,7 +57,7 @@ protected StringToken EvaluateStringToken( 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 @@ protected SequenceToken EvaluateSequenceToken( 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 @@ protected MappingToken EvaluateMappingToken( 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 @@ protected TemplateToken EvaluateTemplateToken( 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 87bb00baec8..38176eb36c2 100644 --- a/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs +++ b/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs @@ -681,7 +681,7 @@ private static String ConvertToIfCondition( 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 fd7dd1b458b..3c4ad104f5c 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 @@ public IExpressionNode CreateTree( 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 @@ private static void FlushTopEndParameters(ParseContext context) context.Operators.Pop(); } var functionOperands = PopOperands(context, parameterCount); - + // Node already exists on the operand stack function = (Function)context.Operands.Peek(); @@ -416,12 +415,6 @@ private static Boolean TryGetFunctionInfo( 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 @@ private static Boolean TryGetFunctionInfo( 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 @@ public ParseContext( 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 @@ public ParseContext( 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 8ae6ea0c9f7..0c53f87fcbd 100644 --- a/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs +++ b/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs @@ -1828,7 +1828,7 @@ private static BasicExpressionToken ConvertToIfCondition( 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 6e35e850be3..13a11ca2ceb 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 @@ public IDictionary State /// 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 0c1295ccb7b..fe90b4957de 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 @@ protected StringToken EvaluateStringToken( 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 @@ protected SequenceToken EvaluateSequenceToken( 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 @@ protected MappingToken EvaluateMappingToken( 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 @@ protected TemplateToken EvaluateTemplateToken( 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 @@ private MappingToken CreateMappingToken(TemplateContext context) 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 6512ec9e74a..313d038cd6a 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 @@ public sealed class ExpressionParserL0 [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 @@ public void CreateTree_AcceptsRecognizedNamedValue() [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 @@ public void CreateTree_CaseFunctionWorks_WhenAllowed() 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 @@ public void CreateTree_CaseFunctionDoesNotAffectUnknownKeywords() }; 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 9d707c3daa5..6a3da0c7209 100644 --- a/src/Test/L0/Worker/ActionManifestManagerL0.cs +++ b/src/Test/L0/Worker/ActionManifestManagerL0.cs @@ -504,7 +504,7 @@ public void Load_Node20Action() } } - [Fact] + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] public void Load_Node24Action() @@ -1006,6 +1006,45 @@ private void Setup([CallerMemberName] string name = "") _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();