Skip to content

Conversation

@JamieMagee
Copy link
Member

  • 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

@JamieMagee JamieMagee requested a review from a team as a code owner November 26, 2025 00:05
@JamieMagee JamieMagee requested review from RushabhBhansali and removed request for a team November 26, 2025 00:05
@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.9%. Comparing base (fdf9916) to head (1bfb72a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...etection.Detectors/spdx/Spdx22ComponentDetector.cs 77.7% 0 Missing and 6 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JamieMagee JamieMagee requested a review from Copilot November 26, 2025 05:30
Copilot finished reviewing on behalf of JamieMagee November 26, 2025 05:31
Copy link

Copilot AI left a 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/JsonTextReader with JsonDocument.ParseAsync for async JSON parsing
  • Converted property access patterns from nullable ?. operators to explicit TryGetProperty checks
  • Updated exception handling from JsonReaderException to JsonException
Comments suppressed due to low confidence (1)

src/Microsoft.ComponentDetection.Detectors/spdx/Spdx22ComponentDetector.cs:1

  • The JsonDocument returned by ParseAsync should be disposed. In DotNetComponentDetector (line 281), the document is not wrapped in a using statement, which could lead to a memory leak. However, in this file (line 60), you correctly use using var document. Consider checking if DotNetComponentDetector needs the same fix, but this current usage is correct.
#nullable disable

@JamieMagee JamieMagee force-pushed the users/jamagee/go-vcpkg-system.text.json branch 2 times, most recently from b7ebff5 to bd4dcaa Compare November 26, 2025 23:16
@JamieMagee JamieMagee force-pushed the users/jamagee/spdx-system-text-json branch from 56a1717 to b2d4f8e Compare December 1, 2025 15:45
@github-actions
Copy link

github-actions bot commented Dec 1, 2025

👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:

  • The detector detects more or fewer components than before
  • The detector generates different parent/child graph relationships than before
  • The detector generates different devDependencies values than before

If none of the above scenarios apply, feel free to ignore this comment 🙂

Base automatically changed from users/jamagee/go-vcpkg-system.text.json to main December 1, 2025 18:31
Copilot AI review requested due to automatic review settings December 1, 2025 18:45
@JamieMagee JamieMagee force-pushed the users/jamagee/spdx-system-text-json branch from b2d4f8e to ec4fb32 Compare December 1, 2025 18:45
…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`
@JamieMagee JamieMagee force-pushed the users/jamagee/spdx-system-text-json branch from ec4fb32 to 1bfb72a Compare December 1, 2025 18:46
Copilot finished reviewing on behalf of JamieMagee December 1, 2025 18:47
Copy link

Copilot AI left a 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.

@JamieMagee JamieMagee merged commit b369daa into main Dec 1, 2025
27 of 28 checks passed
@JamieMagee JamieMagee deleted the users/jamagee/spdx-system-text-json branch December 1, 2025 19:46
@trirawat131201-cpu
Copy link

0b60165

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.

4 participants