From 3a1542ddbc3da0fd1e1ca0717b82c0d8a1bbd3b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 01:31:00 +0000 Subject: [PATCH 1/2] Resolve a schema's relative paths through Semantics.Paths [patch] Schema.Paths.cs combined two already-typed Semantics.Paths values through a System.IO.Path string round-trip, re-parsing and re-validating both sides of a combine the library already owns. Resolution now goes through RelativeFilePath/RelativeDirectoryPath.AsAbsolute, and the anchor through AbsoluteFilePath.AbsoluteDirectoryPath. AsAbsolute rather than the `/` combine operator the issue sketched: `/` joins without normalising, so a data source reaching a sibling directory through `..` came back as a route to the file rather than the file. AsAbsolute reproduces Path.GetFullPath(Path.Combine(...)) exactly, including `..`, `.`, repeated separators and the empty-path cases the existing guards already reject. SetSourceFile gains the null guard CA1062 requires once the parameter is dereferenced rather than cast. Fixes #200 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By7NnPN7STCZqeAftJ1BmH --- Schema.Test/SchemaDataSourceTests.cs | 50 ++++++++++++++++++++++++++++ Schema/Models/Schema.Paths.cs | 15 ++++++--- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/Schema.Test/SchemaDataSourceTests.cs b/Schema.Test/SchemaDataSourceTests.cs index 62421a3..19bf458 100644 --- a/Schema.Test/SchemaDataSourceTests.cs +++ b/Schema.Test/SchemaDataSourceTests.cs @@ -107,6 +107,56 @@ public void TestCodeGeneratorOutputPathResolves() Assert.AreEqual(Path.GetFullPath(Path.Combine(workingDirectory, "generated")), resolved.ToString()); } + /// + /// A resolved path is normalised, not merely concatenated. A schema beside the data it + /// describes is the easy case; one that reaches a sibling directory through .. is the + /// case that tells a resolver from a string join, and the resolved value is compared, + /// displayed and used as a key, so it has to come back as the directory it names rather than + /// as a route to it. + /// + [TestMethod] + public void TestFilePathsResolveThroughTraversalSegments() + { + Schema schema = CreateAnchoredSchema("../shared/./items.json"); + + Assert.IsTrue(schema.GetDataSource("Items".As())!.TryResolveFile(out AbsoluteFilePath resolved)); + Assert.AreEqual( + Path.GetFullPath(Path.Combine(workingDirectory, "../shared/./items.json")), + resolved.ToString()); + Assert.IsFalse(resolved.ToString().Contains("..", StringComparison.Ordinal), resolved.ToString()); + } + + [TestMethod] + public void TestDirectoryPathsResolveThroughTraversalSegments() + { + Schema schema = CreateAnchoredSchema(); + SchemaCodeGenerator generator = schema.AddCodeGenerator("CSharp".As())!; + generator.OutputPath = "../build/./generated".As(); + + Assert.IsTrue(generator.TryResolveOutputPath(out AbsoluteDirectoryPath resolved)); + Assert.AreEqual( + Path.GetFullPath(Path.Combine(workingDirectory, "../build/./generated")), + resolved.ToString()); + Assert.IsFalse(resolved.ToString().Contains("..", StringComparison.Ordinal), resolved.ToString()); + } + + /// + /// The anchor is the directory holding the schema file, and nothing else about the file. + /// + [TestMethod] + public void TestTheAnchorIsTheSchemaFilesDirectory() + { + Schema schema = new(); + schema.SetSourceFile(SchemaPath); + + Assert.AreEqual(workingDirectory, schema.SourceDirectory.ToString()); + Assert.AreEqual("test.schema.json", schema.SourceFileName); + } + + [TestMethod] + public void TestSetSourceFileRefusesNull() => + Assert.ThrowsExactly(() => new Schema().SetSourceFile(null!)); + [TestMethod] public void TestLoadWithASourcePathAnchorsTheSchema() { diff --git a/Schema/Models/Schema.Paths.cs b/Schema/Models/Schema.Paths.cs index 646dff8..833b77a 100644 --- a/Schema/Models/Schema.Paths.cs +++ b/Schema/Models/Schema.Paths.cs @@ -3,7 +3,6 @@ namespace ktsu.Schema.Models; using ktsu.Semantics.Paths; -using ktsu.Semantics.Strings; /// /// Resolving the relative paths a schema holds. @@ -16,6 +15,10 @@ namespace ktsu.Schema.Models; /// The anchor is supplied by whoever read the file, so the serializer itself stays free of the /// filesystem. A schema that was never read from a file has no anchor and cannot resolve /// anything, which every resolution API reports rather than guessing at the working directory. +/// +/// Resolution goes through AsAbsolute rather than the / combine operator: a schema +/// may reach a sibling directory through .., and only the former normalises those segments +/// away. The operator joins, which hands back a route to the file rather than the file. /// public partial class Schema { @@ -40,7 +43,7 @@ public bool TryResolvePath(RelativeFilePath relativePath, out AbsoluteFilePath r return false; } - resolved = Path.GetFullPath(Path.Combine(SourceDirectory, relativePath)).As(); + resolved = relativePath.AsAbsolute(SourceDirectory); return true; } @@ -59,7 +62,7 @@ public bool TryResolvePath(RelativeDirectoryPath relativePath, out AbsoluteDirec return false; } - resolved = Path.GetFullPath(Path.Combine(SourceDirectory, relativePath)).As(); + resolved = relativePath.AsAbsolute(SourceDirectory); return true; } @@ -71,10 +74,12 @@ public bool TryResolvePath(RelativeDirectoryPath relativePath, out AbsoluteDirec /// the directory containing it becomes the anchor. /// /// The path of the .schema.json file this schema came from. + /// is null. public void SetSourceFile(AbsoluteFilePath schemaFilePath) { - string? directory = Path.GetDirectoryName((string)schemaFilePath); - SourceDirectory = string.IsNullOrEmpty(directory) ? new() : directory.As(); + Ensure.NotNull(schemaFilePath); + + SourceDirectory = schemaFilePath.AbsoluteDirectoryPath; SourceFileName = Path.GetFileName((string)schemaFilePath); } } From e57fa12596afb254027d0db908ce3c10f0edd67a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 02:02:22 +0000 Subject: [PATCH 2/2] Keep taking the anchor from the string, not AbsoluteDirectoryPath [patch] CI caught a real regression: FileBrowserTests.SavingRecordsTheChosenPathAsRecentlyUsed failed with an expected and actual that render character-for-character identical. Reading AbsoluteFilePath.AbsoluteDirectoryPath mutates the instance it is read from. In ktsu.Semantics.Paths 5.4.2 an AbsoluteFilePath stops comparing equal to an identical one, and its hash code changes, once that property has been touched, while its text stays the same. SchemaEditor.SaveToCurrentPath hands the same instance to SetSourceFile and then to RecordRecentFile, so anchoring the schema silently broke equality for a path this code does not own. So SetSourceFile goes back to Path.GetDirectoryName, and with it the null guard CA1062 only wanted because the parameter was being dereferenced. The two TryResolvePath overloads keep AsAbsolute: measured, it mutates neither its receiver nor its argument, and it is where the issue's actual win was. Adds a test that fails on the property version and passes on this one, and records the hazard where the next reader would otherwise 'tidy' it back. Verified: Schema.Test 502/502, Schema.Editor.UITests 190/190 (the suite that failed), Schema.Cpp.Test 104/104, Schema.Editor.Test 17/17. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By7NnPN7STCZqeAftJ1BmH --- Schema.Test/SchemaDataSourceTests.cs | 19 +++++++++++++++++-- Schema/Models/Schema.Paths.cs | 16 ++++++++++++---- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/Schema.Test/SchemaDataSourceTests.cs b/Schema.Test/SchemaDataSourceTests.cs index 19bf458..770098d 100644 --- a/Schema.Test/SchemaDataSourceTests.cs +++ b/Schema.Test/SchemaDataSourceTests.cs @@ -153,9 +153,24 @@ public void TestTheAnchorIsTheSchemaFilesDirectory() Assert.AreEqual("test.schema.json", schema.SourceFileName); } + /// + /// Anchoring a schema must not disturb the path it was handed. Callers pass an instance they + /// keep using - the editor records the same one as a recent file straight afterwards - and + /// reading AbsoluteFilePath.AbsoluteDirectoryPath to find the anchor silently breaks + /// equality and the hash code of the instance it is read from (ktsu.Semantics.Paths 5.4.2), + /// while leaving its text alone. That is why the anchor is still taken from the string. + /// [TestMethod] - public void TestSetSourceFileRefusesNull() => - Assert.ThrowsExactly(() => new Schema().SetSourceFile(null!)); + public void TestSettingTheSourceFileLeavesTheCallersPathEqualToItself() + { + AbsoluteFilePath handedIn = SchemaPath; + AbsoluteFilePath untouched = SchemaPath; + + new Schema().SetSourceFile(handedIn); + + Assert.AreEqual(untouched, handedIn, "Anchoring the schema changed the path it was given."); + Assert.AreEqual(untouched.GetHashCode(), handedIn.GetHashCode(), "Anchoring the schema changed the hash code of the path it was given."); + } [TestMethod] public void TestLoadWithASourcePathAnchorsTheSchema() diff --git a/Schema/Models/Schema.Paths.cs b/Schema/Models/Schema.Paths.cs index 833b77a..eecf914 100644 --- a/Schema/Models/Schema.Paths.cs +++ b/Schema/Models/Schema.Paths.cs @@ -3,6 +3,7 @@ namespace ktsu.Schema.Models; using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; /// /// Resolving the relative paths a schema holds. @@ -19,6 +20,15 @@ namespace ktsu.Schema.Models; /// Resolution goes through AsAbsolute rather than the / combine operator: a schema /// may reach a sibling directory through .., and only the former normalises those segments /// away. The operator joins, which hands back a route to the file rather than the file. +/// +/// The anchor is still taken with rather than +/// AbsoluteFilePath.AbsoluteDirectoryPath, which would read better. Reading that property +/// mutates the instance it is read from: in ktsu.Semantics.Paths 5.4.2 an +/// AbsoluteFilePath stops comparing equal to an identical one, and its hash code changes, +/// once the property has been touched, while its text stays the same. Callers hand the same +/// instance on afterwards - the editor records it as a recent file - so reading it here corrupted +/// equality for a value this code does not own. AsAbsolute carries no such hazard, which is +/// why only the resolution moved. /// public partial class Schema { @@ -74,12 +84,10 @@ public bool TryResolvePath(RelativeDirectoryPath relativePath, out AbsoluteDirec /// the directory containing it becomes the anchor. /// /// The path of the .schema.json file this schema came from. - /// is null. public void SetSourceFile(AbsoluteFilePath schemaFilePath) { - Ensure.NotNull(schemaFilePath); - - SourceDirectory = schemaFilePath.AbsoluteDirectoryPath; + string? directory = Path.GetDirectoryName((string)schemaFilePath); + SourceDirectory = string.IsNullOrEmpty(directory) ? new() : directory.As(); SourceFileName = Path.GetFileName((string)schemaFilePath); } }