Fix typos and code style (microsoft#377)

* Extension methods: use the same namespace of the class being extended * Fix typos * Removed unused vars and params * Use correct cancellation token
banreyms · Apr 10, 2023 · 98a38fa · 98a38fa
1 parent 0d9727a
commit 98a38fa
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 46 deletions.
diff --git a/dotnet/src/SemanticKernel.Skills/Skills.OpenAPI/Extensions/KernelChatGptPluginExtensions.cs b/dotnet/src/SemanticKernel.Skills/Skills.OpenAPI/Extensions/KernelChatGptPluginExtensions.cs
@@ -14,7 +14,8 @@
 using Microsoft.SemanticKernel.Skills.OpenAPI.Authentication;
 using Microsoft.SemanticKernel.Skills.OpenAPI.Skills;
 
-namespace Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
+// ReSharper disable once CheckNamespace
+namespace Microsoft.SemanticKernel;
 
 /// <summary>
 /// Class for extensions methods for IKernel interface.
@@ -32,7 +33,12 @@ public static class KernelChatGptPluginExtensions
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>A list of all the semantic functions representing the skill.</returns>
     public static async Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSkillFromUrlAsync(
-        this IKernel kernel, string skillName, Uri url, HttpClient? httpClient = null, AuthenticateRequestAsyncCallback? authCallback = null, CancellationToken cancellationToken = default)
+        this IKernel kernel,
+        string skillName,
+        Uri url,
+        HttpClient? httpClient = null,
+        AuthenticateRequestAsyncCallback? authCallback = null,
+        CancellationToken cancellationToken = default)
     {
         Verify.ValidSkillName(skillName);
 
@@ -47,7 +53,7 @@ public static async Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSk
                 //  log: null);
 
                 //using HttpClient client = new HttpClient(retryHandler, false);
-                using HttpClient client = new HttpClient();
+                using HttpClient client = new();
 
                 response = await client.GetAsync(url, cancellationToken).ConfigureAwait(false);
             }
@@ -83,7 +89,8 @@ public static async Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSk
     /// <returns>A list of all the semantic functions representing the skill.</returns>
     public static async Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSkillFromResourceAsync(
         this IKernel kernel,
-        string skillName, HttpClient? httpClient = null,
+        string skillName,
+        HttpClient? httpClient = null,
         AuthenticateRequestAsyncCallback? authCallback = null,
         CancellationToken cancellationToken = default)
     {
@@ -135,12 +142,10 @@ public async static Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSk
         var chatGptPluginPath = Path.Join(skillDir, CHATGPT_PLUGIN_FILE);
         if (!File.Exists(chatGptPluginPath))
         {
-            throw new FileNotFoundException($"No ChatGPT plugin for the specified path - {chatGptPluginPath} is found.");
+            throw new FileNotFoundException($"No ChatGPT plugin for the specified path - {chatGptPluginPath} is found");
         }
 
-        kernel.Log.LogTrace("Registering Rest functions from {0} ChatGPT Plugin.", chatGptPluginPath);
-
-        var skill = new Dictionary<string, ISKFunction>();
+        kernel.Log.LogTrace("Registering Rest functions from {0} ChatGPT Plugin", chatGptPluginPath);
 
         using var stream = File.OpenRead(chatGptPluginPath);
 
@@ -153,15 +158,13 @@ public async static Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSk
     /// <param name="kernel">Semantic Kernel instance.</param>
     /// <param name="skillName">Name of the skill to register.</param>
     /// <param name="filePath">File path to the ChatGPT plugin definition.</param>
-    /// <param name="httpClient">Optional HttpClient to use for the request.</param>
     /// <param name="authCallback">Optional callback for adding auth data to the API requests.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>A list of all the semantic functions representing the skill.</returns>
     public async static Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSkillSkillFromFileAsync(
         this IKernel kernel,
         string skillName,
         string filePath,
-        HttpClient? httpClient = null,
         AuthenticateRequestAsyncCallback? authCallback = null,
         CancellationToken cancellationToken = default)
     {
@@ -172,8 +175,6 @@ public async static Task<IDictionary<string, ISKFunction>> ImportChatGptPluginSk
 
         kernel.Log.LogTrace("Registering Rest functions from {0} ChatGPT Plugin.", filePath);
 
-        var skill = new Dictionary<string, ISKFunction>();
-
         using var stream = File.OpenRead(filePath);
 
         return await kernel.RegisterOpenApiSkillAsync(stream, skillName, authCallback, cancellationToken);
@@ -186,13 +187,13 @@ private static string ParseOpenApiUrl(string gptPluginJson)
         string? apiType = gptPlugin?["api"]?["type"]?.ToString();
         if (string.IsNullOrWhiteSpace(apiType) || apiType != "openapi")
         {
-            throw new InvalidOperationException($"Invalid ChatGPT plugin document. Supported api types are: openapi.");
+            throw new InvalidOperationException($"Invalid ChatGPT plugin document. Supported api types are: openapi");
         }
 
         string? openApiUrl = gptPlugin?["api"]?["url"]?.ToString();
         if (string.IsNullOrWhiteSpace(openApiUrl))
         {
-            throw new InvalidOperationException($"Invalid ChatGPT plugin document. OpenAPI url is missing.");
+            throw new InvalidOperationException($"Invalid ChatGPT plugin document. OpenAPI url is missing");
         }
 
         return openApiUrl;

diff --git a/dotnet/src/SemanticKernel.Skills/Skills.OpenAPI/Extensions/KernelOpenApiExtensions.cs b/dotnet/src/SemanticKernel.Skills/Skills.OpenAPI/Extensions/KernelOpenApiExtensions.cs
@@ -18,7 +18,8 @@
 using Microsoft.SemanticKernel.Skills.OpenAPI.Rest;
 using Microsoft.SemanticKernel.Skills.OpenAPI.Skills;
 
-namespace Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
+// ReSharper disable once CheckNamespace
+namespace Microsoft.SemanticKernel;
 
 /// <summary>
 /// Class for extensions methods for <see cref="IKernel"/> interface.
@@ -92,7 +93,11 @@ public static async Task<IDictionary<string, ISKFunction>> ImportOpenApiSkillFro
     /// <param name="authCallback">Optional callback for adding auth data to the API requests.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>A list of all the semantic functions representing the skill.</returns>
-    public static Task<IDictionary<string, ISKFunction>> ImportOpenApiSkillFromResourceAsync(this IKernel kernel, string skillName, AuthenticateRequestAsyncCallback? authCallback = null, CancellationToken cancellationToken = default)
+    public static Task<IDictionary<string, ISKFunction>> ImportOpenApiSkillFromResourceAsync(
+        this IKernel kernel,
+        string skillName,
+        AuthenticateRequestAsyncCallback? authCallback = null,
+        CancellationToken cancellationToken = default)
     {
         Verify.ValidSkillName(skillName);
 
@@ -157,14 +162,19 @@ public static async Task<IDictionary<string, ISKFunction>> ImportOpenApiSkillFro
     /// <param name="authCallback">Optional callback for adding auth data to the API requests.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>A list of all the semantic functions representing the skill.</returns>
-    public static async Task<IDictionary<string, ISKFunction>> ImportOpenApiSkillFromFileAsync(this IKernel kernel, string skillName, string filePath, AuthenticateRequestAsyncCallback? authCallback = null, CancellationToken cancellationToken = default)
+    public static async Task<IDictionary<string, ISKFunction>> ImportOpenApiSkillFromFileAsync(
+        this IKernel kernel,
+        string skillName,
+        string filePath,
+        AuthenticateRequestAsyncCallback? authCallback = null,
+        CancellationToken cancellationToken = default)
     {
         if (!File.Exists(filePath))
         {
             throw new FileNotFoundException($"No OpenApi document for the specified path - {filePath} is found.");
         }
 
-        kernel.Log.LogTrace("Registering Rest functions from {0} OpenApi document.", filePath);
+        kernel.Log.LogTrace("Registering Rest functions from {0} OpenApi document", filePath);
 
         // TODO: never used, why?
         var skill = new Dictionary<string, ISKFunction>();
@@ -183,7 +193,12 @@ public static async Task<IDictionary<string, ISKFunction>> ImportOpenApiSkillFro
     /// <param name="authCallback">Optional callback for adding auth data to the API requests.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>A list of all the semantic functions representing the skill.</returns>
-    public static async Task<IDictionary<string, ISKFunction>> RegisterOpenApiSkillAsync(this IKernel kernel, Stream documentStream, string skillName, AuthenticateRequestAsyncCallback? authCallback = null, CancellationToken cancellationToken = default)
+    public static async Task<IDictionary<string, ISKFunction>> RegisterOpenApiSkillAsync(
+        this IKernel kernel,
+        Stream documentStream,
+        string skillName,
+        AuthenticateRequestAsyncCallback? authCallback = null,
+        CancellationToken cancellationToken = default)
     {
         Verify.NotNull(kernel, nameof(kernel));
         Verify.ValidSkillName(skillName);
@@ -199,15 +214,15 @@ public static async Task<IDictionary<string, ISKFunction>> RegisterOpenApiSkillA
         {
             try
             {
-                kernel.Log.LogTrace("Registering Rest function {0}.{1}.", skillName, operation.Id);
+                kernel.Log.LogTrace("Registering Rest function {0}.{1}", skillName, operation.Id);
                 var function = kernel.RegisterRestApiFunction(skillName, operation, authCallback, cancellationToken);
                 skill[function.Name] = function;
             }
             catch (Exception ex) when (!ex.IsCriticalException())
             {
                 //Logging the exception and keep registering other Rest functions
-                kernel.Log.LogWarning(ex, "Something went wrong while rendering the Rest function. Function: {0}.{1}. Error: {2}", skillName, operation.Id,
-                    ex.Message);
+                kernel.Log.LogWarning(ex, "Something went wrong while rendering the Rest function. Function: {0}.{1}. Error: {2}",
+                    skillName, operation.Id, ex.Message);
             }
         }
 
@@ -225,7 +240,12 @@ public static async Task<IDictionary<string, ISKFunction>> RegisterOpenApiSkillA
     /// <param name="authCallback">Optional callback for adding auth data to the API requests.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>An instance of <see cref="SKFunction"/> class.</returns>
-    private static ISKFunction RegisterRestApiFunction(this IKernel kernel, string skillName, RestApiOperation operation, AuthenticateRequestAsyncCallback? authCallback = null, CancellationToken cancellationToken = default)
+    private static ISKFunction RegisterRestApiFunction(
+        this IKernel kernel,
+        string skillName,
+        RestApiOperation operation,
+        AuthenticateRequestAsyncCallback? authCallback = null,
+        CancellationToken cancellationToken = default)
     {
         var restOperationParameters = operation.GetParameters();
 
@@ -260,7 +280,7 @@ async Task<SKContext> ExecuteAsync(SKContext context)
                     }
                 }
 
-                var result = await runner.RunAsync(operation, arguments, context.CancellationToken).ConfigureAwait(false);
+                var result = await runner.RunAsync(operation, arguments, cancellationToken).ConfigureAwait(false);
                 if (result != null)
                 {
                     context.Variables.Update(result.ToString());
@@ -276,18 +296,21 @@ async Task<SKContext> ExecuteAsync(SKContext context)
             return context;
         }
 
-        // TODO: to be fixed later
-#pragma warning disable CA2000 // Dispose objects before losing scope.
-        var function = new SKFunction(
-            delegateType: SKFunction.DelegateTypes.ContextSwitchInSKContextOutTaskSKContext,
-            delegateFunction: ExecuteAsync,
-            parameters: restOperationParameters.Select(p => new ParameterView()
+        var parameters = restOperationParameters
+            .Select(p => new ParameterView
             {
                 Name = p.AlternativeName ?? p.Name,
                 Description = p.Name,
                 DefaultValue = p.DefaultValue ?? string.Empty
             })
-                .ToList(), //functionConfig.PromptTemplate.GetParameters(),
+            .ToList();
+
+        // TODO: to be fixed later
+#pragma warning disable CA2000 // Dispose objects before losing scope.
+        var function = new SKFunction(
+            delegateType: SKFunction.DelegateTypes.ContextSwitchInSKContextOutTaskSKContext,
+            delegateFunction: ExecuteAsync,
+            parameters: parameters,
             description: operation.Description,
             skillName: skillName,
             functionName: operation.Id,

diff --git a/.../Extensions/RestApiOperationExtensions.cs → ...ions/RestApiOperationOpenAPIExtensions.cs b/.../Extensions/RestApiOperationExtensions.cs → ...ions/RestApiOperationOpenAPIExtensions.cs
@@ -3,14 +3,14 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Text.RegularExpressions;
-using Microsoft.SemanticKernel.Skills.OpenAPI.Model;
 
-namespace Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
+// ReSharper disable once CheckNamespace
+namespace Microsoft.SemanticKernel.Skills.OpenAPI.Model;
 
 /// <summary>
 /// Class for extensions methods for the <see cref="RestApiOperation"/> class.
 /// </summary>
-internal static class RestApiOperationExtensions
+internal static class RestApiOperationOpenAPIExtensions
 {
     /// <summary>
     /// Returns list of REST API operation parameters.

diff --git a/dotnet/src/SemanticKernel.Skills/Skills.OpenAPI/OpenApi/OpenApiDocumentParser.cs b/dotnet/src/SemanticKernel.Skills/Skills.OpenAPI/OpenApi/OpenApiDocumentParser.cs
@@ -28,7 +28,7 @@ internal class OpenApiDocumentParser : IOpenApiDocumentParser
     /// <inheritdoc/>
     public async Task<IList<RestApiOperation>> ParseAsync(Stream stream, CancellationToken cancellationToken = default)
     {
-        var jsonObject = await this.DowngradeDocumentVersionToSuportedOneAsync(stream, cancellationToken).ConfigureAwait(false);
+        var jsonObject = await this.DowngradeDocumentVersionToSupportedOneAsync(stream, cancellationToken).ConfigureAwait(false);
 
         using var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(jsonObject.ToJson()));
 
@@ -54,7 +54,7 @@ public async Task<IList<RestApiOperation>> ParseAsync(Stream stream, Cancellatio
     /// <param name="stream">The original OpenAPI document stream.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>OpenAPI document with downgraded document version.</returns>
-    private async Task<JsonObject> DowngradeDocumentVersionToSuportedOneAsync(Stream stream, CancellationToken cancellationToken)
+    private async Task<JsonObject> DowngradeDocumentVersionToSupportedOneAsync(Stream stream, CancellationToken cancellationToken)
     {
         var jsonObject = await ConvertContentToJsonAsync(stream, cancellationToken);
         if (jsonObject == null)
@@ -63,7 +63,7 @@ private async Task<JsonObject> DowngradeDocumentVersionToSuportedOneAsync(Stream
             throw new OpenApiDocumentParsingException($"Parsing of OpenAPI document failed.");
         }
 
-        if (!jsonObject.TryGetPropertyValue(OpenApiVersionPropetyName, out var propertyNode))
+        if (!jsonObject.TryGetPropertyValue(OpenApiVersionPropertyName, out var propertyNode))
         {
             //The document is either malformed or has 2.x version that specifies document version in the 'swagger' property rather than in the 'openapi' one.
             return jsonObject;
@@ -83,18 +83,18 @@ private async Task<JsonObject> DowngradeDocumentVersionToSuportedOneAsync(Stream
 
         if (version > s_latestSupportedVersion)
         {
-            jsonObject[OpenApiVersionPropetyName] = s_latestSupportedVersion.ToString();
+            jsonObject[OpenApiVersionPropertyName] = s_latestSupportedVersion.ToString();
         }
 
         return jsonObject;
     }
 
     /// <summary>
     /// Converts YAML content to JSON content.
-    /// The method uses SharpYaml library that comes as a not-direct dependency of Microsoft.AopenAPI.NET library.
+    /// The method uses SharpYaml library that comes as a not-direct dependency of Microsoft.OpenAPI.NET library.
     /// Should be replaced later when there's more convenient way to convert YAML content to JSON one.
     /// </summary>
-    /// <param name="stream">The JAML/JSON content stream.</param>
+    /// <param name="stream">The YAML/JSON content stream.</param>
     /// <param name="cancellationToken">Cancellation token.</param>
     /// <returns>JSON content stream.</returns>
     private static async Task<JsonObject?> ConvertContentToJsonAsync(Stream stream, CancellationToken cancellationToken = default)
@@ -371,7 +371,7 @@ private static IList<RestApiOperationPayloadProperty> GetPayloadProperties(strin
     /// <summary>
     /// Name of property that contains OpenAPI document version.
     /// </summary>
-    private const string OpenApiVersionPropetyName = "openapi";
+    private const string OpenApiVersionPropertyName = "openapi";
 
     #endregion
 }
diff --git a/dotnet/src/SemanticKernel.Skills/Skills.UnitTests/OpenAPI/OpenApiDocumentParserV20Tests.cs b/dotnet/src/SemanticKernel.Skills/Skills.UnitTests/OpenAPI/OpenApiDocumentParserV20Tests.cs
@@ -6,7 +6,6 @@
 using System.Linq;
 using System.Net.Http;
 using System.Threading.Tasks;
-using Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
 using Microsoft.SemanticKernel.Skills.OpenAPI.Model;
 using Microsoft.SemanticKernel.Skills.OpenAPI.OpenApi;
 using SemanticKernel.Skills.UnitTests.OpenAPI.TestSkills;

diff --git a/dotnet/src/SemanticKernel.Skills/Skills.UnitTests/OpenAPI/OpenApiDocumentParserV30Tests.cs b/dotnet/src/SemanticKernel.Skills/Skills.UnitTests/OpenAPI/OpenApiDocumentParserV30Tests.cs
@@ -6,7 +6,6 @@
 using System.Linq;
 using System.Net.Http;
 using System.Threading.Tasks;
-using Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
 using Microsoft.SemanticKernel.Skills.OpenAPI.Model;
 using Microsoft.SemanticKernel.Skills.OpenAPI.OpenApi;
 using SemanticKernel.Skills.UnitTests.OpenAPI.TestSkills;

diff --git a/dotnet/src/SemanticKernel.Skills/Skills.UnitTests/OpenAPI/OpenApiDocumentParserV31Tests.cs b/dotnet/src/SemanticKernel.Skills/Skills.UnitTests/OpenAPI/OpenApiDocumentParserV31Tests.cs
@@ -6,7 +6,6 @@
 using System.Linq;
 using System.Net.Http;
 using System.Threading.Tasks;
-using Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
 using Microsoft.SemanticKernel.Skills.OpenAPI.Model;
 using Microsoft.SemanticKernel.Skills.OpenAPI.OpenApi;
 using SemanticKernel.Skills.UnitTests.OpenAPI.TestSkills;

diff --git a/samples/dotnet/kernel-syntax-examples/Example21_ChatGPTPlugins.cs b/samples/dotnet/kernel-syntax-examples/Example21_ChatGPTPlugins.cs
@@ -4,7 +4,6 @@
 using System.Threading.Tasks;
 using Microsoft.SemanticKernel;
 using Microsoft.SemanticKernel.Orchestration;
-using Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
 using RepoUtils;
 
 // ReSharper disable once InconsistentNaming

diff --git a/samples/dotnet/kernel-syntax-examples/Example22_OpenApiSkill.cs b/samples/dotnet/kernel-syntax-examples/Example22_OpenApiSkill.cs
@@ -6,7 +6,6 @@
 using System.Threading.Tasks;
 using Microsoft.SemanticKernel;
 using Microsoft.SemanticKernel.Orchestration;
-using Microsoft.SemanticKernel.Skills.OpenAPI.Extensions;
 using Microsoft.SemanticKernel.Skills.OpenAPI.Skills;
 using RepoUtils;