chore(sonar): suppress S3459 false positives on EF backing fields (#243)

[thomasluizon]

What

Resolves the last two SonarCloud csharpsquid:S3459 ("Remove unassigned field ... or set its value") CODE_SMELLs on orbit-api:

• src/Orbit.Domain/Entities/Report.cs — _reviewedAtUtc
• src/Orbit.Domain/Entities/PendingClarification.cs — _resolvedAtUtc

Why these are false positives

Both fields are EF Core relational backing fields, mapped via Property(x => x.Prop).HasField("_field") in the entity configuration. They are written only by EF Core — during materialization from the DB (via reflection) and via ExecuteUpdate(SetProperty(...)) at the SQL layer — never by any CLR code. Sonar's static analysis therefore correctly-but-uselessly sees them as "never assigned."

This is the same reason each field already carries a #pragma warning disable CS0649 suppression for the compiler's equivalent never-assigned warning. The read-only-property-over-backing-field pattern was introduced in #396 to resolve the prior S1144 smells while keeping the EF model byte-identical.

The fix

Extended the existing #pragma warning disable CS0649 on each field to also cover S3459, keeping the URL-linked WHY note (satisfies ORBIT0001). No new suppressions, no attributes to remove later — the pattern already in the file is simply widened by one rule id.

Suppression, not removal: the field cannot be deleted (it is the EF-mapped column store) and must not be dummy-initialized (that would fight EF and/or introduce a different smell). Suppression is the correct resolution for an EF-reflection-written field.

Verification

• dotnet build Orbit.slnx — 0 errors
• dotnet ef migrations has-pending-model-changes — false ("No changes have been made to the model since the last migration"); EF mapping and model are unchanged
• dotnet test Orbit.slnx — green (5301 passed, 0 failed)

Refs thomasluizon/orbit-ui-mobile#243

[claude]

Code Review: PR #399 — chore(sonar): suppress S3459 false positives on EF backing fields

Recommendation: APPROVE

Summary

This is a 4-line, comment-only diff across two files (src/Orbit.Domain/Entities/Report.cs, src/Orbit.Domain/Entities/PendingClarification.cs) that widens two existing #pragma warning disable CS0649 directives to also cover Sonar's S3459 on EF-mapped private backing fields (_reviewedAtUtc, _resolvedAtUtc). No behavior, no logic, no EF mapping change — confirmed by the PR's own dotnet ef migrations has-pending-model-changes → false.

Findings

Critical / High / Medium: None.

Low / Info:

• src/Orbit.Domain/Entities/Report.cs:16 — the updated comment claims _reviewedAtUtc is written "via reflection on materialization and at the SQL layer." A repo-wide grep for ReviewedAtUtc outside migrations/designer files found no ExecuteUpdate/SetProperty writer for this field anywhere in Orbit.Application/Orbit.Infrastructure — unlike its sibling _resolvedAtUtc in PendingClarification.cs, which genuinely is written via ExecuteUpdate (src/Orbit.Infrastructure/Services/PendingClarificationStore.cs:81). The "and at the SQL layer" clause appears copied from the PendingClarification justification without currently applying to Report (there's no admin "review a report" feature wired up yet — the column is always null in production). This doesn't undermine the suppression's validity, just slightly overstates the WHY note. Non-blocking.

Security

Independently corroborated PASS: both suppressed fields are private DateTime? EF backing fields with no C# writer, mutated only via EF Core reflection/ExecuteUpdate and exposed strictly read-only. CS0649/S3459 are genuine false positives here, not masking a real defect. No injection/auth/data-exposure surface touched.

Validation

Build/test checks skipped in this review per CI job scope — covered by separate required checks (Build / Unit Tests / SonarCloud).

What's good

• Correctly widens an existing, already-reviewed suppression pattern (PR #390) rather than inventing a new one — minimal, surgical diff.
• Keeps the mandatory WHY-note-with-URL on both pragma lines.
• PR description is thorough: explains the false-positive mechanism and includes verification evidence (clean build, no pending EF model changes, green test suite).
• Suppression, not workaround-by-deletion — the fields are EF-mapped and genuinely can't be removed or dummy-initialized without fighting EF.

Severity	Count
Critical	0
High	0
Medium	0
Low / Info	1

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #399 — chore(sonar): suppress S3459 false positives on EF backing fields

Recommendation: APPROVE

Summary

Re-review: the diff is unchanged since my prior approval on this PR. It remains a 4-line, comment-only change across two files (src/Orbit.Domain/Entities/Report.cs, src/Orbit.Domain/Entities/PendingClarification.cs) widening two existing #pragma warning disable CS0649 directives to also cover Sonar's S3459 on EF-mapped private backing fields (_reviewedAtUtc, _resolvedAtUtc). No behavior, logic, or EF mapping change.

Findings

Critical / High / Medium: None.

Low / Info:

• src/Orbit.Domain/Entities/Report.cs:16 — the WHY note claims _reviewedAtUtc is written "via reflection on materialization and at the SQL layer." No ExecuteUpdate/SetProperty writer for this field exists anywhere in Orbit.Application/Orbit.Infrastructure (unlike its sibling _resolvedAtUtc, which genuinely is written via ExecuteUpdate in PendingClarificationStore.cs:81). The "and at the SQL layer" clause appears copied from the PendingClarification justification and currently overstates the WHY for Report. Doesn't undermine the suppression's validity — non-blocking, already noted in my prior review.

Security

Independently corroborated PASS (subagent re-verified this run): both pragma pairs bracket only the single backing-field declaration in each file, both fields are confirmed genuinely EF-mapped via HasField in OrbitDbContext.cs:405,569, no other field is newly masked, and there is no authorization/data-exposure/logic change — purely a suppression-comment widening.

Validation

Build/test checks skipped in this review per CI job scope — covered by separate required checks (Build / Unit Tests / SonarCloud), which have already passed (Quality Gate passed, 0 new issues).

What's good

• Minimal, surgical diff that correctly widens an already-reviewed suppression pattern (PR #390) rather than inventing a new one.
• Keeps the mandatory WHY-note-with-URL on both pragma lines.
• Suppression, not workaround-by-deletion — the fields are EF-mapped and genuinely can't be removed or dummy-initialized without fighting EF.

Severity	Count
Critical	0
High	0
Medium	0
Low / Info	1