Skip to content

Commit 3264bc2

Browse files
authored
fix: Enforce stricter validation for [GenerateIniFileSection] (#119)
**Changes:** - Added 3 diagnostic descriptors (`BB84SG0011`, `BB84SG0012`, `BB84SG0013`) for nullable, uninitialized, and missing public getter section properties. - Added `ValidateSectionProperties` and `ValidateSectionPropertiesOnType` methods that recursively validate all section-decorated properties. - Removed `NeedsInitialization` from `SectionInfo` and all related null-check/initialization logic from the generated `Read`, `Write`, and `Load` methods. - Removed `BuildNullCheck` helper since it's no longer needed. - Updated test models to conform to the new requirements (non-nullable, initialized, public getter). - Added new tests in `GeneratorDriverTests.cs` to verify that the `IniFileGenerator` reports diagnostics for nullable, uninitialized, and non-public section properties, and does not report errors for valid properties. - Updated `IniFileGeneratorTests.cs` to modify assertions in `ReadShouldHandleEmptyContent` and `ReadShouldNotMatchWhenCaseSensitiveAndCaseDiffers` to ensure proper handling of default values. - Removed the `LoadShouldNotThrowWhenNestedParentIsNull` test as it is no longer relevant.
2 parents 86d0e52 + 08a4f40 commit 3264bc2

5 files changed

Lines changed: 256 additions & 141 deletions

File tree

src/BB84.SourceGenerators/Analyzers/DiagnosticDescriptors.cs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,40 @@ internal static class DiagnosticDescriptors
129129
category: "BB84.SourceGenerators",
130130
defaultSeverity: DiagnosticSeverity.Error,
131131
isEnabledByDefault: true);
132+
133+
/// <summary>
134+
/// Represents a diagnostic error indicating that a property decorated with [GenerateIniFileSection]
135+
/// has a nullable type, which is not supported.
136+
/// </summary>
137+
internal static readonly DiagnosticDescriptor IniFileSectionNullablePropertyDiagnostic = new(
138+
id: "BB84SG0011",
139+
title: "IniFile section property must not be nullable",
140+
messageFormat: "The property '{0}' on class '{1}' decorated with [GenerateIniFileSection] must not be nullable",
141+
category: "BB84.SourceGenerators",
142+
defaultSeverity: DiagnosticSeverity.Error,
143+
isEnabledByDefault: true);
144+
145+
/// <summary>
146+
/// Represents a diagnostic error indicating that a property decorated with [GenerateIniFileSection]
147+
/// is not initialized.
148+
/// </summary>
149+
internal static readonly DiagnosticDescriptor IniFileSectionUninitializedPropertyDiagnostic = new(
150+
id: "BB84SG0012",
151+
title: "IniFile section property must be initialized",
152+
messageFormat: "The property '{0}' on class '{1}' decorated with [GenerateIniFileSection] must be initialized",
153+
category: "BB84.SourceGenerators",
154+
defaultSeverity: DiagnosticSeverity.Error,
155+
isEnabledByDefault: true);
156+
157+
/// <summary>
158+
/// Represents a diagnostic error indicating that a property decorated with [GenerateIniFileSection]
159+
/// does not have a public getter.
160+
/// </summary>
161+
internal static readonly DiagnosticDescriptor IniFileSectionNoPublicGetterDiagnostic = new(
162+
id: "BB84SG0013",
163+
title: "IniFile section property must have a public getter",
164+
messageFormat: "The property '{0}' on class '{1}' decorated with [GenerateIniFileSection] must have a public getter",
165+
category: "BB84.SourceGenerators",
166+
defaultSeverity: DiagnosticSeverity.Error,
167+
isEnabledByDefault: true);
132168
}

src/BB84.SourceGenerators/IniFileGenerator.cs

Lines changed: 82 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
//
44
// This source code is licensed under the MIT license found in the
55
// LICENSE file in the root directory of this source tree.
6+
using BB84.SourceGenerators.Analyzers;
67
using BB84.SourceGenerators.Attributes;
78
using BB84.SourceGenerators.Extensions;
89
using BB84.SourceGenerators.Helpers;
@@ -45,6 +46,9 @@ private void Execute(SourceProductionContext context, (ClassDeclarationSyntax Cl
4546

4647
List<SectionInfo> sections = GetSections(ctx.ClassDeclaration, ctx.SemanticModel, sectionDelimiter, serializeComments);
4748

49+
if (!ValidateSectionProperties(context, ctx.ClassDeclaration, ctx.SemanticModel))
50+
return;
51+
4852
SourceBuilder sb = new();
4953

5054
sb.AppendAutoGeneratedWarning(GeneratorNames.IniFileGeneratorFullName);
@@ -127,33 +131,6 @@ private static void AppendReadMethod(SourceBuilder sb, string className, string
127131
sb.AppendLine($"{elsePrefix}if (string.Equals(currentSection, \"{section.SectionName}\", StringComparison.{stringComparison}))");
128132
sb.OpenBrace();
129133

130-
if (section.NeedsInitialization)
131-
{
132-
sb.AppendLine($"if (result.{section.PropertyPath} == null)");
133-
sb.Indent();
134-
sb.AppendLine($"result.{section.PropertyPath} = new {section.TypeName}();");
135-
sb.Outdent();
136-
sb.AppendLine();
137-
}
138-
else if (section.PropertyPath.Contains("."))
139-
{
140-
// Ensure parent path segments are initialized for nested sections
141-
string[] segments = section.PropertyPath.Split('.');
142-
for (int i = 0; i < segments.Length - 1; i++)
143-
{
144-
string parentPath = string.Join(".", segments, 0, i + 1);
145-
SectionInfo? parentSection = sections.FirstOrDefault(ps => ps.PropertyPath == parentPath);
146-
if (parentSection is not null)
147-
{
148-
sb.AppendLine($"if (result.{parentPath} == null)");
149-
sb.Indent();
150-
sb.AppendLine($"result.{parentPath} = new {parentSection.TypeName}();");
151-
sb.Outdent();
152-
sb.AppendLine();
153-
}
154-
}
155-
}
156-
157134
for (int v = 0; v < section.Values.Count; v++)
158135
{
159136
ValueInfo value = section.Values[v];
@@ -220,10 +197,6 @@ private static void AppendWriteMethod(SourceBuilder sb, string className, List<S
220197
if (s > 0)
221198
sb.AppendLine("sb.AppendLine();");
222199

223-
string nullCheck = BuildNullCheck($"instance.{section.PropertyPath}");
224-
sb.AppendLine($"if ({nullCheck})");
225-
sb.OpenBrace();
226-
227200
if (serializeComments && section.Comment is not null)
228201
sb.AppendLine($"sb.AppendLine(\"; {GeneratorHelpers.EscapeString(section.Comment)}\");");
229202

@@ -236,8 +209,6 @@ private static void AppendWriteMethod(SourceBuilder sb, string className, List<S
236209

237210
sb.AppendLine($"sb.AppendLine(\"{value.KeyName}=\" + {GetToStringExpression($"instance.{section.PropertyPath}.{value.PropertyName}", value)});");
238211
}
239-
240-
sb.CloseBrace();
241212
}
242213

243214
sb.AppendLine();
@@ -318,15 +289,8 @@ private static void AppendLoadMethod(SourceBuilder sb, string className, List<Se
318289

319290
foreach (SectionInfo section in sections)
320291
{
321-
string targetNullCheck = BuildNullCheck($"this.{section.PropertyPath}");
322-
string sourceNullCheck = BuildNullCheck($"other.{section.PropertyPath}");
323-
sb.AppendLine($"if ({targetNullCheck} && {sourceNullCheck})");
324-
sb.OpenBrace();
325-
326292
foreach (ValueInfo value in section.Values)
327293
sb.AppendLine($"{section.PropertyPath}.{value.PropertyName} = other.{section.PropertyPath}.{value.PropertyName};");
328-
329-
sb.CloseBrace();
330294
}
331295

332296
sb.CloseBrace();
@@ -354,7 +318,6 @@ private static List<SectionInfo> GetSections(ClassDeclarationSyntax classDeclara
354318
continue;
355319

356320
string sectionName = GetExplicitNameOrDefault(sectionAttribute, propertySymbol.Name);
357-
bool needsInit = !HasInitializer(classDeclaration, propertySymbol.Name);
358321
string propertyPath = propertySymbol.Name;
359322
string? comment = serializeComments ? GetXmlSummaryComment(propertySymbol) : null;
360323

@@ -366,7 +329,6 @@ private static List<SectionInfo> GetSections(ClassDeclarationSyntax classDeclara
366329
PropertyPath: propertyPath,
367330
SectionName: sectionName,
368331
TypeName: sectionTypeName,
369-
NeedsInitialization: needsInit,
370332
Values: values,
371333
Comment: comment
372334
));
@@ -408,10 +370,9 @@ private static void CollectNestedSections(List<SectionInfo> sections, INamedType
408370
PropertyPath: propertyPath,
409371
SectionName: fullSectionName,
410372
TypeName: sectionTypeName,
411-
NeedsInitialization: false,
412373
Values: values,
413374
Comment: comment
414-
));
375+
));
415376

416377
CollectNestedSections(sections, sectionType, fullSectionName, propertyPath, sectionDelimiter, depth + 1, serializeComments);
417378
}
@@ -488,12 +449,19 @@ private static bool HasInitializer(ClassDeclarationSyntax classDeclaration, stri
488449
{
489450
foreach (MemberDeclarationSyntax member in classDeclaration.Members)
490451
{
491-
if (member is PropertyDeclarationSyntax propSyntax
492-
&& propSyntax.Identifier.Text == propertyName
493-
&& propSyntax.Initializer is not null)
494-
{
452+
if (member is PropertyDeclarationSyntax propSyntax && propSyntax.Identifier.Text == propertyName && propSyntax.Initializer is not null)
453+
return true;
454+
}
455+
456+
return false;
457+
}
458+
459+
private static bool HasInitializer(INamedTypeSymbol typeSymbol, string propertyName)
460+
{
461+
foreach (SyntaxReference syntaxRef in typeSymbol.DeclaringSyntaxReferences)
462+
{
463+
if (syntaxRef.GetSyntax() is ClassDeclarationSyntax classDeclaration && HasInitializer(classDeclaration, propertyName))
495464
return true;
496-
}
497465
}
498466

499467
return false;
@@ -575,7 +543,7 @@ private static string GetToStringExpression(string expression, ValueInfo value)
575543
string emptyFallback = value.IsArray
576544
? $"Array.Empty<{value.CollectionElementTypeName}>()"
577545
: $"Enumerable.Empty<{value.CollectionElementTypeName}>()";
578-
546+
579547
return $"string.Join(\",\", ({expression} ?? {emptyFallback}).Select(e => {elementToString}))";
580548
}
581549

@@ -711,19 +679,74 @@ private static bool GetSerializeComments(ClassDeclarationSyntax classDeclaration
711679
return null;
712680
}
713681

714-
private static string BuildNullCheck(string propertyPath)
682+
private static bool ValidateSectionProperties(SourceProductionContext context, ClassDeclarationSyntax classDeclaration, SemanticModel semanticModel)
715683
{
716-
string[] segments = propertyPath.Split('.');
717-
if (segments.Length <= 2)
718-
return $"{propertyPath} != null";
684+
INamedTypeSymbol? classSymbol = semanticModel.GetDeclaredSymbol(classDeclaration);
685+
686+
if (classSymbol is null)
687+
return false;
688+
689+
bool isValid = true;
690+
ValidateSectionPropertiesOnType(context, classSymbol, classDeclaration, ref isValid);
691+
return isValid;
692+
}
719693

720-
List<string> checks = [];
721-
for (int i = 1; i < segments.Length; i++)
694+
private static void ValidateSectionPropertiesOnType(SourceProductionContext context, INamedTypeSymbol typeSymbol, ClassDeclarationSyntax? classDeclaration, ref bool isValid)
695+
{
696+
foreach (ISymbol member in typeSymbol.GetMembers())
722697
{
723-
checks.Add($"{string.Join(".", segments, 0, i + 1)} != null");
724-
}
698+
if (member is not IPropertySymbol propertySymbol)
699+
continue;
725700

726-
return string.Join(" && ", checks);
701+
AttributeData? sectionAttribute = FindAttribute(propertySymbol, SectionAttributeName);
702+
703+
if (sectionAttribute is null)
704+
continue;
705+
706+
string className = typeSymbol.Name;
707+
Location location = propertySymbol.Locations.FirstOrDefault() ?? Location.None;
708+
709+
// Check nullable
710+
if (propertySymbol.Type.NullableAnnotation == NullableAnnotation.Annotated)
711+
{
712+
context.ReportDiagnostic(Diagnostic.Create(
713+
DiagnosticDescriptors.IniFileSectionNullablePropertyDiagnostic,
714+
location,
715+
propertySymbol.Name,
716+
className));
717+
isValid = false;
718+
}
719+
720+
// Check initialized
721+
bool hasInit = classDeclaration is not null
722+
? HasInitializer(classDeclaration, propertySymbol.Name)
723+
: HasInitializer(typeSymbol, propertySymbol.Name);
724+
725+
if (!hasInit)
726+
{
727+
context.ReportDiagnostic(Diagnostic.Create(
728+
DiagnosticDescriptors.IniFileSectionUninitializedPropertyDiagnostic,
729+
location,
730+
propertySymbol.Name,
731+
className));
732+
isValid = false;
733+
}
734+
735+
// Check public getter
736+
if (propertySymbol.GetMethod is null || propertySymbol.GetMethod.DeclaredAccessibility != Accessibility.Public)
737+
{
738+
context.ReportDiagnostic(Diagnostic.Create(
739+
DiagnosticDescriptors.IniFileSectionNoPublicGetterDiagnostic,
740+
location,
741+
propertySymbol.Name,
742+
className));
743+
isValid = false;
744+
}
745+
746+
// Recursively validate nested section types
747+
if (propertySymbol.Type is INamedTypeSymbol nestedType)
748+
ValidateSectionPropertiesOnType(context, nestedType, null, ref isValid);
749+
}
727750
}
728751

729752
private static AttributeData? FindAttribute(IPropertySymbol propertySymbol, string attributeName)
@@ -774,7 +797,6 @@ private sealed record SectionInfo(
774797
string PropertyPath,
775798
string SectionName,
776799
string TypeName,
777-
bool NeedsInitialization,
778800
List<ValueInfo> Values,
779801
string? Comment = null
780802
);

tests/BB84.SourceGenerators.Tests/EdgeCaseTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,8 @@ public void IniFileReadShouldHandleOnlyComments()
253253
TestIniFile result = TestIniFile.Read(content);
254254

255255
Assert.IsNotNull(result);
256-
Assert.IsNull(result.General);
257-
Assert.IsNull(result.Database);
256+
Assert.IsNotNull(result.General);
257+
Assert.IsNotNull(result.Database);
258258
}
259259

260260
[TestMethod]

0 commit comments

Comments
 (0)