From f602d3bd1dfc6c7b1f63caab62d5d05e34036e0e Mon Sep 17 00:00:00 2001
From: Igor Bagdamyan <37334640+En3Tho@users.noreply.github.com>
Date: Thu, 30 Sep 2021 00:14:32 +0300
Subject: [PATCH] ComboBox cursonDownKey nullref fix (#1472)

* added null guard to fix null ref when pressing keyDown inside combobox
Improved an error message when view cannot be found

* Added a unit test to ensure combobox can process all key events
Found and fixed a new nullref

* Found a new bug when source is already present and combobox is added to a top view

* searchSet is auto initialized to new List() now to make the code a little bit safer
---
 Terminal.Gui/Core/View.cs      | 23 +++++++++++++----------
 Terminal.Gui/Views/ComboBox.cs | 19 ++++++-------------
 UnitTests/ComboBoxTests.cs     | 24 ++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 23 deletions(-)
 create mode 100644 UnitTests/ComboBoxTests.cs

diff --git a/Terminal.Gui/Core/View.cs b/Terminal.Gui/Core/View.cs
index e94b1fa81..642a92729 100644
--- a/Terminal.Gui/Core/View.cs
+++ b/Terminal.Gui/Core/View.cs
@@ -1870,16 +1870,19 @@ namespace Terminal.Gui {
 				}
 			}
 
-			if (edges.Any () && edges.First ().From != Application.Top) {
-				if (!ReferenceEquals (edges.First ().From, edges.First ().To)) {
-					throw new InvalidOperationException ($"TopologicalSort (for Pos/Dim) cannot find {edges.First ().From}. Did you forget to add it to {this}?");
-				} else {
-					throw new InvalidOperationException ("TopologicalSort encountered a recursive cycle in the relative Pos/Dim in the views of " + this);
-				}
-			} else {
-				// return L (a topologically sorted order)
-				return result;
-			}
+            if (edges.Any ()) {
+                var (from, to) = edges.First ();
+                if (from != Application.Top) {
+                    if (!ReferenceEquals (from, to)) {
+                        throw new InvalidOperationException ($"TopologicalSort (for Pos/Dim) cannot find {from} linked with {to}. Did you forget to add it to {this}?");
+                    } else {
+                        throw new InvalidOperationException ("TopologicalSort encountered a recursive cycle in the relative Pos/Dim in the views of " + this);
+                    }
+                }
+            }
+
+            // return L (a topologically sorted order)
+            return result;
 		}
 
 		/// <summary>
diff --git a/Terminal.Gui/Views/ComboBox.cs b/Terminal.Gui/Views/ComboBox.cs
index 3eea30eb9..edda3950b 100644
--- a/Terminal.Gui/Views/ComboBox.cs
+++ b/Terminal.Gui/Views/ComboBox.cs
@@ -64,7 +64,7 @@ namespace Terminal.Gui {
 		/// </summary>
 		public event Action<ListViewItemEventArgs> OpenSelectedItem;
 
-		IList searchset;
+        readonly IList searchset = new List<object> ();
 		ustring text = "";
 		readonly TextField search;
 		readonly ListView listview;
@@ -314,7 +314,7 @@ namespace Terminal.Gui {
 			}
 
 			if (e.Key == Key.CursorDown && search.HasFocus) { // jump to list
-				if (searchset.Count > 0) {
+				if (searchset?.Count > 0) {
 					listview.TabStop = true;
 					listview.SetFocus ();
 					SetValue (searchset [listview.SelectedItem]);
@@ -329,7 +329,7 @@ namespace Terminal.Gui {
 				return true;
 			}
 
-			if (e.Key == Key.CursorUp && listview.HasFocus && listview.SelectedItem == 0 && searchset.Count > 0) // jump back to search
+			if (e.Key == Key.CursorUp && listview.HasFocus && listview.SelectedItem == 0 && searchset?.Count > 0) // jump back to search
 			{
 				search.CursorPosition = search.Text.RuneCount;
 				search.SetFocus ();
@@ -406,7 +406,7 @@ namespace Terminal.Gui {
 		{
 			isShow = false;
 			listview.TabStop = false;
-			if (listview.Source.Count == 0 || searchset.Count == 0) {
+			if (listview.Source.Count == 0 || (searchset?.Count ?? 0) == 0) {
 				text = "";
 				return;
 			}
@@ -452,11 +452,7 @@ namespace Terminal.Gui {
 
 		private void ResetSearchSet (bool noCopy = false)
 		{
-			if (searchset == null) {
-				searchset = new List<object> ();
-			} else {
-				searchset.Clear ();
-			}
+            searchset.Clear ();
 
 			if (autoHide || noCopy)
 				return;
@@ -465,9 +461,6 @@ namespace Terminal.Gui {
 
 		private void SetSearchSet ()
 		{
-			if (searchset == null) {
-				searchset = new List<object> ();
-			}
 			// force deep copy
 			foreach (var item in Source.ToList ()) {
 				searchset.Add (item);
@@ -503,7 +496,7 @@ namespace Terminal.Gui {
 		/// Consider making public
 		private void ShowList ()
 		{
-			listview.SetSource (searchset);
+            listview.SetSource (searchset);
 			listview.Clear (); // Ensure list shrinks in Dialog as you type
 			listview.Height = CalculatetHeight ();
 			this.SuperView?.BringSubviewToFront (this);
diff --git a/UnitTests/ComboBoxTests.cs b/UnitTests/ComboBoxTests.cs
new file mode 100644
index 000000000..2b1088d33
--- /dev/null
+++ b/UnitTests/ComboBoxTests.cs
@@ -0,0 +1,24 @@
+using System;
+using System.Linq;
+using Terminal.Gui;
+using Xunit;
+
+namespace UnitTests {
+    public class ComboBoxTests {
+        [Fact]
+        [AutoInitShutdown]
+        public void EnsureKeyEventsDoNotCauseExceptions ()
+        {
+            var comboBox = new ComboBox ("0");
+
+            var source = Enumerable.Range (0, 15).Select (x => x.ToString ()).ToArray ();
+            comboBox.SetSource(source);
+
+            Application.Top.Add(comboBox);
+
+            foreach (var key in (Key [])Enum.GetValues (typeof(Key))) {
+                comboBox.ProcessKey (new KeyEvent (key, new KeyModifiers ()));
+            }
+        }
+    }
+}
\ No newline at end of file