Add declaration inside new method for return parameter if not already in the selected code

Author: tommy9

Rubberduck.Refactorings/ExtractMethod/ExtractMethodModel.cs

@@ -79,7 +79,6 @@ private void Setup()
             var declarationsInParentMethod = GetDeclarationsInSelection(sourceMethodSelection);
 
             //List of "inbound" variables. Parent procedure parameters + explicit dims which get referenced inside the selection.
-            //TODO - add case where reference earlier in the same line as where the selection starts (unusual but could exist if using colons to separate multiple statements)
             var inboundParameters = sourceMethodParameters.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)));
             var inboundLocalVariables = declarationsInParentMethod
                 .Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
@@ -161,9 +160,7 @@ private void SetUpParameters(IEnumerable<Declaration> inboundVariables, IEnumera
                     paramType = ExtractMethodParameterType.ByRefParameter;
                     canReturn = true;
                 }
-                Parameters.Add(new ExtractMethodParameter(declaration.AsTypeNameWithoutArrayDesignator,
-                                                          paramType, declaration.IdentifierName,
-                                                          declaration.IsArray, declaration.IsObject, canReturn));
+                Parameters.Add(new ExtractMethodParameter(declaration, paramType, canReturn));
             }
         }
 
@@ -362,10 +359,16 @@ public string NewMethodCode
                         }({string.Join(", ", _parametersToExtract)}) {returnType}");
                 foreach (var dec in _declarationsToMoveIn)
                 {
-                    //go from variableSubStmt to variableListStmt to variableStmt
-                    strings.Add(dec.Context.Parent.Parent.GetText());
+                    strings.Add(dec.Context.GetAncestor<VBAParser.VariableStmtContext>().GetText());
                     //TODO - handle case of variable list having multiple parts (if not excluded by validator)
                 }
+                if (isFunction)
+                {
+                    if (!QualifiedSelection.Selection.Contains(ReturnParameter.Declaration.Context))
+                    {
+                        strings.Add(ReturnParameter.Declaration.Context.GetAncestor<VBAParser.VariableStmtContext>().GetText());
+                    }
+                }
                 strings.AddRange(SelectedCodeToExtract.Split(new[] { Environment.NewLine }, StringSplitOptions.None));
                 if (isFunction)
                 {

Rubberduck.Refactorings/ExtractMethod/ExtractMethodParameter.cs

@@ -4,6 +4,8 @@
 using Rubberduck.Parsing.Grammar;
 using System.ComponentModel;
 using System.Collections.ObjectModel;
+using Rubberduck.Parsing.Symbols;
+using System.Xml.Linq;
 
 namespace Rubberduck.Refactorings.ExtractMethod
 {
@@ -27,16 +29,28 @@ public class ExtractMethodParameter : INotifyPropertyChanged
 
         public event PropertyChangedEventHandler PropertyChanged;
 
-        public ExtractMethodParameter(string typeName, ExtractMethodParameterType parameterType, string name, bool isArray, bool isObject, bool canReturn)
+        public ExtractMethodParameter(Declaration declaration, ExtractMethodParameterType parameterType, bool canReturn)
         {
-            Name = name ?? NoneLabel;
-            TypeName = typeName;
             ParameterType = parameterType;
-            IsArray = isArray;
             CanReturn = canReturn;
-            IsObject = isObject;
+            Declaration = declaration;
+            if (declaration == null)
+            {
+                Name = NoneLabel;
+                TypeName = string.Empty;
+                IsArray = false;
+                IsObject = false;
+            }
+            else
+            {
+                Name = declaration.IdentifierName;
+                TypeName = declaration.AsTypeNameWithoutArrayDesignator;
+                IsArray = declaration.IsArray;
+                IsObject = declaration.IsObject;
+            }
         }
 
+        public Declaration Declaration { get; }
         public string Name { get; set; }
 
         public string TypeName { get; set; }
@@ -111,9 +125,7 @@ public override string ToString()
         }
 
 
-        public static ExtractMethodParameter None => new ExtractMethodParameter(string.Empty,
-            ExtractMethodParameterType.ByValParameter,
-            RefactoringsUI.ExtractMethod_NoneSelected, false, false, false);
+        public static ExtractMethodParameter None => new ExtractMethodParameter(null, ExtractMethodParameterType.ByValParameter, false);
 
         public static ObservableCollection<ExtractMethodParameterType> AllowableTypes = new ObservableCollection<ExtractMethodParameterType>
             {
@@ -144,43 +156,31 @@ public static Dictionary<ExtractMethodParameterType, string> ParameterTypes
         public static bool operator ==(ExtractMethodParameter left, ExtractMethodParameter right)
         {
             return left?.ParameterType == right?.ParameterType &&
-                   left?.TypeName == right?.TypeName &&
-                   left?.Name == right?.Name &&
-                   left?.IsArray == right?.IsArray &&
                    left?.CanReturn == right?.CanReturn &&
-                   left?.IsObject == right?.IsObject;
+                   left?.Declaration == right?.Declaration;
         }
 
         public static bool operator !=(ExtractMethodParameter left, ExtractMethodParameter right)
         {
             return !(left?.ParameterType == right?.ParameterType &&
-                     left?.TypeName == right?.TypeName &&
-                     left?.Name == right?.Name &&
-                     left?.IsArray == right?.IsArray &&
                      left?.CanReturn == right?.CanReturn &&
-                     left?.IsObject == right?.IsObject);
+                     left?.Declaration == right?.Declaration);
         }
 
         public override bool Equals(object obj)
         {
             return obj is ExtractMethodParameter parameter &&
-                   Name == parameter.Name &&
-                   TypeName == parameter.TypeName &&
                    CanReturn == parameter.CanReturn &&
                    ParameterType == parameter.ParameterType &&
-                   IsArray == parameter.IsArray &&
-                   IsObject == parameter.IsObject;
+                   Declaration == parameter.Declaration;
         }
 
         public override int GetHashCode()
         {
             int hashCode = 1661774273;
-            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(Name);
-            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(TypeName);
             hashCode = (hashCode * -1521134295) + CanReturn.GetHashCode();
             hashCode = (hashCode * -1521134295) + ParameterType.GetHashCode();
-            hashCode = (hashCode * -1521134295) + IsArray.GetHashCode();
-            hashCode = (hashCode * -1521134295) + IsObject.GetHashCode();
+            hashCode = (hashCode * -1521134295) + (Declaration == null ? 0 : Declaration.GetHashCode());
             return hashCode;
         }
     }

RubberduckTests/Refactoring/ExtractMethod/ExtractMethodModelTests.cs

@@ -307,6 +307,7 @@ Debug.Print a
             var selection = new Selection(4, 5, 4, 11);
             var expectedNewMethodCode = @"
 Private Function NewMethod() As Integer
+    Dim a As Integer
     a = 10
 
     NewMethod = a