Skip to content

Improve test naming, add comprehensive documentation, and introduce JsonDateTimeFormatAttribute#1

Merged
ScarletKuro merged 9 commits intomasterfrom
copilot/improve-naming-and-update-readme
Jan 15, 2026
Merged

Improve test naming, add comprehensive documentation, and introduce JsonDateTimeFormatAttribute#1
ScarletKuro merged 9 commits intomasterfrom
copilot/improve-naming-and-update-readme

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 15, 2026

Task: Improve test naming, update documentation, and eliminate SYSLIB1223 warnings

Completed Work

✅ Test Improvements

  1. Improved test method names - Changed from verbose names to concise, descriptive names focusing on key aspects:

    • Removed redundant "ProducesFormattedJsonString" suffix
    • Focused on: reflection vs source generator, attribute vs converter, initialization method
    • Examples: ReflectionBased_DateTime_WithAttribute, SourceGenerator_CompleteModel_WithFormatConverter, SourceGenerator_WithResolver_WithAttribute_UsingOptions
  2. Renamed test model classes for better clarity:

    • TestModelJsonDateTimeConverterReflectionBasedModel
    • TestModelSourceGeneratorSourceGeneratorWithConverterModel
    • TestModelSourceGeneratorAttributesSourceGeneratorWithResolverAttributeModel
    • Added SourceGeneratorWithResolverFormatModel (uses new attribute, no warnings)
  3. Added XML documentation to all test model classes explaining their purpose and usage scenarios

  4. Moved resolver tests to correct location - Moved SourceGenerator_WithResolver_WithAttribute_UsingOptions and SourceGenerator_WithResolver_WithAttribute_UsingContext from JsonDateTimeFormatConverterTests.cs to JsonDateTimeConverterAttributeTests.cs since they test attributes, not converters

  5. Split test contexts - Created separate ConverterModelJsonSerializerContext and ResolverModelJsonSerializerContext for better organization

  6. Added comprehensive test coverage - Added 3 new tests for JsonDateTimeFormatAttribute (16 total tests)

✅ New Feature: JsonDateTimeFormatAttribute

  1. Created new JsonDateTimeFormatAttribute - A clean attribute that derives from Attribute (not JsonConverterAttribute) specifically for use with .NET 9+ source generators and DateTimeConverterResolver
  2. Eliminates SYSLIB1223 warnings - The new attribute produces zero warnings when used with source generators, unlike JsonDateTimeConverterAttribute which produces 4 warnings
  3. Updated DateTimeConverterResolver - Now intelligently supports both JsonDateTimeFormatAttribute (recommended, no warnings) and JsonDateTimeConverterAttribute (backward compatibility, has warnings)
  4. Dual approach - Users can choose:
    • Warning-free: JsonDateTimeFormatAttribute for new code
    • Backward compatible: JsonDateTimeConverterAttribute for existing code or reflection-based scenarios

✅ Documentation

  1. Completely reconstructed README.md with:

    • Professional structure with table of contents
    • Four comprehensive usage scenarios (added new warning-free approach)
    • Comparison table for when to use each approach
    • Fixed accuracy: Clarified that JsonPropertyInfo.AttributeProvider exists in .NET 7-8, but source generators don't populate it until .NET 9+ (with references to GitHub issues runtime#100095 and runtime#102078)
    • Detailed documentation of both attributes with pros/cons
    • New "SYSLIB1223 Warning" section in quirks explaining the issue and solution
    • Quick start guide with both reflection and source generator examples
  2. Created AGENT.md with:

    • Package overview and architecture
    • Complete repository structure (updated with new files)
    • Build and test instructions
    • Development workflow
    • Updated testing strategy to cover four distinct usage patterns
    • Troubleshooting guide
    • CI/CD information

Verification

  • ✅ All 16 tests pass successfully
  • ✅ Build succeeds for all target frameworks (.NET 6, 7, 8, 9, 10)
  • SourceGeneratorWithResolverFormatModel: 0 SYSLIB1223 warnings (uses new JsonDateTimeFormatAttribute)
  • SourceGeneratorWithResolverAttributeModel: 4 SYSLIB1223 warnings (uses old JsonDateTimeConverterAttribute, kept for backward compatibility testing)
  • ✅ Tests properly organized by what they test (attributes vs converters)
  • ✅ Documentation accuracy improved with GitHub issue references

Summary

This PR provides a complete solution for warning-free source generator usage while maintaining backward compatibility. Users can now choose JsonDateTimeFormatAttribute for clean, modern development or continue using JsonDateTimeConverterAttribute for existing code.

Original prompt

Improve the naming of tests, classes, etc in the Scarlet.System.Text.Json.DateTimeConverter.Tests

Also in this commit fa78a97

I added DateTimeConverterResolver for NET9 since now the System.Text.Json source generator populates JsonPropertyInfo.AttributeProvider and we can use the contract customization.
Please update the README.md. add info about it with examples, and how to use.
Actually fully reconstruct it so it would be clean and professional with all the usages examples, quirkcs, explanation and the available options
depending on the tfm and whatever you need sourcegenerator or not.
Also add AGENT.MD on for this repo about this package, how to test and run it, or whatever is required there


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits January 15, 2026 18:03
Co-authored-by: ScarletKuro <19953225+ScarletKuro@users.noreply.github.com>
Co-authored-by: ScarletKuro <19953225+ScarletKuro@users.noreply.github.com>
Co-authored-by: ScarletKuro <19953225+ScarletKuro@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve naming conventions and update documentation Improve test naming and add comprehensive documentation Jan 15, 2026
Copilot AI requested a review from ScarletKuro January 15, 2026 18:18
@ScarletKuro
Copy link
Copy Markdown
Owner

ScarletKuro commented Jan 15, 2026

@copilot

That line you added: DateTimeConverterResolver only works on .NET 9+ because it relies on JsonPropertyInfo.AttributeProvider, which was added in .NET 9.

Is not very accurate, the JsonPropertyInfo.AttributeProvider exist in net 7-8, the problem is that when using the source-generator in this .net versions, the AttributeProvider is not populated when using source generator, and such support was added in .net9+, for more context see this issue dotnet/runtime#100095 and dotnet/runtime#102078 and the implementation dotnet/runtime#100173

Also add unit test if I'm missing some cases. And seems SourceGenerator_WithResolver_WithAttribute_UsingContext and SourceGenerator_WithResolver_WithAttribute_UsingOptions are in the wrong place, because those are using attributes, not converter but placed in test file for converters? So it should be moved in appropriate place.

…t file

Co-authored-by: ScarletKuro <19953225+ScarletKuro@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 15, 2026

@copilot

That line you added: DateTimeConverterResolver only works on .NET 9+ because it relies on JsonPropertyInfo.AttributeProvider, which was added in .NET 9.
...

Fixed in commit ed07f04:

  1. Corrected AttributeProvider documentation - Updated README.md and AGENT.md to clarify that JsonPropertyInfo.AttributeProvider exists in .NET 7-8, but source generators don't populate it until .NET 9+. Added references to runtime#100095 and runtime#102078.

  2. Moved resolver tests - Moved SourceGenerator_WithResolver_WithAttribute_UsingOptions and SourceGenerator_WithResolver_WithAttribute_UsingContext from JsonDateTimeFormatConverterTests.cs to JsonDateTimeConverterAttributeTests.cs since they test attributes, not converters.

  3. Added missing test - Added SourceGenerator_WithResolver_WithAttribute_WithNullValues_UsingOptions to test null value handling with the resolver.

All 15 tests now pass.

@ScarletKuro
Copy link
Copy Markdown
Owner

@copilot we might need to create a new dedicated attribute for DateTimeConverterResolver that uses just : Attribute
because right now we get this warnings:

  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(10,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(13,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(16,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(19,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG]   Scarlet.System.Text.Json.DateTimeConverter.Tests -> /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/bin/Release/net10.0/Scarlet.System.Text.Json.DateTimeConverter.Tests.dll
  18:36:09 [DBG] 
  18:36:09 [DBG] Build succeeded.
  18:36:09 [DBG] 
  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(10,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(13,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(16,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG] /home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Model/SourceGeneratorWithResolverModel.cs(19,6): warning SYSLIB1223: The custom attribute 'Scarlet.System.Text.Json.DateTimeConverter.JsonDateTimeConverterAttribute' deriving from JsonConverterAttribute is not supported by the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1223) [/home/runner/work/Scarlet.System.Text.Json.DateTimeConverter/Scarlet.System.Text.Json.DateTimeConverter/src/Scarlet.System.Text.Json.DateTimeConverter.Tests/Scarlet.System.Text.Json.DateTimeConverter.Tests.csproj]
  18:36:09 [DBG]     4 Warning(s)

Because JsonConverterAttribute contains CreateConverter which on it's own is not supported with sourcegenerator without the DateTimeConverterResolver and since we combined all in one the analyzer complains which is not good for the end user as it might confuse them.
We need to update the code, including the tests, readme.md. agent.md and maybe I would split the TestModelSourceGeneratorJsonSerializerContext into two different version for SourceGeneratorModel and SourceGeneratorWithResolverModel (maybe the names should also better reflect whatever it uses custom attribute or [JsonConverter(XXX)))]?

… source generators

Co-authored-by: ScarletKuro <19953225+ScarletKuro@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 15, 2026

@copilot we might need to create a new dedicated attribute for DateTimeConverterResolver that uses just : Attribute
because right now we get this warnings:

...

Fixed in commit 3b44e3e:

Created JsonDateTimeFormatAttribute - A new attribute that derives from Attribute (not JsonConverterAttribute) specifically for use with source generators and DateTimeConverterResolver. This eliminates all SYSLIB1223 warnings.

Key changes:

  1. New attribute: JsonDateTimeFormatAttribute for warning-free source generator usage
  2. Updated DateTimeConverterResolver to support both attributes (new one preferred, old one for backward compatibility)
  3. Split test contexts: ConverterModelJsonSerializerContext and ResolverModelJsonSerializerContext
  4. Renamed models for clarity:
    • SourceGeneratorModelSourceGeneratorWithConverterModel
    • Created SourceGeneratorWithResolverFormatModel (uses new attribute, 0 warnings)
    • Kept SourceGeneratorWithResolverAttributeModel (uses old attribute, 4 warnings for backward compat testing)
  5. Added 3 new tests for the warning-free attribute
  6. Updated README.md and AGENT.md with comprehensive documentation

Result: Users can now choose JsonDateTimeFormatAttribute for clean, warning-free source generator usage, while JsonDateTimeConverterAttribute remains available for reflection-based and backward compatibility scenarios.

Copilot AI changed the title Improve test naming and add comprehensive documentation Improve test naming, add comprehensive documentation, and introduce JsonDateTimeFormatAttribute Jan 15, 2026
@ScarletKuro ScarletKuro marked this pull request as ready for review January 15, 2026 20:12
@ScarletKuro ScarletKuro merged commit 9ec5c7f into master Jan 15, 2026
1 check passed
@ScarletKuro ScarletKuro deleted the copilot/improve-naming-and-update-readme branch January 15, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants