Activate .NET code analysis (#48)

* Enable .NET source code analysis

* Fix CA2016 (Forward the CancellationToken parameter to methods that take one)

* +Fix for CA2016 (Forward the CancellationToken parameter to methods that take one)

* Fix CA1826 (Use property instead of Linq Enumerable method)

* Fix CA1825 (Avoid zero-length array allocations)

* Fix CA1822 (Mark members as static)

* Fix CA2208 (Instantiate argument exceptions correctly)

* Fix CA2254 (Template should be a static expression)

* Fix CA1816 (Call GC.SuppressFinalize correctly)

* Remove unnecessary build property

* Fix CA1510 (Use throw helper ThrowIfNull)

* Fix CA1861 (Avoid constant arrays as arguments)

* Fix CA1862 (Use the 'StringComparison' method overloads )

* Fix CA1854 (Prefer the IDictionary.TryGetValue(TKey, out TValue) method)

* Fix CA1859 (Use concrete types when possible for improved performance)

* Fix CA1854 (Use span-based 'string.Concat')

* Un-fix CA1862 (Use the 'StringComparison' method overloads )

EF Core needs this one.

* use Array.Empty

* Fix CA1860 (Avoid using 'Enumerable.Any()' extension method)

---------

Co-authored-by: Cédric Luthi <[email protected]>
Co-authored-by: Regenhardt Marlon <[email protected]>
Co-authored-by: FroggieFrog <[email protected]>

.editorconfig

@@ -36,6 +36,9 @@ indent_size = 4
 
 # .NET Coding Conventions
 
+# .NET source code analysis
+dotnet_code_quality.CA1826.exclude_ordefault_methods = true
+
 # Organize usings
 dotnet_sort_system_directives_first = true:warning
 

src/BaGetter.Aliyun/AliyunStorageService.cs

@@ -17,8 +17,7 @@ public class AliyunStorageService : IStorageService
 
         public AliyunStorageService(IOptionsSnapshot<AliyunStorageOptions> options, OssClient client)
         {
-            if (options == null)
-                throw new ArgumentNullException(nameof(options));
+            ArgumentNullException.ThrowIfNull(options);
 
             _bucket = options.Value.Bucket;
             _prefix = options.Value.Prefix;

src/BaGetter.Aws/S3StorageService.cs

@@ -18,8 +18,7 @@ public class S3StorageService : IStorageService
 
         public S3StorageService(IOptionsSnapshot<S3StorageOptions> options, AmazonS3Client client)
         {
-            if (options == null)
-                throw new ArgumentNullException(nameof(options));
+            ArgumentNullException.ThrowIfNull(options);
 
             _bucket = options.Value.Bucket;
             _prefix = options.Value.Prefix;
@@ -42,7 +41,7 @@ public async Task<Stream> GetAsync(string path, CancellationToken cancellationTo
             {
                 using (var request = await _client.GetObjectAsync(_bucket, PrepareKey(path), cancellationToken))
                 {
-                    await request.ResponseStream.CopyToAsync(stream);
+                    await request.ResponseStream.CopyToAsync(stream, cancellationToken);
                 }
 
                 stream.Seek(0, SeekOrigin.Begin);

src/BaGetter.Azure/Search/AzureSearchService.cs

@@ -205,7 +205,7 @@ private string BuildSeachQuery(string query, string packageType, string framewor
             return queryBuilder.ToString();
         }
 
-        private string BuildSearchFilter(bool includePrerelease, bool includeSemVer2)
+        private static string BuildSearchFilter(bool includePrerelease, bool includeSemVer2)
         {
             var searchFilters = SearchFilters.Default;
 

src/BaGetter.Azure/Search/IndexActionBuilder.cs

@@ -21,7 +21,7 @@ public virtual IReadOnlyList<IndexAction<KeyedDocument>> UpdatePackage(
             return AddOrUpdatePackage(registration, isUpdate: true);
         }
 
-        private IReadOnlyList<IndexAction<KeyedDocument>> AddOrUpdatePackage(
+        private static IReadOnlyList<IndexAction<KeyedDocument>> AddOrUpdatePackage(
             PackageRegistration registration,
             bool isUpdate)
         {
@@ -105,7 +105,7 @@ private IReadOnlyList<IndexAction<KeyedDocument>> AddOrUpdatePackage(
             return result;
         }
 
-        private string EncodePackageId(string key)
+        private static string EncodePackageId(string key)
         {
             // Keys can only contain letters, digits, underscore(_), dash(-), or equal sign(=).
             // TODO: Align with NuGet.org's algorithm.

src/BaGetter.Azure/Table/TableOperationBuilder.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using BaGetter.Core;
 using Microsoft.Azure.Cosmos.Table;
@@ -8,6 +9,7 @@
 
 namespace BaGetter.Azure
 {
+    [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Would be a breaking change since it's part of the public API")]
     public class TableOperationBuilder
     {
         public TableOperation AddPackage(Package package)

src/BaGetter.Azure/Table/TablePackageDatabase.cs

@@ -82,8 +82,8 @@ public async Task AddDownloadAsync(
                     attempt++;
                     _logger.LogWarning(
                         e,
-                        $"Retrying due to precondition failure, attempt {{Attempt}} of {MaxPreconditionFailures}..",
-                        attempt);
+                        "Retrying due to precondition failure, attempt {Attempt} of {MaxPreconditionFailures}",
+                        attempt, MaxPreconditionFailures);
                 }
             }
         }
@@ -196,7 +196,7 @@ public async Task<bool> UnlistPackageAsync(string id, NuGetVersion version, Canc
                 cancellationToken);
         }
 
-        private List<string> MinimalColumnSet => new List<string> { "PartitionKey" };
+        private static List<string> MinimalColumnSet => new List<string> { "PartitionKey" };
 
         private async Task<bool> TryUpdatePackageAsync(TableOperation operation, CancellationToken cancellationToken)
         {

src/BaGetter.Azure/Table/TableSearchService.cs

@@ -133,7 +133,7 @@ private async Task<IReadOnlyList<Package>> LoadPackagesAsync(
             return results;
         }
 
-        private string GenerateSearchFilter(string searchText, bool includePrerelease, bool includeSemVer2)
+        private static string GenerateSearchFilter(string searchText, bool includePrerelease, bool includeSemVer2)
         {
             var result = "";
 

src/BaGetter.Core/Authentication/ApiKeyAuthenticationService.cs

@@ -11,7 +11,7 @@ public class ApiKeyAuthenticationService : IAuthenticationService
 
         public ApiKeyAuthenticationService(IOptionsSnapshot<BaGetterOptions> options)
         {
-            if (options == null) throw new ArgumentNullException(nameof(options));
+            ArgumentNullException.ThrowIfNull(options);
 
             _apiKey = string.IsNullOrEmpty(options.Value.ApiKey) ? null : options.Value.ApiKey;
         }

src/BaGetter.Core/Entities/Converters/StringArrayToJsonConverter.cs

@@ -1,3 +1,4 @@
+using System;
 using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
 using Newtonsoft.Json;
 
@@ -10,7 +11,7 @@ public class StringArrayToJsonConverter : ValueConverter<string[], string>
         public StringArrayToJsonConverter()
             : base(
                 v => JsonConvert.SerializeObject(v),
-                v => (!string.IsNullOrEmpty(v)) ? JsonConvert.DeserializeObject<string[]>(v) : new string[0])
+                v => (!string.IsNullOrEmpty(v)) ? JsonConvert.DeserializeObject<string[]>(v) : Array.Empty<string>())
         {
         }
     }

src/BaGetter.Core/Extensions/PackageArchiveReaderExtensions.cs

@@ -109,18 +109,20 @@ private static Uri ParseUri(string uriString)
             return new Uri(uriString);
         }
 
+        private static readonly char[] Separator = { ',', ';', '\t', '\n', '\r' };
+
         private static string[] ParseAuthors(string authors)
         {
-            if (string.IsNullOrEmpty(authors)) return new string[0];
+            if (string.IsNullOrEmpty(authors)) return Array.Empty<string>();
 
-            return authors.Split(new[] { ',', ';', '\t', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries);
+            return authors.Split(Separator, StringSplitOptions.RemoveEmptyEntries);
         }
 
         private static string[] ParseTags(string tags)
         {
-            if (string.IsNullOrEmpty(tags)) return new string[0];
+            if (string.IsNullOrEmpty(tags)) return Array.Empty<string>();
 
-            return tags.Split(new[] { ',', ';', ' ', '\t', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries);
+            return tags.Split(Separator, StringSplitOptions.RemoveEmptyEntries);
         }
 
         private static (Uri repositoryUrl, string repositoryType) GetRepositoryMetadata(NuspecReader nuspec)

src/BaGetter.Core/Indexing/SymbolIndexingService.cs

@@ -133,7 +133,7 @@ bool IsValidSymbolFileInfo(FileInfo file)
             return entries.Select(e => new FileInfo(e)).All(IsValidSymbolFileInfo);
         }
 
-        private async Task<PortablePdb> ExtractPortablePdbAsync(
+        private static async Task<PortablePdb> ExtractPortablePdbAsync(
             PackageArchiveReader symbolPackage,
             string pdbPath,
             CancellationToken cancellationToken)
@@ -147,7 +147,7 @@ private async Task<PortablePdb> ExtractPortablePdbAsync(
             {
                 using (var rawPdbStream = await symbolPackage.GetStreamAsync(pdbPath, cancellationToken))
                 {
-                    pdbStream = await rawPdbStream.AsTemporaryFileStreamAsync();
+                    pdbStream = await rawPdbStream.AsTemporaryFileStreamAsync(cancellationToken);
 
                     string signature;
                     using (var pdbReaderProvider = MetadataReaderProvider.FromPortablePdbStream(pdbStream, MetadataStreamOptions.LeaveOpen))

src/BaGetter.Core/Metadata/RegistrationBuilder.cs

@@ -32,7 +32,7 @@ public virtual BaGetterRegistrationIndexResponse BuildIndex(PackageRegistration
                     new BaGetterRegistrationIndexPage
                     {
                         RegistrationPageUrl = _url.GetRegistrationIndexUrl(registration.PackageId),
-                        Count = registration.Packages.Count(),
+                        Count = registration.Packages.Count,
                         Lower = sortedPackages.First().Version.ToNormalizedString().ToLowerInvariant(),
                         Upper = sortedPackages.Last().Version.ToNormalizedString().ToLowerInvariant(),
                         ItemsOrNull = sortedPackages.Select(ToRegistrationIndexPageItem).ToList(),
@@ -92,7 +92,7 @@ private BaGetRegistrationIndexPageItem ToRegistrationIndexPageItem(Package packa
                 },
             };
 
-        private IReadOnlyList<DependencyGroupItem> ToDependencyGroups(Package package)
+        private static List<DependencyGroupItem> ToDependencyGroups(Package package)
         {
             return package.Dependencies
                 .GroupBy(d => d.TargetFramework)

src/BaGetter.Core/PackageDatabase.cs

@@ -132,7 +132,7 @@ private async Task<bool> TryUpdatePackageAsync(
             var package = await _context.Packages
                 .Where(p => p.Id == id)
                 .Where(p => p.NormalizedVersionString == version.ToNormalizedString())
-                .FirstOrDefaultAsync();
+                .FirstOrDefaultAsync(cancellationToken);
 
             if (package != null)
             {

src/BaGetter.Core/Search/DatabaseSearchService.cs

@@ -145,7 +145,8 @@ public async Task<DependentsResponse> FindDependentsAsync(string packageId, Canc
             return _searchBuilder.BuildDependents(dependents);
         }
 
-        private IQueryable<Package> ApplySearchQuery(IQueryable<Package> query, string search)
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1862:Use the 'StringComparison' method overloads to perform case-insensitive string comparisons", Justification = "Not for EF queries")]
+        private static IQueryable<Package> ApplySearchQuery(IQueryable<Package> query, string search)
         {
             if (string.IsNullOrEmpty(search))
             {
@@ -157,7 +158,7 @@ private IQueryable<Package> ApplySearchQuery(IQueryable<Package> query, string s
             return query.Where(p => p.Id.ToLower().Contains(search));
         }
 
-        private IQueryable<Package> ApplySearchFilters(
+        private static IQueryable<Package> ApplySearchFilters(
             IQueryable<Package> query,
             bool includePrerelease,
             bool includeSemVer2,

src/BaGetter.Core/ServiceIndex/BaGetterServiceIndex.cs

@@ -15,7 +15,7 @@ public BaGetterServiceIndex(IUrlGenerator url)
             _url = url ?? throw new ArgumentNullException(nameof(url));
         }
 
-        private IEnumerable<ServiceIndexItem> BuildResource(string name, string url, params string[] versions)
+        private static IEnumerable<ServiceIndexItem> BuildResource(string name, string url, params string[] versions)
         {
             foreach (var version in versions)
             {

src/BaGetter.Core/Storage/FileStorageService.cs

@@ -18,7 +18,7 @@ public class FileStorageService : IStorageService
 
         public FileStorageService(IOptionsSnapshot<FileSystemStorageOptions> options)
         {
-            if (options == null) throw new ArgumentNullException(nameof(options));
+            ArgumentNullException.ThrowIfNull(options);
 
             // Resolve relative path components ('.'/'..') and ensure there is a trailing slash.
             _storePath = Path.GetFullPath(options.Value.Path);
@@ -51,7 +51,7 @@ public async Task<StoragePutResult> PutAsync(
             string contentType,
             CancellationToken cancellationToken = default)
         {
-            if (content == null) throw new ArgumentNullException(nameof(content));
+            ArgumentNullException.ThrowIfNull(content);
             if (string.IsNullOrEmpty(contentType)) throw new ArgumentException("Content type is required", nameof(contentType));
 
             cancellationToken.ThrowIfCancellationRequested();

src/BaGetter.Core/Storage/SymbolStorageService.cs

@@ -47,27 +47,27 @@ public async Task<Stream> GetPortablePdbContentStreamOrNullAsync(string filename
             }
         }
 
-        private string GetPathForKey(string filename, string key)
+        private static string GetPathForKey(string filename, string key)
         {
             // Ensure the filename doesn't try to escape out of the current directory.
             var tempPath = Path.GetDirectoryName(Path.GetTempPath());
             var expandedPath = Path.GetDirectoryName(Path.Combine(tempPath, filename));
-            
+
             if (expandedPath != tempPath)
             {
-                throw new ArgumentException(nameof(filename));
+                throw new ArgumentException($"Invalid file name: \"{filename}\" (can't escape the current directory)", nameof(filename));
             }
 
             if (!key.All(char.IsLetterOrDigit))
             {
-                throw new ArgumentException(nameof(key));
+                throw new ArgumentException($"Invalid key: \"{key}\" (must contain exclusively letters and digits)", nameof(key));
             }
 
             // The key's first 32 characters are the GUID, the remaining characters are the age.
             // See: https://github.com/dotnet/symstore/blob/98717c63ec8342acf8a07aa5c909b88bd0c664cc/docs/specs/SSQP_Key_Conventions.md#portable-pdb-signature
             // Debuggers should always use the age "ffffffff", however Visual Studio 2019
             // users have reported other age values. We will ignore the age.
-            key = key.Substring(0, 32) + "ffffffff";
+            key = string.Concat(key.AsSpan(0, 32), "ffffffff");
 
             return Path.Combine(
                 SymbolsPathPrefix,

src/BaGetter.Core/Upstream/Clients/V2UpstreamClient.cs

@@ -27,15 +27,14 @@ public class V2UpstreamClient : IUpstreamClient, IDisposable
         private readonly SourceRepository _repository;
         private readonly INuGetLogger _ngLogger;
         private readonly ILogger _logger;
+        private static readonly char[] TagsSeparators = {';'};
+        private static readonly char[] AuthorsSeparators = new[] { ',', ';', '\t', '\n', '\r' };
 
         public V2UpstreamClient(
             IOptionsSnapshot<MirrorOptions> options,
             ILogger logger)
         {
-            if (options is null)
-            {
-                throw new ArgumentNullException(nameof(options));
-            }
+            ArgumentNullException.ThrowIfNull(options);
 
             if (options.Value?.PackageSource?.AbsolutePath == null)
             {
@@ -126,7 +125,11 @@ public async Task<Stream> DownloadPackageOrNullAsync(
             }
         }
 
-        public void Dispose() => _cache.Dispose();
+        public void Dispose()
+        {
+            _cache.Dispose();
+            GC.SuppressFinalize(this);
+        }
 
         private Package ToPackage(IPackageSearchMetadata package)
         {
@@ -151,18 +154,18 @@ private Package ToPackage(IPackageSearchMetadata package)
                 PackageTypes = new List<PackageType>(),
                 RepositoryUrl = null,
                 RepositoryType = null,
-                Tags = package.Tags?.Split(new[] {';'}, StringSplitOptions.RemoveEmptyEntries),
+                Tags = package.Tags?.Split(TagsSeparators, StringSplitOptions.RemoveEmptyEntries),
 
                 Dependencies = ToDependencies(package)
             };
         }
 
-        private string[] ParseAuthors(string authors)
+        private static string[] ParseAuthors(string authors)
         {
             if (string.IsNullOrEmpty(authors)) return Array.Empty<string>();
 
             return authors
-                .Split(new[] { ',', ';', '\t', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries)
+                .Split(AuthorsSeparators, StringSplitOptions.RemoveEmptyEntries)
                 .Select(a => a.Trim())
                 .ToArray();
         }

src/BaGetter.Core/Upstream/Clients/V3UpstreamClient.cs

@@ -18,6 +18,7 @@ public class V3UpstreamClient : IUpstreamClient
     {
         private readonly NuGetClient _client;
         private readonly ILogger<V3UpstreamClient> _logger;
+        private static readonly char[] Separator = { ',', ';', '\t', '\n', '\r' };
 
         public V3UpstreamClient(NuGetClient client, ILogger<V3UpstreamClient> logger)
         {
@@ -117,7 +118,7 @@ private Package ToPackage(PackageMetadata metadata)
             };
         }
 
-        private Uri ParseUri(string uriString)
+        private static Uri ParseUri(string uriString)
         {
             if (uriString == null) return null;
 
@@ -129,12 +130,12 @@ private Uri ParseUri(string uriString)
             return uri;
         }
 
-        private string[] ParseAuthors(string authors)
+        private static string[] ParseAuthors(string authors)
         {
             if (string.IsNullOrEmpty(authors)) return Array.Empty<string>();
 
             return authors
-                .Split(new[] { ',', ';', '\t', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries)
+                .Split(Separator, StringSplitOptions.RemoveEmptyEntries)
                 .Select(a => a.Trim())
                 .ToArray();
         }

src/BaGetter.Core/Upstream/DownloadsImporter.cs

@@ -29,7 +29,7 @@ public DownloadsImporter(
         public async Task ImportAsync(CancellationToken cancellationToken)
         {
             var packageDownloads = await _downloadsSource.GetPackageDownloadsAsync();
-            var packages = await _context.Packages.CountAsync();
+            var packages = await _context.Packages.CountAsync(cancellationToken);
             var batches = (packages / BatchSize) + 1;
 
             for (var batch = 0; batch < batches; batch++)
@@ -41,8 +41,8 @@ public async Task ImportAsync(CancellationToken cancellationToken)
                     var packageId = package.Id.ToLowerInvariant();
                     var packageVersion = package.NormalizedVersionString.ToLowerInvariant();
 
-                    if (!packageDownloads.ContainsKey(packageId) ||
-                        !packageDownloads[packageId].ContainsKey(packageVersion))
+                    if (!packageDownloads.TryGetValue(packageId, out var value) ||
+                        !value.ContainsKey(packageVersion))
                     {
                         continue;
                     }