From 468b38cc162e40ae8053a0dd20d4a55cc2d22da5 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Thu, 7 Nov 2024 13:12:17 -0800 Subject: [PATCH 01/20] Intial impl RecordOf --- .../Parser/TexlParser.cs | 34 ++++++++++++ .../Microsoft.PowerFx.Core/Syntax/NodeKind.cs | 3 +- .../Syntax/Nodes/RecordOfNode.cs | 53 +++++++++++++++++++ .../Syntax/Visitors/TexlFunctionalVisitor.cs | 8 +++ .../Syntax/Visitors/TexlVisitor.cs | 6 +++ .../Utils/LanguageConstants.cs | 5 ++ 6 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs diff --git a/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs b/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs index 9774942c84..1c1a3ec1f9 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs @@ -224,6 +224,35 @@ private TypeLiteralNode ParseTypeLiteral(Identifier typeLiteralIdentifier) return new TypeLiteralNode(ref _idNext, parenOpen, expr, new SourceList(sourceList)); } + private RecordOfNode ParseRecordOf(Identifier recordOfIdentifier) + { + var lefterTrivia = ParseTrivia(); + var parenOpen = TokEat(TokKind.ParenOpen); + var leftTrivia = ParseTrivia(); + var sourceList = new List + { + new IdentifierSource(recordOfIdentifier), + lefterTrivia, + new TokenSource(parenOpen), + leftTrivia + }; + + var identifier = ParseIdentifier(); + var fsNode = new FirstNameNode(ref _idNext, identifier.Token, identifier); + + sourceList.Add(new IdentifierSource(identifier)); + sourceList.Add(ParseTrivia()); + + var parenClose = TokEat(TokKind.ParenClose); + + if (parenClose != null) + { + sourceList.Add(new TokenSource(parenClose)); + } + + return new RecordOfNode(ref _idNext, parenOpen, fsNode, new SourceList(sourceList)); + } + private PartialAttribute MaybeParseAttribute() { if (_curs.TidCur != TokKind.BracketOpen) @@ -1295,6 +1324,11 @@ private TexlNode ParseOperand() return typeLiteralNode; } + if (ident.Token.As().Name.Value == LanguageConstants.RecordOfInvariantName && _features.IsUserDefinedTypesEnabled) + { + return ParseRecordOf(ident); + } + trivia = ParseTrivia(); return ParseInvocation(ident, trivia, null); diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs index 2aae3bbc19..d8d0d09d93 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs @@ -31,6 +31,7 @@ public enum NodeKind Error, StrInterp, - TypeLiteral + TypeLiteral, + RecordOf } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs new file mode 100644 index 0000000000..bdeb083e0f --- /dev/null +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using Microsoft.PowerFx.Core.Errors; +using Microsoft.PowerFx.Core.Localization; +using Microsoft.PowerFx.Core.Parser; +using Microsoft.PowerFx.Core.Types; +using Microsoft.PowerFx.Core.Utils; +using Microsoft.PowerFx.Syntax; +using Microsoft.PowerFx.Syntax.SourceInformation; + +namespace Microsoft.PowerFx.Syntax +{ + public sealed class RecordOfNode : TexlNode + { + internal FirstNameNode TableName { get; } + + internal RecordOfNode(ref int idNext, Token firstToken, FirstNameNode tableName, SourceList sources) + : base(ref idNext, firstToken, sources) + { + TableName = tableName; + } + + internal override TexlNode Clone(ref int idNext, Span ts) + { + var tableName = TableName.Clone(ref idNext, ts); + var newNodes = new Dictionary + { + { TableName, tableName } + }; + + return new RecordOfNode(ref idNext, Token.Clone(ts).As(), TableName, this.SourceList.Clone(ts, newNodes)); + } + + /// + public override void Accept(TexlVisitor visitor) + { + Contracts.AssertValue(visitor); + visitor.Visit(this); + } + + /// + public override TResult Accept(TexlFunctionalVisitor visitor, TContext context) + { + return visitor.Visit(this, context); + } + + /// + public override NodeKind Kind => NodeKind.RecordOf; + } +} diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs index 6f71d7d47d..9fa52ac34d 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs @@ -17,6 +17,14 @@ public abstract class TexlFunctionalVisitor /// The context passed to the node. /// The node visit result. public abstract TResult Visit(TypeLiteralNode node, TContext context); + + /// + /// Visit leaf node. + /// + /// The visited node. + /// The context passed to the node. + /// The node visit result. + public abstract TResult Visit(RecordOfNode node, TContext context); /// /// Visit leaf node. diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs index 9044462882..ddf3c5c52e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs @@ -15,6 +15,12 @@ public abstract class TexlVisitor public virtual void Visit(TypeLiteralNode node) { } + + /// + /// Visit leaf node. + /// + /// The visited node. + public abstract void Visit(RecordOfNode node); /// /// Visit leaf node. diff --git a/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs b/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs index 1e91f2feef..f3b3308f96 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs @@ -80,5 +80,10 @@ internal class LanguageConstants /// The string value representing the Type literal keyword. /// public const string TypeLiteralInvariantName = "Type"; + + /// + /// The string value representing the RecordOf keyword. + /// + public const string RecordOfInvariantName = "RecordOf"; } } From fa86309c941bb64e70f1d916d2602fab123602f3 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Mon, 11 Nov 2024 13:31:49 -0800 Subject: [PATCH 02/20] Fix visitors --- .../Microsoft.PowerFx.Core/Binding/Binder.cs | 9 +++++++ .../Microsoft.PowerFx.Core/IR/IRTranslator.cs | 8 +++++++ .../Logging/StructuralPrint.cs | 16 +++++++++++++ .../Syntax/Nodes/TypeLiteralNode.cs | 4 ++++ .../Syntax/TexlPretty.cs | 24 +++++++++++++++++++ .../Syntax/Visitors/DTypeVisitor.cs | 15 ++++++++++++ .../Syntax/Visitors/IdentityTexlVisitor.cs | 5 ++++ .../Texl/ViewFilterDataSourceVisitor.cs | 2 +- .../Texl/ViewFinderVisitor.cs | 2 +- .../EngineTests.cs | 5 ++++ 10 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs index 6c741567de..9dc034f67e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs @@ -2579,6 +2579,15 @@ public override void Visit(TypeLiteralNode node) _txb.SetType(node, DType.Error); _txb.ErrorContainer.Error(node, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()); } + } + + public override void Visit(RecordOfNode node) + { + AssertValid(); + Contracts.AssertValue(node); + + _txb.SetType(node, DType.Error); + _txb.ErrorContainer.Error(node, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()); } public override void Visit(BoolLitNode node) diff --git a/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs b/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs index f6619a7639..0097d14748 100644 --- a/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs +++ b/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs @@ -88,6 +88,14 @@ public override IntermediateNode Visit(TypeLiteralNode node, IRTranslatorContext Contracts.AssertValue(context); return new TextLiteralNode(IRContext.NotInSource(FormulaType.String), context.Binding.GetType(node).ToString()); + } + + public override IntermediateNode Visit(RecordOfNode node, IRTranslatorContext context) + { + Contracts.AssertValue(node); + Contracts.AssertValue(context); + + return new ErrorNode(context.GetIRContext(node), node.ToString()); } public override IntermediateNode Visit(NumLitNode node, IRTranslatorContext context) diff --git a/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs b/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs index a5330764cb..952f70344d 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs @@ -352,6 +352,22 @@ public override LazyList Visit(TypeLiteralNode node, Precedence context) .With(node.TypeRoot.Accept(this, Precedence.None)) .With(TexlLexer.PunctuatorParenClose); + return result; + } + + public override LazyList Visit(RecordOfNode node, Precedence context) + { + Contracts.AssertValue(node); + + var result = LazyList.Empty; + + result = result + .With( + LanguageConstants.RecordOfInvariantName, + TexlLexer.PunctuatorParenOpen) + .With(node.TableName.Accept(this, Precedence.None)) + .With(TexlLexer.PunctuatorParenClose); + return result; } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs index b3b23265cd..9844bdb090 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs @@ -89,6 +89,10 @@ public override void Visit(FirstNameNode node) { } + public override void Visit(RecordOfNode node) + { + } + public override bool PreVisit(RecordNode node) { return true; diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs index 8a186245c0..721b98d568 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs @@ -417,6 +417,23 @@ public override LazyList Visit(TypeLiteralNode node, Precedence parentPr .With(node.TypeRoot.Accept(this, Precedence.Atomic)) .With(TexlLexer.PunctuatorParenClose); + return result; + } + + public override LazyList Visit(RecordOfNode node, Precedence parentPrecedence) + { + Contracts.AssertValue(node); + + var result = LazyList.Empty; + var sb = new StringBuilder(); + + result = result + .With( + LanguageConstants.TypeLiteralInvariantName, + TexlLexer.PunctuatorParenOpen) + .With(node.TableName.Accept(this, Precedence.None)) + .With(TexlLexer.PunctuatorParenClose); + return result; } @@ -1001,6 +1018,13 @@ public override LazyList Visit(TypeLiteralNode node, Context context) { Contracts.AssertValue(node); + return Basic(node, context); + } + + public override LazyList Visit(RecordOfNode node, Context context) + { + Contracts.AssertValue(node); + return Basic(node, context); } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs index d5fd5be6b4..19a64dc4d2 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs @@ -107,5 +107,20 @@ public override DType Visit(TableNode node, INameResolver context) return rowType.ToTable(); } + + public override DType Visit(RecordOfNode node, INameResolver context) + { + Contracts.AssertValue(node); + Contracts.AssertValue(context); + + var inputType = node.TableName.Accept(this, context); + + if (!inputType.IsTable) + { + return DType.Invalid; + } + + return inputType.ToRecord(); + } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs index 3d8325c3dc..644a91062f 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs @@ -46,6 +46,11 @@ public override void Visit(FirstNameNode node) /// public override void Visit(ParentNode node) { + } + + /// + public override void Visit(RecordOfNode node) + { } /// diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs index 86a590ffc1..019ed2d398 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs @@ -13,7 +13,7 @@ namespace Microsoft.PowerFx.Core.Texl /// This visitor is used to walkthrough the first node of a filter to get the datasource name and /// whether or not there is any other filter sub expression that uses a view. /// - internal sealed class ViewFilterDataSourceVisitor : TexlVisitor + internal sealed class ViewFilterDataSourceVisitor : IdentityTexlVisitor { private const string FilterFunctionName = "Filter"; private readonly TexlBinding _txb; diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs index 774486f644..9a3c7f7429 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs @@ -12,7 +12,7 @@ namespace Microsoft.PowerFx.Core.Texl /// /// This visitor is used to walkthrough the tree to check the existence of a view. /// - internal sealed class ViewFinderVisitor : TexlVisitor + internal sealed class ViewFinderVisitor : IdentityTexlVisitor { private readonly TexlBinding _txb; diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs index d45976473d..561d439a40 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs @@ -138,6 +138,11 @@ public override void Visit(SelfNode node) { throw new NotImplementedException(); } + + public override void Visit(RecordOfNode node) + { + throw new NotImplementedException(); + } } } } From 7a12483d9565147798d1646db48e3d34afee5bed Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Tue, 12 Nov 2024 15:04:49 -0800 Subject: [PATCH 03/20] Add Flag, tests --- .../Parser/TexlParser.cs | 13 ++++++++-- .../UserDefinedTypeTests.cs | 6 +++++ .../RecalcEngineTests.cs | 26 +++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs b/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs index 1c1a3ec1f9..bd7f5c962b 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs @@ -34,6 +34,9 @@ public enum Flags // When specified, allows reserved keywords to be used as identifiers. DisableReservedKeywords = 1 << 3, + // Parse as RecordOf node + ParseAsRecordOf = 1 << 4, + // Text first. Implemented entirely in the Lexer. TextFirst = 1 << 5, @@ -210,7 +213,10 @@ private TypeLiteralNode ParseTypeLiteral(Identifier typeLiteralIdentifier) leftTrivia }; + _flagsMode.Push(_flagsMode.Peek() | Flags.ParseAsRecordOf); var expr = ParseExpr(Precedence.None); + _flagsMode.Pop(); + sourceList.Add(new NodeSource(expr)); sourceList.Add(ParseTrivia()); @@ -1312,7 +1318,8 @@ private TexlNode ParseOperand() if (AfterSpaceTokenId() == TokKind.ParenOpen) { - if (ident.Token.As().Name.Value == LanguageConstants.TypeLiteralInvariantName && _features.IsUserDefinedTypesEnabled) + if (ident.Token.As().Name.Value == LanguageConstants.TypeLiteralInvariantName + && _features.IsUserDefinedTypesEnabled) { var typeLiteralNode = ParseTypeLiteral(ident); @@ -1324,7 +1331,9 @@ private TexlNode ParseOperand() return typeLiteralNode; } - if (ident.Token.As().Name.Value == LanguageConstants.RecordOfInvariantName && _features.IsUserDefinedTypesEnabled) + if (ident.Token.As().Name.Value == LanguageConstants.RecordOfInvariantName + && _flagsMode.Peek().HasFlag(Flags.ParseAsRecordOf) + && _features.IsUserDefinedTypesEnabled) { return ParseRecordOf(ident); } diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs index 769c40e6a9..34a24d2617 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs @@ -93,6 +93,12 @@ public void TestUserDefinedType(string typeDefinition, string expectedDefinedTyp // Have named formulas and udf in the script [InlineData("NAlias := Type(Number);X := 5; ADDX(n:Number): Number = n + X; SomeType := Type(UntypedObject)", 2)] + + // Have RecordOf with/ without errors + [InlineData("Numbers := Type([Number]);T1 := Type(RecordOf([Number])); Num := Type(RecordOf(Numbers)); T2 := Type(Num);", 3)] + + // Cannot do RecordOf on a Record type + [InlineData("Point := Type({x: Number, y: Number});PointR := Type(RecordOf(Point));", 1)] public void TestValidUDTCounts(string typeDefinition, int expectedDefinedTypesCount) { var checkResult = new DefinitionsCheckResult() diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 0947791cbd..b1ac410930 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -1954,6 +1954,32 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression { Assert.False(recalcEngine.AddUserDefinedFunction(udf, CultureInfo.InvariantCulture, extraSymbols, true).IsSuccess); } + } + + [Theory] + [InlineData( + "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points)); distance(a: Point, b: Point): Number = Sqrt(Power(b.x-a.x, 2) + Power(b.y-a.y, 2));", + "distance({x: 0, y: 0}, {x: 0, y: 5})", + true, + 5.0)] + public void RecordOfTests(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) + { + var config = new PowerFxConfig(); + var recalcEngine = new RecalcEngine(config); + var parserOptions = new ParserOptions() + { + AllowsSideEffects = false, + }; + + if (isValid) + { + recalcEngine.AddUserDefinitions(userDefinitions, CultureInfo.InvariantCulture); + Assert.Equal(expectedResult, recalcEngine.Eval(evalExpression, options: parserOptions).ToObject()); + } + else + { + Assert.Throws(() => recalcEngine.AddUserDefinitions(userDefinitions, CultureInfo.InvariantCulture)); + } } #region Test From 71d67e096bc87f34d81816e8fa5ffe62afb2b437 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Thu, 14 Nov 2024 10:58:49 -0800 Subject: [PATCH 04/20] Change logic to use callnode --- .../Microsoft.PowerFx.Core/Binding/Binder.cs | 9 --- .../Microsoft.PowerFx.Core/IR/IRTranslator.cs | 8 --- .../Logging/StructuralPrint.cs | 16 ----- .../Parser/TexlParser.cs | 45 +----------- .../Microsoft.PowerFx.Core/Syntax/NodeKind.cs | 3 +- .../Syntax/Nodes/RecordOfNode.cs | 53 --------------- .../Syntax/Nodes/TypeLiteralNode.cs | 21 ++++-- .../Syntax/TexlPretty.cs | 24 ------- .../Syntax/Visitors/DTypeVisitor.cs | 17 +++-- .../Syntax/Visitors/IdentityTexlVisitor.cs | 10 +-- .../Syntax/Visitors/TexlFunctionalVisitor.cs | 8 --- .../Syntax/Visitors/TexlVisitor.cs | 10 +-- .../Texl/ViewFilterDataSourceVisitor.cs | 68 ------------------- .../Texl/ViewFinderVisitor.cs | 68 ------------------- .../EngineTests.cs | 2 +- 15 files changed, 39 insertions(+), 323 deletions(-) delete mode 100644 src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs diff --git a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs index 9dc034f67e..6c741567de 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs @@ -2579,15 +2579,6 @@ public override void Visit(TypeLiteralNode node) _txb.SetType(node, DType.Error); _txb.ErrorContainer.Error(node, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()); } - } - - public override void Visit(RecordOfNode node) - { - AssertValid(); - Contracts.AssertValue(node); - - _txb.SetType(node, DType.Error); - _txb.ErrorContainer.Error(node, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()); } public override void Visit(BoolLitNode node) diff --git a/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs b/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs index 0097d14748..f6619a7639 100644 --- a/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs +++ b/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs @@ -88,14 +88,6 @@ public override IntermediateNode Visit(TypeLiteralNode node, IRTranslatorContext Contracts.AssertValue(context); return new TextLiteralNode(IRContext.NotInSource(FormulaType.String), context.Binding.GetType(node).ToString()); - } - - public override IntermediateNode Visit(RecordOfNode node, IRTranslatorContext context) - { - Contracts.AssertValue(node); - Contracts.AssertValue(context); - - return new ErrorNode(context.GetIRContext(node), node.ToString()); } public override IntermediateNode Visit(NumLitNode node, IRTranslatorContext context) diff --git a/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs b/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs index 952f70344d..a5330764cb 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs @@ -352,22 +352,6 @@ public override LazyList Visit(TypeLiteralNode node, Precedence context) .With(node.TypeRoot.Accept(this, Precedence.None)) .With(TexlLexer.PunctuatorParenClose); - return result; - } - - public override LazyList Visit(RecordOfNode node, Precedence context) - { - Contracts.AssertValue(node); - - var result = LazyList.Empty; - - result = result - .With( - LanguageConstants.RecordOfInvariantName, - TexlLexer.PunctuatorParenOpen) - .With(node.TableName.Accept(this, Precedence.None)) - .With(TexlLexer.PunctuatorParenClose); - return result; } diff --git a/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs b/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs index bd7f5c962b..9774942c84 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs @@ -34,9 +34,6 @@ public enum Flags // When specified, allows reserved keywords to be used as identifiers. DisableReservedKeywords = 1 << 3, - // Parse as RecordOf node - ParseAsRecordOf = 1 << 4, - // Text first. Implemented entirely in the Lexer. TextFirst = 1 << 5, @@ -213,10 +210,7 @@ private TypeLiteralNode ParseTypeLiteral(Identifier typeLiteralIdentifier) leftTrivia }; - _flagsMode.Push(_flagsMode.Peek() | Flags.ParseAsRecordOf); var expr = ParseExpr(Precedence.None); - _flagsMode.Pop(); - sourceList.Add(new NodeSource(expr)); sourceList.Add(ParseTrivia()); @@ -230,35 +224,6 @@ private TypeLiteralNode ParseTypeLiteral(Identifier typeLiteralIdentifier) return new TypeLiteralNode(ref _idNext, parenOpen, expr, new SourceList(sourceList)); } - private RecordOfNode ParseRecordOf(Identifier recordOfIdentifier) - { - var lefterTrivia = ParseTrivia(); - var parenOpen = TokEat(TokKind.ParenOpen); - var leftTrivia = ParseTrivia(); - var sourceList = new List - { - new IdentifierSource(recordOfIdentifier), - lefterTrivia, - new TokenSource(parenOpen), - leftTrivia - }; - - var identifier = ParseIdentifier(); - var fsNode = new FirstNameNode(ref _idNext, identifier.Token, identifier); - - sourceList.Add(new IdentifierSource(identifier)); - sourceList.Add(ParseTrivia()); - - var parenClose = TokEat(TokKind.ParenClose); - - if (parenClose != null) - { - sourceList.Add(new TokenSource(parenClose)); - } - - return new RecordOfNode(ref _idNext, parenOpen, fsNode, new SourceList(sourceList)); - } - private PartialAttribute MaybeParseAttribute() { if (_curs.TidCur != TokKind.BracketOpen) @@ -1318,8 +1283,7 @@ private TexlNode ParseOperand() if (AfterSpaceTokenId() == TokKind.ParenOpen) { - if (ident.Token.As().Name.Value == LanguageConstants.TypeLiteralInvariantName - && _features.IsUserDefinedTypesEnabled) + if (ident.Token.As().Name.Value == LanguageConstants.TypeLiteralInvariantName && _features.IsUserDefinedTypesEnabled) { var typeLiteralNode = ParseTypeLiteral(ident); @@ -1331,13 +1295,6 @@ private TexlNode ParseOperand() return typeLiteralNode; } - if (ident.Token.As().Name.Value == LanguageConstants.RecordOfInvariantName - && _flagsMode.Peek().HasFlag(Flags.ParseAsRecordOf) - && _features.IsUserDefinedTypesEnabled) - { - return ParseRecordOf(ident); - } - trivia = ParseTrivia(); return ParseInvocation(ident, trivia, null); diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs index d8d0d09d93..2aae3bbc19 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/NodeKind.cs @@ -31,7 +31,6 @@ public enum NodeKind Error, StrInterp, - TypeLiteral, - RecordOf + TypeLiteral } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs deleted file mode 100644 index bdeb083e0f..0000000000 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/RecordOfNode.cs +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System.Collections.Generic; -using System.Linq; -using Microsoft.PowerFx.Core.Errors; -using Microsoft.PowerFx.Core.Localization; -using Microsoft.PowerFx.Core.Parser; -using Microsoft.PowerFx.Core.Types; -using Microsoft.PowerFx.Core.Utils; -using Microsoft.PowerFx.Syntax; -using Microsoft.PowerFx.Syntax.SourceInformation; - -namespace Microsoft.PowerFx.Syntax -{ - public sealed class RecordOfNode : TexlNode - { - internal FirstNameNode TableName { get; } - - internal RecordOfNode(ref int idNext, Token firstToken, FirstNameNode tableName, SourceList sources) - : base(ref idNext, firstToken, sources) - { - TableName = tableName; - } - - internal override TexlNode Clone(ref int idNext, Span ts) - { - var tableName = TableName.Clone(ref idNext, ts); - var newNodes = new Dictionary - { - { TableName, tableName } - }; - - return new RecordOfNode(ref idNext, Token.Clone(ts).As(), TableName, this.SourceList.Clone(ts, newNodes)); - } - - /// - public override void Accept(TexlVisitor visitor) - { - Contracts.AssertValue(visitor); - visitor.Visit(this); - } - - /// - public override TResult Accept(TexlFunctionalVisitor visitor, TContext context) - { - return visitor.Visit(this, context); - } - - /// - public override NodeKind Kind => NodeKind.RecordOf; - } -} diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs index 9844bdb090..62b342cb53 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs @@ -31,6 +31,7 @@ internal TypeLiteralNode(ref int idNext, Token firstToken, TexlNode type, Source : base(ref idNext, firstToken, sources) { TypeRoot = type; + TypeRoot.Parent = this; } internal override TexlNode Clone(ref int idNext, Span ts) @@ -89,10 +90,6 @@ public override void Visit(FirstNameNode node) { } - public override void Visit(RecordOfNode node) - { - } - public override bool PreVisit(RecordNode node) { return true; @@ -195,12 +192,28 @@ public override bool PreVisit(VariadicOpNode node) public override bool PreVisit(CallNode node) { + if (node.Parent is TypeLiteralNode && + node.Head.Token.Name == LanguageConstants.RecordOfInvariantName && + node.Args.Count == 1 && + node.Args.ChildNodes.Single().AsFirstName() != null) + { + return true; + } + _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString())); return false; } public override bool PreVisit(ListNode node) { + if (node.Parent is CallNode cn && + cn.Head.Token.Name == LanguageConstants.RecordOfInvariantName && + node.ChildNodes.Count == 1 && + node.ChildNodes.Single().AsFirstName() != null) + { + return true; + } + _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString())); return false; } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs index 721b98d568..8a186245c0 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs @@ -417,23 +417,6 @@ public override LazyList Visit(TypeLiteralNode node, Precedence parentPr .With(node.TypeRoot.Accept(this, Precedence.Atomic)) .With(TexlLexer.PunctuatorParenClose); - return result; - } - - public override LazyList Visit(RecordOfNode node, Precedence parentPrecedence) - { - Contracts.AssertValue(node); - - var result = LazyList.Empty; - var sb = new StringBuilder(); - - result = result - .With( - LanguageConstants.TypeLiteralInvariantName, - TexlLexer.PunctuatorParenOpen) - .With(node.TableName.Accept(this, Precedence.None)) - .With(TexlLexer.PunctuatorParenClose); - return result; } @@ -1018,13 +1001,6 @@ public override LazyList Visit(TypeLiteralNode node, Context context) { Contracts.AssertValue(node); - return Basic(node, context); - } - - public override LazyList Visit(RecordOfNode node, Context context) - { - Contracts.AssertValue(node); - return Basic(node, context); } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs index 19a64dc4d2..24192eee39 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs @@ -108,19 +108,28 @@ public override DType Visit(TableNode node, INameResolver context) return rowType.ToTable(); } - public override DType Visit(RecordOfNode node, INameResolver context) + public override DType Visit(CallNode node, INameResolver context) { Contracts.AssertValue(node); Contracts.AssertValue(context); + Contracts.AssertValue(node.Args); + Contracts.AssertAllValues(node.Args.ChildNodes); - var inputType = node.TableName.Accept(this, context); + if (node.Head.Name != LanguageConstants.RecordOfInvariantName || + node.Args.Count != 1 || + node.Args.ChildNodes.Single().AsFirstName() == null) + { + return DType.Invalid; + } + + var childType = node.Args.ChildNodes.Single().Accept(this, context); - if (!inputType.IsTable) + if (!childType.IsTable) { return DType.Invalid; } - return inputType.ToRecord(); + return childType.ToRecord(); } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs index 644a91062f..fffe53fed0 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/IdentityTexlVisitor.cs @@ -46,16 +46,16 @@ public override void Visit(FirstNameNode node) /// public override void Visit(ParentNode node) { - } - - /// - public override void Visit(RecordOfNode node) - { } /// public override void Visit(SelfNode node) { + } + + /// + public override void Visit(TypeLiteralNode node) + { } /// diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs index 9fa52ac34d..6f71d7d47d 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlFunctionalVisitor.cs @@ -17,14 +17,6 @@ public abstract class TexlFunctionalVisitor /// The context passed to the node. /// The node visit result. public abstract TResult Visit(TypeLiteralNode node, TContext context); - - /// - /// Visit leaf node. - /// - /// The visited node. - /// The context passed to the node. - /// The node visit result. - public abstract TResult Visit(RecordOfNode node, TContext context); /// /// Visit leaf node. diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs index ddf3c5c52e..10146d2105 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/TexlVisitor.cs @@ -12,15 +12,7 @@ public abstract class TexlVisitor /// Visit leaf node. /// /// The visited node. - public virtual void Visit(TypeLiteralNode node) - { - } - - /// - /// Visit leaf node. - /// - /// The visited node. - public abstract void Visit(RecordOfNode node); + public abstract void Visit(TypeLiteralNode node); /// /// Visit leaf node. diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs index 019ed2d398..330342235d 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFilterDataSourceVisitor.cs @@ -52,73 +52,5 @@ public override void PostVisit(CallNode node) } } } - - public override void PostVisit(DottedNameNode node) - { - } - - public override void PostVisit(VariadicOpNode node) - { - } - - public override void PostVisit(StrInterpNode node) - { - } - - public override void PostVisit(RecordNode node) - { - } - - public override void PostVisit(ListNode node) - { - } - - public override void PostVisit(BinaryOpNode node) - { - } - - public override void PostVisit(UnaryOpNode node) - { - } - - public override void PostVisit(TableNode node) - { - } - - public override void PostVisit(AsNode node) - { - } - - public override void Visit(ParentNode node) - { - } - - public override void Visit(NumLitNode node) - { - } - - public override void Visit(DecLitNode node) - { - } - - public override void Visit(StrLitNode node) - { - } - - public override void Visit(BoolLitNode node) - { - } - - public override void Visit(BlankNode node) - { - } - - public override void Visit(ErrorNode node) - { - } - - public override void Visit(SelfNode node) - { - } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs index 9a3c7f7429..1781236c15 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/ViewFinderVisitor.cs @@ -42,73 +42,5 @@ public override void Visit(FirstNameNode node) ContainsView = true; } } - - public override void PostVisit(CallNode node) - { - } - - public override void PostVisit(VariadicOpNode node) - { - } - - public override void PostVisit(StrInterpNode node) - { - } - - public override void PostVisit(RecordNode node) - { - } - - public override void PostVisit(ListNode node) - { - } - - public override void PostVisit(BinaryOpNode node) - { - } - - public override void PostVisit(UnaryOpNode node) - { - } - - public override void PostVisit(TableNode node) - { - } - - public override void PostVisit(AsNode node) - { - } - - public override void Visit(ParentNode node) - { - } - - public override void Visit(NumLitNode node) - { - } - - public override void Visit(DecLitNode node) - { - } - - public override void Visit(StrLitNode node) - { - } - - public override void Visit(BoolLitNode node) - { - } - - public override void Visit(BlankNode node) - { - } - - public override void Visit(ErrorNode node) - { - } - - public override void Visit(SelfNode node) - { - } } } diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs index 561d439a40..03824b6ca8 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/EngineTests.cs @@ -139,7 +139,7 @@ public override void Visit(SelfNode node) throw new NotImplementedException(); } - public override void Visit(RecordOfNode node) + public override void Visit(TypeLiteralNode node) { throw new NotImplementedException(); } From 9b886bcd7809374dd333b5a34a7dfc8aca0adcf7 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Thu, 14 Nov 2024 12:16:39 -0800 Subject: [PATCH 05/20] . --- .../Syntax/Nodes/TypeLiteralNode.cs | 20 ++++++++++--------- .../Syntax/Visitors/DTypeVisitor.cs | 4 +--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs index 62b342cb53..5f00b64370 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs @@ -42,7 +42,7 @@ internal override TexlNode Clone(ref int idNext, Span ts) { TypeRoot, typeRoot } }; - return new TypeLiteralNode(ref idNext, Token.Clone(ts).As(), TypeRoot, this.SourceList.Clone(ts, newNodes)); + return new TypeLiteralNode(ref idNext, Token.Clone(ts).As(), typeRoot, this.SourceList.Clone(ts, newNodes)); } /// @@ -192,10 +192,7 @@ public override bool PreVisit(VariadicOpNode node) public override bool PreVisit(CallNode node) { - if (node.Parent is TypeLiteralNode && - node.Head.Token.Name == LanguageConstants.RecordOfInvariantName && - node.Args.Count == 1 && - node.Args.ChildNodes.Single().AsFirstName() != null) + if (ValidRecordOfNode(node)) { return true; } @@ -206,10 +203,7 @@ public override bool PreVisit(CallNode node) public override bool PreVisit(ListNode node) { - if (node.Parent is CallNode cn && - cn.Head.Token.Name == LanguageConstants.RecordOfInvariantName && - node.ChildNodes.Count == 1 && - node.ChildNodes.Single().AsFirstName() != null) + if (node.Parent is CallNode cn && ValidRecordOfNode(cn)) { return true; } @@ -257,5 +251,13 @@ public override void PostVisit(AsNode node) { } } + + internal static bool ValidRecordOfNode(CallNode node) + { + return node.Parent is TypeLiteralNode && + node.Head.Token.Name == LanguageConstants.RecordOfInvariantName && + node.Args.Count == 1 && + node.Args.ChildNodes.Single().AsFirstName() != null; + } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs index 24192eee39..7caf7e3f8f 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs @@ -115,9 +115,7 @@ public override DType Visit(CallNode node, INameResolver context) Contracts.AssertValue(node.Args); Contracts.AssertAllValues(node.Args.ChildNodes); - if (node.Head.Name != LanguageConstants.RecordOfInvariantName || - node.Args.Count != 1 || - node.Args.ChildNodes.Single().AsFirstName() == null) + if (!TypeLiteralNode.ValidRecordOfNode(node)) { return DType.Invalid; } From 9a31411dbeed03cd7112f987ff29e00376b3896b Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Thu, 14 Nov 2024 16:00:10 -0800 Subject: [PATCH 06/20] Add a bunch of tests --- .../Syntax/Nodes/TypeLiteralNode.cs | 47 ++++++----- .../Syntax/TexlPretty.cs | 2 +- .../Syntax/Visitors/DTypeVisitor.cs | 2 + .../Texl/BuiltinFunctionsCore.cs | 13 ++- .../FormatterTests.cs | 5 ++ .../ParseTests.cs | 2 + .../RecalcEngineTests.cs | 81 +++++++++++++++++-- .../AsTypeIsTypeParseJSONTests.cs | 6 ++ 8 files changed, 125 insertions(+), 33 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs index 5f00b64370..31ad332ae0 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs @@ -114,6 +114,28 @@ public override void PostVisit(TableNode node) { } + public override bool PreVisit(CallNode node) + { + if (ValidRecordOfNode(node)) + { + return true; + } + + _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString())); + return false; + } + + public override bool PreVisit(ListNode node) + { + if (node.Parent is CallNode cn && ValidRecordOfNode(cn)) + { + return true; + } + + _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString())); + return false; + } + // Invalid nodes public override void Visit(ErrorNode node) { @@ -190,28 +212,6 @@ public override bool PreVisit(VariadicOpNode node) return false; } - public override bool PreVisit(CallNode node) - { - if (ValidRecordOfNode(node)) - { - return true; - } - - _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString())); - return false; - } - - public override bool PreVisit(ListNode node) - { - if (node.Parent is CallNode cn && ValidRecordOfNode(cn)) - { - return true; - } - - _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString())); - return false; - } - public override bool PreVisit(AsNode node) { _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString())); @@ -254,8 +254,7 @@ public override void PostVisit(AsNode node) internal static bool ValidRecordOfNode(CallNode node) { - return node.Parent is TypeLiteralNode && - node.Head.Token.Name == LanguageConstants.RecordOfInvariantName && + return node.Head.Token.Name == LanguageConstants.RecordOfInvariantName && node.Args.Count == 1 && node.Args.ChildNodes.Single().AsFirstName() != null; } diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs index 8a186245c0..176bf01773 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs @@ -414,7 +414,7 @@ public override LazyList Visit(TypeLiteralNode node, Precedence parentPr .With( LanguageConstants.TypeLiteralInvariantName, TexlLexer.PunctuatorParenOpen) - .With(node.TypeRoot.Accept(this, Precedence.Atomic)) + .With(node.TypeRoot.Accept(this, Precedence.None)) .With(TexlLexer.PunctuatorParenClose); return result; diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs index 7caf7e3f8f..477e1534e2 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs @@ -120,6 +120,8 @@ public override DType Visit(CallNode node, INameResolver context) return DType.Invalid; } + Contracts.Assert(node.Args.ChildNodes.Count == 1); + var childType = node.Args.ChildNodes.Single().Accept(this, context); if (!childType.IsTable) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs index 9679ce1de0..4460f31f36 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs @@ -4,7 +4,8 @@ using System.Collections.Generic; using System.Linq; using Microsoft.PowerFx.Core.Functions; -using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Core.Utils; namespace Microsoft.PowerFx.Core.Texl { @@ -23,6 +24,11 @@ internal class BuiltinFunctionsCore "RecordInfo", "Relate", "RemoveAll", "RemoveIf", "RequestHide", "Reset", "ResetForm", "Revert", "SaveData", "ScanBarcode", "Select", "SetFocus", "SetProperty", "ShowColumns", "State", "SubmitForm", "TraceValue", "Ungroup", "Unrelate", "Update", "UpdateContext", "UpdateIf", "User", "Validate", "ValidateRecord", "ViewForm", "Collect", "Clear", "Patch", "Remove", "ClearCollect", "Set" + }; + + internal static readonly IReadOnlyCollection TypeHelperFunctions = new HashSet() + { + LanguageConstants.RecordOfInvariantName, }; // Functions in this list are shared and may show up in other hosts by default. @@ -277,7 +283,10 @@ internal class BuiltinFunctionsCore public static bool IsKnownPublicFunction(string functionName) { - if (_library.AnyWithName(functionName) || OtherKnownFunctions.Contains(functionName) || _featureGateFunctions.AnyWithName(functionName)) + if (_library.AnyWithName(functionName) || + OtherKnownFunctions.Contains(functionName) || + _featureGateFunctions.AnyWithName(functionName) || + TypeHelperFunctions.Contains(functionName)) { return true; } diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/FormatterTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/FormatterTests.cs index 26d9e2b1a7..57cb68c8d9 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/FormatterTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/FormatterTests.cs @@ -30,6 +30,9 @@ public sealed class FormatterTests : PowerFxTest [InlineData( "ParseJSON(\"[{ \"\"Age\"\": 5}]\", Type([{Age: Number}]))", "ParseJSON(#$string$#, Type([ { #$fieldname$#:#$firstname$# } ]))")] + [InlineData( + "ParseJSON(\"{}\", Type(RecordOf(Accounts)))", + "ParseJSON(#$string$#, Type(RecordOf(#$firstname$#)))")] public void TestStucturalPrint(string script, string expected) { var result = ParseScript( @@ -262,6 +265,7 @@ public void TestMinificationWithCommentRemovalInTextFirst(string script, string [InlineData("$\"This is {{\"Another\"}} interpolated {{string}}\"", "$\"This is {{\"Another\"}} interpolated {{string}}\"")] [InlineData("ParseJSON(\"[]\", Type([{Age: Number}]))", "ParseJSON(\n \"[]\",\n Type([{Age: Number}])\n)")] [InlineData("Type([{Age: Number, Name: Text}])", "Type(\n [\n {\n Age: Number,\n Name: Text\n }\n ]\n)")] + [InlineData("Type(RecordOf(Accounts))", "Type(RecordOf(Accounts))")] public void TestPrettyPrint(string script, string expected) { // Act & Assert @@ -284,6 +288,7 @@ public void TestPrettyPrint(string script, string expected) [InlineData("T := Type(Number);", "T := Type(Number);")] [InlineData("N = 5; /*Type Dec*/ T := Type([{name: Text, age: Number}]);", "N = 5;\n/*Type Dec*/ T := Type([\n {\n name: Text,\n age: Number\n }\n]);")] [InlineData("T := Type/*com*/( /*com*/ Number /*com*/) /*com*/;", "T := Type/*com*/( /*com*/Number /*com*/)/*com*/;")] + [InlineData("T := Type(/*RecordOf*/ RecordOf(Accounts));", "T := Type(/*RecordOf*/RecordOf(Accounts));")] public void TestUserDefinitionsPrettyPrint(string script, string expected) { var parserOptions = new ParserOptions() diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ParseTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ParseTests.cs index 1695c2f576..875122f5ba 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ParseTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ParseTests.cs @@ -498,8 +498,10 @@ public void TexlParseFunctionCalls(string script) [Theory] [InlineData("Type(Decimal)", "Type(Decimal)")] [InlineData("Type([Number])", "Type([ Number ])")] + [InlineData("Type(RecordOf(Accounts))", "Type(RecordOf(Accounts))")] [InlineData("Type([{Age: Number}])", "Type([ { Age:Number } ])")] [InlineData("IsType(ParseJSON(\"42\"),Type(Decimal))", "IsType(ParseJSON(\"42\"), Type(Decimal))")] + [InlineData("IsType(ParseJSON(\"{}\"),Type(RecordOf(Accounts)))", "IsType(ParseJSON(\"{}\"), Type(RecordOf(Accounts)))")] public void TexlParseTypeLiteral(string script, string expected) { TestRoundtrip(script, expected, features: Features.PowerFxV1); diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index b1ac410930..b40636beb2 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -1882,13 +1882,11 @@ public void LookupBuiltinOptionSets() false)] // UDFs with Enitity Types should work in parameter and return types - [InlineData( "f():TestEntity = Entity; g(e: TestEntity):Number = 1;", "g(f())", true, 1.0)] - public void UserDefinedTypeTest(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) { var config = new PowerFxConfig(); @@ -1962,14 +1960,85 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression "distance({x: 0, y: 0}, {x: 0, y: 5})", true, 5.0)] + [InlineData( + "Account := Type(RecordOf(Accounts)); getAccountId(a: Account): Number = a.id;", + "getAccountId({id: 42, name: \"T-Rex\", address: \"Museum\"})", + true, + 42.0)] + [InlineData( + "Account := Type(RecordOf(Accounts)); FirstAccount(a: Accounts): Account = First(a); getAccountId(a: Account): Number = a.id;", + "getAccountId(FirstAccount([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}]))", + true, + 1729.0)] + [InlineData( + "NewAccounts := Type([RecordOf(Accounts)]); getFirstAccountId(a: NewAccounts): Number = First(a).id;", + "getFirstAccountId([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}])", + true, + 1729.0)] + [InlineData( + "AccountWithAge := Type({age: Number, acc: RecordOf(Accounts)}); getAccountAge(a: AccountWithAge): Number = a.age;", + "getAccountAge({age : 25, acc:First([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}])})", + true, + 25.0)] + [InlineData( + "Points := Type([{x : Number, y : Number}]); ComplexType := Type({p: RecordOf(Points), a: RecordOf(Accounts), s: SomeRecord}); getX(c: ComplexType): Number = c.p.x;", + "getX({s: {id: 1729, name: \"Bob\"}, p : {x: 1, y: 2}, a: {id: 42, name: \"Alice\", address: \"internet\"}})", + true, + 1.0)] + + // Fails for anyother type other than table + [InlineData( + "Account := Type(RecordOf(SomeRecord));", + "", + false)] + [InlineData( + "Account := Type(RecordOf(Void));", + "", + false)] + [InlineData( + "Account := Type(RecordOf(UntypedObject));", + "", + false)] + + // invalid helper name + [InlineData( + "Account := Type(Recordof(Accounts));", + "", + false)] + [InlineData( + "Account := Type(recordOf(Accounts));", + "", + false)] + [InlineData( + "Account := Type(First(Accounts));", + "", + false)] + + // Does not allow anything other firstname + [InlineData( + "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points As P));", + "", + false)] + [InlineData( + "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points, Accounts));", + "", + false)] + [InlineData( + "Account := Type((Accounts, SomeRecord));", + "", + false)] + [InlineData( + "Point := Type(RecordOf([{x : Number, y : Number}]));", + "", + false)] public void RecordOfTests(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) { var config = new PowerFxConfig(); var recalcEngine = new RecalcEngine(config); - var parserOptions = new ParserOptions() - { - AllowsSideEffects = false, - }; + var parserOptions = new ParserOptions(); + + recalcEngine.Config.SymbolTable.AddType(new DName("Accounts"), FormulaType.Build(TestUtils.DT("*[id: n, name:s, address:s]"))); + recalcEngine.Config.SymbolTable.AddType(new DName("SomeRecord"), FormulaType.Build(TestUtils.DT("![id: n, name:s]"))); if (isValid) { diff --git a/src/tests/Microsoft.PowerFx.Json.Tests.Shared/AsTypeIsTypeParseJSONTests.cs b/src/tests/Microsoft.PowerFx.Json.Tests.Shared/AsTypeIsTypeParseJSONTests.cs index 21d84ba836..ff8619308d 100644 --- a/src/tests/Microsoft.PowerFx.Json.Tests.Shared/AsTypeIsTypeParseJSONTests.cs +++ b/src/tests/Microsoft.PowerFx.Json.Tests.Shared/AsTypeIsTypeParseJSONTests.cs @@ -92,6 +92,7 @@ public void RecordsTest() CheckIsTypeAsTypeParseJSON(engine, "\"{\"\"a\"\": 5}\"", "Type({a: Text})", obj1, isValid: false); CheckIsTypeAsTypeParseJSON(engine, "\"{\"\"a\"\": 5, \"\"b\"\": 6}\"", "Type({a: Number})", obj1, false); CheckIsTypeAsTypeParseJSONCompileErrors(engine, "\"{\"\"a\"\": \"\"foo/bar/uri\"\"}\"", "Type({a: Void})", TexlStrings.ErrUnsupportedTypeInTypeArgument.Key); + CheckIsTypeAsTypeParseJSONCompileErrors(engine, "\"{\"\"a\"\": \"\"foo/bar/uri\"\"}\"", "Type(RecordOf(T))", TexlStrings.ErrTypeLiteral_InvalidTypeDefinition.Key); } [Fact] @@ -106,8 +107,13 @@ public void TablesTest() var t3a = new object[] { true, true, false, true }; var t3 = new object[] { t3a }; + dynamic obj1 = new ExpandoObject(); + obj1.a = 5D; + CheckIsTypeAsTypeParseJSON(engine, "\"[{\"\"a\"\": 5}]\"", "T", t1); CheckIsTypeAsTypeParseJSON(engine, "\"[{\"\"a\"\": 5}]\"", "Type([{a: Number}])", t1); + CheckIsTypeAsTypeParseJSON(engine, "\"[{\"\"a\"\": 5}]\"", "Type([RecordOf(T)])", t1); + CheckIsTypeAsTypeParseJSON(engine, "\"{\"\"a\"\": 5}\"", "Type(RecordOf(T))", obj1); CheckIsTypeAsTypeParseJSON(engine, "\"[{\"\"a\"\": [true, true, false, true]}]\"", "Type([{a: [Boolean]}])", t3); CheckIsTypeAsTypeParseJSON(engine, "\"[1, 2, 3, 4]\"", "Type([Decimal])", t2); From 835d3502c0ba0053a5c1ce4e20cf1dca21261f5c Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Thu, 14 Nov 2024 16:18:19 -0800 Subject: [PATCH 07/20] add asserts --- .../Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs index 31ad332ae0..ca94de3ee2 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs @@ -127,7 +127,7 @@ public override bool PreVisit(CallNode node) public override bool PreVisit(ListNode node) { - if (node.Parent is CallNode cn && ValidRecordOfNode(cn)) + if (node.Parent != null && node.Parent is CallNode cn && ValidRecordOfNode(cn)) { return true; } @@ -254,6 +254,10 @@ public override void PostVisit(AsNode node) internal static bool ValidRecordOfNode(CallNode node) { + Contracts.AssertValue(node); + Contracts.AssertValue(node.Args); + Contracts.AssertAllValues(node.Args.ChildNodes); + return node.Head.Token.Name == LanguageConstants.RecordOfInvariantName && node.Args.Count == 1 && node.Args.ChildNodes.Single().AsFirstName() != null; From 31ee2dbc960bf0918c6b371931c49b0da0a5af1d Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Mon, 18 Nov 2024 15:47:49 -0800 Subject: [PATCH 08/20] Add specific error for incorrect type helper usage --- src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs | 4 ++++ src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs | 1 + src/strings/PowerFxResources.en-US.resx | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs index 6c741567de..870e7e8ee1 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs @@ -4296,6 +4296,10 @@ public override bool PreVisit(CallNode node) if (BuiltinFunctionsCore.OtherKnownFunctions.Contains(node.Head.Name.Value, StringComparer.OrdinalIgnoreCase)) { _txb.ErrorContainer.Error(node, TexlStrings.ErrUnimplementedFunction, node.Head.Name.Value); + } + else if (BuiltinFunctionsCore.TypeHelperFunctions.Contains(node.Head.Name.Value, StringComparer.OrdinalIgnoreCase)) + { + _txb.ErrorContainer.Error(node, TexlStrings.ErrknownTypeHelperFunction, node.Head.Name.Value); } else { diff --git a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs index f6dc74da5c..a675153518 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs @@ -632,6 +632,7 @@ internal static class TexlStrings public static ErrorResourceKey ErrInvalidDot = new ErrorResourceKey("ErrInvalidDot"); public static ErrorResourceKey ErrUnknownFunction = new ErrorResourceKey("ErrUnknownFunction"); public static ErrorResourceKey ErrUnimplementedFunction = new ErrorResourceKey("ErrUnimplementedFunction"); + public static ErrorResourceKey ErrknownTypeHelperFunction = new ErrorResourceKey("ErrknownTypeHelperFunction"); public static ErrorResourceKey ErrUnknownNamespaceFunction = new ErrorResourceKey("ErrUnknownNamespaceFunction"); public static ErrorResourceKey ErrBadArity = new ErrorResourceKey("ErrBadArity"); public static ErrorResourceKey ErrBadArityRange = new ErrorResourceKey("ErrBadArityRange"); diff --git a/src/strings/PowerFxResources.en-US.resx b/src/strings/PowerFxResources.en-US.resx index bcc81eff3e..4c952df24a 100644 --- a/src/strings/PowerFxResources.en-US.resx +++ b/src/strings/PowerFxResources.en-US.resx @@ -1917,6 +1917,10 @@ '{0}' is an unknown or unsupported function. Error Message. + + '{0}' is recognized as type helper function and it can only be used within a type literal. + Error message shown when a type helper is used outside of type literal expression. Example of valid usage: "Type(RecordOf(Accounts))" + '{0}' is an unknown or unsupported function in namespace '{1}'. Error Message. From 36bac28519e16ae1cf318bd549c5f6381670aaee Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Tue, 19 Nov 2024 11:14:23 -0800 Subject: [PATCH 09/20] tests --- .../Public/DefinitionsCheckResult.cs | 5 +++-- .../UserDefinedTypeTests.cs | 20 +++++++++++++++++++ .../RecalcEngineTests.cs | 10 ++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs index cb83794fd4..f45456fa47 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs @@ -166,14 +166,15 @@ internal TexlFunctionSet ApplyCreateUserDefinedFunctions() { _userDefinedFunctions = new TexlFunctionSet(); - var partialUDFs = UserDefinedFunction.CreateFunctions(_parse.UDFs.Where(udf => udf.IsParseValid), _symbols, out var errors); + var composedSymbols = ReadOnlySymbolTable.Compose(_localSymbolTable, _symbols); + + var partialUDFs = UserDefinedFunction.CreateFunctions(_parse.UDFs.Where(udf => udf.IsParseValid), composedSymbols, out var errors); if (errors.Any()) { _errors.AddRange(ExpressionError.New(errors, _defaultErrorCulture)); } - var composedSymbols = ReadOnlySymbolTable.Compose(_localSymbolTable, _symbols); foreach (var udf in partialUDFs) { var config = new BindingConfig(allowsSideEffects: _parserOptions.AllowsSideEffects, useThisRecordForRuleScope: false, numberIsFloat: false, userDefinitionsMode: true); diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs index 34a24d2617..ff22c7993b 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs @@ -99,6 +99,9 @@ public void TestUserDefinedType(string typeDefinition, string expectedDefinedTyp // Cannot do RecordOf on a Record type [InlineData("Point := Type({x: Number, y: Number});PointR := Type(RecordOf(Point));", 1)] + + // Cannot use RecordOf outside of type literal + [InlineData("Point := Type({x: Number, y: Number});PointR = RecordOf(Point);", 1)] public void TestValidUDTCounts(string typeDefinition, int expectedDefinedTypesCount) { var checkResult = new DefinitionsCheckResult() @@ -132,6 +135,23 @@ public void TestUDTErrors(string typeDefinition, int expectedErrorCount, string Assert.All(errors, e => Assert.Contains(expectedMessageKey, e.MessageKey)); } + [Theory] + + // not in Type literal expression; + [InlineData("Points := Type([{ x: Number, y: Number }]); F(): Points = RecordOf(Points); ", "ErrknownTypeHelperFunction")] + [InlineData("Points := Type([{ x: Number, y: Number }]); F(): Number = RecordOf(Points).x; ", "ErrknownTypeHelperFunction")] + + // RecordOf record type + [InlineData("Point := Type({ x: Number, y: Number }); PointR := Type(RecordOf(Point)); ", "ErrNamedType_InvalidTypeDefinition")] + public void TestRecordOfErrors(string typeDefinition, string expectedMessageKey) + { + var checkResult = new DefinitionsCheckResult() + .SetText(typeDefinition) + .SetBindingInfo(_primitiveTypes); + var errors = checkResult.ApplyErrors(); + Assert.Contains(errors, e => e.MessageKey.Contains(expectedMessageKey)); + } + [Theory] [InlineData("T := Type({ x: 5+5, y: -5 });", 2)] [InlineData("T := Type(Type(Number));", 1)] diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 90e4a553fa..e3f6f32100 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -2094,6 +2094,16 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression "Point := Type(RecordOf([{x : Number, y : Number}]));", "", false)] + + // RecordOf not in type literal + [InlineData( + "Account = RecordOf(Accounts);", + "", + false)] + [InlineData( + "F():Accounts = RecordOf(Accounts);", + "", + false)] public void RecordOfTests(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) { var config = new PowerFxConfig(); From 079cc00f7578302c65379f865dbeeff16c79def7 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Tue, 19 Nov 2024 11:32:35 -0800 Subject: [PATCH 10/20] RecordOf UDF test --- .../RecalcEngineTests.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index e3f6f32100..d1e3c6a840 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -1889,7 +1889,7 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out hasNoAge(e: Employee): Number = IsBlank(getAge(e));", "hasNoAge({Name: \"Bob\", Title: \"CEO\"})", true, - 1.0)] + 1.0)] // Types with UDF restricted primitive types resolve successfully [InlineData( @@ -2049,6 +2049,13 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression true, 1.0)] + // No error on a UDF named RecordOf + [InlineData( + @"RecordOf(x:Number): Number = x + 1;", + "RecordOf(41)", + true, + 42.0)] + // Fails for anyother type other than table [InlineData( "Account := Type(RecordOf(SomeRecord));", From 3046a169c44d672f5ca33a096b07782dc2dea1b8 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Fri, 22 Nov 2024 12:54:31 -0800 Subject: [PATCH 11/20] Restrictive aggregate types in UDF --- .../Functions/TexlFunction.cs | 2 +- .../Functions/UserDefinedFunction.cs | 33 +- .../Microsoft.PowerFx.Core/Types/DType.cs | 53 +- .../RecalcEngineTests.cs | 498 +++++++++--------- 4 files changed, 322 insertions(+), 264 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs index 5271868816..53e022d83f 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs @@ -548,7 +548,7 @@ private bool CheckTypesCore(CheckTypesContext context, TexlNode[] args, DType[] } Contracts.AssertValid(argTypes[i]); - var expectedParamType = ParamTypes[i]; + var expectedParamType = ParamTypes[i]; // If the strong-enum type flag is disabled, treat an enum option set type as the enum supertype instead if (!context.Features.StronglyTypedBuiltinEnums && expectedParamType.OptionSetInfo is EnumSymbol enumSymbol) diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs index 31d436703c..69981a88b8 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs @@ -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; @@ -77,6 +78,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 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, argTypes[i].GetKindString(), ParamTypes[i].GetKindString()); + return false; + } + } + + return true; + } + /// /// Initializes a new instance of the class. /// @@ -167,15 +189,22 @@ 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, true)) { - if (actualBodyReturnType.CoercesTo(ReturnType, true, false, context.Features)) + if (actualBodyReturnType.CoercesTo(ReturnType, true, false, context.Features, 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.ErrBadSchema_ExpectedType, ReturnType.GetKindString(), actualBodyReturnType.GetKindString()); + return; + } + binding.ErrorContainer.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrUDF_ReturnTypeDoesNotMatch, ReturnType.GetKindString(), actualBodyReturnType.GetKindString()); } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs b/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs index b70fec26c5..ec31a52b8b 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs @@ -1846,12 +1846,13 @@ private bool AcceptsEntityType(DType type, bool usePowerFxV1CompatibilityRules) /// Legacy rules for accepting date/time types. /// Use PFx v1 compatibility rules if enabled (less /// permissive Accepts relationships). + /// restrictiveAggregateTypes. /// /// True if accepts , false otherwise. /// - 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); } /// @@ -1883,7 +1884,7 @@ public bool Accepts(DType type, bool exact, bool useLegacyDateTimeAccepts, bool /// /// True if accepts , false otherwise. /// - public virtual bool Accepts(DType type, out KeyValuePair schemaDifference, out DType schemaDifferenceType, bool exact, bool useLegacyDateTimeAccepts, bool usePowerFxV1CompatibilityRules) + public virtual bool Accepts(DType type, out KeyValuePair schemaDifference, out DType schemaDifferenceType, bool exact, bool useLegacyDateTimeAccepts, bool usePowerFxV1CompatibilityRules, bool restrictiveAggregateTypes = false) { AssertValid(); type.AssertValid(); @@ -1936,7 +1937,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; @@ -1950,7 +1951,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; @@ -2170,7 +2171,7 @@ bool DefaultReturnValue(DType targetType) => } // Implements Accepts for Record and Table types. - private static bool TreeAccepts(DType parentType, TypeTree treeDst, TypeTree treeSrc, out KeyValuePair 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 schemaDifference, out DType treeSrcSchemaDifferenceType, bool exact = true, bool useLegacyDateTimeAccepts = false, bool usePowerFxV1CompatibilityRules = false, bool restrictiveAggregateTypes = false) { treeDst.AssertValid(); treeSrc.AssertValid(); @@ -2210,7 +2211,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)) { @@ -2232,6 +2233,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; } @@ -3136,17 +3148,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 schemaDifference, out DType schemaDifferenceType, Features features, bool aggregateCoercion = true) + public bool AggregateCoercesTo(DType typeDest, out bool isSafe, out DType coercionType, out KeyValuePair schemaDifference, out DType schemaDifferenceType, Features features, bool aggregateCoercion = true, bool restrictiveAggregateTypes = false) { Contracts.Assert(IsAggregate); @@ -3254,6 +3266,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; } @@ -3268,7 +3291,8 @@ public virtual bool CoercesTo( out DType schemaDifferenceType, bool aggregateCoercion, bool isTopLevelCoercion, - Features features) + Features features, + bool restrictiveAggregateTypes = false) { AssertValid(); Contracts.Assert(typeDest.IsValid); @@ -3330,7 +3354,8 @@ public virtual bool CoercesTo( out schemaDifference, out schemaDifferenceType, features, - aggregateCoercion); + aggregateCoercion, + restrictiveAggregateTypes); } var subtypeCoerces = SubtypeCoercesTo( diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 2f117e3273..05ef9e8597 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -10,7 +10,7 @@ using System.Threading.Tasks; using Microsoft.PowerFx.Core; using Microsoft.PowerFx.Core.Functions; -using Microsoft.PowerFx.Core.Functions.Delegation; +using Microsoft.PowerFx.Core.Functions.Delegation; using Microsoft.PowerFx.Core.Functions.Delegation.DelegationMetadata; using Microsoft.PowerFx.Core.IR; using Microsoft.PowerFx.Core.Localization; @@ -398,8 +398,8 @@ public void UserDefinitionOnUpdateTest() // After (A,B) = 3,6 var result = engine.Eval("With({x:A, y:B}, Set(A,3); x & y & A & B)", options: _opts); Assert.Equal("102036", result.ToObject()); - } - + } + [Fact] public void BasicEval() { @@ -412,20 +412,20 @@ public void BasicEval() [Fact] public void BuiltInEnumConfigCheck() - { - var config = new PowerFxConfig() - { - SymbolTable = null - }; - -#pragma warning disable CS0618 // Type or member is obsolete - config.EnableRegExFunctions(); -#pragma warning restore CS0618 // Type or member is obsolete - var expression = "Match(\"test\", \"t\", MatchOptions.Contains)"; - - var engine = new RecalcEngine(config); - var check = engine.Check(expression); - Assert.True(check.IsSuccess); + { + var config = new PowerFxConfig() + { + SymbolTable = null + }; + +#pragma warning disable CS0618 // Type or member is obsolete + config.EnableRegExFunctions(); +#pragma warning restore CS0618 // Type or member is obsolete + var expression = "Match(\"test\", \"t\", MatchOptions.Contains)"; + + var engine = new RecalcEngine(config); + var check = engine.Check(expression); + Assert.True(check.IsSuccess); } [Fact] @@ -486,7 +486,7 @@ public void FormulaCantRedefine() false, 14.0)] - // Recursive calls are not allowed + // Recursive calls are not allowed [InlineData( "foo(x:Number):Number = If(x=0,foo(1),If(x=1,foo(2),If(x=2,Float(2))));", "foo(Float(0))", @@ -614,78 +614,78 @@ public void FunctionPrecedenceTest(string script, double expected) var result = check.GetEvaluator().Eval(); Assert.Equal(expected, result.AsDouble()); - } - + } + [Fact] public void ShadowingFunctionPrecedenceTest() - { + { var engine = new RecalcEngine(); - engine.AddUserDefinedFunction("Concat(x:Text):Text = \"xyz\"; Average(x:Number):Number = 11111;"); + engine.AddUserDefinedFunction("Concat(x:Text):Text = \"xyz\"; Average(x:Number):Number = 11111;"); var check = engine.Check("Concat(\"abc\")"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.String, check.ReturnType); - var result = check.GetEvaluator().Eval(); - Assert.Equal("xyz", ((StringValue)result).Value); - + var result = check.GetEvaluator().Eval(); + Assert.Equal("xyz", ((StringValue)result).Value); + check = engine.Check("Average(123)"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Number, check.ReturnType); - result = check.GetEvaluator().Eval(); - Assert.Equal(11111, result.AsDouble()); - - engine.AddUserDefinitions("Test := Type({A: Number}); TestTable := Type([{A: Number}]);" + - "Filter(X: TestTable):Test = First(X); ShowColumns(X: TestTable):TestTable = FirstN(X, 3);"); - + result = check.GetEvaluator().Eval(); + Assert.Equal(11111, result.AsDouble()); + + engine.AddUserDefinitions("Test := Type({A: Number}); TestTable := Type([{A: Number}]);" + + "Filter(X: TestTable):Test = First(X); ShowColumns(X: TestTable):TestTable = FirstN(X, 3);"); + check = engine.Check("Filter([{A: 123}]).A"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Number, check.ReturnType); - result = check.GetEvaluator().Eval(); - Assert.Equal(123, result.AsDouble()); - + result = check.GetEvaluator().Eval(); + Assert.Equal(123, result.AsDouble()); + check = engine.Check("CountRows(ShowColumns([{A: 123}, {A: 124}, {A:125}, {A:126}, {A: 127}]))"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Decimal, check.ReturnType); - result = check.GetEvaluator().Eval(); + result = check.GetEvaluator().Eval(); Assert.Equal(3, result.AsDouble()); } - - [Theory] - - // Behavior function in non-imperative udf + + [Theory] + + // Behavior function in non-imperative udf [InlineData( "TestFunc():Void = Set(a, 123);", true, - "Behavior function in a non-behavior user-defined function", - false)] - - // Behavior function in imperative udf + "Behavior function in a non-behavior user-defined function", + false)] + + // Behavior function in imperative udf [InlineData( "TestFunc():Void = { Set(a, 123); };", false, - null, - true)] + null, + true)] public void BehaviorFunctionInImperativeUDF(string udfExpression, bool expectedError, string expectedErrorKey, bool allowSideEffects) - { - var config = new PowerFxConfig(); - config.EnableSetFunction(); - var engine = new RecalcEngine(config); - engine.UpdateVariable("a", 1m); - - var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); - Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); - - if (expectedError) - { - result.Errors.Any(error => error.MessageKey == expectedErrorKey); + { + var config = new PowerFxConfig(); + config.EnableSetFunction(); + var engine = new RecalcEngine(config); + engine.UpdateVariable("a", 1m); + + var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); + Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); + + if (expectedError) + { + result.Errors.Any(error => error.MessageKey == expectedErrorKey); } - } - + } + [Theory] // Return value with side effectful UDF @@ -710,12 +710,12 @@ public void ImperativeUserDefinedFunctionTest(string udfExpression, string expre config.EnableSetFunction(); var recalcEngine = new RecalcEngine(config); recalcEngine.UpdateVariable("a", 1m); - - var definitionsCheckResult = recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - if (!expectedError) - { - Assert.True(definitionsCheckResult.IsSuccess); + var definitionsCheckResult = recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); + + if (!expectedError) + { + Assert.True(definitionsCheckResult.IsSuccess); var result = recalcEngine.Eval(expression, options: _opts); var fvExpected = FormulaValue.New(expected); @@ -724,136 +724,136 @@ public void ImperativeUserDefinedFunctionTest(string udfExpression, string expre } else { - Assert.False(definitionsCheckResult.IsSuccess); + Assert.False(definitionsCheckResult.IsSuccess); Assert.Single(definitionsCheckResult.Errors); Assert.Contains(definitionsCheckResult.Errors, err => err.MessageKey == errorKey); } - } - - [Theory] - - [InlineData( + } + + [Theory] + + [InlineData( "MismatchType():Number = { 1+3; Color.Blue; };", true, - true, - 36, - 41)] - [InlineData( + true, + 36, + 41)] + [InlineData( "MatchType():Text = { 4; 3 };", false, - true, - 0, + true, + 0, 0)] public void TestMismatchReturnType(string udfExpression, bool expectedError, bool allowSideEffects, int min, int lim) { var config = new PowerFxConfig(); config.EnableSetFunction(); var engine = new RecalcEngine(config); - engine.UpdateVariable("x", 1m); - - var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); - Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); - - if (expectedError) - { - var error = result.Errors.First(error => error.MessageKey == "ErrUDF_ReturnTypeDoesNotMatch"); - Assert.NotNull(error); - var span = error.Span; - Assert.True(span.Min == min && span.Lim == lim); + engine.UpdateVariable("x", 1m); + + var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); + Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); + + if (expectedError) + { + var error = result.Errors.First(error => error.MessageKey == "ErrUDF_ReturnTypeDoesNotMatch"); + Assert.NotNull(error); + var span = error.Span; + Assert.True(span.Min == min && span.Lim == lim); } - } - + } + [Fact] public void DelegableUDFTest() - { - var symbolTable = new DelegatableSymbolTable(); - var schema = DType.CreateTable( - new TypedName(DType.Guid, new DName("ID")), + { + var symbolTable = new DelegatableSymbolTable(); + var schema = DType.CreateTable( + new TypedName(DType.Guid, new DName("ID")), new TypedName(DType.Number, new DName("Value"))); - symbolTable.AddEntity(new TestDelegableDataSource( - "MyDataSource", - schema, - new TestDelegationMetadata( - new DelegationCapability(DelegationCapability.Filter), - schema, - new FilterOpMetadata( - schema, - new Dictionary(), - new Dictionary(), - new DelegationCapability(DelegationCapability.GreaterThan), - null)), - true)); + symbolTable.AddEntity(new TestDelegableDataSource( + "MyDataSource", + schema, + new TestDelegationMetadata( + new DelegationCapability(DelegationCapability.Filter), + schema, + new FilterOpMetadata( + schema, + new Dictionary(), + new Dictionary(), + new DelegationCapability(DelegationCapability.GreaterThan), + null)), + true)); symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema)); - var config = new PowerFxConfig() - { + var config = new PowerFxConfig() + { SymbolTable = symbolTable }; - config.EnableSetFunction(); - - var recalcEngine = new RecalcEngine(config); - - recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("C").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("B").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("D").First() as UserDefinedFunction; - - // Imperative function is not delegable - Assert.True(func.IsAsync); - Assert.True(!func.IsDelegatable); - - // Binding fails for recursive definitions and hence function is not added. - Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess); - } - - [Fact] + config.EnableSetFunction(); + + var recalcEngine = new RecalcEngine(config); + + recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); + var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("C").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("B").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("D").First() as UserDefinedFunction; + + // Imperative function is not delegable + Assert.True(func.IsAsync); + Assert.True(!func.IsDelegatable); + + // Binding fails for recursive definitions and hence function is not added. + Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess); + } + + [Fact] public void TestInheritanceOfDelegationWarningsInUDFs() - { - var symbolTable = new DelegatableSymbolTable(); - var schema = DType.CreateTable( + { + var symbolTable = new DelegatableSymbolTable(); + var schema = DType.CreateTable( new TypedName(DType.Number, new DName("Value"))); - symbolTable.AddEntity(new TestDelegableDataSource( - "MyDataSource", - schema, - new TestDelegationMetadata( - new DelegationCapability(DelegationCapability.Filter), - schema, - new FilterOpMetadata( - schema, - new Dictionary(), - new Dictionary(), - new DelegationCapability(DelegationCapability.GreaterThan), - null)), - true)); + symbolTable.AddEntity(new TestDelegableDataSource( + "MyDataSource", + schema, + new TestDelegationMetadata( + new DelegationCapability(DelegationCapability.Filter), + schema, + new FilterOpMetadata( + schema, + new Dictionary(), + new Dictionary(), + new DelegationCapability(DelegationCapability.GreaterThan), + null)), + true)); symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema)); - var config = new PowerFxConfig() - { + var config = new PowerFxConfig() + { SymbolTable = symbolTable }; - var engine = new RecalcEngine(config); + var engine = new RecalcEngine(config); + + var result = engine.AddUserDefinedFunction("NonDelegatableUDF():MyDataSourceTableType = Filter(MyDataSource, Sqrt(Value) > 5);", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + var func = engine.Functions.WithName("NonDelegatableUDF").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); - var result = engine.AddUserDefinedFunction("NonDelegatableUDF():MyDataSourceTableType = Filter(MyDataSource, Sqrt(Value) > 5);", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); - Assert.True(result.IsSuccess); - var func = engine.Functions.WithName("NonDelegatableUDF").First() as UserDefinedFunction; - Assert.True(func.HasDelegationWarning); - - result = engine.AddUserDefinedFunction("NonDelegatableUDF2():MyDataSourceTableType = NonDelegatableUDF();", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); - Assert.True(result.IsSuccess); - func = engine.Functions.WithName("NonDelegatableUDF2").First() as UserDefinedFunction; - Assert.True(func.HasDelegationWarning); + result = engine.AddUserDefinedFunction("NonDelegatableUDF2():MyDataSourceTableType = NonDelegatableUDF();", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + func = engine.Functions.WithName("NonDelegatableUDF2").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); } // Binding to inner functions does not impact outer functions. @@ -1768,68 +1768,68 @@ public void LookupBuiltinOptionSets() Assert.True(ok); // Wrong type: https://github.com/microsoft/Power-Fx/issues/2342 - } - - [Fact] - public void LazyJsonTest() - { - bool LazyGetField1(string name, out FormulaType type) - { - type = name switch - { - "field1" => FormulaType.Decimal, - "field2" => FormulaType.String, - _ => FormulaType.Blank, - }; - - return type != FormulaType.Blank; - } - - PowerFxConfig config = new PowerFxConfig(); - config.EnableJsonFunctions(); - - RecalcEngine engine = new RecalcEngine(config); - SymbolTable symbolTable = new SymbolTable(); - TestLazyRecordType lazyRecordType = new TestLazyRecordType("test", new List() { "field1", "field2" }, LazyGetField1); - - ISymbolSlot slot = symbolTable.AddVariable("var1", lazyRecordType); - - CheckResult checkResult = engine.Check("JSON(var1)", symbolTable: symbolTable); - Assert.True(checkResult.IsSuccess, string.Join(", ", checkResult.Errors.Select(err => err.Message))); - - SymbolValues symbolValues = new SymbolValues(symbolTable); - symbolValues.Set(slot, new LazyRecordValue(lazyRecordType)); - - FormulaValue formulaValue = checkResult.GetEvaluator().Eval(symbolValues); - StringValue stringValue = Assert.IsType(formulaValue); - - Assert.Equal(@"{""field1"":10,""field2"":""Str""}", stringValue.Value); - } - - public class LazyRecordValue : RecordValue - { - public LazyRecordValue(RecordType type) - : base(type) - { - } - - protected override bool TryGetField(FormulaType fieldType, string fieldName, out FormulaValue result) - { - if (fieldName == "field1") - { - result = FormulaValue.New(10); - return true; - } - - if (fieldName == "field2") - { - result = FormulaValue.New("Str"); - return true; - } - - result = null; - return false; - } + } + + [Fact] + public void LazyJsonTest() + { + bool LazyGetField1(string name, out FormulaType type) + { + type = name switch + { + "field1" => FormulaType.Decimal, + "field2" => FormulaType.String, + _ => FormulaType.Blank, + }; + + return type != FormulaType.Blank; + } + + PowerFxConfig config = new PowerFxConfig(); + config.EnableJsonFunctions(); + + RecalcEngine engine = new RecalcEngine(config); + SymbolTable symbolTable = new SymbolTable(); + TestLazyRecordType lazyRecordType = new TestLazyRecordType("test", new List() { "field1", "field2" }, LazyGetField1); + + ISymbolSlot slot = symbolTable.AddVariable("var1", lazyRecordType); + + CheckResult checkResult = engine.Check("JSON(var1)", symbolTable: symbolTable); + Assert.True(checkResult.IsSuccess, string.Join(", ", checkResult.Errors.Select(err => err.Message))); + + SymbolValues symbolValues = new SymbolValues(symbolTable); + symbolValues.Set(slot, new LazyRecordValue(lazyRecordType)); + + FormulaValue formulaValue = checkResult.GetEvaluator().Eval(symbolValues); + StringValue stringValue = Assert.IsType(formulaValue); + + Assert.Equal(@"{""field1"":10,""field2"":""Str""}", stringValue.Value); + } + + public class LazyRecordValue : RecordValue + { + public LazyRecordValue(RecordType type) + : base(type) + { + } + + protected override bool TryGetField(FormulaType fieldType, string fieldName, out FormulaValue result) + { + if (fieldName == "field1") + { + result = FormulaValue.New(10); + return true; + } + + if (fieldName == "field2") + { + result = FormulaValue.New("Str"); + return true; + } + + result = null; + return false; + } } [Theory] @@ -1865,12 +1865,7 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out true, 42.0)] - // Functions accept record with more/less fields - [InlineData( - "People := Type([{Name: Text, Age: Number}]); countMinors(p: People): Number = CountRows(Filter(p, Age < 18));", - "countMinors([{Name: \"Bob\", Age: 21, Title: \"Engineer\"}, {Name: \"Alice\", Age: 25, Title: \"Manager\"}])", - true, - 0.0)] + // Functions accept record with less fields [InlineData( "Employee := Type({Name: Text, Age: Number, Title: Text}); getAge(e: Employee): Number = e.Age;", "getAge({Name: \"Bob\", Age: 21})", @@ -1952,13 +1947,22 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out true, 1.0)] + // Aggregate types with more than expected fields are not allowed in UDF + [InlineData( + "f():T = {x: 5, y: 5}; T := Type({x: Number});", + "f().x", + false)] + [InlineData( + "People := Type([{Name: Text, Age: Number}]); countMinors(p: People): Number = CountRows(Filter(p, Age < 18));", + "countMinors([{Name: \"Bob\", Age: 21, Title: \"Engineer\"}, {Name: \"Alice\", Age: 25, Title: \"Manager\"}])", + false)] public void UserDefinedTypeTest(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) { var config = new PowerFxConfig(); var recalcEngine = new RecalcEngine(config); var parserOptions = new ParserOptions() { - AllowsSideEffects = false, + AllowsSideEffects = false, }; if (isValid) @@ -1974,24 +1978,24 @@ public void UserDefinedTypeTest(string userDefinitions, string evalExpression, b { Assert.Throws(() => recalcEngine.AddUserDefinitions(userDefinitions, CultureInfo.InvariantCulture)); } - } - - [Theory] + } + + [Theory] [InlineData( "F():Point = {x: 5, y:5};", "F().x", - true, + true, 5.0)] [InlineData( "F():Number = { Sqrt(1); 42; };", "F()", - true, - 42.0)] + true, + 42.0)] [InlineData( "F():Point = { {x:0, y:1729}; };", "F().y", - true, - 1729.0)] + true, + 1729.0)] [InlineData( "F():Point = {x: 5, 42};", "F().x", @@ -2003,9 +2007,9 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression var parserOptions = new ParserOptions() { AllowsSideEffects = true, - }; - - var extraSymbols = new SymbolTable(); + }; + + var extraSymbols = new SymbolTable(); extraSymbols.AddType(new DName("Point"), FormulaType.Build(TestUtils.DT("![x:n, y:n]"))); if (isValid) From 9a6f43c040e75676f863b9117d58d9b323c29d0d Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Fri, 22 Nov 2024 15:22:53 -0800 Subject: [PATCH 12/20] fix test --- .../RecalcEngineTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 05ef9e8597..ec2fff95ac 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -1976,7 +1976,11 @@ public void UserDefinedTypeTest(string userDefinitions, string evalExpression, b } else { - Assert.Throws(() => recalcEngine.AddUserDefinitions(userDefinitions, CultureInfo.InvariantCulture)); + Assert.ThrowsAny(() => + { + recalcEngine.AddUserDefinitions(userDefinitions, CultureInfo.InvariantCulture); + recalcEngine.Eval(evalExpression, options: parserOptions); + }); } } From c68111e8458f2ed14563e75ed61fb178a320c0f3 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Tue, 3 Dec 2024 15:51:54 -0800 Subject: [PATCH 13/20] UDT in REPL --- .../Public/DefinitionsCheckResult.cs | 2 ++ src/libraries/Microsoft.PowerFx.Repl/Repl.cs | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs index f45456fa47..88c02a23b0 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs @@ -116,6 +116,8 @@ internal ParseUserDefinitionResult ApplyParse() public bool ContainsUDF => _parse.UDFs.Any(); + public bool ContainsUDT => _parse.DefinedTypes.Any(); + internal IReadOnlyDictionary ApplyResolveTypes() { if (_parse == null) diff --git a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs index 1a04efc2ee..b6a135d739 100644 --- a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs +++ b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs @@ -527,6 +527,30 @@ await this.Output.WriteLineAsync(lineError + msg, kind, cancel) return new ReplResult(); } + if (definitionsCheckResult.IsSuccess && definitionsCheckResult.ContainsUDT) + { + try + { + this.Engine.AddUserDefinitions(expression, this.ParserOptions.Culture); + } + catch (Exception ex) + { + await this.Output.WriteLineAsync(lineError + ex.Message, OutputKind.Error, cancel) + .ConfigureAwait(false); + } + + return new ReplResult(); + } + + 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; From f2fa6adf4b6e1aa5c29659565f626f1b3de1cc82 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Tue, 3 Dec 2024 17:17:07 -0800 Subject: [PATCH 14/20] fix --- src/libraries/Microsoft.PowerFx.Repl/Repl.cs | 21 +------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs index b6a135d739..19d1e81411 100644 --- a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs +++ b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs @@ -508,26 +508,7 @@ 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) - { - var defCheckResult = this.Engine.AddUserDefinedFunction(expression, this.ParserOptions.Culture, extraSymbolTable); - - if (!defCheckResult.IsSuccess) - { - foreach (var error in defCheckResult.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(); - } - - if (definitionsCheckResult.IsSuccess && definitionsCheckResult.ContainsUDT) + if (definitionsCheckResult.IsSuccess) { try { From 68e86ff458c2859702686c601fd71caa019dc2a7 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Fri, 13 Dec 2024 11:12:36 -0800 Subject: [PATCH 15/20] . --- .../Microsoft.PowerFx.Core/Binding/Binder.cs | 510 ++++++++-------- .../RecalcEngineTests.cs | 576 +++++++++--------- 2 files changed, 543 insertions(+), 543 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs index 70fd5fb2b1..9f6cbd876a 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs @@ -132,8 +132,8 @@ internal sealed partial class TexlBinding /// /// The maximum number of selects in a table that will be included in data call. /// - public const int MaxSelectsToInclude = 100; - + public const int MaxSelectsToInclude = 100; + /// /// Default name used to access a Lambda scope. /// @@ -350,7 +350,7 @@ private TexlBinding( entityName: EntityName, propertyName: Property?.InvariantName ?? string.Empty, allowsSideEffects: bindingConfig.AllowsSideEffects, - numberIsFloat: bindingConfig.NumberIsFloat, + numberIsFloat: bindingConfig.NumberIsFloat, analysisMode: bindingConfig.AnalysisMode); } @@ -760,7 +760,7 @@ private bool TryGetEntityInfo(CallNode node, out IExpandInfo info) // It is possible for function to be null here if it referred to // a service function from a service we are in the process of - // deregistering. + // deregistering. // GetInfo on a callNode may return null hence need null conditional operator and it short circuits if null. return GetInfo(callNode)?.VerifyValue().Function?.TryGetEntityInfo(node, this, out info) ?? false; } @@ -2556,43 +2556,43 @@ public override void Visit(BlankNode node) _txb.SetSelfContainedConstant(node, true); _txb.SetType(node, DType.ObjNull); } - - // Binding TypeLiteralNode from anywhere other than valid type context should be an error. - // This ensures that binding of unintended use of TypeLiteralNode eg: "If(Type(Boolean), 1, 2)" will result in an error. + + // Binding TypeLiteralNode from anywhere other than valid type context should be an error. + // This ensures that binding of unintended use of TypeLiteralNode eg: "If(Type(Boolean), 1, 2)" will result in an error. // VisitType method is used to resolve the type of TypeLiteralNode from valid context. public override void Visit(TypeLiteralNode node) { AssertValid(); - Contracts.AssertValue(node); - - _txb.SetType(node, DType.Error); + Contracts.AssertValue(node); + + _txb.SetType(node, DType.Error); _txb.ErrorContainer.Error(node, TexlStrings.ErrTypeLiteral_UnsupportedUsage); - } - - // Method to bind TypeLiteralNode from valid context where a type is expected. - // Binding TypeLiteralNode in an expression where a type is not expected invokes the Visit method - // from normal visitor pattern and results in error. + } + + // Method to bind TypeLiteralNode from valid context where a type is expected. + // Binding TypeLiteralNode in an expression where a type is not expected invokes the Visit method + // from normal visitor pattern and results in error. private void VisitType(TypeLiteralNode node) { AssertValid(); - Contracts.AssertValue(node); - - if (_nameResolver == null) - { - _txb.SetType(node, DType.Unknown); - return; - } - - var type = DTypeVisitor.Run(node.TypeRoot, _nameResolver); - - if (type.IsValid) - { - _txb.SetType(node, type); - } - else - { - _txb.SetType(node, DType.Error); - _txb.ErrorContainer.Error(node, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()); + Contracts.AssertValue(node); + + if (_nameResolver == null) + { + _txb.SetType(node, DType.Unknown); + return; + } + + var type = DTypeVisitor.Run(node.TypeRoot, _nameResolver); + + if (type.IsValid) + { + _txb.SetType(node, type); + } + else + { + _txb.SetType(node, DType.Error); + _txb.ErrorContainer.Error(node, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()); } } @@ -2809,20 +2809,20 @@ public override void Visit(FirstNameNode node) return; } - 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 = RightRecord.a, ...) - // The expression above will create LeftRecord and RightRecord scopes. The scope type will be ![LeftRecord:![...],RightRecord:![...]] - // Example: Join(t1 As X1, t2 As X2, X1.a = X2.a, ...) - // The expression above will create LeftRecord and RightRecord scopes. The scope type will be ![X1:![...],X2:![...]] - nodeType = scope.Type.GetType(nodeName); + 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 = RightRecord.a, ...) + // The expression above will create LeftRecord and RightRecord scopes. The scope type will be ![LeftRecord:![...],RightRecord:![...]] + // Example: Join(t1 As X1, t2 As X2, X1.a = X2.a, ...) + // The expression above will create LeftRecord and RightRecord scopes. The scope type will be ![X1:![...],X2:![...]] + nodeType = scope.Type.GetType(nodeName); } if (!isWholeScope) @@ -2881,29 +2881,29 @@ public override void Visit(FirstNameNode node) _txb.SetSetMutable(node, nameSymbol?.Props.CanSetMutate ?? false); if (lookupInfo.Data is IExternalNamedFormula formula) { - isConstantNamedFormula = formula.IsConstant; - - // If the definition of the named formula has a delegation warning, every use should also inherit this warning - if (formula.HasDelegationWarning) - { - _txb.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, node, TexlStrings.SuggestRemoteExecutionHint_NF, node.Ident.Name); + isConstantNamedFormula = formula.IsConstant; + + // If the definition of the named formula has a delegation warning, every use should also inherit this warning + if (formula.HasDelegationWarning) + { + _txb.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, node, TexlStrings.SuggestRemoteExecutionHint_NF, node.Ident.Name); } } } - else if (lookupInfo.Kind == BindKind.Data) - { - if (lookupInfo.Data is IExternalCdsDataSource or IExternalTabularDataSource) - { - _txb.SetMutable(node, true); - } - else if (lookupInfo.Data is IExternalDataSource ds) - { - _txb.SetMutable(node, ds.IsWritable); - } - } - else if (lookupInfo.Kind == BindKind.ScopeCollection) - { - _txb.SetMutable(node, true); + else if (lookupInfo.Kind == BindKind.Data) + { + if (lookupInfo.Data is IExternalCdsDataSource or IExternalTabularDataSource) + { + _txb.SetMutable(node, true); + } + else if (lookupInfo.Data is IExternalDataSource ds) + { + _txb.SetMutable(node, ds.IsWritable); + } + } + else if (lookupInfo.Kind == BindKind.ScopeCollection) + { + _txb.SetMutable(node, true); } else if (lookupInfo.Kind == BindKind.ScopeCollection) { @@ -3018,14 +3018,14 @@ public override void Visit(FirstNameNode node) { _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrInvalidControlReference); } - } - - if (_txb.BindingConfig.MarkAsAsyncOnLazilyLoadedControlRef && - lookupType.IsControl && - lookupInfo.Data is IExternalControl control && - !control.IsAppGlobalControl) - { - _txb.FlagPathAsAsync(_txb.Top); + } + + if (_txb.BindingConfig.MarkAsAsyncOnLazilyLoadedControlRef && + lookupType.IsControl && + lookupInfo.Data is IExternalControl control && + !control.IsAppGlobalControl) + { + _txb.FlagPathAsAsync(_txb.Top); } // Update _usesGlobals, _usesResources, etc. @@ -3559,7 +3559,7 @@ public override void PostVisit(DottedNameNode node) // If the reference is to Control.Property and the rule for that Property is a constant, // we need to mark the node as constant, and save the control info so we may look up the // rule later. - if (controlInfo?.GetRule(property.InvariantName) is IExternalRule rule && + if (controlInfo?.GetRule(property.InvariantName) is IExternalRule rule && rule.IsInvariantExpression) { value = controlInfo; @@ -3818,16 +3818,16 @@ protected DType GetExpandedEntityType(DType expandEntityType, string relatedEnti // Update the datasource and relatedEntity path. type.ExpandInfo.UpdateEntityInfo(expandEntityInfo.ParentDataSource, relatedEntityPath); entityTypes.Add(expandEntityInfo.ExpandPath, type); - } - else if (!type.ExpandInfo.ExpandPath.IsReachedFromPath(relatedEntityPath)) - { - // Expands reached via a different path should have a different relatedentitypath. - // If we found an expand in the cache but it's not accessed via the same relationship - // we need to create a different expand info but with the same type. - // DType.Clone doesn't clone expand info, so we force that with CopyExpandInfo, - // because that sadly mutates expand info on what should otherwise be an immutable dtype. + } + else if (!type.ExpandInfo.ExpandPath.IsReachedFromPath(relatedEntityPath)) + { + // Expands reached via a different path should have a different relatedentitypath. + // If we found an expand in the cache but it's not accessed via the same relationship + // we need to create a different expand info but with the same type. + // DType.Clone doesn't clone expand info, so we force that with CopyExpandInfo, + // because that sadly mutates expand info on what should otherwise be an immutable dtype. type = DType.CopyExpandInfo(type.Clone(), type); - type.ExpandInfo.UpdateEntityInfo(expandEntityInfo.ParentDataSource, relatedEntityPath); + type.ExpandInfo.UpdateEntityInfo(expandEntityInfo.ParentDataSource, relatedEntityPath); } return type; @@ -4105,13 +4105,13 @@ private void SetCallNodePurity(CallNode node, CallInfo info) var infoTexlFunction = info.Function; if (_txb._glue.IsComponentScopedPropertyFunction(infoTexlFunction)) - { - // Behavior only component properties should be treated as stateful. - hasSideEffects |= infoTexlFunction.IsBehaviorOnly; - - // At the moment, we're going to treat all invocations of component scoped property functions as stateful. - // This ensures that we don't lift these function invocations in loops, and that they are re-evaluated every time they are called, - // which is always correct, although less efficient in some cases. + { + // Behavior only component properties should be treated as stateful. + hasSideEffects |= infoTexlFunction.IsBehaviorOnly; + + // At the moment, we're going to treat all invocations of component scoped property functions as stateful. + // This ensures that we don't lift these function invocations in loops, and that they are re-evaluated every time they are called, + // which is always correct, although less efficient in some cases. isStateFul |= true; } else @@ -4277,34 +4277,34 @@ private void UntypedObjectScopeError(CallNode node, TexlFunction maybeFunc, Texl _txb.SetInfo(node, new CallInfo(maybeFunc, node, null, default, false, _currentScope.Nest)); _txb.SetType(node, maybeFunc.ReturnType); - } - - // checks if the call node best matches function overloads with UntypedObject/JSON - private bool MatchOverloadWithUntypedOrJSONConversionFunctions(CallNode node, TexlFunction maybeFunc) - { - Contracts.AssertValue(node); - Contracts.AssertValue(maybeFunc); - Contracts.Assert(maybeFunc.HasTypeArgs); - - if (maybeFunc.Name == AsTypeFunction.AsTypeInvariantFunctionName && - _txb.GetType(node.Args.Children[0]) == DType.UntypedObject) - { - return true; - } - - if (maybeFunc.Name == IsTypeFunction_UO.IsTypeInvariantFunctionName && - _txb.GetType(node.Args.Children[0]) == DType.UntypedObject) - { - return true; - } - - if (maybeFunc.Name == ParseJSONFunction.ParseJSONInvariantFunctionName && - node.Args.Count > 1) - { - return true; - } - - return false; + } + + // checks if the call node best matches function overloads with UntypedObject/JSON + private bool MatchOverloadWithUntypedOrJSONConversionFunctions(CallNode node, TexlFunction maybeFunc) + { + Contracts.AssertValue(node); + Contracts.AssertValue(maybeFunc); + Contracts.Assert(maybeFunc.HasTypeArgs); + + if (maybeFunc.Name == AsTypeFunction.AsTypeInvariantFunctionName && + _txb.GetType(node.Args.Children[0]) == DType.UntypedObject) + { + return true; + } + + if (maybeFunc.Name == IsTypeFunction_UO.IsTypeInvariantFunctionName && + _txb.GetType(node.Args.Children[0]) == DType.UntypedObject) + { + return true; + } + + if (maybeFunc.Name == ParseJSONFunction.ParseJSONInvariantFunctionName && + node.Args.Count > 1) + { + return true; + } + + return false; } public override bool PreVisit(CallNode node) @@ -4325,7 +4325,7 @@ public override bool PreVisit(CallNode node) if (BuiltinFunctionsCore.OtherKnownFunctions.Contains(node.Head.Name.Value, StringComparer.OrdinalIgnoreCase)) { _txb.ErrorContainer.Error(node, TexlStrings.ErrUnimplementedFunction, node.Head.Name.Value); - } + } else if (BuiltinFunctionsCore.TypeHelperFunctions.Contains(node.Head.Name.Value, StringComparer.OrdinalIgnoreCase)) { _txb.ErrorContainer.Error(node, TexlStrings.ErrKnownTypeHelperFunction, node.Head.Name.Value); @@ -4359,9 +4359,9 @@ public override bool PreVisit(CallNode node) // If there are no overloads with lambdas or identifiers, we can continue the visitation and // yield to the normal overload resolution. - var overloadsWithLambdasOrIdentifiers = overloads.Where(func => func.HasLambdas || func.HasColumnIdentifiers); - - var overloadsWithTypeArgs = overloads.Where(func => func.HasTypeArgs); + var overloadsWithLambdasOrIdentifiers = overloads.Where(func => func.HasLambdas || func.HasColumnIdentifiers); + + var overloadsWithTypeArgs = overloads.Where(func => func.HasTypeArgs); if (!overloadsWithLambdasOrIdentifiers.Any()) { @@ -4396,18 +4396,18 @@ public override bool PreVisit(CallNode node) if (overloadsWithTypeArgs.Any() && node.Args.Count > 1) { var nodeInp = node.Args.Children[0]; - nodeInp.Accept(this); - - Contracts.Assert(overloadsWithTypeArgs.Count() == 1); - - var functionWithTypeArg = overloadsWithTypeArgs.First(); + nodeInp.Accept(this); + + Contracts.Assert(overloadsWithTypeArgs.Count() == 1); + + var functionWithTypeArg = overloadsWithTypeArgs.First(); if (MatchOverloadWithUntypedOrJSONConversionFunctions(node, functionWithTypeArg)) { - PreVisitTypeArgAndProccesCallNode(node, functionWithTypeArg); - FinalizeCall(node); + PreVisitTypeArgAndProccesCallNode(node, functionWithTypeArg); + FinalizeCall(node); return false; - } + } startArg++; } @@ -4472,16 +4472,16 @@ public override bool PreVisit(CallNode node) scopeNew = new Scope(node, _currentScope, scopeInfo.ScopeType, skipForInlineRecords: maybeFunc.SkipScopeForInlineRecords); } else if (carg > 0) - { - // Gets the lesser number between the CallNode chidl args and func.ScopeArgs. - // There reason why is that the intellisense can visit this code and provide a number of args less than the func.ScopeArgs. - // Example: Join(| - var argsCount = Math.Min(node.Args.Children.Count, maybeFunc.ScopeArgs); - var types = new DType[argsCount]; - - for (int i = 0; i < argsCount; i++) - { - node.Args.Children[i].Accept(this); + { + // Gets the lesser number between the CallNode chidl args and func.ScopeArgs. + // There reason why is that the intellisense can visit this code and provide a number of args less than the func.ScopeArgs. + // Example: Join(| + var argsCount = Math.Min(node.Args.Children.Count, maybeFunc.ScopeArgs); + var types = new DType[argsCount]; + + for (int i = 0; i < argsCount; i++) + { + node.Args.Children[i].Accept(this); } // At this point we know the type of the first argument, so we can check for untyped objects @@ -4491,7 +4491,7 @@ public override bool PreVisit(CallNode node) scopeInfo = maybeFunc.ScopeInfo; } - // Determine the Scope Identifier using the func.ScopeArgs arg + // Determine the Scope Identifier using the func.ScopeArgs arg required = scopeInfo.GetScopeIdent(node.Args.Children.ToArray(), out scopeIdentifiers); if (scopeInfo.CheckInput(_txb.Features, node, node.Args.Children.ToArray(), out scope, GetScopeArgsTypes(node.Args.Children, argsCount))) @@ -4569,14 +4569,14 @@ public override bool PreVisit(CallNode node) { _currentScopeDsNodeId = dsNode.Id; } - - argTypes[0] = _txb.GetType(nodeInput); + + argTypes[0] = _txb.GetType(nodeInput); // Get the cursor type for this arg. Note we're not adding document errors at this point. DType typeScope; DName[] scopeIdent = default; var identRequired = false; - var fArgsValid = true; + var fArgsValid = true; if (scopeInfo.ScopeType != null) { @@ -4585,17 +4585,17 @@ public override bool PreVisit(CallNode node) // For functions with a Scope Type, there is no ScopeIdent needed } else - { - // Starting from 1 since 0 was visited above. - for (int i = 1; i < maybeFunc.ScopeArgs; i++) - { - _txb.AddVolatileVariables(node, _txb.GetVolatileVariables(args[i])); - args[i].Accept(this); - } - + { + // Starting from 1 since 0 was visited above. + for (int i = 1; i < maybeFunc.ScopeArgs; i++) + { + _txb.AddVolatileVariables(node, _txb.GetVolatileVariables(args[i])); + args[i].Accept(this); + } + fArgsValid = scopeInfo.CheckInput(_txb.Features, node, args, out typeScope, GetScopeArgsTypes(node.Args.Children, maybeFunc.ScopeArgs)); - // Determine the scope identifier using the first node for lambda params + // Determine the scope identifier using the first node for lambda params identRequired = scopeInfo.GetScopeIdent(args, out scopeIdent); } @@ -4668,15 +4668,15 @@ public override bool PreVisit(CallNode node) _currentScope = (isIdentifier || isLambdaArg) ? scopeNew : scopeNew.Parent; if (!isIdentifier || maybeFunc.GetIdentifierParamStatus(args[i], _features, i) == TexlFunction.ParamIdentifierStatus.PossiblyIdentifier) - { - if (_txb.GetTypeAllowInvalid(args[i]) != null && !_txb.GetTypeAllowInvalid(args[i]).IsValid) - { - args[i].Accept(this); - _txb.AddVolatileVariables(node, _txb.GetVolatileVariables(args[i])); - } - - argTypes[i] = _txb.GetType(args[i]); - + { + if (_txb.GetTypeAllowInvalid(args[i]) != null && !_txb.GetTypeAllowInvalid(args[i]).IsValid) + { + args[i].Accept(this); + _txb.AddVolatileVariables(node, _txb.GetVolatileVariables(args[i])); + } + + argTypes[i] = _txb.GetType(args[i]); + Contracts.Assert(argTypes[i].IsValid); } else @@ -4743,17 +4743,17 @@ public override bool PreVisit(CallNode node) // We fully processed the call, so don't visit children or call PostVisit. return false; - } - - /// - /// Get all DType used to compose the scope of the function (func.ScopeArgs). - /// - /// Call child nodes. - /// TexlFunction ScopeArgs property. - /// DType array. - private DType[] GetScopeArgsTypes(IReadOnlyList args, int scopeArgs) - { - return args.Take(scopeArgs).Select(node => _txb.GetType(node)).ToArray(); + } + + /// + /// Get all DType used to compose the scope of the function (func.ScopeArgs). + /// + /// Call child nodes. + /// TexlFunction ScopeArgs property. + /// DType array. + private DType[] GetScopeArgsTypes(IReadOnlyList args, int scopeArgs) + { + return args.Take(scopeArgs).Select(node => _txb.GetType(node)).ToArray(); } private void FinalizeCall(CallNode node) @@ -4808,14 +4808,14 @@ private void FinalizeCall(CallNode node) // Invalid datasources always result in error if (func.IsBehaviorOnly && !_txb.BindingConfig.AllowsSideEffects) - { - if (_txb.BindingConfig.UserDefinitionsMode) - { - _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrBehaviorFunctionInDataUDF); - } - else - { - _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrBehaviorPropertyExpected); + { + if (_txb.BindingConfig.UserDefinitionsMode) + { + _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrBehaviorFunctionInDataUDF); + } + else + { + _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrBehaviorPropertyExpected); } } @@ -4853,12 +4853,12 @@ private void FinalizeCall(CallNode node) { _txb.ErrorContainer.EnsureError(node, errorKey, badAncestor.Head.Name); } - } - - // If the definition of the user-defined function has a delegation warning, every usage should also inherit this warning - if (func is UserDefinedFunction udf && udf.HasDelegationWarning) - { - _txb.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, node, TexlStrings.SuggestRemoteExecutionHint_UDF, udf.Name); + } + + // If the definition of the user-defined function has a delegation warning, every usage should also inherit this warning + if (func is UserDefinedFunction udf && udf.HasDelegationWarning) + { + _txb.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, node, TexlStrings.SuggestRemoteExecutionHint_UDF, udf.Name); } _txb.CheckAndMarkAsDelegatable(node); @@ -5062,9 +5062,9 @@ private void PreVisitMetadataArg(CallNode node, TexlFunction func) } else { - if (_txb.GetTypeAllowInvalid(args[i]) != null && !_txb.GetTypeAllowInvalid(args[i]).IsValid) - { - args[i].Accept(this); + if (_txb.GetTypeAllowInvalid(args[i]) != null && !_txb.GetTypeAllowInvalid(args[i]).IsValid) + { + args[i].Accept(this); } } @@ -5115,70 +5115,70 @@ private void PreVisitMetadataArg(CallNode node, TexlFunction func) } _txb.SetType(node, returnType); - } - - // Method to previsit type arg of callnode if it is determined as untyped/json conversion function - private void PreVisitTypeArgAndProccesCallNode(CallNode node, TexlFunction func) - { + } + + // Method to previsit type arg of callnode if it is determined as untyped/json conversion function + private void PreVisitTypeArgAndProccesCallNode(CallNode node, TexlFunction func) + { AssertValid(); Contracts.AssertValue(node); - Contracts.AssertValue(func); - Contracts.Assert(func.HasTypeArgs); - - var args = node.Args.Children.ToArray(); - var argCount = args.Count(); - - Contracts.AssertValue(_txb.GetType(args[0])); - + Contracts.AssertValue(func); + Contracts.Assert(func.HasTypeArgs); + + var args = node.Args.Children.ToArray(); + var argCount = args.Count(); + + Contracts.AssertValue(_txb.GetType(args[0])); + if (argCount < func.MinArity || argCount > func.MaxArity) { ArityError(func.MinArity, func.MaxArity, node, argCount, _txb.ErrorContainer); _txb.SetInfo(node, new CallInfo(func, node)); _txb.SetType(node, DType.Error); return; - } - + } + if (!_features.IsUserDefinedTypesEnabled) { _txb.ErrorContainer.Error(node, TexlStrings.ErrUserDefinedTypesDisabled); _txb.SetInfo(node, new CallInfo(func, node)); _txb.SetType(node, DType.Error); return; - } - - Contracts.Assert(argCount > 1); - Contracts.AssertValue(args[1]); - - if (args[1] is FirstNameNode typeName) - { - if (_nameResolver.LookupType(typeName.Ident.Name, out var typeArgType)) - { - _txb.SetType(typeName, typeArgType._type); - _txb.SetInfo(typeName, FirstNameInfo.Create(typeName, new NameLookupInfo(BindKind.NamedType, typeArgType._type, DPath.Root, 0))); - } - else - { - _txb.ErrorContainer.Error(args[1], TexlStrings.ErrInvalidName, typeName.Ident.Name.Value); - _txb.SetType(args[1], DType.Error); - _txb.SetInfo(typeName, FirstNameInfo.Create(typeName, new NameLookupInfo(BindKind.NamedType, DType.Error, DPath.Root, 0))); - - _txb.ErrorContainer.Error(node, TexlStrings.ErrInvalidArgumentExpectedType, typeName.Ident.Name.Value); - _txb.SetInfo(node, new CallInfo(func, node)); - _txb.SetType(node, DType.Error); - } - } - else if (args[1] is TypeLiteralNode typeLiteral) - { - VisitType(typeLiteral); - } - else - { - _txb.ErrorContainer.Error(args[1], TexlStrings.ErrInvalidArgumentExpectedType, args[1]); - _txb.SetType(args[1], DType.Error); - } - - PostVisit(node.Args); - + } + + Contracts.Assert(argCount > 1); + Contracts.AssertValue(args[1]); + + if (args[1] is FirstNameNode typeName) + { + if (_nameResolver.LookupType(typeName.Ident.Name, out var typeArgType)) + { + _txb.SetType(typeName, typeArgType._type); + _txb.SetInfo(typeName, FirstNameInfo.Create(typeName, new NameLookupInfo(BindKind.NamedType, typeArgType._type, DPath.Root, 0))); + } + else + { + _txb.ErrorContainer.Error(args[1], TexlStrings.ErrInvalidName, typeName.Ident.Name.Value); + _txb.SetType(args[1], DType.Error); + _txb.SetInfo(typeName, FirstNameInfo.Create(typeName, new NameLookupInfo(BindKind.NamedType, DType.Error, DPath.Root, 0))); + + _txb.ErrorContainer.Error(node, TexlStrings.ErrInvalidArgumentExpectedType, typeName.Ident.Name.Value); + _txb.SetInfo(node, new CallInfo(func, node)); + _txb.SetType(node, DType.Error); + } + } + else if (args[1] is TypeLiteralNode typeLiteral) + { + VisitType(typeLiteral); + } + else + { + _txb.ErrorContainer.Error(args[1], TexlStrings.ErrInvalidArgumentExpectedType, args[1]); + _txb.SetType(args[1], DType.Error); + } + + PostVisit(node.Args); + var info = _txb.GetInfo(node); // If PreVisit resulted in errors for the node (and a non-null CallInfo), @@ -5192,27 +5192,27 @@ private void PreVisitTypeArgAndProccesCallNode(CallNode node, TexlFunction func) Contracts.AssertNull(info); - _txb.SetInfo(node, new CallInfo(func, node)); - - var returnType = func.ReturnType; + _txb.SetInfo(node, new CallInfo(func, node)); + + var returnType = func.ReturnType; var argTypes = args.Select(_txb.GetType).ToArray(); bool fArgsValid; var checkErrorContainer = new ErrorContainer(); // Typecheck the invocation and infer the return type. fArgsValid = func.HandleCheckInvocation(_txb, args, argTypes, checkErrorContainer, out returnType, out var _); - - if (checkErrorContainer?.HasErrors() == true) - { - _txb.ErrorContainer.MergeErrors(checkErrorContainer.GetErrors()); - } + + if (checkErrorContainer?.HasErrors() == true) + { + _txb.ErrorContainer.MergeErrors(checkErrorContainer.GetErrors()); + } if (!fArgsValid) { _txb.ErrorContainer.Error(DocumentErrorSeverity.Severe, node.Head.Token, TexlStrings.ErrInvalidArgs_Func, func.Name); } - _txb.SetType(node, returnType); + _txb.SetType(node, returnType); } private void PreVisitBottomUp(CallNode node, int argCountVisited, Scope scopeNew = null) @@ -5277,10 +5277,10 @@ private void PreVisitBottomUp(CallNode node, int argCountVisited, Scope scopeNew { _txb.AddVolatileVariables(args[i], volatileVariables); } - - if (_txb.GetTypeAllowInvalid(args[i]) != null && !_txb.GetTypeAllowInvalid(args[i]).IsValid) - { - args[i].Accept(this); + + if (_txb.GetTypeAllowInvalid(args[i]) != null && !_txb.GetTypeAllowInvalid(args[i]).IsValid) + { + args[i].Accept(this); } // In case weight was added during visitation diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 9d93f72821..5efafa90d1 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -10,7 +10,7 @@ using System.Threading.Tasks; using Microsoft.PowerFx.Core; using Microsoft.PowerFx.Core.Functions; -using Microsoft.PowerFx.Core.Functions.Delegation; +using Microsoft.PowerFx.Core.Functions.Delegation; using Microsoft.PowerFx.Core.Functions.Delegation.DelegationMetadata; using Microsoft.PowerFx.Core.IR; using Microsoft.PowerFx.Core.Localization; @@ -398,8 +398,8 @@ public void UserDefinitionOnUpdateTest() // After (A,B) = 3,6 var result = engine.Eval("With({x:A, y:B}, Set(A,3); x & y & A & B)", options: _opts); Assert.Equal("102036", result.ToObject()); - } - + } + [Fact] public void BasicEval() { @@ -412,20 +412,20 @@ public void BasicEval() [Fact] public void BuiltInEnumConfigCheck() - { - var config = new PowerFxConfig() - { - SymbolTable = null - }; - -#pragma warning disable CS0618 // Type or member is obsolete - config.EnableRegExFunctions(); -#pragma warning restore CS0618 // Type or member is obsolete - var expression = "Match(\"test\", \"t\", MatchOptions.Contains)"; - - var engine = new RecalcEngine(config); - var check = engine.Check(expression); - Assert.True(check.IsSuccess); + { + var config = new PowerFxConfig() + { + SymbolTable = null + }; + +#pragma warning disable CS0618 // Type or member is obsolete + config.EnableRegExFunctions(); +#pragma warning restore CS0618 // Type or member is obsolete + var expression = "Match(\"test\", \"t\", MatchOptions.Contains)"; + + var engine = new RecalcEngine(config); + var check = engine.Check(expression); + Assert.True(check.IsSuccess); } [Fact] @@ -486,7 +486,7 @@ public void FormulaCantRedefine() false, 14.0)] - // Recursive calls are not allowed + // Recursive calls are not allowed [InlineData( "foo(x:Number):Number = If(x=0,foo(1),If(x=1,foo(2),If(x=2,Float(2))));", "foo(Float(0))", @@ -614,78 +614,78 @@ public void FunctionPrecedenceTest(string script, double expected) var result = check.GetEvaluator().Eval(); Assert.Equal(expected, result.AsDouble()); - } - + } + [Fact] public void ShadowingFunctionPrecedenceTest() - { + { var engine = new RecalcEngine(); - engine.AddUserDefinedFunction("Concat(x:Text):Text = \"xyz\"; Average(x:Number):Number = 11111;"); + engine.AddUserDefinedFunction("Concat(x:Text):Text = \"xyz\"; Average(x:Number):Number = 11111;"); var check = engine.Check("Concat(\"abc\")"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.String, check.ReturnType); - var result = check.GetEvaluator().Eval(); - Assert.Equal("xyz", ((StringValue)result).Value); - + var result = check.GetEvaluator().Eval(); + Assert.Equal("xyz", ((StringValue)result).Value); + check = engine.Check("Average(123)"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Number, check.ReturnType); - result = check.GetEvaluator().Eval(); - Assert.Equal(11111, result.AsDouble()); - - engine.AddUserDefinitions("Test := Type({A: Number}); TestTable := Type([{A: Number}]);" + - "Filter(X: TestTable):Test = First(X); ShowColumns(X: TestTable):TestTable = FirstN(X, 3);"); - + result = check.GetEvaluator().Eval(); + Assert.Equal(11111, result.AsDouble()); + + engine.AddUserDefinitions("Test := Type({A: Number}); TestTable := Type([{A: Number}]);" + + "Filter(X: TestTable):Test = First(X); ShowColumns(X: TestTable):TestTable = FirstN(X, 3);"); + check = engine.Check("Filter([{A: 123}]).A"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Number, check.ReturnType); - result = check.GetEvaluator().Eval(); - Assert.Equal(123, result.AsDouble()); - + result = check.GetEvaluator().Eval(); + Assert.Equal(123, result.AsDouble()); + check = engine.Check("CountRows(ShowColumns([{A: 123}, {A: 124}, {A:125}, {A:126}, {A: 127}]))"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Decimal, check.ReturnType); - result = check.GetEvaluator().Eval(); + result = check.GetEvaluator().Eval(); Assert.Equal(3, result.AsDouble()); } - - [Theory] - - // Behavior function in non-imperative udf + + [Theory] + + // Behavior function in non-imperative udf [InlineData( "TestFunc():Void = Set(a, 123);", true, - "Behavior function in a non-behavior user-defined function", - false)] - - // Behavior function in imperative udf + "Behavior function in a non-behavior user-defined function", + false)] + + // Behavior function in imperative udf [InlineData( "TestFunc():Void = { Set(a, 123); };", false, - null, - true)] + null, + true)] public void BehaviorFunctionInImperativeUDF(string udfExpression, bool expectedError, string expectedErrorKey, bool allowSideEffects) - { - var config = new PowerFxConfig(); - config.EnableSetFunction(); - var engine = new RecalcEngine(config); - engine.UpdateVariable("a", 1m); - - var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); - Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); - - if (expectedError) - { - result.Errors.Any(error => error.MessageKey == expectedErrorKey); + { + var config = new PowerFxConfig(); + config.EnableSetFunction(); + var engine = new RecalcEngine(config); + engine.UpdateVariable("a", 1m); + + var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); + Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); + + if (expectedError) + { + result.Errors.Any(error => error.MessageKey == expectedErrorKey); } - } - + } + [Theory] // Return value with side effectful UDF @@ -710,12 +710,12 @@ public void ImperativeUserDefinedFunctionTest(string udfExpression, string expre config.EnableSetFunction(); var recalcEngine = new RecalcEngine(config); recalcEngine.UpdateVariable("a", 1m); + + var definitionsCheckResult = recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - var definitionsCheckResult = recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - - if (!expectedError) - { - Assert.True(definitionsCheckResult.IsSuccess); + if (!expectedError) + { + Assert.True(definitionsCheckResult.IsSuccess); var result = recalcEngine.Eval(expression, options: _opts); var fvExpected = FormulaValue.New(expected); @@ -724,136 +724,136 @@ public void ImperativeUserDefinedFunctionTest(string udfExpression, string expre } else { - Assert.False(definitionsCheckResult.IsSuccess); + Assert.False(definitionsCheckResult.IsSuccess); Assert.Single(definitionsCheckResult.Errors); Assert.Contains(definitionsCheckResult.Errors, err => err.MessageKey == errorKey); } - } - - [Theory] - - [InlineData( + } + + [Theory] + + [InlineData( "MismatchType():Number = { 1+3; Color.Blue; };", true, - true, - 36, - 41)] - [InlineData( + true, + 36, + 41)] + [InlineData( "MatchType():Text = { 4; 3 };", false, - true, - 0, + true, + 0, 0)] public void TestMismatchReturnType(string udfExpression, bool expectedError, bool allowSideEffects, int min, int lim) { var config = new PowerFxConfig(); config.EnableSetFunction(); var engine = new RecalcEngine(config); - engine.UpdateVariable("x", 1m); - - var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); - Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); - - if (expectedError) - { - var error = result.Errors.First(error => error.MessageKey == "ErrUDF_ReturnTypeDoesNotMatch"); - Assert.NotNull(error); - var span = error.Span; - Assert.True(span.Min == min && span.Lim == lim); + engine.UpdateVariable("x", 1m); + + var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); + Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); + + if (expectedError) + { + var error = result.Errors.First(error => error.MessageKey == "ErrUDF_ReturnTypeDoesNotMatch"); + Assert.NotNull(error); + var span = error.Span; + Assert.True(span.Min == min && span.Lim == lim); } - } - + } + [Fact] public void DelegableUDFTest() - { - var symbolTable = new DelegatableSymbolTable(); - var schema = DType.CreateTable( - new TypedName(DType.Guid, new DName("ID")), + { + var symbolTable = new DelegatableSymbolTable(); + var schema = DType.CreateTable( + new TypedName(DType.Guid, new DName("ID")), new TypedName(DType.Number, new DName("Value"))); - symbolTable.AddEntity(new TestDelegableDataSource( - "MyDataSource", - schema, - new TestDelegationMetadata( - new DelegationCapability(DelegationCapability.Filter), - schema, - new FilterOpMetadata( - schema, - new Dictionary(), - new Dictionary(), - new DelegationCapability(DelegationCapability.GreaterThan), - null)), - true)); + symbolTable.AddEntity(new TestDelegableDataSource( + "MyDataSource", + schema, + new TestDelegationMetadata( + new DelegationCapability(DelegationCapability.Filter), + schema, + new FilterOpMetadata( + schema, + new Dictionary(), + new Dictionary(), + new DelegationCapability(DelegationCapability.GreaterThan), + null)), + true)); symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema)); - var config = new PowerFxConfig() - { + var config = new PowerFxConfig() + { SymbolTable = symbolTable }; - config.EnableSetFunction(); - - var recalcEngine = new RecalcEngine(config); - - recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("C").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("B").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("D").First() as UserDefinedFunction; - - // Imperative function is not delegable - Assert.True(func.IsAsync); - Assert.True(!func.IsDelegatable); - - // Binding fails for recursive definitions and hence function is not added. - Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess); - } - - [Fact] + config.EnableSetFunction(); + + var recalcEngine = new RecalcEngine(config); + + recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); + var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("C").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("B").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("D").First() as UserDefinedFunction; + + // Imperative function is not delegable + Assert.True(func.IsAsync); + Assert.True(!func.IsDelegatable); + + // Binding fails for recursive definitions and hence function is not added. + Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess); + } + + [Fact] public void TestInheritanceOfDelegationWarningsInUDFs() - { - var symbolTable = new DelegatableSymbolTable(); - var schema = DType.CreateTable( + { + var symbolTable = new DelegatableSymbolTable(); + var schema = DType.CreateTable( new TypedName(DType.Number, new DName("Value"))); - symbolTable.AddEntity(new TestDelegableDataSource( - "MyDataSource", - schema, - new TestDelegationMetadata( - new DelegationCapability(DelegationCapability.Filter), - schema, - new FilterOpMetadata( - schema, - new Dictionary(), - new Dictionary(), - new DelegationCapability(DelegationCapability.GreaterThan), - null)), - true)); + symbolTable.AddEntity(new TestDelegableDataSource( + "MyDataSource", + schema, + new TestDelegationMetadata( + new DelegationCapability(DelegationCapability.Filter), + schema, + new FilterOpMetadata( + schema, + new Dictionary(), + new Dictionary(), + new DelegationCapability(DelegationCapability.GreaterThan), + null)), + true)); symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema)); - var config = new PowerFxConfig() - { + var config = new PowerFxConfig() + { SymbolTable = symbolTable }; - var engine = new RecalcEngine(config); - - var result = engine.AddUserDefinedFunction("NonDelegatableUDF():MyDataSourceTableType = Filter(MyDataSource, Sqrt(Value) > 5);", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); - Assert.True(result.IsSuccess); - var func = engine.Functions.WithName("NonDelegatableUDF").First() as UserDefinedFunction; - Assert.True(func.HasDelegationWarning); + var engine = new RecalcEngine(config); - result = engine.AddUserDefinedFunction("NonDelegatableUDF2():MyDataSourceTableType = NonDelegatableUDF();", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); - Assert.True(result.IsSuccess); - func = engine.Functions.WithName("NonDelegatableUDF2").First() as UserDefinedFunction; - Assert.True(func.HasDelegationWarning); + var result = engine.AddUserDefinedFunction("NonDelegatableUDF():MyDataSourceTableType = Filter(MyDataSource, Sqrt(Value) > 5);", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + var func = engine.Functions.WithName("NonDelegatableUDF").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); + + result = engine.AddUserDefinedFunction("NonDelegatableUDF2():MyDataSourceTableType = NonDelegatableUDF();", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + func = engine.Functions.WithName("NonDelegatableUDF2").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); } // Binding to inner functions does not impact outer functions. @@ -1768,68 +1768,68 @@ public void LookupBuiltinOptionSets() Assert.True(ok); // Wrong type: https://github.com/microsoft/Power-Fx/issues/2342 - } - - [Fact] - public void LazyJsonTest() - { - bool LazyGetField1(string name, out FormulaType type) - { - type = name switch - { - "field1" => FormulaType.Decimal, - "field2" => FormulaType.String, - _ => FormulaType.Blank, - }; - - return type != FormulaType.Blank; - } - - PowerFxConfig config = new PowerFxConfig(); - config.EnableJsonFunctions(); - - RecalcEngine engine = new RecalcEngine(config); - SymbolTable symbolTable = new SymbolTable(); - TestLazyRecordType lazyRecordType = new TestLazyRecordType("test", new List() { "field1", "field2" }, LazyGetField1); - - ISymbolSlot slot = symbolTable.AddVariable("var1", lazyRecordType); - - CheckResult checkResult = engine.Check("JSON(var1)", symbolTable: symbolTable); - Assert.True(checkResult.IsSuccess, string.Join(", ", checkResult.Errors.Select(err => err.Message))); - - SymbolValues symbolValues = new SymbolValues(symbolTable); - symbolValues.Set(slot, new LazyRecordValue(lazyRecordType)); - - FormulaValue formulaValue = checkResult.GetEvaluator().Eval(symbolValues); - StringValue stringValue = Assert.IsType(formulaValue); - - Assert.Equal(@"{""field1"":10,""field2"":""Str""}", stringValue.Value); - } - - public class LazyRecordValue : RecordValue - { - public LazyRecordValue(RecordType type) - : base(type) - { - } - - protected override bool TryGetField(FormulaType fieldType, string fieldName, out FormulaValue result) - { - if (fieldName == "field1") - { - result = FormulaValue.New(10); - return true; - } - - if (fieldName == "field2") - { - result = FormulaValue.New("Str"); - return true; - } - - result = null; - return false; - } + } + + [Fact] + public void LazyJsonTest() + { + bool LazyGetField1(string name, out FormulaType type) + { + type = name switch + { + "field1" => FormulaType.Decimal, + "field2" => FormulaType.String, + _ => FormulaType.Blank, + }; + + return type != FormulaType.Blank; + } + + PowerFxConfig config = new PowerFxConfig(); + config.EnableJsonFunctions(); + + RecalcEngine engine = new RecalcEngine(config); + SymbolTable symbolTable = new SymbolTable(); + TestLazyRecordType lazyRecordType = new TestLazyRecordType("test", new List() { "field1", "field2" }, LazyGetField1); + + ISymbolSlot slot = symbolTable.AddVariable("var1", lazyRecordType); + + CheckResult checkResult = engine.Check("JSON(var1)", symbolTable: symbolTable); + Assert.True(checkResult.IsSuccess, string.Join(", ", checkResult.Errors.Select(err => err.Message))); + + SymbolValues symbolValues = new SymbolValues(symbolTable); + symbolValues.Set(slot, new LazyRecordValue(lazyRecordType)); + + FormulaValue formulaValue = checkResult.GetEvaluator().Eval(symbolValues); + StringValue stringValue = Assert.IsType(formulaValue); + + Assert.Equal(@"{""field1"":10,""field2"":""Str""}", stringValue.Value); + } + + public class LazyRecordValue : RecordValue + { + public LazyRecordValue(RecordType type) + : base(type) + { + } + + protected override bool TryGetField(FormulaType fieldType, string fieldName, out FormulaValue result) + { + if (fieldName == "field1") + { + result = FormulaValue.New(10); + return true; + } + + if (fieldName == "field2") + { + result = FormulaValue.New("Str"); + return true; + } + + result = null; + return false; + } } [Theory] @@ -1889,7 +1889,7 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out hasNoAge(e: Employee): Number = IsBlank(getAge(e));", "hasNoAge({Name: \"Bob\", Title: \"CEO\"})", true, - 1.0)] + 1.0)] // Types with UDF restricted primitive types resolve successfully [InlineData( @@ -1956,7 +1956,7 @@ public void UserDefinedTypeTest(string userDefinitions, string evalExpression, b var recalcEngine = new RecalcEngine(config); var parserOptions = new ParserOptions() { - AllowsSideEffects = false, + AllowsSideEffects = false, }; if (isValid) @@ -1972,24 +1972,24 @@ public void UserDefinedTypeTest(string userDefinitions, string evalExpression, b { Assert.Throws(() => recalcEngine.AddUserDefinitions(userDefinitions, CultureInfo.InvariantCulture)); } - } - - [Theory] + } + + [Theory] [InlineData( "F():Point = {x: 5, y:5};", "F().x", - true, + true, 5.0)] [InlineData( "F():Number = { Sqrt(1); 42; };", "F()", - true, - 42.0)] + true, + 42.0)] [InlineData( "F():Point = { {x:0, y:1729}; };", "F().y", - true, - 1729.0)] + true, + 1729.0)] [InlineData( "F():Point = {x: 5, 42};", "F().x", @@ -2001,9 +2001,9 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression var parserOptions = new ParserOptions() { AllowsSideEffects = true, - }; - - var extraSymbols = new SymbolTable(); + }; + + var extraSymbols = new SymbolTable(); extraSymbols.AddType(new DName("Point"), FormulaType.Build(TestUtils.DT("![x:n, y:n]"))); if (isValid) @@ -2015,117 +2015,117 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression { Assert.False(recalcEngine.AddUserDefinedFunction(udf, CultureInfo.InvariantCulture, extraSymbols, true).IsSuccess); } - } - - [Theory] + } + + [Theory] [InlineData( "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points)); distance(a: Point, b: Point): Number = Sqrt(Power(b.x-a.x, 2) + Power(b.y-a.y, 2));", "distance({x: 0, y: 0}, {x: 0, y: 5})", true, - 5.0)] + 5.0)] [InlineData( "Account := Type(RecordOf(Accounts)); getAccountId(a: Account): Number = a.id;", "getAccountId({id: 42, name: \"T-Rex\", address: \"Museum\"})", true, - 42.0)] + 42.0)] [InlineData( "Account := Type(RecordOf(Accounts)); FirstAccount(a: Accounts): Account = First(a); getAccountId(a: Account): Number = a.id;", "getAccountId(FirstAccount([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}]))", true, - 1729.0)] + 1729.0)] [InlineData( "NewAccounts := Type([RecordOf(Accounts)]); getFirstAccountId(a: NewAccounts): Number = First(a).id;", "getFirstAccountId([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}])", true, - 1729.0)] + 1729.0)] [InlineData( "AccountWithAge := Type({age: Number, acc: RecordOf(Accounts)}); getAccountAge(a: AccountWithAge): Number = a.age;", "getAccountAge({age : 25, acc:First([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}])})", true, - 25.0)] + 25.0)] [InlineData( "Points := Type([{x : Number, y : Number}]); ComplexType := Type({p: RecordOf(Points), a: RecordOf(Accounts), s: SomeRecord}); getX(c: ComplexType): Number = c.p.x;", "getX({s: {id: 1729, name: \"Bob\"}, p : {x: 1, y: 2}, a: {id: 42, name: \"Alice\", address: \"internet\"}})", true, - 1.0)] - - // No error on a UDF named RecordOf + 1.0)] + + // No error on a UDF named RecordOf [InlineData( @"RecordOf(x:Number): Number = x + 1;", "RecordOf(41)", true, - 42.0)] - - // Fails for any type other than table - [InlineData( - "Account := Type(RecordOf(SomeRecord));", - "", - false)] - [InlineData( - "Account := Type(RecordOf(Void));", - "", - false)] - [InlineData( - "Account := Type(RecordOf(UntypedObject));", - "", - false)] - - // invalid helper name + 42.0)] + + // Fails for any type other than table + [InlineData( + "Account := Type(RecordOf(SomeRecord));", + "", + false)] + [InlineData( + "Account := Type(RecordOf(Void));", + "", + false)] + [InlineData( + "Account := Type(RecordOf(UntypedObject));", + "", + false)] + + // invalid helper name [InlineData( "Account := Type(Recordof(Accounts));", "", - false)] + false)] [InlineData( "Account := Type(recordOf(Accounts));", "", - false)] + false)] [InlineData( "Account := Type(First(Accounts));", "", - false)] - - // Does not allow anything other firstname + false)] + + // Does not allow anything other firstname [InlineData( "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points As P));", "", - false)] + false)] [InlineData( "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points, Accounts));", "", - false)] + false)] [InlineData( "Account := Type((Accounts, SomeRecord));", "", - false)] + false)] [InlineData( "Point := Type(RecordOf([{x : Number, y : Number}]));", "", - false)] + false)] [InlineData( "T1 := Type(RecordOf(Type([{A:Number}])));", "", - false)] + false)] [InlineData( "T1 := Type(RecordOf(RecordOf([{x:Number, y:Number}])));", "", - false)] - - // RecordOf not in type literal + false)] + + // RecordOf not in type literal [InlineData( "Account = RecordOf(Accounts);", "", - false)] + false)] [InlineData( "F():Accounts = RecordOf(Accounts);", "", - false)] + false)] public void RecordOfTests(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) { var config = new PowerFxConfig(); var recalcEngine = new RecalcEngine(config); - var parserOptions = new ParserOptions(); - - recalcEngine.Config.SymbolTable.AddType(new DName("Accounts"), FormulaType.Build(TestUtils.DT("*[id: n, name:s, address:s]"))); + var parserOptions = new ParserOptions(); + + recalcEngine.Config.SymbolTable.AddType(new DName("Accounts"), FormulaType.Build(TestUtils.DT("*[id: n, name:s, address:s]"))); recalcEngine.Config.SymbolTable.AddType(new DName("SomeRecord"), FormulaType.Build(TestUtils.DT("![id: n, name:s]"))); if (isValid) From 7f5a52538f6e9d84ea0792d948553d71159d2619 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Fri, 13 Dec 2024 15:41:51 -0800 Subject: [PATCH 16/20] improve and fix error messages --- .../Functions/TexlFunction.cs | 2 +- .../Functions/UserDefinedFunction.cs | 6 +- .../Localization/Strings.cs | 1 + .../Microsoft.PowerFx.Core/Types/DType.cs | 2 +- src/strings/PowerFxResources.en-US.resx | 4 + .../RecalcEngineTests.cs | 584 +++++++++--------- 6 files changed, 303 insertions(+), 296 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs index f3b9d73073..5778346a41 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs @@ -559,7 +559,7 @@ private bool CheckTypesCore(CheckTypesContext context, TexlNode[] args, DType[] } Contracts.AssertValid(argTypes[i]); - var expectedParamType = ParamTypes[i]; + var expectedParamType = ParamTypes[i]; // If the strong-enum type flag is disabled, treat an enum option set type as the enum supertype instead if (!context.Features.StronglyTypedBuiltinEnums && expectedParamType.OptionSetInfo is EnumSymbol enumSymbol) diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs index 69981a88b8..23acd4ebde 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs @@ -58,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; } @@ -91,7 +93,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp !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, argTypes[i].GetKindString(), ParamTypes[i].GetKindString()); + errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrBadSchema_ExpectedType, ParamTypes[i].GetKindString()); return false; } } @@ -201,7 +203,7 @@ public void CheckTypesOnDeclaration(CheckTypesContext context, DType actualBodyR if ((ReturnType.IsTable && actualBodyReturnType.IsTable) || (ReturnType.IsRecord && actualBodyReturnType.IsRecord)) { - binding.ErrorContainer.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrBadSchema_ExpectedType, ReturnType.GetKindString(), actualBodyReturnType.GetKindString()); + binding.ErrorContainer.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrUDF_ReturnTypeSchemaDoesNotMatch, ReturnType.GetKindString()); return; } diff --git a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs index 33610571dc..b3eecd25c9 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs @@ -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"); diff --git a/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs b/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs index 11bf2f5d06..2bfbd65ae5 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs @@ -3314,7 +3314,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; diff --git a/src/strings/PowerFxResources.en-US.resx b/src/strings/PowerFxResources.en-US.resx index 998cd665d6..e8f01ecbdc 100644 --- a/src/strings/PowerFxResources.en-US.resx +++ b/src/strings/PowerFxResources.en-US.resx @@ -4221,6 +4221,10 @@ The stated function return type '{0}' does not match the return type of the function body '{1}'. This error message shows up when expected return type does not match with actual return type. The arguments '{0}' and '{1}' will be replaced with data types. For example, "The stated function return type 'Number' does not match the return type of the function body 'Table'" + + The schema of stated function return type '{0}' does not match the schema of return type of the function body. + This error message shows up when expected return type schema does not match with schema of actual return type. The arguments '{0}' will be replaced with aggregate data types. For example, "The schema of stated function return type 'Table' does not match the schema of return type of the function body." + Function {0} has too many parameters. User-defined functions support up to {1} parameters. This error message shows up when a user tries to define a function with too many parameters. {0} - the name of the user-defined function, {1} - the max number of parameters allowed. diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 3487130dc5..f0873041d2 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -10,7 +10,7 @@ using System.Threading.Tasks; using Microsoft.PowerFx.Core; using Microsoft.PowerFx.Core.Functions; -using Microsoft.PowerFx.Core.Functions.Delegation; +using Microsoft.PowerFx.Core.Functions.Delegation; using Microsoft.PowerFx.Core.Functions.Delegation.DelegationMetadata; using Microsoft.PowerFx.Core.IR; using Microsoft.PowerFx.Core.Localization; @@ -398,8 +398,8 @@ public void UserDefinitionOnUpdateTest() // After (A,B) = 3,6 var result = engine.Eval("With({x:A, y:B}, Set(A,3); x & y & A & B)", options: _opts); Assert.Equal("102036", result.ToObject()); - } - + } + [Fact] public void BasicEval() { @@ -412,20 +412,20 @@ public void BasicEval() [Fact] public void BuiltInEnumConfigCheck() - { - var config = new PowerFxConfig() - { - SymbolTable = null - }; - -#pragma warning disable CS0618 // Type or member is obsolete - config.EnableRegExFunctions(); -#pragma warning restore CS0618 // Type or member is obsolete - var expression = "Match(\"test\", \"t\", MatchOptions.Contains)"; - - var engine = new RecalcEngine(config); - var check = engine.Check(expression); - Assert.True(check.IsSuccess); + { + var config = new PowerFxConfig() + { + SymbolTable = null + }; + +#pragma warning disable CS0618 // Type or member is obsolete + config.EnableRegExFunctions(); +#pragma warning restore CS0618 // Type or member is obsolete + var expression = "Match(\"test\", \"t\", MatchOptions.Contains)"; + + var engine = new RecalcEngine(config); + var check = engine.Check(expression); + Assert.True(check.IsSuccess); } [Fact] @@ -486,7 +486,7 @@ public void FormulaCantRedefine() false, 14.0)] - // Recursive calls are not allowed + // Recursive calls are not allowed [InlineData( "foo(x:Number):Number = If(x=0,foo(1),If(x=1,foo(2),If(x=2,Float(2))));", "foo(Float(0))", @@ -614,78 +614,78 @@ public void FunctionPrecedenceTest(string script, double expected) var result = check.GetEvaluator().Eval(); Assert.Equal(expected, result.AsDouble()); - } - + } + [Fact] public void ShadowingFunctionPrecedenceTest() - { + { var engine = new RecalcEngine(); - engine.AddUserDefinedFunction("Concat(x:Text):Text = \"xyz\"; Average(x:Number):Number = 11111;"); + engine.AddUserDefinedFunction("Concat(x:Text):Text = \"xyz\"; Average(x:Number):Number = 11111;"); var check = engine.Check("Concat(\"abc\")"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.String, check.ReturnType); - var result = check.GetEvaluator().Eval(); - Assert.Equal("xyz", ((StringValue)result).Value); - + var result = check.GetEvaluator().Eval(); + Assert.Equal("xyz", ((StringValue)result).Value); + check = engine.Check("Average(123)"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Number, check.ReturnType); - result = check.GetEvaluator().Eval(); - Assert.Equal(11111, result.AsDouble()); - - engine.AddUserDefinitions("Test := Type({A: Number}); TestTable := Type([{A: Number}]);" + - "Filter(X: TestTable):Test = First(X); ShowColumns(X: TestTable):TestTable = FirstN(X, 3);"); - + result = check.GetEvaluator().Eval(); + Assert.Equal(11111, result.AsDouble()); + + engine.AddUserDefinitions("Test := Type({A: Number}); TestTable := Type([{A: Number}]);" + + "Filter(X: TestTable):Test = First(X); ShowColumns(X: TestTable):TestTable = FirstN(X, 3);"); + check = engine.Check("Filter([{A: 123}]).A"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Number, check.ReturnType); - result = check.GetEvaluator().Eval(); - Assert.Equal(123, result.AsDouble()); - + result = check.GetEvaluator().Eval(); + Assert.Equal(123, result.AsDouble()); + check = engine.Check("CountRows(ShowColumns([{A: 123}, {A: 124}, {A:125}, {A:126}, {A: 127}]))"); Assert.True(check.IsSuccess); Assert.Equal(FormulaType.Decimal, check.ReturnType); - result = check.GetEvaluator().Eval(); + result = check.GetEvaluator().Eval(); Assert.Equal(3, result.AsDouble()); } - - [Theory] - - // Behavior function in non-imperative udf + + [Theory] + + // Behavior function in non-imperative udf [InlineData( "TestFunc():Void = Set(a, 123);", true, - "Behavior function in a non-behavior user-defined function", - false)] - - // Behavior function in imperative udf + "Behavior function in a non-behavior user-defined function", + false)] + + // Behavior function in imperative udf [InlineData( "TestFunc():Void = { Set(a, 123); };", false, - null, - true)] + null, + true)] public void BehaviorFunctionInImperativeUDF(string udfExpression, bool expectedError, string expectedErrorKey, bool allowSideEffects) - { - var config = new PowerFxConfig(); - config.EnableSetFunction(); - var engine = new RecalcEngine(config); - engine.UpdateVariable("a", 1m); - - var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); - Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); - - if (expectedError) - { - result.Errors.Any(error => error.MessageKey == expectedErrorKey); + { + var config = new PowerFxConfig(); + config.EnableSetFunction(); + var engine = new RecalcEngine(config); + engine.UpdateVariable("a", 1m); + + var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); + Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); + + if (expectedError) + { + result.Errors.Any(error => error.MessageKey == expectedErrorKey); } - } - + } + [Theory] // Return value with side effectful UDF @@ -710,12 +710,12 @@ public void ImperativeUserDefinedFunctionTest(string udfExpression, string expre config.EnableSetFunction(); var recalcEngine = new RecalcEngine(config); recalcEngine.UpdateVariable("a", 1m); + + var definitionsCheckResult = recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - var definitionsCheckResult = recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - - if (!expectedError) - { - Assert.True(definitionsCheckResult.IsSuccess); + if (!expectedError) + { + Assert.True(definitionsCheckResult.IsSuccess); var result = recalcEngine.Eval(expression, options: _opts); var fvExpected = FormulaValue.New(expected); @@ -724,136 +724,136 @@ public void ImperativeUserDefinedFunctionTest(string udfExpression, string expre } else { - Assert.False(definitionsCheckResult.IsSuccess); + Assert.False(definitionsCheckResult.IsSuccess); Assert.Single(definitionsCheckResult.Errors); Assert.Contains(definitionsCheckResult.Errors, err => err.MessageKey == errorKey); } - } - - [Theory] - - [InlineData( + } + + [Theory] + + [InlineData( "MismatchType():Number = { 1+3; Color.Blue; };", true, - true, - 36, - 41)] - [InlineData( + true, + 36, + 41)] + [InlineData( "MatchType():Text = { 4; 3 };", false, - true, - 0, + true, + 0, 0)] public void TestMismatchReturnType(string udfExpression, bool expectedError, bool allowSideEffects, int min, int lim) { var config = new PowerFxConfig(); config.EnableSetFunction(); var engine = new RecalcEngine(config); - engine.UpdateVariable("x", 1m); - - var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); - Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); - - if (expectedError) - { - var error = result.Errors.First(error => error.MessageKey == "ErrUDF_ReturnTypeDoesNotMatch"); - Assert.NotNull(error); - var span = error.Span; - Assert.True(span.Min == min && span.Lim == lim); + engine.UpdateVariable("x", 1m); + + var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects); + Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0); + + if (expectedError) + { + var error = result.Errors.First(error => error.MessageKey == "ErrUDF_ReturnTypeDoesNotMatch"); + Assert.NotNull(error); + var span = error.Span; + Assert.True(span.Min == min && span.Lim == lim); } - } - + } + [Fact] public void DelegableUDFTest() - { - var symbolTable = new DelegatableSymbolTable(); - var schema = DType.CreateTable( - new TypedName(DType.Guid, new DName("ID")), + { + var symbolTable = new DelegatableSymbolTable(); + var schema = DType.CreateTable( + new TypedName(DType.Guid, new DName("ID")), new TypedName(DType.Number, new DName("Value"))); - symbolTable.AddEntity(new TestDelegableDataSource( - "MyDataSource", - schema, - new TestDelegationMetadata( - new DelegationCapability(DelegationCapability.Filter), - schema, - new FilterOpMetadata( - schema, - new Dictionary(), - new Dictionary(), - new DelegationCapability(DelegationCapability.GreaterThan), - null)), - true)); + symbolTable.AddEntity(new TestDelegableDataSource( + "MyDataSource", + schema, + new TestDelegationMetadata( + new DelegationCapability(DelegationCapability.Filter), + schema, + new FilterOpMetadata( + schema, + new Dictionary(), + new Dictionary(), + new DelegationCapability(DelegationCapability.GreaterThan), + null)), + true)); symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema)); - var config = new PowerFxConfig() - { + var config = new PowerFxConfig() + { SymbolTable = symbolTable }; - config.EnableSetFunction(); - - var recalcEngine = new RecalcEngine(config); - - recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); - var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("C").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("B").First() as UserDefinedFunction; - - Assert.True(func.IsAsync); - Assert.True(func.IsDelegatable); - - func = recalcEngine.Functions.WithName("D").First() as UserDefinedFunction; - - // Imperative function is not delegable - Assert.True(func.IsAsync); - Assert.True(!func.IsDelegatable); - - // Binding fails for recursive definitions and hence function is not added. - Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess); - } - - [Fact] + config.EnableSetFunction(); + + var recalcEngine = new RecalcEngine(config); + + recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true); + var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("C").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("B").First() as UserDefinedFunction; + + Assert.True(func.IsAsync); + Assert.True(func.IsDelegatable); + + func = recalcEngine.Functions.WithName("D").First() as UserDefinedFunction; + + // Imperative function is not delegable + Assert.True(func.IsAsync); + Assert.True(!func.IsDelegatable); + + // Binding fails for recursive definitions and hence function is not added. + Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess); + } + + [Fact] public void TestInheritanceOfDelegationWarningsInUDFs() - { - var symbolTable = new DelegatableSymbolTable(); - var schema = DType.CreateTable( + { + var symbolTable = new DelegatableSymbolTable(); + var schema = DType.CreateTable( new TypedName(DType.Number, new DName("Value"))); - symbolTable.AddEntity(new TestDelegableDataSource( - "MyDataSource", - schema, - new TestDelegationMetadata( - new DelegationCapability(DelegationCapability.Filter), - schema, - new FilterOpMetadata( - schema, - new Dictionary(), - new Dictionary(), - new DelegationCapability(DelegationCapability.GreaterThan), - null)), - true)); + symbolTable.AddEntity(new TestDelegableDataSource( + "MyDataSource", + schema, + new TestDelegationMetadata( + new DelegationCapability(DelegationCapability.Filter), + schema, + new FilterOpMetadata( + schema, + new Dictionary(), + new Dictionary(), + new DelegationCapability(DelegationCapability.GreaterThan), + null)), + true)); symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema)); - var config = new PowerFxConfig() - { + var config = new PowerFxConfig() + { SymbolTable = symbolTable }; - var engine = new RecalcEngine(config); - - var result = engine.AddUserDefinedFunction("NonDelegatableUDF():MyDataSourceTableType = Filter(MyDataSource, Sqrt(Value) > 5);", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); - Assert.True(result.IsSuccess); - var func = engine.Functions.WithName("NonDelegatableUDF").First() as UserDefinedFunction; - Assert.True(func.HasDelegationWarning); + var engine = new RecalcEngine(config); - result = engine.AddUserDefinedFunction("NonDelegatableUDF2():MyDataSourceTableType = NonDelegatableUDF();", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); - Assert.True(result.IsSuccess); - func = engine.Functions.WithName("NonDelegatableUDF2").First() as UserDefinedFunction; - Assert.True(func.HasDelegationWarning); + var result = engine.AddUserDefinedFunction("NonDelegatableUDF():MyDataSourceTableType = Filter(MyDataSource, Sqrt(Value) > 5);", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + var func = engine.Functions.WithName("NonDelegatableUDF").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); + + result = engine.AddUserDefinedFunction("NonDelegatableUDF2():MyDataSourceTableType = NonDelegatableUDF();", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + func = engine.Functions.WithName("NonDelegatableUDF2").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); } // Binding to inner functions does not impact outer functions. @@ -1768,68 +1768,68 @@ public void LookupBuiltinOptionSets() Assert.True(ok); // Wrong type: https://github.com/microsoft/Power-Fx/issues/2342 - } - - [Fact] - public void LazyJsonTest() - { - bool LazyGetField1(string name, out FormulaType type) - { - type = name switch - { - "field1" => FormulaType.Decimal, - "field2" => FormulaType.String, - _ => FormulaType.Blank, - }; - - return type != FormulaType.Blank; - } - - PowerFxConfig config = new PowerFxConfig(); - config.EnableJsonFunctions(); - - RecalcEngine engine = new RecalcEngine(config); - SymbolTable symbolTable = new SymbolTable(); - TestLazyRecordType lazyRecordType = new TestLazyRecordType("test", new List() { "field1", "field2" }, LazyGetField1); - - ISymbolSlot slot = symbolTable.AddVariable("var1", lazyRecordType); - - CheckResult checkResult = engine.Check("JSON(var1)", symbolTable: symbolTable); - Assert.True(checkResult.IsSuccess, string.Join(", ", checkResult.Errors.Select(err => err.Message))); - - SymbolValues symbolValues = new SymbolValues(symbolTable); - symbolValues.Set(slot, new LazyRecordValue(lazyRecordType)); - - FormulaValue formulaValue = checkResult.GetEvaluator().Eval(symbolValues); - StringValue stringValue = Assert.IsType(formulaValue); - - Assert.Equal(@"{""field1"":10,""field2"":""Str""}", stringValue.Value); - } - - public class LazyRecordValue : RecordValue - { - public LazyRecordValue(RecordType type) - : base(type) - { - } - - protected override bool TryGetField(FormulaType fieldType, string fieldName, out FormulaValue result) - { - if (fieldName == "field1") - { - result = FormulaValue.New(10); - return true; - } - - if (fieldName == "field2") - { - result = FormulaValue.New("Str"); - return true; - } - - result = null; - return false; - } + } + + [Fact] + public void LazyJsonTest() + { + bool LazyGetField1(string name, out FormulaType type) + { + type = name switch + { + "field1" => FormulaType.Decimal, + "field2" => FormulaType.String, + _ => FormulaType.Blank, + }; + + return type != FormulaType.Blank; + } + + PowerFxConfig config = new PowerFxConfig(); + config.EnableJsonFunctions(); + + RecalcEngine engine = new RecalcEngine(config); + SymbolTable symbolTable = new SymbolTable(); + TestLazyRecordType lazyRecordType = new TestLazyRecordType("test", new List() { "field1", "field2" }, LazyGetField1); + + ISymbolSlot slot = symbolTable.AddVariable("var1", lazyRecordType); + + CheckResult checkResult = engine.Check("JSON(var1)", symbolTable: symbolTable); + Assert.True(checkResult.IsSuccess, string.Join(", ", checkResult.Errors.Select(err => err.Message))); + + SymbolValues symbolValues = new SymbolValues(symbolTable); + symbolValues.Set(slot, new LazyRecordValue(lazyRecordType)); + + FormulaValue formulaValue = checkResult.GetEvaluator().Eval(symbolValues); + StringValue stringValue = Assert.IsType(formulaValue); + + Assert.Equal(@"{""field1"":10,""field2"":""Str""}", stringValue.Value); + } + + public class LazyRecordValue : RecordValue + { + public LazyRecordValue(RecordType type) + : base(type) + { + } + + protected override bool TryGetField(FormulaType fieldType, string fieldName, out FormulaValue result) + { + if (fieldName == "field1") + { + result = FormulaValue.New(10); + return true; + } + + if (fieldName == "field2") + { + result = FormulaValue.New("Str"); + return true; + } + + result = null; + return false; + } } [Theory] @@ -1884,7 +1884,7 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out hasNoAge(e: Employee): Number = IsBlank(getAge(e));", "hasNoAge({Name: \"Bob\", Title: \"CEO\"})", true, - 1.0)] + 1.0)] // Types with UDF restricted primitive types resolve successfully [InlineData( @@ -1944,8 +1944,8 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out "f():TestEntity = Entity; g(e: TestEntity):Number = 1;", "g(f())", true, - 1.0)] - + 1.0)] + // Aggregate types with more than expected fields are not allowed in UDF [InlineData( "f():T = {x: 5, y: 5}; T := Type({x: Number});", @@ -1961,7 +1961,7 @@ public void UserDefinedTypeTest(string userDefinitions, string evalExpression, b var recalcEngine = new RecalcEngine(config); var parserOptions = new ParserOptions() { - AllowsSideEffects = false, + AllowsSideEffects = false, }; if (isValid) @@ -1975,30 +1975,30 @@ public void UserDefinedTypeTest(string userDefinitions, string evalExpression, b } else { - Assert.ThrowsAny(() => - { + Assert.ThrowsAny(() => + { recalcEngine.AddUserDefinitions(userDefinitions, CultureInfo.InvariantCulture); recalcEngine.Eval(evalExpression, options: parserOptions); }); } - } - - [Theory] + } + + [Theory] [InlineData( "F():Point = {x: 5, y:5};", "F().x", - true, + true, 5.0)] [InlineData( "F():Number = { Sqrt(1); 42; };", "F()", - true, - 42.0)] + true, + 42.0)] [InlineData( "F():Point = { {x:0, y:1729}; };", "F().y", - true, - 1729.0)] + true, + 1729.0)] [InlineData( "F():Point = {x: 5, 42};", "F().x", @@ -2010,9 +2010,9 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression var parserOptions = new ParserOptions() { AllowsSideEffects = true, - }; - - var extraSymbols = new SymbolTable(); + }; + + var extraSymbols = new SymbolTable(); extraSymbols.AddType(new DName("Point"), FormulaType.Build(TestUtils.DT("![x:n, y:n]"))); if (isValid) @@ -2024,117 +2024,117 @@ public void UDFImperativeVsRecordAmbiguityTest(string udf, string evalExpression { Assert.False(recalcEngine.AddUserDefinedFunction(udf, CultureInfo.InvariantCulture, extraSymbols, true).IsSuccess); } - } - - [Theory] + } + + [Theory] [InlineData( "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points)); distance(a: Point, b: Point): Number = Sqrt(Power(b.x-a.x, 2) + Power(b.y-a.y, 2));", "distance({x: 0, y: 0}, {x: 0, y: 5})", true, - 5.0)] + 5.0)] [InlineData( "Account := Type(RecordOf(Accounts)); getAccountId(a: Account): Number = a.id;", "getAccountId({id: 42, name: \"T-Rex\", address: \"Museum\"})", true, - 42.0)] + 42.0)] [InlineData( "Account := Type(RecordOf(Accounts)); FirstAccount(a: Accounts): Account = First(a); getAccountId(a: Account): Number = a.id;", "getAccountId(FirstAccount([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}]))", true, - 1729.0)] + 1729.0)] [InlineData( "NewAccounts := Type([RecordOf(Accounts)]); getFirstAccountId(a: NewAccounts): Number = First(a).id;", "getFirstAccountId([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}])", true, - 1729.0)] + 1729.0)] [InlineData( "AccountWithAge := Type({age: Number, acc: RecordOf(Accounts)}); getAccountAge(a: AccountWithAge): Number = a.age;", "getAccountAge({age : 25, acc:First([{id: 1729, name: \"Bob\"}, {id: 42, name: \"T-Rex\"}])})", true, - 25.0)] + 25.0)] [InlineData( "Points := Type([{x : Number, y : Number}]); ComplexType := Type({p: RecordOf(Points), a: RecordOf(Accounts), s: SomeRecord}); getX(c: ComplexType): Number = c.p.x;", "getX({s: {id: 1729, name: \"Bob\"}, p : {x: 1, y: 2}, a: {id: 42, name: \"Alice\", address: \"internet\"}})", true, - 1.0)] - - // No error on a UDF named RecordOf + 1.0)] + + // No error on a UDF named RecordOf [InlineData( @"RecordOf(x:Number): Number = x + 1;", "RecordOf(41)", true, - 42.0)] - - // Fails for any type other than table - [InlineData( - "Account := Type(RecordOf(SomeRecord));", - "", - false)] - [InlineData( - "Account := Type(RecordOf(Void));", - "", - false)] - [InlineData( - "Account := Type(RecordOf(UntypedObject));", - "", - false)] - - // invalid helper name + 42.0)] + + // Fails for any type other than table + [InlineData( + "Account := Type(RecordOf(SomeRecord));", + "", + false)] + [InlineData( + "Account := Type(RecordOf(Void));", + "", + false)] + [InlineData( + "Account := Type(RecordOf(UntypedObject));", + "", + false)] + + // invalid helper name [InlineData( "Account := Type(Recordof(Accounts));", "", - false)] + false)] [InlineData( "Account := Type(recordOf(Accounts));", "", - false)] + false)] [InlineData( "Account := Type(First(Accounts));", "", - false)] - - // Does not allow anything other firstname + false)] + + // Does not allow anything other firstname [InlineData( "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points As P));", "", - false)] + false)] [InlineData( "Points := Type([{x : Number, y : Number}]); Point := Type(RecordOf(Points, Accounts));", "", - false)] + false)] [InlineData( "Account := Type((Accounts, SomeRecord));", "", - false)] + false)] [InlineData( "Point := Type(RecordOf([{x : Number, y : Number}]));", "", - false)] + false)] [InlineData( "T1 := Type(RecordOf(Type([{A:Number}])));", "", - false)] + false)] [InlineData( "T1 := Type(RecordOf(RecordOf([{x:Number, y:Number}])));", "", - false)] - - // RecordOf not in type literal + false)] + + // RecordOf not in type literal [InlineData( "Account = RecordOf(Accounts);", "", - false)] + false)] [InlineData( "F():Accounts = RecordOf(Accounts);", "", - false)] + false)] public void RecordOfTests(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) { var config = new PowerFxConfig(); var recalcEngine = new RecalcEngine(config); - var parserOptions = new ParserOptions(); - - recalcEngine.Config.SymbolTable.AddType(new DName("Accounts"), FormulaType.Build(TestUtils.DT("*[id: n, name:s, address:s]"))); + var parserOptions = new ParserOptions(); + + recalcEngine.Config.SymbolTable.AddType(new DName("Accounts"), FormulaType.Build(TestUtils.DT("*[id: n, name:s, address:s]"))); recalcEngine.Config.SymbolTable.AddType(new DName("SomeRecord"), FormulaType.Build(TestUtils.DT("![id: n, name:s]"))); if (isValid) From 7cbae1a4fdedbdd72035b70f60b93a0d5802dea8 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Fri, 13 Dec 2024 16:21:29 -0800 Subject: [PATCH 17/20] Better error reporting in REPL --- .../RecalcEngine.cs | 7 +++- src/libraries/Microsoft.PowerFx.Repl/Repl.cs | 34 ++++++++++++------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/RecalcEngine.cs b/src/libraries/Microsoft.PowerFx.Interpreter/RecalcEngine.cs index f85f9ad6e4..1189465b3e 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/RecalcEngine.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/RecalcEngine.cs @@ -386,6 +386,11 @@ public void UpdateSupportedFunctions(SymbolTable s) SupportedFunctions = s; } + public ReadOnlySymbolTable GetAllSymbols() + { + return SymbolTable.Compose(Config.ComposedConfigSymbols, SupportedFunctions, _symbolTable, PrimitiveTypes); + } + /// /// Add a set of user-defined formulas and functions to the engine. /// @@ -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()) { diff --git a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs index 19d1e81411..a5940da024 100644 --- a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs +++ b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs @@ -510,26 +510,36 @@ await this.Output.WriteLineAsync($"Error: Can't set '{name}' to a Void value.", if (definitionsCheckResult.IsSuccess) { - try - { - this.Engine.AddUserDefinitions(expression, this.ParserOptions.Culture); - } - catch (Exception ex) + definitionsCheckResult.SetBindingInfo(this.Engine.GetAllSymbols()); + + if (definitionsCheckResult.ApplyErrors().Any()) { - await this.Output.WriteLineAsync(lineError + ex.Message, OutputKind.Error, cancel) - .ConfigureAwait(false); + 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(); } - foreach (var error in definitionsCheckResult.Errors) + if (check.ApplyParse().Errors.Any() && definitionsCheckResult.Errors.Any()) { - var kind = error.IsWarning ? OutputKind.Warning : OutputKind.Error; - var msg = error.ToString(); + 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); + await this.Output.WriteLineAsync(lineError + msg, kind, cancel) + .ConfigureAwait(false); + } } foreach (var error in check.Errors) From 2d85c9ff1c2edb1f12492ab2a367b0a733a16fb9 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Tue, 17 Dec 2024 11:55:10 -0800 Subject: [PATCH 18/20] try add types with displaynames --- .../Microsoft.PowerFx.Core/Public/Types/RecordType.cs | 5 +++++ src/libraries/Microsoft.PowerFx.Repl/Repl.cs | 1 + src/tools/Repl/Program.cs | 10 ++++++++++ 3 files changed, 16 insertions(+) diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Types/RecordType.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Types/RecordType.cs index 72385290c9..0cf8c673f4 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Types/RecordType.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Types/RecordType.cs @@ -177,5 +177,10 @@ internal override void DefaultExpressionValue(StringBuilder sb) sb.Append("}"); } + + public void SetFieldsOptional() + { + _type.AreFieldsOptional = true; + } } } diff --git a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs index a5940da024..9d9a7d1f7a 100644 --- a/src/libraries/Microsoft.PowerFx.Repl/Repl.cs +++ b/src/libraries/Microsoft.PowerFx.Repl/Repl.cs @@ -522,6 +522,7 @@ await this.Output.WriteLineAsync($"Error: Can't set '{name}' to a Void value.", await this.Output.WriteLineAsync(lineError + msg, kind, cancel) .ConfigureAwait(false); } + return new ReplResult(); } diff --git a/src/tools/Repl/Program.cs b/src/tools/Repl/Program.cs index 116e3db275..16f5183838 100644 --- a/src/tools/Repl/Program.cs +++ b/src/tools/Repl/Program.cs @@ -98,6 +98,16 @@ private static RecalcEngine ReplRecalcEngine() config.AddFunction(new Run1Function()); config.AddFunction(new Run2Function()); + var testRecordType = RecordType.Empty() + .Add(new NamedFormulaType("name", FormulaType.String, "Name")) + .Add(new NamedFormulaType("age", FormulaType.Number, "Age")); + + testRecordType + .SetFieldsOptional(); + + config.SymbolTable.AddType(new Core.Utils.DName("TestRecord"), testRecordType); + config.SymbolTable.AddType(new Core.Utils.DName("TestTable"), testRecordType.ToTable()); + var optionsSet = new OptionSet("Options", DisplayNameUtility.MakeUnique(options)); config.EnableRegExFunctions(new TimeSpan(0, 0, 5)); From 422a9b9cd32c8b9ca96c41da03240577520cc191 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Tue, 7 Jan 2025 10:06:41 -0800 Subject: [PATCH 19/20] Add Tests --- .../Functions/UserDefinedFunction.cs | 9 +++- .../Microsoft.PowerFx.Core/Types/DType.cs | 9 ++-- .../UserDefinedTypeTests.cs | 14 +++++ .../RecalcEngineTests.cs | 52 +++++++++++++++---- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs index 23acd4ebde..939db68a88 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs @@ -191,9 +191,14 @@ 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, true)) + if (!ReturnType.Accepts( + actualBodyReturnType, + exact: true, + useLegacyDateTimeAccepts: false, + usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules, + restrictiveAggregateTypes: true)) { - if (actualBodyReturnType.CoercesTo(ReturnType, true, false, context.Features, true)) + if (actualBodyReturnType.CoercesTo(ReturnType, true, false, context.Features, restrictiveAggregateTypes: true)) { _binding.SetCoercedType(binding.Top, ReturnType); } diff --git a/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs b/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs index 2bfbd65ae5..5b9cad8e3f 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Types/DType.cs @@ -1851,7 +1851,7 @@ private bool AcceptsEntityType(DType type, bool usePowerFxV1CompatibilityRules) /// Legacy rules for accepting date/time types. /// Use PFx v1 compatibility rules if enabled (less /// permissive Accepts relationships). - /// restrictiveAggregateTypes. + /// Flag to restrict using aggregate types with more fields than expected. /// /// True if accepts , false otherwise. /// @@ -1886,6 +1886,7 @@ public bool Accepts(DType type, bool exact, bool useLegacyDateTimeAccepts, bool /// Legacy rules for accepting date/time types. /// Use PFx v1 compatibility rules if enabled (less /// permissive Accepts relationships). + /// Flag to restrict using aggregate types with more fields than expected. /// /// True if accepts , false otherwise. /// @@ -3208,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) @@ -3243,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); diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs index 89e951ca6e..880e9ac56f 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/UserDefinedTypeTests.cs @@ -156,6 +156,20 @@ public void TestRecordOfErrors(string typeDefinition, string expectedMessageKey) Assert.Contains(errors, e => e.MessageKey.Contains(expectedMessageKey)); } + [Theory] + [InlineData("f():T = {x: 5, y: 5}; T := Type({x: Number});", "ErrUDF_ReturnTypeSchemaDoesNotMatch")] + [InlineData("f(x:T):Number = x.n; T := Type({n: Number}); g(): Number = f({n: 5, m: 5});", "ErrBadSchema_ExpectedType")] + [InlineData("f():T = [{x: 5, y: 5}]; T := Type([{x: Number}]);", "ErrUDF_ReturnTypeSchemaDoesNotMatch")] + [InlineData("f(x:T):T = x; T := Type([{n: Number}]); g(): T = f([{n: 5, m: 5}]);", "ErrBadSchema_ExpectedType")] + public void TestAggregateTypeErrors(string typeDefinition, string expectedMessageKey) + { + var checkResult = new DefinitionsCheckResult() + .SetText(typeDefinition) + .SetBindingInfo(_primitiveTypes); + var errors = checkResult.ApplyErrors(); + Assert.Contains(errors, e => e.MessageKey.Contains(expectedMessageKey)); + } + [Theory] [InlineData("T := Type({ x: 5+5, y: -5 });", 2)] [InlineData("T := Type(Type(Number));", 1)] diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index f0873041d2..44ffb9aeae 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -1871,13 +1871,6 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out "getAge({Name: \"Bob\", Age: 21})", true, 21.0)] - [InlineData( - @"Employee := Type({Name: Text, Age: Number, Title: Text}); Employees := Type([Employee]); EmployeeNames := Type([{Name: Text}]); - getNames(e: Employees):EmployeeNames = ShowColumns(e, Name); - getNamesCount(e: EmployeeNames):Number = CountRows(getNames(e));", - "getNamesCount([{Name: \"Jim\", Age:25}, {Name: \"Tony\", Age:42}])", - true, - 2.0)] [InlineData( @"Employee := Type({Name: Text, Age: Number, Title: Text}); getAge(e: Employee): Number = e.Age; @@ -1946,14 +1939,55 @@ protected override bool TryGetField(FormulaType fieldType, string fieldName, out true, 1.0)] - // Aggregate types with more than expected fields are not allowed in UDF + // Aggregate types with more than expected fields are not allowed in UDF args and return types + // Records [InlineData( "f():T = {x: 5, y: 5}; T := Type({x: Number});", - "f().x", + "", + false)] + [InlineData( + "f():T = {x: 5, y: 5}; T1 := Type([{x: Number}]); T2 := Type(RecordOf(T1));", + "", + false)] + [InlineData( + "g(x:T):Number = x.n; T := Type({n: Number});", + "g({x: 5, y: 5})", + false)] + + // Nested Records + [InlineData( + "f():T = {a: 5, b: {c: {d: 5, e:42}}}; T := Type({a: Number, b: {c: {d: Number}}});", + "", + false)] + [InlineData( + "g(x:T):Number = x.b.c.d; T := Type({a: Number, b: {c: {d: Number}}});", + "g({a: 5, b: {c: {d: 5, e:42}}})", + false)] + + // Tables + [InlineData( + "f():T = [{x: 5, y: 5}]; T := Type([{x: Number}]);", + "", false)] [InlineData( "People := Type([{Name: Text, Age: Number}]); countMinors(p: People): Number = CountRows(Filter(p, Age < 18));", "countMinors([{Name: \"Bob\", Age: 21, Title: \"Engineer\"}, {Name: \"Alice\", Age: 25, Title: \"Manager\"}])", + false)] + [InlineData( + @"Employee := Type({Name: Text, Age: Number, Title: Text}); Employees := Type([Employee]); EmployeeNames := Type([{Name: Text}]); + getNames(e: Employees):EmployeeNames = ShowColumns(e, Name); + getNamesCount(e: EmployeeNames):Number = CountRows(getNames(e));", + "getNamesCount([{Name: \"Jim\", Age:25}, {Name: \"Tony\", Age:42}])", + false)] + + // Nested Tables + [InlineData( + "f():T = {a: 5, b: [{c: {d: 5, e:42}}]}; T := Type([{a: Number, b: [{c: {d: Number}}]}]);", + "", + false)] + [InlineData( + "g(x:T):Number = First(First(x).b).c.d; T := Type([{a: Number, b: [{c: {d: Number}}]}])", + "g({a: 5, b: [{c: {d: 5, e:42}}]})", false)] public void UserDefinedTypeTest(string userDefinitions, string evalExpression, bool isValid, double expectedResult = 0) { From 9bd29bf9ab3ce555186e9fba67954b3f80bf3e82 Mon Sep 17 00:00:00 2001 From: Adithya Selvaprithiviraj Date: Thu, 9 Jan 2025 21:37:50 -0800 Subject: [PATCH 20/20] pass feature to udf bind --- .../Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs | 2 +- .../Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs index 88c02a23b0..2dc00942ce 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs @@ -180,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 bindErrors = new List(); diff --git a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs index 477e1534e2..f3d67003e9 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs @@ -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;