From 465d69ba008d88ec221652d4e9ef287e83982b40 Mon Sep 17 00:00:00 2001 From: Jedd Morgan <45512892+JR-Morgan@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:37:18 +0000 Subject: [PATCH 1/2] Enforce correct nullability with required --- .editorconfig | 1 - src/Speckle.Objects/Data/DataObject.cs | 2 +- .../GIS/GisMultipatchGeometry.cs | 12 +-- src/Speckle.Objects/GIS/GisPointFeature.cs | 12 +-- src/Speckle.Objects/GIS/GisPolylineFeature.cs | 12 +-- src/Speckle.Objects/GIS/PolygonGeometry.cs | 3 + src/Speckle.Objects/Geometry/Pointcloud.cs | 9 +- src/Speckle.Objects/Geometry/Surface.cs | 83 +++++++++---------- src/Speckle.Sdk/Speckle.Sdk.csproj | 1 + 9 files changed, 63 insertions(+), 72 deletions(-) diff --git a/.editorconfig b/.editorconfig index 0a293a7d..dcb0a171 100644 --- a/.editorconfig +++ b/.editorconfig @@ -256,7 +256,6 @@ dotnet_diagnostic.ca1508.severity = warning # Avoid dead conditional code dotnet_diagnostic.ca1509.severity = warning # Invalid entry in code metrics configuration file dotnet_diagnostic.ca1861.severity = none # 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) -dotnet_diagnostic.cs8618.severity = suggestion # nullable problem # Performance rules diff --git a/src/Speckle.Objects/Data/DataObject.cs b/src/Speckle.Objects/Data/DataObject.cs index baefdb03..27e75c3c 100644 --- a/src/Speckle.Objects/Data/DataObject.cs +++ b/src/Speckle.Objects/Data/DataObject.cs @@ -8,7 +8,7 @@ public class DataObject : Base, IDataObject public required string name { get; set; } [DetachProperty] - public required List displayValue { get; set; } + public required IReadOnlyList displayValue { get; set; } public required Dictionary properties { get; set; } diff --git a/src/Speckle.Objects/GIS/GisMultipatchGeometry.cs b/src/Speckle.Objects/GIS/GisMultipatchGeometry.cs index f4bc67db..9fd2684f 100644 --- a/src/Speckle.Objects/GIS/GisMultipatchGeometry.cs +++ b/src/Speckle.Objects/GIS/GisMultipatchGeometry.cs @@ -5,14 +5,8 @@ namespace Speckle.Objects.GIS; [SpeckleType("Objects.GIS.GisMultipatchGeometry")] public class GisMultipatchGeometry : Base { - public string units { get; set; } - public List faces { get; set; } - public List vertices { get; set; } + public required string units { get; set; } + public required List faces { get; set; } + public required List vertices { get; set; } public List? colors { get; set; } - - public GisMultipatchGeometry() - { - faces = new List(); - vertices = new List(); - } } diff --git a/src/Speckle.Objects/GIS/GisPointFeature.cs b/src/Speckle.Objects/GIS/GisPointFeature.cs index 57493268..a5e85319 100644 --- a/src/Speckle.Objects/GIS/GisPointFeature.cs +++ b/src/Speckle.Objects/GIS/GisPointFeature.cs @@ -10,12 +10,12 @@ public class GisPointFeature : Base, IGisFeature, IDisplayValue> public required Base attributes { get; set; } [JsonIgnore] - public required List geometry - { - get => displayValue; - set => displayValue = value; - } + public required List geometry { get; set; } [DetachProperty] - public List displayValue { get; set; } + public List displayValue + { + get => geometry; + set => geometry = value; + } } diff --git a/src/Speckle.Objects/GIS/GisPolylineFeature.cs b/src/Speckle.Objects/GIS/GisPolylineFeature.cs index 2f4b578d..fc687782 100644 --- a/src/Speckle.Objects/GIS/GisPolylineFeature.cs +++ b/src/Speckle.Objects/GIS/GisPolylineFeature.cs @@ -10,12 +10,12 @@ public class GisPolylineFeature : Base, IGisFeature, IDisplayValue geometry - { - get => displayValue; - set => displayValue = value; - } + public required List geometry { get; set; } [DetachProperty] - public List displayValue { get; set; } + public List displayValue + { + get => geometry; + set => geometry = value; + } } diff --git a/src/Speckle.Objects/GIS/PolygonGeometry.cs b/src/Speckle.Objects/GIS/PolygonGeometry.cs index 2a3a3cdb..b7e905fb 100644 --- a/src/Speckle.Objects/GIS/PolygonGeometry.cs +++ b/src/Speckle.Objects/GIS/PolygonGeometry.cs @@ -3,6 +3,9 @@ namespace Speckle.Objects.GIS; +//Supressing for now, class needs is yet to be decorated with `required` keywords +#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. + [SpeckleType("Objects.GIS.PolygonGeometry")] public class PolygonGeometry : Base { diff --git a/src/Speckle.Objects/Geometry/Pointcloud.cs b/src/Speckle.Objects/Geometry/Pointcloud.cs index cd53a018..ac76f174 100644 --- a/src/Speckle.Objects/Geometry/Pointcloud.cs +++ b/src/Speckle.Objects/Geometry/Pointcloud.cs @@ -11,16 +11,11 @@ namespace Speckle.Objects.Geometry; [SpeckleType("Objects.Geometry.Pointcloud")] public class Pointcloud : Base, IHasBoundingBox, ITransformable { - /// - /// Constructs an empty - /// - public Pointcloud() { } - /// /// Gets or sets the list of points of this , stored as a flat list of coordinates [x1,y1,z1,x2,y2,...] /// [DetachProperty, Chunkable(31250)] - public required List points { get; set; } = new(); + public required List points { get; set; } /// /// Gets or sets the list of colors of this 's points., stored as ARGB s. @@ -38,7 +33,7 @@ public Pointcloud() { } /// The unit's this is in. /// This should be one of /// - public string units { get; set; } + public required string units { get; set; } /// public Box? bbox { get; set; } diff --git a/src/Speckle.Objects/Geometry/Surface.cs b/src/Speckle.Objects/Geometry/Surface.cs index a1ece6b0..d97ed633 100644 --- a/src/Speckle.Objects/Geometry/Surface.cs +++ b/src/Speckle.Objects/Geometry/Surface.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using Speckle.Objects.Other; using Speckle.Objects.Primitive; using Speckle.Sdk.Common; @@ -11,45 +12,43 @@ namespace Speckle.Objects.Geometry; [SpeckleType("Objects.Geometry.Surface")] public class Surface : Base, IHasBoundingBox, IHasArea, ITransformable { - /// - /// Constructs a new empty - /// + [Obsolete("Constructor should only be used by serializer, use one of the other constructors instead")] public Surface() { - applicationId = null; - pointData = new List(); + pointData = []; } - /// - /// Constructs a new empty - /// - /// The units this surface is modeled in - /// This surface's unique identifier on a specific application - public Surface(string units = Units.Meters, string? applicationId = null) + public Surface(List> controlPoints) + { + SetControlPoints(controlPoints); + } + + public Surface(IList pointData, int countU, int countV) { - this.applicationId = applicationId; - this.units = units; + this.pointData = pointData; + this.countU = countU; + this.countV = countV; } /// /// The degree of the surface in the U direction /// - public int degreeU { get; set; } + public required int degreeU { get; set; } /// /// The degree of the surface in the V direction /// - public int degreeV { get; set; } + public required int degreeV { get; set; } /// /// Determines if the is rational. /// - public bool rational { get; set; } + public required bool rational { get; set; } /// - /// The raw data of the surface's control points. Use GetControlPoints or SetControlPoints instead of accessing this directly. + /// The raw data of the surface's control points. Use or instead of accessing this directly. /// - public List pointData { get; set; } + public IList pointData { get; set; } /// /// The number of control points in the U direction @@ -64,38 +63,38 @@ public Surface(string units = Units.Meters, string? applicationId = null) /// /// The knot vector in the U direction /// - public List knotsU { get; set; } + public required List knotsU { get; set; } /// /// The knot vector in the V direction /// - public List knotsV { get; set; } + public required List knotsV { get; set; } /// /// The surface's domain in the U direction /// - public Interval domainU { get; set; } + public required Interval domainU { get; set; } /// /// The surface's domain in the V direction /// - public Interval domainV { get; set; } + public required Interval domainV { get; set; } /// /// Determines if a surface is closed around the . /// - public bool closedU { get; set; } + public required bool closedU { get; set; } /// /// Determines if a surface is closed around the /// - public bool closedV { get; set; } + public required bool closedV { get; set; } /// /// The unit's this is in. /// This should be one of /// - public string units { get; set; } + public required string units { get; set; } /// public double area { get; set; } @@ -116,7 +115,7 @@ public bool TransformTo(Transform transform, out Surface transformed) } } - transformed = new Surface + transformed = new Surface(ptMatrix) { degreeU = degreeU, degreeV = degreeV, @@ -131,7 +130,6 @@ public bool TransformTo(Transform transform, out Surface transformed) knotsV = knotsV, units = units, }; - transformed.SetControlPoints(ptMatrix); return true; } @@ -172,6 +170,9 @@ public List> GetControlPoints() /// /// A 2-dimensional array of instances. /// The must be ordered following directions "[u][v]" + [MemberNotNull(nameof(pointData))] + [MemberNotNull(nameof(countU))] + [MemberNotNull(nameof(countV))] public void SetControlPoints(List> value) { List data = new(); @@ -229,29 +230,27 @@ public List ToList() /// A new with the provided values. public static Surface FromList(List list) { - var srf = new Surface + var pointCount = (int)list[11]; + var knotsUCount = (int)list[12]; + var knotsVCount = (int)list[13]; + var countU = (int)list[2]; + var countV = (int)list[3]; + + var pointData = list.GetRange(14, pointCount); + var u = list[^1]; + + return new Surface(pointData, countU, countV) { degreeU = (int)list[0], degreeV = (int)list[1], - countU = (int)list[2], - countV = (int)list[3], rational = list[4] == 1, closedU = list[5] == 1, closedV = list[6] == 1, domainU = new Interval { start = list[7], end = list[8] }, domainV = new Interval { start = list[9], end = list[10] }, + knotsU = list.GetRange(14 + pointCount, knotsUCount), + knotsV = list.GetRange(14 + pointCount + knotsUCount, knotsVCount), + units = Units.GetUnitFromEncoding(u), }; - - var pointCount = (int)list[11]; - var knotsUCount = (int)list[12]; - var knotsVCount = (int)list[13]; - - srf.pointData = list.GetRange(14, pointCount); - srf.knotsU = list.GetRange(14 + pointCount, knotsUCount); - srf.knotsV = list.GetRange(14 + pointCount + knotsUCount, knotsVCount); - - var u = list[^1]; - srf.units = Units.GetUnitFromEncoding(u); - return srf; } } diff --git a/src/Speckle.Sdk/Speckle.Sdk.csproj b/src/Speckle.Sdk/Speckle.Sdk.csproj index 620f77c0..97406570 100644 --- a/src/Speckle.Sdk/Speckle.Sdk.csproj +++ b/src/Speckle.Sdk/Speckle.Sdk.csproj @@ -11,6 +11,7 @@ Speckle.Sdk The .NET SDK for Speckle $(PackageTags) core sdk + $(NoWarn);CS8618 From 912971d8deb1695ff91c2ea0f9918ba142e4db4b Mon Sep 17 00:00:00 2001 From: Jedd Morgan <45512892+JR-Morgan@users.noreply.github.com> Date: Fri, 10 Jan 2025 18:29:57 +0000 Subject: [PATCH 2/2] reverted unnecessary change to DataObject --- src/Speckle.Objects/Data/DataObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Speckle.Objects/Data/DataObject.cs b/src/Speckle.Objects/Data/DataObject.cs index 27e75c3c..baefdb03 100644 --- a/src/Speckle.Objects/Data/DataObject.cs +++ b/src/Speckle.Objects/Data/DataObject.cs @@ -8,7 +8,7 @@ public class DataObject : Base, IDataObject public required string name { get; set; } [DetachProperty] - public required IReadOnlyList displayValue { get; set; } + public required List displayValue { get; set; } public required Dictionary properties { get; set; }