ci(pr): add ReSharper InspectCode job (canonical) - #245
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new GitHub Actions job to run JetBrains ReSharper InspectCode on PRs and upload findings as SARIF to GitHub Code Scanning, intended to run in parallel with the existing staged test pipeline.
Changes:
- Introduces an
inspectcodejob gated ondetect-projectsandhas-projects == 'true'. - Builds in Release, runs
jb inspectcodeto produceinspect.sarif, uploads SARIF, and gates the job onlevel=errorfindings.
Chris-Wolfgang
added a commit
that referenced
this pull request
Jul 13, 2026
Ran `jb inspectcode TryPattern.sln --severity=WARNING --no-build` locally against the pr.yaml job's exact command line before merging #245. Initial run: 177 findings (0 errors, 177 warnings). Categorization: - ~154 were noise: R#'s copy of PublicApiAnalyzer (RS0016/RS0037) fires in test/benchmark projects where MSBuild's PublicApiAnalyzer is gated by `<AdditionalFiles Condition="Exists('PublicAPI.Shipped.txt')" />` — but R# doesn't honor that MSBuild gating. Similarly Sonar/Meziantou/ VSTHRD rules already NoWarn'd in tests/csproj don't propagate to R#'s own rule engine. - ~10 were false positives for TFM-conditional patterns (RedundantUsingDirective on `using System;` etc. — needed on net462 / netstandard2.0 / net5-7 where ImplicitUsings is off; needed on the polyfill file where CheckNamespace complains because the namespace is intentionally `System.Diagnostics.CodeAnalysis`). - 2 were real: 1. Result.cs L283 — `[System.Diagnostics.CodeAnalysis.SuppressMessage(...)]` fully-qualified when `using System.Diagnostics.CodeAnalysis;` at L2 already imports the type. Shortened to `[SuppressMessage(...)]`. 2. examples/VB.DotNet462.Example/Program.vb L13 — `File.ReadAllText` is on the BannedSymbols list, but net462 has no async equivalent so the example cannot use one. Added inline VB `<SuppressMessage("ApiDesign", "RS0030:...", Justification:="...")>` on the module so the finding is a deliberate opt-out, not an analyzer miss. TryPattern.sln.DotSettings (new): tunes InspectCode's noise floor per the #208 issue-body requirement. Silences the ~154 categorized-noise rules with a top-level comment explaining each group. RS0030 is intentionally NOT globally silenced so any future banned-API use still lights up (the VB example uses inline suppression instead). Result after tuning: 0 findings. The CI job's `jq` gate on `level=="error"` would trivially pass, and the "success as long as main is clean, so the first PR finding is always actionable" policy now has an actual clean-main baseline.
Merged
3 tasks
Copilot findings on the initial InspectCode job: 1. L249 — misleading comment about --severity=WARNING gating failures. In fact --severity=WARNING controls what InspectCode emits into the SARIF; the actual failure gate is the later `jq` check on `level=="error"`. Rewrote the comment to describe the two mechanisms separately. 2. L272 — missing "Fetch trusted configuration files from main branch" step. This job runs under pull_request_target, and jobs don't share workspaces, so detect-projects' fetch doesn't carry over here. Without it, a malicious PR could ship a permissive .editorconfig / BannedSymbols.txt / .DotSettings that would silence InspectCode findings on the PR's own code. Added the same fetch block as Stage 1 (with the process-substitution + fail-loud-on-copy-failure pattern). 3. L277 — dotnet restore / dotnet build from repo root would try to build examples/CSharp.DotNet462.Example (net462-only, incompatible with Linux) and fail. Replaced the root-level build with Stage 1's filter-Framework-only pattern: build each project one Linux- compatible TFM only. InspectCode analyzes source, not runtime output, so any one Linux TFM per project is enough. The InspectCode step itself (`jb inspectcode "$SLN" --no-build`) is unchanged.
Chris-Wolfgang
force-pushed
the
chore/add-inspectcode
branch
from
July 13, 2026 20:40
578bbd9 to
a72c2ff
Compare
This was referenced Jul 14, 2026
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a JetBrains ReSharper InspectCode job to the PR pipeline as a new parallel job in
pr.yaml. Findings upload to GitHub Code Scanning (Security → Code scanning alerts) and inline-annotate the PR diff.error-severity findings fail the job (via ajqcheck in the Gate step);warning-severity uploads but doesn't fail.This PR is protected-file-only per
protected-file-pr-split. It's paired with #246 — the analyzer noise tuning + source hygiene needed for a clean-main InspectCode baseline. Merge #246 FIRST (normal flow, no bypass), THEN this PR (admin-bypass required).What's in this PR
Only
.github/workflows/pr.yaml— theinspectcodejob definition. Includes:pull_request_targetparallel todetect-projects(needs it) and to Stage 1 / 2 / 3.main(defense-in-depth against a permissive PR-supplied.editorconfig/BannedSymbols.txt/.DotSettings).jb inspectcodeagainst the.slnx/.slnat repo root; SARIF uploads to Code Scanning.level=="error"findings viajq.Expected CI state
Detect .NET Projects❌ — expected, that's the whole point of this PR; touches.github/workflows/*.yaml. Admin-bypass at merge.detect-projects.outputs.has-projects.Verification
Local
jb inspectcode TryPattern.sln --severity=WARNING --no-buildreports 0 findings once #246 merges to main. Verified against the exact command line used in the workflow.Closes #208