feat: roslyn analyzers - #9727
Conversation
…ad config Violations surfaced by the new deterministic lint layer and DCL.Analyzers (companion PRs), all mechanical: - CharacterPreviewController.UpdateAvatarAsync held 'ref avatarShape' across structural changes (globalWorld.Create, plus the entity creation/destruction hidden inside Promise.Create and ForgetLoading) - writes through a relocated ref are silently lost. All structural work now completes into locals before the ref is taken. Verified on Unity 6000.4.0f1 (GREEN, 219 assemblies). - DCLPlayerPrefs: shutdown warning + cleanup-timing logs migrated to ReportHub (the TODO claiming it was unavailable is stale - Utility references Diagnostics); the editor-only MenuItem log keeps Debug.Log. - SituationalReactionPresenter: Camera.main replaced with injected IExposedCameraData.CinemachineBrain?.OutputCamera, threaded ChatContainer -> ChatPlugin -> ChatPanelPresenter. - All 21 banned '// ReSharper disable CheckNamespace' comments deleted. - 37 dead csc.rsp files next to asmrefs deleted - Unity only honors rsp beside an asmdef (verified against the Bee build graph: zero references); orphaned by the assembly consolidation (#8961). - NRT enabled on the four uncovered assemblies: MarketplaceCredits.API, Character, Quality.RenderFeatures, DCL.Editor (~154 new nullable warnings; 'no-warning-ratchet' label applies if the ReSharper count rises).
Regex rules distilled from CLAUDE.md and the skills, enforced on ADDED lines only (git diff -U0 parsing) so pre-existing violations never block - the same ratchet philosophy as the ReSharper warning count. 19 rules (14 BLOCK, 5 WARN), each cited to its source doc in its message, with a per-line '// lint-ignore: <rule-id>' escape hatch visible to reviewers. grep -E candidate pass + awk exact verification (~0.5s at 140k added lines); broken patterns exit loudly; vendored code excluded by pathspec. Wired twice: the Claude-session Stop hook (fast-fail before the ReSharper pass) and a custom-lint CI job over the PR's merge-base diff with ::error/::warning annotations; merge_group gets a real base; the watchdog gains the job plus an always() fix (its condition could never fire). scripts/lint/tests: fixtures + goldens + selftest.sh run first in CI, so a dead rule or broken engine fails the build instead of passing silently.
Five semantic rules in Unity's csc via the RoslynAnalyzer asset label (same integration as the CodeLess and Arch source-generator DLLs), scoped by placement at Assets/DCL/ to first-party code only: - DCLA001 (error): ref component used after a structural change; reachability-aware (exclusive branches, pre-call args, re-fetch idiom) - DCLA002 (warning): detached UniTask flow without exception handling - DCLA003 (warning): heap allocation in Update/[Query]/[HotPath] bodies - DCLA004 (warning): pooled rental that provably leaks - DCLA005 (error): FFI enum without explicit underlying type Severities pinned in Explorer/.editorconfig (tests scoped to warning). netstandard2.0 / CodeAnalysis 4.3.1; 83 NUnit tests; analyzers CI job feeds the watchdog; scripts/build-analyzers.sh syncs the LFS DLL. Calibrated against two Unity 6000.4.0f1 compiles - every FP class carved out with a regression test; the one true positive (CharacterPreviewController) is fixed in the companion cleanup PR.
🚦 CI Status
Build skipped — no changes detected under Warnings not reduced: 13169 => 13354 — remove at least 186 warnings to merge. Warnings/errors in files changed by this PR (85)All Unity tests passed ✅
|
This comment has been minimized.
This comment has been minimized.
NickKhalow
added
the
no QA needed
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
STEP 1 — Context
Loaded: CLAUDE.md, docs/code-style-guidelines.md, .github/prompts/review-instructions.md. Subsystem docs: N/A (tooling PR, no runtime subsystem touched).
Scope. This PR introduces three new layers of automated code-quality enforcement:
- Roslyn semantic analyzers (
Analyzers/DCL.Analyzers/) — five rules (DCLA001–DCLA005) covering structural-change-after-ref, detached UniTask flows, allocations in system Update, pooled rental leaks, and FFI enum typing. Shipped asExplorer/Assets/DCL/DCL.Analyzers.dll(LFS,RoslynAnalyzerlabel, all runtime platforms disabled). - Shell-based regex linter (
scripts/lint/custom-rules.sh) — deterministic project-rule checks applied only to added lines (ratchet model), covering Debug.Log, ObjectProxy, Camera.main, LINQ in systems, nullable locals, null-forgiving!, and ~15 other CLAUDE.md / skill-sourced patterns. Selftest framework underscripts/lint/tests/. - CI integration (
.github/workflows/test.yml) — newcustom-lintjob (regex rules),analyzersjob (Roslyn test suite), watchdog updates, merge-group event support, diff-range output propagation.
The only runtime code is HotPathAttribute.cs — a 12-line marker attribute with no logic.
Changed files: 32 (+3296 −10). All new except the workflow, .gitignore, CLAUDE.md, editorconfig, code-style-guidelines, and lint-changed.sh.
STEP 2 — Root-cause check: PASS
This PR adds preventive tooling that catches code-quality violations at compile time / CI time. It enforces existing project conventions (CLAUDE.md, code-standards skill, docs/code-style-guidelines.md) automatically rather than relying solely on manual review. This is a root-cause approach — the goal is to prevent violations from entering the codebase at all.
STEP 3 — Design & integration: PASS
No new runtime lifecycle units, systems, or state holders. The design decisions are all tooling/infrastructure:
- Three-layer enforcement is well-layered. Regex rules (fast, deterministic, added-lines-only ratchet) complement Roslyn analyzers (symbol-aware, type-aware) and existing ReSharper inspections (comprehensive but slower). Each layer has a clear strength and they don't duplicate effort. The regex layer explicitly excludes
Analyzers/from its pathspec, and the Roslyn analyzers cover what regex cannot (ref-after-structural-change, UniTask flow analysis, pool leak detection). - LFS-committed DLL is the standard Unity pattern for analyzer distribution. The
.metafile correctly configures it asRoslynAnalyzerwith all runtime platforms disabled — it will only be fed tocsc, never included in builds. - Selftest framework (
scripts/lint/tests/selftest.sh) catches dead rules, engine regressions, and whole-script breakage. CI runs the selftest before every diff lint, so a broken rule fails the build instead of passing silently. - CI job dependency graph is correct:
watchdogneeds[analyzers, custom-lint, lint, test]withalways()so it fires on failures (thealways()is required — without a status-check function, GitHub's implicitsuccess()would prevent the watchdog from running on failures, which is exactly when it should fire). Skipped jobs (e.g.,analyzerswhen noAnalyzers/files changed) haveresult == 'skipped', not'failure', so they don't trigger the watchdog. - Security model for PR-head code execution is adequate:
pull_requesttrigger (fork PRs get read-only tokens, no secrets),persist-credentials: false, job-levelpermissions: contents: read, and timeout limits. Consistent with pre-existing CI jobs that already execute PR-head code. Shell scripts are defensively coded (quoted variables,--with grep, env-var transport to awk, SIGPIPE-safe grep).
Owner search: N/A — no new runtime units to trace lifecycle ownership for.
Observation — stale DLL risk: The analyzers CI job runs dotnet test on the source, not the committed DLL. A developer could modify analyzer source, have tests pass in CI, but ship a stale DLL with old behavior. scripts/build-analyzers.sh is the manual rebuild mechanism. Consider adding a rebuild-and-diff CI step in a follow-up to close this gap.
STEP 4 — Member audit: PASS
The only new public API is HotPathAttribute — a sealed marker attribute with no members beyond the inherited Attribute base. No consumers yet (the attribute is provided for future annotation of hot-path methods outside ECS systems). No properties or accessors to audit.
STEP 5 — Line-level findings
See inline comments for the two P2 findings with suggestion blocks.
Security review: No security issues found. Shell scripts follow best practices (quoted variables, -- with grep, env-var transport to awk, explicit diff format pins). CI job permissions are minimal (contents: read). DLL .meta correctly disables all runtime platforms. NuGet dependencies are well-known Microsoft/NUnit packages.
STEP 6 — Complexity: COMPLEX
Introduces new CI jobs, a Roslyn analyzer infrastructure with five rules and a comprehensive test suite, a shell-based linting framework with selftest, and modifies the build pipeline's job dependency graph and event handling.
STEP 7 — QA assessment: QA_REQUIRED: NO
All changes are CI/CD workflows, build-time Roslyn analyzers (DLL with all runtime platforms disabled), shell scripts, documentation, and .editorconfig configuration. The only runtime code (HotPathAttribute.cs) is a zero-logic marker attribute. No user-facing behavior is affected.
STEP 8 — Non-blocking warnings
No Explorer/Assets/Scenes/Main.unity in the changed files. No warnings to emit.
STEP 9 — Verdict
Both findings are P2 (minor). No P0 or P1 issues. The analyzers are well-designed with comprehensive test coverage (all five follow Roslyn best practices: CompilationStartAction, concurrent execution, proper symbol resolution, short-circuit when anchor types are absent). The shell linter is defensively coded (grep+awk two-pass, pattern validation with exit 3, selftest framework, explicit diff format pins). The CI integration is correct.
REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Introduces Roslyn analyzer infrastructure, shell-based linting framework, and CI pipeline modifications across 32 files
QA_REQUIRED: NO
Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub
|
@NickKhalow , does it close this ticket #9400 ? |
|
additionally, @NickKhalow, will you replace with this all unit tests from [CodeConvention] category or it is supposed to be in the next iteration? |
I pushed some fixes to these warnings here: |
… feat/roslyn-analyzers
This comment has been minimized.
This comment has been minimized.
…ft check, HotPath targets - changes job emits lintscripts output; custom-lint runs when only the linter changed (previously a lint-only PR skipped the selftest entirely) - custom-lint and analyzers jobs blank the workflow-level Unity credentials; both execute PR-authored code and need no license - lint-changed.sh no longer swallows custom-rules.sh stderr or exit code 3 (broken rule pattern) - both now block like BLOCK findings - HotPathAttribute narrowed to methods: DCLA003 does not inspect constructors, so allowing the attribute there created unchecked annotations - analyzers job rebuilds the DLL deterministically (pinned SDK via Analyzers/global.json, ContinuousIntegrationBuild, DebugType=none) and fails on drift against the committed DLL; build-analyzers.sh uses identical flags; DLL recommitted from the deterministic build - filter-warnings.sh excludes DCLA* from the ReSharper ratchet count - the analyzers have their own enforcement channel (Unity csc + IDE) - selftest.sh golden comparison strips CR so it passes on CRLF checkouts
…commit Deliberate ref-after-structural-change violation in a production assembly. Expected CI outcome: the Unity build job FAILS with error DCLA001. If it stays green, Unity's csc is not applying the Explorer/.editorconfig severities and the corruption-class rules need a ruleset or Error-default descriptors. Revert this commit after recording the result.
|
Review findings addressed in
|
This comment has been minimized.
This comment has been minimized.
…ic MVID hashes source bytes The drift check's CI rebuild (LF checkout) produced a different DLL than the committed one built from a CRLF checkout. Pin Analyzers/** to LF and recommit the DLL from an LF build; local and CI hashes now match byte-exactly.
…on this commit" This reverts commit f4420ef.
…ignores editorconfig severities Probe result (CI run 31708955588): DCLA001 fired inside Unity's compile at the exact violation, but as a WARNING despite the .editorconfig error pin - Unity's csc does not apply dotnet_diagnostic severities. DCLA001/DCLA005 descriptors now carry DiagnosticSeverity.Error so the Unity build actually fails; the editorconfig pins keep governing IDEs and dotnet builds (Tests downgrade is IDE-only). DLL rebuilt and recommitted. Also fixes the two violations the probe run surfaced in the merge preview: - UUAVState pinned ': int' - it crosses the DllImport boundary and DCLA005 at Error would otherwise fail the build (explicit int == previous implicit ABI) - SituationalReactionPresenter used CinemachineBrain?.OutputCamera, forcing a Cinemachine reference the Chat assembly lacks (CS0012, broke editmode+lint solution builds); IExposedCameraData now exposes OutputCamera as a default member so consumers never touch the Cinemachine type
This comment has been minimized.
This comment has been minimized.
…clean Run 31710811877 showed two things: - DCLA005 at Error fails vendored package code: Unity feeds the analyzer DLL to package compilations too (com.decentraland.pulse.transport ENet enums, com.unity.cloud.ktx), not just asmdefs under Assets/DCL as the README claimed. Vendored sources can't be fixed here, so every analyzer now skips syntax trees under /PackageCache/ (VendoredCode.cs) with a regression test; README scope section corrected. - The drift check caught a self-inflicted mismatch: the previous DLL was synced from an INCREMENTAL build whose stale obj/ state produced different bytes than CI's clean rebuild. build-analyzers.sh now wipes bin/obj before building; DLL recommitted from the clean build.
This comment has been minimized.
This comment has been minimized.
…reference Windows builds of byte-identical sources can differ from the Linux build (observed across two drift-gate failures with LF-normalized sources and the pinned SDK). Make the drift gate self-serving: the job uploads its deterministic rebuild as 'DCL.Analyzers.dll-canonical' before comparing, so on drift the fix is to download and commit that artifact.
This comment has been minimized.
This comment has been minimized.
…ypass; fix ref-temporary in preview controller Three fixes from review round 3 and the drift-gate investigation: - Root cause of every hash mismatch: the SDK's implicit SourceLink appends the git HEAD sha to AssemblyInformationalVersion, so each build embedded the CURRENT commit and a committed DLL could never byte-match a rebuild at any other commit (the OS was never the issue). IncludeSourceRevisionInInformationalVersion=false makes output depend on sources and flags only; DLL recommitted, hash now stable across commits. - The analyzers job gate now also fires when ONLY the committed DLL changed (Explorer/Assets/DCL/DCL.Analyzers.dll in the pathspec) - previously a swapped binary alone skipped the drift check entirely. - CharacterPreviewController: ForgetLoading was still invoked through a temporary ref into the chunk (World.Get(...).WearablePromise.Forget...), keeping 'this' pointed at ECS memory across the structural change inside. The old promise is now copied out first; the copy's mutations are deliberately discarded since the component slot is overwritten below.
|
Claude finished @NickKhalow's task in 3m 26s —— View job Security + code review: re-review after
|
| ID | Where | State |
|---|---|---|
| N13 | test.yml:196-198 |
Unchanged. "Tests only; the shipped DLL is synced manually" — the job now rebuilds, publishes the canonical artifact, and drift-gates the binary. Same stale line in the scripts/build-analyzers.sh header. |
| N12 | Explorer/.editorconfig:819-831 |
Unchanged. "Corruption-class rules fail the build" / "never fail the build" under [**/Tests/**] are both false under Unity. Analyzers/README.md:8-14 now states this correctly and at length; the editorconfig — where a developer looks when a build goes red — still contradicts it. |
| N8 (wording) | Analyzers/README.md:35 |
"first-party code under Assets/ is analyzed in full" — the filter is "not /PackageCache/", and Assets/Plugins/ holds vendored source too. Scope decision is right; the sentence overstates it. |
| N1 | PR labels | no-warning-ratchet still not applied (~154 new nullable warnings from the NRT enablement). Apply → |
| M3/M4 | workflow | checkout@v6, setup-dotnet@v4, upload-artifact@v4 tag-pinned rather than SHA-pinned (repo-wide follow-up; none of these jobs hold secrets). No NuGet lock file — with the commit variable eliminated, restore is the last unpinned input the gate depends on. RestorePackagesWithLockFile + --locked-mode closes it. |
Dependency / workflow surface (delta only)
No new packages, no registry sources, no native code. Runtime delta is three lines in CharacterPreviewController. The DLL changed (19b4ab29… → d0f55f15…, 35840 bytes, unchanged size) — expected, since dropping the informational-version suffix rewrites metadata; I can't inspect it (LFS pointer only in this checkout), so the provenance argument rests entirely on the drift gate passing.
Workflow delta is the one-line pathspec change. Job hardening from earlier rounds is intact: permissions: contents: read, blanked UNITY_* on both PR-code-executing jobs, persist-credentials: false, artifact retention 7 days, no pull_request_target, no prompt files, no new secret consumers.
Worth restating once, because the README's artifact instructions read stronger than they are: the canonical artifact is a build product of PR-side sources and a PR-side .csproj. "CI published it" is not independent attestation — trust comes from Analyzers/** and the csproj being reviewable text in the diff. That's the right achievable property here; it just means the artifact path isn't a way to accept a binary whose sources weren't read.
Verdict
This round found the actual bug, and it was upstream of everything I'd been proposing — the gate was unsatisfiable by construction, not flaky. With IncludeSourceRevisionInInformationalVersion=false the output depends on sources, flags, and the pinned SDK, which is what the invariant needed all along. N5 and N2 are both properly closed.
The one merge condition left is empirical: the drift gate has to go green on the merged head. It hasn't reported yet on this run, and with the severities living in the descriptors, that binary is what decides whether builds fail — this gate is the only thing tying it to reviewable text. Everything else (N12, N13, N14, N8-wording, N1) is documentation and labels.
DEPENDENCY_REVIEW: NEEDS_ATTENTION
· branch feat/roslyn-analyzers
|
PR #9727, run #31721567656 Builds: Windows change, Windows baseline, macOS change, macOS baseline How to read this table
Apple M1
Intel Core i5
Exception breakdown
|
review.md (feat-roslyn-analyzers, PR #9727) judged an earlier sibling history of this branch. Its two must_fix items are already resolved: the analyzer/lint-infrastructure work here never merged in fix/lint-surfaced-cleanups (the split dalkia/popuz demanded already exists, as separate branches from the same parent), and the requested CharacterPreviewController regression test belongs to that other branch, not this one - see FIXNOTES.md for the verified topology. What's left is mikhail-dcl's nit: PooledRentalLeakAnalyzer (DCLA004) disqualifies a rental symbol as soon as any assignment of it reaches an escaping use, so reassigning a local before releasing it silently drops the earlier, actually-leaked rental - flow-insensitive tracking at the symbol level, not a gap in the escape analysis. Root cause documented on the class doc comment (the invariant the code itself can't show), and locked in with a regression test asserting the no-report behavior is intentional rather than an oversight, so an accidental "fix" that starts flagging it fails loudly instead of silently changing the analyzer's false-negative bias. Verified: scripts/lint/custom-rules.sh --working-tree (0 findings), scripts/lint/tests/selftest.sh (OK), scripts/build-analyzers.sh (99/99 tests, DLL resynced). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018c638dR1vPysCMbYt2qQg5
4 participants
Pull Request Description
What does this PR change?
Adds automated enforcement of the project's code standards (CLAUDE.md / skills docs) in two layers — a fast deterministic regex lint and a set of Roslyn analyzers running inside Unity's compilation and the IDE — plus the mechanical cleanup of the existing violations they surfaced (
fix/lint-surfaced-cleanups, merged into this branch), so the rules land on a clean baseline.Layer 1 — deterministic project-rule lint (
scripts/lint/custom-rules.sh)~20 textually-checkable rules, each with a severity and a pointer to the doc that defines it:
Debug.Logoutside ReportHub,new ObjectProxy<, CheckNamespace suppressions, LINQ in*System.cs,Camera.main,#nullable disable, non-Iinterface names, foreign test frameworks (Moq/Xunit/FluentAssertions),async voidtests, unguardedTryAddWidget(...)., thread affinity in scene-runtime code, nullable locals, the null-forgiving!(with carve-outs for the sanctioned= null!DTO idiom and the framework-forcedviewInstance!/Instance!/World!),var x = World.Get<...>component copies,StringBuilder.Append($"...").World.Query, raw HTTP outsideWebRequests/, exception messages withoutnameof.Design properties:
// lint-ignore: <rule-id>comment reviewers can challenge in the diff.scripts/lint/tests/selftest.shruns fixture files against expected-output files, in CI before every lint.Layer 2 —
DCL.Analyzers, five Roslyn analyzersSemantic checks the regex layer cannot do (symbol resolution, statement ordering). Severities pinned in
Explorer/.editorconfig; corruption-class rules fail the build, the rest are advisory while their real-world precision is established:reflocal fromWorld.Get/TryGetRefused after a structural change (Add/Remove/Create/Destroy) relocated it. Reachability-aware: exclusive branches, pre-call arguments, and thex = ref World.Get(...)re-fetch idiom stay silent.async UniTaskVoid,.Forget()) with no exception handling of its own.Update()/[Query]bodies, plus any method tagged with the new[Utility.HotPath]attribute. Throw paths exempt. Body-only — callees are not chased.Get()rental that provably never escapes or releases.[DllImport]boundary without an explicit underlying type.Both error-severity rules downgrade to warnings under
**/Tests/**(tests exercise structural-change scenarios deliberately).Integration
Explorer/Assets/DCL/DCL.Analyzers.dll(LFS) carries theRoslynAnalyzerlabel with all platforms disabled — Unity feeds it tocscfor every asmdef at or belowAssets/DCL/: first-party code only, vendored code untouched. Analyzers also run live in Rider/VS.bash scripts/build-analyzers.sh= tests + Release build + DLL sync.scripts/lint/lint-changed.sh) runs the regex layer first — instant feedback before the multi-minute ReSharper load, and ReSharper is skipped while BLOCK findings are unresolved.custom-lintjob (seconds, no Unity, diff-ranged with base detection for PRs / pushes / merge queues; runs the linter selftest first) andanalyzersjob (runs the analyzer test suite whenAnalyzers/changes). Both are forced to run when the linter itself changes so a broken rule cannot merge unexecuted. ~1,450 lines of analyzer tests using the standardMicrosoft.CodeAnalysis.Testingmarkup idiom with metadata-name stubs for external types.Layer 3 — cleanup of the violations the new tooling surfaced
The branch also merges
fix/lint-surfaced-cleanups(104 files, +45/−308, all mechanical), which fixes the existing violations so the new rules land on a clean baseline — without it, DCLA001 (error) would fail the build on pre-existing code the moment this PR merged:CharacterPreviewController.UpdateAvatarAsyncheldref avatarShapeacross structural changes (globalWorld.Create, plus the entity creation/destruction hidden insidePromise.Create/ForgetLoading) — writes through a relocated ref are silently lost. All structural work now completes into locals before the ref is taken. A real, pre-existing lost-write bug found by DCLA001 — verified GREEN on Unity 6000.4.0f1 (219 assemblies).DCLPlayerPrefs: shutdown/cleanup logs migrated to ReportHub (the TODO claiming it was unavailable was stale); the editor-onlyMenuItemlog keepsDebug.Logwith a justified// lint-ignore.SituationalReactionPresenter:Camera.mainreplaced with injectedIExposedCameraData.CinemachineBrain?.OutputCamera, threaded throughChatPlugin→ChatPanelPresenter.// ReSharper disable CheckNamespacecomments deleted.csc.rspfiles next to asmrefs deleted — Unity only honors rsp beside an asmdef (verified against the Bee build graph: zero references); orphaned by the assembly consolidation (refactor: consolidate assembly definitions #8961).MarketplaceCredits.API,Character,Quality.RenderFeatures,DCL.Editor(~154 new nullable warnings —no-warning-ratchetlabel applies).Review fixes (second push)
All review findings addressed:
custom-lintgate: thechangesjob now emits alintscriptsoutput; the job runs when onlyscripts/lint/**changed, so a broken rule can no longer merge with its selftest skipped.custom-lintandanalyzersblank the workflow-level Unity credentials at job level — both execute PR-authored code and need no license.lint-changed.sh: no longer suppressescustom-rules.shstderr; exit code 3 (broken rule pattern) now blocks like BLOCK findings.[HotPath]: narrowed toAttributeTargets.Method— the analyzer doesn't inspect constructors, so the wider target created unchecked annotations.analyzersjob deterministically rebuilds the DLL (SDK pinned exactly inAnalyzers/global.json,ContinuousIntegrationBuild=true,DebugType=none) and fails on any byte difference vs the committed DLL.build-analyzers.shuses identical flags; the DLL is recommitted from the deterministic build (byte-stable across clean rebuilds).filter-warnings.shexcludesDCLA*diagnostics from the warning count — the analyzers have their own enforcement channel. This dropped the lint delta from +329 to +62; the remainder is the deliberate NRT enablement on four assemblies (CS8618/CS8625/CS8765), covered by theno-warning-ratchetlabel.Probe results and hardening (third push)
A deliberate DCLA001 violation was pushed, its CI result recorded, and reverted. The findings drove three further changes:
.editorconfigseverities — the probe compiled as a warning despite theerrorpin. DCLA001/DCLA005 now carryDiagnosticSeverity.Errorin their descriptors, which is what actually fails the Unity build (verified: the analyzer does run inside Unity's compile and fired at the exact violation line). The.editorconfigpins remain for IDEs anddotnetbuilds; the**/Tests/**downgrade is therefore IDE-only.Library/PackageCache), not justAssets/DCLas originally assumed; DCLA005-at-Error was failingcom.decentraland.pulse.transportandcom.unity.cloud.ktx. All five analyzers now skip/PackageCache/syntax trees (VendoredCode.cs, with regression test). First-party violations surfaced by the probe run were fixed instead:UUAVStatepins: int, andSituationalReactionPresenter's Cinemachine type leak into the Chat assembly (CS0012) is resolved via a newIExposedCameraData.OutputCameradefault member.Analyzers/** text eol=lfin.gitattributes) and an incremental-build DLL that differed from a clean rebuild (fixed:build-analyzers.shwipesbin/objfirst). Local and CI builds are now byte-identical.Known limitations
Update()is not flagged unless tagged[HotPath].Test Instructions
Tooling + mechanical cleanups; the only runtime-behavior surface worth a functional pass is the avatar preview flow touched by the DCLA001 fix.
Test Steps
bash scripts/lint/tests/selftest.sh— regex-layer fixtures pass.dotnet test Analyzers/DCL.Analyzers.Tests(orbash scripts/build-analyzers.sh) — analyzer suite passes.Debug.Logto any production.csfile → the Stop hook /custom-lintCI job reportsdebug-logas BLOCK; remove it.ref var c = ref World.Get<T>(e), callWorld.Remove<T>(e), then readc→ DCLA001 appears in the IDE and Unity compile.CharacterPreviewControllerfix reorders promise creation relative to component writes).Quality Checklist
Code Review Reference
Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.