From f5bca9b9f7577b795d1611364e364b37b4006f01 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 15:34:19 +0000 Subject: [PATCH 1/2] [patch] Fold the two NormalizePropertyName helpers and stop decoration-only keys being renamed NameStandardizer and PropertyMerger each carried their own copy of NormalizePropertyName, and the copies had drifted. The standardizer lower-cased without trimming, compared its prefix and suffix culture-sensitively, and replaced only the space character, so a key carrying leading or trailing whitespace - a tab especially - kept the padding and missed the affix strip entirely. Measured over a corpus of 30 keys, the two implementations disagreed on 15. Both now call a single PropertyNameNormalizer, which takes the merger's behaviour: trim first so a padded key normalizes like its bare form, ordinal affix comparisons, and one separator collapse. Folding them surfaced a latent bug that was already reachable on main without any padding. A key made only of decoration - "page_", "_value", "_", "page__value" - normalizes to the empty string, and every string contains the empty string, so the containment tests in both classes admitted it against the whole candidate set. The standardizer renamed such a key to "title" and the merger folded it into an unrelated property, losing the value either way, which is the one thing a round-tripping frontmatter library must not do. Both call sites now treat an empty normalized form as "nothing to match on" and leave the key alone. Padding previously masked some of this by accident: " page_ " survived only because the missing trim blocked the prefix strip, while "page_" did not. That inconsistency is gone. Three regression tests cover the two guards and the padded/bare invariant; all three fail against the previous implementation. The existing 129 tests are unchanged and still pass. Refs #113 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UJa1Uvm6PDR74cLBoE4PMY --- Frontmatter.Test/DecorationOnlyKeyTests.cs | 108 ++++++++++++++++++ .../PropertyNameNormalizerTests.cs | 72 ++++++++++++ Frontmatter/NameStandardizer.cs | 50 ++------ Frontmatter/PropertyMerger.cs | 47 +++----- Frontmatter/PropertyNameNormalizer.cs | 83 ++++++++++++++ 5 files changed, 288 insertions(+), 72 deletions(-) create mode 100644 Frontmatter.Test/DecorationOnlyKeyTests.cs create mode 100644 Frontmatter.Test/PropertyNameNormalizerTests.cs create mode 100644 Frontmatter/PropertyNameNormalizer.cs diff --git a/Frontmatter.Test/DecorationOnlyKeyTests.cs b/Frontmatter.Test/DecorationOnlyKeyTests.cs new file mode 100644 index 0000000..9c73fe4 --- /dev/null +++ b/Frontmatter.Test/DecorationOnlyKeyTests.cs @@ -0,0 +1,108 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Frontmatter.Test; + +using System.Collections.Concurrent; +using System.Reflection; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Regression tests for keys that normalize away to nothing — names made only of a decorative +/// prefix, suffix or separator, such as page_ or _value. +/// +/// +/// Every non-empty string contains the empty string, so a key whose normalized form is empty is +/// admitted by the containment tests in both the standardizer and the merger against the entire +/// candidate set, and is then renamed to, or merged into, whichever candidate ranks first. The +/// value is silently attributed to an unrelated property. These tests pin the guards that stop it. +/// +[TestClass] +public class DecorationOnlyKeyTests +{ + [TestInitialize] + public void ClearCaches() + { + ClearCache(typeof(NameStandardizer), "PropertyNameCache"); + ClearCache(typeof(PropertyMerger), "PropertyMergeCache"); + } + + private static void ClearCache(Type type, string fieldName) + { + FieldInfo? field = type.GetField(fieldName, BindingFlags.NonPublic | BindingFlags.Static); + if (field?.GetValue(null) is ConcurrentDictionary cache) + { + cache.Clear(); + } + } + + [TestMethod] + public void StandardizePropertyNames_PreservesDecorationOnlyKeys() + { + foreach (string key in new[] { "page_", "_value", "_", "page__value", "meta_", "custom_" }) + { + Dictionary result = + NameStandardizer.StandardizePropertyNames(new Dictionary { [key] = "V" }); + + Assert.IsTrue(result.ContainsKey(key), $"[{key}] should be preserved, got [{string.Join(",", result.Keys)}]"); + Assert.AreEqual("V", result[key]); + } + } + + [TestMethod] + public void MergeSimilarProperties_DoesNotMergeDecorationOnlyKeyIntoAnother() + { + Dictionary frontmatter = new() + { + ["page_"] = "Decoration", + ["description"] = "Real", + }; + + Dictionary result = + PropertyMerger.MergeSimilarProperties(frontmatter, FrontmatterMergeStrategy.Maximum); + + Assert.IsTrue(result.ContainsKey("page_"), $"[page_] should survive, got [{string.Join(",", result.Keys)}]"); + Assert.AreEqual("Decoration", result["page_"]); + Assert.AreEqual("Real", result["description"]); + } + + /// + /// A padded key and its unpadded form must reach the same decision: either both match the same + /// standard property, or neither matches and each is preserved as written. They did not before — + /// the standardizer lower-cased without trimming and replaced only the space character, so + /// padding blocked the prefix/suffix strip and sent the two forms down different paths. + /// + /// + /// An unmatched key is preserved verbatim, padding included, which is the correct behaviour for + /// a round-tripping library — so the invariant is over the decision, not the literal spelling. + /// + [TestMethod] + public void StandardizePropertyNames_TreatsPaddedAndUnpaddedKeysAlike() + { + foreach ((string padded, string bare) in new[] + { + ("\tmeta_x_field", "meta_x_field"), + (" page_title ", "page_title"), + ("\tuser_name_value\t", "user_name_value"), + (" post_headline ", "post_headline"), + }) + { + ClearCaches(); + string paddedResult = Single(NameStandardizer.StandardizePropertyNames(new Dictionary { [padded] = "V" })); + ClearCaches(); + string bareResult = Single(NameStandardizer.StandardizePropertyNames(new Dictionary { [bare] = "V" })); + + // When the bare key went unmatched it comes back as itself; the padded key should then + // come back as itself too. Otherwise both should land on the same standard property. + string expected = string.Equals(bareResult, bare, StringComparison.Ordinal) ? padded : bareResult; + + Assert.AreEqual(expected, paddedResult, $"padded [{padded}] and bare [{bare}] disagree"); + } + } + + private static string Single(Dictionary result) + { + Assert.AreEqual(1, result.Count); + return result.Keys.First(); + } +} diff --git a/Frontmatter.Test/PropertyNameNormalizerTests.cs b/Frontmatter.Test/PropertyNameNormalizerTests.cs new file mode 100644 index 0000000..92b2974 --- /dev/null +++ b/Frontmatter.Test/PropertyNameNormalizerTests.cs @@ -0,0 +1,72 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Frontmatter.Test; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests for , the single normalization used by both +/// and . +/// +[TestClass] +public class PropertyNameNormalizerTests +{ + private static readonly string[] reviewStatusWords = ["review", "status"]; + private static readonly string[] noWords = []; + + [TestMethod] + public void Normalize_LowercasesAndCollapsesSeparators() + { + Assert.AreEqual("my_key", PropertyNameNormalizer.Normalize("My-Key")); + Assert.AreEqual("my_key", PropertyNameNormalizer.Normalize("my key")); + Assert.AreEqual("my_key", PropertyNameNormalizer.Normalize("my__key")); + Assert.AreEqual("my_key", PropertyNameNormalizer.Normalize("_my_key_")); + } + + [TestMethod] + public void Normalize_StripsOneDecorativePrefixAndSuffix() + { + Assert.AreEqual("title", PropertyNameNormalizer.Normalize("page_title")); + Assert.AreEqual("note", PropertyNameNormalizer.Normalize("note_value")); + Assert.AreEqual("note", PropertyNameNormalizer.Normalize("custom_note_field")); + } + + /// + /// Whitespace is removed before the prefix and suffix strip, so a padded key normalizes to the + /// same thing as its unpadded form. The standardizer's former copy of this logic lower-cased + /// without trimming and replaced only the space character, so a padded — and especially a + /// tab-padded — key kept its padding and missed the strip entirely. + /// + [TestMethod] + public void Normalize_TrimsAllWhitespaceBeforeStrippingAffixes() + { + foreach (string padded in new[] { " page_title ", "\tpage_title\t", "\npage_title\n", " page_title " }) + { + Assert.AreEqual("title", PropertyNameNormalizer.Normalize(padded), $"for [{padded}]"); + } + + Assert.AreEqual("name", PropertyNameNormalizer.Normalize("\tuser_name_value\t")); + Assert.AreEqual("headline", PropertyNameNormalizer.Normalize(" post_headline ")); + } + + /// + /// A key made only of decoration normalizes to the empty string. Both callers must treat that as + /// "nothing to match on" rather than as a value to compare, because every string contains the + /// empty string. + /// + [TestMethod] + public void Normalize_DecorationOnlyKeysNormalizeToEmpty() + { + foreach (string decoration in new[] { "page_", "_value", "_", "-", " ", "\t", "", "page__value" }) + { + Assert.AreEqual(string.Empty, PropertyNameNormalizer.Normalize(decoration), $"for [{decoration}]"); + } + } + + [TestMethod] + public void NormalizeToWords_SplitsNormalizedForm() + { + CollectionAssert.AreEqual(reviewStatusWords, PropertyNameNormalizer.NormalizeToWords(" page_Review-Status ")); + CollectionAssert.AreEqual(noWords, PropertyNameNormalizer.NormalizeToWords("page_")); + } +} diff --git a/Frontmatter/NameStandardizer.cs b/Frontmatter/NameStandardizer.cs index def4542..3686b1a 100644 --- a/Frontmatter/NameStandardizer.cs +++ b/Frontmatter/NameStandardizer.cs @@ -90,6 +90,17 @@ internal static Dictionary StandardizePropertyNames(Dictionary string.Equals(NormalizePropertyName(p), normalizedKey, StringComparison.OrdinalIgnoreCase)); @@ -124,42 +135,5 @@ internal static Dictionary StandardizePropertyNames(Dictionary PropertyNameNormalizer.Normalize(key); } diff --git a/Frontmatter/PropertyMerger.cs b/Frontmatter/PropertyMerger.cs index f209501..899e0bf 100644 --- a/Frontmatter/PropertyMerger.cs +++ b/Frontmatter/PropertyMerger.cs @@ -206,6 +206,16 @@ private static string FindBasicCanonicalName(string key, string[] existingKeys) // Remove common prefixes/suffixes and special characters string normalizedKey = NormalizePropertyName(key); + // A key that normalizes away to nothing has no content to match on. Both loops below would + // otherwise treat it as equal to, or contained by, every other key — the exact-match loop + // pairs it with any other decoration-only key, and the partial-match loop admits it against + // all of them, since every string contains the empty string. Either way the key is merged + // into an unrelated one and its value is lost, so leave it alone. + if (normalizedKey.Length == 0) + { + return key; + } + // Look for exact matches after normalization foreach (string existingKey in existingKeys) { @@ -258,7 +268,7 @@ private static string FindSemanticCanonicalName(string key, string[] existingKey } // Then try more aggressive matching using word similarity - string[] keyWords = NormalizePropertyName(key).Split(['-', ' ', '_'], StringSplitOptions.RemoveEmptyEntries); + string[] keyWords = PropertyNameNormalizer.NormalizeToWords(key); // Find best match among existing keys (string Key, int Score)? bestMatch = existingKeys @@ -266,7 +276,7 @@ private static string FindSemanticCanonicalName(string key, string[] existingKey Key: existingKey, Score: CalculateWordMatchScore( keyWords, - NormalizePropertyName(existingKey).Split(['-', ' ', '_'], StringSplitOptions.RemoveEmptyEntries) + PropertyNameNormalizer.NormalizeToWords(existingKey) ) )) .Where(match => match.Score > 0) @@ -277,38 +287,7 @@ private static string FindSemanticCanonicalName(string key, string[] existingKey return bestMatch?.Key ?? key; } - private static string NormalizePropertyName(string key) - { - // Convert to lowercase and trim - key = key.Trim().ToLowerInvariant(); - - // Common prefixes and suffixes to remove - string[] prefixes = ["page_", "post_", "meta_", "custom_", "user_", "site_"]; - string[] suffixes = ["_value", "_text", "_data", "_info", "_meta", "_field"]; - - // Remove prefixes - foreach (string prefix in prefixes) - { - if (key.StartsWith(prefix, StringComparison.Ordinal)) - { - key = key[prefix.Length..]; - break; - } - } - - // Remove suffixes - foreach (string suffix in suffixes) - { - if (key.EndsWith(suffix, StringComparison.Ordinal)) - { - key = key[..^suffix.Length]; - break; - } - } - - // Replace special characters with underscores and remove duplicates - return string.Join("_", key.Split(['-', ' ', '_'], StringSplitOptions.RemoveEmptyEntries)); - } + private static string NormalizePropertyName(string key) => PropertyNameNormalizer.Normalize(key); private static int CalculateWordMatchScore(string[] words1, string[] words2) { diff --git a/Frontmatter/PropertyNameNormalizer.cs b/Frontmatter/PropertyNameNormalizer.cs new file mode 100644 index 0000000..0ed829f --- /dev/null +++ b/Frontmatter/PropertyNameNormalizer.cs @@ -0,0 +1,83 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Frontmatter; + +/// +/// Normalizes frontmatter property names into a common form so that keys which differ only by +/// casing, separators, surrounding whitespace or a decorative prefix/suffix compare equal. +/// +/// +/// +/// and each carried their own copy of +/// this logic. The two had drifted apart: the standardizer lower-cased without trimming, stripped +/// its prefix and suffix with culture-sensitive comparisons, and replaced only the space character, +/// so a key carrying leading or trailing whitespace — a tab especially — kept it and missed the +/// prefix/suffix strip entirely. This single implementation takes the merger's behaviour, which is +/// the more robust of the two on every point of difference. +/// +/// +/// Trimming happens before the prefix and suffix strip so that " page_title " normalizes the +/// same as "page_title", and covers every whitespace character +/// rather than the space alone. The prefix and suffix comparisons are ordinal because these are +/// fixed ASCII tokens, matched against an already invariantly-lowercased key. +/// +/// +internal static class PropertyNameNormalizer +{ + /// + /// Decorative leading tokens that carry no meaning for matching purposes. + /// + private static readonly string[] Prefixes = ["page_", "post_", "meta_", "custom_", "user_", "site_"]; + + /// + /// Decorative trailing tokens that carry no meaning for matching purposes. + /// + private static readonly string[] Suffixes = ["_value", "_text", "_data", "_info", "_meta", "_field"]; + + /// + /// Characters treated as word separators within a property name. + /// + private static readonly char[] Separators = ['-', ' ', '_']; + + /// + /// Normalizes a property name for comparison. + /// + /// The property name to normalize. + /// + /// The name lower-cased and trimmed, with at most one decorative prefix and one decorative + /// suffix removed, and all separator runs collapsed to a single underscore. Leading and + /// trailing separators are dropped, so the result never begins or ends with an underscore. + /// + internal static string Normalize(string key) + { + key = key.Trim().ToLowerInvariant(); + + foreach (string prefix in Prefixes) + { + if (key.StartsWith(prefix, StringComparison.Ordinal)) + { + key = key[prefix.Length..]; + break; + } + } + + foreach (string suffix in Suffixes) + { + if (key.EndsWith(suffix, StringComparison.Ordinal)) + { + key = key[..^suffix.Length]; + break; + } + } + + return string.Join("_", key.Split(Separators, StringSplitOptions.RemoveEmptyEntries)); + } + + /// + /// Normalizes a property name and splits it into its constituent words. + /// + /// The property name to normalize and split. + /// The normalized name's words, with empty entries removed. + internal static string[] NormalizeToWords(string key) => + Normalize(key).Split(Separators, StringSplitOptions.RemoveEmptyEntries); +} From b6cec44f51f46eabcf0d191d5f47f45004eba810 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 15:48:02 +0000 Subject: [PATCH 2/2] [patch] Clear the five SonarCloud findings on the new code All five land in code this PR introduces. csharpsquid:S3267 x2 on the affix loops. Sonar suggests Where, but the loop body reassigns the captured key between the prefix and suffix steps, so a deferred LINQ filter over it would be a hazard rather than a simplification. Array.Find says what the loop actually does - take the first match, then act - and removes the finding on its own terms. MSTEST0068 x2: CollectionAssert.AreEqual -> Assert.AreSequenceEqual. MSTEST0037: Assert.AreEqual(1, x.Count) -> Assert.HasCount(1, x). No behaviour change; Array.Find returns the first match exactly as the foreach/break did. Suite still 137 green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UJa1Uvm6PDR74cLBoE4PMY --- Frontmatter.Test/DecorationOnlyKeyTests.cs | 2 +- .../PropertyNameNormalizerTests.cs | 4 ++-- Frontmatter/PropertyNameNormalizer.cs | 20 ++++++++----------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Frontmatter.Test/DecorationOnlyKeyTests.cs b/Frontmatter.Test/DecorationOnlyKeyTests.cs index 9c73fe4..9c0f4af 100644 --- a/Frontmatter.Test/DecorationOnlyKeyTests.cs +++ b/Frontmatter.Test/DecorationOnlyKeyTests.cs @@ -102,7 +102,7 @@ public void StandardizePropertyNames_TreatsPaddedAndUnpaddedKeysAlike() private static string Single(Dictionary result) { - Assert.AreEqual(1, result.Count); + Assert.HasCount(1, result); return result.Keys.First(); } } diff --git a/Frontmatter.Test/PropertyNameNormalizerTests.cs b/Frontmatter.Test/PropertyNameNormalizerTests.cs index 92b2974..577fcd2 100644 --- a/Frontmatter.Test/PropertyNameNormalizerTests.cs +++ b/Frontmatter.Test/PropertyNameNormalizerTests.cs @@ -66,7 +66,7 @@ public void Normalize_DecorationOnlyKeysNormalizeToEmpty() [TestMethod] public void NormalizeToWords_SplitsNormalizedForm() { - CollectionAssert.AreEqual(reviewStatusWords, PropertyNameNormalizer.NormalizeToWords(" page_Review-Status ")); - CollectionAssert.AreEqual(noWords, PropertyNameNormalizer.NormalizeToWords("page_")); + Assert.AreSequenceEqual(reviewStatusWords, PropertyNameNormalizer.NormalizeToWords(" page_Review-Status ")); + Assert.AreSequenceEqual(noWords, PropertyNameNormalizer.NormalizeToWords("page_")); } } diff --git a/Frontmatter/PropertyNameNormalizer.cs b/Frontmatter/PropertyNameNormalizer.cs index 0ed829f..86798e9 100644 --- a/Frontmatter/PropertyNameNormalizer.cs +++ b/Frontmatter/PropertyNameNormalizer.cs @@ -52,22 +52,18 @@ internal static string Normalize(string key) { key = key.Trim().ToLowerInvariant(); - foreach (string prefix in Prefixes) + // At most one prefix and one suffix are removed, so this is find-first-then-act rather than + // a filter: the captured key is reassigned between the two steps. + string? matchedPrefix = Array.Find(Prefixes, prefix => key.StartsWith(prefix, StringComparison.Ordinal)); + if (matchedPrefix is not null) { - if (key.StartsWith(prefix, StringComparison.Ordinal)) - { - key = key[prefix.Length..]; - break; - } + key = key[matchedPrefix.Length..]; } - foreach (string suffix in Suffixes) + string? matchedSuffix = Array.Find(Suffixes, suffix => key.EndsWith(suffix, StringComparison.Ordinal)); + if (matchedSuffix is not null) { - if (key.EndsWith(suffix, StringComparison.Ordinal)) - { - key = key[..^suffix.Length]; - break; - } + key = key[..^matchedSuffix.Length]; } return string.Join("_", key.Split(Separators, StringSplitOptions.RemoveEmptyEntries));