diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisMetathesisRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisMetathesisRuleSpec.cs index 6581ffff4..b98495a65 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisMetathesisRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisMetathesisRuleSpec.cs @@ -50,7 +50,8 @@ private void AddGroup(Dictionary> groups, string { var newGroup = new Group(name); foreach ( - Constraint constraint in groups[name].Children.Cast>() + Constraint constraint in groups[name] + .Children.CastToConstraints("A switch group of a metathesis rule") ) { Constraint newConstraint = constraint.Clone(); diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/EpenthesisAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/EpenthesisAnalysisRewriteRuleSpec.cs index 6c29369b9..2a643282a 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/EpenthesisAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/EpenthesisAnalysisRewriteRuleSpec.cs @@ -1,4 +1,3 @@ -using System.Linq; using SIL.Machine.Annotations; using SIL.Machine.FeatureModel; using SIL.Machine.Matching; @@ -15,7 +14,11 @@ public EpenthesisAnalysisRewriteRuleSpec(MatcherSettings matcherSetti Pattern.Acceptable = IsUnapplicationNonvacuous; _targetCount = subrule.Rhs.Children.Count; - foreach (Constraint constraint in subrule.Rhs.Children.Cast>()) + foreach ( + Constraint constraint in subrule.Rhs.Children.CastToConstraints( + "The replacement (right-hand side) of an epenthesis rewrite subrule" + ) + ) { Constraint newConstraint = constraint.Clone(); newConstraint.FeatureStruct.AddValue(HCFeatureSystem.Modified, HCFeatureSystem.Clean); diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/FeatureAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/FeatureAnalysisRewriteRuleSpec.cs index e98d95ec6..588f12fdf 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/FeatureAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/FeatureAnalysisRewriteRuleSpec.cs @@ -39,8 +39,12 @@ Tuple, PatternNode> tuple in lhs.C ) ) { - var lhsConstraint = (Constraint)tuple.Item1; - var rhsConstraint = (Constraint)tuple.Item2; + Constraint lhsConstraint = tuple.Item1.AsConstraint( + "The target (left-hand side) of a rewrite rule" + ); + Constraint rhsConstraint = tuple.Item2.AsConstraint( + "The replacement (right-hand side) of a rewrite subrule" + ); if (lhsConstraint.Type() == HCFeatureSystem.Segment && rhsConstraint.Type() == HCFeatureSystem.Segment) { diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs index 469f3020a..9d913b06c 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs @@ -1,4 +1,3 @@ -using System.Linq; using SIL.Extensions; using SIL.Machine.Annotations; using SIL.Machine.FeatureModel; @@ -42,7 +41,9 @@ private void Unapply(Match targetMatch, Range range, { ShapeNode curNode = IsTargetEmpty ? range.Start : range.End; foreach ( - Constraint constraint in _analysisRhs.Children.Cast>() + Constraint constraint in _analysisRhs.Children.CastToConstraints( + "The target (left-hand side) of a rewrite rule" + ) ) { FeatureStruct fs = constraint.FeatureStruct.Clone(); diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/PatternNodeCastExtensions.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/PatternNodeCastExtensions.cs new file mode 100644 index 000000000..19498c5ff --- /dev/null +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/PatternNodeCastExtensions.cs @@ -0,0 +1,42 @@ +using System.Collections.Generic; +using SIL.Machine.Annotations; +using SIL.Machine.Matching; + +namespace SIL.Machine.Morphology.HermitCrab.PhonologicalRules +{ + /// + /// Rewrite and metathesis rule specs assume that every top-level child of a rule's target/replacement + /// pattern (or a metathesis switch group) is a simple (a + /// segment, natural class, or boundary marker). and + /// are both direct subclasses of + /// -- siblings, not related by inheritance -- but the DTD and + /// XmlLanguageLoader allow an OptionalSegmentSequence (loaded as a ) + /// in exactly the same positions as a plain segment. These helpers replace unchecked + /// Cast<Constraint<Word, ShapeNode>>() calls and direct casts with a checked + /// conversion that raises a clear, actionable naming the unsupported + /// construct and where it appeared, instead of an opaque . + /// + internal static class PatternNodeCastExtensions + { + public static IEnumerable> CastToConstraints( + this IEnumerable> nodes, + string context + ) + { + foreach (PatternNode node in nodes) + yield return node.AsConstraint(context); + } + + public static Constraint AsConstraint(this PatternNode node, string context) + { + if (node is Constraint constraint) + return constraint; + + throw new CompileException( + $"{context} contains a '{node.GetType().Name}', which is not supported there. Only simple " + + "segments, natural classes, and boundary markers are supported; optional or repeated " + + "segment sequences (quantifiers) are only supported within a rule's left/right environment." + ); + } + } +} diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisMetathesisRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisMetathesisRuleSpec.cs index 312e0a677..e2031f64e 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisMetathesisRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisMetathesisRuleSpec.cs @@ -28,7 +28,9 @@ string rightGroupName { var newGroup = new Group(group.Name); foreach ( - Constraint constraint in group.Children.Cast>() + Constraint constraint in group.Children.CastToConstraints( + "A switch group of a metathesis rule" + ) ) { Constraint newConstraint = constraint.Clone(); diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisRewriteRuleSpec.cs index 87fa10955..b201b92a1 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/SynthesisRewriteRuleSpec.cs @@ -30,7 +30,11 @@ IEnumerable subrules } else { - foreach (Constraint constraint in lhs.Children.Cast>()) + foreach ( + Constraint constraint in lhs.Children.CastToConstraints( + "The target (left-hand side) of a rewrite rule" + ) + ) { var newConstraint = constraint.Clone(); if (isIterative) @@ -90,7 +94,9 @@ Tuple> tuple in match .Zip(lhs.Children) ) { - var constraints = (Constraint)tuple.Item2; + Constraint constraints = tuple.Item2.AsConstraint( + "The target (left-hand side) of a rewrite rule" + ); if (tuple.Item1.Annotation.Type() != constraints.Type()) return false; } diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs index f7c56ecaa..5e0bf867b 100644 --- a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs @@ -1923,4 +1923,89 @@ public void MultipleApplicationRules() morpher = new Morpher(TraceManager, Language); AssertMorphsEqual(morpher.ParseWord("gigugi"), "44"); } + + // Constraint and Quantifier are both direct subclasses of + // PatternNode (siblings, not related by inheritance). The DTD permits an + // OptionalSegmentSequence (loaded as a Quantifier) anywhere a plain Segment/SimpleContext is + // permitted in a PhoneticSequence, including as the target (Lhs) or replacement (Rhs) of a rewrite + // rule -- not just in its environments. Before the fix, the rule specs assumed every top-level + // Lhs/Rhs child was a Constraint and cast accordingly, so a DTD-legal Quantifier there crashed with + // an unhandled InvalidCastException deep inside Morpher construction. These tests build such rules + // directly (bypassing the XML loader, like the rest of this file) and confirm that constructing a + // Morpher over them now fails fast with a clear, typed CompileException instead. + [Test] + public void QuantifierInTargetIsRejectedWithClearError() + { + var highVowel = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons-") + .Symbol("voc+") + .Symbol("high+") + .Value; + + var rule = new RewriteRule + { + Name = "quantifierInLhs", + // The Lhs is a single top-level Quantifier (an optional segment). The subrule has no Rhs + // (a deletion, 0 children), so the Lhs/Rhs child counts intentionally differ: this steers + // the analysis side to NarrowAnalysisRewriteRuleSpec, which does not cast its Lhs/Rhs + // children eagerly, isolating the crash to SynthesisRewriteRuleSpec, which always validates + // every Lhs child up front regardless of subrule shape. + Lhs = Pattern.New().Annotation(highVowel).Optional.Value, + }; + Allophonic.PhonologicalRules.Add(rule); + rule.Subrules.Add(new RewriteSubrule()); + + CompileException ex = Assert.Throws(() => new Morpher(TraceManager, Language)); + Exception innermost = Innermost(ex); + Assert.That(innermost, Is.TypeOf()); + Assert.That(innermost.Message, Does.Contain("Quantifier")); + } + + [Test] + public void QuantifierInReplacementIsRejectedWithClearError() + { + var highVowel = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons-") + .Symbol("voc+") + .Symbol("high+") + .Value; + var backRnd = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("back+") + .Symbol("round+") + .Value; + + var rule = new RewriteRule + { + Name = "quantifierInRhs", + Lhs = Pattern.New().Annotation(highVowel).Value, + }; + Allophonic.PhonologicalRules.Add(rule); + rule.Subrules.Add( + // Lhs and Rhs child counts match (1 == 1), so this exercises FeatureAnalysisRewriteRuleSpec, + // which casts both the Lhs and Rhs children eagerly. + new RewriteSubrule { Rhs = Pattern.New().Annotation(backRnd).Optional.Value } + ); + + // Before the fix, this already surfaced as a CompileException at the top level (an unrelated, + // pre-existing catch-all in AnalysisStratumRule wraps *any* exception from compiling a + // phonological rule), but its InnerException was a raw, uninformative InvalidCastException. The + // fix replaces that with a clear, typed CompileException naming the unsupported construct. + CompileException ex = Assert.Throws(() => new Morpher(TraceManager, Language)); + Exception innermost = Innermost(ex); + Assert.That(innermost, Is.TypeOf()); + Assert.That(innermost.Message, Does.Contain("Quantifier")); + } + + private static Exception Innermost(Exception ex) + { + while (ex.InnerException != null) + ex = ex.InnerException; + return ex; + } }