Fix MA0070 obsolete attribute warning#829
Conversation
Provided explanations for empty `[Obsolete]` attributes across `ChangeRequest.cs`, `OslcClient.cs` and `OslcQuery.cs` inside OSLC4Net SDK as required by the `Meziantou.Analyzer` MA0070 rule, pointing developers towards current usages instead of obsolete methods. Co-authored-by: berezovskyi <64734+berezovskyi@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
WalkthroughThe PR updates obsolete attribute messages across SDK APIs, replacing generic deprecation markers with explicit replacement text for RDF type helpers, query accessors, and ChangesPublic API deprecation message updates
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #829 +/- ##
=======================================
Coverage 51.60% 51.60%
=======================================
Files 178 178
Lines 10586 10586
Branches 1115 1115
=======================================
Hits 5463 5463
Misses 4848 4848
Partials 275 275 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Coverage OverviewLanguages: C# C# / dotnet/coberturaThe overall coverage remains at 56%, unchanged from the branch. Code Coverage is in Public Preview. Learn more and provide us with your feedback. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
OSLC4Net_SDK/OSLC4Net.Client/Oslc/OslcClient.cs (3)
546-546: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMisleading obsolete messages:
UpdateResourceoverloads should direct toUpdateResourceRawAsync.These synchronous methods block on
.Result(lines 581, 635). The actual replacements areUpdateResourceRawAsync(line 491). The currentHttpClientFactorymessage doesn't guide callers to the correct API. Consider"Use UpdateResourceRawAsync instead".Also applies to: 560-560, 613-613
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OSLC4Net_SDK/OSLC4Net.Client/Oslc/OslcClient.cs` at line 546, The obsolete message on the UpdateResource overloads is misleading because it points callers to HttpClientFactory instead of the actual replacement API. Update the Obsolete attributes on the affected UpdateResource methods in OslcClient so they consistently direct users to UpdateResourceRawAsync, matching the async implementation that should replace these synchronous .Result-based calls.
303-303: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMisleading obsolete message:
DeleteResourceshould direct toDeleteResourceAsync.The current message suggests
HttpClientFactoryas the alternative, but the actual supported replacement isDeleteResourceAsync(Uri, CancellationToken?)(line 355). The sync method blocks on.Result(line 314), which is the real deprecation reason. Consider a message like"Use DeleteResourceAsync instead".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OSLC4Net_SDK/OSLC4Net.Client/Oslc/OslcClient.cs` at line 303, The Obsolete message on DeleteResource in OslcClient is misleading because it points to HttpClientFactory instead of the supported replacement. Update the attribute on DeleteResource to reference DeleteResourceAsync(Uri, CancellationToken?) so callers are directed to the async API that actually replaces the sync wrapper, and keep the message aligned with the method’s deprecation reason rather than HttpClient usage.
867-867: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMisleading obsolete message:
AcceptAllServerCertificatesshould mention certificate validation alternative.This static callback is about bypassing TLS validation. The
HttpClientFactorymessage doesn't match. Callers should instead configureServerCertificateCustomValidationCallbackonHttpClientHandlervia theirHttpClientFactorysetup, or useDangerousAcceptAnyServerCertificateValidatorexplicitly. Consider a message like"Configure certificate validation via HttpClientHandler.ServerCertificateCustomValidationCallback or use DangerousAcceptAnyServerCertificateValidator".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OSLC4Net_SDK/OSLC4Net.Client/Oslc/OslcClient.cs` at line 867, The obsolete message on AcceptAllServerCertificates is misleading because it points to HttpClientFactory instead of explaining the certificate-validation alternative. Update the [Obsolete] text on AcceptAllServerCertificates to direct callers toward HttpClientHandler.ServerCertificateCustomValidationCallback via their HttpClientFactory setup, or to DangerousAcceptAnyServerCertificateValidator, so the deprecation guidance matches the TLS-bypass behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@OSLC4Net_SDK/OSLC4Net.ChangeManagement/ChangeRequest.cs`:
- Line 1106: The obsolete annotation on SetRdfTypes has the wrong replacement
guidance and currently points callers to GetTypes(), which is misleading for a
setter. Update the obsolete message on SetRdfTypes in ChangeRequest to direct
users to SetTypes() or .Types instead, matching the corresponding ChangeRequest
client API and keeping the setter replacement consistent.
In `@OSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/ChangeRequest.cs`:
- Line 123: The deprecation state for AddRdfType is inconsistent between the
client and server ChangeRequest implementations. Update the server-side
ChangeRequest AddRdfType method in OSLC4Net.ChangeManagement so it matches the
client-side ChangeRequest behavior by adding the same Obsolete annotation and
message used by the corresponding AddRdfType/GetTypes members, keeping the API
surface aligned across both assemblies.
---
Nitpick comments:
In `@OSLC4Net_SDK/OSLC4Net.Client/Oslc/OslcClient.cs`:
- Line 546: The obsolete message on the UpdateResource overloads is misleading
because it points callers to HttpClientFactory instead of the actual replacement
API. Update the Obsolete attributes on the affected UpdateResource methods in
OslcClient so they consistently direct users to UpdateResourceRawAsync, matching
the async implementation that should replace these synchronous .Result-based
calls.
- Line 303: The Obsolete message on DeleteResource in OslcClient is misleading
because it points to HttpClientFactory instead of the supported replacement.
Update the attribute on DeleteResource to reference DeleteResourceAsync(Uri,
CancellationToken?) so callers are directed to the async API that actually
replaces the sync wrapper, and keep the message aligned with the method’s
deprecation reason rather than HttpClient usage.
- Line 867: The obsolete message on AcceptAllServerCertificates is misleading
because it points to HttpClientFactory instead of explaining the
certificate-validation alternative. Update the [Obsolete] text on
AcceptAllServerCertificates to direct callers toward
HttpClientHandler.ServerCertificateCustomValidationCallback via their
HttpClientFactory setup, or to DangerousAcceptAnyServerCertificateValidator, so
the deprecation guidance matches the TLS-bypass behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 679b9ad3-3e05-43f9-8580-e4aaba357f7d
📒 Files selected for processing (4)
OSLC4Net_SDK/OSLC4Net.ChangeManagement/ChangeRequest.csOSLC4Net_SDK/OSLC4Net.Client/Oslc/OslcClient.csOSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/ChangeRequest.csOSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/OslcQuery.cs
| } | ||
|
|
||
| [Obsolete] | ||
| [Obsolete("Use GetTypes() or .Types instead")] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inconsistent obsolete message on SetRdfTypes.
The setter directs callers to GetTypes(), which is misleading. It should read "Use SetTypes() or .Types instead" to match the client-side ChangeRequest.cs (OSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/ChangeRequest.cs:696) and correctly guide callers to the replacement setter API.
- [Obsolete("Use GetTypes() or .Types instead")]
+ [Obsolete("Use SetTypes() or .Types instead")]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [Obsolete("Use GetTypes() or .Types instead")] | |
| [Obsolete("Use SetTypes() or .Types instead")] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OSLC4Net_SDK/OSLC4Net.ChangeManagement/ChangeRequest.cs` at line 1106, The
obsolete annotation on SetRdfTypes has the wrong replacement guidance and
currently points callers to GetTypes(), which is misleading for a setter. Update
the obsolete message on SetRdfTypes in ChangeRequest to direct users to
SetTypes() or .Types instead, matching the corresponding ChangeRequest client
API and keeping the setter replacement consistent.
| } | ||
|
|
||
| [Obsolete] | ||
| [Obsolete("Use GetTypes() or .Types instead")] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cross-file inconsistency: AddRdfType obsolete marking differs between ChangeRequest implementations.
This client-side AddRdfType is now obsolete, but the server-side ChangeRequest.cs (OSLC4Net_SDK/OSLC4Net.ChangeManagement/ChangeRequest.cs:87-90) has no [Obsolete] on its AddRdfType method. Unify the deprecation state across both implementations to avoid confusing consumers of either assembly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/ChangeRequest.cs` at line 123,
The deprecation state for AddRdfType is inconsistent between the client and
server ChangeRequest implementations. Update the server-side ChangeRequest
AddRdfType method in OSLC4Net.ChangeManagement so it matches the client-side
ChangeRequest behavior by adding the same Obsolete annotation and message used
by the corresponding AddRdfType/GetTypes members, keeping the API surface
aligned across both assemblies.
Provided explanations for empty
[Obsolete]attributes acrossChangeRequest.cs,OslcClient.csandOslcQuery.csinside OSLC4Net SDK as required by theMeziantou.AnalyzerMA0070 rule, pointing developers towards current usages instead of obsolete methods.PR created automatically by Jules for task 8407036304385018379 started by @berezovskyi
Summary by CodeRabbit