.Net: Remove ReadOnlySkillCollection accessor from ISkillCollection (microsoft#2178)

This ISkillCollection inherits IReadOnlySkillCollection, and therefore
this is an implicit cast. There's certainly no need to implement it as a
required property for future implementers of ISkillCollection.

### Description
Remove property `ISkillCollection.ReadOnlySkillCollection`. Clean up
comments, samples and tests.

### Contribution Checklist
- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
-> This could be a (very low impact) breaking change, if anyone happens
to be calling this.

Author: shawncal

dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SKContextExtensionsTests.cs

@@ -43,7 +43,7 @@ public async Task CanCallGetAvailableFunctionsWithNoFunctionsAsync()
             .Returns(asyncEnumerable);
 
         // Arrange GetAvailableFunctionsAsync parameters
-        var context = new SKContext(variables, skills.ReadOnlySkillCollection, logger);
+        var context = new SKContext(variables, skills, logger);
         var config = new SequentialPlannerConfig() { Memory = memory.Object };
         var semanticQuery = "test";
 
@@ -95,7 +95,6 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsAsync()
         skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
         skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(functionMock.Object);
         skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
-        skills.SetupGet(x => x.ReadOnlySkillCollection).Returns(skills.Object);
 
         // Arrange GetAvailableFunctionsAsync parameters
         var context = new SKContext(variables, skills.Object, logger);
@@ -161,7 +160,6 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsWithRelevancyAsync()
         skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
         skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(functionMock.Object);
         skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
-        skills.SetupGet(x => x.ReadOnlySkillCollection).Returns(skills.Object);
 
         // Arrange GetAvailableFunctionsAsync parameters
         var context = new SKContext(variables, skills.Object, logger);
@@ -217,7 +215,7 @@ public async Task CanCallGetAvailableFunctionsAsyncWithDefaultRelevancyAsync()
             .Returns(asyncEnumerable);
 
         // Arrange GetAvailableFunctionsAsync parameters
-        var context = new SKContext(variables, skills.ReadOnlySkillCollection, logger);
+        var context = new SKContext(variables, skills, logger);
         var config = new SequentialPlannerConfig { RelevancyThreshold = 0.78, Memory = memory.Object };
         var semanticQuery = "test";
 

dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs

@@ -7,7 +7,7 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
 /// <summary>
 /// Read-only skill collection interface.
 /// </summary>
-[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
+[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix")]
 public interface IReadOnlySkillCollection
 {
     /// <summary>

dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs

@@ -7,14 +7,9 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
 /// <summary>
 /// Skill collection interface.
 /// </summary>
-[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
+[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix")]
 public interface ISkillCollection : IReadOnlySkillCollection
 {
-    /// <summary>
-    /// Readonly only access into the collection
-    /// </summary>
-    IReadOnlySkillCollection ReadOnlySkillCollection { get; }
-
     /// <summary>
     /// Add a function to the collection
     /// </summary>

dotnet/src/SemanticKernel/Kernel.cs

@@ -42,7 +42,7 @@ public sealed class Kernel : IKernel, IDisposable
     public ISemanticTextMemory Memory => this._memory;
 
     /// <inheritdoc/>
-    public IReadOnlySkillCollection Skills => this._skillCollection.ReadOnlySkillCollection;
+    public IReadOnlySkillCollection Skills => this._skillCollection;
 
     /// <inheritdoc/>
     public IPromptTemplateEngine PromptTemplateEngine { get; }
@@ -172,7 +172,7 @@ public async Task<SKContext> RunAsync(ContextVariables variables, CancellationTo
     {
         var context = new SKContext(
             variables,
-            this._skillCollection.ReadOnlySkillCollection,
+            this._skillCollection,
             this.Log);
 
         int pipelineStepCount = -1;
@@ -222,7 +222,7 @@ public ISKFunction Func(string skillName, string functionName)
     public SKContext CreateNewContext()
     {
         return new SKContext(
-            skills: this._skillCollection.ReadOnlySkillCollection,
+            skills: this._skillCollection,
             logger: this.Log);
     }
 

dotnet/src/SemanticKernel/SkillDefinition/IReadOnlySkillCollectionTypeProxy.cs

@@ -7,7 +7,7 @@
 namespace Microsoft.SemanticKernel.SkillDefinition;
 
 /// <summary>
-/// Debugger type proxy for <see cref="SkillCollection"/> and <see cref="ReadOnlySkillCollection"/>.
+/// Debugger type proxy for <see cref="SkillCollection"/>.
 /// </summary>
 // ReSharper disable once InconsistentNaming
 internal sealed class IReadOnlySkillCollectionTypeProxy

dotnet/src/SemanticKernel/SkillDefinition/ReadOnlySkillCollection.cs

@@ -16,22 +16,17 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
 /// The class holds a list of all the functions, native and semantic, known to the kernel instance.
 /// The list is used by the planner and when executing pipelines of function compositions.
 /// </summary>
-[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
+[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix")]
 [DebuggerTypeProxy(typeof(IReadOnlySkillCollectionTypeProxy))]
 [DebuggerDisplay("{DebuggerDisplay,nq}")]
 public class SkillCollection : ISkillCollection
 {
     internal const string GlobalSkill = "_GLOBAL_FUNCTIONS_";
 
-    /// <inheritdoc/>
-    public IReadOnlySkillCollection ReadOnlySkillCollection { get; }
-
     public SkillCollection(ILogger? log = null)
     {
         this._log = log ?? NullLogger.Instance;
 
-        this.ReadOnlySkillCollection = new ReadOnlySkillCollection(this);
-
         // Important: names are case insensitive
         this._skillCollection = new(StringComparer.OrdinalIgnoreCase);
     }