Raise AnalysisLevel to 6.0-Recommended, fix warnings (#2086)

Raise the .NET Code Analysis Level to `6.0-Minimum` and then `6.0-Recommended`.
This commit is contained in:
George Pollard
2022-06-29 09:27:33 +12:00
committed by GitHub
parent 3c3aeaa569
commit 2a6a0d9996
24 changed files with 100 additions and 64 deletions

View File

@ -139,3 +139,26 @@ csharp_preserve_single_line_blocks = true
[*.{cs}]
dotnet_diagnostic.IDE0005.severity = warning
# allow use of FirstOrDefault/LastOrDefault: https://github.com/dotnet/roslyn-analyzers/issues/1817 & https://github.com/dotnet/roslyn-analyzers/pull/4211#issuecomment-1003578755
dotnet_code_quality.CA1826.exclude_ordefault_methods = true
# permit underscores in identifier names (e.g. tests, constants)
dotnet_diagnostic.CA1707.severity = none
# don't care about names that conflict with keywords in other languages
dotnet_diagnostic.CA1716.severity = none
# don't care about names that contain built-in identifiers
dotnet_diagnostic.CA1711.severity = none
dotnet_diagnostic.CA1720.severity = none
# allow static fields on generic types
dotnet_diagnostic.CA1000.severity = none
# don't worry about performance of ILogger invocations (yet?)
dotnet_diagnostic.CA1848.severity = none
# allow throwing base "Exception" class, since it's done a lot
# TODO: improve this
dotnet_diagnostic.CA2201.severity = suggestion

View File

@ -59,7 +59,7 @@ class AppInsights : ILog {
//TODO: Should we write errors and Exception to std err ?
class Console : ILog {
private string DictToString<T>(IReadOnlyDictionary<string, T>? d) {
private static string DictToString<T>(IReadOnlyDictionary<string, T>? d) {
if (d is null) {
return string.Empty;
} else {
@ -67,14 +67,14 @@ class Console : ILog {
}
}
private void LogTags(Guid correlationId, IReadOnlyDictionary<string, string> tags) {
private static void LogTags(Guid correlationId, IReadOnlyDictionary<string, string> tags) {
var ts = DictToString(tags);
if (!string.IsNullOrEmpty(ts)) {
System.Console.WriteLine($"[{correlationId}] Tags:{ts}");
}
}
private void LogMetrics(Guid correlationId, IReadOnlyDictionary<string, double>? metrics) {
private static void LogMetrics(Guid correlationId, IReadOnlyDictionary<string, double>? metrics) {
var ms = DictToString(metrics);
if (!string.IsNullOrEmpty(ms)) {
System.Console.Out.WriteLine($"[{correlationId}] Metrics:{DictToString(metrics)}");
@ -126,7 +126,7 @@ internal interface ILogTracerInternal : ILogTracer {
public class LogTracer : ILogTracerInternal {
private string? GetCaller() {
private static string? GetCaller() {
return new StackTrace()?.GetFrame(2)?.GetMethod()?.DeclaringType?.FullName;
}

View File

@ -8,7 +8,7 @@ public enum ErrorCode {
INVALID_PERMISSION = 451,
MISSING_EULA_AGREEMENT = 452,
INVALID_JOB = 453,
INVALID_TASK = 453,
INVALID_TASK = INVALID_JOB,
UNABLE_TO_ADD_TASK_TO_JOB = 454,
INVALID_CONTAINER = 455,
UNABLE_TO_RESIZE = 456,

View File

@ -333,7 +333,7 @@ public record InstanceConfig
"Standard_B2s") { }
public InstanceConfig() : this(String.Empty) { }
public List<Guid>? CheckAdmins(List<Guid>? value) {
public static List<Guid>? CheckAdmins(List<Guid>? value) {
if (value is not null && value.Count == 0) {
throw new ArgumentException("admins must be null or contain at least one UUID");
} else {

View File

@ -20,14 +20,14 @@ public class QueueTaskHearbeat {
[Function("QueueTaskHeartbeat")]
public async Async.Task Run([QueueTrigger("task-heartbeat", Connection = "AzureWebJobsStorage")] string msg) {
_logger.LogInformation($"heartbeat: {msg}");
_logger.LogInformation("heartbeat: {Message}", msg);
var hb = JsonSerializer.Deserialize<TaskHeartbeatEntry>(msg, EntityConverter.GetJsonSerializerOptions()).EnsureNotNull($"wrong data {msg}");
var task = await _tasks.GetByTaskId(hb.TaskId);
if (task == null) {
_logger.LogWarning($"invalid task id: {hb.TaskId}");
_logger.LogWarning("invalid task id: {TaskId}", hb.TaskId);
return;
}

View File

@ -21,7 +21,7 @@ public class TimerDaily {
public async Async.Task Run([TimerTrigger("1.00:00:00")] TimerInfo myTimer) {
var scalesets = _scalesets.Search();
await foreach (var scaleset in scalesets) {
_logger.LogInformation($"updating scaleset configs: {scaleset.ScalesetId}");
_logger.LogInformation("updating scaleset configs: {ScaleSetId}", scaleset.ScalesetId);
// todo: do it in batches
await _scalesets.Replace(scaleset with { NeedsConfigUpdate = true });
}
@ -29,7 +29,7 @@ public class TimerDaily {
var expiredWebhookLogs = _webhookMessageLogs.SearchExpired();
await foreach (var logEntry in expiredWebhookLogs) {
_logger.LogInformation($"stopping expired webhook message log: {logEntry.WebhookId}:{logEntry.EventId}");
_logger.LogInformation("stopping expired webhook message log: {WebhookId}:{EventId}", logEntry.WebhookId, logEntry.EventId);
await _webhookMessageLogs.Delete(logEntry);
}
}

View File

@ -21,7 +21,7 @@ public class Config : IConfig {
_queue = queue;
}
private BlobContainerSasPermissions ConvertPermissions(ContainerPermission permission) {
private static BlobContainerSasPermissions ConvertPermissions(ContainerPermission permission) {
BlobContainerSasPermissions blobPermissions = 0;
if (permission.HasFlag(ContainerPermission.Read)) {
blobPermissions |= BlobContainerSasPermissions.Read;

View File

@ -14,7 +14,7 @@ public interface IContainers {
public Async.Task<BlobContainerClient?> FindContainer(Container container, StorageType storageType);
public Async.Task<Uri> GetFileSasUrl(Container container, string name, StorageType storageType, BlobSasPermissions permissions, TimeSpan? duration = null);
public Async.Task SaveBlob(Container container, string v1, string v2, StorageType config);
public Async.Task SaveBlob(Container container, string name, string data, StorageType storageType);
public Async.Task<Guid> GetInstanceId();
public Async.Task<Uri?> GetFileUrl(Container container, string name, StorageType storageType);
@ -125,7 +125,7 @@ public class Containers : IContainers {
return sasUrl;
}
public (DateTimeOffset, DateTimeOffset) SasTimeWindow(TimeSpan timeSpan) {
public static (DateTimeOffset, DateTimeOffset) SasTimeWindow(TimeSpan timeSpan) {
// SAS URLs are valid 6 hours earlier, primarily to work around dev
// workstations having out-of-sync time. Additionally, SAS URLs are stopped
// 15 minutes later than requested based on "Be careful with SAS start time"
@ -149,7 +149,7 @@ public class Containers : IContainers {
public Async.Task<Guid> GetInstanceId() => _getInstanceId.Value;
private readonly Lazy<Async.Task<Guid>> _getInstanceId;
public Uri? GetContainerSasUrlService(
public static Uri? GetContainerSasUrlService(
BlobContainerClient client,
BlobSasPermissions permissions,
bool tag = false,

View File

@ -2,7 +2,7 @@
public static class Defs {
public static Dictionary<TaskType, TaskDefinition> TASK_DEFINITIONS = new Dictionary<TaskType, TaskDefinition>() {
public static readonly IReadOnlyDictionary<TaskType, TaskDefinition> TASK_DEFINITIONS = new Dictionary<TaskType, TaskDefinition>() {
{ TaskType.Coverage ,
new TaskDefinition(
Features: new[] {

View File

@ -35,9 +35,9 @@ namespace Microsoft.OneFuzz.Service {
_creds = creds;
}
public async Async.Task QueueSignalrEvent(EventMessage eventMessage) {
var message = new SignalREvent("events", new List<EventMessage>() { eventMessage });
await _queue.SendMessage("signalr-events", JsonSerializer.Serialize(message), StorageType.Config);
public async Async.Task QueueSignalrEvent(EventMessage message) {
var ev = new SignalREvent("events", new List<EventMessage>() { message });
await _queue.SendMessage("signalr-events", JsonSerializer.Serialize(ev), StorageType.Config);
}
public async Async.Task SendEvent(BaseEvent anEvent) {

View File

@ -71,7 +71,7 @@ public class Extensions : IExtensions {
return extensions;
}
public VirtualMachineScaleSetExtensionData KeyVaultExtension(string region, KeyvaultExtensionConfig keyVault, Os vmOs) {
public static VirtualMachineScaleSetExtensionData KeyVaultExtension(string region, KeyvaultExtensionConfig keyVault, Os vmOs) {
var keyVaultName = keyVault.KeyVaultName;
var certName = keyVault.CertName;
var uri = keyVaultName + certName;
@ -119,7 +119,7 @@ public class Extensions : IExtensions {
}
}
public VirtualMachineScaleSetExtensionData AzSecExtension(string region) {
public static VirtualMachineScaleSetExtensionData AzSecExtension(string region) {
return new VirtualMachineScaleSetExtensionData {
Name = "AzureSecurityLinuxAgent",
Publisher = "Microsoft.Azure.Security.Monitoring",
@ -131,7 +131,7 @@ public class Extensions : IExtensions {
}
public VirtualMachineScaleSetExtensionData AzMonExtension(string region, AzureMonitorExtensionConfig azureMonitor) {
public static VirtualMachineScaleSetExtensionData AzMonExtension(string region, AzureMonitorExtensionConfig azureMonitor) {
var authId = azureMonitor.MonitoringGCSAuthId;
var configVersion = azureMonitor.ConfigVersion;
var moniker = azureMonitor.Moniker;
@ -164,7 +164,7 @@ public class Extensions : IExtensions {
public VirtualMachineScaleSetExtensionData GenevaExtension(string region) {
public static VirtualMachineScaleSetExtensionData GenevaExtension(string region) {
return new VirtualMachineScaleSetExtensionData {
Name = "Microsoft.Azure.Geneva.GenevaMonitoring",
Publisher = "Microsoft.Azure.Geneva",
@ -175,7 +175,7 @@ public class Extensions : IExtensions {
};
}
public VirtualMachineScaleSetExtensionData? DependencyExtension(string region, Os vmOs) {
public static VirtualMachineScaleSetExtensionData? DependencyExtension(string region, Os vmOs) {
if (vmOs == Os.Windows) {
return new VirtualMachineScaleSetExtensionData {

View File

@ -41,8 +41,8 @@ public interface INodeOperations : IStatefulOrm<Node, NodeState> {
IAsyncEnumerable<Node> GetDeadNodes(Guid scaleSetId, TimeSpan expirationPeriod);
Async.Task MarkTasksStoppedEarly(Node node, Error? error = null);
static TimeSpan NODE_EXPIRATION_TIME = TimeSpan.FromHours(1.0);
static TimeSpan NODE_REIMAGE_TIME = TimeSpan.FromDays(6.0);
static readonly TimeSpan NODE_EXPIRATION_TIME = TimeSpan.FromHours(1.0);
static readonly TimeSpan NODE_REIMAGE_TIME = TimeSpan.FromDays(6.0);
Async.Task StopTask(Guid task_id);
}

View File

@ -42,7 +42,6 @@ public class PoolOperations : StatefulOrm<Pool, PoolState, PoolOperations>, IPoo
return QueryAsync(filter: $"client_id eq '{clientId.ToString()}'");
}
private string GetPoolQueue(Pool pool) {
return $"pool-{pool.PoolId.ToString("N")}";
}
private static string GetPoolQueue(Pool pool)
=> $"pool-{pool.PoolId.ToString("N")}";
}

View File

@ -17,7 +17,7 @@ public class Reports : IReports {
public async Async.Task<RegressionReportOrReport?> GetReportOrRegression(Container container, string fileName, bool expectReports = false, params string[] args) {
var filePath = String.Join("/", new[] { container.ContainerName, fileName });
if (!fileName.EndsWith(".json")) {
if (!fileName.EndsWith(".json", StringComparison.Ordinal)) {
if (expectReports) {
_log.Error($"get_report invalid extension: {filePath}");
}

View File

@ -7,7 +7,7 @@ public interface IReproOperations : IStatefulOrm<Repro, VmState> {
public Async.Task<Repro> Stopping(Repro repro);
public IAsyncEnumerable<Repro> SearchStates(IEnumerable<VmState>? States);
public IAsyncEnumerable<Repro> SearchStates(IEnumerable<VmState>? states);
}
public class ReproOperations : StatefulOrm<Repro, VmState, ReproOperations>, IReproOperations {

View File

@ -53,7 +53,7 @@ public class VmOperations : IVmOperations {
var disks = await _diskOperations.ListDisks(resourceGroup)
.ToAsyncEnumerable()
.Where(disk => disk.Data.Name.StartsWith(name))
.Where(disk => disk.Data.Name.StartsWith(name, StringComparison.Ordinal))
.AnyAsync();
if (disks) {
@ -99,7 +99,7 @@ public class VmOperations : IVmOperations {
var disks = _diskOperations.ListDisks(resourceGroup)
.ToAsyncEnumerable()
.Where(disk => disk.Data.Name.StartsWith(name));
.Where(disk => disk.Data.Name.StartsWith(name, StringComparison.Ordinal));
if (await disks.AnyAsync()) {
await foreach (var disk in disks) {

View File

@ -1,4 +1,6 @@
namespace Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
using System.Globalization;
namespace Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
public class CaseConverter {
/// get the start indices of each word and the lat indice
@ -37,6 +39,7 @@ public class CaseConverter {
}
public static string SnakeToPascal(string input) {
return string.Join("", input.Split('_', StringSplitOptions.RemoveEmptyEntries).Select(x => $"{Char.ToUpper(x[0])}{x.Substring(1)}"));
var ti = CultureInfo.InvariantCulture.TextInfo;
return string.Concat(input.Split('_', StringSplitOptions.RemoveEmptyEntries).Select(x => ti.ToTitleCase(x)));
}
}

View File

@ -51,7 +51,7 @@ public sealed class CustomEnumConverter<T> : JsonConverter<T> where T : Enum {
}
var type = typeof(T);
_skipFormat = type.GetCustomAttribute<SkipRename>() != null;
_skipFormat = type.GetCustomAttribute<SkipRenameAttribute>() != null;
if (continueProcessing) {
Array values = Enum.GetValues(type);
@ -102,7 +102,7 @@ public sealed class CustomEnumConverter<T> : JsonConverter<T> where T : Enum {
if (!_writeCache.TryGetValue(value, out JsonEncodedText formatted)) {
if (_writeCache.Count == NameCacheLimit) {
Debug.Assert(_readCache.Count == NameCacheLimit);
throw new ArgumentOutOfRangeException();
throw new InvalidOperationException("Hit cache limit");
}
formatted = FormatAndAddToCaches(value, options.Encoder, _skipFormat);
@ -118,7 +118,7 @@ public sealed class CustomEnumConverter<T> : JsonConverter<T> where T : Enum {
return valueEncoded;
}
private ValueTuple<string, JsonEncodedText> FormatEnumValue(string value, JsonNamingPolicy namingPolicy, JavaScriptEncoder? encoder, bool skipFormat = false) {
private static ValueTuple<string, JsonEncodedText> FormatEnumValue(string value, JsonNamingPolicy namingPolicy, JavaScriptEncoder? encoder, bool skipFormat = false) {
string converted;
if (!value.Contains(ValueSeparator)) {
@ -195,7 +195,7 @@ public sealed class PolymorphicConverter<T> : JsonConverter<T> {
public PolymorphicConverter(TypeDiscrimnatorAttribute typeDiscriminator, string discriminatedField) : base() {
_discriminatorField = typeDiscriminator.FieldName;
_typeProvider = (ITypeProvider)(typeDiscriminator.ConverterType.GetConstructor(new Type[] { })?.Invoke(null) ?? throw new JsonException());
_typeProvider = (ITypeProvider)(Activator.CreateInstance(typeDiscriminator.ConverterType) ?? throw new JsonException());
_discriminatedField = discriminatedField;
_constructorInfo = typeof(T).GetConstructors().FirstOrDefault() ?? throw new JsonException("No Constructor found");
_parameters = _constructorInfo.GetParameters()?.ToDictionary(x => x.Name ?? "") ?? throw new JsonException();

View File

@ -26,9 +26,12 @@ public class SerializeValueAttribute : Attribute { }
/// Indicates that the enum cases should no be renamed
[AttributeUsage(AttributeTargets.Enum)]
public class SkipRename : Attribute { }
public class SkipRenameAttribute : Attribute { }
[AttributeUsage(AttributeTargets.Parameter)]
public class RowKeyAttribute : Attribute { }
[AttributeUsage(AttributeTargets.Parameter)]
public class PartitionKeyAttribute : Attribute { }
[AttributeUsage(AttributeTargets.Property)]
public class TypeDiscrimnatorAttribute : Attribute {
public string FieldName { get; }
// the type of a function that takes the value of fieldName as an input and return the type
@ -66,15 +69,11 @@ public class EntityConverter {
private readonly ConcurrentDictionary<Type, EntityInfo> _cache;
private readonly ETag _emptyETag = new ETag();
public EntityConverter() {
_options = GetJsonSerializerOptions();
_cache = new ConcurrentDictionary<Type, EntityInfo>();
}
public static JsonSerializerOptions GetJsonSerializerOptions() {
var options = new JsonSerializerOptions() {
PropertyNamingPolicy = new OnefuzzNamingPolicy(),
@ -102,7 +101,7 @@ public class EntityConverter {
return ctor;
}
private IEnumerable<EntityProperty> GetEntityProperties<T>(ParameterInfo parameterInfo) {
private static IEnumerable<EntityProperty> GetEntityProperties<T>(ParameterInfo parameterInfo) {
var name = parameterInfo.Name.EnsureNotNull($"Invalid paramter {parameterInfo}");
var parameterType = parameterInfo.ParameterType.EnsureNotNull($"Invalid paramter {parameterInfo}");
var isRowkey = parameterInfo.GetCustomAttribute(typeof(RowKeyAttribute)) != null;
@ -112,7 +111,7 @@ public class EntityConverter {
(TypeDiscrimnatorAttribute, ITypeProvider)? discriminator = null;
if (discriminatorAttribute != null) {
var t = (ITypeProvider)(discriminatorAttribute.ConverterType.GetConstructor(new Type[] { })?.Invoke(null) ?? throw new Exception("unable to retrive the type provider"));
var t = (ITypeProvider)(Activator.CreateInstance(discriminatorAttribute.ConverterType) ?? throw new Exception("unable to retrive the type provider"));
discriminator = (discriminatorAttribute, t);
}
@ -149,13 +148,11 @@ public class EntityConverter {
public TableEntity ToTableEntity<T>(T typedEntity) where T : EntityBase {
if (typedEntity == null) {
throw new NullReferenceException();
}
var type = typeof(T)!;
if (type is null) {
throw new NullReferenceException();
throw new ArgumentNullException(nameof(typedEntity));
}
var type = typeof(T);
var entityInfo = GetEntityInfo<T>();
Dictionary<string, object?> columnValues = entityInfo.properties.SelectMany(x => x).Select(prop => {
var value = entityInfo.type.GetProperty(prop.name)?.GetValue(typedEntity);
@ -270,7 +267,7 @@ public class EntityConverter {
entityInfo.properties.Select(grouping => GetFieldValue(entityInfo, grouping.Key, entity)).ToArray();
try {
var entityRecord = (T)entityInfo.constructor.Invoke(parameters);
if (entity.ETag != _emptyETag) {
if (entity.ETag != default) {
entityRecord.ETag = entity.ETag;
}
entityRecord.TimeStamp = entity.Timestamp;

View File

@ -27,9 +27,11 @@ namespace ApiService.OneFuzzLib.Orm {
public class Orm<T> : IOrm<T> where T : EntityBase {
#pragma warning disable CA1051 // permit visible instance fields
protected readonly EntityConverter _entityConverter;
protected readonly IOnefuzzContext _context;
protected readonly ILogTracer _logTracer;
#pragma warning restore CA1051
public Orm(ILogTracer logTracer, IOnefuzzContext context) {
@ -136,7 +138,7 @@ namespace ApiService.OneFuzzLib.Orm {
}
public class StatefulOrm<T, TState, Self> : Orm<T>, IStatefulOrm<T, TState> where T : StatefulEntityBase<TState> where TState : Enum {
public class StatefulOrm<T, TState, TSelf> : Orm<T>, IStatefulOrm<T, TState> where T : StatefulEntityBase<TState> where TState : Enum {
static Lazy<Func<object>>? _partitionKeyGetter;
static Lazy<Func<object>>? _rowKeyGetter;
static ConcurrentDictionary<string, Func<T, Async.Task<T>>?> _stateFuncs = new ConcurrentDictionary<string, Func<T, Async.Task<T>>?>();
@ -147,7 +149,7 @@ namespace ApiService.OneFuzzLib.Orm {
static StatefulOrm() {
/// verify that all state transition function have the correct signature:
var thisType = typeof(Self);
var thisType = typeof(TSelf);
var states = Enum.GetNames(typeof(TState));
var delegateType = typeof(StateTransition);
MethodInfo delegateSignature = delegateType.GetMethod("Invoke")!;

View File

@ -4,10 +4,19 @@
<!-- set overall target framework -->
<TargetFramework>net6.0</TargetFramework>
<!-- build in Invariant mode: https://github.com/dotnet/runtime/blob/main/docs/design/features/globalization-invariant-mode.md -->
<!--
we never need locale-specific comparisons in the service,
so turning this on has two effects:
- reduces bundle size of .NET service
- disables a lot of .NET code analyzers that check for
correct use of locale-specific comparisons
-->
<InvariantGlobalization>true</InvariantGlobalization>
<!-- enable code analysis -->
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisLevel>6.0-Default</AnalysisLevel>
<!-- Goal: raise to 6.0-Minimum and then eventually 6.0-Recommended -->
<AnalysisLevel>6.0-Recommended</AnalysisLevel>
<!-- check nullable types -->
<Nullable>enable</Nullable>

View File

@ -65,6 +65,8 @@ public abstract class FunctionTestBase : IDisposable {
=> new EntityConverter().FromJsonString<T>(BodyAsString(data)) ?? throw new Exception($"unable to deserialize body as {typeof(T)}");
public void Dispose() {
GC.SuppressFinalize(this);
var (accountName, accountKey) = _storage.GetStorageAccountNameAndKey("").Result; // sync for test impls
if (accountName is not null && accountKey is not null) {
// clean up any tables & blobs that this test created

View File

@ -439,19 +439,20 @@ namespace Tests {
typeof(SecureString)
});
static bool IEnumerableEqual<T>(IEnumerable<T>? a, IEnumerable<T>? b) {
if (a is null && b is null) {
return true;
if (a is null) {
return b is null;
}
if (a!.Count() != b!.Count()) {
if (b is null) {
return false;
}
if (a!.Count() == 0 && b!.Count() == 0) {
return true;
if (a.Count() != b.Count()) {
return false;
}
foreach (var v in a!.Zip(b!)) {
if (!AreEqual(v.First, v.Second)) {
foreach (var (first, second) in a.Zip(b)) {
if (!AreEqual(first, second)) {
return false;
}
}

View File

@ -8,7 +8,7 @@ namespace Tests;
public class SchedulerTests {
IEnumerable<Task> BuildTasks(int size) {
static IEnumerable<Task> BuildTasks(int size) {
return Enumerable.Range(0, size).Select(i =>
new Task(
Guid.Empty,
@ -123,7 +123,7 @@ public class SchedulerTests {
CheckBuckets(buckets, tasks, 12);
}
void CheckBuckets(ILookup<Scheduler.BucketId, Task> buckets, List<Task> tasks, int bucketCount) {
static void CheckBuckets(ILookup<Scheduler.BucketId, Task> buckets, List<Task> tasks, int bucketCount) {
Assert.Equal(buckets.Count, bucketCount);
foreach (var task in tasks) {