Visitar URL original
PropertyTypeConverter - Disable legacy TypeDescriptor when AOT by snakefoot · Pull Request #6230 · NLog/NLog · GitHub
Skip to content

PropertyTypeConverter - Disable legacy TypeDescriptor when AOT - #6230

Merged
snakefoot merged 1 commit into
NLog:devfrom
snakefoot:TypeConverterAOT2
Aug 11, 2026
Merged

PropertyTypeConverter - Disable legacy TypeDescriptor when AOT#6230
snakefoot merged 1 commit into
NLog:devfrom
snakefoot:TypeConverterAOT2

Conversation

@snakefoot

@snakefoot snakefoot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Wait for NLog v6.2

NLog functionality should not rely on legacy TypeDescriptor. TypeDescriptor should be removed completely with NLog v7

Restore the revert made with #6226

This PR
ProcessPath: 'C:\projects\nlog\Tests\TestTrimPublish\bin\release\net10.0\win-X64\publish\TestTrimPublish.exe' with FileSize: 4773888

NLog v6.1.4
ProcessPath: 'C:\projects\nlog\Tests\TestTrimPublish\bin\release\net10.0\win-X64\publish\TestTrimPublish.exe' with FileSize: 5384192

@snakefoot snakefoot added this to the 6.2 milestone Jul 9, 2026
@snakefoot snakefoot added the breaking behavior change Same API, different result label Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Modifies complex-type detection logic in PropertyTypeConverter by removing a TypeConverterAttribute check, and adds conditional guards in PropertyTypeConverter and PropertyHelper that bypass TypeDescriptor-based string conversion when RuntimeFeature.IsDynamicCodeSupported is false on NETSTANDARD2_1_OR_GREATER or NET targets.

Changes

Dynamic Code Support Guard

Layer / File(s) Summary
Complex type detection change
src/NLog/Config/PropertyTypeConverter.cs		 IsComplexType no longer requires absence of a TypeConverterAttribute; determination now relies solely on value-type, IConvertible, and HasConvertFromStringSupport checks.
TypeConverter conversion guarded by dynamic code support
src/NLog/Config/PropertyTypeConverter.cs, src/NLog/Internal/PropertyHelper.cs		 TryConvertToType and TryTypeConverterConversion add conditional compilation checking RuntimeFeature.IsDynamicCodeSupported, returning false early to skip TypeDescriptor-based conversion when dynamic code isn't supported.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit checks the runtime's might,
"Can code run dynamic, or not tonight?"
If not, we skip the converter's dance,
No TypeDescriptor, no second chance.
Hop, hop, safely through the build! 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title matches the main change: disabling legacy TypeDescriptor behavior for AOT in PropertyTypeConverter.
Description check ✅ Passed The description is clearly related to the change and references removing reliance on TypeDescriptor.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@snakefoot snakefoot added the enhancement Improvement on existing feature label Jul 9, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
14.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@snakefoot
snakefoot merged commit f9d7de4 into NLog:dev Aug 11, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

breaking behavior change Same API, different result enhancement Improvement on existing feature size/S

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

1 participant

@snakefoot