PR feedback.

src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs

@@ -1747,26 +1747,12 @@ internal ArgPreprocessor GetGenericArgPreprocessor(int index)
         /// <param name="node">IR CallNode.</param>
         /// <param name="visitor">Dependency visitor.</param>
         /// <param name="context">Dependency context.</param>
-        /// <returns>Static boolean value.</returns>
+        /// <returns></returns>
         public virtual bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyContext context)
         {
-            for (int i = 0; i < node.Args.Count; i++)
+            foreach (var arg in node.Args)
             {
-                if (node.Scope != null && i < node.Function.ScopeArgs)
-                {
-                    if (node.Args[i] is IRCallNode callNode)
-                    {
-                        callNode.Accept(visitor, context);
-                    }
-                    else
-                    {
-                        continue;
-                    }
-                }
-                else
-                {
-                    node.Args[i].Accept(visitor, context);
-                }
+                arg.Accept(visitor, context);
             }
 
             // The return value is used by DepedencyScanFunctionTests test case.

src/libraries/Microsoft.PowerFx.Core/IR/Visitors/DependencyVisitor.cs

@@ -3,14 +3,11 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Text;
-using System.Xml;
 using Microsoft.PowerFx.Core.IR.Nodes;
 using Microsoft.PowerFx.Core.IR.Symbols;
-using Microsoft.PowerFx.Core.Utils;
 using Microsoft.PowerFx.Types;
-using IRCallNode = Microsoft.PowerFx.Core.IR.Nodes.CallNode;
+using static Microsoft.PowerFx.Syntax.PrettyPrintVisitor;
 
 namespace Microsoft.PowerFx.Core.IR
 {
@@ -167,9 +164,14 @@ public override RetVal Visit(ResolvedObjectNode node, DependencyContext context)
                 }
             }
 
-            // Check if identifer is a field access on a table in row scope
-            var obj = node.Value;
-            if (obj is NameSymbol sym)
+            CheckResolvedObjectNodeValue(node, context);
+
+            return null;
+        }
+
+        protected virtual void CheckResolvedObjectNodeValue(ResolvedObjectNode node, DependencyContext context)
+        {
+            if (node.Value is NameSymbol sym)
             {
                 if (sym.Owner is SymbolTableOverRecordType symTable)
                 {
@@ -180,7 +182,7 @@ public override RetVal Visit(ResolvedObjectNode node, DependencyContext context)
                     {
                         // "ThisRecord". Whole entity
                         AddField(context, tableLogicalName, null);
-                        return null;
+                        return;
                     }
 
                     // on current table
@@ -189,8 +191,6 @@ public override RetVal Visit(ResolvedObjectNode node, DependencyContext context)
                     AddField(context, tableLogicalName, fieldLogicalName);
                 }
             }
-
-            return null;
         }
 
         public override RetVal Visit(SingleColumnTableAccessNode node, DependencyContext context)
@@ -224,17 +224,19 @@ public class RetVal
 
         public class DependencyContext
         {
-            public bool WriteState { get; set; }
+            // Indicates the scanning is working on the args of a non self-contained function.
+            public bool WriteState { get; init; }
 
-            public TableType ScopeType { get; set; }
+            // If WriteState is true, this will be set with the arg0 type.
+            public TableType ScopeType { get; init; }
 
             public DependencyContext()
             {
             }
         }
 
         // Translate relationship names to actual field references.
-        public string Translate(string tableLogicalName, string fieldLogicalName)
+        public virtual string Translate(string tableLogicalName, string fieldLogicalName)
         {
             return fieldLogicalName;
         }
@@ -256,7 +258,7 @@ private void AddField(Dictionary<string, HashSet<string>> list, string tableLogi
             if (fieldLogicalName != null)
             {
                 var name = Translate(tableLogicalName, fieldLogicalName);
-                fieldReads.Add(fieldLogicalName);
+                fieldReads.Add(name);
             }
         }
 

src/libraries/Microsoft.PowerFx.Core/Public/CheckResult.cs

@@ -246,7 +246,7 @@ public HashSet<string> TopLevelIdentifiers
             {
                 if (_topLevelIdentifiers == null)
                 {
-                    throw new InvalidOperationException($"Call {nameof(ApplyTopLevelIdentifiersAnalysis)} first.");
+                    throw new InvalidOperationException($"Call {nameof(ApplyDependencyAnalysis)} first.");
                 }
 
                 return _topLevelIdentifiers;
@@ -483,13 +483,8 @@ private void VerifyReturnTypeMatch()
         /// <summary>
         /// Compute the dependencies. Called after binding. 
         /// </summary>
-        public DependencyInfo ApplyDependencyAnalysis()
+        public DependencyInfo ApplyDependencyInfoScan()
         {
-            if (!IsSuccess)
-            {
-                return null;
-            }
-
             var ir = ApplyIR(); //throws on errors
 
             var ctx = new DependencyVisitor.DependencyContext();
@@ -503,7 +498,7 @@ public DependencyInfo ApplyDependencyAnalysis()
         /// <summary>
         /// Compute the dependencies. Called after binding. 
         /// </summary>
-        public void ApplyTopLevelIdentifiersAnalysis()
+        public void ApplyDependencyAnalysis()
         {
             var binding = this.Binding; // will throw if binding wasn't run
             this._topLevelIdentifiers = DependencyFinder.FindDependencies(binding.Top, binding);

src/libraries/Microsoft.PowerFx.Core/Public/Engine.cs

@@ -291,7 +291,7 @@ private void CheckWorker(CheckResult check)
         {
             check.ApplyBindingInternal();
             check.ApplyErrors();
-            check.ApplyTopLevelIdentifiersAnalysis();
+            check.ApplyDependencyAnalysis();
         }
 
         // Called after check result, can inject additional errors or constraints. 

src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs

@@ -10,13 +10,13 @@
 using Microsoft.PowerFx.Core.Binding;
 using Microsoft.PowerFx.Core.Errors;
 using Microsoft.PowerFx.Core.Functions;
-using Microsoft.PowerFx.Core.Localization;
+using Microsoft.PowerFx.Core.IR;
 using Microsoft.PowerFx.Core.Types;
 using Microsoft.PowerFx.Core.Utils;
 using Microsoft.PowerFx.Syntax;
 using Microsoft.PowerFx.Types;
 using static Microsoft.PowerFx.Core.Localization.TexlStrings;
-using static Microsoft.PowerFx.Syntax.PrettyPrintVisitor;
+using IRCallNode = Microsoft.PowerFx.Core.IR.Nodes.CallNode;
 
 namespace Microsoft.PowerFx.Functions
 {
@@ -235,8 +235,24 @@ public async Task<FormulaValue> InvokeAsync(FunctionInvokeInfo invokeInfo, Cance
             {
                 result = returnType == FormulaType.Void ? FormulaValue.NewVoid() : FormulaValue.NewBlank();
             }
-
+
             return result;
+        }
+
+        public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context)
+        {
+            var newContext = new DependencyVisitor.DependencyContext()
+            {
+                WriteState = true,
+                ScopeType = node.Args[0].IRContext.ResultType as TableType
+            };
+
+            foreach (var arg in node.Args.Skip(1))
+            {
+                arg.Accept(visitor, newContext);
+            }
+
+            return true;
         }
     }
 }

src/libraries/Microsoft.PowerFx.LanguageServerProtocol/Public/EditorContextScope.cs

@@ -87,7 +87,7 @@ public CheckResult Check(string expression)
             // By default ...
             check.ApplyBindingInternal();
             check.ApplyErrors();
-            check.ApplyTopLevelIdentifiersAnalysis();
+            check.ApplyDependencyAnalysis();
 
             return check;
         }

src/tests/Microsoft.PowerFx.Core.Tests.Shared/CheckResultTests.cs

@@ -270,7 +270,7 @@ public void Binding()
             // Binding doesn't compute dependency analysis. 
             // These are other Apply* calls. 
             Assert.Throws<InvalidOperationException>(() => check.TopLevelIdentifiers);
-            check.ApplyTopLevelIdentifiersAnalysis();
+            check.ApplyDependencyAnalysis();
             Assert.NotNull(check.TopLevelIdentifiers);
         }
 

src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/DependencyTests.cs

@@ -12,6 +12,7 @@
 using Microsoft.PowerFx.Core.Texl.Builtins;
 using Microsoft.PowerFx.Core.Types;
 using Microsoft.PowerFx.Core.Types.Enums;
+using Microsoft.PowerFx.Functions;
 using Microsoft.PowerFx.Interpreter;
 using Microsoft.PowerFx.Types;
 using Xunit;
@@ -53,6 +54,9 @@ public class DependencyTests : PowerFxTest
         [InlineData("Patch(local, Table({accountid:GUID(), 'Address 1: City':\"test\"},{accountid:GUID(), 'Account Name':\"test\"}),Table({'Address 1: City':\"test\"},{'Address 1: City':\"test\",'Account Name':\"test\"}))", "Read Accounts: accountid, address1_city, name; Write Accounts: address1_city, name;")]
         [InlineData("Patch(local, First(local), { 'Address 1: City' : First(local).'Account Name'  } )", "Read Accounts: name; Write Accounts: address1_city;")]
 
+        // Remove
+        [InlineData("Remove(local, {name: First(remote).name})", "Read Contacts: name; Write Accounts: name;")]
+
         // Collect and ClearCollect.
         [InlineData("Collect(local, Table({ 'Account Name' : \"some name\"}))", "Write Accounts: name;")]
         [InlineData("Collect(local, local)", "Write Accounts: ;")]
@@ -81,6 +85,7 @@ public class DependencyTests : PowerFxTest
         [InlineData("Set(numberofemployees, 200)", "Write Accounts: numberofemployees;")]
         [InlineData("Set('Address 1: City', 'Account Name')", "Read Accounts: name; Write Accounts: address1_city;")]
         [InlineData("Set('Address 1: City', 'Address 1: City' & \"test\")", "Read Accounts: address1_city; Write Accounts: address1_city;")]
+        [InlineData("Set(NewRecord.'Address 1: City', \"test\")", "Write Accounts: address1_city;")]
         public void GetDependencies(string expr, string expected)
         {
             var opt = new ParserOptions() { AllowsSideEffects = true };
@@ -92,7 +97,7 @@ public void GetDependencies(string expr, string expected)
 
             check.ApplyBinding();            
 
-            var info = check.ApplyDependencyAnalysis();
+            var info = check.ApplyDependencyInfoScan();
             var actual = info.ToString().Replace("\r", string.Empty).Replace("\n", string.Empty).Trim();
             Assert.Equal(expected, actual);
         }
@@ -117,6 +122,7 @@ private ReadOnlySymbolTable GetSymbols()
             customSymbols.AddFunction(new PatchSingleRecordFunction());
             customSymbols.AddFunction(new SummarizeFunction());
             customSymbols.AddFunction(new RecalcEngineSetFunction());
+            customSymbols.AddFunction(new RemoveFunction());
             customSymbols.AddVariable("local", localType, mutable: true);
             customSymbols.AddVariable("remote", remoteType, mutable: true);
 
@@ -159,15 +165,18 @@ private TableType Contacts()
             return (TableType)FormulaType.Build(contactType);
         }
 
-        // Some functions might require an different dependecy scan. This test case is to ensure that any new functions that
-        // is not self-contained or has a scope info has been assessed and either added to the exception list or has a dependency scan.
+        /// <summary>
+        /// This test case is to ensure that all functions that are not self-contained or 
+        /// have a scope info have been assessed and either added to the exception list or overrides <see cref="TexlFunction.ComposeDependencyInfo"/>.
+        /// </summary>
         [Fact]
         public void DepedencyScanFunctionTests()
         {
             var names = new List<string>();
             var functions = new List<TexlFunction>();
             functions.AddRange(BuiltinFunctionsCore.BuiltinFunctionsLibrary);
 
+            // These methods use default implementation of ComposeDependencyInfo and do not neeed to override it. 
             var exceptionList = new HashSet<string>()
             {
                 "AddColumns",
@@ -198,11 +207,11 @@ public void DepedencyScanFunctionTests()
             {
                 if (!func.IsSelfContained || func.ScopeInfo != null)
                 {
+                    var irContext = IRContext.NotInSource(FormulaType.String);
+                    var node = new CallNode(irContext, func, new ErrorNode(irContext, "test"));
                     var visitor = new DependencyVisitor();
                     var context = new DependencyVisitor.DependencyContext();
-                    var node = new CallNode(IRContext.NotInSource(FormulaType.String), func);
                     var overwritten = func.ComposeDependencyInfo(node, visitor, context);
-
                     if (!overwritten && !exceptionList.Contains(func.Name))
                     {
                         names.Add(func.Name);