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

Standardize and Optimize Code with dotnet format #3754

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
8 changes: 6 additions & 2 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ csharp_indent_switch_labels = true
csharp_indent_case_contents = true
csharp_indent_labels = one_less_than_current

# Globalization rules
dotnet_code_quality.CA1305.excluded_symbol_names = T:System.Byte|T:System.SByte|T:System.Int16|T:System.UInt16|T:System.Int32|T:System.UInt32|T:System.Int64|T:System.UInt64|T:System.String|T:System.Text.StringBuilder|T:System.Convert

# Style rules
# Can't be in .globalconfig because dotnet format doesn't respect that https://github.com/dotnet/format/issues/1643

# Remove unnecessary cast
dotnet_diagnostic.IDE0004.severity = warning
# Remove unnecessary import
Expand Down Expand Up @@ -49,6 +51,8 @@ dotnet_diagnostic.IDE0032.severity = suggestion
dotnet_diagnostic.IDE0034.severity = suggestion
# Use pattern matching to avoid is check followed by a cast (without variable)
dotnet_diagnostic.IDE0038.severity = warning
# Use a local function instead of an anonymous function
dotnet_diagnostic.IDE0039.severity = none
# Use is null check
dotnet_diagnostic.IDE0041.severity = warning
# Deconstruct variable declaration
Expand Down Expand Up @@ -109,7 +113,7 @@ dotnet_diagnostic.IDE0260.severity = suggestion
# Use nameof
dotnet_diagnostic.IDE0280.severity = error

csharp_style_var_when_type_is_apparent = true
csharp_style_var_when_type_is_apparent = false
csharp_style_var_elsewhere = true

csharp_style_expression_bodied_methods = when_on_single_line
Expand Down
2 changes: 1 addition & 1 deletion Common.ruleset
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<Rule Id="BHI1004" Action="Error" />

<!-- Default branch of switch expression should throw InvalidOperationException/SwitchExpressionException or not throw -->
<Rule Id="BHI1005" Action="Error" />
<Rule Id="BHI1005" Action="Hidden" />

<!-- Do not discard local variables -->
<Rule Id="BHI1006" Action="Error" />
Expand Down
2 changes: 2 additions & 0 deletions contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ It's probably a good idea to get the .NET SDK, even if you're not working on a .

For EmuHawk and libraries in the main solution, which do not target .NET 6, we have [this page](https://github.com/TASEmulators/BizHawk/wiki/Available-C%23-and-.NET-features) documenting which features are actually available to use.

### Code Analysis

Visual Studio can detect and show issues and possible optimizations in the IDE before build time. In the Analyze menu pick Run Code Analysis -> On Solution. From there you can open the Error list in the bottom left and highlight issues of different severity level Errors/Warnings/Messages and adjust the scope as small as the open documents and as wide as the entire solution. Developers should target to have no code introduced flagging the Error and Warning levels. It's also good to minimize warnings at the Message severity level either by adopting the suggested fixes or by ignoring irrelevant Messages with `#pragma` directives. Many warnings can be auto-fixed by the `dotnet format` command or by the quick actions menu found on the left next to a line number when hovering a line.

## blip_buf
> Audio resampling library.
Expand Down
18 changes: 9 additions & 9 deletions src/BizHawk.BizInvoke/BizExvoker.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
Expand All @@ -23,7 +23,7 @@ public static class BizExvoker

static BizExvoker()
{
var aname = new AssemblyName("BizExvokeProxyAssembly");
AssemblyName aname = new("BizExvokeProxyAssembly");
ImplAssemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(aname, AssemblyBuilderAccess.Run);
ImplModuleBuilder = ImplAssemblyBuilder.DefineDynamicModule("BizExvokerModule");
}
Expand Down Expand Up @@ -65,10 +65,10 @@ public DelegateStorage(Type type)

var typeBuilder = ImplModuleBuilder.DefineType($"Bizhawk.BizExvokeHolder{type.Name}", TypeAttributes.Class | TypeAttributes.Public | TypeAttributes.Sealed);

foreach (var a in methods)
foreach (var (Info, Attr) in methods)
{
var delegateType = BizInvokeUtilities.CreateDelegateType(a.Info, a.Attr!.CallingConvention, typeBuilder, out _).CreateType()!;
DelegateTypes.Add(new StoredDelegateInfo(a.Info, delegateType, a.Attr.EntryPoint ?? a.Info.Name));
var delegateType = BizInvokeUtilities.CreateDelegateType(Info, Attr!.CallingConvention, typeBuilder, out _).CreateType()!;
DelegateTypes.Add(new StoredDelegateInfo(Info, delegateType, Attr.EntryPoint ?? Info.Name));
}
StorageType = typeBuilder.CreateType()!;
OriginalType = type;
Expand All @@ -77,15 +77,15 @@ public DelegateStorage(Type type)

private class ExvokerImpl : IImportResolver
{
private readonly Dictionary<string, IntPtr> EntryPoints = new Dictionary<string, IntPtr>();
private readonly Dictionary<string, IntPtr> EntryPoints = new();

private readonly List<Delegate> Delegates = new List<Delegate>();
private readonly List<Delegate> Delegates = new();

public ExvokerImpl(object o, DelegateStorage d, ICallingConventionAdapter a)
{
foreach (var sdt in d.DelegateTypes)
{
var del = Delegate.CreateDelegate(sdt.DelegateType, o, sdt.Method);
Delegate del = Delegate.CreateDelegate(sdt.DelegateType, o, sdt.Method);
Delegates.Add(del); // prevent garbage collection of the delegate, which would invalidate the pointer
EntryPoints.Add(sdt.EntryPointName, a.GetFunctionPointerForDelegate(del));
}
Expand All @@ -96,7 +96,7 @@ public ExvokerImpl(object o, DelegateStorage d, ICallingConventionAdapter a)
public IntPtr GetProcAddrOrThrow(string entryPoint) => EntryPoints.TryGetValue(entryPoint, out var ret) ? ret : throw new InvalidOperationException($"could not find {entryPoint} in exports");
}

private static readonly Dictionary<Type, DelegateStorage> Impls = new Dictionary<Type, DelegateStorage>();
private static readonly Dictionary<Type, DelegateStorage> Impls = new();


public static IImportResolver GetExvoker(object o, ICallingConventionAdapter a)
Expand Down
12 changes: 6 additions & 6 deletions src/BizHawk.BizInvoke/BizInvokeUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,24 @@ public static TypeBuilder CreateDelegateType(MethodInfo method, CallingConventio
for (int i = 0; i < paramInfos.Length; i++)
{
var p = delegateInvoke.DefineParameter(i + 1, ParameterAttributes.None, paramInfos[i].Name);
foreach (var a in paramInfos[i].GetCustomAttributes(false))
foreach (object? a in paramInfos[i].GetCustomAttributes(false))
{
p.SetCustomAttribute(GetAttributeBuilder(a));
}
}

{
var p = delegateInvoke.DefineParameter(0, ParameterAttributes.Retval, method.ReturnParameter!.Name);
foreach (var a in method.ReturnParameter.GetCustomAttributes(false))
foreach (object? a in method.ReturnParameter.GetCustomAttributes(false))
{
p.SetCustomAttribute(GetAttributeBuilder(a));
}
}

delegateInvoke.SetImplementationFlags(MethodImplAttributes.Runtime | MethodImplAttributes.Managed);

// add the [UnmanagedFunctionPointer] to the delegate so interop will know how to call it
var attr = new CustomAttributeBuilder(
delegateInvoke.SetImplementationFlags(MethodImplAttributes.Runtime | MethodImplAttributes.Managed);
// add the [UnmanagedFunctionPointer] to the delegate so interop will know how to call it
CustomAttributeBuilder attr = new(
typeof(UnmanagedFunctionPointerAttribute).GetConstructor(new[] { typeof(CallingConvention) })!,
new object[] { nativeCall });
delegateType.SetCustomAttribute(attr);
Expand Down
33 changes: 14 additions & 19 deletions src/BizHawk.BizInvoke/BizInvoker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public InvokerImpl(

public object Create(IImportResolver dll, IMonitor? monitor, ICallingConventionAdapter adapter)
{
var ret = Activator.CreateInstance(_implType)!;
object ret = Activator.CreateInstance(_implType)!;
_connectCallingConventionAdapter(ret, adapter);
foreach (var f in _hooks)
{
Expand Down Expand Up @@ -93,7 +93,7 @@ public object Create(IImportResolver dll, IMonitor? monitor, ICallingConventionA

static BizInvoker()
{
var aname = new AssemblyName("BizInvokeProxyAssembly");
AssemblyName aname = new("BizInvokeProxyAssembly");
ImplAssemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(aname, AssemblyBuilderAccess.Run);
ImplModuleBuilder = ImplAssemblyBuilder.DefineDynamicModule("BizInvokerModule");
ClassFieldOffset = BizInvokerUtilities.ComputeClassFirstFieldOffset();
Expand All @@ -109,7 +109,7 @@ static BizInvoker()
public static T GetInvoker<T>(IImportResolver dll, ICallingConventionAdapter adapter)
where T : class
{
var nonTrivialAdapter = adapter.GetType() != CallingConventionAdapters.Native.GetType();
bool nonTrivialAdapter = adapter.GetType() != CallingConventionAdapters.Native.GetType();
InvokerImpl impl;
lock (Impls) impl = Impls.GetValueOrPut(
typeof(T),
Expand All @@ -126,7 +126,7 @@ public static T GetInvoker<T>(IImportResolver dll, ICallingConventionAdapter ada
public static T GetInvoker<T>(IImportResolver dll, IMonitor monitor, ICallingConventionAdapter adapter)
where T : class
{
var nonTrivialAdapter = adapter.GetType() != CallingConventionAdapters.Native.GetType();
bool nonTrivialAdapter = adapter.GetType() != CallingConventionAdapters.Native.GetType();
InvokerImpl impl;
lock (Impls) impl = Impls.GetValueOrPut(
typeof(T),
Expand All @@ -152,13 +152,8 @@ private static InvokerImpl CreateProxy(Type baseType, bool monitor, bool nonTriv
throw new InvalidOperationException("Type must be public");
}

var baseConstructor = baseType.GetConstructor(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, Type.EmptyTypes, null);
if (baseConstructor == null)
{
throw new InvalidOperationException("Base type must have a zero arg constructor");
}

var baseMethods = baseType.GetMethods(BindingFlags.Instance | BindingFlags.Public)
var baseConstructor = baseType.GetConstructor(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, Type.EmptyTypes, null) ?? throw new InvalidOperationException("Base type must have a zero arg constructor");
List<(MethodInfo Info, BizImportAttribute Attr)> baseMethods = baseType.GetMethods(BindingFlags.Instance | BindingFlags.Public)
.Select(static m => (Info: m, Attr: m.GetCustomAttributes(true).OfType<BizImportAttribute>().FirstOrDefault()))
.Where(a => a.Attr != null)
.ToList();
Expand All @@ -179,21 +174,21 @@ private static InvokerImpl CreateProxy(Type baseType, bool monitor, bool nonTriv
}

// hooks that will be run on the created proxy object
var postCreateHooks = new List<Action<object, IImportResolver, ICallingConventionAdapter>>();
List<Action<object, IImportResolver, ICallingConventionAdapter>> postCreateHooks = new();

var type = ImplModuleBuilder.DefineType($"Bizhawk.BizInvokeProxy{baseType.Name}", TypeAttributes.Class | TypeAttributes.Public | TypeAttributes.Sealed, baseType);

var monitorField = monitor ? type.DefineField("MonitorField", typeof(IMonitor), FieldAttributes.Public) : null;

var adapterField = type.DefineField("CallingConvention", typeof(ICallingConventionAdapter), FieldAttributes.Public);

foreach (var mi in baseMethods)
foreach (var (Info, Attr) in baseMethods)
{
var entryPointName = mi.Attr!.EntryPoint ?? mi.Info.Name;
string entryPointName = Attr!.EntryPoint ?? Info.Name;

var hook = mi.Attr.Compatibility
? ImplementMethodDelegate(type, mi.Info, mi.Attr.CallingConvention, entryPointName, monitorField, nonTrivialAdapter)
: ImplementMethodCalli(type, mi.Info, mi.Attr.CallingConvention, entryPointName, monitorField, adapterField);
var hook = Attr.Compatibility
? ImplementMethodDelegate(type, Info, Attr.CallingConvention, entryPointName, monitorField, nonTrivialAdapter)
: ImplementMethodCalli(type, Info, Attr.CallingConvention, entryPointName, monitorField, adapterField);

postCreateHooks.Add(hook);
}
Expand Down Expand Up @@ -336,7 +331,7 @@ private static Action<object, IImportResolver, ICallingConventionAdapter> Implem
{
var paramInfos = baseMethod.GetParameters();
var paramTypes = paramInfos.Select(p => p.ParameterType).ToArray();
var paramLoadInfos = new List<ParameterLoadInfo>();
List<ParameterLoadInfo> paramLoadInfos = new();
var returnType = baseMethod.ReturnType;
if (returnType != typeof(void) && !returnType.IsPrimitive && !returnType.IsPointer && !returnType.IsEnum)
{
Expand Down Expand Up @@ -381,7 +376,7 @@ private static Action<object, IImportResolver, ICallingConventionAdapter> Implem

bool WantsWinAPIBool()
{
var attrs = baseMethod.ReturnTypeCustomAttributes.GetCustomAttributes(typeof(MarshalAsAttribute), false);
object[] attrs = baseMethod.ReturnTypeCustomAttributes.GetCustomAttributes(typeof(MarshalAsAttribute), false);
return attrs.Length > 0 && ((MarshalAsAttribute)attrs[0]).Value is UnmanagedType.Bool;
}

Expand Down
21 changes: 9 additions & 12 deletions src/BizHawk.BizInvoke/BizInvokerUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,15 @@ private class CF
/// Computes the byte offset of the first field of any class relative to a class pointer.
/// </summary>
/// <returns></returns>
public static int ComputeClassFirstFieldOffset()
{
return ComputeFieldOffset(typeof(CF).GetField("FirstField"));
}
public static int ComputeClassFirstFieldOffset() => ComputeFieldOffset(typeof(CF).GetField("FirstField"));

/// <summary>
/// Compute the byte offset of the first byte of string data (UTF16) relative to a pointer to the string.
/// </summary>
/// <returns></returns>
public static int ComputeStringOffset()
{
var s = new string(Array.Empty<char>());
string s = new(Array.Empty<char>());
int ret;
fixed(char* fx = s)
{
Expand All @@ -77,7 +74,7 @@ public static int ComputeStringOffset()
/// <returns></returns>
public static int ComputeValueArrayElementOffset()
{
var arr = new int[4];
int[] arr = new int[4];
int ret;
fixed (int* p = arr)
{
Expand All @@ -94,8 +91,8 @@ public static int ComputeValueArrayElementOffset()
/// <returns></returns>
public static int ComputeObjectArrayElementOffset()
{
var obj = new object[4];
var method = new DynamicMethod("ComputeObjectArrayElementOffsetHelper", typeof(int), new[] { typeof(object[]) }, typeof(string).Module, true);
object[] obj = new object[4];
DynamicMethod method = new("ComputeObjectArrayElementOffsetHelper", typeof(int), new[] { typeof(object[]) }, typeof(string).Module, true);
var il = method.GetILGenerator();
var local = il.DeclareLocal(typeof(object[]), true);
il.Emit(OpCodes.Ldarg_0);
Expand All @@ -109,7 +106,7 @@ public static int ComputeObjectArrayElementOffset()
il.Emit(OpCodes.Sub);
il.Emit(OpCodes.Conv_I4);
il.Emit(OpCodes.Ret);
var del = (Func<object[], int>)method.CreateDelegate(typeof(Func<object[], int>));
Func<object[], int> del = (Func<object[], int>)method.CreateDelegate(typeof(Func<object[], int>));
return del(obj);
}

Expand All @@ -124,8 +121,8 @@ public static int ComputeFieldOffset(FieldInfo fi)
throw new NotImplementedException("Only supported for class fields right now");
}

var obj = FormatterServices.GetUninitializedObject(fi.DeclaringType);
var method = new DynamicMethod("ComputeFieldOffsetHelper", typeof(int), new[] { typeof(object) }, typeof(string).Module, true);
object obj = FormatterServices.GetUninitializedObject(fi.DeclaringType);
DynamicMethod method = new("ComputeFieldOffsetHelper", typeof(int), new[] { typeof(object) }, typeof(string).Module, true);
var il = method.GetILGenerator();
var local = il.DeclareLocal(fi.DeclaringType, true);
il.Emit(OpCodes.Ldarg_0);
Expand All @@ -138,7 +135,7 @@ public static int ComputeFieldOffset(FieldInfo fi)
il.Emit(OpCodes.Sub);
il.Emit(OpCodes.Conv_I4);
il.Emit(OpCodes.Ret);
var del = (Func<object, int>)method.CreateDelegate(typeof(Func<object, int>));
Func<object, int> del = (Func<object, int>)method.CreateDelegate(typeof(Func<object, int>));
return del(obj);
}
}
Expand Down
Loading