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

[DRAFT] UDT in REPL #2767

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
468b38c
Intial impl RecordOf
adithyaselv Nov 7, 2024
fa86309
Fix visitors
adithyaselv Nov 11, 2024
7a12483
Add Flag, tests
adithyaselv Nov 12, 2024
71d67e0
Change logic to use callnode
adithyaselv Nov 14, 2024
c00d011
Merge branch 'main' into adithyase/record-of-syntax
adithyaselv Nov 14, 2024
9b886bc
.
adithyaselv Nov 14, 2024
9a31411
Add a bunch of tests
adithyaselv Nov 15, 2024
835d350
add asserts
adithyaselv Nov 15, 2024
31ee2db
Add specific error for incorrect type helper usage
adithyaselv Nov 18, 2024
32053d3
Merge branch 'main' into adithyase/record-of-syntax
adithyaselv Nov 18, 2024
36bac28
tests
adithyaselv Nov 19, 2024
079cc00
RecordOf UDF test
adithyaselv Nov 19, 2024
3046a16
Restrictive aggregate types in UDF
adithyaselv Nov 22, 2024
9a6f43c
fix test
adithyaselv Nov 22, 2024
2c8dcca
Merge branch 'main' into adithyase/record-of-syntax
adithyaselv Dec 2, 2024
c68111e
UDT in REPL
adithyaselv Dec 3, 2024
f2fa6ad
fix
adithyaselv Dec 4, 2024
a4aa143
Merge branch 'main' into adithyase/udf-restrictive-types
adithyaselv Dec 11, 2024
05c446c
Merge branch 'main' into adithyase/types-repl
adithyaselv Dec 13, 2024
68e86ff
.
adithyaselv Dec 13, 2024
b386621
Merge branch 'main' into adithyase/udf-restrictive-types
adithyaselv Dec 13, 2024
34f6f50
Merge branch 'adithyase/udf-restrictive-types' into adithyase/types-repl
adithyaselv Dec 13, 2024
7f5a525
improve and fix error messages
adithyaselv Dec 13, 2024
d0da3b4
Merge branch 'adithyase/udf-restrictive-types' into adithyase/types-repl
adithyaselv Dec 13, 2024
7cbae1a
Better error reporting in REPL
adithyaselv Dec 14, 2024
2d85c9f
try add types with displaynames
adithyaselv Dec 17, 2024
422a9b9
Add Tests
adithyaselv Jan 7, 2025
da6b7f3
Merge branch 'adithyase/udf-restrictive-types' into adithyase/types-repl
adithyaselv Jan 7, 2025
9bd29bf
pass feature to udf bind
adithyaselv Jan 10, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.PowerFx.Core.App;
using Microsoft.PowerFx.Core.App.Controls;
using Microsoft.PowerFx.Core.App.ErrorContainers;
using Microsoft.PowerFx.Core.Binding;
using Microsoft.PowerFx.Core.Binding.BindInfo;
using Microsoft.PowerFx.Core.Entities;
Expand Down Expand Up @@ -57,6 +58,8 @@ public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding)

public override bool SupportsParamCoercion => true;

public override bool HasPreciseErrors => true;

private const int MaxParameterCount = 30;

public TexlNode UdfBody { get; }
Expand All @@ -77,6 +80,27 @@ public override bool TryGetDataSource(CallNode callNode, TexlBinding binding, ou

public bool HasDelegationWarning => _binding?.ErrorContainer.GetErrors().Any(error => error.MessageKey.Contains("SuggestRemoteExecutionHint")) ?? false;

public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DType[] argTypes, IErrorContainer errors, out DType returnType, out Dictionary<TexlNode, DType> nodeToCoercedTypeMap)
{
if (!base.CheckTypes(context, args, argTypes, errors, out returnType, out nodeToCoercedTypeMap))
{
return false;
}

for (int i = 0; i < argTypes.Length; i++)
{
if ((argTypes[i].IsTableNonObjNull || argTypes[i].IsRecordNonObjNull) &&
!ParamTypes[i].Accepts(argTypes[i], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules, true) &&
!argTypes[i].CoercesTo(ParamTypes[i], true, false, context.Features, true))
{
errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrBadSchema_ExpectedType, ParamTypes[i].GetKindString());
return false;
}
}

return true;
}

/// <summary>
/// Initializes a new instance of the <see cref="UserDefinedFunction"/> class.
/// </summary>
Expand Down Expand Up @@ -167,15 +191,27 @@ public void CheckTypesOnDeclaration(CheckTypesContext context, DType actualBodyR
Contracts.AssertValue(actualBodyReturnType);
Contracts.AssertValue(binding);

if (!ReturnType.Accepts(actualBodyReturnType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules))
if (!ReturnType.Accepts(
actualBodyReturnType,
exact: true,
useLegacyDateTimeAccepts: false,
usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules,
restrictiveAggregateTypes: true))
{
if (actualBodyReturnType.CoercesTo(ReturnType, true, false, context.Features))
if (actualBodyReturnType.CoercesTo(ReturnType, true, false, context.Features, restrictiveAggregateTypes: true))
{
_binding.SetCoercedType(binding.Top, ReturnType);
}
else
{
var node = UdfBody is VariadicOpNode variadicOpNode ? variadicOpNode.Children.Last() : UdfBody;

if ((ReturnType.IsTable && actualBodyReturnType.IsTable) || (ReturnType.IsRecord && actualBodyReturnType.IsRecord))
{
binding.ErrorContainer.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrUDF_ReturnTypeSchemaDoesNotMatch, ReturnType.GetKindString());
return;
}

binding.ErrorContainer.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrUDF_ReturnTypeDoesNotMatch, ReturnType.GetKindString(), actualBodyReturnType.GetKindString());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,7 @@ internal static class TexlStrings
public static ErrorResourceKey ErrUDF_DuplicateParameter = new ErrorResourceKey("ErrUDF_DuplicateParameter");
public static ErrorResourceKey ErrUDF_UnknownType = new ErrorResourceKey("ErrUDF_UnknownType");
public static ErrorResourceKey ErrUDF_ReturnTypeDoesNotMatch = new ErrorResourceKey("ErrUDF_ReturnTypeDoesNotMatch");
public static ErrorResourceKey ErrUDF_ReturnTypeSchemaDoesNotMatch = new ErrorResourceKey("ErrUDF_ReturnTypeSchemaDoesNotMatch");
public static ErrorResourceKey ErrUDF_TooManyParameters = new ErrorResourceKey("ErrUDF_TooManyParameters");
public static ErrorResourceKey ErrUDF_MissingReturnType = new ErrorResourceKey("ErrUDF_MissingReturnType");
public static ErrorResourceKey ErrUDF_MissingParamType = new ErrorResourceKey("ErrUDF_MissingParamType");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ internal ParseUserDefinitionResult ApplyParse()

public bool ContainsUDF => _parse.UDFs.Any();

public bool ContainsUDT => _parse.DefinedTypes.Any();

internal IReadOnlyDictionary<DName, FormulaType> ApplyResolveTypes()
{
if (_parse == null)
Expand Down Expand Up @@ -178,7 +180,7 @@ internal TexlFunctionSet ApplyCreateUserDefinedFunctions()
foreach (var udf in partialUDFs)
{
var config = new BindingConfig(allowsSideEffects: _parserOptions.AllowsSideEffects, useThisRecordForRuleScope: false, numberIsFloat: false, userDefinitionsMode: true);
var binding = udf.BindBody(composedSymbols, new Glue2DocumentBinderGlue(), config);
var binding = udf.BindBody(composedSymbols, new Glue2DocumentBinderGlue(), config, features: _features);

List<TexlError> bindErrors = new List<TexlError>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,5 +177,10 @@ internal override void DefaultExpressionValue(StringBuilder sb)

sb.Append("}");
}

public void SetFieldsOptional()
{
_type.AreFieldsOptional = true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public override DType Visit(TableNode node, INameResolver context)
{
Contracts.AssertValue(node);
Contracts.AssertValue(context);

if (node.ChildNodes.Count != 1)
{
return DType.Invalid;
Expand Down
62 changes: 45 additions & 17 deletions src/libraries/Microsoft.PowerFx.Core/Types/DType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1851,12 +1851,13 @@ private bool AcceptsEntityType(DType type, bool usePowerFxV1CompatibilityRules)
/// <param name="useLegacyDateTimeAccepts">Legacy rules for accepting date/time types.</param>
/// <param name="usePowerFxV1CompatibilityRules">Use PFx v1 compatibility rules if enabled (less
/// permissive Accepts relationships).</param>
/// <param name="restrictiveAggregateTypes">Flag to restrict using aggregate types with more fields than expected.</param>
/// <returns>
/// True if <see cref="DType"/> accepts <paramref name="type"/>, false otherwise.
/// </returns>
public bool Accepts(DType type, bool exact, bool useLegacyDateTimeAccepts, bool usePowerFxV1CompatibilityRules)
public bool Accepts(DType type, bool exact, bool useLegacyDateTimeAccepts, bool usePowerFxV1CompatibilityRules, bool restrictiveAggregateTypes = false)
{
return Accepts(type, out _, out _, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules);
return Accepts(type, out _, out _, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules, restrictiveAggregateTypes);
}

/// <summary>
Expand Down Expand Up @@ -1885,10 +1886,11 @@ public bool Accepts(DType type, bool exact, bool useLegacyDateTimeAccepts, bool
/// <param name="useLegacyDateTimeAccepts">Legacy rules for accepting date/time types.</param>
/// <param name="usePowerFxV1CompatibilityRules">Use PFx v1 compatibility rules if enabled (less
/// permissive Accepts relationships).</param>
/// <param name="restrictiveAggregateTypes">Flag to restrict using aggregate types with more fields than expected.</param>
/// <returns>
/// True if <see cref="DType"/> accepts <paramref name="type"/>, false otherwise.
/// </returns>
public virtual bool Accepts(DType type, out KeyValuePair<string, DType> schemaDifference, out DType schemaDifferenceType, bool exact, bool useLegacyDateTimeAccepts, bool usePowerFxV1CompatibilityRules)
public virtual bool Accepts(DType type, out KeyValuePair<string, DType> schemaDifference, out DType schemaDifferenceType, bool exact, bool useLegacyDateTimeAccepts, bool usePowerFxV1CompatibilityRules, bool restrictiveAggregateTypes = false)
{
AssertValid();
type.AssertValid();
Expand Down Expand Up @@ -1941,7 +1943,7 @@ bool DefaultReturnValue(DType targetType) =>

if (Kind == type.Kind)
{
return TreeAccepts(this, TypeTree, type.TypeTree, out schemaDifference, out schemaDifferenceType, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules);
return TreeAccepts(this, TypeTree, type.TypeTree, out schemaDifference, out schemaDifferenceType, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules, restrictiveAggregateTypes);
}

accepts = type.Kind == DKind.Unknown || type.Kind == DKind.Deferred;
Expand All @@ -1955,7 +1957,7 @@ bool DefaultReturnValue(DType targetType) =>

if (Kind == type.Kind || type.IsExpandEntity)
{
return TreeAccepts(this, TypeTree, type.TypeTree, out schemaDifference, out schemaDifferenceType, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules);
return TreeAccepts(this, TypeTree, type.TypeTree, out schemaDifference, out schemaDifferenceType, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules, restrictiveAggregateTypes);
}

accepts = (IsMultiSelectOptionSet() && TypeTree.GetPairs().First().Value.OptionSetInfo == type.OptionSetInfo) || type.Kind == DKind.Unknown || type.Kind == DKind.Deferred;
Expand Down Expand Up @@ -2175,7 +2177,7 @@ bool DefaultReturnValue(DType targetType) =>
}

// Implements Accepts for Record and Table types.
private static bool TreeAccepts(DType parentType, TypeTree treeDst, TypeTree treeSrc, out KeyValuePair<string, DType> schemaDifference, out DType treeSrcSchemaDifferenceType, bool exact = true, bool useLegacyDateTimeAccepts = false, bool usePowerFxV1CompatibilityRules = false)
private static bool TreeAccepts(DType parentType, TypeTree treeDst, TypeTree treeSrc, out KeyValuePair<string, DType> schemaDifference, out DType treeSrcSchemaDifferenceType, bool exact = true, bool useLegacyDateTimeAccepts = false, bool usePowerFxV1CompatibilityRules = false, bool restrictiveAggregateTypes = false)
{
treeDst.AssertValid();
treeSrc.AssertValid();
Expand Down Expand Up @@ -2215,7 +2217,7 @@ private static bool TreeAccepts(DType parentType, TypeTree treeDst, TypeTree tre
return false;
}

if (!pairDst.Value.Accepts(type, out var recursiveSchemaDifference, out var recursiveSchemaDifferenceType, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules))
if (!pairDst.Value.Accepts(type, out var recursiveSchemaDifference, out var recursiveSchemaDifferenceType, exact, useLegacyDateTimeAccepts, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules, restrictiveAggregateTypes))
{
if (!TryGetDisplayNameForColumn(parentType, pairDst.Key, out var colName))
{
Expand All @@ -2237,6 +2239,17 @@ private static bool TreeAccepts(DType parentType, TypeTree treeDst, TypeTree tre
}
}

if (restrictiveAggregateTypes)
{
foreach (var pairSrc in treeSrc)
{
if (!treeDst.Contains(pairSrc.Key))
{
return false;
}
}
}

return true;
}

Expand Down Expand Up @@ -3141,17 +3154,17 @@ public bool ContainsControlType(DPath path)
(n.Type.IsAggregate && n.Type.ContainsControlType(DPath.Root)));
}

public bool CoercesTo(DType typeDest, bool aggregateCoercion, bool isTopLevelCoercion, Features features)
public bool CoercesTo(DType typeDest, bool aggregateCoercion, bool isTopLevelCoercion, Features features, bool restrictiveAggregateTypes = false)
{
return CoercesTo(typeDest, out _, aggregateCoercion, isTopLevelCoercion, features);
return CoercesTo(typeDest, out _, aggregateCoercion, isTopLevelCoercion, features, restrictiveAggregateTypes);
}

public bool CoercesTo(DType typeDest, out bool isSafe, bool aggregateCoercion, bool isTopLevelCoercion, Features features)
public bool CoercesTo(DType typeDest, out bool isSafe, bool aggregateCoercion, bool isTopLevelCoercion, Features features, bool restrictiveAggregateTypes = false)
{
return CoercesTo(typeDest, out isSafe, out _, out _, out _, aggregateCoercion, isTopLevelCoercion, features);
return CoercesTo(typeDest, out isSafe, out _, out _, out _, aggregateCoercion, isTopLevelCoercion, features, restrictiveAggregateTypes);
}

public bool AggregateCoercesTo(DType typeDest, out bool isSafe, out DType coercionType, out KeyValuePair<string, DType> schemaDifference, out DType schemaDifferenceType, Features features, bool aggregateCoercion = true)
public bool AggregateCoercesTo(DType typeDest, out bool isSafe, out DType coercionType, out KeyValuePair<string, DType> schemaDifference, out DType schemaDifferenceType, Features features, bool aggregateCoercion = true, bool restrictiveAggregateTypes = false)
{
Contracts.Assert(IsAggregate);

Expand Down Expand Up @@ -3196,7 +3209,8 @@ public bool AggregateCoercesTo(DType typeDest, out bool isSafe, out DType coerci
out schemaDifferenceType,
aggregateCoercion: true,
isTopLevelCoercion: false,
features);
features,
restrictiveAggregateTypes);
}

if (Kind != typeDest.Kind)
Expand Down Expand Up @@ -3231,7 +3245,8 @@ public bool AggregateCoercesTo(DType typeDest, out bool isSafe, out DType coerci
out var fieldSchemaDifferenceType,
aggregateCoercion,
isTopLevelCoercion: false,
features);
features,
restrictiveAggregateTypes);

// This is the attempted coercion type. If we fail, we need to know this for error handling
coercionType = coercionType.Add(typedName.Name, fieldCoercionType);
Expand Down Expand Up @@ -3259,6 +3274,17 @@ public bool AggregateCoercesTo(DType typeDest, out bool isSafe, out DType coerci
isSafe &= fieldIsSafe;
}

if (restrictiveAggregateTypes)
{
foreach (var typedName in GetNames(DPath.Root))
{
if (!typeDest.TryGetType(typedName.Name, out _))
{
return false;
}
}
}

return isValid;
}

Expand All @@ -3273,7 +3299,8 @@ public virtual bool CoercesTo(
out DType schemaDifferenceType,
bool aggregateCoercion,
bool isTopLevelCoercion,
Features features)
Features features,
bool restrictiveAggregateTypes = false)
{
AssertValid();
Contracts.Assert(typeDest.IsValid);
Expand All @@ -3290,7 +3317,7 @@ public virtual bool CoercesTo(
return false;
}

if (typeDest.Accepts(this, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules))
if (typeDest.Accepts(this, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: usePowerFxV1CompatibilityRules, restrictiveAggregateTypes))
{
coercionType = typeDest;
return true;
Expand Down Expand Up @@ -3335,7 +3362,8 @@ public virtual bool CoercesTo(
out schemaDifference,
out schemaDifferenceType,
features,
aggregateCoercion);
aggregateCoercion,
restrictiveAggregateTypes);
}

var subtypeCoerces = SubtypeCoercesTo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,11 @@ public void UpdateSupportedFunctions(SymbolTable s)
SupportedFunctions = s;
}

public ReadOnlySymbolTable GetAllSymbols()
{
return SymbolTable.Compose(Config.ComposedConfigSymbols, SupportedFunctions, _symbolTable, PrimitiveTypes);
}

/// <summary>
/// Add a set of user-defined formulas and functions to the engine.
/// </summary>
Expand Down Expand Up @@ -420,7 +425,7 @@ public void AddUserDefinitions(string script, CultureInfo parseCulture = null, A
}

// Compose will handle null symbols
var composedSymbols = SymbolTable.Compose(Config.ComposedConfigSymbols, SupportedFunctions, PrimitiveTypes, _symbolTable);
var composedSymbols = SymbolTable.Compose(Config.ComposedConfigSymbols, SupportedFunctions, _symbolTable, PrimitiveTypes);

if (parseResult.DefinedTypes.Any())
{
Expand Down
24 changes: 20 additions & 4 deletions src/libraries/Microsoft.PowerFx.Repl/Repl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -508,25 +508,41 @@ await this.Output.WriteLineAsync($"Error: Can't set '{name}' to a Void value.",
definitionsCheckResult.SetText(expression, this.ParserOptions)
.ApplyParseErrors();

if (this.AllowUserDefinedFunctions && definitionsCheckResult.IsSuccess && definitionsCheckResult.ContainsUDF)
if (definitionsCheckResult.IsSuccess)
{
var defCheckResult = this.Engine.AddUserDefinedFunction(expression, this.ParserOptions.Culture, extraSymbolTable);
definitionsCheckResult.SetBindingInfo(this.Engine.GetAllSymbols());

if (!defCheckResult.IsSuccess)
if (definitionsCheckResult.ApplyErrors().Any())
{
foreach (var error in defCheckResult.Errors)
foreach (var error in definitionsCheckResult.Errors)
{
var kind = error.IsWarning ? OutputKind.Warning : OutputKind.Error;
var msg = error.ToString();

await this.Output.WriteLineAsync(lineError + msg, kind, cancel)
.ConfigureAwait(false);
}

return new ReplResult();
}

this.Engine.AddUserDefinitions(expression, this.ParserOptions.Culture);

return new ReplResult();
}

if (check.ApplyParse().Errors.Any() && definitionsCheckResult.Errors.Any())
{
foreach (var error in definitionsCheckResult.Errors)
{
var kind = error.IsWarning ? OutputKind.Warning : OutputKind.Error;
var msg = error.ToString();

await this.Output.WriteLineAsync(lineError + msg, kind, cancel)
.ConfigureAwait(false);
}
}

foreach (var error in check.Errors)
{
var kind = error.IsWarning ? OutputKind.Warning : OutputKind.Error;
Expand Down
Loading
Loading