-
Notifications
You must be signed in to change notification settings - Fork 114
Migrate Spdx22ComponentDetector from Newtonsoft.Json to System.Text.json
#1571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1571 +/- ##
=====================================
Coverage 89.9% 89.9%
=====================================
Files 428 428
Lines 36557 36561 +4
Branches 2270 2270
=====================================
+ Hits 32887 32896 +9
+ Misses 3207 3204 -3
+ Partials 463 461 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates the Spdx22ComponentDetector from Newtonsoft.Json to System.Text.Json, aligning with modern .NET standards. The detector parses SPDX 2.2 SBOM files in JSON format to extract component metadata.
Key changes:
- Replaced
JObject/JsonTextReaderwithJsonDocument.ParseAsyncfor async JSON parsing - Converted property access patterns from nullable
?.operators to explicitTryGetPropertychecks - Updated exception handling from
JsonReaderExceptiontoJsonException
Comments suppressed due to low confidence (1)
src/Microsoft.ComponentDetection.Detectors/spdx/Spdx22ComponentDetector.cs:1
- The
JsonDocumentreturned byParseAsyncshould be disposed. In DotNetComponentDetector (line 281), the document is not wrapped in ausingstatement, which could lead to a memory leak. However, in this file (line 60), you correctly useusing var document. Consider checking if DotNetComponentDetector needs the same fix, but this current usage is correct.
#nullable disable
src/Microsoft.ComponentDetection.Detectors/spdx/Spdx22ComponentDetector.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Detectors/spdx/Spdx22ComponentDetector.cs
Outdated
Show resolved
Hide resolved
b7ebff5 to
bd4dcaa
Compare
56a1717 to
b2d4f8e
Compare
|
👋 Hi! It looks like you modified some files in the
If none of the above scenarios apply, feel free to ignore this comment 🙂 |
b2d4f8e to
ec4fb32
Compare
…ext.Json` - Replace `JObject`/`JsonTextReader` with `JsonDocument.ParseAsync` - Convert `JObject` property access to `JsonElement.TryGetProperty` pattern - Make `OnFileFoundAsync` properly `async` - Update exception handling from `JsonReaderException` to `JsonException`
ec4fb32 to
1bfb72a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
JObject/JsonTextReaderwithJsonDocument.ParseAsyncJObjectproperty access toJsonElement.TryGetPropertypatternOnFileFoundAsyncproperlyasyncJsonReaderExceptiontoJsonException