Skip to content

Commit

Permalink
.Net: Cleanup from Resharper analysis (microsoft#1914)
Browse files Browse the repository at this point in the history
### Motivation and Context
<!-- Thank you for your contribution to the semantic-kernel repo!-->
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
 It helps clean the code up
  3. What problem does it solve?
 Better adhere to code standards
  5. What scenario does it contribute to?
A large range of code areas are affected, but all changes should be
neutral
  7. If it fixes an open issue, please link to the issue here.



### Description
<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
Those are a handful of suggestions coming out of a solution wide
Resharper code inspection. out of the 5000+ warnings that Resharper
oututs, not all are actionable (e.g when it thinks a public property
should be made private because it sees no use of it), but there
certainly at least another couple hundred low hanging fruits like the
ones that this PR illustrates.


### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [X] The code builds clean without any errors or warnings
- [X] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [X] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
  • Loading branch information
jsboige and shawncal committed Jul 12, 2023
1 parent 43f4cc2 commit a2d7ee7
Show file tree
Hide file tree
Showing 31 changed files with 49 additions and 105 deletions.
2 changes: 1 addition & 1 deletion dotnet/samples/KernelSyntaxExamples/Example18_DallE.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static async Task RunAsync()
await AzureOpenAIDallEAsync();
}

public static async Task OpenAIDallEAsync()
private static async Task OpenAIDallEAsync()
{
Console.WriteLine("======== OpenAI Dall-E 2 Image Generation ========");

Expand Down
2 changes: 1 addition & 1 deletion dotnet/samples/KernelSyntaxExamples/Example39_Postgres.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static async Task RunAsync()
{
NpgsqlDataSourceBuilder dataSourceBuilder = new(Env.Var("POSTGRES_CONNECTIONSTRING"));
dataSourceBuilder.UseVector();
using NpgsqlDataSource dataSource = dataSourceBuilder.Build();
await using NpgsqlDataSource dataSource = dataSourceBuilder.Build();

PostgresMemoryStore memoryStore = new(dataSource, vectorSize: 1536, schema: "public");

Expand Down
2 changes: 1 addition & 1 deletion dotnet/samples/KernelSyntaxExamples/Example47_Redis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static class Example47_Redis
public static async Task RunAsync()
{
string configuration = Env.Var("REDIS_CONFIGURATION");
using ConnectionMultiplexer connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync(configuration);
await using ConnectionMultiplexer connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync(configuration);
IDatabase database = connectionMultiplexer.GetDatabase();
RedisMemoryStore memoryStore = new(database, vectorSize: 1536);
IKernel kernel = Kernel.Builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,16 @@ namespace Microsoft.SemanticKernel.Connectors.AI.OpenAI.AzureSdk;

internal sealed class ChatResult : IChatResult, ITextResult
{
private readonly ModelResult _modelResult;
private readonly ChatChoice _choice;

public ChatResult(ChatCompletions resultData, ChatChoice choice)
{
Verify.NotNull(choice);
this._choice = choice;
this._modelResult = new ModelResult(resultData);
this.ModelResult = new ModelResult(resultData);
}

public ModelResult ModelResult => this._modelResult;
public ModelResult ModelResult { get; }

public Task<ChatMessageBase> GetChatMessageAsync(CancellationToken cancellationToken = default)
=> Task.FromResult<ChatMessageBase>(new SKChatMessage(this._choice.Message));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ namespace Microsoft.SemanticKernel.Connectors.AI.OpenAI.AzureSdk;

internal sealed class TextStreamingResult : ITextStreamingResult
{
private readonly ModelResult _modelResult;
private readonly StreamingChoice _choice;

public ModelResult ModelResult => this._modelResult;
public ModelResult ModelResult { get; }

public TextStreamingResult(StreamingCompletions resultData, StreamingChoice choice)
{
this._modelResult = new ModelResult(resultData);
this.ModelResult = new ModelResult(resultData);
this._choice = choice;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ private protected T JsonDeserialize<T>(string responseJson)

private protected async Task<HttpResponseMessage> ExecuteRequestAsync(string url, HttpMethod method, HttpContent? content, CancellationToken cancellationToken = default)
{
HttpResponseMessage? response = null;
try
{
HttpResponseMessage? response = null;
using (var request = new HttpRequestMessage(method, url))
{
this.AddRequestHeaders(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@ internal static class GPT3Settings

/// <summary>Lazy load the cached encoding table (encoder.json).</summary>
private static readonly Lazy<Dictionary<string, int>> s_encoder = new(() =>
{
return JsonSerializer.Deserialize<Dictionary<string, int>>(EmbeddedResource.ReadEncodingTable())
?? throw new AIException(AIException.ErrorCodes.InvalidConfiguration,
"Encoding table deserialization returned NULL");
});
JsonSerializer.Deserialize<Dictionary<string, int>>(
EmbeddedResource.ReadEncodingTable()) ?? throw new AIException(
AIException.ErrorCodes.InvalidConfiguration,
"Encoding table deserialization returned NULL"));

/// <summary>Lazy load the cached byte pair encoding table (vocab.bpe).</summary>
private static readonly Lazy<Dictionary<(string, string), int>> s_bpeRanks = new(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ namespace Microsoft.SemanticKernel.Connectors.Memory.Pinecone;
/// </summary>
public class PineconeMemoryException : SKException
{
/// <summary>
/// Initializes a new instance of the <see cref="PineconeMemoryException"/> class with a provided error code.
/// </summary>
/// <param name="errorCode">The error code.</param>
public PineconeMemoryException(ErrorCodes errorCode)
: this(errorCode: errorCode, message: null, innerException: null)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="PineconeMemoryException"/> class with a provided error code and message.
/// </summary>
Expand All @@ -47,7 +38,7 @@ public PineconeMemoryException(ErrorCodes errorCode, Exception? innerException)
/// <param name="errorCode">The error code.</param>
/// <param name="message">A string that describes the error.</param>
/// <param name="innerException">The exception that is the cause of the current exception.</param>
public PineconeMemoryException(ErrorCodes errorCode, string? message, Exception? innerException)
public PineconeMemoryException(ErrorCodes errorCode, string? message = null, Exception? innerException = null)
: base(message: GetDefaultMessage(errorCode: errorCode, message: message, innerException: innerException), innerException: innerException)
{
this.ErrorCode = errorCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ internal static class ValidateExtensions
{
public static void Validate(IValidatable target)
{
target?.Validate();
target.Validate();
}

public static void Validate(params IValidatable[] targets)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public WeaviateMemoryStore(string scheme, string host, int port, string? apiKey
if (httpClient == null)
{
this._httpClient = new();
this._apiKey = apiKey;
if (!string.IsNullOrEmpty(apiKey))
{
this._httpClient.DefaultRequestHeaders.Add(AuthorizationHeaderName, apiKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public async Task ItReturnsNoMatchesFromEmptyCollection()
var nearestMatch = await this._chromaMemoryStore.GetNearestMatchAsync(collectionName, searchEmbedding, withEmbedding: true);

// Assert
Assert.Null(nearestMatch.Value.Item1);
Assert.Null(nearestMatch?.Item1);
}

[Fact(Skip = SkipReason)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ private async Task DropDatabaseAsync()
{
await command.ExecuteNonQueryAsync();
}
};
}
}

private PostgresMemoryStore CreateMemoryStore()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ private NonDisposableHttpClientHandler()
/// <summary>
/// Gets the singleton instance of <see cref="NonDisposableHttpClientHandler"/>.
/// </summary>
public static NonDisposableHttpClientHandler Instance { get; } = new NonDisposableHttpClientHandler();
public static NonDisposableHttpClientHandler Instance { get; } = new();

/// <summary>
/// Disposes the underlying resources held by the <see cref="NonDisposableHttpClientHandler"/>.
Expand Down
11 changes: 1 addition & 10 deletions dotnet/src/SemanticKernel.Abstractions/AI/AIException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ namespace Microsoft.SemanticKernel.AI;
/// </summary>
public class AIException : SKException
{
/// <summary>
/// Initializes a new instance of the <see cref="AIException"/> class with a provided error code.
/// </summary>
/// <param name="errorCode">The error code.</param>
public AIException(ErrorCodes errorCode)
: this(errorCode, message: null, detail: null, innerException: null)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="AIException"/> class with a provided error code and message.
/// </summary>
Expand Down Expand Up @@ -60,7 +51,7 @@ public AIException(ErrorCodes errorCode, string? message, string? detail)
/// <param name="message">A string that describes the error.</param>
/// <param name="detail">A string that provides additional details about the error.</param>
/// <param name="innerException">The exception that is the cause of the current exception.</param>
public AIException(ErrorCodes errorCode, string? message, string? detail, Exception? innerException)
public AIException(ErrorCodes errorCode, string? message = null, string? detail = null, Exception? innerException = null)
: base(GetDefaultMessage(errorCode, message), innerException)
{
this.ErrorCode = errorCode;
Expand Down
11 changes: 1 addition & 10 deletions dotnet/src/SemanticKernel.Abstractions/KernelException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ namespace Microsoft.SemanticKernel;
/// </summary>
public class KernelException : SKException
{
/// <summary>
/// Initializes a new instance of the <see cref="KernelException"/> class with a provided error code.
/// </summary>
/// <param name="errorCode">The error code.</param>
public KernelException(ErrorCodes errorCode)
: this(errorCode, message: null, innerException: null)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="KernelException"/> class with a provided error code and message.
/// </summary>
Expand All @@ -37,7 +28,7 @@ public KernelException(ErrorCodes errorCode, string? message)
/// <param name="errorCode">The error code.</param>
/// <param name="message">A string that describes the error.</param>
/// <param name="innerException">The exception that is the cause of the current exception.</param>
public KernelException(ErrorCodes errorCode, string? message, Exception? innerException)
public KernelException(ErrorCodes errorCode, string? message = null, Exception? innerException = null)
: base(GetDefaultMessage(errorCode, message), innerException)
{
this.ErrorCode = errorCode;
Expand Down
11 changes: 1 addition & 10 deletions dotnet/src/SemanticKernel.Abstractions/Memory/MemoryException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ namespace Microsoft.SemanticKernel.Memory;
/// </summary>
public class MemoryException : SKException
{
/// <summary>
/// Initializes a new instance of the <see cref="MemoryException"/> class with a provided error code.
/// </summary>
/// <param name="error">The error code.</param>
public MemoryException(ErrorCodes error)
: this(error, message: null, innerException: null)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="MemoryException"/> class with a provided error code and message.
/// </summary>
Expand All @@ -37,7 +28,7 @@ public MemoryException(ErrorCodes errorCode, string? message)
/// <param name="errorCode">The error code.</param>
/// <param name="message">A string that describes the error.</param>
/// <param name="innerException">The exception that is the cause of the current exception.</param>
public MemoryException(ErrorCodes errorCode, string? message, Exception? innerException)
public MemoryException(ErrorCodes errorCode, string? message = null, Exception? innerException = null)
: base(GetDefaultMessage(errorCode, message), innerException)
{
this.ErrorCode = errorCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ namespace Microsoft.SemanticKernel.Planning;
/// </summary>
public class PlanningException : SKException
{
/// <summary>
/// Initializes a new instance of the <see cref="PlanningException"/> class with a provided error code.
/// </summary>
/// <param name="errorCode">The error code.</param>
public PlanningException(ErrorCodes errorCode)
: this(errorCode, message: null, innerException: null)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="PlanningException"/> class with a provided error code and message.
/// </summary>
Expand All @@ -37,7 +28,7 @@ public PlanningException(ErrorCodes errorCode, string? message)
/// <param name="errorCode">The error code.</param>
/// <param name="message">A string that describes the error.</param>
/// <param name="innerException">The exception that is the cause of the current exception.</param>
public PlanningException(ErrorCodes errorCode, string? message, Exception? innerException)
public PlanningException(ErrorCodes errorCode, string? message = null, Exception? innerException = null)
: base(GetDefaultMessage(errorCode, message), innerException)
{
this.ErrorCode = errorCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Microsoft.SemanticKernel.Services;

public interface INamedServiceProvider<TService>
public interface INamedServiceProvider<in TService>
{
/// <summary>
/// Gets the service of the specified type and name, or null if not found.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
[DebuggerDisplay("Count = 0")]
internal sealed class NullReadOnlySkillCollection : IReadOnlySkillCollection
{
public static NullReadOnlySkillCollection Instance = new();
public static readonly NullReadOnlySkillCollection Instance = new();

public ISKFunction GetFunction(string functionName)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ public void ItThrowsExceptionWhenCapacityIsInvalid()
// Arrange
const int InvalidCapacity = -1;

var action = () =>
void Action()
{
var minHeap = new MinHeap<int>(MinValue, InvalidCapacity);
};
}

// Act
var exception = Assert.Throws<ArgumentOutOfRangeException>("capacity", () => action());
var exception = Assert.Throws<ArgumentOutOfRangeException>("capacity", Action);

// Assert
Assert.Equal(-1, exception.ActualValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private static Mock<IPromptTemplate> MockPromptTemplate()

promptTemplate
.Setup(x => x.GetParameters())
.Returns(new List<ParameterView>()); ;
.Returns(new List<ParameterView>());

return promptTemplate;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void ItDoesntThrowForValidFunctionsViaDelegate()

// Act
Assert.Equal(methods.Length, functions.Length);
Assert.All(functions, f => Assert.NotNull(f));
Assert.All(functions, Assert.NotNull);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ internal static ReadOnlySpan<TNumber> AsReadOnlySpan<TNumber>(this TNumber[] vec

internal static ReadOnlySpan<TNumber> AsReadOnlySpan<TNumber>(this Span<TNumber> span)
{
return (ReadOnlySpan<TNumber>)span;
return span;
}
}
7 changes: 1 addition & 6 deletions dotnet/src/SemanticKernel/Memory/Collections/MinHeap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ internal sealed class MinHeap<T> : IEnumerable<T> where T : IComparable<T>
private T[] _items;
private int _count;

public MinHeap(T minValue)
: this(minValue, DefaultCapacity)
{
}

public MinHeap(T minValue, int capacity)
public MinHeap(T minValue, int capacity = DefaultCapacity)
{
if (capacity < MinCapacity)
{
Expand Down
2 changes: 1 addition & 1 deletion dotnet/src/SemanticKernel/Services/AIServiceCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class AIServiceCollection
/// <param name="service">The service instance.</param>
/// <exception cref="ArgumentNullException">The service instance is null.</exception>
public void SetService<T>(T service) where T : IAIService
=> this.SetService<T>(DefaultKey, service, true);
=> this.SetService(DefaultKey, service, true);

/// <summary>
/// Registers a singleton service instance with an optional name and default flag.
Expand Down
11 changes: 1 addition & 10 deletions dotnet/src/SemanticKernel/TemplateEngine/TemplateException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ namespace Microsoft.SemanticKernel.TemplateEngine;
/// </summary>
public class TemplateException : SKException
{
/// <summary>
/// Initializes a new instance of the <see cref="TemplateException"/> class with a provided error code.
/// </summary>
/// <param name="error">The error code.</param>
public TemplateException(ErrorCodes error)
: this(error, message: null, innerException: null)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="TemplateException"/> class with a provided error code and message.
/// </summary>
Expand All @@ -37,7 +28,7 @@ public TemplateException(ErrorCodes errorCode, string? message)
/// <param name="errorCode">The error code.</param>
/// <param name="message">A string that describes the error.</param>
/// <param name="innerException">The exception that is the cause of the current exception.</param>
public TemplateException(ErrorCodes errorCode, string? message, Exception? innerException)
public TemplateException(ErrorCodes errorCode, string? message = null, Exception? innerException = null)
: base(GetDefaultMessage(errorCode, message), innerException)
{
this.ErrorCode = errorCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,18 @@ internal static class GrpcOperationExtensions
/// <returns>The list of parameters.</returns>
public static IReadOnlyList<ParameterView> GetParameters(this GrpcOperation operation)
{
var parameters = new List<ParameterView>();
var parameters = new List<ParameterView>
{
// Register the "address" parameter so that it's possible to override it if needed.
new ParameterView(GrpcOperation.AddressArgumentName,
"Address for gRPC channel to use.",
string.Empty),

// Register the "address" parameter so that it's possible to override it if needed.
parameters.Add(new ParameterView(GrpcOperation.AddressArgumentName, "Address for gRPC channel to use.", string.Empty));

// Register the "payload" parameter to be used as gRPC operation request message.
parameters.Add(new ParameterView(GrpcOperation.PayloadArgumentName, "gRPC request message.", string.Empty));
// Register the "payload" parameter to be used as gRPC operation request message.
new ParameterView(GrpcOperation.PayloadArgumentName,
"gRPC request message.",
string.Empty)
};

return parameters;
}
Expand Down
Loading

0 comments on commit a2d7ee7

Please sign in to comment.