Skip to content

Commit

Permalink
Fix typos and code style (#741)
Browse files Browse the repository at this point in the history 
* Fix filename containing a space (`ApiKeyAuthenticationHandler .cs`)
* Fix typos
* Fix code style
* Add missing rule for local constants
* Add commented code to automate code style checks. These don't work
with .NET Standard 2.0, so we'll need some extra research.
* Fix one incorrect ILogger naming
* Add some TODOs about unused code
* Fix xmldocs
  • Loading branch information
@dluc
dluc committed May 1, 2023
1 parent 57cbc93 commit e8dced1
Show file tree
Hide file tree
Showing 42 changed files with 208 additions and 180 deletions.
17 changes: 15 additions & 2 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,17 @@ dotnet_diagnostic.CA2201.severity = warning # Exception type System.Exception is

dotnet_naming_style.pascal_case_style.capitalization = pascal_case

dotnet_naming_style.camel_case_style.capitalization = camel_case

dotnet_naming_style.static_underscored.capitalization = camel_case
dotnet_naming_style.static_underscored.required_prefix = s_

dotnet_naming_style.underscored.capitalization = camel_case
dotnet_naming_style.underscored.required_prefix = _

dotnet_naming_style.uppercase_with_underscore_separator.capitalization = all_upper
dotnet_naming_style.uppercase_with_underscore_separator.word_separator = _

dotnet_naming_style.end_in_async.required_prefix =
dotnet_naming_style.end_in_async.required_suffix = Async
dotnet_naming_style.end_in_async.capitalization = pascal_case
Expand All @@ -148,6 +153,10 @@ dotnet_naming_symbols.constant_fields.applicable_kinds = field
dotnet_naming_symbols.constant_fields.applicable_accessibilities = *
dotnet_naming_symbols.constant_fields.required_modifiers = const

dotnet_naming_symbols.local_constant.applicable_kinds = local
dotnet_naming_symbols.local_constant.applicable_accessibilities = *
dotnet_naming_symbols.local_constant.required_modifiers = const

dotnet_naming_symbols.private_static_fields.applicable_kinds = field
dotnet_naming_symbols.private_static_fields.applicable_accessibilities = private
dotnet_naming_symbols.private_static_fields.required_modifiers = static
Expand All @@ -161,9 +170,13 @@ dotnet_naming_symbols.any_async_methods.required_modifiers = async

# Rules

dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = error
dotnet_naming_rule.constant_fields_should_be_pascal_case.symbols = constant_fields
dotnet_naming_rule.constant_fields_should_be_pascal_case.symbols = constant_fields
dotnet_naming_rule.constant_fields_should_be_pascal_case.style = pascal_case_style
dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = error

dotnet_naming_rule.local_constant_should_be_pascal_case.symbols = local_constant
dotnet_naming_rule.local_constant_should_be_pascal_case.style = pascal_case_style
dotnet_naming_rule.local_constant_should_be_pascal_case.severity = error

dotnet_naming_rule.private_static_fields_underscored.symbols = private_static_fields
dotnet_naming_rule.private_static_fields_underscored.style = static_underscored
Expand Down
18 changes: 17 additions & 1 deletion dotnet/Directory.Packages.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,39 @@
<!-- Symbols -->
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="1.1.1" />


<!-- Analyzers -->
<!-- TODO: Not working with .NET Standard 2.0
<PackageVersion Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" Version="4.1.0" />
<PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="4.5.0" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeStyle">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
-->

<PackageVersion Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" />
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>

<PackageVersion Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.5.22" />
<PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>

<PackageVersion Include="xunit.analyzers" Version="1.1.0" />
<PackageReference Include="xunit.analyzers">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>

<PackageVersion Include="Moq.Analyzers" Version="0.0.9" />
<PackageReference Include="Moq.Analyzers">
<PrivateAssets>all</PrivateAssets>
Expand Down
3 changes: 3 additions & 0 deletions dotnet/SK-dotnet.sln.DotSettings
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=UI/@EntryIndexedValue">UI</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=UID/@EntryIndexedValue">UID</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=URL/@EntryIndexedValue">URL</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=XML/@EntryIndexedValue">XML</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=YAML/@EntryIndexedValue">YAML</s:String>
<s:Boolean x:Key="/Default/CodeStyle/Naming/CSharpNaming/ApplyAutoDetectedRules/@EntryValue">False</s:Boolean>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=Constants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;</s:String>
Expand Down Expand Up @@ -183,6 +184,7 @@ public void It$SOMENAME$()
<s:Boolean x:Key="/Default/UserDictionary/Words/=CHATGPT/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=childrens/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Chunker/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Ctors/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=davinci/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=greaterthan/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Joinable/@EntryIndexedValue">True</s:Boolean>
Expand All @@ -192,6 +194,7 @@ public void It$SOMENAME$()
<s:Boolean x:Key="/Default/UserDictionary/Words/=myfile/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Notegen/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Qdrant/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Roundtrips/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=sandboxing/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=SK/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=SKHTTP/@EntryIndexedValue">True</s:Boolean>
Expand Down
4 changes: 2 additions & 2 deletions ...nectors/Connectors.UnitTests/HuggingFace/TextCompletion/HuggingFaceTextCompletionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ public class HuggingFaceTextCompletionTests : IDisposable
public async Task ItReturnsCompletionCorrectlyAsync()
{
// Arrange
const string prompt = "This is test";
const string Prompt = "This is test";
CompleteRequestSettings requestSettings = new();

using var service = this.CreateService(HuggingFaceTestHelper.GetTestResponse("completion_test_response.json"));

// Act
var completion = await service.CompleteAsync(prompt, requestSettings);
var completion = await service.CompleteAsync(Prompt, requestSettings);

// Assert
Assert.Equal("This is test completion response", completion);
Expand Down
8 changes: 4 additions & 4 deletions ...ors/Connectors.UnitTests/HuggingFace/TextEmbedding/HuggingFaceEmbeddingGenerationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public class HuggingFaceEmbeddingGenerationTests : IDisposable
public async Task ItReturnsEmbeddingsCorrectlyAsync()
{
// Arrange
const int expectedEmbeddingCount = 1;
const int expectedVectorCount = 8;
const int ExpectedEmbeddingCount = 1;
const int ExpectedVectorCount = 8;
List<string> data = new() { "test_string_1", "test_string_2", "test_string_3" };

using var service = this.CreateService(HuggingFaceTestHelper.GetTestResponse("embeddings_test_response.json"));
Expand All @@ -43,8 +43,8 @@ public async Task ItReturnsEmbeddingsCorrectlyAsync()

// Assert
Assert.NotNull(embeddings);
Assert.Equal(expectedEmbeddingCount, embeddings.Count);
Assert.Equal(expectedVectorCount, embeddings.First().Count);
Assert.Equal(ExpectedEmbeddingCount, embeddings.Count);
Assert.Equal(ExpectedVectorCount, embeddings.First().Count);
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions dotnet/src/Connectors/Connectors.UnitTests/OpenAI/Tokenizers/GPT3TokenizerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ public void ItReturnsTheCorrectTokens(string text, string tokens)
public void ItDoesntTimeOutForABigText()
{
// Arrange
const int oneMb = 1000 * 1000;
const int OneMb = 1000 * 1000;
var watch = new Stopwatch();
Random rnd = new();
StringBuilder text = new();
watch.Start();
var count = 0;
while (text.Length < oneMb)
while (text.Length < OneMb)
{
text.Append(GenerateWord(rnd, count++));
}
Expand Down
6 changes: 3 additions & 3 deletions dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlannerConfig.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ public sealed class SequentialPlannerConfig
/// <summary>
/// A list of skills to exclude from the plan creation request.
/// </summary>
public HashSet<string> ExcludedSkills { get; } = new() { };
public HashSet<string> ExcludedSkills { get; } = new();

/// <summary>
/// A list of functions to exclude from the plan creation request.
/// </summary>
public HashSet<string> ExcludedFunctions { get; } = new() { };
public HashSet<string> ExcludedFunctions { get; } = new();

/// <summary>
/// A list of functions to include in the plan creation request.
/// </summary>
public HashSet<string> IncludedFunctions { get; } = new() { };
public HashSet<string> IncludedFunctions { get; } = new();

/// <summary>
/// The maximum number of tokens to allow in a plan.
Expand Down
10 changes: 5 additions & 5 deletions .../IntegrationTests/Connectors/HuggingFace/TextCompletion/HuggingFaceTextCompletionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,21 @@ public HuggingFaceTextCompletionTests()
public async Task HuggingFaceLocalAndRemoteTextCompletionAsync()
{
// Arrange
const string input = "This is test";
const string Input = "This is test";

using var huggingFaceLocal = new HuggingFaceTextCompletion(new Uri(Endpoint), Model);
using var huggingFaceRemote = new HuggingFaceTextCompletion(this.GetApiKey(), Model);

// Act
var localResponse = await huggingFaceLocal.CompleteAsync(input, new CompleteRequestSettings());
var remoteResponse = await huggingFaceRemote.CompleteAsync(input, new CompleteRequestSettings());
var localResponse = await huggingFaceLocal.CompleteAsync(Input, new CompleteRequestSettings());
var remoteResponse = await huggingFaceRemote.CompleteAsync(Input, new CompleteRequestSettings());

// Assert
Assert.NotNull(localResponse);
Assert.NotNull(remoteResponse);

Assert.StartsWith(input, localResponse, StringComparison.Ordinal);
Assert.StartsWith(input, remoteResponse, StringComparison.Ordinal);
Assert.StartsWith(Input, localResponse, StringComparison.Ordinal);
Assert.StartsWith(Input, remoteResponse, StringComparison.Ordinal);
}

private string GetApiKey()
Expand Down
4 changes: 2 additions & 2 deletions dotnet/src/IntegrationTests/Connectors/OpenAI/OpenAICompletionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public async Task CompletionWithDifferentLineEndingsAsync(string lineEnding, AIS
$"Put the result in between <result></result> tags{lineEnding}" +
$"Input:{lineEnding}{{\"name\": \"John\", \"age\": 30}}{lineEnding}{lineEnding}Request:{lineEnding}name";

const string expectedAnswerContains = "<result>John</result>";
const string ExpectedAnswerContains = "<result>John</result>";

IKernel target = Kernel.Builder.WithLogger(this._logger).Build();

Expand All @@ -206,7 +206,7 @@ public async Task CompletionWithDifferentLineEndingsAsync(string lineEnding, AIS
SKContext actual = await target.RunAsync(prompt, skill["Chat"]);

// Assert
Assert.Contains(expectedAnswerContains, actual.Result, StringComparison.OrdinalIgnoreCase);
Assert.Contains(ExpectedAnswerContains, actual.Result, StringComparison.OrdinalIgnoreCase);
}

#region internals
Expand Down
42 changes: 21 additions & 21 deletions dotnet/src/IntegrationTests/TemplateLanguage/PromptTemplateEngineTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,54 +27,54 @@ public PromptTemplateEngineTests(ITestOutputHelper output)
public async Task ItSupportsVariablesAsync()
{
// Arrange
const string input = "template tests";
const string winner = "SK";
const string template = "And the winner\n of {{$input}} \nis: {{ $winner }}!";
const string Input = "template tests";
const string Winner = "SK";
const string Template = "And the winner\n of {{$input}} \nis: {{ $winner }}!";

var kernel = Kernel.Builder.Build();
var context = kernel.CreateNewContext();
context["input"] = input;
context["winner"] = winner;
context["input"] = Input;
context["winner"] = Winner;

// Act
var result = await this._target.RenderAsync(template, context);
var result = await this._target.RenderAsync(Template, context);

// Assert
var expected = template
.Replace("{{$input}}", input, StringComparison.OrdinalIgnoreCase)
.Replace("{{ $winner }}", winner, StringComparison.OrdinalIgnoreCase);
var expected = Template
.Replace("{{$input}}", Input, StringComparison.OrdinalIgnoreCase)
.Replace("{{ $winner }}", Winner, StringComparison.OrdinalIgnoreCase);
Assert.Equal(expected, result);
}

[Fact]
public async Task ItSupportsValuesAsync()
{
// Arrange
const string template = "And the winner\n of {{'template\ntests'}} \nis: {{ \"SK\" }}!";
const string expected = "And the winner\n of template\ntests \nis: SK!";
const string Template = "And the winner\n of {{'template\ntests'}} \nis: {{ \"SK\" }}!";
const string Expected = "And the winner\n of template\ntests \nis: SK!";

var kernel = Kernel.Builder.Build();
var context = kernel.CreateNewContext();

// Act
var result = await this._target.RenderAsync(template, context);
var result = await this._target.RenderAsync(Template, context);

// Assert
Assert.Equal(expected, result);
Assert.Equal(Expected, result);
}

[Fact]
public async Task ItAllowsToPassVariablesToFunctionsAsync()
{
// Arrange
const string template = "== {{my.check123 $call}} ==";
const string Template = "== {{my.check123 $call}} ==";
var kernel = Kernel.Builder.Build();
kernel.ImportSkill(new MySkill(), "my");
var context = kernel.CreateNewContext();
context["call"] = "123";

// Act
var result = await this._target.RenderAsync(template, context);
var result = await this._target.RenderAsync(Template, context);

// Assert
Assert.Equal("== 123 ok ==", result);
Expand All @@ -84,13 +84,13 @@ public async Task ItAllowsToPassVariablesToFunctionsAsync()
public async Task ItAllowsToPassValuesToFunctionsAsync()
{
// Arrange
const string template = "== {{my.check123 '234'}} ==";
const string Template = "== {{my.check123 '234'}} ==";
var kernel = Kernel.Builder.Build();
kernel.ImportSkill(new MySkill(), "my");
var context = kernel.CreateNewContext();

// Act
var result = await this._target.RenderAsync(template, context);
var result = await this._target.RenderAsync(Template, context);

// Assert
Assert.Equal("== 234 != 123 ==", result);
Expand All @@ -100,8 +100,8 @@ public async Task ItAllowsToPassValuesToFunctionsAsync()
public async Task ItAllowsToPassEscapedValues1ToFunctionsAsync()
{
// Arrange
const char ESC = '\\';
string template = "== {{my.check123 'a" + ESC + "'b'}} ==";
const char Esc = '\\';
string template = "== {{my.check123 'a" + Esc + "'b'}} ==";
var kernel = Kernel.Builder.Build();
kernel.ImportSkill(new MySkill(), "my");
var context = kernel.CreateNewContext();
Expand All @@ -117,8 +117,8 @@ public async Task ItAllowsToPassEscapedValues1ToFunctionsAsync()
public async Task ItAllowsToPassEscapedValues2ToFunctionsAsync()
{
// Arrange
const char ESC = '\\';
string template = "== {{my.check123 \"a" + ESC + "\"b\"}} ==";
const char Esc = '\\';
string template = "== {{my.check123 \"a" + Esc + "\"b\"}} ==";
var kernel = Kernel.Builder.Build();
kernel.ImportSkill(new MySkill(), "my");
var context = kernel.CreateNewContext();
Expand Down

0 comments on commit e8dced1

Please sign in to comment.