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

Include the position information for the Expression nodes in the Fhipath Parser #2774

Merged
merged 24 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
b8094ba
Introduce a CustomExpression that can permit the addition of other ex…
brianpos Jan 14, 2024
5cbd43c
Add a debugger display for function expressions
brianpos Jan 14, 2024
4dd5449
Add a new Expresion type for Identifier (IdentifierExpression). Deriv…
brianpos Jan 14, 2024
6beadd7
Introduce a BracketExpression type as a CustomExpression to permit ro…
brianpos Jan 14, 2024
0a17193
Set the location property while parsing the expression (using the Spr…
brianpos Jan 15, 2024
20efc44
* Roundtrip Visualizer to output a canonical version of the expressio…
brianpos Jan 16, 2024
6a19a66
Merge branch 'develop' into feature/fhirpath-parsing
brianpos Apr 22, 2024
36606c1
#2517 line numbering information while parsing the expressions into t…
brianpos Apr 22, 2024
ba16bd6
Merge branch 'develop' into feature/fhirpath-position
brianpos Apr 22, 2024
92c8537
Minor compilation issues
brianpos Apr 22, 2024
7b86874
Roundtrip visitor and tests from the position branch
brianpos Apr 22, 2024
b7e0dc5
Merge branch 'feature/fhirpath-position' into feature/fhirpath-parsing
brianpos Apr 23, 2024
49fd064
Change name from RoundtripVisitor to CanonicalVisitor
brianpos Apr 23, 2024
49d7275
Merge branch 'develop' into feature/fhirpath-parsing
ewoutkramer Apr 23, 2024
32dbabc
Rename SubTokenExpression to just SubToken
brianpos Apr 23, 2024
bc883df
Equals overloads are not accurate and not actually being used in many…
brianpos Apr 23, 2024
861a4d2
Correct the expected unit test values.
brianpos Apr 23, 2024
18a7052
Lots of code comments/documentation for the public classes/properties
brianpos Apr 29, 2024
de58b90
Missed some future parsing use properties (used in the EchoVisitor)
brianpos Apr 29, 2024
2449397
Remove the commented out Lambda Expression as it is not a part of the…
brianpos Apr 29, 2024
eafcb2d
Additional code comments
brianpos Apr 29, 2024
e5cfabe
Merge branch 'develop' into feature/fhirpath-parsing
ewoutkramer May 3, 2024
3a56bef
Change the constructors split out the default ones (which should be b…
brianpos May 10, 2024
7669a75
Merge branch 'develop' into feature/fhirpath-parsing
ewoutkramer May 14, 2024
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
21 changes: 21 additions & 0 deletions src/Hl7.Fhir.Base/FhirPath/Expressions/CanonicalVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,18 @@
using System.Text;
using System.Text.RegularExpressions;

#nullable enable

namespace Hl7.FhirPath.Expressions
{
/// <summary>
/// A simple visitor that can produce a canonical string representation of an expression
/// It will normalize any whitespace into single spaces and remove any comments
/// </summary>
/// <remarks>
/// Delimiters are removed from delimited identifiers that don't require them<br/>
/// Brackets are however retained where they are included in the original expression
/// </remarks>
public class CanonicalVisitor : ExpressionVisitor<StringBuilder>
brianpos marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly StringBuilder _result = new StringBuilder();
Expand Down Expand Up @@ -197,6 +207,17 @@ public override StringBuilder VisitCustomExpression(CustomExpression expression)

public static class CanonicalVisitorExtensions
{
/// <summary>
/// Create a canonical string representation from an expression tree,
/// Normalizing any whitespace into single spaces and remove any comments
/// </summary>
/// <remarks>
/// Delimiters are removed from delimited identifiers that don't require them
/// Brackets are however retained where they are included in the original expression<br/>
/// If you want to see the expression that was parsed, use <see cref="EchoVisitorExtensions.EchoExpression(Expression)">EchoExpression</see>
/// </remarks>
/// <param name="expr">The source Expression</param>
/// <returns>A canonical string representation of the expression</returns>
public static string ToCanonicalExpression(this Expression expr)
{
var dumper = new CanonicalVisitor();
Expand Down
301 changes: 301 additions & 0 deletions src/Hl7.Fhir.Base/FhirPath/Expressions/EchoVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,301 @@
/*
* Copyright (c) 2015, Firely (info@fire.ly) and contributors
* See the file CONTRIBUTORS for details.
*
* This file is licensed under the BSD 3-Clause license
* available at https://raw.githubusercontent.com/FirelyTeam/firely-net-sdk/master/LICENSE
*/
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using P = Hl7.Fhir.ElementModel.Types;

#nullable enable

namespace Hl7.FhirPath.Expressions
{
/// <summary>
///
/// </summary>
public class EchoVisitor : ExpressionVisitor<StringBuilder>
{
private readonly StringBuilder _result = new StringBuilder();

#region << output utilties>>
private void OutputIdentifierName(string identifier)
{
// verify that the identifier only contains valid chars in A-Za-Z0-9
if (Regex.IsMatch(identifier, "^[A-Za-z]+[A-Za-z0-9]*$", RegexOptions.Singleline))
{
_result.Append($"{identifier}");
return;
}
// delimit the output string
_result.Append($"`{identifier.Replace("`","\\`")}`");
}

private void OutputPrecedingTokens(Expression? expr)
{
if (expr?.leadingWhitespace?.Any() == true)
_result.Append(System.String.Join("", expr.leadingWhitespace.Select(ws => ws.ToString())));
}
private void OutputTrailingTokens(Expression expr)
{
if (expr.trailingWhitespace?.Any() == true)
_result.Append(System.String.Join("", expr.trailingWhitespace.Select(ws => ws.ToString())));
}

private void OutputSubToken(SubToken subtoken)
{
if (subtoken == null) return;
OutputPrecedingTokens(subtoken);
_result.Append($"{subtoken.Value}");
OutputTrailingTokens(subtoken);
}

private void OutputPrecedingTokens(SubToken subtoken)
{
if (subtoken == null) return;
if (subtoken.leadingWhitespace?.Any() == true)
_result.Append(System.String.Join("", subtoken.leadingWhitespace.Select(ws => ws.ToString())));
}
private void OutputTrailingTokens(SubToken subtoken)
{
if (subtoken == null) return;
if (subtoken.trailingWhitespace?.Any() == true)
_result.Append(System.String.Join("", subtoken.trailingWhitespace.Select(ws => ws.ToString())));
}
#endregion

public override StringBuilder VisitConstant(ConstantExpression expression)
{
OutputPrecedingTokens(expression);
if (expression is IdentifierExpression identifier)
{
OutputIdentifierName(identifier.Value);
OutputTrailingTokens(expression);
return _result;
}
var t = Fhir.Serialization.PrimitiveTypeConverter.ConvertTo<string>(expression.Value);

switch (expression.ExpressionType.Name)
{
case "Any":
_result.Append($"{t}");
break;
case "Boolean":
_result.Append($"{t}");
break;
case "Code":
_result.Append($"{t}");
break;
case "Concept":
_result.Append($"{t}");
break;
case "Date":
_result.Append($"@{t}");
break;
case "DateTime":
if (t.Contains('T'))
_result.Append($"@{t}");
else
_result.Append($"@{t}T");
break;
case "Decimal":
_result.Append($"{t}");
break;
case "Integer":
_result.Append($"{t}");
break;
case "Long":
Copy link
Member

Choose a reason for hiding this comment

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

You could combine all cases that do the same thing, to make clear they are doing the same thing!

_result.Append($"{t}");
break;
case "Quantity":
if (expression.Value is P.Quantity q)
{
_result.Append($"{q?.Value}");
OutputSubToken(expression.Unit);
}
else
{
_result.Append($"{t}");
}
break;
case "String":
_result.Append($"'{t}'");
Copy link
Collaborator Author

@brianpos brianpos May 2, 2024

Choose a reason for hiding this comment

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

@ewoutkramer ?
This line should really be handling the delimiting of the strings too.
Should the Functions.StringOperators.EscapeJson be the appropriate function to use there?

_result.Append("'" + Functions.StringOperators.EscapeJson(t) + "'"); 

Note: yes something does need to be done here, I found this round tripping all the search parameters/invariants.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if the goal of this visitor is to roundtrip to the original fhirpath as much as possible, it should escape stuff. Not easy to really roundtrip (as you cannot distinguish between allowed unicode characters and escaped unicodes anymore), but that would be good enough. So, it should "undo" the parsing of the string.

If this were C# we could leverage Roslyn:

  return SyntaxFactory.LiteralExpression(SyntaxKind.StringLiteralExpression, SyntaxFactory.Literal(input)).ToFullString();

But I don't know if that's good enough.

break;
case "Ratio":
_result.Append($"{t}");
break;
case "Time":
_result.Append($"@T{t}");
break;
case "Void":
break;
}

OutputTrailingTokens(expression);
return _result;
}

public override StringBuilder VisitFunctionCall(FunctionCallExpression expression)
{
if (expression.FunctionName == "builtin.coreexturl")
{
OutputPrecedingTokens(expression);
expression.Focus.Accept(this);
_result.Append("%`ext-");
if (expression.Arguments.FirstOrDefault() is ConstantExpression ceVar)
_result.Append($"{ceVar.Value}");
_result.Append("`");
OutputTrailingTokens(expression);
return _result;
}
if (expression.FunctionName == "builtin.corevsurl")
{
OutputPrecedingTokens(expression);
expression.Focus.Accept(this);
_result.Append("%`vs-");
if (expression.Arguments.FirstOrDefault() is ConstantExpression ceVar)
_result.Append($"{ceVar.Value}");
_result.Append("`");
OutputTrailingTokens(expression);
return _result;
}
if (expression is UnaryExpression ue)
{
OutputPrecedingTokens(expression);
ue.Focus.Accept(this);
_result.Append($"{ue.Op}");
ue.Arguments.FirstOrDefault()?.Accept(this);
OutputTrailingTokens(expression);
return _result;
}
if (expression is BinaryExpression be)
{
OutputPrecedingTokens(expression);
be.Focus.Accept(this);
be.Arguments.FirstOrDefault()?.Accept(this);
if (be.OpToken != null)
OutputSubToken(be.OpToken);
else
_result.Append($"{be.Op}");
be.Arguments.Skip(1).FirstOrDefault()?.Accept(this);
OutputTrailingTokens(expression);
return _result;
}
if (expression is IndexerExpression ie)
{
OutputPrecedingTokens(expression);
ie.Focus.Accept(this);
OutputSubToken(expression.LeftBrace);
ie.Arguments.FirstOrDefault()?.Accept(this);
OutputSubToken(expression.RightBrace);
OutputTrailingTokens(expression);
return _result;
}

OutputPrecedingTokens(expression);
expression.Focus.Accept(this);
if (!(expression.Focus is VariableRefExpression vrf && vrf.Name == "builtin.that" || expression.Focus is AxisExpression aeFocus && aeFocus.AxisName == "that"))
{
_result.Append($".");
}
if (expression is ChildExpression ce)
{
var ca = ce.Arguments.FirstOrDefault();
OutputPrecedingTokens(ca);
OutputIdentifierName(ce.ChildName);
OutputTrailingTokens(expression);
return _result;
}
OutputIdentifierName(expression.FunctionName);
OutputSubToken(expression.LeftBrace);

expression.Arguments.FirstOrDefault()?.Accept(this);
foreach (var arg in expression.Arguments.Skip(1))
{
_result.Append(",");
arg.Accept(this);
}
OutputSubToken(expression.RightBrace);
OutputTrailingTokens(expression);

return _result;
}

public override StringBuilder VisitNewNodeListInit(NewNodeListInitExpression expression)
{
OutputPrecedingTokens(expression);
OutputSubToken(expression.LeftBrace);
foreach (var element in expression.Contents)
element.Accept(this);
OutputSubToken(expression.RightBrace);
OutputTrailingTokens(expression);
return _result;
}

public override StringBuilder VisitVariableRef(VariableRefExpression expression)
{
if (expression is AxisExpression ae)
{
// No need to output the `that` type
if (ae.AxisName == "that")
return _result;

OutputPrecedingTokens(expression);
_result.Append($"${ae.AxisName}");
OutputTrailingTokens(expression);
return _result;
}
if (expression.Name != "builtin.that")
{
OutputPrecedingTokens(expression);
_result.Append("%");
OutputIdentifierName(expression.Name);
OutputTrailingTokens(expression);
}
return _result;
}

public override StringBuilder VisitCustomExpression(CustomExpression expression)
{
if (expression is BracketExpression be)
{
OutputPrecedingTokens(expression);
OutputPrecedingTokens(be.LeftBrace);
_result.Append("(");
OutputTrailingTokens(be.LeftBrace);
be.Operand.Accept(this);
OutputPrecedingTokens(be.RightBrace);
_result.Append(")");
OutputTrailingTokens(be.RightBrace);
OutputTrailingTokens(expression);
return _result;
}
base.VisitCustomExpression(expression);
return _result;
}
}

public static class EchoVisitorExtensions
{
/// <summary>
/// Create a canonical string representation from an expression tree,
/// Normalizing any whitespace into single spaces and remove any comments
/// </summary>
/// <remarks>
/// Delimiters are removed from delimited identifiers that don't require them
/// Brackets are however retained where they are included in the original expression<br/>
/// If you need a canonical representation of the expression, use <see cref="CanonicalVisitorExtensions.ToCanonicalExpression(Expression)">ToCanonicalExpression</see>
/// </remarks>
/// <param name="expr">The source Expression</param>
/// <returns>A string representation of the expression (including parsed whitespace/comments)</returns>
public static string EchoExpression(this Expression expr)
{
var dumper = new EchoVisitor();
return expr.Accept(dumper).ToString();
}
}

}
Loading
Loading