fix: stop lint flagging PascalCase enum members as UPPER_CASE#9468
fix: stop lint flagging PascalCase enum members as UPPER_CASE#9468dalkia wants to merge 11 commits into
Conversation
The style guide (docs/code-style-guidelines.md § Naming Conventions) says enumerations and their enumerators are PascalCase, but ReSharper swept enum members into the CAPITALS_SNAKE_CASE constants rule (enum members are const fields to the analyzer), flagging every PascalCase member of 265 enums. Add an explicit 'Enum members' naming rule (PascalCase) to the shared .editorconfig so the warning stops firing. No code renames. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. |
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
Review — PR #9468: fix: stop lint flagging PascalCase enum members as UPPER_CASE
STEP 2 — Root-cause check: PASS ✅
The problem: ReSharper's InconsistentNaming inspection flags PascalCase enum members (e.g. FeatureFlag.MultiplayerCompressionWin) with "Suggested name is 'ALL_UPPER'". The root cause is that enum members are const fields to the Roslyn analyzer, so the existing dotnet_naming_symbols.constants rule (applicable_kinds = field + required_modifiers = const → all_upper) sweeps them into the ALL_UPPER_SNAKE_CASE bucket. ReSharper surfaces this as its predefined Enum members rule.
The fix directly addresses the root cause by adding an explicit resharper_csharp_naming_rule.enum_member = AaBb override, which is the correct ReSharper-specific mechanism to separate enum members from the general constants rule. This is not a workaround — it's the intended configuration knob.
STEP 3 — Design & integration: N/A
Pure configuration change — no new code units, no lifecycle changes, no subscriptions, no state. Nothing to trace.
STEP 4 — Member audit: N/A
No new members added or changed.
STEP 5 — Line-level review: No issues found
- Rule syntax:
resharper_csharp_naming_rule.enum_member = AaBbis correct ReSharper notation for PascalCase. Verified. - Alignment with style guide:
docs/code-style-guidelines.md§ Naming Conventions explicitly statesPascalCasefor "Enumeration and its Enumerators" with examplepublic enum Side { Left, Right }. The lint config was out of sync with the written standard; this change aligns them. - No interference with constants rule: The existing
dotnet_naming_rule.constants_should_be_capital_snake_case(lines 71-73) continues to apply toconstfields. The ReSharper-specificenum_memberrule takes precedence only for enum members, which is the correct layering. The PR description confirms local verification. - Precedent for
resharper_properties: The.editorconfigalready contains 60+resharper_-prefixed settings (lines 306+), so this follows established practice. - Placement: Logically placed between the interface naming rule and the CAPITALS_SNAKE_CASE section, with a clear comment referencing the authoritative doc section.
Security review: No security concerns — config-only change with no secrets, auth, input handling, or code execution paths affected.
STEP 6 — Complexity assessment
SIMPLE — single config file, 6 lines added (4 comment + 1 rule + 1 blank), no runtime code.
STEP 7 — QA assessment
NO — lint configuration only; no user-facing behavior, no runtime code, no Unity player impact.
STEP 8 — Non-blocking warnings
None.
STEP 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Single-line ReSharper naming rule added to .editorconfig — lint configuration only, no runtime code.
QA_REQUIRED: NO
Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
github-actions
Bot
added
claude-approved
no QA needed
Local lint measurement (same CLI + flags + filter as CI)Ran ReSharper InspectCode
Verification details:
The CI warnings-ratchet step will report the authoritative count against the S3 baseline. 🤖 Generated with Claude Code |
dalkia
added
the
force-lint
|
Tests: 24281 passed, 0 failed ✅ |
|
Warnings count reduced: 14572 => 14131 Warnings/errors in files changed by this PR (2086) |
Signed-off-by: Juan Ignacio Molteni <juanignaciomolteni@gmail.com>
…' into fix/lint-enum-members-pascalcase
Rename CAP_SNAKE_CASE enum members to PascalCase across the project, enforced by the ENUM_MEMBER UserRule in Explorer.sln.DotSettings. Only provably-safe enums are renamed: each was audited to confirm its member names never cross a by-name boundary (Enum.Parse/nameof/ToString reaching wire, analytics, query strings, PlayerPrefs, disk, cache keys, AudioMixer/shader/SortingLayer names, or JSON StringEnumConverter). Enums whose member name is serialized are intentionally left as-is (SortBy, SortDirection, NotificationType, SDKVersion, SeasonState, AssetBundleRegistryEnum, AudioMixerExposedParam, OpenBadgeSectionOrigin, LoginMethod, ChatChannelType, and the Communities/DTO wire enums). Also fixes Web3IdentitySource, which had regressed to CAP_SNAKE on this branch after the #9100 deep-link merge.
Enum PascalCase migration — scope of this PRThis PR renames enum members Deliberately NOT renamed (left in CAP_SNAKE_CASE) — member name crosses a boundary
Note
|
popuz
removed
the
no QA needed
|
Why this PR moves enum members to PascalCase (not SCREAMING_CASE) This aligns our enum-member casing with the current, official C# convention across all three relevant authorities. There is no official source backing ALL_CAPS enum Microsoft / .NET
JetBrains Rider / ReSharper (our lint engine)
ASP.NET Core / Roslyn / StyleCop
Scope / safety Only provably-safe enums are renamed. Every renamed enum was audited to confirm its member names never cross a by-name boundary (Enum.Parse/nameof/ToString reaching the |
…pascalcase Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Post a Pending banner when Unity Test starts and always upsert the warning-ratchet comment on completion (Passed / Blocked / Skipped / Unavailable), so a failed or lint-skipped run no longer leaves a stale banner. Mirrors the build comment's shields.io badge style.
This comment has been minimized.
This comment has been minimized.
The MCP server feature (#9339) merged from dev references enum members this branch renamed to PascalCase: ChatMessageOrigin.CHAT / RESTRICTED_ACTION_API and CameraMode.SDKCamera. Update the call sites so the merged tree compiles.
This comment has been minimized.
This comment has been minimized.
|
Claude finished @popuz's task in 1m 30s —— View job Security-focused review — dependency & workflow changes (re-run on synchronize)
Scope re-scanDiffed the entire PR against
No dependency changes of any kind:
→ Dependency-review portion is N/A / PASS. The Workflow review —
|
| Item | Source | Type | Scope | Risk | Confidence |
|---|---|---|---|---|---|
pr-comment-warnings.yml (pending job + banner refresh) |
PR | workflow | CI-only | LOW (this PR's changes) | HIGH |
Mutable action pins (peter-evans/*, download-artifact@v4) |
PR + pre-existing | external actions | CI, pull-requests: write |
MEDIUM | HIGH |
Artifact JSON → $GITHUB_OUTPUT echo |
pre-existing | untrusted input | CI | MEDIUM | MEDIUM |
.editorconfig / .DotSettings lint rules |
PR | config | editor/CI | LOW | HIGH |
| C# enum-member renames (~100 files) | PR | source | runtime | LOW (no supply-chain surface) | HIGH |
Verdict
The substance of this PR (enum-member renames + lint config + banner UX) introduces no new secret exposure, checkout, or code-execution surface. The two MEDIUM items are pre-existing repo-wide patterns this PR only lightly extends; neither blocks merge. Flagging so a human can decide on the SHA-pinning hardening.
Note: I can't edit files under
.github/workflows/(GitHub App permission), so the SHA-pin recommendation is advisory.
DEPENDENCY_REVIEW: NEEDS_ATTENTION (no HIGH findings; two MEDIUM hardening items)
· branch fix/lint-enum-members-pascalcase
|
✅ PR reviewed and approved by QA on both platforms following the PR test instructions. Build: Test results:
Unrelated errors noted (do not affect verdict):
Verdict: PASS ✅ Since this PR renames ~100 files of enum members to PascalCase (beyond the original lint-config-only scope), I specifically checked for any casing/casting/serialization fallout and found none — no enum parse failures, no wire/analytics mismatches surfaced in the logs on either platform. |
4 participants
Pull Request Description
What does this PR change?
Adds an explicit 'Enum members' naming rule (PascalCase) to the shared lint settings (
Explorer/.editorconfig) so ReSharper/CI stops flagging PascalCase enum members asInconsistentNaming("Suggested name is 'ALL_UPPER'"). Small, zero-risk diff; no code renames.Why this is correct — the written style guide says PascalCase.
docs/code-style-guidelines.md§ Naming Conventions states:with this example:
So members like
FeatureFlag.MultiplayerCompressionWinare correct per the docs — the lint config was wrong, not the code.Root cause: enum members are
constfields to the analyzer, so the existing ruledotnet_naming_symbols.constants(applicable_kinds = field+required_modifiers = const→ALL_UPPER) swept them into the constants rule. ReSharper maps that onto its predefined Enum members rule, which is why the warning text cites rule 'Enum members'. The newresharper_csharp_naming_rule.enum_member = AaBbline overrides exactly that rule; realconstfields are still enforced asCAPITALS_SNAKE_CASE(verified — see below).Expected reduction: a repo scan finds 265 enums with PascalCase members, 1,440 violating members in total (each member = one warning), out of 399 enums / 2,174 members in code the ratchet counts. The CI warnings count should drop by roughly that amount vs the baseline.
Verified locally with the same ReSharper CLI version and flags CI uses (
scripts/lint/run-inspectcode.sh):Assets/DCL/AvatarRendering/Wearables/IWearablesProvider.cs→ 9InconsistentNamingwarnings ("does not match rule 'Enum members'").const int badConstNameis still flagged → the constants rule remains fully enforced for actual constants.Test Instructions
Steps (standard run): N/A — lint-configuration-only change; no runtime behavior is affected.
Expected result: The
warnings-historyCI check reports a warning count far below the baseline (expected ≈1,440 fewer), and no new warnings appear.Steps (fresh account): N/A
Automation (if applicable): N/A
Prerequisites
Test Steps
Smoke test
Additional Testing Notes
ALL_UPPER(e.g. generated code) still pass: PascalCase rules accept theAA_BBextra style ReSharper applies for abbreviations, and no rename is suggested by this change.Quality Checklist
🤖 Generated with Claude Code