Skip to content

Remove AllowCaseFunction feature flag #4316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ericsciple merged 1 commit into from
Mar 27, 2026
Merged

Remove AllowCaseFunction feature flag #4316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Runner.Worker/ActionManifestManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/Runner.Worker/ActionManifestManagerLegacy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
Copy link
Copy Markdown
Contributor

@jsoref jsoref Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you added a BOM to this file?
ef bb bf

Copy link
Copy Markdown
Contributor

@jsoref jsoref Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericsciple

using System.Collections.Generic;
using System.IO;
using System.Threading;
Expand Down Expand Up @@ -315,7 +315,6 @@ private TemplateContext CreateTemplateContext(
maxBytes: 10 * 1024 * 1024),
Schema = _actionManifestSchema,
TraceWriter = executionContext.ToTemplateTraceWriter(),
AllowCaseFunction = false,
};

// Expression values from execution context
Expand Down
16 changes: 3 additions & 13 deletions src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ public IExpressionNode CreateTree(
String expression,
ITraceWriter trace,
IEnumerable<INamedValueInfo> namedValues,
IEnumerable<IFunctionInfo> functions,
Boolean allowCaseFunction = true)
IEnumerable<IFunctionInfo> 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);
}
Expand Down Expand Up @@ -416,20 +415,13 @@ 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);
}

private sealed class ParseContext
{
public Boolean AllowUnknownKeywords;
public Boolean AllowCaseFunction;
public readonly String Expression;
public readonly Dictionary<String, IFunctionInfo> ExtensionFunctions = new Dictionary<String, IFunctionInfo>(StringComparer.OrdinalIgnoreCase);
public readonly Dictionary<String, INamedValueInfo> ExtensionNamedValues = new Dictionary<String, INamedValueInfo>(StringComparer.OrdinalIgnoreCase);
Expand All @@ -445,8 +437,7 @@ public ParseContext(
ITraceWriter trace,
IEnumerable<INamedValueInfo> namedValues,
IEnumerable<IFunctionInfo> functions,
Boolean allowUnknownKeywords = false,
Boolean allowCaseFunction = true)
Boolean allowUnknownKeywords = false)
{
Expression = expression ?? String.Empty;
if (Expression.Length > ExpressionConstants.MaxLength)
Expand All @@ -467,7 +458,6 @@ public ParseContext(

LexicalAnalyzer = new LexicalAnalyzer(Expression);
AllowUnknownKeywords = allowUnknownKeywords;
AllowCaseFunction = allowCaseFunction;
}

private class NoOperationTraceWriter : ITraceWriter
Expand Down
6 changes: 0 additions & 6 deletions src/Sdk/DTObjectTemplating/ObjectTemplating/TemplateContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,6 @@ internal IDictionary<String, Object> State

internal ITraceWriter TraceWriter { get; set; }

/// <summary>
/// Gets or sets a value indicating whether the case expression function is allowed.
/// Defaults to true. Set to false to disable the case function.
/// </summary>
internal Boolean AllowCaseFunction { get; set; } = true;

private IDictionary<String, Int32> FileIds
{
get
Expand Down
8 changes: 4 additions & 4 deletions src/Sdk/DTObjectTemplating/ObjectTemplating/Tokens/TemplateToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
20 changes: 5 additions & 15 deletions src/Sdk/Expressions/ExpressionParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -17,10 +17,9 @@ public IExpressionNode CreateTree(
String expression,
ITraceWriter trace,
IEnumerable<INamedValueInfo> namedValues,
IEnumerable<IFunctionInfo> functions,
Boolean allowCaseFunction = true)
IEnumerable<IFunctionInfo> 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);
}
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -416,20 +415,13 @@ 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);
}

private sealed class ParseContext
{
public Boolean AllowUnknownKeywords;
public Boolean AllowCaseFunction;
public readonly String Expression;
public readonly Dictionary<String, IFunctionInfo> ExtensionFunctions = new Dictionary<String, IFunctionInfo>(StringComparer.OrdinalIgnoreCase);
public readonly Dictionary<String, INamedValueInfo> ExtensionNamedValues = new Dictionary<String, INamedValueInfo>(StringComparer.OrdinalIgnoreCase);
Expand All @@ -445,8 +437,7 @@ public ParseContext(
ITraceWriter trace,
IEnumerable<INamedValueInfo> namedValues,
IEnumerable<IFunctionInfo> functions,
Boolean allowUnknownKeywords = false,
Boolean allowCaseFunction = true)
Boolean allowUnknownKeywords = false)
{
Expression = expression ?? String.Empty;
if (Expression.Length > ExpressionConstants.MaxLength)
Expand All @@ -467,7 +458,6 @@ public ParseContext(

LexicalAnalyzer = new LexicalAnalyzer(Expression);
AllowUnknownKeywords = allowUnknownKeywords;
AllowCaseFunction = allowCaseFunction;
}

private class NoOperationTraceWriter : ITraceWriter
Expand Down
2 changes: 1 addition & 1 deletion src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
8 changes: 1 addition & 7 deletions src/Sdk/WorkflowParser/ObjectTemplating/TemplateContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -113,12 +113,6 @@ public IDictionary<String, Object> State
/// </summary>
internal Boolean StrictJsonParsing { get; set; }

/// <summary>
/// Gets or sets a value indicating whether the case expression function is allowed.
/// Defaults to true. Set to false to disable the case function.
/// </summary>
internal Boolean AllowCaseFunction { get; set; } = true;

internal ITraceWriter TraceWriter { get; set; }

private IDictionary<String, Int32> FileIds
Expand Down
12 changes: 6 additions & 6 deletions src/Sdk/WorkflowParser/ObjectTemplating/Tokens/TemplateToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -289,4 +289,4 @@ private MappingToken CreateMappingToken(TemplateContext context)
return result;
}
}
}
}
32 changes: 7 additions & 25 deletions src/Test/L0/Sdk/ExpressionParserL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using GitHub.DistributedTask.Expressions2;
using GitHub.DistributedTask.Expressions2;
using GitHub.DistributedTask.Expressions2.Sdk;
using GitHub.DistributedTask.ObjectTemplating;
using System;
Expand All @@ -9,7 +9,7 @@ namespace GitHub.Runner.Common.Tests.Sdk
{
/// <summary>
/// Regression tests for ExpressionParser.CreateTree to verify that
/// allowCaseFunction does not accidentally set allowUnknownKeywords.
/// the case function does not accidentally set allowUnknownKeywords.
/// </summary>
public sealed class ExpressionParserL0
{
Expand All @@ -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();
Expand Down Expand Up @@ -52,51 +52,33 @@ 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<INamedValueInfo>
{
new NamedValueInfo<ContextValueNode>("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<INamedValueInfo>
{
new NamedValueInfo<ContextValueNode>("github"),
};

var ex = Assert.Throws<ParseException>(() =>
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<INamedValueInfo>
{
new NamedValueInfo<ContextValueNode>("inputs"),
};

var ex = Assert.Throws<ParseException>(() =>
parser.CreateTree("github.ref", null, namedValues, null, allowCaseFunction: true));
parser.CreateTree("github.ref", null, namedValues, null));

Assert.Contains("Unrecognized named-value", ex.Message);
}
Expand Down
Loading
Loading