From dec685361ce97f63a807854ee364123814bd3fbd Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Thu, 12 Nov 2026 19:11:49 -0500 Subject: [PATCH] refactor(core): make Math and Geometry Core's bottom layers Move ContourSampler to OpenNest.CNC and DrawingAligner with its AlignmentResult types to OpenNest.Converters: they were the only Geometry files importing CNC and Converters. Their tests move with them. CoreLayeringTests scans the Math and Geometry sources against the type names reflection finds in Core, so neither can name a higher layer again. Geometry keeps six root-namespace types on an explicit allowlist, and the files declaring them may reference only Math and Geometry. Co-Authored-By: Claude Fable 5.2 --- AGENTS.md | 2 +- .../{Geometry => CNC}/ContourSampler.cs | 4 +- .../AlignmentResult.cs | 3 +- .../DrawingAligner.cs | 4 +- .../{Geometry => CNC}/ContourSamplerTests.cs | 2 +- .../DrawingAlignerTests.cs | 2 +- OpenNest.Tests/CoreLayeringTests.cs | 388 ++++++++++++++++++ 7 files changed, 397 insertions(+), 8 deletions(-) rename OpenNest.Core/{Geometry => CNC}/ContourSampler.cs (99%) rename OpenNest.Core/{Geometry => Converters}/AlignmentResult.cs (99%) rename OpenNest.Core/{Geometry => Converters}/DrawingAligner.cs (99%) rename OpenNest.Tests/{Geometry => CNC}/ContourSamplerTests.cs (99%) rename OpenNest.Tests/{Geometry => Converters}/DrawingAlignerTests.cs (99%) create mode 100644 OpenNest.Tests/CoreLayeringTests.cs diff --git a/AGENTS.md b/AGENTS.md index 1759386b..b711149a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,7 +38,7 @@ Releases: follow [the release procedure](docs/releasing.md) and `scripts/Publish ## Project map and boundaries -- `OpenNest.Core`: domain (`Nest -> Plate -> Part -> Drawing -> CNC.Program`), geometry, cutting strategies and diagnostics. Angles are radians; use `Tolerance.Epsilon` for geometry comparisons. `OpenNest.Math` shadows `System.Math`, so qualify the latter, and call `PortableMath` (not `System.Math`) for trig in Core/Engine/IO/Data so layouts match across platforms. +- `OpenNest.Core`: domain (`Nest -> Plate -> Part -> Drawing -> CNC.Program`), geometry, cutting strategies and diagnostics. Angles are radians; use `Tolerance.Epsilon` for geometry comparisons. `OpenNest.Math` shadows `System.Math`, so qualify the latter, and call `PortableMath` (not `System.Math`) for trig in Core/Engine/IO/Data so layouts match across platforms. `OpenNest.Math` and `OpenNest.Geometry` are Core's bottom layers (Math uses nothing else in OpenNest; Geometry uses Math plus the root types `OffsetSide`, `RotationType`, `PushDirection`, `RelativePosition`, `PerfCounters`, `SpecialLayers`); `CoreLayeringTests` enforces it. - `OpenNest.Engine`: whole-job API in `Jobs/`, interactive proposals via `PlateFillService`, fill strategies, best-fit pairs, packing, sequencing and rapid planning. `INestingEngine.Solve(NestJob)` returns stock IDs/poses; boundary adapters map drawings and materialize results. `NestJobRunner` validates its candidates before committing demand/stock accounting. Do not assume arbitrary plug-in output or interactive paths received that validation. Job identity is reference-based, not drawing-name-based. - Built-in whole-job engines live in `OpenNest.Engine/NestingEngines//`, named for the jobs they suit; see [nesting engines](docs/nesting-engines.md). A change must beat that engine's current benchmark result with every layout valid. External plug-ins implement `INestingEngine` with a public parameterless constructor and load from `Engines/` beside the host; keep their projects out of this solution. - `OpenNest.IO`: ACadSharp and SVG import/export and ZIP-based `.nest` persistence; depends on Core only (an IO.Tests guard enforces it). Best fits persist only when callers pass `BestFitCacheStore.Shared` as `NestReader`/`NestWriter.BestFitStore`. All DXF-to-Drawing conversion goes through `CadImporter`: `Import` + `BuildDrawing` for editable/reporting flows, `ImportDrawing` for headless callers. Preserve source offsets, entity IDs, suppressed entities and bends. Bend repair is opt-in, requires explicit source units and may not alter cut geometry or unrelated marks. diff --git a/OpenNest.Core/Geometry/ContourSampler.cs b/OpenNest.Core/CNC/ContourSampler.cs similarity index 99% rename from OpenNest.Core/Geometry/ContourSampler.cs rename to OpenNest.Core/CNC/ContourSampler.cs index 54cda5f2..cc409666 100644 --- a/OpenNest.Core/Geometry/ContourSampler.cs +++ b/OpenNest.Core/CNC/ContourSampler.cs @@ -1,9 +1,9 @@ using System; using System.Collections.Generic; -using OpenNest.CNC; +using OpenNest.Geometry; using OpenNest.Math; -namespace OpenNest.Geometry +namespace OpenNest.CNC { /// /// One world-space point on a cut contour together with the cut direction there. diff --git a/OpenNest.Core/Geometry/AlignmentResult.cs b/OpenNest.Core/Converters/AlignmentResult.cs similarity index 99% rename from OpenNest.Core/Geometry/AlignmentResult.cs rename to OpenNest.Core/Converters/AlignmentResult.cs index bd822700..24d2d16a 100644 --- a/OpenNest.Core/Geometry/AlignmentResult.cs +++ b/OpenNest.Core/Converters/AlignmentResult.cs @@ -1,6 +1,7 @@ using System; +using OpenNest.Geometry; -namespace OpenNest.Geometry +namespace OpenNest.Converters { /// /// Tuning for . Every default is a design choice diff --git a/OpenNest.Core/Geometry/DrawingAligner.cs b/OpenNest.Core/Converters/DrawingAligner.cs similarity index 99% rename from OpenNest.Core/Geometry/DrawingAligner.cs rename to OpenNest.Core/Converters/DrawingAligner.cs index 7ad15a52..4f1c565a 100644 --- a/OpenNest.Core/Geometry/DrawingAligner.cs +++ b/OpenNest.Core/Converters/DrawingAligner.cs @@ -4,11 +4,11 @@ using System.Linq; using System.Threading; using Clipper2Lib; using OpenNest.CNC; -using OpenNest.Converters; +using OpenNest.Geometry; using OpenNest.Math; -namespace OpenNest.Geometry +namespace OpenNest.Converters { /// /// Seed-aligns a revised drawing's geometry to an old drawing's drawing-local diff --git a/OpenNest.Tests/Geometry/ContourSamplerTests.cs b/OpenNest.Tests/CNC/ContourSamplerTests.cs similarity index 99% rename from OpenNest.Tests/Geometry/ContourSamplerTests.cs rename to OpenNest.Tests/CNC/ContourSamplerTests.cs index 0f499c83..d740500c 100644 --- a/OpenNest.Tests/Geometry/ContourSamplerTests.cs +++ b/OpenNest.Tests/CNC/ContourSamplerTests.cs @@ -1,7 +1,7 @@ using OpenNest.CNC; using OpenNest.Geometry; -namespace OpenNest.Tests.Geometry; +namespace OpenNest.Tests.CNC; /// /// Characterization tests for the pure sampling math extracted from diff --git a/OpenNest.Tests/Geometry/DrawingAlignerTests.cs b/OpenNest.Tests/Converters/DrawingAlignerTests.cs similarity index 99% rename from OpenNest.Tests/Geometry/DrawingAlignerTests.cs rename to OpenNest.Tests/Converters/DrawingAlignerTests.cs index 8b2ce4ea..3a05f5c4 100644 --- a/OpenNest.Tests/Geometry/DrawingAlignerTests.cs +++ b/OpenNest.Tests/Converters/DrawingAlignerTests.cs @@ -3,7 +3,7 @@ using OpenNest.CNC; using OpenNest.Converters; using OpenNest.Geometry; -namespace OpenNest.Tests.Geometry; +namespace OpenNest.Tests.Converters; /// /// Slice-1 tests for : one rigid asymmetric line/arc diff --git a/OpenNest.Tests/CoreLayeringTests.cs b/OpenNest.Tests/CoreLayeringTests.cs new file mode 100644 index 00000000..303589ab --- /dev/null +++ b/OpenNest.Tests/CoreLayeringTests.cs @@ -0,0 +1,388 @@ +using System.Text.RegularExpressions; + +namespace OpenNest.Tests; + +/// +/// OpenNest.Math and OpenNest.Geometry are the bottom layers of OpenNest.Core: Math +/// uses nothing else in OpenNest, and Geometry uses only Math plus the root-namespace types in +/// . Namespaces inside one assembly are not checked by the +/// compiler, so this scans the sources against the type names reflection finds in Core. +/// +/// It is a token scan, not a binder: it catches a higher type being named, which is how every +/// dependency so far has arrived. It cannot see a higher type reached without naming it (an +/// extension method declared in the root OpenNest namespace, which child namespaces see +/// without a using). +/// +/// +public class CoreLayeringTests +{ + private const string MathNamespace = "OpenNest.Math"; + private const string GeometryNamespace = "OpenNest.Geometry"; + + /// + /// Root-namespace (OpenNest) types Geometry is allowed to name. They stay in the root + /// namespace because moving them would rename public types external plug-ins bind to; + /// keeps them from pulling a + /// higher layer in behind Geometry's back. Do not grow this list to make a failure go away: + /// move the offending code up a layer instead. + /// + private static readonly string[] RootTypesGeometryMayUse = + [ + "OffsetSide", + "RotationType", + "PushDirection", + "RelativePosition", + "PerfCounters", + "SpecialLayers", + ]; + + /// + /// Higher-layer type names the scan cannot tell from something else of the same name, each + /// with the reason. Names that Math/Geometry declare themselves are not listed here: inside + /// those namespaces C# binds the simple name to the nearer declaration, so the scan drops + /// them on its own (see ). + /// + private static readonly Dictionary AmbiguousNames = new() + { + // SpecialLayers.Tab is a Layer field. The Tab class lives in OpenNest.CNC.CuttingStrategy, + // which no scanned file can reach without a using the namespace rule already reports. + ["Tab"] = "SpecialLayers.Tab field", + }; + + private static readonly Regex NamespaceReference = new(@"(?First segments of Core's namespaces under OpenNest: Math, Geometry, CNC... + private static readonly HashSet ChildNamespaces = typeof(Plate).Assembly.GetTypes() + .Select(type => type.Namespace) + .Where(name => name != null && name.StartsWith("OpenNest.", StringComparison.Ordinal)) + .Select(name => name!.Split('.')[1]) + .ToHashSet(StringComparer.Ordinal); + + [Fact] + public void Math_ReferencesNothingElseInOpenNest() + { + var higher = HigherTypeNames(lowerNamespaces: [MathNamespace], shadowingNamespace: MathNamespace); + + var violations = SourceFiles("Math") + .SelectMany(file => Violations(file, allowedNamespaces: ["Math"], higher)) + .ToList(); + + Assert.True(violations.Count == 0, Report("OpenNest.Math must not reference the rest of OpenNest", violations)); + } + + [Fact] + public void Geometry_ReferencesOnlyMathAndAllowlistedRootTypes() + { + var higher = HigherTypeNames(lowerNamespaces: [MathNamespace, GeometryNamespace], shadowingNamespace: GeometryNamespace); + higher.ExceptWith(RootTypesGeometryMayUse); + + var violations = SourceFiles("Geometry") + .SelectMany(file => Violations(file, allowedNamespaces: ["Math", "Geometry"], higher)) + .ToList(); + + Assert.True(violations.Count == 0, Report("OpenNest.Geometry may reference only OpenNest.Math and the allowlisted root types", violations)); + } + + [Fact] + public void AllowlistedRootTypes_ReferenceOnlyMathAndGeometry() + { + // These files sit in the root namespace, where a root type wins over a same-named + // Geometry type, so nothing is shadowed here. + var higher = HigherTypeNames(lowerNamespaces: [MathNamespace, GeometryNamespace], shadowingNamespace: null); + higher.ExceptWith(RootTypesGeometryMayUse); + + var coreDirectory = CoreDirectory(); + var rootFiles = Directory.GetFiles(coreDirectory, "*.cs", SearchOption.TopDirectoryOnly) + .Select(path => (Path: path, Code: StripCommentsAndLiterals(File.ReadAllText(path)))) + .ToList(); + + var violations = new List(); + foreach (var name in RootTypesGeometryMayUse) + { + var type = typeof(Plate).Assembly.GetType("OpenNest." + name); + Assert.True(type != null, $"Allowlisted type OpenNest.{name} no longer exists in OpenNest.Core; remove it from the allowlist."); + + var declaration = new Regex(@"\b(?:enum|class|struct|interface|record)\s+" + Regex.Escape(name) + @"\b"); + var declaring = rootFiles.Where(file => declaration.IsMatch(file.Code)).ToList(); + Assert.True(declaring.Count == 1, $"Expected exactly one file in OpenNest.Core's root declaring {name}, found {declaring.Count}."); + + violations.AddRange(Violations(declaring[0].Path, allowedNamespaces: ["Math", "Geometry"], higher)); + } + + Assert.True(violations.Count == 0, Report("Types Geometry is allowed to use may reference only OpenNest.Math and OpenNest.Geometry", violations)); + } + + [Fact] + public void AmbiguousNames_AreStillAmbiguous() + { + // An exclusion that no longer names a higher Core type is dead weight hiding nothing. + var higher = HigherTypeNames(lowerNamespaces: [MathNamespace, GeometryNamespace], shadowingNamespace: null, applyExclusions: false); + + Assert.Empty(AmbiguousNames.Keys.Where(name => !higher.Contains(name))); + } + + /// + /// Simple names of Core's top-level types outside . Nested + /// types are left out: they are only reachable through their outer type, which is in the set. + /// Names that also declares are left out too, because + /// code in that namespace binds the simple name to its own type before any outer namespace. + /// + private static HashSet HigherTypeNames(string[] lowerNamespaces, string? shadowingNamespace, bool applyExclusions = true) + { + var types = typeof(Plate).Assembly.GetTypes() + .Where(type => !type.IsNested && type.Namespace != null && !type.Name.Contains('<')) + .ToList(); + + var higher = types + .Where(type => !lowerNamespaces.Contains(type.Namespace)) + .Select(SimpleName) + .ToHashSet(StringComparer.Ordinal); + + if (shadowingNamespace != null) + higher.ExceptWith(types.Where(type => type.Namespace == shadowingNamespace).Select(SimpleName)); + + if (applyExclusions) + higher.ExceptWith(AmbiguousNames.Keys); + + return higher; + } + + private static string SimpleName(Type type) + { + var tick = type.Name.IndexOf('`'); + return tick < 0 ? type.Name : type.Name[..tick]; + } + + private static IEnumerable Violations(string path, string[] allowedNamespaces, HashSet higherTypeNames) + { + var code = StripCommentsAndLiterals(File.ReadAllText(path)); + var file = Path.GetRelativePath(RepositoryRoot(), path).Replace('\\', '/'); + + foreach (Match match in NamespaceReference.Matches(code)) + { + var name = match.Groups[1].Value; + if (!allowedNamespaces.Contains(name)) + yield return $"{file}:{LineOf(code, match.Index)}: OpenNest.{name}"; + } + + foreach (Match match in Identifier.Matches(code)) + { + // A name after a dot is a member (entity.Layer) or was qualified, and a qualified + // OpenNest.X is already reported above. + if (FollowsDot(code, match.Index)) + continue; + + if (higherTypeNames.Contains(match.Value)) + yield return $"{file}:{LineOf(code, match.Index)}: {match.Value}"; + + // Code inside OpenNest.* can also qualify from a sibling namespace: CNC.Program. + else if (ChildNamespaces.Contains(match.Value) && !allowedNamespaces.Contains(match.Value) + && PrecedesDot(code, match.Index + match.Length)) + yield return $"{file}:{LineOf(code, match.Index)}: {match.Value}. (namespace OpenNest.{match.Value})"; + } + } + + private static bool FollowsDot(string code, int index) + { + var i = index - 1; + while (i >= 0 && char.IsWhiteSpace(code[i])) + i--; + + return i >= 0 && code[i] == '.'; + } + + private static bool PrecedesDot(string code, int index) + { + while (index < code.Length && char.IsWhiteSpace(code[index])) + index++; + + return index < code.Length && code[index] == '.'; + } + + private static int LineOf(string code, int index) + { + var line = 1; + for (var i = 0; i < index; i++) + if (code[i] == '\n') + line++; + + return line; + } + + private static string Report(string rule, List violations) => + $"{rule}. Offending references (file:line: identifier):{Environment.NewLine}{string.Join(Environment.NewLine, violations)}"; + + private static IEnumerable SourceFiles(string coreSubdirectory) + { + var directory = Path.Combine(CoreDirectory(), coreSubdirectory); + var files = Directory.GetFiles(directory, "*.cs", SearchOption.AllDirectories); + + Assert.NotEmpty(files); + return files.OrderBy(path => path, StringComparer.Ordinal); + } + + private static string CoreDirectory() => Path.Combine(RepositoryRoot(), "OpenNest.Core"); + + private static string RepositoryRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory != null && !File.Exists(Path.Combine(directory.FullName, "OpenNest.sln"))) + directory = directory.Parent; + + Assert.NotNull(directory); + return directory.FullName; + } + + /// + /// Blanks comments, string literals and char literals with spaces, keeping every offset and + /// line break so matches report true line numbers. Interpolation holes of ordinary and + /// verbatim strings are code and are kept; raw string literals are blanked whole. + /// + internal static string StripCommentsAndLiterals(string source) + { + var output = source.ToCharArray(); + var position = 0; + ScanCode(source, output, ref position, untilClosingBrace: false); + return new string(output); + } + + private static void ScanCode(string source, char[] output, ref int i, bool untilClosingBrace) + { + var depth = 0; + while (i < source.Length) + { + var c = source[i]; + var next = i + 1 < source.Length ? source[i + 1] : '\0'; + + if (c == '/' && next == '/') + { + while (i < source.Length && source[i] != '\n') + Blank(source, output, i++); + } + else if (c == '/' && next == '*') + { + var end = source.IndexOf("*/", i + 2, StringComparison.Ordinal); + end = end < 0 ? source.Length : end + 2; + while (i < end) + Blank(source, output, i++); + } + else if (c == '\'') + { + Blank(source, output, i++); + while (i < source.Length && source[i] != '\'') + { + if (source[i] == '\\') + Blank(source, output, i++); + if (i < source.Length) + Blank(source, output, i++); + } + + if (i < source.Length) + Blank(source, output, i++); + } + else if (c == '"' || ((c == '$' || c == '@') && StartsString(source, i))) + { + ScanString(source, output, ref i); + } + else if (untilClosingBrace && c == '{') + { + depth++; + i++; + } + else if (untilClosingBrace && c == '}') + { + if (depth == 0) + return; + + depth--; + i++; + } + else + { + i++; + } + } + } + + private static bool StartsString(string source, int i) + { + while (i < source.Length && (source[i] == '$' || source[i] == '@')) + i++; + + return i < source.Length && source[i] == '"'; + } + + private static void ScanString(string source, char[] output, ref int i) + { + var interpolated = false; + var verbatim = false; + for (; source[i] != '"'; i++) + { + interpolated |= source[i] == '$'; + verbatim |= source[i] == '@'; + Blank(source, output, i); + } + + var quotes = 0; + while (i + quotes < source.Length && source[i + quotes] == '"') + quotes++; + + if (!verbatim && quotes >= 3) + { + var end = source.IndexOf(new string('"', quotes), i + quotes, StringComparison.Ordinal); + end = end < 0 ? source.Length : end + quotes; + while (end < source.Length && source[end] == '"') + end++; + while (i < end) + Blank(source, output, i++); + return; + } + + Blank(source, output, i++); + while (i < source.Length) + { + var c = source[i]; + var next = i + 1 < source.Length ? source[i + 1] : '\0'; + + if (verbatim && c == '"' && next == '"') + { + Blank(source, output, i++); + Blank(source, output, i++); + } + else if (c == '"') + { + Blank(source, output, i++); + return; + } + else if (!verbatim && c == '\\') + { + Blank(source, output, i++); + if (i < source.Length) + Blank(source, output, i++); + } + else if (interpolated && (c == '{' || c == '}') && next == c) + { + Blank(source, output, i++); + Blank(source, output, i++); + } + else if (interpolated && c == '{') + { + Blank(source, output, i++); + ScanCode(source, output, ref i, untilClosingBrace: true); + if (i < source.Length) + Blank(source, output, i++); + } + else + { + Blank(source, output, i++); + } + } + } + + private static void Blank(string source, char[] output, int i) + { + if (source[i] != '\n' && source[i] != '\r') + output[i] = ' '; + } +}