Skip to content

Commit 93c94b5

Browse files
committed
Refactor: Adopt nullable types and modern C# conventions
Modernized the codebase by adopting nullable reference types (`string?`, `XPathExpression?`, etc.) to improve null safety and align with modern C# practices. Updated null checks to use `is not null` for better readability. Introduced default initialization for properties (e.g., `Schema.Empty`, `NullMatchedNodes.Instance`) and added sentinel values to replace nulls in certain cases. Improved null handling in methods by throwing exceptions when required fields are uninitialized. Refactored the `Reset` method for clarity, reused `StringBuilder` instances to improve memory efficiency, and guarded against nulls with the null-forgiving operator (`!`) where appropriate. Enhanced dictionary merging logic for clarity, added `[MemberNotNull]` attributes to guarantee non-null fields after method execution, and improved conditional logic readability. Refactored validation logic in `Validator` to simplify error handling and replaced nullable `StringBuilder` instances with string variables. Applied general code cleanup, including consistent formatting and spacing, to improve maintainability.
1 parent 243f24c commit 93c94b5

8 files changed

Lines changed: 114 additions & 70 deletions

File tree

src/Schematron/EvaluableExpression.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ namespace Schematron;
1414
/// <progress amount="100" />
1515
public abstract class EvaluableExpression
1616
{
17-
string xpath = null!;
18-
XPathExpression expr = null!;
17+
string? xpath;
18+
XPathExpression? expr;
1919
XmlNamespaceManager? ns;
2020

2121
/// <summary>
@@ -39,7 +39,7 @@ protected void InitializeExpression(string xpathExpression)
3939
expr = Config.DefaultNavigator.Compile(xpathExpression);
4040
ret = expr.ReturnType;
4141

42-
if (ns != null)
42+
if (ns is not null)
4343
expr.SetContext(ns);
4444
}
4545

@@ -48,10 +48,10 @@ protected void InitializeExpression(string xpathExpression)
4848
/// A clone of the expression is always returned, because the compiled
4949
/// expression is not thread-safe for evaluation.
5050
/// </remarks>
51-
public XPathExpression CompiledExpression => expr != null ? expr.Clone() : null!;
51+
public XPathExpression? CompiledExpression => expr?.Clone();
5252

5353
/// <summary>Contains the string version of the expression.</summary>
54-
public string Expression => xpath;
54+
public string Expression => xpath ?? string.Empty;
5555

5656
/// <summary>Contains the string version of the expression.</summary>
5757
public XPathResultType ReturnType => ret;
@@ -62,7 +62,7 @@ protected void InitializeExpression(string xpathExpression)
6262
/// <summary>Sets the manager to use to resolve expression namespaces.</summary>
6363
public void SetContext(XmlNamespaceManager nsManager)
6464
{
65-
if (expr != null)
65+
if (expr is not null)
6666
{
6767
// When the expression contains variable references ($name), .NET requires an
6868
// XsltContext (not just XmlNamespaceManager). Use a load-time stub that satisfies

src/Schematron/EvaluationContextBase.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public abstract class EvaluationContextBase
3737
/// strategy for matching nodes is initialized, depending on the specific
3838
/// implementation of the <see cref="XPathNavigator"/> in use.
3939
/// </remarks>
40-
protected IMatchedNodes? Matched { get; set; }
40+
protected IMatchedNodes Matched { get; set; } = NullMatchedNodes.Instance;
4141

4242
/// <summary>Gets or sets the class to use to format messages.</summary>
4343
/// <remarks>
@@ -66,15 +66,15 @@ public abstract class EvaluationContextBase
6666
public string Phase { get; set; } = string.Empty;
6767

6868
/// <summary>Gets or sets the schema to use for the validation.</summary>
69-
public Schema? Schema { get; set; }
69+
public Schema Schema { get; set; } = Schema.Empty;
7070

7171
/// <remarks>
7272
/// When this property is set, the appropriate <see cref="IMatchedNodes"/>
7373
/// strategy is picked, to perform optimum for various navigator implementations.
7474
/// </remarks>
7575
public XPathNavigator Source
7676
{
77-
get => source!;
77+
get => source ?? throw new InvalidOperationException("Source has not been set.");
7878
set
7979
{
8080
source = value;
@@ -108,6 +108,9 @@ public XPathNavigator Source
108108
/// <remarks>
109109
/// By default, it clears the <see cref="Messages"/> and sets <see cref="HasErrors"/> to false.
110110
/// </remarks>
111-
protected void Reset() => Messages = new StringBuilder();
111+
protected void Reset()
112+
{
113+
Messages.Clear();
114+
HasErrors = false;
115+
}
112116
}
113-

src/Schematron/Formatters/FormatterBase.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ protected static StringBuilder FormatMessage(Test source, XPathNavigator context
104104
sb.Append(msg[offset..name.Index]);
105105

106106
// Does the name element have a path attribute?
107-
if (nameExpr != null)
107+
if (nameExpr is not null)
108108
{
109109
SetExpressionContext(nameExpr, source, ambientCtx);
110110

@@ -122,11 +122,11 @@ protected static StringBuilder FormatMessage(Test source, XPathNavigator context
122122
result = context.Evaluate(nameExpr) as string;
123123
}
124124

125-
if (result != null)
125+
if (result is not null)
126126
sb.Append(result);
127127
}
128128
// Does the value-of element have a select attribute?
129-
else if (selectExpr != null)
129+
else if (selectExpr is not null)
130130
{
131131
SetExpressionContext(selectExpr, source, ambientCtx);
132132

@@ -143,7 +143,7 @@ protected static StringBuilder FormatMessage(Test source, XPathNavigator context
143143
result = context.Evaluate(selectExpr) as string;
144144
}
145145

146-
if (result != null)
146+
if (result is not null)
147147
sb.Append(result);
148148
}
149149
// If there is no path or select expression, there is an empty <name> element.
@@ -159,14 +159,14 @@ protected static StringBuilder FormatMessage(Test source, XPathNavigator context
159159

160160
static void SetExpressionContext(XPathExpression expr, Test source, SchematronXsltContext? ambientCtx)
161161
{
162-
if (ambientCtx != null)
162+
if (ambientCtx is not null)
163163
{
164164
try { expr.SetContext(ambientCtx); return; }
165165
catch (System.Xml.XPath.XPathException) { /* fall through */ }
166166
}
167167

168168
var ns = source.GetContext();
169-
if (ns == null) return;
169+
if (ns is null) return;
170170
try { expr.SetContext(ns); }
171171
catch (System.Xml.XPath.XPathException)
172172
{

src/Schematron/IMatchedNodes.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,13 @@ public interface IMatchedNodes
3535
void Clear();
3636
}
3737

38+
/// <summary>No-op sentinel used as a default before a real matching strategy is selected.</summary>
39+
class NullMatchedNodes : IMatchedNodes
40+
{
41+
public static NullMatchedNodes Instance { get; } = new();
42+
43+
public bool IsMatched(XPathNavigator node) => false;
44+
public void AddMatched(XPathNavigator node) { }
45+
public void Clear() { }
46+
}
47+

src/Schematron/Schema.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ public class Schema
2323
/// <summary>The default Schematron namespace. Kept for backward compatibility; prefer <see cref="IsoNamespace"/>.</summary>
2424
public const string Namespace = LegacyNamespace;
2525

26+
/// <summary>A shared empty schema instance used as a default sentinel.</summary>
27+
public static Schema Empty { get; } = new();
28+
2629
/// <summary>Returns <see langword="true"/> if <paramref name="uri"/> is a recognized Schematron namespace URI.</summary>
2730
public static bool IsSchematronNamespace(string? uri) => uri == IsoNamespace || uri == LegacyNamespace;
2831

@@ -109,6 +112,6 @@ public void Load(XmlReader schema)
109112
public bool IsLibrary { get; set; }
110113

111114
/// <summary />
112-
public XmlNamespaceManager NsManager { get; set; } = null!;
115+
public XmlNamespaceManager NsManager { get; set; } = new(new NameTable());
113116
}
114117

src/Schematron/SchemaLoader.cs

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Collections;
2+
using System.Diagnostics.CodeAnalysis;
23
using System.Xml;
34
using System.Xml.XPath;
45

@@ -14,31 +15,37 @@ namespace Schematron;
1415
public class SchemaLoader(Schema schema)
1516
{
1617
XPathNavigator filenav = null!;
17-
Hashtable? abstracts = null;
18+
Hashtable? abstracts;
1819

1920
// Detected Schematron namespace and the namespace manager derived from the source document.
20-
string schNs = null!;
21-
XmlNamespaceManager mgr = null!;
21+
string? schNs;
22+
XmlNamespaceManager? mgr;
2223

2324
// Instance-level XPath expressions compiled against the detected namespace.
24-
XPathExpression exprSchema = null!;
25-
XPathExpression exprEmbeddedSchema = null!;
26-
XPathExpression exprPhase = null!;
27-
XPathExpression exprPattern = null!;
28-
XPathExpression exprAbstractRule = null!;
29-
XPathExpression exprConcreteRule = null!;
30-
XPathExpression exprRuleExtends = null!;
31-
XPathExpression exprAssert = null!;
32-
XPathExpression exprReport = null!;
33-
XPathExpression exprLet = null!;
34-
XPathExpression exprDiagnostic = null!;
35-
XPathExpression exprParam = null!;
36-
XPathExpression exprLibrary = null!;
37-
XPathExpression exprRulesContainer = null!;
38-
XPathExpression exprGroup = null!;
25+
XPathExpression? exprSchema;
26+
XPathExpression? exprEmbeddedSchema;
27+
XPathExpression? exprPhase;
28+
XPathExpression? exprPattern;
29+
XPathExpression? exprAbstractRule;
30+
XPathExpression? exprConcreteRule;
31+
XPathExpression? exprRuleExtends;
32+
XPathExpression? exprAssert;
33+
XPathExpression? exprReport;
34+
XPathExpression? exprLet;
35+
XPathExpression? exprDiagnostic;
36+
XPathExpression? exprParam;
37+
XPathExpression? exprLibrary;
38+
XPathExpression? exprRulesContainer;
39+
XPathExpression? exprGroup;
3940

4041
/// <summary />
4142
/// <param name="source"></param>
43+
[MemberNotNull(nameof(schNs), nameof(mgr),
44+
nameof(exprSchema), nameof(exprEmbeddedSchema), nameof(exprPhase),
45+
nameof(exprPattern), nameof(exprAbstractRule), nameof(exprConcreteRule),
46+
nameof(exprRuleExtends), nameof(exprAssert), nameof(exprReport),
47+
nameof(exprLet), nameof(exprDiagnostic), nameof(exprParam),
48+
nameof(exprLibrary), nameof(exprRulesContainer), nameof(exprGroup))]
4249
public virtual void LoadSchema(XPathNavigator source)
4350
{
4451
schema.NsManager = new XmlNamespaceManager(source.NameTable);
@@ -84,6 +91,12 @@ public virtual void LoadSchema(XPathNavigator source)
8491
/// Detects the Schematron namespace used in <paramref name="source"/> and compiles all
8592
/// instance-level XPath expressions against that namespace.
8693
/// </summary>
94+
[MemberNotNull(nameof(schNs), nameof(mgr),
95+
nameof(exprSchema), nameof(exprEmbeddedSchema), nameof(exprPhase),
96+
nameof(exprPattern), nameof(exprAbstractRule), nameof(exprConcreteRule),
97+
nameof(exprRuleExtends), nameof(exprAssert), nameof(exprReport),
98+
nameof(exprLet), nameof(exprDiagnostic), nameof(exprParam),
99+
nameof(exprLibrary), nameof(exprRulesContainer), nameof(exprGroup))]
87100
void DetectAndBuildExpressions(XPathNavigator source)
88101
{
89102
schNs = DetectSchematronNamespace(source);
@@ -428,7 +441,7 @@ void LoadExtends(Rule rule, XPathNavigator context)
428441
while (extends.MoveNext())
429442
{
430443
var ruleName = extends.Current.GetAttribute("rule", string.Empty);
431-
if (abstracts != null && abstracts.ContainsKey(ruleName))
444+
if (abstracts?.ContainsKey(ruleName) == true)
432445
rule.Extend((Rule)abstracts[ruleName]!);
433446
else
434447
throw new BadSchemaException("The abstract rule with id=\"" + ruleName + "\" is used but not defined.");

src/Schematron/SyncEvaluationContext.cs

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ public override void Start()
2727
Reset();
2828

2929
// Is there something to evaluate at all?
30-
if (Schema is null || Schema.Patterns.Count == 0)
30+
if (Schema.Patterns.Count == 0)
3131
return;
3232

3333
// If no phase was received, try the default phase defined for the schema.
3434
// If no default phase is defined, all patterns will be tested.
3535
if (Phase == string.Empty)
3636
Phase = Schema.DefaultPhase is { Length: > 0 } phase ? phase : Schematron.Phase.All;
3737

38-
if (Phase != Schematron.Phase.All && Schema.Phases[Phase] == null)
38+
if (Phase != Schematron.Phase.All && Schema.Phases[Phase] is null)
3939
throw new ArgumentException("The specified Phase isn't defined for the current schema.");
4040

4141
if (Evaluate(Schema.Phases[Phase], Messages))
@@ -118,13 +118,14 @@ bool Evaluate(Pattern pattern, StringBuilder output)
118118
// Reset matched nodes, as across patters, nodes can be
119119
// evaluated more than once.
120120
Matched.Clear();
121-
var isGroup = pattern is Group;
122121

123122
foreach (var rule in pattern.Rules)
124123
{
125124
// For groups (ISO Schematron 2025), each rule evaluates independently.
126-
if (isGroup) Matched.Clear();
127-
if (Evaluate(rule, sb, BuildLets(pattern.Lets))) failed = true;
125+
if (pattern is Group)
126+
Matched.Clear();
127+
if (Evaluate(rule, sb, BuildLets(pattern.Lets)))
128+
failed = true;
128129
}
129130
if (failed)
130131
{
@@ -176,7 +177,8 @@ bool Evaluate(Rule rule, StringBuilder output, Dictionary<string, string>? patte
176177
var failed = false;
177178
var sb = new StringBuilder();
178179
Source.MoveToRoot();
179-
var nodes = Source.Select(rule.CompiledExpression);
180+
// Non-abstract rules always have a compiled expression (guarded by IsAbstract check above).
181+
var nodes = Source.Select(rule.CompiledExpression!);
180182
var evaluables = new ArrayList(nodes.Count);
181183

182184
// The navigator doesn't contain line info
@@ -253,7 +255,7 @@ bool Evaluate(Rule rule, StringBuilder output, Dictionary<string, string>? patte
253255
bool EvaluateAssert(Assert assert, XPathNavigator context, StringBuilder output,
254256
Dictionary<string, string>? patternLets = null, LetCollection? ruleLets = null)
255257
{
256-
var expr = PrepareExpression(assert.CompiledExpression, context, patternLets, ruleLets, out var xsltCtx);
258+
var expr = PrepareExpression(assert.CompiledExpression!, context, patternLets, ruleLets, out var xsltCtx);
257259
var eval = context.Evaluate(expr);
258260
var result = true;
259261

@@ -293,7 +295,7 @@ bool EvaluateAssert(Assert assert, XPathNavigator context, StringBuilder output,
293295
bool EvaluateReport(Report report, XPathNavigator context, StringBuilder output,
294296
Dictionary<string, string>? patternLets = null, LetCollection? ruleLets = null)
295297
{
296-
var expr = PrepareExpression(report.CompiledExpression, context, patternLets, ruleLets, out var xsltCtx);
298+
var expr = PrepareExpression(report.CompiledExpression!, context, patternLets, ruleLets, out var xsltCtx);
297299
var eval = context.Evaluate(expr);
298300
var result = false;
299301

@@ -322,13 +324,18 @@ bool EvaluateReport(Report report, XPathNavigator context, StringBuilder output,
322324

323325
Dictionary<string, string> BuildLets(LetCollection? extraLets = null)
324326
{
325-
var d = new Dictionary<string, string>(StringComparer.Ordinal);
327+
var result = new Dictionary<string, string>(StringComparer.Ordinal);
328+
326329
foreach (var let in Schema.Lets)
327-
if (let.Value is not null) d[let.Name] = let.Value;
328-
if (extraLets != null)
330+
if (let.Value is not null)
331+
result[let.Name] = let.Value;
332+
333+
if (extraLets is not null)
329334
foreach (var let in extraLets)
330-
if (let.Value is not null) d[let.Name] = let.Value;
331-
return d;
335+
if (let.Value is not null)
336+
result[let.Name] = let.Value;
337+
338+
return result;
332339
}
333340

334341
// Prepares an XPathExpression to be evaluated with variable support.
@@ -341,14 +348,20 @@ XPathExpression PrepareExpression(
341348
out SchematronXsltContext? xsltCtx)
342349
{
343350
var hasVars = (Schema.Lets.Count + (patternLets?.Count ?? 0) + (ruleLets?.Count ?? 0)) > 0;
344-
if (!hasVars) { xsltCtx = null; return expr; }
351+
if (!hasVars)
352+
{
353+
xsltCtx = null;
354+
return expr;
355+
}
345356

346357
var vars = new Dictionary<string, string>(StringComparer.Ordinal);
347358
foreach (var let in Schema.Lets)
348359
if (let.Value is not null) vars[let.Name] = let.Value;
349-
if (patternLets != null)
360+
361+
if (patternLets is not null)
350362
foreach (var kv in patternLets) vars[kv.Key] = kv.Value;
351-
if (ruleLets != null)
363+
364+
if (ruleLets is not null)
352365
foreach (var let in ruleLets)
353366
if (let.Value is not null) vars[let.Name] = let.Value;
354367

0 commit comments

Comments
 (0)