Sourcegen refactoring for better compile-time XML parsing & hardening…#8
Conversation
… against malformed input as well.
…Empty, numeric names)
…ew edge cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the source generator’s XML ingestion away from XmlSerializer to XDocument-based parsing, adds “safe” identifiers derived from XML names, and introduces additional diagnostics/testing intended to harden generation against malformed or hostile input.
Changes:
- Replace
XmlSerializerparsing withXDocument+ a newXDocumentParserfor nodes and types metadata. - Introduce
SafeNamefields across parsed model types and use them in generated identifiers. - Add new generator diagnostic
ADBUS002for invalid enum values in type metadata, plus extensive new/updated tests.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Avalonia.DBus.SourceGen.Tests/XDocumentParserTests.cs | Adds unit tests covering new XDocumentParser behavior and name sanitization expectations. |
| tests/Avalonia.DBus.SourceGen.Tests/TestAnalyzerConfigOptionsProvider.cs | Expands test options provider to support per-additional-text generator modes. |
| tests/Avalonia.DBus.SourceGen.Tests/StructGenerationTests.cs | Removes tests that validated the old XmlSerializer-based deserialization path. |
| tests/Avalonia.DBus.SourceGen.Tests/SanitizationTests.cs | Adds generator-level tests for identifier sanitization and codegen hardening scenarios. |
| tests/Avalonia.DBus.SourceGen.Tests/GeneratorTestHelper.cs | Adds overload to run the generator with multiple AdditionalText inputs (e.g., imported types XML). |
| tests/Avalonia.DBus.SourceGen.Tests/ErrorReportingTests.cs | Updates error-reporting tests after parsing refactor and diagnostic restructuring. |
| tests/Avalonia.DBus.SourceGen.Tests/DiagnosticEmissionTests.cs | Adds targeted tests for ADBUS001/ADBUS002 emission and behavior. |
| src/Avalonia.DBus.SourceGen/XDocumentParser.cs | New parser that maps XDocument into the generator’s model types and computes SafeName. |
| src/Avalonia.DBus.SourceGen/GlobalUsings.cs | Switches global XML using from serialization to LINQ-to-XML. |
| src/Avalonia.DBus.SourceGen/* (model classes) | Removes XML serialization attributes and adds SafeName properties used by codegen. |
| src/Avalonia.DBus.SourceGen/DBusSourceGenerator.cs | Uses XDocument parsing, strengthens XML reader settings, reports ADBUS002 from alias generation, and switches some identifiers to SafeName. |
| src/Avalonia.DBus.SourceGen/DBusSourceGenerator.TypeAliases.cs | Parses enum values more safely and emits ADBUS002 for non-numeric values. |
| src/Avalonia.DBus.SourceGen/DBusSourceGenerator.Parsing.cs | Strengthens SanitizeIdentifier and introduces MakeSafeIdentifier for consistent safe naming. |
| src/Avalonia.DBus.SourceGen/DBusSourceGenerator.Handler.cs | Shifts generated identifiers toward SafeName and adjusts emitted error message formatting. |
| src/Avalonia.DBus.SourceGen/Avalonia.DBus.SourceGen.csproj | Adds InternalsVisibleTo for the new internal parser tests. |
| src/Avalonia.DBus.SourceGen/AnalyzerReleases.Unshipped.md | Registers the new ADBUS002 rule. |
Comments suppressed due to low confidence (1)
src/Avalonia.DBus.SourceGen/DBusSourceGenerator.Handler.cs:237
- These identifier builders assume
SafeNameis non-null (e.g.,I{dBusInterface.SafeName},{method.SafeName}Async,property.SafeName!). Since the parser can produce nullSafeNamewhen the corresponding XMLnameattribute is missing, generation can produce invalid identifiers or hit null dereferences later (e.g.,method.Name!). It would be safer to skip/diagnose methods/properties with missing names before codegen, or to fall back to a deterministic placeholder whenSafeNameis null.
private static string GetHandlerInterfaceIdentifier(DBusInterface dBusInterface) => $"I{dBusInterface.SafeName}";
private static string GetHandlerDispatcherIdentifier(DBusInterface dBusInterface) => $"{dBusInterface.SafeName}Dispatcher";
private static string GetHandlerRegistrationHelperIdentifier(DBusInterface dBusInterface) => $"{dBusInterface.SafeName}HandlerMetadata";
private static string GetInterfaceMethodIdentifier(DBusMethod method)
=> $"{method.SafeName}Async";
private static string GetPropertyIdentifier(DBusProperty property)
=> property.SafeName!;
private static string GetParameterIdentifier(DBusValue argument, int index)
=> argument.Name is not null
? SanitizeIdentifier(Camelize(argument.Name.AsSpan()))
: $"arg{index}";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- XDocumentParser: trim and filter empty ImportTypes values - DBusSourceGenerator: use SafeName for hint names (avoids CS8785), skip interfaces with null SafeName instead of throwing on null Name - TypeAliases: wrap negative values in unchecked cast for uint/ulong enums - TestAnalyzerConfigOptionsProvider: copy modeByPath with OrdinalIgnoreCase - SanitizationTests: update special-char interface test to assert success now that hint names use SafeName
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…2 assertion - Filter methods/properties with null Name or SafeName before codegen in BuildHandlerSource to prevent null-dereference on malformed XML - Add null Name guards in EmitWriteIntrospectionXml for property/method/signal loops - Assert ADBUS002 is emitted in NonNumericEnumValue_ProducesWarning test
… against malformed input as well.