Fix #3119. Don't expose ListValue<T> lists to users (#3129)

* Fix #3119. Don't expose ListValue<T> lists to users

Use Collections.Generic.List<T> instead of ListValue<T>. Return a ListValue copy of these lists to user code.

Changes:
FileContent:binary
Lexicon:keys
Lexicon:values
Part:children
Stage:resources
Stage:resourceslex
String:split
Structure:suffixnames
Timewarp:ratelist
Timewarp:railsratelist
Timewarp:physicsratelist
Vessel:parts
Vessel:dockingports
Vessel:decouplers (separators)
Vessel:resources
allnodes bound variable
allwaypoints function

Stage:resources, Stage:resourceslex, Vessel:parts, Vessel:dockingports, Vessel:decouplers now return copies instead of references to internal lists. All lists returned by these suffixes don't get modified over time so the only way to tell that they are copies now is to use the equality operator. Stage:resources and Stage:resourceslex returning a reference was a bug.

* Optimize by not creating list copies where possible. Revert Vessel parts, dockingports, decouplers suffixes to return readonly references to internal lists

Author: sug44

src/kOS.Safe/Encapsulation/Lexicon.cs

@@ -115,10 +115,10 @@ private void FillWithEnumerableValues(IEnumerable<Structure> values)
         private void InitalizeSuffixes()
         {
             AddSuffix("CLEAR", new NoArgsVoidSuffix(Clear, "Removes all items from Lexicon"));
-            AddSuffix("KEYS", new Suffix<ListValue<Structure>>(GetKeys, "Returns the lexicon keys"));
+            AddSuffix("KEYS", new Suffix<ListValue>(GetKeys, "Returns the lexicon keys"));
             AddSuffix("HASKEY", new OneArgsSuffix<BooleanValue, Structure>(HasKey, "Returns true if a key is in the Lexicon"));
             AddSuffix("HASVALUE", new OneArgsSuffix<BooleanValue, Structure>(HasValue, "Returns true if value is in the Lexicon"));
-            AddSuffix("VALUES", new Suffix<ListValue<Structure>>(GetValues, "Returns the lexicon values"));
+            AddSuffix("VALUES", new Suffix<ListValue>(GetValues, "Returns the lexicon values"));
             AddSuffix("COPY", new NoArgsSuffix<Lexicon>(() => new Lexicon(this), "Returns a copy of Lexicon"));
             AddSuffix("LENGTH", new NoArgsSuffix<ScalarValue>(() => internalDictionary.Count, "Returns the number of elements in the collection"));
             AddSuffix("REMOVE", new OneArgsSuffix<BooleanValue, Structure>(one => Remove(one), "Removes the value at the given key"));
@@ -158,12 +158,12 @@ private BooleanValue HasKey(Structure key)
             return internalDictionary.ContainsKey(key);
         }
 
-        public ListValue<Structure> GetValues()
+        public ListValue GetValues()
         {
             return ListValue.CreateList(Values);
         }
 
-        public ListValue<Structure> GetKeys()
+        public ListValue GetKeys()
         {
             return ListValue.CreateList(Keys);
         }
@@ -380,9 +380,9 @@ public override BooleanValue HasSuffix(StringValue suffixName)
         /// to the list.
         /// </summary>
         /// <returns></returns>
-        public override ListValue<StringValue> GetSuffixNames()
+        public override ListValue GetSuffixNames()
         {
-            ListValue<StringValue> theList = base.GetSuffixNames();
+            ListValue theList = base.GetSuffixNames();
 
             foreach (Structure key in internalDictionary.Keys)
             {
@@ -392,7 +392,7 @@ public override ListValue<StringValue> GetSuffixNames()
                     theList.Add(keyStr);
                 }
             }
-            return new ListValue<StringValue>(theList.OrderBy(item => item.ToString()));
+            return new ListValue(theList.OrderBy(item => item.ToString()));
         }
         public override Dump Dump()
         {

src/kOS.Safe/Encapsulation/StringValue.cs

@@ -239,10 +239,10 @@ System.Collections.IEnumerator IEnumerable.GetEnumerator()
         }
 
         // As the regular Split, except returning a ListValue rather than an array.
-        public ListValue<StringValue> SplitToList(string separator)
+        public ListValue SplitToList(string separator)
         {
             string[] split = Regex.Split(internalString, Regex.Escape(separator), RegexOptions.IgnoreCase);
-            ListValue<StringValue> returnList = new ListValue<StringValue>();
+            ListValue returnList = new ListValue();
             foreach (string s in split)
                 returnList.Add(new StringValue(s));
             return returnList;
@@ -273,7 +273,7 @@ private void StringInitializeSuffixes()
             AddSuffix("PADRIGHT",   new OneArgsSuffix<StringValue, ScalarValue>( one => PadRight(one)));
             AddSuffix("REMOVE",     new TwoArgsSuffix<StringValue, ScalarValue, ScalarValue>( (one, two) => Remove(one, two)));
             AddSuffix("REPLACE",    new TwoArgsSuffix<StringValue, StringValue, StringValue>( (one, two) => Replace(one, two)));
-            AddSuffix("SPLIT",      new OneArgsSuffix<ListValue<StringValue>, StringValue>( one => SplitToList(one)));
+            AddSuffix("SPLIT",      new OneArgsSuffix<ListValue, StringValue>( one => SplitToList(one)));
             AddSuffix("STARTSWITH", new OneArgsSuffix<BooleanValue, StringValue>( one => StartsWith(one)));
             AddSuffix("TOLOWER",    new NoArgsSuffix<StringValue>(() => ToLower()));
             AddSuffix("TOUPPER",    new NoArgsSuffix<StringValue>(() => ToUpper()));

src/kOS.Safe/Encapsulation/Structure.cs

@@ -63,7 +63,7 @@ private void InitializeInstanceSuffixes()
 
               AddSuffix("TOSTRING",       new NoArgsSuffix<StringValue>(() => ToString()));
               AddSuffix("HASSUFFIX",      new OneArgsSuffix<BooleanValue, StringValue>(HasSuffix));
-              AddSuffix("SUFFIXNAMES",    new NoArgsSuffix<ListValue<StringValue>>(GetSuffixNames));
+              AddSuffix("SUFFIXNAMES",    new NoArgsSuffix<ListValue>(GetSuffixNames));
               AddSuffix("ISSERIALIZABLE", new NoArgsSuffix<BooleanValue>(() => this is SerializableStructure));
               AddSuffix("TYPENAME",       new NoArgsSuffix<StringValue>(() => new StringValue(KOSName)));
               AddSuffix("ISTYPE",         new OneArgsSuffix<BooleanValue,StringValue>(GetKOSIsType));
@@ -208,17 +208,17 @@ public virtual BooleanValue HasSuffix(StringValue suffixName)
             return false;
         }
 
-        public virtual ListValue<StringValue> GetSuffixNames()
+        public virtual ListValue GetSuffixNames()
         {
             callInitializeSuffixes();
-            List<StringValue> names = new List<StringValue>();            
-            
-            names.AddRange(instanceSuffixes.Keys.Select(item => (StringValue)item));
-            names.AddRange(GetStaticSuffixesForType(GetType()).Keys.Select(item => (StringValue)item));
+            List<StringValue> names = new List<StringValue>();
+
+            foreach (var suffix in instanceSuffixes.Keys) names.Add(suffix);
+            foreach (var staticSuffix in GetStaticSuffixesForType(GetType()).Keys) names.Add(staticSuffix);
 
             // Return the list alphabetized by suffix name.  The key lookups above, since they're coming
             // from a hashed dictionary, won't be in any predictable ordering:
-            return new ListValue<StringValue>(names.OrderBy(item => item.ToString()));
+            return new ListValue(names.OrderBy(item => item.ToString()));
         }
 
         public virtual BooleanValue GetKOSIsType(StringValue queryTypeName)

src/kOS.Safe/Persistence/FileContent.cs

@@ -61,15 +61,10 @@ private void InitializeSuffixes()
             AddSuffix("EMPTY", new Suffix<BooleanValue>(() => Size == 0));
             AddSuffix("TYPE", new Suffix<StringValue>(() => Category.ToString()));
             AddSuffix("STRING", new Suffix<StringValue>(() => String));
-            AddSuffix("BINARY", new Suffix<ListValue<ScalarIntValue>>(() => ContentAsIntList()));
+            AddSuffix("BINARY", new Suffix<ListValue>(() => new ListValue(Bytes.Select(x => new ScalarIntValue(x)))));
             AddSuffix("ITERATOR", new Suffix<Enumerator>(() => new Enumerator(GetEnumerator())));
         }
 
-        private ListValue<ScalarIntValue> ContentAsIntList()
-        {
-            return new ListValue<ScalarIntValue>(Bytes.Select(x => new ScalarIntValue(x)));
-        }
-
         public override Dump Dump()
         {
             return new Dump { { DumpContent, PersistenceUtilities.EncodeBase64(Bytes) } };

src/kOS/Binding/FlightStats.cs

@@ -1,3 +1,4 @@
+using System.Collections.Generic;
 using kOS.Module;
 using kOS.Control;
 using kOS.Safe.Binding;
@@ -59,12 +60,12 @@ public override void AddTo(SharedObjects shared)
             shared.BindingMgr.AddGetter("ALLNODES", () => GetAllNodes(shared));
         }
 
-        public ListValue<Node> GetAllNodes(SharedObjects shared)
+        public ListValue GetAllNodes(SharedObjects shared)
         {
             var vessel = shared.Vessel;
             if (vessel.patchedConicSolver == null || vessel.patchedConicSolver.maneuverNodes.Count == 0)
-                return new ListValue<Node>();
-            var ret = new ListValue<Node>(vessel.patchedConicSolver.maneuverNodes.Select(e => Node.FromExisting(vessel, e, shared)));
+                return new ListValue();
+            var ret = new ListValue(vessel.patchedConicSolver.maneuverNodes.Select(e => Node.FromExisting(vessel, e, shared)));
             return ret;
         }
     }

src/kOS/Function/Suffixed.cs

@@ -732,7 +732,7 @@ public override void Execute(SharedObjects shared)
             AssertArgBottomAndConsume(shared); // no args
 
             // ReSharper disable SuggestUseVarKeywordEvident
-            ListValue<WaypointValue> returnList = new ListValue<WaypointValue>();
+            ListValue returnList = new ListValue();
             // ReSharper enable SuggestUseVarKeywordEvident
 
             WaypointManager wpm = WaypointManager.Instance();

src/kOS/Suffixed/AggregateResourceValue.cs

@@ -99,10 +99,10 @@ public static ListValue PartsToList(IEnumerable<global::Part> parts, SharedObjec
             return list;
         }
 
-        public static ListValue<AggregateResourceValue> FromVessel(Vessel vessel, SharedObjects shared)
+        public static ListValue FromVessel(Vessel vessel, SharedObjects shared)
         {
             var resources = ProspectResources(vessel.parts, shared);
-            return ListValue<AggregateResourceValue>.CreateList(resources.Values);
+            return new ListValue(resources.Values);
         }
     }
 }

src/kOS/Suffixed/Part/DockingPortValue.cs

@@ -62,7 +62,7 @@ public PartValue GetPartner()
             }
 
             var otherVessel = VesselTarget.CreateOrGetExisting(otherNode.vessel, Shared);
-            foreach (var part in otherVessel.Parts)
+            foreach (PartValue part in otherVessel.Parts)
             {
                 if (part.Part == otherNode.part)
                 {

src/kOS/Suffixed/Part/PartValue.cs

@@ -22,7 +22,7 @@ public class PartValue : Structure, IKOSTargetable
         public global::Part Part { get; private set; }
         public PartValue Parent { get; private set; }
         public DecouplerValue Decoupler { get; private set; }
-        public ListValue<PartValue> Children { get; private set; }
+        public ListValue Children { get; private set; }
         public Structure ParentValue { get { return (Structure)Parent ?? StringValue.None; } }
         public Structure DecouplerValue { get { return (Structure)Decoupler ?? StringValue.None; } }
         public int DecoupledIn { get { return (Decoupler != null) ? Decoupler.Part.inverseStage : -1; } }
@@ -37,7 +37,7 @@ internal PartValue(SharedObjects shared, global::Part part, PartValue parent, De
             Parent = parent;
             Decoupler = decoupler;
             RegisterInitializer(PartInitializeSuffixes);
-            Children  = new ListValue<PartValue>();
+            Children  = new ListValue();
         }
 
         private void PartInitializeSuffixes()
@@ -66,7 +66,7 @@ private void PartInitializeSuffixes()
             AddSuffix(new[] { "DECOUPLER", "SEPARATOR" }, new Suffix<Structure>(() => DecouplerValue, "The part that will decouple/separate this part when activated"));
             AddSuffix(new[] { "DECOUPLEDIN", "SEPARATEDIN" }, new Suffix<ScalarValue>(() => DecoupledIn));
             AddSuffix("HASPARENT", new Suffix<BooleanValue>(() => Part.parent != null, "Tells you if this part has a parent, is used to avoid null exception from PARENT"));
-            AddSuffix("CHILDREN", new Suffix<ListValue<PartValue>>(() => PartValueFactory.ConstructGeneric(Part.children, Shared), "A LIST() of the children parts of this part"));
+            AddSuffix("CHILDREN", new Suffix<ListValue>(() => PartValueFactory.ConstructGeneric(Part.children, Shared), "A LIST() of the children parts of this part"));
             AddSuffix("DRYMASS", new Suffix<ScalarValue>(() => Part.GetDryMass(), "The Part's mass when empty"));
             AddSuffix("MASS", new Suffix<ScalarValue>(() => Part.CalculateCurrentMass(), "The Part's current mass"));
             AddSuffix("WETMASS", new Suffix<ScalarValue>(() => Part.GetWetMass(), "The Part's mass when full"));

src/kOS/Suffixed/Part/PartValueFactory.cs

@@ -12,8 +12,8 @@ public static ListValue Construct(IEnumerable<global::Part> parts, SharedObjects
             return ListValue.CreateList(parts.Select(part => Construct(part, shared)).Where(p => p != null));
         }
 
-        public static ListValue<PartValue> ConstructGeneric(IEnumerable<global::Part> parts, SharedObjects shared) {
-            return ListValue<PartValue>.CreateList(parts.Select(part => Construct(part, shared)).Where(p => p != null));
+        public static ListValue ConstructGeneric(IEnumerable<global::Part> parts, SharedObjects shared) {
+            return new ListValue(parts.Select(part => Construct(part, shared)).Where(p => p != null));
         }
 
         public static PartValue Construct(global::Part part, SharedObjects shared) {