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 9 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
2 changes: 1 addition & 1 deletion src/BaGetter.Aws/S3StorageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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 @@ -111,16 +111,16 @@

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);

Check warning on line 116 in src/BaGetter.Core/Extensions/PackageArchiveReaderExtensions.cs

View workflow job for this annotation

GitHub Actions / build

Prefer 'static readonly' fields over constant array arguments if the called method is called repeatedly and is not mutating the passed array (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)
}

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);

Check warning on line 123 in src/BaGetter.Core/Extensions/PackageArchiveReaderExtensions.cs

View workflow job for this annotation

GitHub Actions / build

Prefer 'static readonly' fields over constant array arguments if the called method is called repeatedly and is not mutating the passed array (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)
}

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 IReadOnlyList<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
4 changes: 2 additions & 2 deletions src/BaGetter.Core/Search/DatabaseSearchService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public async Task<DependentsResponse> FindDependentsAsync(string packageId, Canc
return _searchBuilder.BuildDependents(dependents);
}

private IQueryable<Package> ApplySearchQuery(IQueryable<Package> query, string search)
private static IQueryable<Package> ApplySearchQuery(IQueryable<Package> query, string search)
{
if (string.IsNullOrEmpty(search))
{
Expand All @@ -157,7 +157,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
8 changes: 4 additions & 4 deletions src/BaGetter.Core/Storage/SymbolStorageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,20 @@ 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.
Expand Down
8 changes: 6 additions & 2 deletions src/BaGetter.Core/Upstream/Clients/V2UpstreamClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
IOptionsSnapshot<MirrorOptions> options,
ILogger logger)
{
if (options is null)

Check warning on line 35 in src/BaGetter.Core/Upstream/Clients/V2UpstreamClient.cs

View workflow job for this annotation

GitHub Actions / build

Use 'ArgumentNullException.ThrowIfNull' instead of explicitly throwing a new exception instance (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1510)
{
throw new ArgumentNullException(nameof(options));
}
Expand Down Expand Up @@ -126,7 +126,11 @@
}
}

public void Dispose() => _cache.Dispose();
public void Dispose()
{
_cache.Dispose();
GC.SuppressFinalize(this);
}

private Package ToPackage(IPackageSearchMetadata package)
{
Expand All @@ -151,18 +155,18 @@
PackageTypes = new List<PackageType>(),
RepositoryUrl = null,
RepositoryType = null,
Tags = package.Tags?.Split(new[] {';'}, StringSplitOptions.RemoveEmptyEntries),

Check warning on line 158 in src/BaGetter.Core/Upstream/Clients/V2UpstreamClient.cs

View workflow job for this annotation

GitHub Actions / build

Prefer 'static readonly' fields over constant array arguments if the called method is called repeatedly and is not mutating the passed array (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)

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)

Check warning on line 169 in src/BaGetter.Core/Upstream/Clients/V2UpstreamClient.cs

View workflow job for this annotation

GitHub Actions / build

Prefer 'static readonly' fields over constant array arguments if the called method is called repeatedly and is not mutating the passed array (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)
.Select(a => a.Trim())
.ToArray();
}
Expand Down
4 changes: 2 additions & 2 deletions src/BaGetter.Core/Upstream/Clients/V3UpstreamClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@
};
}

private Uri ParseUri(string uriString)
private static Uri ParseUri(string uriString)
{
if (uriString == null) return null;

Expand All @@ -129,12 +129,12 @@
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)

Check warning on line 137 in src/BaGetter.Core/Upstream/Clients/V3UpstreamClient.cs

View workflow job for this annotation

GitHub Actions / build

Prefer 'static readonly' fields over constant array arguments if the called method is called repeatedly and is not mutating the passed array (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)
.Select(a => a.Trim())
.ToArray();
}
Expand Down
2 changes: 1 addition & 1 deletion src/BaGetter.Core/Upstream/DownloadsImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
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,7 +41,7 @@
var packageId = package.Id.ToLowerInvariant();
var packageVersion = package.NormalizedVersionString.ToLowerInvariant();

if (!packageDownloads.ContainsKey(packageId) ||

Check warning on line 44 in src/BaGetter.Core/Upstream/DownloadsImporter.cs

View workflow job for this annotation

GitHub Actions / build

Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854)
!packageDownloads[packageId].ContainsKey(packageVersion))
{
continue;
Expand Down
6 changes: 3 additions & 3 deletions src/BaGetter.Protocol/Catalog/CatalogProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private async Task<bool> ProcessPageAsync(

if (newCursor.HasValue && success)
{
await _cursor.SetAsync(newCursor.Value);
await _cursor.SetAsync(newCursor.Value, cancellationToken);
}

return success;
Expand All @@ -146,12 +146,12 @@ private async Task<bool> ProcessLeafAsync(CatalogLeafItem leafItem, Cancellation
{
if (leafItem.IsPackageDelete())
{
var packageDelete = await _client.GetPackageDeleteLeafAsync(leafItem.CatalogLeafUrl);
var packageDelete = await _client.GetPackageDeleteLeafAsync(leafItem.CatalogLeafUrl, cancellationToken);
success = await _leafProcessor.ProcessPackageDeleteAsync(packageDelete, cancellationToken);
}
else if (leafItem.IsPackageDetails())
{
var packageDetails = await _client.GetPackageDetailsLeafAsync(leafItem.CatalogLeafUrl);
var packageDetails = await _client.GetPackageDetailsLeafAsync(leafItem.CatalogLeafUrl, cancellationToken);
success = await _leafProcessor.ProcessPackageDetailsAsync(packageDetails, cancellationToken);
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/BaGetter.Protocol/Search/SearchClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async Task<SearchResponse> SearchAsync(
// See: https://github.com/loic-sharma/BaGet/issues/314
var client = await _clientfactory.GetSearchClientAsync(cancellationToken);

return await client.SearchAsync(query, skip, take, includePrerelease, includeSemVer2);
return await client.SearchAsync(query, skip, take, includePrerelease, includeSemVer2, cancellationToken);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/BaGetter.Web/BaGetterEndpointBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
using System.Diagnostics.CodeAnalysis;
using BaGetter.Web;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Routing.Constraints;

namespace BaGetter
{
[SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Would be a breaking change since it's part of the public API")]
public class BaGetterEndpointBuilder
{
public void MapEndpoints(IEndpointRouteBuilder endpoints)
Expand Down
8 changes: 4 additions & 4 deletions src/BaGetter.Web/Pages/Package.cshtml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public async Task OnGetAsync(string id, string version, CancellationToken cancel
PackageDownloadUrl = _url.GetPackageDownloadUrl(Package.Id, packageVersion);
}

private IReadOnlyList<DependencyGroupModel> ToDependencyGroups(Package package)
private static IReadOnlyList<DependencyGroupModel> ToDependencyGroups(Package package)
{
return package
.Dependencies
Expand All @@ -135,7 +135,7 @@ private IReadOnlyList<DependencyGroupModel> ToDependencyGroups(Package package)
.ToList();
}

private string PrettifyTargetFramework(string targetFramework)
private static string PrettifyTargetFramework(string targetFramework)
{
if (targetFramework == null) return "All Frameworks";

Expand Down Expand Up @@ -179,7 +179,7 @@ private string PrettifyTargetFramework(string targetFramework)
return $"{frameworkName} {frameworkVersion}";
}

private IReadOnlyList<VersionModel> ToVersions(IReadOnlyList<Package> packages, NuGetVersion selectedVersion)
private static IReadOnlyList<VersionModel> ToVersions(IReadOnlyList<Package> packages, NuGetVersion selectedVersion)
{
return packages
.Select(p => new VersionModel
Expand All @@ -205,7 +205,7 @@ private async Task<HtmlString> GetReadmeHtmlStringOrNullAsync(

using (var reader = new StreamReader(readmeStream))
{
readme = await reader.ReadToEndAsync();
readme = await reader.ReadToEndAsync(cancellationToken);
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
<NoWarn>$(NoWarn);1591</NoWarn>
</PropertyGroup>

<!-- .NET source code analysis https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview -->
<PropertyGroup>
Regenhardt marked this conversation as resolved.
Show resolved Hide resolved
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisLevel>latest-Minimum</AnalysisLevel>
</PropertyGroup>

<!-- Debugging properties -->
<PropertyGroup>
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
Expand Down
Loading