Re-introduced code analysers and fixed many violations (#92)

* Sdk

* Objects

* Supressed IDE warnings via editor config instead of nowarn

* Nullability and other warnings

* using

* Fixed warnings

* Important fix

* More fixes

.editorconfig

@@ -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,6 +234,17 @@ 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 # 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
+dotnet_diagnostic.ide0021.severity = suggestion # Use expression body for constructors : Subjective
+dotnet_diagnostic.ide0090.severity = suggestion # Simplify new expression : Subjective
+
+dotnet_diagnostic.ide0047.severity = suggestion # Parentheses preferences: IDEs don't properly pick it up
+dotnet_diagnostic.ide0130.severity = suggestion # Namespace does not match folder structure : Aspirational
+dotnet_diagnostic.ide1006.severity = suggestion # Naming rule violation : Aspirational
 
 # Maintainability rules
 dotnet_diagnostic.ca1501.severity = warning # Avoid excessive inheritance

CodeMetricsConfig.txt

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

Directory.Build.props

@@ -6,17 +6,9 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <LangVersion>latest</LangVersion>
+    <LangVersion>12</LangVersion>
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>
-    <AnalysisMode>Recommended</AnalysisMode>
-    <WarningsAsErrors>true</WarningsAsErrors>
-    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
-    <CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors>
-    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
-    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <RunAnalyzersDuringLiveAnalysis>False</RunAnalyzersDuringLiveAnalysis>
-    <RunAnalyzersDuringBuild>False</RunAnalyzersDuringBuild>
     <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
     <RestorePackagesWithLockFile>true</RestorePackagesWithLockFile>
     <IsPackable>false</IsPackable>  
@@ -25,6 +17,36 @@
   <PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">
     <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
   </PropertyGroup>
+
+  <PropertyGroup Label="Analyers">
+    <EnableNetAnalyzers>true</EnableNetAnalyzers>
+    <AnalysisLevel>latest-AllEnabledByDefault</AnalysisLevel>
+    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
+    <AllowUnsafeBlocks>false</AllowUnsafeBlocks>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+
+    <!-- Ingored warnings, some aspirational but too noisy for now, some by design.  -->
+    <NoWarn>
+      <!--Disabled by design-->
+      CA5399;CA1812;
+      <!--XML comment-->
+      CS1591;CS1573;
+      <!-- Globalization rules -->
+      CA1303;CA1304;CA1305;CA1307;CA1308;CA1309;CA1310;CA1311;
+      <!-- Logging -->
+      CA1848;CA2254;CA1727;
+      <!-- Others we don't want -->
+      CA1815;CA1725;
+      <!-- Naming things is hard enough -->
+      CA1710;CA1711;CA1724;CA1716;CA1720;CA1724;
+      <!-- Aspirational -->
+      CA1502;
+      $(NoWarn)
+    </NoWarn>
+  </PropertyGroup>
+
+
 
   <PropertyGroup>
     <EmbedUntrackedSources>true</EmbedUntrackedSources>
@@ -39,8 +61,6 @@
     <PublishRepositoryUrl>true</PublishRepositoryUrl>
     <PackageReadmeFile>README.md</PackageReadmeFile>
     <PackageLicenseExpression>Apache-2.0</PackageLicenseExpression>
-    <GenerateDocumentationFile>true</GenerateDocumentationFile>
-    <NoWarn>$(NoWarn);1591;1573</NoWarn>
   </PropertyGroup>  
   <ItemGroup>
     <None Include="..\..\README.md" Pack="true" PackagePath="\"/>
@@ -51,4 +71,11 @@
       PackagePath="\"
       Visible="false"/>
   </ItemGroup>
+
+  <ItemGroup>
+    <!-- This file contains the configuration for some analyzer warnings, such as cyclomatic
+complexity threshold -->
+    <AdditionalFiles Include="$(RepositoryRoot)CodeMetricsConfig.txt"/>
+  </ItemGroup>
+
 </Project>

Directory.Build.targets

@@ -0,0 +1,14 @@
+<Project>
+    <PropertyGroup Condition="'$(IsTestProject)' == 'true'">
+        <AnalysisMode>Recommended</AnalysisMode>
+        <NoWarn>
+            $(NoWarn);
+            <!-- Things we need to test -->
+            CS0618;CA1034;CA2201;CA1051;CA1040;CA1724;
+            IDE0044;IDE0130;CA1508;
+            <!-- Analysers that provide no tangeable value to a test project -->
+            CA5394;CA2007;CA1852;CA1819;CA1711;CA1063;CA1816;CA2234;CS8618;CA1054;CA1810;CA2208;CA1019;
+        </NoWarn>
+        <EnforceCodeStyleInBuild>false</EnforceCodeStyleInBuild>
+    </PropertyGroup>
+</Project>

Speckle.Sdk.sln

@@ -22,6 +22,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "config", "config", "{DA2AED
 		README.md = README.md
 		GitVersion.yml = GitVersion.yml
 		docker-compose.yml = docker-compose.yml
+		CodeMetricsConfig.txt = CodeMetricsConfig.txt
+		Directory.Build.Targets = Directory.Build.Targets
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{58D37DA9-F948-48CA-9A73-F5BBBD533DBF}"

build/Program.cs

@@ -65,7 +65,7 @@ void RemoveDirectory(string d)
   DependsOn(RESTORE),
   async () =>
   {
-    await RunAsync("dotnet", $"build Speckle.Sdk.sln -c Release --no-restore");
+    await RunAsync("dotnet", $"build Speckle.Sdk.sln -c Release --no-restore").ConfigureAwait(false);
   }
 );
 
@@ -76,9 +76,11 @@ void RemoveDirectory(string d)
   async file =>
   {
     await RunAsync(
-      "dotnet",
-      $"test {file} -c Release --no-build --no-restore --verbosity=normal  /p:AltCover=true  /p:AltCoverAttributeFilter=ExcludeFromCodeCoverage /p:AltCoverVerbosity=Warning"
-    );
+        "dotnet",
+        $"test {file} -c Release --no-build --no-restore --verbosity=normal  /p:AltCover=true  /p:AltCoverAttributeFilter=ExcludeFromCodeCoverage /p:AltCoverVerbosity=Warning"
+      )
+      .ConfigureAwait(false);
+    ;
   }
 );
 
@@ -87,15 +89,18 @@ await RunAsync(
   DependsOn(BUILD),
   async () =>
   {
-    await RunAsync("docker", "compose -f docker-compose.yml up --wait");
+    await RunAsync("docker", "compose -f docker-compose.yml up --wait").ConfigureAwait(false);
+    ;
     foreach (var test in Glob.Files(".", "**/*.Tests.Integration.csproj"))
     {
       await RunAsync(
-        "dotnet",
-        $"test {test} -c Release --no-build --no-restore --verbosity=normal  /p:AltCover=true  /p:AltCoverAttributeFilter=ExcludeFromCodeCoverage"
-      );
+          "dotnet",
+          $"test {test} -c Release --no-build --no-restore --verbosity=normal  /p:AltCover=true  /p:AltCoverAttributeFilter=ExcludeFromCodeCoverage"
+        )
+        .ConfigureAwait(false);
+      ;
     }
-    await RunAsync("docker", "compose down");
+    await RunAsync("docker", "compose down").ConfigureAwait(false);
   }
 );
 

src/Speckle.Objects/BuiltElements/Rebar.cs

@@ -9,11 +9,8 @@ namespace Speckle.Objects.BuiltElements;
 /// <remarks>
 /// This class is not suitable for freeform rebar, which can have multiple shapes.
 /// </remarks>
-public abstract class RebarGroup<T> : Base, IHasVolume, IDisplayValue<List<ICurve>>
-  where T : RebarShape
+public abstract class RebarGroup : Base, IHasVolume, IDisplayValue<List<ICurve>>
 {
-  public RebarGroup() { }
-
   /// <summary>
   /// The shape of the rebar group
   /// </summary>

src/Speckle.Objects/BuiltElements/Revit/FreeformElement.cs

@@ -24,7 +24,7 @@ public FreeformElement(List<Base> baseGeometries, string subcategory = "", List<
     this.subcategory = subcategory;
     if (!IsValid())
     {
-      throw new Exception("Freeform elements can only be created from BREPs or Meshes");
+      throw new ArgumentException("Freeform elements can only be created from BREPs or Meshes", nameof(baseGeometries));
     }
 
     this.parameters = parameters?.ToBase();

src/Speckle.Objects/BuiltElements/Revit/RevitOpening.cs

@@ -2,7 +2,6 @@
 using Speckle.Objects.Utils;
 using Speckle.Sdk;
 using Speckle.Sdk.Host;
-using Speckle.Sdk.Logging;
 using Speckle.Sdk.Models;
 
 namespace Speckle.Objects.BuiltElements.Revit;

src/Speckle.Objects/BuiltElements/Revit/RevitRebar.cs

@@ -5,7 +5,7 @@
 namespace Speckle.Objects.BuiltElements.Revit;
 
 [SpeckleType("Objects.BuiltElements.Revit.RevitRebarGroup")]
-public class RevitRebarGroup : RebarGroup<RevitRebarShape>
+public class RevitRebarGroup : RebarGroup
 {
   public RevitRebarGroup() { }
 

src/Speckle.Objects/BuiltElements/Revit/RevitWall.cs

@@ -147,13 +147,14 @@ public RevitFaceWall(
   {
     if (surface.Surfaces.Count == 0)
     {
-      throw new Exception("Cannot create a RevitWall with an empty BREP");
+      throw new ArgumentException("Cannot create a RevitWall with an empty BREP", nameof(surface));
     }
 
     if (surface.Surfaces.Count > 1)
     {
-      throw new Exception(
-        "The provided brep has more than 1 surface. Please deconstruct/explode it to create multiple instances"
+      throw new ArgumentException(
+        "The provided brep has more than 1 surface. Please deconstruct/explode it to create multiple instances",
+        nameof(surface)
       );
     }
 

src/Speckle.Objects/EncodingOptimisations.cs

@@ -18,7 +18,7 @@ public static class CurveTypeEncoding
 /// </summary>
 public static class CurveArrayEncodingExtensions
 {
-  public static List<double> ToArray(List<ICurve> curves)
+  public static List<double> ToArray(IReadOnlyCollection<ICurve> curves)
   {
     var list = new List<double>();
     foreach (var curve in curves)
@@ -47,7 +47,7 @@ public static List<double> ToArray(List<ICurve> curves)
           list.AddRange(p.ToList());
           break;
         default:
-          throw new Exception($"Unkown curve type: {curve.GetType()}.");
+          throw new ArgumentOutOfRangeException(nameof(curves), $"Unkown curve type: {curve.GetType()}.");
       }
     }
 

src/Speckle.Objects/GIS/GisPointFeature.cs

@@ -12,8 +12,8 @@ public class GisPointFeature : Base, IGisFeature, IDisplayValue<List<Point>>
   [JsonIgnore]
   public required List<Point> geometry
   {
-    get { return displayValue; }
-    set { displayValue = value; }
+    get => displayValue;
+    set => displayValue = value;
   }
 
   [DetachProperty]

src/Speckle.Objects/GIS/GisPolylineFeature.cs

@@ -12,8 +12,8 @@ public class GisPolylineFeature : Base, IGisFeature, IDisplayValue<List<Polyline
   [JsonIgnore]
   public required List<Polyline> geometry
   {
-    get { return displayValue; }
-    set { displayValue = value; }
+    get => displayValue;
+    set => displayValue = value;
   }
 
   [DetachProperty]

src/Speckle.Objects/Geometry/Arc.cs

@@ -2,7 +2,6 @@
 using Speckle.Objects.Primitive;
 using Speckle.Sdk;
 using Speckle.Sdk.Common;
-using Speckle.Sdk.Logging;
 using Speckle.Sdk.Models;
 
 namespace Speckle.Objects.Geometry;
@@ -293,7 +292,7 @@ public static Arc FromList(List<double> list)
       endAngle = list[4],
       angleRadians = list[5],
       domain = new Interval { start = list[6], end = list[7] },
-      units = Units.GetUnitFromEncoding(list[list.Count - 1]),
+      units = Units.GetUnitFromEncoding(list[^1]),
       plane = Plane.FromList(list.GetRange(8, 13))
     };
     arc.startPoint = Point.FromList(list.GetRange(21, 3), arc.units);

src/Speckle.Objects/Geometry/Circle.cs

@@ -74,7 +74,7 @@ public static Circle FromList(List<double> list)
       radius = list[2],
       domain = new Interval { start = list[3], end = list[4] },
       plane = Plane.FromList(list.GetRange(5, 13)),
-      units = Units.GetUnitFromEncoding(list[list.Count - 1])
+      units = Units.GetUnitFromEncoding(list[^1])
     };
 
     return circle;

src/Speckle.Objects/Geometry/Ellipse.cs

@@ -69,7 +69,7 @@ public static Ellipse FromList(List<double> list)
       secondRadius = list[3],
       domain = new Interval { start = list[4], end = list[5] },
       plane = Plane.FromList(list.GetRange(6, 13)),
-      units = Units.GetUnitFromEncoding(list[list.Count - 1])
+      units = Units.GetUnitFromEncoding(list[^1])
     };
     return ellipse;
   }

src/Speckle.Objects/Geometry/Line.cs

@@ -3,7 +3,6 @@
 using Speckle.Objects.Primitive;
 using Speckle.Sdk;
 using Speckle.Sdk.Common;
-using Speckle.Sdk.Logging;
 using Speckle.Sdk.Models;
 
 namespace Speckle.Objects.Geometry;
@@ -121,9 +120,9 @@ public List<double> ToList()
     return list;
   }
 
-  public static Line FromList(List<double> list)
+  public static Line FromList(IReadOnlyList<double> list)
   {
-    var units = Units.GetUnitFromEncoding(list[list.Count - 1]);
+    var units = Units.GetUnitFromEncoding(list[^1]);
     var startPt = new Point(list[2], list[3], list[4], units);
     var endPt = new Point(list[5], list[6], list[7], units);
     var line = new Line(startPt, endPt, units)

src/Speckle.Objects/Geometry/Mesh.cs

@@ -2,7 +2,6 @@
 using Speckle.Objects.Other;
 using Speckle.Sdk;
 using Speckle.Sdk.Common;
-using Speckle.Sdk.Logging;
 using Speckle.Sdk.Models;
 
 namespace Speckle.Objects.Geometry;

src/Speckle.Objects/Geometry/Plane.cs

@@ -87,9 +87,9 @@ public List<double> ToList()
   /// </summary>
   /// <param name="list">The list of values representing this plane</param>
   /// <returns>A new <see cref="Plane"/> with the provided values.</returns>
-  public static Plane FromList(List<double> list)
+  public static Plane FromList(IReadOnlyList<double> list)
   {
-    var units = Units.GetUnitFromEncoding(list[list.Count - 1]);
+    var units = Units.GetUnitFromEncoding(list[^1]);
     var plane = new Plane
     {
       origin = new Point(list[0], list[1], list[2], units),

src/Speckle.Objects/Geometry/Point.cs

@@ -216,14 +216,14 @@ public double DistanceTo(Point point)
     return Math.Sqrt(Math.Pow(x - point.x, 2) + Math.Pow(y - point.y, 2) + Math.Pow(z - point.z, 2));
   }
 
-  public override bool Equals(object obj)
+  public override bool Equals(object? obj)
   {
     if (ReferenceEquals(this, obj))
     {
       return true;
     }
 
-    if (ReferenceEquals(obj, null))
+    if (obj is null)
     {
       return false;
     }

src/Speckle.Objects/Geometry/Pointcloud.cs

@@ -2,7 +2,6 @@
 using Speckle.Sdk;
 using Speckle.Sdk.Common;
 using Speckle.Sdk.Host;
-using Speckle.Sdk.Logging;
 using Speckle.Sdk.Models;
 
 namespace Speckle.Objects.Geometry;

src/Speckle.Objects/Geometry/Surface.cs

@@ -250,7 +250,7 @@ public static Surface FromList(List<double> list)
     srf.knotsU = list.GetRange(14 + pointCount, knotsUCount);
     srf.knotsV = list.GetRange(14 + pointCount + knotsUCount, knotsVCount);
 
-    var u = list[list.Count - 1];
+    var u = list[^1];
     srf.units = Units.GetUnitFromEncoding(u);
     return srf;
   }

src/Speckle.Objects/Other/Transform.cs

@@ -3,7 +3,6 @@
 using Speckle.Objects.Geometry;
 using Speckle.Sdk;
 using Speckle.Sdk.Common;
-using Speckle.Sdk.Logging;
 using Speckle.Sdk.Models;
 using Vector = Speckle.Objects.Geometry.Vector;