diff --git a/Semantics.Paths/Implementations/AbsoluteDirectoryPath.cs b/Semantics.Paths/Implementations/AbsoluteDirectoryPath.cs index 809cea57..313c6c1e 100644 --- a/Semantics.Paths/Implementations/AbsoluteDirectoryPath.cs +++ b/Semantics.Paths/Implementations/AbsoluteDirectoryPath.cs @@ -10,7 +10,23 @@ namespace ktsu.Semantics.Paths; [IsAbsolutePath] public sealed record AbsoluteDirectoryPath : SemanticDirectoryPath, IAbsoluteDirectoryPath { - // Cache for expensive parent directory computation + /// + /// Determines whether this path and are the same path. + /// + /// The path to compare with, or . + /// if both are absolute directory paths with the same text; otherwise, . + /// + /// Declared explicitly, rather than left to the record's generated equality, because that would compare the + /// private memoization fields below as well — so reading a derived property would change both equality and the + /// hash code while leaving the text untouched, moving the instance out of its own hash bucket. A path's identity + /// is its text. See . + /// + public bool Equals(AbsoluteDirectoryPath? other) => base.Equals(other); + + /// + public override int GetHashCode() => base.GetHashCode(); + + // Cache for expensive parent directory computation. Excluded from equality by the members above. private AbsoluteDirectoryPath? _cachedParent; /// diff --git a/Semantics.Paths/Implementations/AbsoluteFilePath.cs b/Semantics.Paths/Implementations/AbsoluteFilePath.cs index 67e7e3c8..f0dd29bd 100644 --- a/Semantics.Paths/Implementations/AbsoluteFilePath.cs +++ b/Semantics.Paths/Implementations/AbsoluteFilePath.cs @@ -8,7 +8,23 @@ namespace ktsu.Semantics.Paths; [IsAbsolutePath] public sealed record AbsoluteFilePath : SemanticFilePath, IAbsoluteFilePath { - // Cache for expensive directory path computation + /// + /// Determines whether this path and are the same path. + /// + /// The path to compare with, or . + /// if both are absolute file paths with the same text; otherwise, . + /// + /// Declared explicitly, rather than left to the record's generated equality, because that would compare the + /// private memoization fields below as well — so reading a derived property would change both equality and the + /// hash code while leaving the text untouched, moving the instance out of its own hash bucket. A path's identity + /// is its text. See . + /// + public bool Equals(AbsoluteFilePath? other) => base.Equals(other); + + /// + public override int GetHashCode() => base.GetHashCode(); + + // Cache for expensive directory path computation. Excluded from equality by the members above. private AbsoluteDirectoryPath? _cachedDirectoryPath; /// diff --git a/Semantics.Paths/Implementations/RelativeDirectoryPath.cs b/Semantics.Paths/Implementations/RelativeDirectoryPath.cs index 76172fb6..664b2d2b 100644 --- a/Semantics.Paths/Implementations/RelativeDirectoryPath.cs +++ b/Semantics.Paths/Implementations/RelativeDirectoryPath.cs @@ -14,7 +14,23 @@ namespace ktsu.Semantics.Paths; [IsRelativePath] public sealed record RelativeDirectoryPath : SemanticDirectoryPath, IRelativeDirectoryPath { - // Cache for expensive parent directory computation + /// + /// Determines whether this path and are the same path. + /// + /// The path to compare with, or . + /// if both are relative directory paths with the same text; otherwise, . + /// + /// Declared explicitly, rather than left to the record's generated equality, because that would compare the + /// private memoization fields below as well — so reading a derived property would change both equality and the + /// hash code while leaving the text untouched, moving the instance out of its own hash bucket. A path's identity + /// is its text. See . + /// + public bool Equals(RelativeDirectoryPath? other) => base.Equals(other); + + /// + public override int GetHashCode() => base.GetHashCode(); + + // Cache for expensive parent directory computation. Excluded from equality by the members above. private RelativeDirectoryPath? _cachedParent; /// diff --git a/Semantics.Paths/Implementations/RelativeFilePath.cs b/Semantics.Paths/Implementations/RelativeFilePath.cs index a359705f..d972f8be 100644 --- a/Semantics.Paths/Implementations/RelativeFilePath.cs +++ b/Semantics.Paths/Implementations/RelativeFilePath.cs @@ -8,7 +8,23 @@ namespace ktsu.Semantics.Paths; [IsRelativePath] public sealed record RelativeFilePath : SemanticFilePath, IRelativeFilePath { - // Cache for expensive directory path computation + /// + /// Determines whether this path and are the same path. + /// + /// The path to compare with, or . + /// if both are relative file paths with the same text; otherwise, . + /// + /// Declared explicitly, rather than left to the record's generated equality, because that would compare the + /// private memoization fields below as well — so reading a derived property would change both equality and the + /// hash code while leaving the text untouched, moving the instance out of its own hash bucket. A path's identity + /// is its text. See . + /// + public bool Equals(RelativeFilePath? other) => base.Equals(other); + + /// + public override int GetHashCode() => base.GetHashCode(); + + // Cache for expensive directory path computation. Excluded from equality by the members above. private RelativeDirectoryPath? _cachedDirectoryPath; /// diff --git a/Semantics.Test/Paths/PathCacheEqualityTests.cs b/Semantics.Test/Paths/PathCacheEqualityTests.cs new file mode 100644 index 00000000..95165980 --- /dev/null +++ b/Semantics.Test/Paths/PathCacheEqualityTests.cs @@ -0,0 +1,260 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Semantics.Test.Paths; + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// A path's identity is its text. Reading a derived property must not change whether the instance +/// compares equal to an identical one, nor what it hashes to. +/// +/// +/// The path types memoize their derived properties in private instance fields. A record's generated +/// Equals and GetHashCode compare every instance field, so populating a cache on +/// first read used to move the instance out of its own hash bucket and stop it comparing equal to an +/// identical value — while its text stayed character-for-character the same. See +/// . +/// +[TestClass] +public class PathCacheEqualityTests +{ + /// + /// Asserts that reading leaves equal to + /// , in every direction and by hash code. + /// + /// The path type under test. + /// The instance the property is read from. + /// An instance built from the same text, never read from. + /// Reads the derived property under test. + /// The property being read, for the failure message. + private static void AssertStableAfterReading(T subject, T twin, Action read, string propertyName) + where T : SemanticString + { + Assert.AreEqual(twin, subject, $"{typeof(T).Name} was not equal to its twin before reading {propertyName}"); + + read(subject); + + Assert.AreEqual(twin, subject, $"reading {typeof(T).Name}.{propertyName} broke Equals(twin, subject)"); + Assert.AreEqual(subject, twin, $"reading {typeof(T).Name}.{propertyName} broke Equals(subject, twin)"); + Assert.AreEqual( + twin.GetHashCode(), + subject.GetHashCode(), + $"reading {typeof(T).Name}.{propertyName} changed the hash code"); + } + + [TestMethod] + public void ReadingAbsoluteDirectoryPathLeavesTheFilePathEqualAndHashable() + { + string text = TestPaths.Absolute("tmp", "x", "y.schema.json"); + AbsoluteFilePath subject = AbsoluteFilePath.Create(text); + AbsoluteFilePath twin = AbsoluteFilePath.Create(text); + + AssertStableAfterReading(subject, twin, path => _ = path.AbsoluteDirectoryPath, nameof(AbsoluteFilePath.AbsoluteDirectoryPath)); + } + + [TestMethod] + public void ReadingFileNameWithoutExtensionLeavesTheAbsoluteFilePathEqualAndHashable() + { + string text = TestPaths.Absolute("tmp", "x", "y.schema.json"); + AbsoluteFilePath subject = AbsoluteFilePath.Create(text); + AbsoluteFilePath twin = AbsoluteFilePath.Create(text); + + AssertStableAfterReading(subject, twin, path => _ = path.FileNameWithoutExtension, nameof(AbsoluteFilePath.FileNameWithoutExtension)); + } + + [TestMethod] + public void ReadingEveryDerivedPropertyLeavesTheAbsoluteFilePathEqualAndHashable() + { + string text = TestPaths.Absolute("tmp", "x", "y.schema.json"); + AbsoluteFilePath subject = AbsoluteFilePath.Create(text); + AbsoluteFilePath twin = AbsoluteFilePath.Create(text); + + AssertStableAfterReading( + subject, + twin, + path => + { + _ = path.AbsoluteDirectoryPath; + _ = path.FileNameWithoutExtension; + _ = path.FileName; + _ = path.FileExtension; + }, + "every derived property"); + } + + [TestMethod] + public void ReadingEveryDerivedPropertyLeavesTheRelativeFilePathEqualAndHashable() + { + string text = TestPaths.Relative("x", "y.schema.json"); + RelativeFilePath subject = RelativeFilePath.Create(text); + RelativeFilePath twin = RelativeFilePath.Create(text); + + AssertStableAfterReading( + subject, + twin, + path => + { + _ = path.RelativeDirectoryPath; + _ = path.FileNameWithoutExtension; + _ = path.FileName; + _ = path.FileExtension; + }, + "every derived property"); + } + + [TestMethod] + public void ReadingEveryDerivedPropertyLeavesTheAbsoluteDirectoryPathEqualAndHashable() + { + string text = TestPaths.Absolute("tmp", "x", "src"); + AbsoluteDirectoryPath subject = AbsoluteDirectoryPath.Create(text); + AbsoluteDirectoryPath twin = AbsoluteDirectoryPath.Create(text); + + AssertStableAfterReading( + subject, + twin, + path => + { + _ = path.Parent; + _ = path.Name; + _ = path.Depth; + _ = path.IsRoot; + }, + "every derived property"); + } + + [TestMethod] + public void ReadingEveryDerivedPropertyLeavesTheRelativeDirectoryPathEqualAndHashable() + { + string text = TestPaths.Relative("projects", "app", "src"); + RelativeDirectoryPath subject = RelativeDirectoryPath.Create(text); + RelativeDirectoryPath twin = RelativeDirectoryPath.Create(text); + + AssertStableAfterReading( + subject, + twin, + path => + { + _ = path.Parent; + _ = path.Name; + _ = path.Depth; + }, + "every derived property"); + } + + [TestMethod] + public void APathStaysFindableAsADictionaryKeyAfterItsDirectoryIsRead() + { + string text = TestPaths.Absolute("tmp", "x", "y.schema.json"); + AbsoluteFilePath key = AbsoluteFilePath.Create(text); + Dictionary recentFiles = new() { [key] = "recent" }; + + _ = key.AbsoluteDirectoryPath; + + Assert.IsTrue( + recentFiles.ContainsKey(key), + "the stored key could not be found in the dictionary it was stored in, after its directory was read"); + Assert.IsTrue( + recentFiles.ContainsKey(AbsoluteFilePath.Create(text)), + "an equal key could not be found in the dictionary after the stored key's directory was read"); + } + + [TestMethod] + public void APathStaysFindableInASetAfterItsDirectoryIsRead() + { + string text = TestPaths.Absolute("tmp", "x", "y.schema.json"); + AbsoluteFilePath member = AbsoluteFilePath.Create(text); + HashSet seen = [member]; + + _ = member.AbsoluteDirectoryPath; + + Assert.IsTrue(seen.Contains(member), "the set no longer contains the member it was built from"); + Assert.IsFalse( + seen.Add(AbsoluteFilePath.Create(text)), + "an equal path was added to the set a second time after the first one's directory was read"); + } + + [TestMethod] + public void DifferentPathsStayUnequal() + { + AbsoluteFilePath first = AbsoluteFilePath.Create(TestPaths.Absolute("tmp", "x", "y.schema.json")); + AbsoluteFilePath second = AbsoluteFilePath.Create(TestPaths.Absolute("tmp", "x", "z.schema.json")); + + _ = first.AbsoluteDirectoryPath; + _ = second.AbsoluteDirectoryPath; + + Assert.AreNotEqual(first, second); + } + + /// + /// Determines whether was written by hand rather than synthesized for the record. + /// + /// The candidate equality member, or if the type declares none. + /// if the method exists and is not compiler-generated. + /// + /// A record's generated Equals and GetHashCode are declared members of the type, so their presence + /// alone proves nothing — the distinction is . It must be read with + /// inherit: false: a hand-written GetHashCode overrides the base record's generated one and would + /// otherwise inherit its attribute, making every type look compiler-generated. + /// + private static bool IsHandWritten(MethodInfo? method) => + method is not null && !method.IsDefined(typeof(CompilerGeneratedAttribute), inherit: false); + + /// + /// The structural rule behind the behavioural tests above, so a path type added later cannot + /// reintroduce the defect by declaring a cache field and inheriting the generated equality. + /// + [TestMethod] + public void EveryPathTypeWithInstanceFieldsDeclaresItsOwnEqualityMembers() + { + IEnumerable pathTypes = typeof(AbsoluteFilePath).Assembly + .GetTypes() + .Where(type => type is { IsClass: true, IsAbstract: false, IsPublic: true }) + .Where(typeof(ISemanticString).IsAssignableFrom); + + List offenders = []; + + foreach (Type type in pathTypes) + { + FieldInfo[] instanceFields = type.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly); + if (instanceFields.Length == 0) + { + continue; + } + + bool declaresGetHashCode = IsHandWritten(type.GetMethod( + nameof(GetHashCode), + BindingFlags.Instance | BindingFlags.Public | BindingFlags.DeclaredOnly, + binder: null, + types: Type.EmptyTypes, + modifiers: null)); + + bool declaresEquals = IsHandWritten(type.GetMethod( + nameof(Equals), + BindingFlags.Instance | BindingFlags.Public | BindingFlags.DeclaredOnly, + binder: null, + types: [type], + modifiers: null)); + + if (!declaresGetHashCode || !declaresEquals) + { + string fields = string.Join(", ", instanceFields.Select(field => field.Name)); + offenders.Add($"{type.Name} declares instance field(s) {fields} but no {(declaresEquals ? "" : $"Equals({type.Name}?)")}{(declaresEquals || declaresGetHashCode ? "" : " and no ")}{(declaresGetHashCode ? "" : "GetHashCode()")}"); + } + } + + Assert.AreEqual( + 0, + offenders.Count, + "A record's generated equality includes every instance field, so a type that memoizes into one " + + "must declare Equals and GetHashCode that defer to WeakString only:" + + Environment.NewLine + + string.Join(Environment.NewLine, offenders)); + } +}