More warnings (#41)

* More warnings

* Code warnings fixes and setup

* more warning fixes

* Fixed all warnings

* format

* Fixed new warnings

* registration

---------

Co-authored-by: Adam Hathcock <[email protected]>

.editorconfig

@@ -96,7 +96,7 @@ csharp_style_pattern_matching_over_as_with_null_check = true:suggestion
 csharp_style_inlined_variable_declaration = true:suggestion
 csharp_style_throw_expression = true:suggestion
 csharp_style_conditional_delegate_call = true:suggestion
-csharp_style_namespace_declarations = file_scoped:silent
+csharp_style_namespace_declarations = file_scoped
 
 # Newline settings
 csharp_new_line_before_open_brace = all
@@ -134,7 +134,7 @@ csharp_preserve_single_line_blocks = true
 # Non-private fields are PascalCase
 dotnet_naming_rule.non_private_readonly_fields_should_be_pascal_case.severity = warning
 dotnet_naming_rule.non_private_readonly_fields_should_be_pascal_case.symbols = non_private_readonly_fields
-dotnet_naming_rule.non_private_readonly_fields_should_be_pascal_case.style = pascal_case_style
+dotnet_naming_rule.non_private_readonly_fields_should_be_pascal_case.style = non_private_readonly_field_style
 
 dotnet_naming_symbols.non_private_readonly_fields.applicable_kinds = field
 dotnet_naming_symbols.non_private_readonly_fields.applicable_accessibilities = public, protected, internal, protected_internal, private_protected
@@ -186,7 +186,7 @@ dotnet_naming_style.camel_case_style.capitalization = camel_case
 # Local functions are PascalCase
 dotnet_naming_rule.local_functions_should_be_pascal_case.severity = warning
 dotnet_naming_rule.local_functions_should_be_pascal_case.symbols = local_functions
-dotnet_naming_rule.local_functions_should_be_pascal_case.style = pascal_case_style
+dotnet_naming_rule.local_functions_should_be_pascal_case.style = local_function_style
 
 dotnet_naming_symbols.local_functions.applicable_kinds = local_function
 
@@ -212,6 +212,7 @@ dotnet_diagnostic.ide0058.severity = none # Remove unnecessary expression value:
 dotnet_diagnostic.ide0010.severity = none # Add missing cases to switch statement: Too verbose
 dotnet_diagnostic.ide0200.severity = none # Remove unnecessary lambda expression: may be performance reasons not to
 dotnet_diagnostic.ide0058.severity = none # Remove unnecessary expression value: Subjective
+dotnet_diagnostic.ide0305.severity = none # Use collection expression for fluent: Can obfuscate intent
 dotnet_diagnostic.ide0001.severity = suggestion # Name can be simplified: Non enforceable in build
 dotnet_diagnostic.ide0046.severity = suggestion # Use conditional expression for return: Subjective
 dotnet_diagnostic.ide0045.severity = suggestion # Use conditional expression for assignment: Subjective
@@ -233,9 +234,11 @@ dotnet_diagnostic.ide0042.severity = suggestion # Deconstruct variable declarati
 dotnet_diagnostic.ide0028.severity = suggestion # Use collection initializers: Subjective
 dotnet_diagnostic.ide0072.severity = suggestion # Populate switch statement: Subjective
 dotnet_diagnostic.ide0074.severity = suggestion # Use compound assignment: Subjective
-dotnet_diagnostic.ide0300.severity = suggestion # collection can be simplied
-dotnet_diagnostic.ide0005.severity = error # usings
-dotnet_diagnostic.ide0290.severity = suggestion # primary constructors
+dotnet_diagnostic.ide0300.severity = suggestion # Use collection expression for array: Subjective, maybe aspirational
+dotnet_diagnostic.ide0290.severity = suggestion # primary constructors: subjective, and readonly properties are not a thing
+dotnet_diagnostic.ide0290.severity = suggestion # Use primary constructor: Subjective
+dotnet_diagnostic.ide0037.severity = suggestion # Use inferred member names: Sometimes its nice to be explicit
+dotnet_diagnostic.ide0301.severity = suggestion # Use collection expression for empty: Subjective, intent
 
 # Maintainability rules
 dotnet_diagnostic.ca1501.severity = warning # Avoid excessive inheritance
@@ -305,17 +308,8 @@ dotnet_diagnostic.NUnit2036.severity = warning # Consider using Assert.That(coll
 dotnet_diagnostic.NUnit2037.severity = warning # Consider using Assert.That(collection, Does.Contain(instance)) instead of Assert.Contains(instance, collection)
 dotnet_diagnostic.NUnit2038.severity = warning # Consider using Assert.That(actual, Is.InstanceOf(expected)) instead of Assert.IsInstanceOf(expected, actual)
 dotnet_diagnostic.NUnit2039.severity = warning # Consider using Assert.That(actual, Is.Not.InstanceOf(expected)) instead of Assert.IsNotInstanceOf(expected, actual)
-csharp_prefer_simple_using_statement = true:suggestion
-csharp_prefer_braces = true:silent
-csharp_style_prefer_method_group_conversion = true:silent
-csharp_style_prefer_top_level_statements = true:silent
-csharp_style_prefer_primary_constructors = true:suggestion
-csharp_indent_labels = one_less_than_current
-csharp_style_expression_bodied_lambdas = true:silent
 
 [*.{appxmanifest,asax,ascx,aspx,axaml,build,c,c++,cc,cginc,compute,cp,cpp,cs,cshtml,cu,cuh,cxx,dtd,fs,fsi,fsscript,fsx,fx,fxh,h,hh,hlsl,hlsli,hlslinc,hpp,hxx,inc,inl,ino,ipp,ixx,master,ml,mli,mpp,mq4,mq5,mqh,nuspec,paml,razor,resw,resx,shader,skin,tpp,usf,ush,vb,xaml,xamlx,xoml,xsd}]
 indent_style = space
 indent_size = 2
 tab_width = 2
-dotnet_style_operator_placement_when_wrapping = beginning_of_line
-end_of_line = crlf

Build/Consts.cs

@@ -6,35 +6,16 @@ public static class Consts
 
   public static readonly InstallerProject[] InstallerManifests =
   {
-    new("arcgis", new InstallerAsset[] { new("Connectors/ArcGIS/Speckle.Connectors.ArcGIS3", "net6.0-windows") }),
-    new("rhino", new InstallerAsset[] { new("Connectors/Rhino/Speckle.Connectors.Rhino7", "net48") }),
-    new("revit", new InstallerAsset[] { new("Connectors/Revit/Speckle.Connectors.Revit2023", "net48") }),
-    new("autocad", new InstallerAsset[] { new("Connectors/Autocad/Speckle.Connectors.Autocad2023", "net48") })
+    new("arcgis", [new("Connectors/ArcGIS/Speckle.Connectors.ArcGIS3", "net6.0-windows")]),
+    new("rhino", [new("Connectors/Rhino/Speckle.Connectors.Rhino7", "net48")]),
+    new("revit", [new("Connectors/Revit/Speckle.Connectors.Revit2023", "net48")]),
+    new("autocad", [new("Connectors/Autocad/Speckle.Connectors.Autocad2023", "net48")])
   };
 }
 
-public readonly struct InstallerProject
+public readonly record struct InstallerProject(string HostAppSlug, IReadOnlyList<InstallerAsset> Projects)
 {
-  public string HostAppSlug { get; init; }
-  public IReadOnlyList<InstallerAsset> Projects { get; init; }
-
-  public InstallerProject(string hostAppSlug, IReadOnlyList<InstallerAsset> projects)
-  {
-    HostAppSlug = hostAppSlug;
-    Projects = projects;
-  }
-
   public override string ToString() => $"{HostAppSlug}";
 }
 
-public readonly struct InstallerAsset
-{
-  public InstallerAsset(string projectPath, string targetName)
-  {
-    ProjectPath = projectPath;
-    TargetName = targetName;
-  }
-
-  public string ProjectPath { get; init; }
-  public string TargetName { get; init; }
-}
+public readonly record struct InstallerAsset(string ProjectPath, string TargetName);

Build/Github.cs

@@ -1,9 +1,6 @@
-using System;
-using System.Net.Http;
-using System.Net.Http.Headers;
+using System.Net.Http.Headers;
 using System.Net.Mime;
 using System.Text.Json;
-using System.Threading.Tasks;
 
 namespace Build;
 

CodeMetricsConfig.txt

@@ -0,0 +1,4 @@
+CA1502: 25
+CA1501: 5
+CA1506(Method): 50
+CA1506(Type): 95

Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Bindings/ArcGISReceiveBinding.cs

@@ -3,7 +3,6 @@
 using Speckle.Connectors.DUI.Bridge;
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.DUI.Models.Card;
-using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Cancellation;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Core.Transports;

Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Send/ArcGISRootObjectBuilder.cs

@@ -2,13 +2,11 @@
 using ArcGIS.Core.Geometry;
 using ArcGIS.Desktop.Mapping;
 using Objects.GIS;
-using Speckle.Autofac.DependencyInjection;
 using Speckle.Connectors.Utils.Builders;
 using Speckle.Connectors.Utils.Caching;
 using Speckle.Connectors.Utils.Conversion;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Converters.ArcGIS3;
-using Speckle.Converters.ArcGIS3.Utils;
 using Speckle.Converters.Common;
 using Speckle.Core.Logging;
 using Speckle.Core.Models;

Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Utils/ArcGisDocumentStore.cs

@@ -37,7 +37,7 @@ ITopLevelExceptionHandler topLevelExceptionHandler
     );
 
     // in case plugin was loaded into already opened Map, read metadata from the current Map
-    if (IsDocumentInit == false && MapView.Active != null)
+    if (!IsDocumentInit && MapView.Active != null)
     {
       IsDocumentInit = true;
       ReadFromFile();

Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadReceiveBinding.cs

@@ -4,7 +4,6 @@
 using Speckle.Connectors.DUI.Bridge;
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.DUI.Models.Card;
-using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Cancellation;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Core.Transports;

Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs

@@ -73,7 +73,9 @@ CancellationToken cancellationToken
     // POC: claire doesn't like this - it's confusing to have block definitions in the same instanceComponents list as block instances since they don't have layers
     if (instanceDefinitionProxies != null && instanceDefinitionProxies.Count > 0)
     {
-      var transformed = instanceDefinitionProxies.Select(proxy => (new Collection[] { }, proxy as IInstanceComponent));
+      var transformed = instanceDefinitionProxies.Select(proxy =>
+        (Array.Empty<Collection>(), proxy as IInstanceComponent)
+      );
       instanceComponents.AddRange(transformed);
     }
 
@@ -85,7 +87,7 @@ CancellationToken cancellationToken
 
       if (tc.Current is IInstanceComponent instanceComponent)
       {
-        instanceComponents.Add((new Collection[] { layerCollection }, instanceComponent));
+        instanceComponents.Add(([layerCollection], instanceComponent));
       }
       else
       {

Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/RevitReceiveBinding.cs

@@ -12,7 +12,7 @@
 
 namespace Speckle.Connectors.Revit.Bindings;
 
-internal class RevitReceiveBinding : IReceiveBinding
+internal sealed class RevitReceiveBinding : IReceiveBinding
 {
   public string Name => "receiveBinding";
   public IBridge Parent { get; }

Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/SelectionBinding.cs

@@ -4,13 +4,16 @@
 using Speckle.Connectors.Revit.Plugin;
 using Speckle.Converters.Common;
 using Speckle.Converters.RevitShared.Helpers;
-using Timer = System.Timers.Timer;
 
 namespace Speckle.Connectors.Revit.Bindings;
 
 // POC: we need a base a RevitBaseBinding
-internal sealed class SelectionBinding : RevitBaseBinding, ISelectionBinding
+internal sealed class SelectionBinding : RevitBaseBinding, ISelectionBinding, IDisposable
 {
+#if REVIT2022
+  private readonly System.Timers.Timer _selectionTimer;
+#endif
+
   public SelectionBinding(
     RevitContext revitContext,
     DocumentModelStore store,
@@ -25,9 +28,9 @@ ITopLevelExceptionHandler topLevelExceptionHandler
       revitIdleManager.SubscribeToIdle(nameof(SelectionBinding), OnSelectionChanged);
 #else
     // NOTE: getting the selection data should be a fast function all, even for '000s of elements - and having a timer hitting it every 1s is ok.
-    var timer = new Timer(1000);
-    timer.Elapsed += (_, _) => topLevelExceptionHandler.CatchUnhandled(OnSelectionChanged);
-    timer.Start();
+    _selectionTimer = new System.Timers.Timer(1000);
+    _selectionTimer.Elapsed += (_, _) => topLevelExceptionHandler.CatchUnhandled(OnSelectionChanged);
+    _selectionTimer.Start();
 #endif
   }
 
@@ -44,7 +47,7 @@ public SelectionInfo GetSelection()
   {
     if (RevitContext.UIApplication == null || RevitContext.UIApplication.ActiveUIDocument == null)
     {
-      return new SelectionInfo(new List<string>(), "No objects selected.");
+      return new SelectionInfo(Array.Empty<string>(), "No objects selected.");
     }
 
     // POC: this was also being called on shutdown
@@ -56,4 +59,11 @@ public SelectionInfo GetSelection()
       .ToList();
     return new SelectionInfo(selectionIds, $"{selectionIds.Count} objects selected.");
   }
+
+  public void Dispose()
+  {
+#if REVIT2022
+    _selectionTimer.Dispose();
+#endif
+  }
 }

Connectors/Revit/Speckle.Connectors.RevitShared/ElementIdHelper.cs

@@ -7,8 +7,11 @@ public static class ElementIdHelper
 {
   public static ElementId Parse(string idStr)
   {
-    if (int.TryParse(idStr, out var result))
-      return new ElementId(result);
-    throw new SpeckleConversionException($"Cannot parse ElementId: {idStr}");
+    if (!int.TryParse(idStr, out var result))
+    {
+      throw new SpeckleConversionException($"Cannot parse ElementId: {idStr}");
+    }
+
+    return new ElementId(result);
   }
 }

Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitContextAccessor.cs

@@ -3,7 +3,7 @@
 
 namespace Speckle.Connectors.Revit.Operations.Receive;
 
-internal class RevitContextAccessor : ISyncToThread
+internal sealed class RevitContextAccessor : ISyncToThread
 {
   public Task<T> RunOnThread<T>(Func<T> func) => RevitTask.RunAsync(func);
 }

Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs

@@ -14,7 +14,7 @@ namespace Speckle.Connectors.Revit.Operations.Receive;
 /// Potentially consolidate all application specific IHostObjectBuilders
 /// https://spockle.atlassian.net/browse/DUI3-465
 /// </summary>
-internal class RevitHostObjectBuilder : IHostObjectBuilder, IDisposable
+internal sealed class RevitHostObjectBuilder : IHostObjectBuilder, IDisposable
 {
   private readonly IRootToHostConverter _converter;
   private readonly IRevitConversionContextStack _contextStack;

Connectors/Revit/Speckle.Connectors.RevitShared/Plugin/RevitSettings.cs

@@ -1,6 +1,6 @@
 namespace Speckle.Connectors.Revit.Plugin;
 
-internal record RevitSettings( // POC: need to derive some interface for things that require IHostSettings{
+internal sealed record RevitSettings( // POC: need to derive some interface for things that require IHostSettings{
   string RevitPanelName,
   string RevitTabName,
   string RevitTabTitle,

Connectors/Rhino/Speckle.Connectors.Rhino7/Bindings/RhinoReceiveBinding.cs

@@ -4,7 +4,6 @@
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.DUI.Models.Card;
 using Speckle.Connectors.Rhino7.HostApp;
-using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Builders;
 using Speckle.Connectors.Utils.Cancellation;
 using Speckle.Connectors.Utils.Operations;

Connectors/Rhino/Speckle.Connectors.Rhino7/Bindings/RhinoSelectionBinding.cs

@@ -24,7 +24,7 @@ public RhinoSelectionBinding(IRhinoIdleManager idleManager, IBridge parent)
     RhinoDoc.DeselectAllObjects += OnSelectionChange;
   }
 
-  void OnSelectionChange(object o, EventArgs eventArgs) =>
+  private void OnSelectionChange(object o, EventArgs eventArgs) =>
     _idleManager.SubscribeToIdle(nameof(RhinoSelectionBinding), UpdateSelection);
 
   private void UpdateSelection()

Connectors/Rhino/Speckle.Connectors.Rhino7/Bindings/RhinoSendBinding.cs

@@ -8,7 +8,6 @@
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.DUI.Models.Card;
 using Speckle.Connectors.DUI.Models.Card.SendFilter;
-using Speckle.Connectors.DUI.Settings;
 using Speckle.Connectors.Rhino7.HostApp;
 using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Caching;

Connectors/Rhino/Speckle.Connectors.Rhino7/Operations/Receive/RhinoHostObjectBuilder.cs

@@ -98,7 +98,9 @@ private HostObjectBuilderResult BakeObjects(
     // POC: these are not captured by traversal, so we need to re-add them here
     if (instanceDefinitionProxies != null && instanceDefinitionProxies.Count > 0)
     {
-      var transformed = instanceDefinitionProxies.Select(proxy => (new Collection[] { }, proxy as IInstanceComponent));
+      var transformed = instanceDefinitionProxies.Select(proxy =>
+        (Array.Empty<Collection>(), proxy as IInstanceComponent)
+      );
       instanceComponents.AddRange(transformed);
     }
 

Connectors/Rhino/Speckle.Connectors.Rhino7/Operations/Send/RhinoRootObjectBuilder.cs

@@ -1,6 +1,5 @@
 using Rhino;
 using Rhino.DocObjects;
-using Speckle.Autofac.DependencyInjection;
 using Speckle.Connectors.DUI.Models.Card.SendFilter;
 using Speckle.Connectors.Rhino7.HostApp;
 using Speckle.Connectors.Utils.Builders;

Converters/ArcGIS/Speckle.Converters.ArcGIS3/ToSpeckle/TopLevel/PointcloudLayerToSpeckleConverter.cs

@@ -73,9 +73,8 @@ public Base Convert(object target)
 
   public SGIS.VectorLayer Convert(LasDatasetLayer target)
   {
-    SGIS.VectorLayer speckleLayer = new();
-    speckleLayer.nativeGeomType = target.MapLayerType.ToString();
-    speckleLayer.geomType = GISLayerGeometryType.POINTCLOUD;
+    SGIS.VectorLayer speckleLayer =
+      new() { nativeGeomType = target.MapLayerType.ToString(), geomType = GISLayerGeometryType.POINTCLOUD };
 
     // prepare data for pointcloud
     List<SOG.Point> specklePts = new();

Converters/ArcGIS/Speckle.Converters.ArcGIS3/ToSpeckle/TopLevel/VectorLayerToSpeckleConverter.cs

@@ -1,4 +1,3 @@
-using ArcGIS.Core.CIM;
 using ArcGIS.Core.Data;
 using ArcGIS.Core.Geometry;
 using ArcGIS.Desktop.Mapping;

Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/ArcGISFieldUtils.cs

@@ -260,7 +260,7 @@ List<string> fieldAdded
         // 2. change to NewType if it's String (and the old one is not)
         if (
           newFieldType != FieldType.Blob && existingFieldType == FieldType.Blob
-          || (newFieldType == FieldType.String && existingFieldType != FieldType.String)
+          || newFieldType == FieldType.String && existingFieldType != FieldType.String
         )
         {
           fieldsAndFunctions[index] = (

Converters/ArcGIS/Speckle.Converters.ArcGIS3/Utils/CRSoffsetRotation.cs

@@ -141,9 +141,9 @@ public CRSoffsetRotation(Map map)
 
     if (
       textData != null
-      && textData.ToLower().Contains("_specklexoffset=")
-      && textData.ToLower().Contains("_speckleyoffset=")
-      && textData.ToLower().Contains("_specklenorth=")
+      && textData.Contains("_specklexoffset=", StringComparison.CurrentCultureIgnoreCase)
+      && textData.Contains("_speckleyoffset=", StringComparison.CurrentCultureIgnoreCase)
+      && textData.Contains("_specklenorth=", StringComparison.CurrentCultureIgnoreCase)
     )
     {
       string? latElement = textData.ToLower().Split("_speckleyoffset=")[^1].Split("_")[0];