From 46a1bfc11a173fd22c04f3b0b2ae7a55b72e6368 Mon Sep 17 00:00:00 2001 From: Anderson Silva Date: Tue, 3 Dec 2024 18:55:01 -0600 Subject: [PATCH] Join suggestions and error span (#2766) - Fixing span information for error messages. - Scope fields suggestions. --- .../Functions/TexlFunction.cs | 3 ++ .../Texl/Builtins/Join.cs | 19 ++++++++----- .../Texl/Intellisense/ArgumentSuggestions.cs | 28 +++++++++++++++++-- .../Functions/LibraryTable.cs | 2 ++ .../ExpressionTestCases/Join.txt | 18 ++++++------ .../IntellisenseTests/SuggestTest.cs | 9 +++++- .../MutationScripts/Join.txt | 4 +-- 7 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs index c928971a07..5778346a41 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs @@ -242,6 +242,9 @@ public virtual bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) /// Indicates whether table and record param types require all columns to be specified in the input argument. public virtual bool RequireAllParamColumns => false; + // Indicates the base type of the function. The base type can differ if the function extends multiple base classes i.e. Join function. + public virtual Type DeclarationType => this.GetType(); + /// /// Indicates whether the function will propagate the mutability of its first argument. /// For example, if x is a mutable reference (i.e., a variable), then First(x) will still diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs index 15e6cc376a..4ed7a644d3 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs @@ -80,21 +80,21 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp { if (node is not AsNode asNode) { - errors.EnsureError(DocumentErrorSeverity.Severe, args[0], TexlStrings.ErrJoinArgIsNotAsNode); + errors.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrJoinArgIsNotAsNode); valid = false; break; } if (asNode.Left is not DottedNameNode dottedNameNode) { - errors.EnsureError(DocumentErrorSeverity.Severe, args[0], TexlStrings.ErrJoinArgIsNotAsNode); + errors.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrJoinArgIsNotAsNode); valid = false; break; } if (dottedNameNode.Left is not FirstNameNode firstNameNode) { - errors.EnsureError(DocumentErrorSeverity.Severe, args[0], TexlStrings.ErrJoinDottedNameleft, dottedNameNode.Left, scopeIdent[0], scopeIdent[1]); + errors.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrJoinDottedNameleft, dottedNameNode.Left, scopeIdent[0], scopeIdent[1]); valid = false; break; } @@ -103,7 +103,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp { if (!leftTable.TryGetType(dottedNameNode.Right.Name, out var leftType)) { - errors.EnsureError(DocumentErrorSeverity.Severe, args[0], TexlStrings.ErrColDNE_Name, dottedNameNode); + errors.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrColDNE_Name, dottedNameNode); valid = false; break; } @@ -115,7 +115,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp { if (!rightTable.TryGetType(dottedNameNode.Right.Name, out var rightType)) { - errors.EnsureError(DocumentErrorSeverity.Severe, args[0], TexlStrings.ErrColDNE_Name, dottedNameNode); + errors.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrColDNE_Name, dottedNameNode); valid = false; break; } @@ -132,7 +132,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp if (fError) { - errors.EnsureError(DocumentErrorSeverity.Severe, args[0], TexlStrings.ErrJoinCantAddRename, node); + errors.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.ErrJoinCantAddRename, node); valid = false; break; } @@ -150,7 +150,12 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp public override bool IsLambdaParam(TexlNode node, int index) { - return index == 2 || (index > 3 && node is AsNode); + return index == 2 || index > 3; + } + + public override bool HasSuggestionsForParam(int argumentIndex) + { + return argumentIndex == 3; } public override IEnumerable GetRequiredEnumNames() diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Intellisense/ArgumentSuggestions.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Intellisense/ArgumentSuggestions.cs index ea4b57f96e..436be9a4d7 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Intellisense/ArgumentSuggestions.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Intellisense/ArgumentSuggestions.cs @@ -39,7 +39,8 @@ internal static class ArgumentSuggestions { typeof(SplitFunction), DiscardEnumParam(StringTypeSuggestions) }, { typeof(StartsWithFunction), DiscardEnumParam(StringTypeSuggestions) }, { typeof(TextFunction), TextSuggestions }, - { typeof(ValueFunction), LanguageCodeSuggestion } + { typeof(ValueFunction), LanguageCodeSuggestion }, + { typeof(JoinFunction), JoinSuggestion }, }, isThreadSafe: true); public static IEnumerable> GetArgumentSuggestions(IntellisenseData.IntellisenseData intellisenseData, TryGetEnumSymbol tryGetEnumSymbol, bool suggestUnqualifiedEnums, TexlFunction function, DType scopeType, int argumentIndex, out bool requiresSuggestionEscaping) @@ -49,7 +50,7 @@ public static IEnumerable> GetArgumentSuggestions(In return NamedTypeSuggestions(intellisenseData, function, argumentIndex, out requiresSuggestionEscaping); } - if (CustomFunctionSuggestionProviders.Value.TryGetValue(function.GetType(), out var suggestor)) + if (CustomFunctionSuggestionProviders.Value.TryGetValue(function.DeclarationType, out var suggestor)) { return suggestor(tryGetEnumSymbol, suggestUnqualifiedEnums, scopeType, argumentIndex, out requiresSuggestionEscaping); } @@ -129,6 +130,29 @@ private static IEnumerable> TextSuggestions(TryGetEn } } + private static IEnumerable> JoinSuggestion(TryGetEnumSymbol tryGetEnumSymbol, bool suggestUnqualifedEnums, DType scopeType, int argumentIndex, out bool requiresSuggestionEscaping) + { + Contracts.Assert(scopeType.IsValid); + Contracts.Assert(argumentIndex >= 0); + + requiresSuggestionEscaping = false; + tryGetEnumSymbol(LanguageConstants.JoinTypeEnumString, out var enumInfo); + + if (argumentIndex == 3) + { + var retVal = new List>(); + + foreach (var name in enumInfo.EnumType.GetNames(DPath.Root)) + { + retVal.Add(new KeyValuePair(enumInfo.EntityName.Value + TexlLexer.PunctuatorDot + name.Name.Value, name.Type)); + } + + return retVal; + } + + return EnumerableUtils.Yield>(); + } + /// /// Cached list of language code suggestions. /// diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs index d3ce3816ba..97bc51faff 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs @@ -1573,6 +1573,8 @@ public static NamedLambda[] Parse(FormulaValue[] args) internal class JoinImpl : JoinFunction, IAsyncTexlFunctionJoin #pragma warning restore CS0618 // Type or member is obsolete { + public override Type DeclarationType => typeof(JoinFunction); + public async Task InvokeAsync(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args) { return await Library.JoinTables(runner, context, irContext, args).ConfigureAwait(false); diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Join.txt b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Join.txt index bed8db7946..724a898d31 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Join.txt +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Join.txt @@ -26,16 +26,16 @@ Errors: Error 98-121: Expected plain 'JoinType' enum value. Different enums or p Table(Error({Kind:ErrorKind.Div0}),Error({Kind:ErrorKind.Div0}),Error({Kind:ErrorKind.Div0}),Error({Kind:ErrorKind.Div0})) >> Join(Table({a:1}),Table({b:1}),LeftRecord.a = RightRecord.b, JoinType.Inner, LeftRecord As Abc) -Errors: Error 5-17: Arguments starting from the 5th position are meant to rename columns and are expected to be 'As' node arguments i.e LeftRecord.Name As NewName.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 88-90: Arguments starting from the 5th position are meant to rename columns and are expected to be 'As' node arguments i.e LeftRecord.Name As NewName.|Error 5-17: At least one 'RightRecord' column must be declared to compose the join result.|Error 0-4: The function 'Join' has some invalid arguments. >> Join(Table({a:1}),Table({b:1}),LeftRecord.a = RightRecord.b, JoinType.Inner, LeftRecord.Void As Abc) -Errors: Error 87-92: Name isn't valid. 'Void' isn't recognized.|Error 93-95: Invalid argument type.|Error 5-17: The specified column 'LeftRecord.Void' does not exist.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 87-92: Name isn't valid. 'Void' isn't recognized.|Error 93-95: Invalid argument type.|Error 5-17: At least one 'RightRecord' column must be declared to compose the join result.|Error 0-4: The function 'Join' has some invalid arguments. >> Join(Table({a:1}),Table({b:1}),LeftRecord.a = RightRecord.b, JoinType.Inner, Test.Void As Abc) Errors: Error 77-81: Name isn't valid. 'Test' isn't recognized.|Error 81-86: The '.' operator cannot be used on Error values.|Error 87-89: Invalid argument type.|Error 0-4: The function 'Join' has some invalid arguments. >> Join(Table({a:1}),Table({b:1}),LeftRecord.a = RightRecord.b, JoinType.Inner, LeftRecord.a As A1, RightRecord.b As A1) -Errors: Error 5-17: 'RightRecord.b As A1' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 111-113: 'RightRecord.b As A1' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. >> Join(If(1/0, Table({a:1})), Table({b:1}), LeftRecord.a = RightRecord.b,JoinType.Inner,RightRecord.b As b) Error({Kind:ErrorKind.Div0}) @@ -44,7 +44,7 @@ Error({Kind:ErrorKind.Div0}) Error({Kind:ErrorKind.Div0}) >> Join([{a:1}],[{a:1}], LeftRecord.a=RightRecord.a,JoinType.Inner, RightRecord.a As AA, LeftRecord.a) -Errors: Error 86-96: Name isn't valid. 'LeftRecord' isn't recognized.|Error 96-98: The '.' operator cannot be used on Error values.|Error 5-12: Arguments starting from the 5th position are meant to rename columns and are expected to be 'As' node arguments i.e LeftRecord.Name As NewName.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 96-98: Arguments starting from the 5th position are meant to rename columns and are expected to be 'As' node arguments i.e LeftRecord.Name As NewName.|Error 0-4: The function 'Join' has some invalid arguments. >> Join([{a:1,b:1}] As T1,[{a:1,c:99}] As T2, T1.a=T2.a,JoinType.Inner, T2.c As CC) Table({CC:99,a:1,b:1}) @@ -53,19 +53,19 @@ Table({CC:99,a:1,b:1}) Table() >> Join([{a:1}] As t1,[{a:1}] As t2, t1.a=t2.a, JoinType.Inner, t2.c As d) -Errors: Error 63-65: Name isn't valid. 'c' isn't recognized.|Error 66-68: Invalid argument type.|Error 13-15: The specified column 't2.c' does not exist.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 63-65: Name isn't valid. 'c' isn't recognized.|Error 66-68: Invalid argument type.|Error 13-15: At least one 't2' column must be declared to compose the join result.|Error 0-4: The function 'Join' has some invalid arguments. >> Join( [{a:1,b:2},{a:3,b:4}], [{a:1,c:5},{a:3,c:6}], LeftRecord.a = RightRecord.a, JoinType.Inner, RightRecord.c As c, LeftRecord.a As b ) -Errors: Error 6-27: 'LeftRecord.a As b' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 131-133: 'LeftRecord.a As b' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. >> Join( [{a:1,b:2},{a:3,b:4}], [{a:1,c:5},{a:3,c:6}], LeftRecord.a = RightRecord.a, JoinType.Inner, RightRecord.c As b ) -Errors: Error 6-27: 'RightRecord.c As b' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 112-114: 'RightRecord.c As b' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. >> Join(Table({a:1},{a:1},{a:0}),Table({b:1}),RightRecord.b/LeftRecord.a > 0,JoinType.Inner,RightRecord.b As b) Table({a:1,b:1},{a:1,b:1},Error({Kind:ErrorKind.Div0})) >> Join([{a:1}] As T1,[{a:1}] As T2,true,JoinType.Inner,{a:1}.a As AAA) -Errors: Error 13-15: The '{ a:1 }' value is invalid in this context. It should be a reference to either 'T1' or 'T2' scope name.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 61-63: The '{ a:1 }' value is invalid in this context. It should be a reference to either 'T1' or 'T2' scope name.|Error 13-15: At least one 'T2' column must be declared to compose the join result.|Error 0-4: The function 'Join' has some invalid arguments. >> Join([{a:1}],[{a:1}],true,JoinType.Inner,{a:1}.a As AAA) -Errors: Error 5-12: The '{ a:1 }' value is invalid in this context. It should be a reference to either 'LeftRecord' or 'RightRecord' scope name.|Error 0-4: The function 'Join' has some invalid arguments. \ No newline at end of file +Errors: Error 49-51: The '{ a:1 }' value is invalid in this context. It should be a reference to either 'LeftRecord' or 'RightRecord' scope name.|Error 5-12: At least one 'RightRecord' column must be declared to compose the join result.|Error 0-4: The function 'Join' has some invalid arguments. \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/IntellisenseTests/SuggestTest.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/IntellisenseTests/SuggestTest.cs index 2078bbe04a..ac243c30c2 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/IntellisenseTests/SuggestTest.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/IntellisenseTests/SuggestTest.cs @@ -213,9 +213,16 @@ public void TestSuggest(string expression, params string[] expectedSuggestions) // Suggests multiple scopes for functions that creates multiple scopes. [Theory] [InlineData("Join(Table({a:1,b:1}),Table({a:1,c:2,d:2}),|", "LeftRecord", "RightRecord")] + [InlineData("Join(Table({a:1,b:1}) As t1,Table({a:1,c:2,d:2}) As t2,|", "t1", "t2")] + [InlineData("Join(Table({a:1,b:1}),Table({a:1,c:2,d:2}),LeftRecord.|", "a", "b")] + [InlineData("Join(Table({a:1,b:1}),Table({a:1,c:2,d:2}),LeftRecord.a = RightRecord.|", "a", "c", "d")] + [InlineData("Join(Table({a:1,b:1}),Table({a:1,c:2,d:2}),LeftRecord.a = RightRecord.a,|", "JoinType.Full", "JoinType.Inner", "JoinType.Left", "JoinType.Right")] + [InlineData("Join(Table({a:1,b:1}),Table({a:1,c:2,d:2}),LeftRecord.a = RightRecord.a, JoinType.Inner,|", "LeftRecord", "RightRecord")] + [InlineData("Join(Table({a:1,b:1}),Table({a:1,c:2,d:2}),LeftRecord.a = RightRecord.a, JoinType.Inner,LeftRecord.|", "a", "b")] + [InlineData("Join(Table({a:1,b:1}),Table({a:1,c:2,d:2}),LeftRecord.a = RightRecord.a, JoinType.Inner,RightRecord.|", "a", "c", "d")] public void TestSuggestMultipleScopes(string expression, params string[] expectedSuggestions) { - var config = new PowerFxConfig(); + var config = Default; config.SymbolTable.AddFunction(new JoinFunction()); var actualSuggestions = SuggestStrings(expression, config); diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Join.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Join.txt index 3beca28266..302a13c223 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Join.txt +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Join.txt @@ -62,7 +62,7 @@ Table({Country:"USA",FruitName:"Grapes",SupId:"AAA111",SupName:"Contoso"},{Count // Renaming first, with duplicated column names >> Join(RenameColumns(t1, Id, SupplierId, Name, SupName), RenameColumns(t2, Fruit, FruitName), LeftRecord.SupplierId = RightRecord.SupplierId, JoinType.Inner, LeftRecord.SupplierId As RowId , RightRecord.RowId As RowId) -Errors: Error 5-53: 'RightRecord.RowId As RowId' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 207-209: 'RightRecord.RowId As RowId' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. >> Join(RenameColumns(t1, Id, SupplierId, Name, SupName) As x1, RenameColumns(t2, Fruit, FruitName) As x2, x1.SupplierId = x2.SupplierId, JoinType.Inner, x1.SupplierId As RowId , x2.RowId As RowId) -Errors: Error 54-56: 'x2.RowId As RowId' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments. +Errors: Error 185-187: 'x2.RowId As RowId' can not be added/renamed due to colission with another column with same name.|Error 0-4: The function 'Join' has some invalid arguments.