Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block at compile time invalid arguments to Distinct #2857

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal class BuiltinFunctionsCore
// included into the BuiltinFunctionsLibrary library. Examples: Set, Join.
internal static readonly IReadOnlyCollection<string> OtherKnownFunctions = new HashSet<string>()
{
"Assert", "Back", "Choices", "ClearData", "Concurrent", "Confirm", "Copy", "DataSourceInfo", "Defaults", "Disable", "Distinct", "Download", "EditForm", "Enable", "Errors", "Exit",
"Assert", "Back", "Choices", "ClearData", "Concurrent", "Confirm", "Copy", "DataSourceInfo", "Defaults", "Disable", "Download", "EditForm", "Enable", "Errors", "Exit",
"GroupBy", "HashTags", "IsMatch", "IsType", "Join", "JSON", "Launch", "LoadData", "Match", "MatchAll", "Navigate", "NewForm", "Notify", "PDF", "Param", "Pending", "Print", "ReadNFC",
"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",
Expand Down Expand Up @@ -102,7 +102,8 @@ internal class BuiltinFunctionsCore
public static readonly TexlFunction Dec2Hex = _library.Add(new Dec2HexFunction());
public static readonly TexlFunction Dec2HexT = _library.Add(new Dec2HexTFunction());
public static readonly TexlFunction Degrees = _library.Add(new DegreesFunction());
public static readonly TexlFunction DegreesT = _library.Add(new DegreesTableFunction());
public static readonly TexlFunction DegreesT = _library.Add(new DegreesTableFunction());
public static readonly TexlFunction Distinct = _library.Add(new DistinctFunction());
public static readonly TexlFunction DropColumns = _library.Add(new DropColumnsFunction());
public static readonly TexlFunction EDate = _library.Add(new EDateFunction());
public static readonly TexlFunction EOMonth = _library.Add(new EOMonthFunction());
Expand Down
22 changes: 20 additions & 2 deletions src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Distinct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.PowerFx.Core.Binding;
using Microsoft.PowerFx.Core.Binding.BindInfo;
using Microsoft.PowerFx.Core.Entities;
using Microsoft.PowerFx.Core.Errors;
using Microsoft.PowerFx.Core.Functions;
using Microsoft.PowerFx.Core.Functions.Delegation;
using Microsoft.PowerFx.Core.Functions.Delegation.DelegationMetadata;
Expand All @@ -31,7 +32,7 @@ internal sealed class DistinctFunction : FunctionWithTableInput
public DistinctFunction()
: base("Distinct", TexlStrings.AboutDistinct, FunctionCategories.Table, DType.EmptyTable, 0x02, 2, 2, DType.EmptyTable)
{
ScopeInfo = new FunctionScopeInfo(this);
ScopeInfo = new FunctionScopeInfo(this, usesAllFieldsInScope: false);
}

public override IEnumerable<TexlStrings.StringGetter[]> GetSignatures()
Expand All @@ -53,7 +54,24 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp

var exprType = argTypes[1];

returnType = DType.CreateTable(new TypedName(exprType, GetOneColumnTableResultName(context.Features)));
// Restricted supported types
if (context.Features.PowerFxV1CompatibilityRules && !(exprType == DType.String ||
exprType == DType.Number ||
exprType == DType.Decimal ||
exprType == DType.Boolean ||
exprType == DType.Time ||
exprType == DType.OptionSetValue ||
exprType == DType.DateTime ||
exprType == DType.Date ||
exprType == DType.Guid))
{
errors.EnsureError(DocumentErrorSeverity.Severe, args[1], TexlStrings.ErrNeedPrimitive);
fValid = false;
}
else
{
returnType = DType.CreateTable(new TypedName(exprType, GetOneColumnTableResultName(context.Features)));
}

return fValid;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ namespace Microsoft.PowerFx.Functions
{
internal static partial class Library
{
/// <summary>
/// This isn't part of <see cref="BuiltinFunctionsCore"/> since PA has different implementation of
/// Texl Instance of <see cref="DistinctFunction"/>.
/// </summary>
public static readonly TexlFunction DistinctInterpreterFunction = new DistinctFunction();

internal static readonly DateTime _epoch = new DateTime(1899, 12, 30, 0, 0, 0, 0);

// Helper to get a service or fallback to a default if the service is missing.
Expand Down Expand Up @@ -600,9 +594,9 @@ static Library()
targetFunction: Decimal_UO)
},
{
DistinctInterpreterFunction,
BuiltinFunctionsCore.Distinct,
StandardErrorHandlingAsync<FormulaValue>(
DistinctInterpreterFunction.Name,
BuiltinFunctionsCore.Distinct.Name,
expandArguments: NoArgExpansion,
replaceBlankValues: DoNotReplaceBlank,
checkRuntimeTypes: ExactSequence(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,3 @@ Table({Value:1},{Value:2},{Value:3},{Value:4})

>> Distinct([GUID("c203b79b-b985-42f0-b523-c10eb64387c6"), GUID("c203b79b-b985-42f0-b523-c10eb64387c6")], Value)
Table({Value:GUID("c203b79b-b985-42f0-b523-c10eb64387c6")})

>> Distinct(Table({a:1,b:2},{a:3,b:4},{a:5,b:6}), Blank())
Table({Value:Blank()})

// Distinct only supports Primitive types.
>> Distinct(Table({a:1,b:2},{a:3,b:4},{a:5,b:6}), Table({test: a * 2}))
Error({Kind:ErrorKind.InvalidArgument})

>> Distinct(Table({x:1,y:2},{x:10,y:2}), ThisRecord)
Error({Kind:ErrorKind.InvalidArgument})
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,13 @@ Errors: Error 9-16: Invalid argument type.|Error 0-8: The function 'Distinct' ha
// Untyped blanks are not allowed in the argument that defines the row scope
>> Distinct(Error("error"), true)
Errors: Error 9-23: Invalid argument type.|Error 0-8: The function 'Distinct' has some invalid arguments.

>> Distinct(Table({a:1,b:2},{a:3,b:4},{a:5,b:6}), Blank())
Errors: Error 47-54: Expected a text, numeric, boolean, or date/time value.|Error 0-8: The function 'Distinct' has some invalid arguments.

// Distinct only supports Primitive types.
>> Distinct(Table({a:1,b:2},{a:3,b:4},{a:5,b:6}), Table({test: a * 2}))
Errors: Error 47-67: Expected a text, numeric, boolean, or date/time value.|Error 0-8: The function 'Distinct' has some invalid arguments.

>> Distinct(Table({x:1,y:2},{x:10,y:2}), ThisRecord)
Errors: Error 38-48: Expected a text, numeric, boolean, or date/time value.|Error 0-8: The function 'Distinct' has some invalid arguments.
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,13 @@ Blank()
// Legacy behavior: untyped blanks are allowed in the argument that defines the row scope
>> Distinct(Error("error"), true)
Error({Kind:ErrorKind.Custom})

>> Distinct(Table({a:1,b:2},{a:3,b:4},{a:5,b:6}), Blank())
Table({Value:Blank()})

// Distinct only supports Primitive types.
>> Distinct(Table({a:1,b:2},{a:3,b:4},{a:5,b:6}), Table({test: a * 2}))
Error({Kind:ErrorKind.InvalidArgument})

>> Distinct(Table({x:1,y:2},{x:10,y:2}), ThisRecord)
Error({Kind:ErrorKind.InvalidArgument})
39 changes: 39 additions & 0 deletions src/tests/Microsoft.PowerFx.Core.Tests.Shared/TexlTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,45 @@ public void TexlFunctionTypeSemanticsFirstN()
symbol);
}

[Theory]
[InlineData("Distinct(Table, A)", "*[A:n, B:b, C:s]", "*[Value:n]")]
[InlineData("Distinct(Table, B)", "*[A:n, B:b, C:s]", "*[Value:b]")]
[InlineData("Distinct(Table, C)", "*[A:n, B:b, C:s]", "*[Value:s]")]
[InlineData("Distinct(Table, \"hello\")", "*[A:n, B:b, C:s]", "*[Value:s]")]
[InlineData("Distinct(Table, A * 2 / 3)", "*[A:n, B:b, C:s]", "*[Value:n]")]
[InlineData("Distinct(Table, REC)", "*[A:n, REC:![b:n], C:s]", "*[Value:![b:n]]", true)]
[InlineData("Distinct(Table, TBL)", "*[A:n, TBL:*[b:n], C:s]", "*[Value:*[b:n]]", true)]
[InlineData("Distinct(Table, Date)", "*[Date:D, DateTime:d, Time:T]", "*[Value:D]")]
[InlineData("Distinct(Table, DateTime)", "*[Date:D, DateTime:d, Time:T]", "*[Value:d]")]
[InlineData("Distinct(Table, Time)", "*[Date:D, DateTime:d, Time:T]", "*[Value:T]")]
[InlineData("Distinct(Table, G)", "*[G:g, OS:l]", "*[Value:g]")]
[InlineData("Distinct(Table, OS)", "*[G:g, OS:l]", "*[Value:l]")]
[InlineData("Distinct(Table, Untyped)", "*[Untyped:O]", "*[Value:O]", true)]
public void TexlFunctionTypeSemanticsDistinct(string expression, string tableType, string expectedResult, bool failsForPFxV1 = false)
{
foreach (var usePFxV1 in new[] { false, true })
{
var symbolTable = new SymbolTable();
symbolTable.AddVariable("Table", new TableType(TestUtils.DT(tableType)));

var features = new Features
{
PowerFxV1CompatibilityRules = usePFxV1,
ConsistentOneColumnTableResult = true,
};

var expectedDType = TestUtils.DT(expectedResult);
if (!usePFxV1 || !failsForPFxV1)
{
TestSimpleBindingSuccess(expression, expectedDType, symbolTable: symbolTable, features: features);
}
else
{
TestBindingErrors(expression, TestUtils.DT("*[]"), symbolTable: symbolTable, features: features);
}
}
}

[Fact]
public void TexlFunctionTypeSemanticsIf()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ private ReadOnlySymbolTable GetSymbols()
customSymbols.AddFunction(new SummarizeFunction());
customSymbols.AddFunction(new RecalcEngineSetFunction());
customSymbols.AddFunction(new RemoveFunction());
customSymbols.AddFunction(Library.DistinctInterpreterFunction);
customSymbols.AddVariable("local", localType, mutable: true);
customSymbols.AddVariable("remote", remoteType, mutable: true);
customSymbols.AddVariable("simple1", simple1Type, mutable: true);
Expand Down Expand Up @@ -235,6 +234,7 @@ public void DepedencyScanFunctionTests()
"Average",
"Concat",
"CountIf",
"Distinct",
"Filter",
"ForAll",
"IfError",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void TestSuggestOptionSets(string expression, params string[] expectedSug

// Test with display names.
[Theory]
[InlineData("Dis|", "DisplayOpt", "DisplayRowScope")] // Match to row scope
[InlineData("Dis|", "DisplayOpt", "DisplayRowScope", "Distinct")] // Match to row scope
public void TestSuggestOptionSetsDisplayName(string expression, params string[] expectedSuggestions)
{
var config = PowerFxConfig.BuildWithEnumStore(new EnumStoreBuilder(), new TexlFunctionSet());
Expand Down