Skip to content

Commit

Permalink
Enable more C# code style checks + fixes (microsoft#1142)
Browse files Browse the repository at this point in the history
1. Fix missing `this.` and `Async` suffix
2. Fix some indentation, spacing, etc.
3. Enable these rules and format accordingly:

IDE0001: Simplify name
IDE0005: Remove unnecessary using directives
IDE0009: Add this or Me qualification
IDE0011: Add braces
IDE0018: Inline variable declaration
IDE0032: Use auto-implemented property
IDE0034: Simplify 'default' expression
IDE0035: Remove unreachable code
IDE0040: Add accessibility modifiers
IDE0049: Use language keywords instead of framework type names for type
references
IDE0050: Convert anonymous type to tuple
IDE0051: Remove unused private member
IDE0055: Formatting rule
IDE0060: Remove unused parameter
IDE0070: Use 'System.HashCode.Combine'
IDE0071: Simplify interpolation
IDE0073: Require file header
IDE0082: Convert typeof to nameof
IDE0090: Simplify new expression
IDE0161: Use file-scoped namespace
  • Loading branch information
dluc authored and name committed Jul 5, 2023
1 parent 6eb0815 commit 9578399
Show file tree
Hide file tree
Showing 39 changed files with 126 additions and 115 deletions.
38 changes: 22 additions & 16 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,27 @@ dotnet_diagnostic.CA1860.severity = warning # Prefer comparing 'Count' to 0 rath
dotnet_diagnostic.CA2000.severity = warning # Call System.IDisposable.Dispose on object before all references to it are out of scope
dotnet_diagnostic.CA2201.severity = warning # Exception type System.Exception is not sufficiently specific

dotnet_diagnostic.IDE0005.severity = warning # Remove unnecessary usings
dotnet_diagnostic.IDE0001.severity = warning # Simplify name
dotnet_diagnostic.IDE0005.severity = warning # Remove unnecessary using directives
dotnet_diagnostic.IDE0009.severity = warning # Add this or Me qualification
dotnet_diagnostic.IDE0011.severity = warning # Add braces
dotnet_diagnostic.IDE0018.severity = warning # Inline variable declaration
dotnet_diagnostic.IDE0032.severity = warning # Use auto-implemented property
dotnet_diagnostic.IDE0034.severity = warning # Simplify 'default' expression
dotnet_diagnostic.IDE0035.severity = warning # Remove unreachable code
dotnet_diagnostic.IDE0040.severity = warning # Add accessibility modifiers
dotnet_diagnostic.IDE0049.severity = warning # Use language keywords instead of framework type names for type references
dotnet_diagnostic.IDE0050.severity = warning # Convert anonymous type to tuple
dotnet_diagnostic.IDE0051.severity = warning # Remove unused private member
dotnet_diagnostic.IDE0055.severity = warning # Formatting rule
dotnet_diagnostic.IDE0060.severity = warning # Remove unused parameter
dotnet_diagnostic.IDE0070.severity = warning # Use 'System.HashCode.Combine'
dotnet_diagnostic.IDE0071.severity = warning # Simplify interpolation
dotnet_diagnostic.IDE0073.severity = warning # Require file header
dotnet_diagnostic.IDE0082.severity = warning # Convert typeof to nameof
dotnet_diagnostic.IDE0090.severity = warning # Simplify new expression
dotnet_diagnostic.IDE0130.severity = warning # Namespace does not match folder structure
dotnet_diagnostic.IDE0161.severity = warning # Use file-scoped namespace

# Suppressed diagnostics
dotnet_diagnostic.CA1002.severity = none # Change 'List<string>' in '...' to use 'Collection<T>' ...
Expand Down Expand Up @@ -276,7 +295,7 @@ dotnet_naming_rule.async_methods_end_in_async.severity = error
###############################
# C# Coding Conventions #
###############################
[*.cs]

# var preferences
csharp_style_var_for_built_in_types = false:none
csharp_style_var_when_type_is_apparent = false:none
Expand Down Expand Up @@ -306,7 +325,7 @@ csharp_style_inlined_variable_declaration = true:suggestion
###############################
# C# Formatting Rules #
###############################
[*.cs]

# New line preferences
csharp_new_line_before_open_brace = all
csharp_new_line_before_else = true
Expand Down Expand Up @@ -344,19 +363,6 @@ csharp_style_prefer_top_level_statements = true:silent
csharp_style_expression_bodied_lambdas = true:silent
csharp_style_expression_bodied_local_functions = false:silent

##################
# C# Style #
##################

###############################
# VB Coding Conventions #
###############################
[*.vb]
trim_trailing_whitespace = true
tab_width = 4
indent_size = 4
# Modifier preferences
visual_basic_preferred_modifier_order = Partial,Default,Private,Protected,Public,Friend,NotOverridable,Overridable,MustOverride,Overloads,Overrides,MustInherit,NotInheritable,Static,Shared,Shadows,ReadOnly,WriteOnly,Dim,Const,WithEvents,Widening,Narrowing,Custom,Async:suggestion

###############################
# Java Coding Conventions #
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dotnet-format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,5 @@ jobs:
run: |
for csproj in ${{ steps.find-csproj.outputs.csproj_files }}; do
echo "Running dotnet format on $csproj"
dotnet format $csproj --verify-no-changes --verbosity detailed
dotnet format $csproj --verify-no-changes --verbosity diagnostic
done
1 change: 1 addition & 0 deletions dotnet/SK-dotnet.sln
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
Directory.Build.targets = Directory.Build.targets
Directory.Packages.props = Directory.Packages.props
..\README.md = ..\README.md
..\.github\workflows\dotnet-format.yml = ..\.github\workflows\dotnet-format.yml
EndProjectSection
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SemanticKernel.UnitTests", "src\SemanticKernel.UnitTests\SemanticKernel.UnitTests.csproj", "{37E39C68-5A40-4E63-9D3C-0C66AD98DFCB}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private static List<string> BytePairEncoding(string token)
return list;
}

List<string> word = new List<string>(token.Length);
List<string> word = new(token.Length);
foreach (char c in token)
{
word.Add(c.ToString());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft.All rights reserved.
// Copyright (c) Microsoft. All rights reserved.

using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -223,7 +223,7 @@ public async Task TestGetNearestMatchesAsync()
// Arrange
Embedding<float> embedding = new(new float[] { 0.1f, 0.2f });

List<(PineconeDocument, double)> queryResults = new List<(PineconeDocument, double)>
List<(PineconeDocument, double)> queryResults = new()
{
new(new()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ public class QdrantMemoryStoreTests
private readonly string _description = "description";
private readonly string _description2 = "description2";
private readonly string _description3 = "description3";
private readonly Embedding<float> _embedding = new Embedding<float>(new float[] { 1, 1, 1 });
private readonly Embedding<float> _embedding2 = new Embedding<float>(new float[] { 2, 2, 2 });
private readonly Embedding<float> _embedding3 = new Embedding<float>(new float[] { 3, 3, 3 });
private readonly Embedding<float> _embedding = new(new float[] { 1, 1, 1 });
private readonly Embedding<float> _embedding2 = new(new float[] { 2, 2, 2 });
private readonly Embedding<float> _embedding3 = new(new float[] { 3, 3, 3 });

[Fact]
public void ConnectionCanBeInitialized()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ public class QdrantMemoryStoreTests2
private readonly string _description = "description";
private readonly string _description2 = "description2";
private readonly string _description3 = "description3";
private readonly Embedding<float> _embedding = new Embedding<float>(new float[] { 1, 1, 1 });
private readonly Embedding<float> _embedding2 = new Embedding<float>(new float[] { 2, 2, 2 });
private readonly Embedding<float> _embedding3 = new Embedding<float>(new float[] { 3, 3, 3 });
private readonly Embedding<float> _embedding = new(new float[] { 1, 1, 1 });
private readonly Embedding<float> _embedding2 = new(new float[] { 2, 2, 2 });
private readonly Embedding<float> _embedding3 = new(new float[] { 3, 3, 3 });

[Fact]
public async Task GetAsyncCallsDoNotRequestVectorsUnlessSpecifiedAsync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class QdrantMemoryStoreTests3
private readonly string _id = "Id";
private readonly string _text = "text";
private readonly string _description = "description";
private readonly Embedding<float> _embedding = new Embedding<float>(new float[] { 1, 1, 1 });
private readonly Embedding<float> _embedding = new(new float[] { 1, 1, 1 });

[Fact]
public async Task GetNearestMatchesAsyncCallsDoNotReturnVectorsUnlessSpecifiedAsync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
using NpgsqlConnection conn = new NpgsqlConnection(string.Format(CultureInfo.CurrentCulture, ConnectionString, "postgres"));
using NpgsqlConnection conn = new(string.Format(CultureInfo.CurrentCulture, ConnectionString, "postgres"));
conn.Open();
#pragma warning disable CA2100 // Review SQL queries for security vulnerabilities
using NpgsqlCommand command = new NpgsqlCommand($"DROP DATABASE IF EXISTS \"{this._databaseName}\"", conn);
using NpgsqlCommand command = new($"DROP DATABASE IF EXISTS \"{this._databaseName}\"", conn);
#pragma warning restore CA2100 // Review SQL queries for security vulnerabilities
command.ExecuteNonQuery();
}
Expand All @@ -63,9 +63,9 @@ protected virtual void Dispose(bool disposing)

private async Task TryCreateDatabaseAsync()
{
using NpgsqlConnection conn = new NpgsqlConnection(string.Format(CultureInfo.CurrentCulture, ConnectionString, "postgres"));
using NpgsqlConnection conn = new(string.Format(CultureInfo.CurrentCulture, ConnectionString, "postgres"));
await conn.OpenAsync();
using NpgsqlCommand checkCmd = new NpgsqlCommand("SELECT COUNT(*) FROM pg_database WHERE datname = @databaseName", conn);
using NpgsqlCommand checkCmd = new("SELECT COUNT(*) FROM pg_database WHERE datname = @databaseName", conn);
checkCmd.Parameters.AddWithValue("@databaseName", this._databaseName);

var count = (long?)await checkCmd.ExecuteScalarAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public Embedding(IEnumerable<TEmbedding> vector, bool transferOwnership)
// Create a local, protected copy if transferOwnership is false or if the vector is not an array.
// If the vector is an array and transferOwnership is true, then we can use the array directly.
this._vector =
transferOwnership && vector is TEmbedding[] array ? array :
vector.ToArray();
transferOwnership && vector is TEmbedding[] array ? array : vector.ToArray();
}

private static void ThrowNotSupportedEmbedding() =>
Expand Down Expand Up @@ -205,7 +204,7 @@ public static class Embedding
/// <typeparam name="TEmbedding">The type to be checked.</typeparam>
/// <returns>
/// <see langword="true"/> if the type is supported; otherwise, <see langword="true"/>.
/// Currently only <see cref="Single"/> and <see cref="Double"/> are supported.
/// Currently only <see cref="float"/> and <see cref="double"/> are supported.
/// </returns>
public static bool IsSupported<TEmbedding>() => typeof(TEmbedding) == typeof(float) || typeof(TEmbedding) == typeof(double);

Expand All @@ -215,7 +214,7 @@ public static class Embedding
/// <param name="type">The type to be checked.</param>
/// <returns>
/// <see langword="true"/> if the type is supported; otherwise, <see langword="true"/>.
/// Currently only <see cref="Single"/> and <see cref="Double"/> are supported.
/// Currently only <see cref="float"/> and <see cref="double"/> are supported.
/// </returns>
public static bool IsSupported(Type type) => type == typeof(float) || type == typeof(double);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,9 @@ public ContextVariables Clone()

[DebuggerBrowsable(DebuggerBrowsableState.Never)]
internal string DebuggerDisplay =>
this._variables.TryGetValue(MainKey, out string input) && !string.IsNullOrEmpty(input) ?
$"Variables = {this._variables.Count}, Input = {input}" :
$"Variables = {this._variables.Count}";
this._variables.TryGetValue(MainKey, out string input) && !string.IsNullOrEmpty(input)
? $"Variables = {this._variables.Count}, Input = {input}"
: $"Variables = {this._variables.Count}";

#region private ================================================================================

Expand All @@ -180,10 +180,10 @@ private sealed class TypeProxy
{
private readonly ContextVariables _variables;

public TypeProxy(ContextVariables variables) => _variables = variables;
public TypeProxy(ContextVariables variables) => this._variables = variables;

[DebuggerBrowsable(DebuggerBrowsableState.RootHidden)]
public KeyValuePair<string, string>[] Items => _variables._variables.ToArray();
public KeyValuePair<string, string>[] Items => this._variables._variables.ToArray();
}

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,5 @@ public FunctionView(
}

[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private string DebuggerDisplay => $"{Name} ({Description})";
private string DebuggerDisplay => $"{this.Name} ({this.Description})";
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,5 +111,5 @@ public bool IsNative(string skillName, string functionName)
}

[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private string DebuggerDisplay => $"Native = {NativeFunctions.Count}, Semantic = {SemanticFunctions.Count}";
private string DebuggerDisplay => $"Native = {this.NativeFunctions.Count}, Semantic = {this.SemanticFunctions.Count}";
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public ParameterView(
}

[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private string DebuggerDisplay => string.IsNullOrEmpty(this.Description) ?
this.Name :
$"{this.Name} ({this.Description})";
private string DebuggerDisplay => string.IsNullOrEmpty(this.Description)
? this.Name
: $"{this.Name} ({this.Description})";
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ public void ItDoesntCopyVectorWhenCastingToSpan()
public void ItTransfersOwnershipWhenRequested()
{
// Assert
Assert.False(ReferenceEquals(_vector, new Embedding<float>(_vector).Vector));
Assert.False(ReferenceEquals(_vector, new Embedding<float>(_vector, transferOwnership: false).Vector));
Assert.True(ReferenceEquals(_vector, new Embedding<float>(_vector, transferOwnership: true).Vector));
Assert.False(ReferenceEquals(this._vector, new Embedding<float>(this._vector).Vector));
Assert.False(ReferenceEquals(this._vector, new Embedding<float>(this._vector, transferOwnership: false).Vector));
Assert.True(ReferenceEquals(this._vector, new Embedding<float>(this._vector, transferOwnership: true).Vector));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class MemoryRecordTests
private readonly string _description = "description";
private readonly string _externalSourceName = "externalSourceName";
private readonly string _additionalMetadata = "value";
private readonly Embedding<float> _embedding = new Embedding<float>(new float[] { 1, 2, 3 });
private readonly Embedding<float> _embedding = new(new float[] { 1, 2, 3 });

[Fact]
public void ItCanBeConstructedFromMetadataAndVector()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ public async Task ItSupportsType15Async()
// Arrange
[SKFunction("Test")]
[SKFunctionName("Test")]
static Task Test(string input)
static Task TestAsync(string input)
{
s_canary = s_expected;
return Task.CompletedTask;
Expand All @@ -507,7 +507,7 @@ static Task Test(string input)
var context = this.MockContext("");

// Act
var function = SKFunction.FromNativeMethod(Method(Test), log: this._log.Object);
var function = SKFunction.FromNativeMethod(Method(TestAsync), log: this._log.Object);
Assert.NotNull(function);
SKContext result = await function.InvokeAsync(context);

Expand All @@ -523,7 +523,7 @@ public async Task ItSupportsType16Async()
// Arrange
[SKFunction("Test")]
[SKFunctionName("Test")]
static Task Test(SKContext cx)
static Task TestAsync(SKContext cx)
{
s_canary = s_expected;
cx["canary"] = s_expected;
Expand All @@ -534,7 +534,7 @@ static Task Test(SKContext cx)
var context = this.MockContext("");

// Act
var function = SKFunction.FromNativeMethod(Method(Test), log: this._log.Object);
var function = SKFunction.FromNativeMethod(Method(TestAsync), log: this._log.Object);
Assert.NotNull(function);
SKContext result = await function.InvokeAsync(context);

Expand All @@ -552,7 +552,7 @@ public async Task ItSupportsType17Async()
// Arrange
[SKFunction("Test")]
[SKFunctionName("Test")]
static Task Test(string input, SKContext cx)
static Task TestAsync(string input, SKContext cx)
{
s_canary = s_expected;
cx["canary"] = s_expected;
Expand All @@ -563,7 +563,7 @@ static Task Test(string input, SKContext cx)
var context = this.MockContext("input:");

// Act
var function = SKFunction.FromNativeMethod(Method(Test), log: this._log.Object);
var function = SKFunction.FromNativeMethod(Method(TestAsync), log: this._log.Object);
Assert.NotNull(function);
SKContext result = await function.InvokeAsync(context);

Expand All @@ -581,7 +581,7 @@ public async Task ItSupportsType18Async()
// Arrange
[SKFunction("Test")]
[SKFunctionName("Test")]
static Task Test()
static Task TestAsync()
{
s_canary = s_expected;
return Task.CompletedTask;
Expand All @@ -590,7 +590,7 @@ static Task Test()
var context = this.MockContext("");

// Act
var function = SKFunction.FromNativeMethod(Method(Test), log: this._log.Object);
var function = SKFunction.FromNativeMethod(Method(TestAsync), log: this._log.Object);
Assert.NotNull(function);
SKContext result = await function.InvokeAsync(context);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private static unsafe void DivideByInPlaceImplementation(Span<float> x, float di
if (Vector.IsHardwareAccelerated &&
x.Length >= Vector<float>.Count)
{
Vector<float> divisorVec = new Vector<float>(divisor);
Vector<float> divisorVec = new(divisor);
float* pxOneVectorFromEnd = pxEnd - Vector<float>.Count;
do
{
Expand All @@ -87,7 +87,7 @@ private static unsafe void DivideByInPlaceImplementation(Span<double> x, double
if (Vector.IsHardwareAccelerated &&
x.Length >= Vector<double>.Count)
{
Vector<double> divisorVec = new Vector<double>(divisor);
Vector<double> divisorVec = new(divisor);
double* pxOneVectorFromEnd = pxEnd - Vector<double>.Count;
do
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ private static unsafe void MultiplyByInPlaceImplementation(Span<float> x, float
if (Vector.IsHardwareAccelerated &&
x.Length >= Vector<float>.Count)
{
Vector<float> multiplierVec = new Vector<float>(multiplier);
Vector<float> multiplierVec = new(multiplier);
float* pxOneVectorFromEnd = pxEnd - Vector<float>.Count;
do
{
Expand All @@ -89,7 +89,7 @@ private static unsafe void MultiplyByInPlaceImplementation(Span<double> x, doubl
if (Vector.IsHardwareAccelerated &&
x.Length >= Vector<double>.Count)
{
Vector<double> multiplierVec = new Vector<double>(multiplier);
Vector<double> multiplierVec = new(multiplier);
double* pxOneVectorFromEnd = pxEnd - Vector<double>.Count;
do
{
Expand Down
2 changes: 1 addition & 1 deletion dotnet/src/SemanticKernel/CoreSkills/HttpSkill.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace Microsoft.SemanticKernel.CoreSkills;
Justification = "Semantic Kernel operates on strings")]
public class HttpSkill : IDisposable
{
private static readonly HttpClientHandler s_httpClientHandler = new HttpClientHandler() { CheckCertificateRevocationList = true };
private static readonly HttpClientHandler s_httpClientHandler = new() { CheckCertificateRevocationList = true };
private readonly HttpClient _client;

/// <summary>
Expand Down
Loading

0 comments on commit 9578399

Please sign in to comment.