Merge pull request microsoft#2768 from microsoft/feature/code-cleanup

feature/code cleanup

.editorconfig

@@ -125,3 +125,15 @@ visual_basic_preferred_modifier_order = Partial,Default,Private,Protected,Public
 [*.json]
 indent_size = 2
 insert_final_newline = true
+
+[src/Kiota.Builder/SearchProviders/GitHub/GitHubClient/**/*.cs]
+dotnet_diagnostic.CA1002.severity = none
+dotnet_diagnostic.CA1032.severity = none
+dotnet_diagnostic.CA1034.severity = none
+dotnet_diagnostic.CA1054.severity = none
+dotnet_diagnostic.CA1056.severity = none
+dotnet_diagnostic.CA1302.severity = none
+dotnet_diagnostic.CA1707.severity = none
+dotnet_diagnostic.CA1720.severity = none
+dotnet_diagnostic.CA2007.severity = none
+dotnet_diagnostic.CA2227.severity = none

.github/workflows/dotnet.yml

@@ -44,7 +44,6 @@ jobs:
         run: dotnet restore kiota.sln
       - name: Check formatting
         run: dotnet format --verify-no-changes
-        continue-on-error: true # locking until https://github.com/dotnet/sdk/issues/30742
       - name: Build
         run: dotnet build kiota.sln --no-restore
       - name: Install PlayWright browsers

src/Kiota.Builder/BaseCodeParameterOrderComparer.cs

@@ -1,5 +1,5 @@
-using System.Collections.Generic;
-
+using System;
+using System.Collections.Generic;
 using Kiota.Builder.CodeDOM;
 
 namespace Kiota.Builder;
@@ -12,9 +12,11 @@ public int Compare(CodeParameter? x, CodeParameter? y)
             (null, null) => 0,
             (null, _) => -1,
             (_, null) => 1,
-            _ => x.Optional.CompareTo(y.Optional) * optionalWeight +
-                 GetKindOrderHint(x.Kind).CompareTo(GetKindOrderHint(y.Kind)) * kindWeight +
-                 x.Name.CompareTo(y.Name) * nameWeight,
+#pragma warning disable CA1062
+            _ => x.Optional.CompareTo(y.Optional) * OptionalWeight +
+                 GetKindOrderHint(x.Kind).CompareTo(GetKindOrderHint(y.Kind)) * KindWeight +
+                 StringComparer.OrdinalIgnoreCase.Compare(x.Name, y.Name) * NameWeight,
+#pragma warning restore CA1062
         };
     }
     protected virtual int GetKindOrderHint(CodeParameterKind kind)
@@ -36,7 +38,7 @@ protected virtual int GetKindOrderHint(CodeParameterKind kind)
             _ => 13,
         };
     }
-    private static readonly int optionalWeight = 10000;
-    private static readonly int kindWeight = 100;
-    private static readonly int nameWeight = 10;
+    private const int OptionalWeight = 10000;
+    private const int KindWeight = 100;
+    private const int NameWeight = 10;
 }

src/Kiota.Builder/Caching/DocumentCachingProvider.cs

@@ -37,40 +37,40 @@ public Task<Stream> GetDocumentAsync(Uri documentUri, string intermediateFolderN
     }
     private async Task<Stream> GetDocumentInternalAsync(Uri documentUri, string intermediateFolderName, string fileName, bool couldNotDelete, string? accept, CancellationToken token)
     {
-        var hashedUrl = BitConverter.ToString((HashAlgorithm.Value ?? throw new InvalidOperationException("unable to get hash algorithm")).ComputeHash(Encoding.UTF8.GetBytes(documentUri.ToString()))).Replace("-", string.Empty);
+        var hashedUrl = BitConverter.ToString((HashAlgorithm.Value ?? throw new InvalidOperationException("unable to get hash algorithm")).ComputeHash(Encoding.UTF8.GetBytes(documentUri.ToString()))).Replace("-", string.Empty, StringComparison.OrdinalIgnoreCase);
         var target = Path.Combine(Path.GetTempPath(), "kiota", "cache", intermediateFolderName, hashedUrl, fileName);
         var currentLock = _locks.GetOrAdd(target, _ => new AsyncLock());
-        using (await currentLock.LockAsync(token))
+        using (await currentLock.LockAsync(token).ConfigureAwait(false))
         {// if multiple clients are being updated for the same description, we'll have concurrent download of the file without the lock
             if (!File.Exists(target) || couldNotDelete)
-                return await DownloadDocumentFromSourceAsync(documentUri, target, accept, token);
+                return await DownloadDocumentFromSourceAsync(documentUri, target, accept, token).ConfigureAwait(false);
 
             var lastModificationDate = File.GetLastWriteTime(target);
             if (lastModificationDate.Add(Duration) > DateTime.Now && !ClearCache)
             {
-                Logger.LogDebug("cache file {cacheFile} is up to date and clearCache is {clearCache}, using it", target, ClearCache);
+                Logger.LogDebug("cache file {CacheFile} is up to date and clearCache is {ClearCache}, using it", target, ClearCache);
                 return File.OpenRead(target);
             }
             else
             {
-                Logger.LogDebug("cache file {cacheFile} is out of date, downloading from {url}", target, documentUri);
+                Logger.LogDebug("cache file {CacheFile} is out of date, downloading from {Url}", target, documentUri);
                 try
                 {
                     File.Delete(target);
                 }
                 catch (IOException ex)
                 {
                     couldNotDelete = true;
-                    Logger.LogWarning("could not delete cache file {cacheFile}, reason: {reason}", target, ex.Message);
+                    Logger.LogWarning("could not delete cache file {CacheFile}, reason: {Reason}", target, ex.Message);
                 }
             }
-            return await GetDocumentInternalAsync(documentUri, intermediateFolderName, fileName, couldNotDelete, accept, token);
+            return await GetDocumentInternalAsync(documentUri, intermediateFolderName, fileName, couldNotDelete, accept, token).ConfigureAwait(false);
         }
     }
     private static readonly ConcurrentDictionary<string, AsyncLock> _locks = new(StringComparer.OrdinalIgnoreCase);
     private async Task<Stream> DownloadDocumentFromSourceAsync(Uri documentUri, string target, string? accept, CancellationToken token)
     {
-        Logger.LogDebug("cache file {cacheFile} not found, downloading from {url}", target, documentUri);
+        Logger.LogDebug("cache file {CacheFile} not found, downloading from {Url}", target, documentUri);
         var directory = Path.GetDirectoryName(target);
         if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory))
             Directory.CreateDirectory(directory);
@@ -83,11 +83,13 @@ private async Task<Stream> DownloadDocumentFromSourceAsync(Uri documentUri, stri
             using var responseMessage = await HttpClient.SendAsync(requestMessage, token).ConfigureAwait(false);
             responseMessage.EnsureSuccessStatusCode();
             content = new MemoryStream();
-            await responseMessage.Content.CopyToAsync(content, token);
+            await responseMessage.Content.CopyToAsync(content, token).ConfigureAwait(false);
+#pragma warning disable CA2007
             await using var fileStream = File.Create(target);
+#pragma warning restore CA2007
             content.Position = 0;
-            await content.CopyToAsync(fileStream, token);
-            await fileStream.FlushAsync(token);
+            await content.CopyToAsync(fileStream, token).ConfigureAwait(false);
+            await fileStream.FlushAsync(token).ConfigureAwait(false);
             content.Position = 0;
             return content;
         }
@@ -97,7 +99,7 @@ private async Task<Stream> DownloadDocumentFromSourceAsync(Uri documentUri, stri
         }
         catch (IOException ex)
         {
-            Logger.LogWarning("could not write to cache file {cacheFile}, reason: {reason}", target, ex.Message);
+            Logger.LogWarning("could not write to cache file {CacheFile}, reason: {Reason}", target, ex.Message);
             content.Position = 0;
             return content;
         }

src/Kiota.Builder/CodeDOM/CodeBlock.cs

@@ -8,21 +8,21 @@ namespace Kiota.Builder.CodeDOM;
 /// <summary>
 /// 
 /// </summary>
-public class CodeBlock<V, U> : CodeElement, IBlock where V : BlockDeclaration, new() where U : BlockEnd, new()
+public class CodeBlock<TBlockDeclaration, TBlockEnd> : CodeElement, IBlock where TBlockDeclaration : BlockDeclaration, new() where TBlockEnd : BlockEnd, new()
 {
-    public V StartBlock
+    public TBlockDeclaration StartBlock
     {
         get; set;
     }
     protected ConcurrentDictionary<string, CodeElement> InnerChildElements { get; private set; } = new(StringComparer.OrdinalIgnoreCase);
-    public U EndBlock
+    public TBlockEnd EndBlock
     {
         get; set;
     }
     public CodeBlock()
     {
-        StartBlock = new V { Parent = this };
-        EndBlock = new U { Parent = this };
+        StartBlock = new TBlockDeclaration { Parent = this };
+        EndBlock = new TBlockEnd { Parent = this };
     }
     public override IEnumerable<CodeElement> GetChildElements(bool innerOnly = false)
     {

src/Kiota.Builder/CodeDOM/CodeClass.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Linq;
 
 using Kiota.Builder.Extensions;
@@ -87,23 +88,20 @@ public IEnumerable<CodeInterface> AddInnerInterface(params CodeInterface[] codeI
             throw new ArgumentOutOfRangeException(nameof(codeInterfaces));
         return AddRange(codeInterfaces);
     }
-    public CodeClass? GetParentClass()
-    {
-        return StartBlock.Inherits?.TypeDefinition as CodeClass;
-    }
+    public CodeClass? ParentClass => StartBlock.Inherits?.TypeDefinition as CodeClass;
     public bool DerivesFrom(CodeClass codeClass)
     {
         ArgumentNullException.ThrowIfNull(codeClass);
-        var parent = GetParentClass();
+        var parent = ParentClass;
         if (parent == null)
             return false;
         if (parent == codeClass)
             return true;
         return parent.DerivesFrom(codeClass);
     }
-    public List<CodeClass> GetInheritanceTree(bool currentNamespaceOnly = false, bool includeCurrentClass = true)
+    public Collection<CodeClass> GetInheritanceTree(bool currentNamespaceOnly = false, bool includeCurrentClass = true)
     {
-        var parentClass = GetParentClass();
+        var parentClass = ParentClass;
         if (parentClass == null || (currentNamespaceOnly && parentClass.GetImmediateParentOfType<CodeNamespace>() != GetImmediateParentOfType<CodeNamespace>()))
             if (includeCurrentClass)
                 return new() { this };
@@ -115,7 +113,7 @@ public List<CodeClass> GetInheritanceTree(bool currentNamespaceOnly = false, boo
     }
     public CodeClass? GetGreatestGrandparent(CodeClass? startClassToSkip = default)
     {
-        var parentClass = GetParentClass();
+        var parentClass = ParentClass;
         if (parentClass == null)
             return startClassToSkip != null && startClassToSkip == this ? null : this;
         // we don't want to return the current class if this is the start node in the inheritance tree and doesn't have parent

src/Kiota.Builder/CodeDOM/CodeComposedTypeBase.cs

@@ -36,15 +36,16 @@ public DiscriminatorInformation DiscriminatorInformation
             _discriminatorInformation = value;
         }
     }
-    protected override ChildType BaseClone<ChildType>(CodeTypeBase source)
+    protected override TChildType BaseClone<TChildType>(CodeTypeBase source)
     {
+        ArgumentNullException.ThrowIfNull(source);
         if (source is not CodeComposedTypeBase sourceComposed)
             throw new InvalidCastException($"Cannot cast {source.GetType().Name} to {nameof(CodeComposedTypeBase)}");
-        base.BaseClone<ChildType>(source);
+        base.BaseClone<TChildType>(source);
         if (sourceComposed.Types?.Any() ?? false)
             AddType(sourceComposed.Types.ToArray());
         DiscriminatorInformation = (DiscriminatorInformation)sourceComposed.DiscriminatorInformation.Clone();
-        return this is ChildType casted ? casted : throw new InvalidCastException($"Cannot cast {GetType().Name} to {typeof(ChildType).Name}");
+        return this is TChildType casted ? casted : throw new InvalidCastException($"Cannot cast {GetType().Name} to {typeof(TChildType).Name}");
     }
     /// <summary>
     /// The target namespace if the composed type needs to be represented by a class

src/Kiota.Builder/CodeDOM/CodeEnum.cs

@@ -3,8 +3,10 @@
 using System.Collections.Generic;
 
 namespace Kiota.Builder.CodeDOM;
+#pragma warning disable CA1711
 public class CodeEnum : CodeBlock<BlockDeclaration, BlockEnd>, IDocumentedElement, ITypeDefinition
 {
+#pragma warning restore CA2227
     private readonly HashSet<string> optionsNames = new(StringComparer.OrdinalIgnoreCase); // this structure is used to check if an option name is unique   
     private readonly ConcurrentQueue<CodeEnumOption> OptionsInternal = new(); // this structure is used to maintain the order of the options
     public bool Flags
@@ -15,6 +17,7 @@ public bool Flags
 
     public void AddOption(params CodeEnumOption[] codeEnumOptions)
     {
+        if (codeEnumOptions is null) return;
         EnsureElementsAreChildren(codeEnumOptions);
         foreach (var option in codeEnumOptions)
         {

src/Kiota.Builder/CodeDOM/CodeFile.cs

@@ -16,13 +16,10 @@ public IEnumerable<T> AddElements<T>(params T[] elements) where T : CodeElement
         return AddRange(elements);
     }
 
-    public IEnumerable<CodeUsing> GetUsings()
-    {
-        return GetChildElements().SelectMany(x => x.GetChildElements())
-            .Where(x => x is ProprietableBlockDeclaration)
-            .Cast<ProprietableBlockDeclaration>()
-            .SelectMany(x => x.Usings);
-    }
+    public IEnumerable<CodeUsing> AllUsingsFromChildElements => GetChildElements()
+            .SelectMany(static x => x.GetChildElements())
+            .OfType<ProprietableBlockDeclaration>()
+            .SelectMany(static x => x.Usings);
 }
 public class CodeFileDeclaration : ProprietableBlockDeclaration
 {

src/Kiota.Builder/CodeDOM/CodeMethod.cs

@@ -78,6 +78,8 @@ public class CodeMethod : CodeTerminalWithKind<CodeMethodKind>, ICloneable, IDoc
     public static CodeMethod FromIndexer(CodeIndexer originalIndexer, Func<string, string> methodNameCallback, Func<string, string> parameterNameCallback, bool parameterNullable)
     {
         ArgumentNullException.ThrowIfNull(originalIndexer);
+        ArgumentNullException.ThrowIfNull(methodNameCallback);
+        ArgumentNullException.ThrowIfNull(parameterNameCallback);
         var method = new CodeMethod
         {
             IsAsync = false,
@@ -115,7 +117,9 @@ public HttpMethod? HttpMethod
         get; set;
     }
     public string RequestBodyContentType { get; set; } = string.Empty;
-    public HashSet<string> AcceptedResponseTypes = new(StringComparer.OrdinalIgnoreCase);
+#pragma warning disable CA2227
+    public HashSet<string> AcceptedResponseTypes { get; set; } = new(StringComparer.OrdinalIgnoreCase);
+#pragma warning restore CA2227
     public AccessModifier Access { get; set; } = AccessModifier.Public;
 #nullable disable // exposing property is required
     private CodeTypeBase returnType;
@@ -198,8 +202,10 @@ public bool IsAccessor
     {
         get => IsOfKind(CodeMethodKind.Getter, CodeMethodKind.Setter);
     }
+#pragma warning disable CA2227
     public HashSet<string> SerializerModules { get; set; } = new(StringComparer.OrdinalIgnoreCase);
     public HashSet<string> DeserializerModules { get; set; } = new(StringComparer.OrdinalIgnoreCase);
+#pragma warning restore CA2227
     /// <summary>
     /// Indicates whether this method is an overload for another method.
     /// </summary>
@@ -228,7 +234,9 @@ public CodeIndexer? OriginalIndexer
     /// The base url for every request read from the servers property on the description.
     /// Only provided for constructor on Api client
     /// </summary>
+#pragma warning disable CA1056 // Uri properties should not be strings
     public string BaseUrl { get; set; } = string.Empty;
+#pragma warning restore CA1056 // Uri properties should not be strings
 
     /// <summary>
     /// This is currently used for CommandBuilder methods to get the original name without the Build prefix & Command suffix.

src/Kiota.Builder/CodeDOM/CodeNamespace.cs

@@ -59,7 +59,7 @@ public IEnumerable<CodeClass> AddClass(params CodeClass[] codeClasses)
             throw new ArgumentOutOfRangeException(nameof(codeClasses));
         return AddRange(codeClasses);
     }
-    private static readonly char namespaceNameSeparator = '.';
+    private const char NamespaceNameSeparator = '.';
     public CodeNamespace GetRootNamespace()
     {
         if (Parent is CodeNamespace parentNS) return parentNS.GetRootNamespace();
@@ -73,7 +73,7 @@ public CodeNamespace GetRootNamespace()
     public CodeNamespace? FindNamespaceByName(string nsName)
     {
         ArgumentException.ThrowIfNullOrEmpty(nsName);
-        if (nsName.Equals(Name)) return this;
+        if (nsName.Equals(Name, StringComparison.OrdinalIgnoreCase)) return this;
         var result = FindChildByName<CodeNamespace>(nsName, false);
         if (result == null)
             foreach (var childNS in InnerChildElements.Values.OfType<CodeNamespace>())
@@ -88,7 +88,7 @@ public CodeNamespace GetRootNamespace()
     public CodeNamespace AddNamespace(string namespaceName)
     {
         ArgumentException.ThrowIfNullOrEmpty(namespaceName);
-        var namespaceNameSegments = namespaceName.Split(namespaceNameSeparator, StringSplitOptions.RemoveEmptyEntries);
+        var namespaceNameSegments = namespaceName.Split(NamespaceNameSeparator, StringSplitOptions.RemoveEmptyEntries);
         var lastPresentSegmentIndex = default(int);
         var lastPresentSegmentNamespace = GetRootNamespace();
         while (lastPresentSegmentIndex < namespaceNameSegments.Length)

src/Kiota.Builder/CodeDOM/CodeProperty.cs

@@ -42,7 +42,10 @@ public enum CodePropertyKind
 
 public class CodeProperty : CodeTerminalWithKind<CodePropertyKind>, IDocumentedElement, IAlternativeName, ICloneable
 {
-    public bool ReadOnly { get; set; } = false;
+    public bool ReadOnly
+    {
+        get; set;
+    }
     public AccessModifier Access { get; set; } = AccessModifier.Public;
     public bool ExistsInBaseType => OriginalPropertyFromBaseType != null;
     public bool ExistsInExternalBaseType

src/Kiota.Builder/CodeDOM/CodeType.cs

@@ -1,5 +1,5 @@
 using System;
-using System.Collections.Generic;
+using System.Collections.ObjectModel;
 
 namespace Kiota.Builder.CodeDOM;
 public class CodeType : CodeTypeBase, ICloneable
@@ -23,5 +23,5 @@ public override object Clone()
             GenericTypeParameterValues = new(GenericTypeParameterValues),
         }.BaseClone<CodeType>(this);
     }
-    public List<CodeType> GenericTypeParameterValues { get; set; } = new();
+    public Collection<CodeType> GenericTypeParameterValues { get; init; } = new();
 }

src/Kiota.Builder/CodeDOM/CodeTypeBase.cs

@@ -37,14 +37,15 @@ public bool IsArray
             return CollectionKind == CodeTypeCollectionKind.Array;
         }
     }
-    protected virtual ChildType BaseClone<ChildType>(CodeTypeBase source) where ChildType : CodeTypeBase
+    protected virtual TChildType BaseClone<TChildType>(CodeTypeBase source) where TChildType : CodeTypeBase
     {
+        ArgumentNullException.ThrowIfNull(source);
         ActionOf = source.ActionOf;
         IsNullable = source.IsNullable;
         CollectionKind = source.CollectionKind;
         Name = source.Name;
         Parent = source.Parent;
-        return this is ChildType cast ? cast : throw new InvalidOperationException($"the type {GetType()} is not compatible with the type {typeof(ChildType)}");
+        return this is TChildType cast ? cast : throw new InvalidOperationException($"the type {GetType()} is not compatible with the type {typeof(TChildType)}");
     }
 
     public abstract object Clone();

src/Kiota.Builder/CodeDOM/ProprietableBlock.cs

@@ -15,7 +15,7 @@ public interface IProprietableBlock : ICodeElement
 /// <summary>
 /// Represents a block of code that can have properties and methods
 /// </summary>
-public abstract class ProprietableBlock<T, U> : CodeBlock<U, BlockEnd>, IDocumentedElement, IProprietableBlock where T : Enum where U : ProprietableBlockDeclaration, new()
+public abstract class ProprietableBlock<TBlockKind, TBlockDeclaration> : CodeBlock<TBlockDeclaration, BlockEnd>, IDocumentedElement, IProprietableBlock where TBlockKind : Enum where TBlockDeclaration : ProprietableBlockDeclaration, new()
 {
     private string name = string.Empty;
     /// <summary>
@@ -48,12 +48,12 @@ public void RemovePropertiesOfKind(params CodePropertyKind[] kind)
             RemoveChildElement(property);
     }
 #nullable disable
-    public T Kind
+    public TBlockKind Kind
     {
         get; set;
     }
 #nullable enable
-    public bool IsOfKind(params T[] kinds)
+    public bool IsOfKind(params TBlockKind[] kinds)
     {
         return kinds?.Contains(Kind) ?? false;
     }