Skip to content

Commit 3075ce6

Browse files
committed
Continued migration away from Collation.Default to more accurate alternatives.
1 parent acec49b commit 3075ce6

43 files changed

Lines changed: 239 additions & 208 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ Six scopes, one home each. **Add new state to whichever class matches its true s
7777

7878
## Conventions that fail builds
7979

80-
- **SSS001**: non-public types may not have auto-properties or trivial wrappers over same-type fields. Expose the field directly: `public readonly T Foo = expr;`. Overrides, abstracts, statics, and interface implementations exempt.
80+
- **SSS001**: non-public types may not have auto-properties or trivial wrappers over same-type fields. Expose the field directly: `public readonly T Foo = expr;` (or `static readonly` for static-singleton state). Overrides, abstracts, and interface implementations exempt.
8181
- **SSS002**: a `readonly` field in a non-public-API type whose declared type is a strict supertype of its initializer should be declared as the concrete type. Public types, value-typed initializers (boxing), const fields, and uninitialized fields exempt.
8282
- **SSS003**: `string.ToUpperInvariant()` / `ToLowerInvariant()` whose result is the *governing expression* of a `switch` allocates a temporary string. Use the `Span<char>` overload — `Span<char> buf = stackalloc char[s.Length]; s.AsSpan().ToUpperInvariant(buf)` — and switch on the resulting count.
8383
- **SSS004**: two or more `if`/`else if` branches with conditions of the shape `<sameScrutinee> is <SameType> { <SameProperty>: ... }` should be a single `switch`. The `switch` form fuses isinst + ldfld; the if-chain repeats both per arm.

SqlServerSimulator.Analyzers.Tests/WrapperPropertyAnalyzerTests.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,15 @@ internal sealed class C : B
151151
}
152152
""");
153153

154+
// Static auto-properties on non-public types have the same overhead as
155+
// instance ones — a backing field plus a getter method, no API-stability
156+
// benefit. Convert to `public static readonly int Value = 42;`.
154157
[TestMethod]
155-
public Task StaticProperty_DoesNotReport() =>
158+
public Task StaticAutoProperty_Reports() =>
156159
RunAsync("""
157160
internal static class C
158161
{
159-
public static int Value { get; } = 42;
162+
public static int {|SSS001:Value|} { get; } = 42;
160163
}
161164
""");
162165

SqlServerSimulator.Analyzers/WrapperPropertyAnalyzer.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ namespace SqlServerSimulator.Analyzers;
2424
/// Public types are exempt — there the property provides genuine API-stability
2525
/// flexibility (the rationale behind CA2227 and similar guidance). Overrides and
2626
/// explicit interface implementations are also exempt: their API shape is
27-
/// dictated by the base type or interface and isn't optional.
27+
/// dictated by the base type or interface and isn't optional. Static
28+
/// auto-properties / wrappers are <em>not</em> exempt — they carry the same
29+
/// backing-field + getter-method overhead as instance ones with no compensating
30+
/// API-stability benefit on a non-public type.
2831
/// </para>
2932
/// </remarks>
3033
[DiagnosticAnalyzer(LanguageNames.CSharp)]
@@ -57,7 +60,7 @@ private static void AnalyzePropertyDeclaration(SyntaxNodeAnalysisContext context
5760
if (context.SemanticModel.GetDeclaredSymbol(propertyDecl, context.CancellationToken) is not IPropertySymbol property)
5861
return;
5962

60-
if (property.IsStatic || property.GetMethod is null)
63+
if (property.GetMethod is null)
6164
return;
6265

6366
// Overrides, abstract/extern declarations, explicit interface

SqlServerSimulator.Tests/NameComparisonRegimeTests.cs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -223,25 +223,31 @@ public void CsDatabase_CreateSchemaFullwidthSys_StillRejectedAsReserved()
223223
_ = CsCollation().AssertSqlError("CREATE SCHEMA sys", 2760);
224224
}
225225

226+
/// <summary>
227+
///`SP_EXECUTESQL` doesn't case-equal `sp_executesql` under CS, so
228+
///the dispatch falls through to generic user-proc lookup which
229+
///also misses → Msg 2812 "Could not find stored procedure".
230+
/// </summary>
226231
[TestMethod]
227232
public void CsDatabase_SystemProcSpExecuteSql_UppercaseCase_NotFound()
228-
// `SP_EXECUTESQL` doesn't case-equal `sp_executesql` under CS, so
229-
// the dispatch falls through to generic user-proc lookup which
230-
// also misses → Msg 2812 "Could not find stored procedure".
231233
=> _ = CsCollation().AssertSqlError("EXEC SP_EXECUTESQL N'select 1'", 2812);
232234

235+
/// <summary>
236+
/// Probe-confirmed: sp_executesql width-folds to sp_executesql
237+
/// under CS_AS (IgnoreWidth stays on), so dispatch still routes
238+
/// through the simulator's sp_executesql handler.
239+
/// </summary>
233240
[TestMethod]
234241
public void CsDatabase_SystemProcSpExecuteSql_FullwidthCase_Dispatches()
235-
// Probe-confirmed: sp_executesql width-folds to sp_executesql
236-
// under CS_AS (IgnoreWidth stays on), so dispatch still routes
237-
// through the simulator's sp_executesql handler.
238242
=> AreEqual(1, CsCollation().ExecuteScalar("EXEC sp_executesql N'select 1'"));
239243

244+
/// <summary>
245+
/// hierarchyid:: is the canonical lowercase form; HIERARCHYID
246+
/// doesn't case-equal it under CS, so the static-call dispatch
247+
/// misses and the parser raises a syntax error.
248+
/// </summary>
240249
[TestMethod]
241250
public void CsDatabase_HierarchyIdTypePrefix_UppercaseHIERARCHYID_NotResolved()
242-
// hierarchyid:: is the canonical lowercase form; HIERARCHYID
243-
// doesn't case-equal it under CS, so the static-call dispatch
244-
// misses and the parser raises a syntax error.
245251
=> _ = Throws<Exception>(() => CsCollation().ExecuteScalar(
246252
"SELECT HIERARCHYID::GetRoot()"));
247253

SqlServerSimulator/BuiltInResources.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ private static Dictionary<string, HeapTable> BuildSystemHeapTables()
8686
_ = systypes.Heap.Insert(RowEncoder.EncodeRow(systypes.Schema, values));
8787
}
8888

89-
return new(Collation.Default) { [systypes.Name] = systypes };
89+
return new(BuiltInToken.Comparer) { [systypes.Name] = systypes };
9090
}
9191

9292
private static SqlValue ObjectToSqlValue(object? value, SqlType type) =>
@@ -1096,7 +1096,7 @@ private static Dictionary<string, CatalogView> BuildCatalogViews()
10961096
};
10971097
var serversView = new CatalogView("servers", serversColumns, EnumerateSysServers);
10981098

1099-
return new Dictionary<string, CatalogView>(Collation.Default)
1099+
return new Dictionary<string, CatalogView>(BuiltInToken.Comparer)
11001100
{
11011101
["sys.databases"] = databasesView,
11021102
["sys.servers"] = serversView,
@@ -2116,7 +2116,7 @@ private static IEnumerable<SqlValue[]> EnumerateSysXmlIndexes(Parser.BatchContex
21162116
// Build a quick name→objectId map so secondary indexes can
21172117
// resolve their using_xml_index_id from the recorded
21182118
// UsingPrimaryIndexName.
2119-
var primaryIds = new Dictionary<string, int>(Collation.Default);
2119+
var primaryIds = new Dictionary<string, int>(database.Collation);
21202120
foreach (var ix in table.XmlIndexes)
21212121
{
21222122
if (ix.IsPrimary)

SqlServerSimulator/BuiltInToken.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,20 @@ public static bool EqualsAny(string? value, params ReadOnlySpan<string> options)
8383
/// </summary>
8484
public static int GetHashCode(string value) =>
8585
compareInfo.GetHashCode(value, Options);
86+
87+
/// <summary>
88+
/// Singleton <see cref="IEqualityComparer{T}"/> wrapper for use as a
89+
/// dictionary / hashset comparer. The wire-up routes through the
90+
/// static <see cref="Equals(string?, string?)"/> /
91+
/// <see cref="GetHashCode(string)"/> entry points, so dict-backed
92+
/// state stays semantically identical to ad-hoc compares.
93+
/// </summary>
94+
internal static readonly ComparerImpl Comparer = new();
95+
96+
internal sealed class ComparerImpl : IEqualityComparer<string>
97+
{
98+
public bool Equals(string? x, string? y) => BuiltInToken.Equals(x, y);
99+
100+
public int GetHashCode(string obj) => BuiltInToken.GetHashCode(obj);
101+
}
86102
}

SqlServerSimulator/Parser/Expressions/Grouping.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ internal static bool FindArg(IReadOnlyList<Expression> haystack, Expression argu
4949
var leaf = reference.ReferencedName.Leaf;
5050
foreach (var entry in haystack)
5151
{
52-
if (entry is Reference r && Collation.Default.Equals(r.ReferencedName.Leaf, leaf))
52+
if (entry is Reference r && BuiltInToken.Equals(r.ReferencedName.Leaf, leaf))
5353
return true;
5454
}
5555
return false;

SqlServerSimulator/Parser/Expressions/TypeId.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public override SqlValue Run(RuntimeContext runtime)
5656
// through the schema's TableTypes dict.
5757
foreach (var row in BuiltInResources.SystypesRowData)
5858
{
59-
if (Collation.Default.Equals((string)row[0]!, leafPart))
59+
if (BuiltInToken.Equals((string)row[0]!, leafPart))
6060
return SqlValue.FromInt32(Convert.ToInt32(row[3]!, System.Globalization.CultureInfo.InvariantCulture));
6161
}
6262

SqlServerSimulator/Parser/Selection.Execution.Aggregate.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ SqlValue ResolveByGroupKey(MultiPartName name)
165165
for (var i = 0; i < capturedSet.Length; i++)
166166
{
167167
if (capturedSet[i] is Reference r
168-
&& Collation.Default.Equals(r.Name, name.Leaf))
168+
&& BuiltInToken.Equals(r.Name, name.Leaf))
169169
{
170170
return state.KeyValues[i];
171171
}
@@ -176,7 +176,7 @@ SqlValue ResolveByGroupKey(MultiPartName name)
176176
foreach (var expr in fromClause.AllGroupingExpressions)
177177
{
178178
if (expr is Reference r
179-
&& Collation.Default.Equals(r.Name, name.Leaf))
179+
&& BuiltInToken.Equals(r.Name, name.Leaf))
180180
{
181181
return SqlValue.Null(expr.GetSqlType(resolveColumnType));
182182
}

SqlServerSimulator/Parser/Selection.Execution.OrderBy.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ private static SqlValue[] ComputeOrderKeys(
3939
{
4040
for (var j = 0; j < outputColumnNames.Length; j++)
4141
{
42-
if (Collation.Default.Equals(outputColumnNames[j], name.Leaf))
42+
if (BuiltInToken.Equals(outputColumnNames[j], name.Leaf))
4343
return projected[j];
4444
}
4545
return distinct

0 commit comments

Comments
 (0)