Skip to content

Commit

Permalink
PR feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
@anderson-joyle
anderson-joyle committed Nov 22, 2024
1 parent a438704 commit 4f8a99c
Show file tree
Hide file tree
Showing 12 changed files with 197 additions and 126 deletions.
4 changes: 2 additions & 2 deletions src/libraries/Microsoft.PowerFx.Core/Binding/BindInfo/CallInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal sealed class CallInfo

// ScopeIdentifier will be "" if the function does not support a scope identifier
// RequiresScopeIdentifier is true if scopeIdentifier is set and there was an As node used
public readonly DName ScopeIdentifier;
public readonly DName[] ScopeIdentifier;
public readonly bool RequiresScopeIdentifier;

public readonly object Data;
Expand Down Expand Up @@ -58,7 +58,7 @@ public CallInfo(TexlFunction function, CallNode node, object data)
Data = data;
}

public CallInfo(TexlFunction function, CallNode node, DType cursorType, DName scopeIdentifier, bool requiresScopeIdentifier, int scopeNest)
public CallInfo(TexlFunction function, CallNode node, DType cursorType, DName[] scopeIdentifier, bool requiresScopeIdentifier, int scopeNest)
{
Contracts.AssertValue(function);
Contracts.AssertValue(node);
Expand Down
55 changes: 32 additions & 23 deletions src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,6 @@ internal sealed partial class TexlBinding
/// </summary>
public const int MaxSelectsToInclude = 100;

/// <summary>
/// Default name used to access a Lambda scope.
/// </summary>
internal static DName ThisRecordDefaultName => new DName("ThisRecord");

public Features Features { get; }

// Property Name or NamedFormula Name to which current rule is being bound to. It could be null in the absence of NameResolver.
Expand Down Expand Up @@ -1348,7 +1343,7 @@ public bool TryGetFullRecordRowScopeAccessInfo(TexlNode node, out FirstNameInfo
/// <returns></returns>
private bool GetScopeIdent(TexlNode node, DType rowType, out DName scopeIdent)
{
scopeIdent = ThisRecordDefaultName;
scopeIdent = FunctionScopeInfo.ThisRecord;
if (node is AsNode asNode)
{
scopeIdent = GetInfo(asNode).AsIdentifier;
Expand Down Expand Up @@ -2426,7 +2421,7 @@ private sealed class Scope
public readonly DType Type;
public readonly bool CreatesRowScope;
public readonly bool SkipForInlineRecords;
public readonly DName ScopeIdentifier;
public readonly DName[] ScopeIdentifiers;
public readonly bool RequireScopeIdentifier;

// Optional data associated with scope. May be null.
Expand All @@ -2438,7 +2433,7 @@ public Scope(DType type)
Type = type;
}

public Scope(CallNode call, Scope parent, DType type, DName scopeIdentifier = default, bool requireScopeIdentifier = false, object data = null, bool createsRowScope = true, bool skipForInlineRecords = false)
public Scope(CallNode call, Scope parent, DType type, DName[] scopeIdentifiers = default, bool requireScopeIdentifier = false, object data = null, bool createsRowScope = true, bool skipForInlineRecords = false)
{
Contracts.Assert(type.IsValid);
Contracts.AssertValueOrNull(data);
Expand All @@ -2449,7 +2444,7 @@ public Scope(CallNode call, Scope parent, DType type, DName scopeIdentifier = de
Data = data;
CreatesRowScope = createsRowScope;
SkipForInlineRecords = skipForInlineRecords;
ScopeIdentifier = scopeIdentifier;
ScopeIdentifiers = scopeIdentifiers;
RequireScopeIdentifier = requireScopeIdentifier;

Nest = parent?.Nest ?? 0;
Expand Down Expand Up @@ -2492,7 +2487,7 @@ public Visitor(TexlBinding txb, INameResolver resolver, DType topScope, bool use
_nameResolver = resolver;
_features = features;

_topScope = new Scope(null, null, topScope ?? DType.Error, useThisRecordForRuleScope ? TexlBinding.ThisRecordDefaultName : default);
_topScope = new Scope(null, null, topScope ?? DType.Error, useThisRecordForRuleScope ? new[] { FunctionScopeInfo.ThisRecord } : default);
_currentScope = _topScope;
_currentScopeDsNodeId = -1;
}
Expand Down Expand Up @@ -2809,7 +2804,21 @@ public override void Visit(FirstNameNode node)
return;
}

var nodeType = scope.Type;
DType nodeType = null;

if (scope.ScopeIdentifiers == null || scope.ScopeIdentifiers.Length == 1)
{
nodeType = scope.Type;
}
else
{
// If scope.ScopeIdentifier.Length > 1, it meant the function creates more than 1 scope and the scope types are contained within a record.
// Example: Join(t1, t2, LeftRecord.a = RigthRecord.a, ...)
// The expression above will create LeftRecord and RigthRecord scopes. The scope type will be ![LeftRecord:![...],RigthRecord:![...]]
// Example: Join(t1 As X1, t2 As X2, X1.a = X2.a, ...)
// The expression above will create LeftRecord and RigthRecord scopes. The scope type will be ![X1:![...],X2:![...]]
Contracts.Assert(scope.Type.TryGetType(node.Ident.Name, out nodeType));
}

if (!isWholeScope)
{
Expand Down Expand Up @@ -3162,14 +3171,14 @@ private bool IsRowScopeField(FirstNameNode node, out Scope scope, out bool fErro

var expandedEntityType = GetExpandedEntityType(typeTmp, parentEntityPath);
var type = scope.Type.SetType(ref fError, DPath.Root.Append(nodeName), expandedEntityType);
scope = new Scope(scope.Call, scope.Parent, type, scope.ScopeIdentifier, scope.RequireScopeIdentifier, expandedEntityType.ExpandInfo);
scope = new Scope(scope.Call, scope.Parent, type, scope.ScopeIdentifiers, scope.RequireScopeIdentifier, expandedEntityType.ExpandInfo);
}

return true;
}
}

if (scope.ScopeIdentifier == nodeName)
if (scope.ScopeIdentifiers?.Any(dname => dname.Value == nodeName) ?? false)
{
isWholeScope = true;
return true;
Expand Down Expand Up @@ -4448,7 +4457,7 @@ public override bool PreVisit(CallNode node)
{
var scope = DType.Invalid;
var required = false;
DName scopeIdentifier = default;
DName[] scopeIdentifiers = default;
if (scopeInfo.ScopeType != null)
{
scopeNew = new Scope(node, _currentScope, scopeInfo.ScopeType, skipForInlineRecords: maybeFunc.SkipScopeForInlineRecords);
Expand All @@ -4466,19 +4475,19 @@ public override bool PreVisit(CallNode node)
scopeInfo = maybeFunc.ScopeInfo;
}

// Determine the Scope Identifier using the 1st arg
required = _txb.GetScopeIdent(nodeInp, _txb.GetType(nodeInp), out scopeIdentifier);
// Determine the Scope Identifier using the 1st arg
required = scopeInfo.GetScopeIdent(node.Args.Children.ToArray(), out scopeIdentifiers);

if (scopeInfo.CheckInput(_txb.Features, node, nodeInp, _txb.GetType(nodeInp), out scope))
{
if (_txb.TryGetEntityInfo(nodeInp, out expandInfo))
{
scopeNew = new Scope(node, _currentScope, scope, scopeIdentifier, required, expandInfo, skipForInlineRecords: maybeFunc.SkipScopeForInlineRecords);
scopeNew = new Scope(node, _currentScope, scope, scopeIdentifiers, required, expandInfo, skipForInlineRecords: maybeFunc.SkipScopeForInlineRecords);
}
else
{
maybeFunc.TryGetDelegationMetadata(node, _txb, out metadata);
scopeNew = new Scope(node, _currentScope, scope, scopeIdentifier, required, metadata, skipForInlineRecords: maybeFunc.SkipScopeForInlineRecords);
scopeNew = new Scope(node, _currentScope, scope, scopeIdentifiers, required, metadata, skipForInlineRecords: maybeFunc.SkipScopeForInlineRecords);
}
}

Expand All @@ -4488,7 +4497,7 @@ public override bool PreVisit(CallNode node)
// If there is only one function with this name and its arity doesn't match,
// that means the invocation is erroneous.
ArityError(maybeFunc.MinArity, maybeFunc.MaxArity, node, carg, _txb.ErrorContainer);
_txb.SetInfo(node, new CallInfo(maybeFunc, node, scope, scopeIdentifier, required, _currentScope.Nest));
_txb.SetInfo(node, new CallInfo(maybeFunc, node, scope, scopeIdentifiers, required, _currentScope.Nest));
_txb.SetType(node, maybeFunc.ReturnType);
}

Expand Down Expand Up @@ -4549,7 +4558,7 @@ public override bool PreVisit(CallNode node)

// Get the cursor type for this arg. Note we're not adding document errors at this point.
DType typeScope;
DName scopeIdent = default;
DName[] scopeIdent = default;
var identRequired = false;
var fArgsValid = true;

Expand All @@ -4574,10 +4583,10 @@ public override bool PreVisit(CallNode node)
typeInputs[args[i]] = _txb.GetType(args[i]);
}

fArgsValid = scopeInfo.CheckInput(_txb.Features, node, nodeInput, out typeScope, typeInputs.Values.ToArray());
fArgsValid = scopeInfo.CheckInput(_txb.Features, node, args, out typeScope, typeInputs.Values.ToArray());

// Determine the scope identifier using the first node for lambda params
identRequired = _txb.GetScopeIdent(nodeInput, typeScope, out scopeIdent);
// Determine the scope identifier using the first node for lambda params
identRequired = scopeInfo.GetScopeIdent(args, out scopeIdent);
}

if (!fArgsValid)
Expand Down
57 changes: 52 additions & 5 deletions src/libraries/Microsoft.PowerFx.Core/Functions/FunctionScopeInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ internal class FunctionScopeInfo
// True indicates that this function cannot guarantee that it will iterate over the datasource in order.
// This means it should not allow lambdas that operate on the same data multiple times, as this will
// cause nondeterministic behavior.
public bool HasNondeterministicOperationOrder => IteratesOverScope && SupportsAsyncLambdas;
public bool HasNondeterministicOperationOrder => IteratesOverScope && SupportsAsyncLambdas;

/// <summary>
/// Default name used to access a Lambda scope.
/// </summary>
public static DName ThisRecord => new DName("ThisRecord");

public FunctionScopeInfo(
TexlFunction function,
Expand Down Expand Up @@ -96,9 +101,9 @@ public FunctionScopeInfo(
/// <param name="typeScope">Calculated DType type.</param>
/// <param name="inputSchema">List of data sources to compose the calculated type.</param>
/// <returns></returns>
public virtual bool CheckInput(Features features, CallNode callNode, TexlNode inputNode, out DType typeScope, params DType[] inputSchema)
public virtual bool CheckInput(Features features, CallNode callNode, TexlNode[] inputNodes, out DType typeScope, params DType[] inputSchema)
{
return CheckInput(features, callNode, inputNode, inputSchema[0], out typeScope);
return CheckInput(features, callNode, inputNodes[0], inputSchema[0], out typeScope);
}

// Typecheck an input for this function, and get the cursor type for an invocation with that input.
Expand Down Expand Up @@ -214,6 +219,18 @@ public void CheckLiteralPredicates(TexlNode[] args, IErrorContainer errors)
}
}
}
}

public virtual bool GetScopeIdent(TexlNode[] nodes, out DName[] scopeIdents)
{
scopeIdents = new[] { ThisRecord };
if (nodes[0] is AsNode asNode)
{
scopeIdents = new[] { asNode.Right.Name };
return true;
}

return false;
}
}

Expand Down Expand Up @@ -273,7 +290,7 @@ public FunctionJoinScopeInfo(TexlFunction function)
{
}

public override bool CheckInput(Features features, CallNode callNode, TexlNode inputNode, out DType typeScope, params DType[] inputSchema)
public override bool CheckInput(Features features, CallNode callNode, TexlNode[] inputNodes, out DType typeScope, params DType[] inputSchema)
{
var ret = true;
var input0 = inputSchema[0];
Expand All @@ -284,8 +301,38 @@ public override bool CheckInput(Features features, CallNode callNode, TexlNode i
ret = base.CheckInput(features, callNode, callNode.Args.ChildNodes[0], input0, out var type0);
ret &= base.CheckInput(features, callNode, callNode.Args.ChildNodes[1], input1, out var type1);

typeScope = typeScope.Add(LeftRecord, type0).Add(RightRecord, type1);
GetScopeIdent(inputNodes, out DName[] idents);

typeScope = typeScope.Add(idents[0], type0).Add(idents[1], type1);

return ret;
}

public override bool GetScopeIdent(TexlNode[] nodes, out DName[] scopeIdents)
{
var ret = false;
scopeIdents = new DName[2];

if (nodes[0] is AsNode leftAsNode)
{
scopeIdents[0] = leftAsNode.Right.Name;
ret = true;
}
else
{
scopeIdents[0] = LeftRecord;
}

if (nodes[1] is AsNode rightAsNode)
{
scopeIdents[1] = rightAsNode.Right.Name;
ret = true;
}
else
{
scopeIdents[1] = RightRecord;
}

return ret;
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ internal abstract class TexlFunction : IFunction
public virtual bool CanSuggestInputColumns => false;

/// <summary>
/// Identifies which functions args to use to compose the scope type for subsequent lambda args.
/// Identifies which args to use to compose the scope type for subsequent lambdas.
/// Example:
/// Filter(t1, ...) => ScopeArgs is 1.
/// Join(t1, t2, ...) => ScopeArgs is 2.
/// </summary>
public virtual int ScopeArgs => 1;

Expand Down
3 changes: 2 additions & 1 deletion src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,8 +862,9 @@ internal static class TexlStrings

public static ErrorResourceKey ErrUserDefinedTypesDisabled = new ErrorResourceKey("ErrUserDefinedTypesDisabled");
public static ErrorResourceKey ErrUserDefinedTypeIncorrectSyntax = new ErrorResourceKey("ErrUserDefinedTypeIncorrectSyntax");
public static ErrorResourceKey ErrJoinCantUnion = new ErrorResourceKey("ErrJoinCantUnion");
public static ErrorResourceKey ErrJoinCantAddRename = new ErrorResourceKey("ErrJoinCantAddRename");
public static ErrorResourceKey ErrJoinNotPlainJoinTypeEnum = new ErrorResourceKey("ErrJoinNotPlainJoinTypeEnum");
public static ErrorResourceKey ErrJoinArgIsNotAsNode = new ErrorResourceKey("ErrJoinArgIsNotAsNode");
public static ErrorResourceKey ErrJoinAtLeastOneRigthRecordField = new ErrorResourceKey("ErrJoinAtLeastOneRigthRecordField");
}
}
9 changes: 5 additions & 4 deletions src/libraries/Microsoft.PowerFx.Core/Public/Config/SymbolTableOverRecordType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using Microsoft.PowerFx.Core.Binding;
using Microsoft.PowerFx.Core.Binding.BindInfo;
using Microsoft.PowerFx.Core.Functions;
using Microsoft.PowerFx.Core.Types;
using Microsoft.PowerFx.Core.Utils;
using Microsoft.PowerFx.Types;
Expand Down Expand Up @@ -37,7 +38,7 @@ public SymbolTableOverRecordType(RecordType type, ReadOnlySymbolTable parent = n

if (_allowThisRecord)
{
var data = new NameSymbol(TexlBinding.ThisRecordDefaultName, new SymbolProperties
var data = new NameSymbol(FunctionScopeInfo.ThisRecord, new SymbolProperties
{
CanMutate = false,
CanSet = false,
Expand Down Expand Up @@ -68,7 +69,7 @@ internal SymbolTableOverRecordType(RecordType type, ReadOnlySymbolTable parent =
_mutable = mutable;
_allowThisRecord = true;

var data = new NameSymbol(TexlBinding.ThisRecordDefaultName, new SymbolProperties
var data = new NameSymbol(FunctionScopeInfo.ThisRecord, new SymbolProperties
{
CanMutate = false,
CanSet = false,
Expand Down Expand Up @@ -134,7 +135,7 @@ IEnumerable<KeyValuePair<string, NameLookupInfo>> IGlobalSymbolNameResolver.Glob

if (_allowThisRecord)
{
yield return new KeyValuePair<string, NameLookupInfo>(TexlBinding.ThisRecordDefaultName, _thisRecord);
yield return new KeyValuePair<string, NameLookupInfo>(FunctionScopeInfo.ThisRecord, _thisRecord);
}
}
}
Expand All @@ -156,7 +157,7 @@ bool INameResolver.Lookup(DName name, out NameLookupInfo nameInfo, NameLookupPre

if (_allowThisRecord)
{
if (name == TexlBinding.ThisRecordDefaultName)
if (name == FunctionScopeInfo.ThisRecord)
{
nameInfo = _thisRecord;
return true;
Expand Down
Loading

0 comments on commit 4f8a99c

Please sign in to comment.