Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Activate .NET code analysis #48

Merged
merged 19 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3c10b54
Enable .NET source code analysis
0xced Aug 27, 2023
26881b5
Fix CA2016 (Forward the CancellationToken parameter to methods that t…
0xced Aug 27, 2023
65cf9c0
+Fix for CA2016 (Forward the CancellationToken parameter to methods t…
Jan 28, 2024
5bc45cc
Fix CA1826 (Use property instead of Linq Enumerable method)
0xced Aug 27, 2023
b3196d8
Fix CA1825 (Avoid zero-length array allocations)
0xced Aug 27, 2023
8fac680
Fix CA1822 (Mark members as static)
0xced Aug 27, 2023
d35cf21
Fix CA2208 (Instantiate argument exceptions correctly)
0xced Aug 27, 2023
39c01a1
Fix CA2254 (Template should be a static expression)
0xced Aug 27, 2023
fec99ed
Fix CA1816 (Call GC.SuppressFinalize correctly)
0xced Aug 27, 2023
824bf19
Remove unnecessary build property
Regenhardt Jan 29, 2024
a4e9109
Fix CA1510 (Use throw helper ThrowIfNull)
Regenhardt Jan 29, 2024
71a39c1
Fix CA1861 (Avoid constant arrays as arguments)
Regenhardt Jan 29, 2024
d9a0248
Fix CA1862 (Use the 'StringComparison' method overloads )
Regenhardt Jan 29, 2024
1fea0b1
Fix CA1854 (Prefer the IDictionary.TryGetValue(TKey, out TValue) method)
Regenhardt Jan 29, 2024
290e39d
Fix CA1859 (Use concrete types when possible for improved performance)
Regenhardt Jan 29, 2024
e51a23c
Fix CA1854 (Use span-based 'string.Concat')
Regenhardt Jan 29, 2024
3169475
Un-fix CA1862 (Use the 'StringComparison' method overloads )
Regenhardt Jan 29, 2024
50b9290
use Array.Empty
FroggieFrog Jan 26, 2024
e8ec802
Fix CA1860 (Avoid using 'Enumerable.Any()' extension method)
Regenhardt Jan 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions src/BaGetter.Aliyun/AliyunStorageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/BaGetter.Aws/S3StorageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/BaGetter.Azure/Search/AzureSearchService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions src/BaGetter.Azure/Search/IndexActionBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions src/BaGetter.Azure/Table/TableOperationBuilder.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/BaGetter.Azure/Table/TablePackageDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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)
{
Expand Down
2 changes: 1 addition & 1 deletion src/BaGetter.Azure/Table/TableSearchService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Newtonsoft.Json;

Expand All @@ -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>())
{
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/BaGetter.Core/Extensions/PackageArchiveReaderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/BaGetter.Core/Indexing/SymbolIndexingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions src/BaGetter.Core/Metadata/RegistrationBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/BaGetter.Core/PackageDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
5 changes: 3 additions & 2 deletions src/BaGetter.Core/Search/DatabaseSearchService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/BaGetter.Core/ServiceIndex/BaGetterServiceIndex.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
4 changes: 2 additions & 2 deletions src/BaGetter.Core/Storage/FileStorageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 5 additions & 5 deletions src/BaGetter.Core/Storage/SymbolStorageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 11 additions & 8 deletions src/BaGetter.Core/Upstream/Clients/V2UpstreamClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand All @@ -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();
}
Expand Down
7 changes: 4 additions & 3 deletions src/BaGetter.Core/Upstream/Clients/V3UpstreamClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;

Expand All @@ -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();
}
Expand Down
6 changes: 3 additions & 3 deletions src/BaGetter.Core/Upstream/DownloadsImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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++)
Expand All @@ -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;
}
Expand Down
Loading