Skip to content
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.

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

Fix validation for c# not considering variable name is case sensitive [STUD-69312] #316

Merged
merged 1 commit into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 31 additions & 0 deletions src/Test/TestCases.Workflows/ExpressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -535,4 +535,35 @@ public void CSharpReferenceTypeIsCheckedForGenerics()
var result = ActivityValidationServices.Validate(sequence, _useValidator);
result.Errors.ShouldBeEmpty();
}

[Fact]
public void CS_IsCaseSensitive_WhenSearchingForVariable_Throws()
{
var seq = new Sequence();
seq.Variables.Add(new Variable<string>("ABC"));
seq.Activities.Add(new WriteLine { Text = new InArgument<string>(new CSharpValue<string>("abc")) });
var valid = ActivityValidationServices.Validate(seq, _useValidator);
valid.Errors.Count.ShouldBe(1);
valid.Errors.First().Message.ShouldBe("CS0103: The name 'abc' does not exist in the current context");
}

[Fact]
public void CS_IsCaseSensitive_WhenSearchingForVariable_Succeeds()
{
var seq = new Sequence();
seq.Variables.Add(new Variable<string>("ABC"));
seq.Activities.Add(new WriteLine { Text = new InArgument<string>(new CSharpValue<string>("ABC")) });
var valid = ActivityValidationServices.Validate(seq, _useValidator);
valid.Errors.ShouldBeEmpty();
}

[Fact]
public void VB_IsCaseInsensitive()
{
var seq = new Sequence();
seq.Variables.Add(new Variable<string>("ABC"));
seq.Activities.Add(new WriteLine { Text = new InArgument<string>(new VisualBasicValue<string>("abc")) });
var valid = ActivityValidationServices.Validate(seq, _useValidator);
valid.Errors.ShouldBeEmpty();
}
}
16 changes: 8 additions & 8 deletions src/Test/TestCases.Workflows/JitCompilerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void VisualBasicJitCompiler_FieldAccess()
[Fact]
public void VisualBasicJitCompiler_PropertyAccess_SameNameAsVariable()
{
static Type VariableTypeGetter(string name)
static Type VariableTypeGetter(string name, StringComparison stringComparison)
{
return name switch
{
Expand All @@ -69,7 +69,7 @@ static Type VariableTypeGetter(string name)
[InlineData(17)]
public void VisualBasicJitCompiler_ExpressionWithMultipleVariablesVariables(int noOfVar)
{
static Type VariableTypeGetter(string name)
static Type VariableTypeGetter(string name, StringComparison stringComparison)
{
return name switch
{
Expand All @@ -85,7 +85,7 @@ static Type VariableTypeGetter(string name)
[Fact]
public void CSharpJitCompiler_PropertyAccess()
{
static Type VariableTypeGetter(string name)
static Type VariableTypeGetter(string name, StringComparison stringComparison)
{
return name switch
{
Expand All @@ -108,7 +108,7 @@ static Type VariableTypeGetter(string name)
[InlineData(17)]
public void CSharpJitCompiler_ExpressionWithMultipleVariablesVariables(int noOfVar)
{
static Type VariableTypeGetter(string name)
static Type VariableTypeGetter(string name, StringComparison stringComparison)
{
return name switch
{
Expand All @@ -125,7 +125,7 @@ static Type VariableTypeGetter(string name)
public void VbExpression_UndeclaredObject()
{
var expressionToCompile = "new UndeclaredClass()";
var sut = () => _vbJitCompiler.CompileExpression(new ExpressionToCompile(expressionToCompile, _namespaces, (s)=>null, typeof(object)));
var sut = () => _vbJitCompiler.CompileExpression(new ExpressionToCompile(expressionToCompile, _namespaces, (s, c)=>null, typeof(object)));

Assert.ThrowsAny<SourceExpressionException>(sut);
}
Expand All @@ -134,19 +134,19 @@ public void VbExpression_UndeclaredObject()
public void VbExpression_WithObjectInitializer()
{
var expressionToCompile = "new TestIndexerClass() With {.Field=\"1\"}";
var result = _vbJitCompiler.CompileExpression(new ExpressionToCompile(expressionToCompile, _namespaces, (s)=>null, typeof(TestIndexerClass)));
var result = _vbJitCompiler.CompileExpression(new ExpressionToCompile(expressionToCompile, _namespaces, (s, c)=>null, typeof(TestIndexerClass)));
result.ReturnType.ShouldBe(typeof(TestIndexerClass));
}

[Fact]
public void VbExpression_UndeclaredObjectWithObjectInitializer()
{
var expressionToCompile = "new UndeclaredClass() With {.Field=\"1\"}";
var sut = () => _vbJitCompiler.CompileExpression(new ExpressionToCompile(expressionToCompile, _namespaces, (s)=>null, typeof(object)));
var sut = () => _vbJitCompiler.CompileExpression(new ExpressionToCompile(expressionToCompile, _namespaces, (s, c)=>null, typeof(object)));

Assert.ThrowsAny<SourceExpressionException>(sut);
}
private static Type VariableTypeGetter(string name)
private static Type VariableTypeGetter(string name, StringComparison stringComparison)
=> name switch
{
"testIndexerClass" => typeof(TestIndexerClass),
Expand Down
11 changes: 8 additions & 3 deletions src/Test/TestCases.Workflows/XamlTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -317,16 +317,16 @@ public VisualBasicInferTypeData()
public void Should_compile_CSharp()
{
var compiler = new CSharpJitCompiler(new[] { typeof(Expression).Assembly, typeof(Enumerable).Assembly }.ToHashSet());
var result = compiler.CompileExpression(new ExpressionToCompile("source.Select(s=>s).Sum()", new[] { "System", "System.Linq", "System.Linq.Expressions", "System.Collections.Generic" },
name => name == "source" ? typeof(List<int>) : null, typeof(int)));
var result = compiler.CompileExpression(new ExpressionToCompile("source.Select(s=>s).Sum()", ["System", "System.Linq", "System.Linq.Expressions", "System.Collections.Generic"],
(name, comparer )=> name == "source" ? typeof(List<int>) : null, typeof(int)));
((Func<List<int>, int>)result.Compile())(new List<int> { 1, 2, 3 }).ShouldBe(6);
}

[Fact]
public void Should_Fail_VBConversion()
{
var compiler = new VbJitCompiler(new[] { typeof(int).Assembly, typeof(Expression).Assembly, typeof(Conversions).Assembly }.ToHashSet());
new Action(() => compiler.CompileExpression(new ExpressionToCompile("1", new[] { "System", "System.Linq", "System.Linq.Expressions" }, _ => typeof(int), typeof(string))))
new Action(() => compiler.CompileExpression(new ExpressionToCompile("1", new[] { "System", "System.Linq", "System.Linq.Expressions" }, (_, __) => typeof(int), typeof(string))))
.ShouldThrow<SourceExpressionException>().Message.ShouldContain("BC30512: Option Strict On disallows implicit conversions");
}

Expand Down Expand Up @@ -391,6 +391,7 @@ public class AheadOfTimeXamlTests : XamlTestsBase
</Activity>";
[Fact]
public void CompileExpressionsDefault() => InvokeWorkflow(CSharpExpressions);

[Fact]
public void CompileExpressionsWithCompiler() =>
new Action(() => ActivityXamlServices.Load(new StringReader(CSharpExpressions),
Expand All @@ -414,20 +415,23 @@ public void CSharpCompileError()
new Action(() => InvokeWorkflow(xaml)).ShouldThrow<InvalidOperationException>().Data.Values.Cast<string>()
.ShouldAllBe(error => error.Contains("error CS0103: The name 'constant' does not exist in the current context"));
}

[Fact]
public void SetCompiledExpressionRootForImplementation()
{
var writeLine = new WriteLine { Text = new InArgument<string>(new VisualBasicValue<string>("[s]")) };
CompiledExpressionInvoker.SetCompiledExpressionRootForImplementation(writeLine, new Expressions());
WorkflowInvoker.Invoke(writeLine);
}

[Fact]
public void ValidateSkipCompilation()
{
var writeLine = new WriteLine { Text = new InArgument<string>(new VisualBasicValue<string>("[s]")) };
var results = ActivityValidationServices.Validate(writeLine, new() { SkipExpressionCompilation = true });
results.Errors.ShouldBeEmpty();
}

[Fact]
public void DuplicateVariable()
{
Expand All @@ -439,6 +443,7 @@ public void DuplicateVariable()
var withMyVar = (WithMyVar)WorkflowInspectionServices.Resolve(root, "1.1");
((ITextExpression)((Sequence)withMyVar.Body.Handler).Activities[0]).GetExpressionTree();
}

[Fact]
public void CSharpInputOutput()
{
Expand Down
26 changes: 14 additions & 12 deletions src/UiPath.Workflow/Activities/JitCompilerHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ internal abstract class JitCompilerHelper
private static readonly FindMatch s_delegateFindAllLocationReferenceMatch = FindAllLocationReferenceMatch;

protected LocationReferenceEnvironment Environment;
protected abstract StringComparison StringComparison { get; }

// this is a flag to differentiate the cached short-cut Rewrite from the normal post-compilation Rewrite
protected bool IsShortCutRewrite;
Expand Down Expand Up @@ -190,10 +191,10 @@ private static void ExtractNamespacesAndReferences(VisualBasicSettings vbSetting
}

private static bool FindLocationReferenceMatchShortcut(LocationReference reference, string targetName,
Type targetType, out bool terminateSearch)
Type targetType, StringComparison stringComparison, out bool terminateSearch)
{
terminateSearch = false;
if (string.Equals(reference.Name, targetName, StringComparison.OrdinalIgnoreCase))
if (string.Equals(reference.Name, targetName, stringComparison))
{
if (targetType != reference.Type)
{
Expand All @@ -207,11 +208,11 @@ private static bool FindLocationReferenceMatchShortcut(LocationReference referen
return false;
}

private static bool FindFirstLocationReferenceMatch(LocationReference reference, string targetName, Type targetType,
private static bool FindFirstLocationReferenceMatch(LocationReference reference, string targetName, Type targetType, StringComparison stringComparison,
out bool terminateSearch)
{
terminateSearch = false;
if (string.Equals(reference.Name, targetName, StringComparison.OrdinalIgnoreCase))
if (string.Equals(reference.Name, targetName, stringComparison))
{
terminateSearch = true;
return true;
Expand All @@ -220,11 +221,11 @@ private static bool FindFirstLocationReferenceMatch(LocationReference reference,
return false;
}

private static bool FindAllLocationReferenceMatch(LocationReference reference, string targetName, Type targetType,
private static bool FindAllLocationReferenceMatch(LocationReference reference, string targetName, Type targetType, StringComparison stringComparison,
out bool terminateSearch)
{
terminateSearch = false;
if (string.Equals(reference.Name, targetName, StringComparison.OrdinalIgnoreCase))
if (string.Equals(reference.Name, targetName, stringComparison))
{
return true;
}
Expand Down Expand Up @@ -434,6 +435,7 @@ protected Expression Rewrite(Expression expression, ReadOnlyCollection<Parameter
findMatch,
variableExpression.Name,
variableExpression.Type,
StringComparison,
out var foundMultiple);

if (finalReference != null && !foundMultiple)
Expand Down Expand Up @@ -1082,7 +1084,7 @@ private static void EnsureTypeReferencedRecurse(Type type, HashSet<Type> already
}

private static LocationReference FindLocationReferencesFromEnvironment(LocationReferenceEnvironment environment,
FindMatch findMatch, string targetName, Type targetType, out bool foundMultiple)
FindMatch findMatch, string targetName, Type targetType, StringComparison stringComparison, out bool foundMultiple)
{
var currentEnvironment = environment;
foundMultiple = false;
Expand All @@ -1091,7 +1093,7 @@ private static LocationReference FindLocationReferencesFromEnvironment(LocationR
LocationReference toReturn = null;
foreach (var reference in currentEnvironment.GetLocationReferences())
{
if (findMatch(reference, targetName, targetType, out var terminateSearch))
if (findMatch(reference, targetName, targetType, stringComparison, out var terminateSearch))
{
if (toReturn != null)
{
Expand Down Expand Up @@ -1119,7 +1121,7 @@ private static LocationReference FindLocationReferencesFromEnvironment(LocationR
return null;
}

private delegate bool FindMatch(LocationReference reference, string targetName, Type targetType,
private delegate bool FindMatch(LocationReference reference, string targetName, Type targetType, StringComparison stringComparison,
out bool terminateSearch);

// this is a place holder for LambdaExpression(raw Expression Tree) that is to be stored in the cache
Expand Down Expand Up @@ -1192,12 +1194,12 @@ public ScriptAndTypeScope(LocationReferenceEnvironment environmentProvider)

public string ErrorMessage { get; private set; }

public Type FindVariable(string name)
public Type FindVariable(string name, StringComparison stringComparison)
{
LocationReference referenceToReturn = null;
var findMatch = s_delegateFindAllLocationReferenceMatch;
referenceToReturn =
FindLocationReferencesFromEnvironment(_environmentProvider, findMatch, name, null, out var foundMultiple);
FindLocationReferencesFromEnvironment(_environmentProvider, findMatch, name, null, stringComparison, out var foundMultiple);
if (referenceToReturn != null)
{
if (foundMultiple)
Expand Down Expand Up @@ -1489,7 +1491,7 @@ public override LambdaExpression CompileNonGeneric(LocationReferenceEnvironment
return Expression.Lambda(finalBody, lambda.Parameters);
}

private ExpressionToCompile ExpressionToCompile(Func<string, Type> variableTypeGetter, Type lambdaReturnType)
private ExpressionToCompile ExpressionToCompile(Func<string, StringComparison, Type> variableTypeGetter, Type lambdaReturnType)
{
return new ExpressionToCompile(TextToCompile, NamespaceImports, variableTypeGetter, lambdaReturnType);
}
Expand Down
4 changes: 2 additions & 2 deletions src/UiPath.Workflow/Activities/ScriptingJitCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public abstract class JustInTimeCompiler
public record CompilerInput(string Code, IReadOnlyCollection<string> ImportedNamespaces) { }

public record ExpressionToCompile(string Code, IReadOnlyCollection<string> ImportedNamespaces,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Func<string, Type> VariableTypeGetter, Type LambdaReturnType)
Func<string, StringComparison, Type> VariableTypeGetter, Type LambdaReturnType)
: CompilerInput(Code, ImportedNamespaces)
{ }

Expand Down Expand Up @@ -57,7 +57,7 @@ public override LambdaExpression CompileExpression(ExpressionToCompile expressio
var identifiers = GetIdentifiers(syntaxTree);
var resolvedIdentifiers =
identifiers
.Select(name => (Name: name, Type: expressionToCompile.VariableTypeGetter(name)))
.Select(name => (Name: name, Type: expressionToCompile.VariableTypeGetter(name, CompilerHelper.IdentifierNameComparison)))
.Where(var => var.Type != null)
.ToArray();
var names = string.Join(CompilerHelper.Comma, resolvedIdentifiers.Select(var => var.Name));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public sealed class CSharpCompilerHelper : CompilerHelper

public override StringComparer IdentifierNameComparer { get; } = StringComparer.Ordinal;

public override StringComparison IdentifierNameComparison { get; } = StringComparison.Ordinal;

public override string GetTypeName(Type type) =>
(string)s_typeNameFormatter.FormatTypeName(type, s_typeOptions);

Expand Down
2 changes: 2 additions & 0 deletions src/UiPath.Workflow/Activities/Utils/CompilerHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public abstract class CompilerHelper

public abstract StringComparer IdentifierNameComparer { get; }

public abstract StringComparison IdentifierNameComparison { get; }

public abstract int IdentifierKind { get; }

public abstract (string, string) DefineDelegate(string types);
Expand Down
2 changes: 2 additions & 0 deletions src/UiPath.Workflow/Activities/Utils/VBCompilerHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public sealed class VBCompilerHelper : CompilerHelper

public override StringComparer IdentifierNameComparer { get; } = StringComparer.OrdinalIgnoreCase;

public override StringComparison IdentifierNameComparison { get; } = StringComparison.OrdinalIgnoreCase;

public override VisualBasicParseOptions ScriptParseOptions { get; } = new VisualBasicParseOptions(kind: SourceCodeKind.Script, languageVersion: LanguageVersion.Latest);


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ namespace Microsoft.CSharp.Activities;

internal class CSharpHelper : JitCompilerHelper<CSharpHelper>
{
public CSharpHelper(string expressionText, HashSet<AssemblyReference> assemblyReferences,
HashSet<string> namespaceImportsNames) : base(expressionText, assemblyReferences, namespaceImportsNames) { }
protected override StringComparison StringComparison => StringComparison.Ordinal;

protected override JustInTimeCompiler CreateCompiler(HashSet<Assembly> references) =>
protected override JustInTimeCompiler CreateCompiler(HashSet<Assembly> references) =>
new CSharpJitCompiler(references);

public CSharpHelper(string expressionText, HashSet<AssemblyReference> assemblyReferences,
HashSet<string> namespaceImportsNames) : base(expressionText, assemblyReferences, namespaceImportsNames) { }

internal const string Language = "C#";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected override SyntaxTree GetSyntaxTreeForExpression(string expression, bool
var identifiers = syntaxTree.GetRoot().DescendantNodesAndSelf().Where(n => n.RawKind == (int)SyntaxKind.IdentifierName)
.Select(n => n.ToString()).Distinct(_compilerHelper.IdentifierNameComparer);
var resolvedIdentifiers = identifiers
.Select(name => (Name: name, Type: new ScriptAndTypeScope(environment).FindVariable(name)))
.Select(name => (Name: name, Type: new ScriptAndTypeScope(environment).FindVariable(name, _compilerHelper.IdentifierNameComparison)))
.Where(var => var.Type != null)
.ToArray();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace Microsoft.VisualBasic.Activities;

internal class VisualBasicHelper : JitCompilerHelper<VisualBasicHelper>
{
protected override StringComparison StringComparison => StringComparison.OrdinalIgnoreCase;

public VisualBasicHelper(string expressionText, HashSet<AssemblyReference> assemblyReferences,
HashSet<string> namespaceImportsNames) : base(expressionText, assemblyReferences, namespaceImportsNames) { }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected override SyntaxTree GetSyntaxTreeForExpression(string expression, bool
var identifiers = syntaxTree.GetRoot().DescendantNodesAndSelf().Where(n => n.RawKind == (int)SyntaxKind.IdentifierName)
.Select(n => n.ToString()).Distinct(_compilerHelper.IdentifierNameComparer);
var resolvedIdentifiers = identifiers
.Select(name => (Name: name, Type: new ScriptAndTypeScope(environment).FindVariable(name)))
.Select(name => (Name: name, Type: new ScriptAndTypeScope(environment).FindVariable(name, _compilerHelper.IdentifierNameComparison)))
.Where(var => var.Type != null)
.ToArray();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ private void AddExpressionToValidate(ExpressionToValidate expressionToValidate,
.Select(n => n.ToString()).Distinct(CompilerHelper.IdentifierNameComparer);
var resolvedIdentifiers =
identifiers
.Select(name => (Name: name, Type: new ScriptAndTypeScope(expressionToValidate.Environment).FindVariable(name)))
.Select(name => (Name: name, Type: new ScriptAndTypeScope(expressionToValidate.Environment).FindVariable(name, CompilerHelper.IdentifierNameComparison)))
.Where(var => var.Type != null)
.ToArray();

Expand Down