diff --git a/.gitattributes b/.gitattributes index f5c83116f38..198cf5fd373 100644 --- a/.gitattributes +++ b/.gitattributes @@ -61,3 +61,9 @@ Explorer/Assets/*.rsp text eol=lf # fail them with "$'\r': command not found" (seen on every Windows cloud build # running the UBA preBuildScript). *.sh text eol=lf + +# Analyzers sources must check out LF everywhere: the analyzers CI job +# byte-compares a deterministic rebuild against the committed DLL, and the +# deterministic MVID hashes the source BYTES - a CRLF checkout builds a +# different DLL than CI's LF checkout. +Analyzers/** text eol=lf diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ebe112a57cf..a4a38cdd09e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,6 +7,10 @@ on: - 'Explorer/**' # The rsp-drift job must run when the generator changes, or drift merges unnoticed. - 'scripts/generate-ignore-warnings.sh' + # The custom-lint job must run when the linter changes, or a broken rule merges unexecuted. + - 'scripts/lint/**' + # The analyzers job must run when the Roslyn analyzers change. + - 'Analyzers/**' types: - opened - reopened @@ -73,6 +77,13 @@ jobs: runs-on: ubuntu-latest outputs: cs: ${{ steps.detect.outputs.cs }} + analyzers: ${{ steps.detect.outputs.analyzers }} + # True when the linter itself changed - custom-lint must run its selftest + # even on a PR with no .cs changes, or a broken rule merges unexecuted. + lintscripts: ${{ steps.detect.outputs.lintscripts }} + # Diff range for added-lines linting; base is empty when undeterminable. + base: ${{ steps.detect.outputs.base }} + head: ${{ steps.detect.outputs.head }} steps: - name: Checkout code uses: actions/checkout@v6 @@ -87,24 +98,31 @@ jobs: BASE_REF: ${{ github.event.pull_request.base.ref }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} BEFORE_SHA: ${{ github.event.before }} + MG_BASE: ${{ github.event.merge_group.base_sha }} + MG_HEAD: ${{ github.event.merge_group.head_sha }} + FALLBACK_SHA: ${{ github.sha }} run: | set -euo pipefail + head="${HEAD_SHA:-$FALLBACK_SHA}" if [ "$EVENT" = "pull_request" ]; then - base=$(git merge-base "origin/$BASE_REF" "$HEAD_SHA" 2>/dev/null || true) + base=$(git merge-base "origin/$BASE_REF" "$head" 2>/dev/null || true) if [ -z "$base" ]; then echo "merge-base with origin/$BASE_REF unavailable - linting to be safe." - echo "cs=true" >> "$GITHUB_OUTPUT"; exit 0 + { echo "cs=true"; echo "analyzers=true"; echo "lintscripts=true"; } >> "$GITHUB_OUTPUT"; exit 0 fi - range="$base $HEAD_SHA" + elif [ "$EVENT" = "merge_group" ]; then + base="$MG_BASE"; head="$MG_HEAD" elif [ "$EVENT" = "push" ] && [ -n "$BEFORE_SHA" ] && [ "$BEFORE_SHA" != "0000000000000000000000000000000000000000" ] && git cat-file -e "${BEFORE_SHA}^{commit}" 2>/dev/null; then - range="$BEFORE_SHA ${{ github.sha }}" + base="$BEFORE_SHA" else echo "Cannot determine a diff base for event '$EVENT' - linting to be safe." - echo "cs=true" >> "$GITHUB_OUTPUT"; exit 0 + { echo "cs=true"; echo "analyzers=true"; echo "lintscripts=true"; } >> "$GITHUB_OUTPUT"; exit 0 fi - if git diff --name-only $range -- '*.cs' | grep -q .; then + echo "base=$base" >> "$GITHUB_OUTPUT" + echo "head=$head" >> "$GITHUB_OUTPUT" + if git diff --name-only "$base" "$head" -- '*.cs' | grep -q .; then echo "C# files changed - lint will run." echo "cs=true" >> "$GITHUB_OUTPUT" else @@ -112,6 +130,138 @@ jobs: echo "cs=false" >> "$GITHUB_OUTPUT" fi + # The committed DLL is in the pathspec so a PR that swaps ONLY the binary + # still runs the drift check - that is the exact attack/mistake it exists for. + if git diff --name-only "$base" "$head" -- 'Analyzers/' 'scripts/build-analyzers.sh' 'Explorer/Assets/DCL/DCL.Analyzers.dll' | grep -q .; then + echo "analyzers=true" >> "$GITHUB_OUTPUT" + else + echo "analyzers=false" >> "$GITHUB_OUTPUT" + fi + + if git diff --name-only "$base" "$head" -- 'scripts/lint/' | grep -q .; then + echo "lintscripts=true" >> "$GITHUB_OUTPUT" + else + echo "lintscripts=false" >> "$GITHUB_OUTPUT" + fi + + # Fast deterministic project rules (CLAUDE.md / .claude/skills) checked over the + # lines ADDED in this PR/push — runs in seconds without Unity, complementing the + # ReSharper ratchet below. Pre-existing violations never fail this job. + custom-lint: + needs: changes + if: needs.changes.outputs.cs == 'true' || needs.changes.outputs.lintscripts == 'true' || github.event.label.name == 'force-lint' + name: Project rules lint + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + # This job executes PR-authored shell; it must not see the workflow-level + # Unity credentials it doesn't need. + env: + UNITY_EMAIL: '' + UNITY_PASSWORD: '' + UNITY_LICENSE: '' + steps: + - name: Checkout code + uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + fetch-depth: 0 + # This job runs PR-head shell; don't leave the token in .git/config. + persist-credentials: false + + # Runs before the diff lint so a broken linter fails the job even when + # the diff itself is clean (or the base is undeterminable). + - name: Linter selftest + run: bash scripts/lint/tests/selftest.sh + + - name: Lint added lines against project rules + env: + BASE: ${{ needs.changes.outputs.base }} + HEAD: ${{ needs.changes.outputs.head }} + run: | + set -uo pipefail + if [ -z "$BASE" ]; then + echo "::warning::custom-lint: no diff base for event '${{ github.event_name }}' - added-lines rules were NOT evaluated." + exit 0 + fi + out="$(bash scripts/lint/custom-rules.sh --diff "$BASE" "$HEAD")"; rc=$? + printf '%s\n' "$out" + # Surface findings as PR annotations - WARNs otherwise die in a green job's log. + printf '%s\n' "$out" | awk -F' ' 'NF >= 4 { + split($1, loc, ":") + level = ($2 == "BLOCK") ? "error" : "warning" + printf "::%s file=%s,line=%s::[%s] %s\n", level, loc[1], loc[2], $3, $4 + }' + exit "$rc" + + # Roslyn analyzer test suite (Analyzers/DCL.Analyzers) - semantic rules that + # run inside Unity's csc. Tests only; the shipped DLL is synced manually via + # scripts/build-analyzers.sh and committed (LFS). + analyzers: + needs: changes + if: needs.changes.outputs.analyzers == 'true' + name: Roslyn analyzers + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + # This job restores and executes a PR-authored .csproj (arbitrary MSBuild + # targets); it must not see the workflow-level Unity credentials. + env: + UNITY_EMAIL: '' + UNITY_PASSWORD: '' + UNITY_LICENSE: '' + steps: + - name: Checkout code + uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + persist-credentials: false + lfs: true + + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + # The exact SDK pin (not a floating channel) is what makes the drift + # rebuild below byte-identical to scripts/build-analyzers.sh output. + global-json-file: Analyzers/global.json + + - name: Test analyzers + working-directory: Analyzers + run: dotnet test DCL.Analyzers.Tests -v q --nologo + + # The committed DLL is what Unity actually enforces; it must be provably + # built from the reviewed source. THIS job's Linux build is the canonical + # one: Windows builds of the same sources can differ byte-wise (observed), + # so on drift the fix is to commit the DLL this job uploads below. + - name: Rebuild DLL deterministically + run: | + set -euo pipefail + (cd Analyzers && dotnet build DCL.Analyzers -c Release -v q --nologo \ + -p:ContinuousIntegrationBuild=true -p:DebugType=none) + + # Uploaded before the comparison so the canonical DLL is available from + # this run precisely when the drift gate below fails. + - name: Upload canonical DLL + uses: actions/upload-artifact@v4 + with: + name: DCL.Analyzers.dll-canonical + path: Analyzers/DCL.Analyzers/bin/Release/netstandard2.0/DCL.Analyzers.dll + retention-days: 7 + + - name: Fail on DLL drift + run: | + set -euo pipefail + built=Analyzers/DCL.Analyzers/bin/Release/netstandard2.0/DCL.Analyzers.dll + committed=Explorer/Assets/DCL/DCL.Analyzers.dll + if ! cmp -s "$built" "$committed"; then + echo "Committed DCL.Analyzers.dll does not match this job's canonical build of Analyzers/." + echo "Download the 'DCL.Analyzers.dll-canonical' artifact from this run, copy it to $committed, and commit it." + sha256sum "$built" "$committed" + exit 1 + fi + lint: needs: changes if: needs.changes.outputs.cs == 'true' || github.event.label.name == 'force-lint' @@ -861,9 +1011,16 @@ jobs: watchdog: runs-on: ubuntu-latest - needs: [lint, test] + needs: [analyzers, custom-lint, lint, test] + # always() is required: without a status-check function GitHub implicitly + # ANDs success(), which is false whenever a needed job failed - i.e. the + # watchdog could never fire on exactly the events it exists to catch. if: | - needs.lint.result == 'failure' || - needs.test.result == 'failure' + always() && ( + needs.analyzers.result == 'failure' || + needs['custom-lint'].result == 'failure' || + needs.lint.result == 'failure' || + needs.test.result == 'failure' + ) steps: - run: exit 1 diff --git a/.gitignore b/.gitignore index afb1fc93ba2..1cb841569a2 100644 --- a/.gitignore +++ b/.gitignore @@ -84,3 +84,8 @@ Explorer/.cursorignore # ReSharper CLI download for local linting (scripts/lint/download-resharper.sh) /rsharp/ rsharp.zip +Analyzers/**/bin/ +Analyzers/**/obj/ + +# Unity generates *.csproj at the repo root; the Analyzers projects are real. +!Analyzers/**/*.csproj diff --git a/Analyzers/DCL.Analyzers.Tests/AllocationInSystemUpdateTests.cs b/Analyzers/DCL.Analyzers.Tests/AllocationInSystemUpdateTests.cs new file mode 100644 index 00000000000..5e7471e2833 --- /dev/null +++ b/Analyzers/DCL.Analyzers.Tests/AllocationInSystemUpdateTests.cs @@ -0,0 +1,422 @@ +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; +using System.Threading.Tasks; + +namespace DCL.Analyzers.Tests +{ + public class AllocationInSystemUpdateTests + { + private const string SYSTEM_STUB = @" +namespace ECS.Abstract +{ + public abstract class BaseUnityLoopSystem + { + protected abstract void Update(float t); + } +} +"; + + private static Task VerifyAsync(string source, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest + { + TestCode = "using System.Linq;\n" + SYSTEM_STUB + source, + }; + + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + private static Task VerifySystemAsync(string fields, string updateBody) => + VerifyAsync(@" +public class MoveSystem : ECS.Abstract.BaseUnityLoopSystem +{ +" + fields + @" + protected override void Update(float t) + { +" + updateBody + @" + } +}"); + + [Test] + public Task ReportsAllocationInHotPathMethod() => + VerifyAsync(@" +namespace Utility { public sealed class HotPathAttribute : System.Attribute { } } + +public class UrlResolver +{ + private System.Collections.Generic.List parts; + + [Utility.HotPath] + public void Resolve() + { + parts = {|DCLA003:new System.Collections.Generic.List()|}; + } + + public void ColdSetup() + { + parts = new System.Collections.Generic.List(); + } +}"); + + [Test] + public Task CleanForAllocationFreeHotPath() => + VerifyAsync(@" +namespace Utility { public sealed class HotPathAttribute : System.Attribute { } } + +public class UrlResolver +{ + private int count; + + [Utility.HotPath] + public void Resolve() + { + count++; + } +}"); + + [Test] + public Task CleanForExceptionConstructionOutsideThrow() => + VerifySystemAsync(@" + private System.Exception stored; + + private class LoadFailedException : System.Exception { } +", @" + stored = new LoadFailedException(); +"); + + [Test] + public Task ReportsReferenceTypeCreation() => + VerifySystemAsync(@" + private System.Collections.Generic.List list; +", @" + list = {|DCLA003:new System.Collections.Generic.List()|}; + list.Clear(); +"); + + [Test] + public Task ReportsArrayCreationInDerivedChain() => + VerifyAsync(@" +public abstract class MiddleSystem : ECS.Abstract.BaseUnityLoopSystem { } + +public class BufferSystem : MiddleSystem +{ + private float[] buffer; + private int[] ids; + + protected override void Update(float t) + { + buffer = {|DCLA003:new float[16]|}; + ids = {|DCLA003:new[] { 1, 2, 3 }|}; + buffer[0] = t; + } +}"); + + [Test] + public Task ReportsCapturingLambdas() => + VerifySystemAsync(@" + private int total; + private System.Action action; +", @" + action = {|DCLA003:() => total += 1|}; + action = {|DCLA003:() => System.Console.WriteLine(t)|}; +"); + + [Test] + public Task ReportsStringInterpolation() => + VerifySystemAsync(@" + private string label; +", @" + label = {|DCLA003:$""t={t}""|}; +"); + + [Test] + public Task ReportsStringConcatenation() => + VerifySystemAsync(@" + private string label; + private string name = ""n""; +", @" + label = {|DCLA003:""t="" + name|}; +"); + + [Test] + public Task ReportsChainedConcatenationOnceAtTheTop() => + VerifySystemAsync(@" + private string label; + private string name = ""n""; +", @" + label = {|DCLA003:""a"" + name + ""b""|}; +"); + + [Test] + public Task ReportsLinqInvocations() => + VerifySystemAsync(@" + private int[] items = new int[4]; + private System.Linq.IQueryable query; + private int count; + private System.Collections.Generic.IEnumerable positives; +", @" + count = {|DCLA003:items.Count()|}; + positives = {|DCLA003:items.Where(x => x > 0)|}; + count = {|DCLA003:System.Linq.Queryable.Count(query)|}; +"); + + [Test] + public Task CleanForAllShapesOutsideUpdate() => + VerifyAsync(@" +public class MoveSystem : ECS.Abstract.BaseUnityLoopSystem +{ + private int total; + private string label; + private int[] items = new int[4]; + private System.Action action; + + protected override void Update(float t) + { + total++; + } + + public void Prepare(float t) + { + var list = new System.Collections.Generic.List(); + var buffer = new float[16]; + action = () => total += 1; + label = $""t={t}"" + label; + total = System.Linq.Enumerable.Count(items); + } +}"); + + [Test] + public Task CleanForNonSystemOverrideWithSystemTypePresent() => + VerifyAsync(@" +public abstract class NotASystem +{ + protected virtual void Update(float t) { } +} + +public class Widget : NotASystem +{ + private string label; + + protected override void Update(float t) + { + var list = new System.Collections.Generic.List(); + label = $""t={t}""; + } +}"); + + [Test] + public Task CleanWhenNoSystemTypeInCompilation() + { + var test = new CSharpAnalyzerTest + { + TestCode = @" +public abstract class NotASystem +{ + protected virtual void Update(float t) { } +} + +public class Widget : NotASystem +{ + private string label; + + protected override void Update(float t) + { + var list = new System.Collections.Generic.List(); + label = $""t={t}""; + } +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task CleanForStructCreationAndNameof() => + VerifySystemAsync(@" + private int total; +", @" + var span = new System.TimeSpan(1); + string n = nameof(Update); + total = span.Seconds + n.Length; +"); + + [Test] + public Task CleanForNonCapturingAndStaticLambdas() => + VerifySystemAsync(@" + private System.Func func; +", @" + func = x => x + 1; + func = static x => x * 2; +"); + + [Test] + public Task CleanForConstantFoldedConcat() => + VerifySystemAsync(@" + private string label; +", @" + const string prefix = ""p:""; + label = ""a"" + ""b""; + label = prefix + ""c""; + label = nameof(Update) + ""!""; +"); + + [Test] + public Task CleanForExceptionCreationOnThrowPaths() => + VerifySystemAsync(@" + private int total; +", @" + if (total < 0) + throw new System.InvalidOperationException(""negative""); + + int next = total > 0 ? total + 1 : throw new System.NotImplementedException(); + total = next; +"); + + [Test] + public Task ReportsAllocationInQueryAttributedMethod() + { + var test = new CSharpAnalyzerTest + { + TestCode = @" +namespace ECS.Abstract +{ + public abstract class BaseUnityLoopSystem + { + protected abstract void Update(float t); + } +} + +public class QueryAttribute : System.Attribute { } + +public partial class MoveSystem : ECS.Abstract.BaseUnityLoopSystem +{ + private string label; + + protected override void Update(float t) + { + HandleMove(t); + } + + [Query] + private void HandleMove(float t) + { + label = {|DCLA003:$""t={t}""|}; + } +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task CleanForImpostorBaseWhenRealAnchorExists() + { + var test = new CSharpAnalyzerTest + { + TestCode = @" +namespace ECS.Abstract +{ + public abstract class BaseUnityLoopSystem + { + protected abstract void Update(float t); + } +} + +namespace ThirdParty +{ + public abstract class BaseUnityLoopSystem + { + protected abstract void Update(float t); + } +} + +public class Widget : ThirdParty.BaseUnityLoopSystem +{ + private string label; + + protected override void Update(float t) + { + label = $""t={t}""; + } +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task ReportsInterpolationInExpressionBodiedUpdate() => + VerifyAsync(@" +public class LabelSystem : ECS.Abstract.BaseUnityLoopSystem +{ + private string label; + + protected override void Update(float t) => label = {|DCLA003:$""t={t}""|}; +}"); + + [Test] + public Task ReportsTargetTypedNewForReferenceType() => + VerifySystemAsync(@" + private System.Collections.Generic.List list; +", @" + list = {|DCLA003:new()|}; +"); + + [Test] + public Task ReportsLinqBehindConditionalAccess() => + VerifySystemAsync(@" + private int[] items = new int[4]; + private object result; +", @" + result = items?{|DCLA003:.Where(x => x > 0)|}; +"); + + [Test] + public Task ReportsStringAppendAssignment() => + VerifySystemAsync(@" + private string label; + private string name = ""n""; +", @" + {|DCLA003:label += name|}; +"); + + [Test] + public Task ReportsAnonymousObjectCreation() => + VerifySystemAsync(@" + private object cached; +", @" + cached = {|DCLA003:new { X = 1 }|}; +"); + + [Test] + public Task ReportsWhenBaseMatchedBySimpleNameOnly() + { + var test = new CSharpAnalyzerTest + { + TestCode = @" +namespace Stubs +{ + public abstract class BaseUnityLoopSystem + { + protected abstract void Update(float t); + } +} + +public class OtherSystem : Stubs.BaseUnityLoopSystem +{ + private System.Collections.Generic.List list; + + protected override void Update(float t) + { + list = {|DCLA003:new System.Collections.Generic.List()|}; + list.Clear(); + } +}", + }; + + return test.RunAsync(); + } + } +} diff --git a/Analyzers/DCL.Analyzers.Tests/DCL.Analyzers.Tests.csproj b/Analyzers/DCL.Analyzers.Tests/DCL.Analyzers.Tests.csproj new file mode 100644 index 00000000000..8c7ed49f7db --- /dev/null +++ b/Analyzers/DCL.Analyzers.Tests/DCL.Analyzers.Tests.csproj @@ -0,0 +1,25 @@ + + + + net8.0 + latest + enable + false + + + + + + + + + + + + + + + + + diff --git a/Analyzers/DCL.Analyzers.Tests/DetachedUniTaskFlowTests.cs b/Analyzers/DCL.Analyzers.Tests/DetachedUniTaskFlowTests.cs new file mode 100644 index 00000000000..440af5e7b1f --- /dev/null +++ b/Analyzers/DCL.Analyzers.Tests/DetachedUniTaskFlowTests.cs @@ -0,0 +1,364 @@ +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; +using System.Threading.Tasks; + +namespace DCL.Analyzers.Tests +{ + public class DetachedUniTaskFlowTests + { + private const string UNITASK_STUB = @" +using System; +using System.Runtime.CompilerServices; +using Cysharp.Threading.Tasks; + +namespace Cysharp.Threading.Tasks +{ + [AsyncMethodBuilder(typeof(AsyncUniTaskVoidMethodBuilder))] + public struct UniTaskVoid + { + public void Forget() { } + } + + [AsyncMethodBuilder(typeof(AsyncUniTaskMethodBuilder))] + public struct UniTask + { + public static UniTask CompletedTask => default; + public Awaiter GetAwaiter() => default; + + public struct Awaiter : ICriticalNotifyCompletion + { + public bool IsCompleted => true; + public void GetResult() { } + public void OnCompleted(Action continuation) { } + public void UnsafeOnCompleted(Action continuation) { } + } + } + + public struct AsyncUniTaskVoidMethodBuilder + { + public static AsyncUniTaskVoidMethodBuilder Create() => default; + public UniTaskVoid Task => default; + public void SetResult() { } + public void SetException(Exception exception) { } + public void SetStateMachine(IAsyncStateMachine stateMachine) { } + public void Start(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine => stateMachine.MoveNext(); + public void AwaitOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : INotifyCompletion where TStateMachine : IAsyncStateMachine { } + public void AwaitUnsafeOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : ICriticalNotifyCompletion where TStateMachine : IAsyncStateMachine { } + } + + public struct AsyncUniTaskMethodBuilder + { + public static AsyncUniTaskMethodBuilder Create() => default; + public UniTask Task => default; + public void SetResult() { } + public void SetException(Exception exception) { } + public void SetStateMachine(IAsyncStateMachine stateMachine) { } + public void Start(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine => stateMachine.MoveNext(); + public void AwaitOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : INotifyCompletion where TStateMachine : IAsyncStateMachine { } + public void AwaitUnsafeOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : ICriticalNotifyCompletion where TStateMachine : IAsyncStateMachine { } + } + + public static class UniTaskExtensions + { + public static void Forget(this UniTask task) { } + public static UniTask SuppressCancellationThrow(this UniTask task) => task; + public static UniTask SuppressToResultAsync(this UniTask task) => task; + } +} +"; + + private static Task VerifyAsync(string members, string? extraFile = null) + { + var test = new CSharpAnalyzerTest + { + TestCode = UNITASK_STUB + @" +public class DetachedFlowScenarios +{ +" + members + @" +}", + }; + + if (extraFile != null) + test.TestState.Sources.Add(extraFile); + + return test.RunAsync(); + } + + [Test] + public Task ReportsUnguardedUniTaskVoidMethod() => + VerifyAsync(@" + private async UniTaskVoid {|DCLA002:RunAsync|}() + { + await UniTask.CompletedTask; + } +"); + + [Test] + public Task ReportsUnguardedUniTaskVoidLocalFunction() => + VerifyAsync(@" + public void Trigger() + { + async UniTaskVoid {|DCLA002:PumpAsync|}() => await UniTask.CompletedTask; + PumpAsync(); + } +"); + + [Test] + public Task ReportsUnguardedUniTaskVoidLambda() => + VerifyAsync(@" + public void Trigger() + { + Func detached = {|DCLA002:async|} () => await UniTask.CompletedTask; + detached(); + } +"); + + [Test] + public Task ReportsForgetOnUnguardedSameFileMethod() => + VerifyAsync(@" + public void Trigger() + { + LoadAsync().{|DCLA002:Forget|}(); + } + + private async UniTask LoadAsync() + { + await UniTask.CompletedTask; + } +"); + + [Test] + public Task ReportsWhenOnlyCancellationIsCaught() => + VerifyAsync(@" + private async UniTaskVoid {|DCLA002:RunAsync|}() + { + try { await UniTask.CompletedTask; } + catch (OperationCanceledException) { } + } +"); + + [Test] + public Task CleanWhenBodyGuardedByCatchException() => + VerifyAsync(@" + private async UniTaskVoid RunAsync() + { + try { await UniTask.CompletedTask; } + catch (Exception) { } + } +"); + + [Test] + public Task CleanWhenWholeBodyInsideTryWithGeneralCatch() => + VerifyAsync(@" + private async UniTaskVoid RunAsync() + { + try + { + await UniTask.CompletedTask; + await UniTask.CompletedTask; + } + catch { } + } +"); + + [Test] + public Task CleanForgetOnGuardedSameFileMethod() => + VerifyAsync(@" + public void Trigger() + { + LoadAsync().Forget(); + } + + private async UniTask LoadAsync() + { + try { await UniTask.CompletedTask; } + catch (Exception) { } + } +"); + + [Test] + public Task CleanForgetOnMethodFromAnotherFile() => + VerifyAsync(@" + public void Trigger() + { + RemoteLoader.RunAsync().Forget(); + } +", extraFile: @" +using Cysharp.Threading.Tasks; + +public static class RemoteLoader +{ + public static async UniTask RunAsync() => await UniTask.CompletedTask; +} +"); + + [Test] + public Task CleanForAwaitedUnguardedUniTask() => + VerifyAsync(@" + public async UniTask OuterAsync() => await LoadAsync(); + + private async UniTask LoadAsync() => await UniTask.CompletedTask; +"); + + [Test] + public Task CleanWhenSuppressToResultAsyncChainGuards() => + VerifyAsync(@" + private async UniTaskVoid RunAsync() => await LoadAsync().SuppressToResultAsync(); + + private async UniTask LoadAsync() => await UniTask.CompletedTask; +"); + + [Test] + public Task ReportsUniTaskVoidFlowOnlyAtDeclarationNotAtForgetCallsite() => + VerifyAsync(@" + public void Trigger() + { + RunAsync().Forget(); + } + + private async UniTaskVoid {|DCLA002:RunAsync|}() + { + await UniTask.CompletedTask; + } +"); + + [Test] + public Task CleanWhenForgetTakesExceptionHandler() => + VerifyAsync(@" + public void Trigger() + { + LoadAsync().Forget(e => { }); + } + + private async UniTask LoadAsync() + { + await UniTask.CompletedTask; + } +", extraFile: @" +using System; + +namespace Cysharp.Threading.Tasks +{ + public static class UniTaskForgetWithHandlerExtensions + { + public static void Forget(this UniTask task, Action exceptionHandler) { } + } +} +"); + + [Test] + public Task ReportsWhenOnlySuppressCancellationThrowGuards() => + VerifyAsync(@" + private async UniTaskVoid {|DCLA002:RunAsync|}() + { + await LoadAsync().SuppressCancellationThrow(); + } + + private async UniTask LoadAsync() => await UniTask.CompletedTask; +"); + + [Test] + public Task CleanWhenForgetIsUnrelatedDomainMethod() + { + var test = new CSharpAnalyzerTest + { + TestCode = UNITASK_STUB + @" +public class TrackedEntry +{ + public void Forget() { } +} + +public class DetachedFlowScenarios +{ + public void Trigger() + { + CreateEntry().Forget(); + } + + private TrackedEntry CreateEntry() => new TrackedEntry(); +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task ReportsWhenGuardOnlyInsideNestedLambda() => + VerifyAsync(@" + private async UniTaskVoid {|DCLA002:RunAsync|}() + { + Action guardedElsewhere = () => + { + try { } catch (Exception) { } + }; + + guardedElsewhere(); + await UniTask.CompletedTask; + } +"); + + [Test] + public Task ReportsForgetOnUnguardedSameFileExtensionMethodCalledInReducedForm() + { + var test = new CSharpAnalyzerTest + { + TestCode = UNITASK_STUB + @" +public static class ScenarioExtensions +{ + public static async UniTask LoadValueAsync(this int value) => await UniTask.CompletedTask; +} + +public class DetachedFlowScenarios +{ + public void Trigger() + { + 42.LoadValueAsync().{|DCLA002:Forget|}(); + } +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task ReportsForgetOnUnguardedSameFilePartialMethod() + { + var test = new CSharpAnalyzerTest + { + TestCode = UNITASK_STUB + @" +public partial class DetachedFlowScenarios +{ + public void Trigger() + { + LoadAsync().{|DCLA002:Forget|}(); + } + + private partial UniTask LoadAsync(); +} + +public partial class DetachedFlowScenarios +{ + private async partial UniTask LoadAsync() => await UniTask.CompletedTask; +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task ReportsForgetOnUnguardedSameFileLocalFunction() => + VerifyAsync(@" + public void Trigger() + { + LoadAsync().{|DCLA002:Forget|}(); + + async UniTask LoadAsync() => await UniTask.CompletedTask; + } +"); + } +} diff --git a/Analyzers/DCL.Analyzers.Tests/FfiEnumUnderlyingTypeTests.cs b/Analyzers/DCL.Analyzers.Tests/FfiEnumUnderlyingTypeTests.cs new file mode 100644 index 00000000000..22418e26523 --- /dev/null +++ b/Analyzers/DCL.Analyzers.Tests/FfiEnumUnderlyingTypeTests.cs @@ -0,0 +1,113 @@ +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; +using System.Threading.Tasks; + +namespace DCL.Analyzers.Tests +{ + public class FfiEnumUnderlyingTypeTests + { + private static Task VerifyAsync(string source, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest + { + TestCode = "using System.Runtime.InteropServices;\n" + source, + }; + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + [Test] + public Task ReportsBareEnumParameter() => + VerifyAsync(@" +public enum Mode { A, B } + +public static class Native +{ + [DllImport(""lib"")] + public static extern void SetMode({|DCLA005:Mode mode|}); +}"); + + [Test] + public Task SkipsVendoredPackageCacheSources() + { + // Unity feeds the analyzer to package compilations too; vendored sources + // (Library/PackageCache) are skipped - the violation below must NOT report. + var test = new CSharpAnalyzerTest(); + + test.TestState.Sources.Add(( + "/Library/PackageCache/com.example.pkg@abc123/Runtime/Native.cs", + @"using System.Runtime.InteropServices; + +public enum Mode { A, B } + +public static class Native +{ + [DllImport(""lib"")] + public static extern void SetMode(Mode mode); +}")); + + return test.RunAsync(); + } + + [Test] + public Task ReportsBareEnumReturnType() => + VerifyAsync(@" +public enum Status { Ok, Fail } + +public static class Native +{ + [DllImport(""lib"")] + public static extern {|DCLA005:Status|} GetStatus(); +}"); + + [Test] + public Task ReportsBareEnumFieldInStructParameter() => + VerifyAsync(@" +public enum Kind { X } + +public struct Payload +{ + public int Size; + public Kind Kind; +} + +public static class Native +{ + [DllImport(""lib"")] + public static extern void Send({|DCLA005:Payload payload|}); +}"); + + [Test] + public Task CleanWhenUnderlyingTypeExplicit() => + VerifyAsync(@" +public enum Mode : byte { A, B } + +public enum Status : int { Ok, Fail } + +public static class Native +{ + [DllImport(""lib"")] + public static extern Status SetMode(Mode mode); +}"); + + [Test] + public Task CleanForBareEnumOutsideFfi() => + VerifyAsync(@" +public enum Mode { A, B } + +public static class Service +{ + public static void SetMode(Mode mode) { } +}"); + + [Test] + public Task CleanForMetadataEnum() => + VerifyAsync(@" +public static class Native +{ + [DllImport(""lib"")] + public static extern void SetDay(System.DayOfWeek day); +}"); + } +} diff --git a/Analyzers/DCL.Analyzers.Tests/PooledRentalLeakTests.cs b/Analyzers/DCL.Analyzers.Tests/PooledRentalLeakTests.cs new file mode 100644 index 00000000000..488083e45b7 --- /dev/null +++ b/Analyzers/DCL.Analyzers.Tests/PooledRentalLeakTests.cs @@ -0,0 +1,408 @@ +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; +using System.Threading.Tasks; + +namespace DCL.Analyzers.Tests +{ + public class PooledRentalLeakTests + { + private const string POOL_STUB = @" +using System.Collections.Generic; +using UnityEngine.Pool; + +namespace UnityEngine.Pool +{ + public interface IObjectPool where T : class + { + T Get(); + PooledObject Get(out T v); + void Release(T element); + } + + public struct PooledObject : System.IDisposable where T : class + { + public void Dispose() { } + } + + public class ObjectPool : IObjectPool where T : class + { + public T Get() => null; + public PooledObject Get(out T v) { v = null; return default; } + public void Release(T element) { } + } + + public static class ListPool + { + public static List Get() => new List(); + public static void Release(List toRelease) { } + } + + public static class HashSetPool + { + public static HashSet Get() => new HashSet(); + public static void Release(HashSet toRelease) { } + } + + public static class DictionaryPool + { + public static Dictionary Get() => new Dictionary(); + public static void Release(Dictionary toRelease) { } + } +} + +public class Payload : System.IDisposable +{ + public int Value; + public void Dispose() { } +} + +public class CustomPool : IObjectPool +{ + public Payload Get() => null; + public PooledObject Get(out Payload v) { v = null; return default; } + public void Release(Payload element) { } +} + +public class Cache +{ + public Payload Get() => null; + public void Release(Payload element) { } +} +"; + + private static Task VerifyAsync(string members, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest + { + TestCode = POOL_STUB + @" +public class SomeService +{ + private readonly ObjectPool pool = new ObjectPool(); + private readonly IObjectPool poolInterface = new ObjectPool(); + private readonly CustomPool customPool = new CustomPool(); + private readonly Cache cache = new Cache(); + private List stored; + +" + members + @" +}", + }; + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + [Test] + public Task CleanWhenRentalAssignedInsideArgument() => + VerifyAsync(@" + private void Attach(object target) { } + + public void Process() + { + Payload transform; + Attach(transform = pool.Get()); + transform.Value = 1; + } +"); + + [Test] + public Task ReportsStaticListPoolRentalNeverReleased() => + VerifyAsync(@" + public void Update() + { + List numbers = {|DCLA004:ListPool.Get()|}; + numbers.Add(1); + } +"); + + [Test] + public Task ReportsInstanceObjectPoolRentalNeverReleased() => + VerifyAsync(@" + public void Update() + { + Payload payload = {|DCLA004:pool.Get()|}; + payload.Value = 1; + } +"); + + [Test] + public Task ReportsRentalFromInterfaceTypedPool() => + VerifyAsync(@" + public void Update() + { + Payload payload = {|DCLA004:poolInterface.Get()|}; + payload.Value = 1; + } +"); + + [Test] + public Task ReportsRentalFromCustomIObjectPoolImplementation() => + VerifyAsync(@" + public void Update() + { + Payload payload = {|DCLA004:customPool.Get()|}; + } +"); + + [Test] + public Task ReportsHashSetAndDictionaryPoolRentals() => + VerifyAsync(@" + public void Update() + { + HashSet set = {|DCLA004:HashSetPool.Get()|}; + set.Add(1); + Dictionary map = {|DCLA004:DictionaryPool.Get()|}; + map[1] = 2; + } +"); + + [Test] + public Task ReportsAssignmentFormRentalWithNullSuppression() => + VerifyAsync(@" + public void Update() + { + List numbers; + numbers = {|DCLA004:ListPool.Get()|}!; + if (numbers != null) + numbers.Add(1); + } +"); + + [Test] + public Task CleanWhenSequentiallyReleased() => + VerifyAsync(@" + public void Update() + { + List numbers = ListPool.Get(); + numbers.Add(1); + ListPool.Release(numbers); + } +"); + + [Test] + public Task CleanForTryFinallyRental() => + VerifyAsync(@" + public void Update() + { + List numbers = ListPool.Get(); + try { numbers.Add(1); } + finally { ListPool.Release(numbers); } + } +"); + + [Test] + public Task CleanWhenInstancePoolReleasesViaReceiver() => + VerifyAsync(@" + public void Update() + { + Payload payload = pool.Get(); + payload.Value = 1; + pool.Release(payload); + } +"); + + [Test] + public Task CleanWhenRentedValueIsReturned() => + VerifyAsync(@" + public List Rent() + { + List numbers = ListPool.Get(); + numbers.Add(1); + return numbers; + } +"); + + [Test] + public Task CleanWhenStoredInFieldOrPassedToAnotherMethod() => + VerifyAsync(@" + public void Update() + { + stored = ListPool.Get(); + List numbers = ListPool.Get(); + Consume(numbers); + } + + private void Consume(List numbers) { } +"); + + [Test] + public Task CleanWhenCapturedByLambda() => + VerifyAsync(@" + public void Update() + { + List numbers = ListPool.Get(); + System.Action release = () => ListPool.Release(numbers); + release(); + } +"); + + [Test] + public Task CleanForUsingScopedRentals() => + VerifyAsync(@" + public void Update() + { + using Payload payload = pool.Get(); + payload.Value = 1; + + using (pool.Get(out Payload other)) + { + other.Value = 2; + } + } +"); + + [Test] + public Task CleanForGetOnNonPoolType() => + VerifyAsync(@" + public void Update() + { + Payload payload = cache.Get(); + payload.Value = 1; + } +"); + + [Test] + public Task CleanWhenPooledObjectSelfReleasesViaMemberInvocation() => + VerifyAsync(@" + public void Update() + { + // mirrors GliderPropView.PlayOneShotDetached: the pooled object's own method + // (OneShotAudioSource.Play -> Invoke(ReturnToPool) -> pool.Release(this)) + // returns it to the pool, so the rental does not leak + Payload payload = pool.Get(); + payload.Dispose(); + } +"); + + [Test] + public Task ReportsRentalWhenGetResolvesToInheritedCollectionPoolBase() + { + // mirrors real UnityEngine.Pool: static pools inherit Get/Release from + // CollectionPool, so the resolved method's containing + // type is CollectionPool`2, not ListPool`1 + var test = new CSharpAnalyzerTest + { + TestCode = @" +using System.Collections.Generic; +using UnityEngine.Pool; + +namespace UnityEngine.Pool +{ + public interface IObjectPool where T : class + { + T Get(); + void Release(T element); + } + + public class CollectionPool where TCollection : class, ICollection, new() + { + public static TCollection Get() => new TCollection(); + public static void Release(TCollection toRelease) { } + } + + public class ListPool : CollectionPool, T> { } +} + +public class SomeService +{ + public void Update() + { + List numbers = {|DCLA004:ListPool.Get()|}; + numbers.Add(1); + } +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task ReportsRentalIteratedWithDeconstructionForeach() + { + // deconstruction foreach is ForEachVariableStatementSyntax, not + // ForEachStatementSyntax - iterating stays provably local either way + var test = new CSharpAnalyzerTest + { + TestCode = @" +using System.Collections.Generic; +using UnityEngine.Pool; + +namespace UnityEngine.Pool +{ + public interface IObjectPool where T : class + { + T Get(); + void Release(T element); + } + + public static class ListPool + { + public static List Get() => new List(); + public static void Release(List toRelease) { } + } +} + +public class Pair +{ + public void Deconstruct(out int key, out int value) { key = 0; value = 0; } +} + +public class SomeService +{ + public void Update() + { + List pairs = {|DCLA004:ListPool.Get()|}; + + foreach ((int key, int value) in pairs) + { + int sum = key + value; + } + } +}", + }; + + return test.RunAsync(); + } + + [Test] + public Task ReportsRentalLeakedEntirelyInsideLambda() => + VerifyAsync(@" + public void Update() + { + System.Action leak = () => + { + List numbers = {|DCLA004:ListPool.Get()|}; + numbers.Add(1); + }; + + leak(); + } +"); + + [Test] + public Task ReportsRentalLeakedInsideLocalFunction() => + VerifyAsync(@" + public void Update() + { + Leak(); + + void Leak() + { + List numbers = {|DCLA004:ListPool.Get()|}; + numbers.Add(1); + } + } +"); + + [Test] + public Task CleanWhenReleasedThroughIsPatternAlias() => + VerifyAsync(@" + public void Update() + { + List numbers = ListPool.Get(); + + if (numbers is { } alias) + ListPool.Release(alias); + } +"); + } +} diff --git a/Analyzers/DCL.Analyzers.Tests/StructuralChangeAfterRefTests.cs b/Analyzers/DCL.Analyzers.Tests/StructuralChangeAfterRefTests.cs new file mode 100644 index 00000000000..3db9e3883b5 --- /dev/null +++ b/Analyzers/DCL.Analyzers.Tests/StructuralChangeAfterRefTests.cs @@ -0,0 +1,170 @@ +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; +using System.Threading.Tasks; + +namespace DCL.Analyzers.Tests +{ + public class StructuralChangeAfterRefTests + { + private const string ARCH_STUB = @" +namespace Arch.Core +{ + public struct Entity { } + + internal static class Storage { public static T Value; } + + public class World + { + public ref T Get(Entity entity) => ref Storage.Value; + public ref T TryGetRef(Entity entity, out bool exists) { exists = true; return ref Storage.Value; } + public void Add(Entity entity) { } + public void Add(Entity entity, T component) { } + public void Remove(Entity entity) { } + public void Destroy(Entity entity) { } + public Entity Create() => default; + } + + public class CommandBuffer + { + public void Add(Entity entity) { } + public void Remove(Entity entity) { } + } +} + +public struct Movement { public float Speed; } +public struct Tag { } +"; + + private static Task VerifyAsync(string body, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest + { + TestCode = ARCH_STUB + @" +public class SomeSystem +{ + private readonly Arch.Core.World world = new Arch.Core.World(); + private readonly Arch.Core.CommandBuffer buffer = new Arch.Core.CommandBuffer(); + private readonly int mode = 0; + + public void Update(Arch.Core.Entity entity) + { +" + body + @" + } +}", + }; + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + [Test] + public Task ReportsUseAfterStructuralChange() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + world.Add(entity); + {|DCLA001:movement|}.Speed = 1f; +"); + + [Test] + public Task ReportsUseAfterDestroy() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + world.Destroy(entity); + float s = {|DCLA001:movement|}.Speed; +"); + + [Test] + public Task ReportsForTryGetRef() => + VerifyAsync(@" + ref var movement = ref world.TryGetRef(entity, out bool exists); + world.Remove(entity); + {|DCLA001:movement|}.Speed = 2f; +"); + + [Test] + public Task CleanWhenUseCompletesBeforeStructuralChange() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + movement.Speed = 1f; + world.Add(entity); +"); + + [Test] + public Task CleanForPlainCopy() => + VerifyAsync(@" + var movement = world.Get(entity); + world.Add(entity); + movement.Speed = 1f; +"); + + [Test] + public Task CleanForTryGetRefBranchIdiom() => + VerifyAsync(@" + ref var movement = ref world.TryGetRef(entity, out bool has); + if (!has) + world.Add(entity); + else + movement.Speed = 1f; +"); + + [Test] + public Task CleanWhenUseIsInsideStructuralCallArguments() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + world.Add(entity, new Movement { Speed = movement.Speed }); +"); + + [Test] + public Task CleanForExclusiveSwitchSections() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + switch (mode) + { + case 0: + world.Destroy(entity); + break; + case 1: + movement.Speed = 2f; + break; + } +"); + + [Test] + public Task CleanWhenRefIsReacquiredAfterStructuralChange() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + world.Add(entity); + movement = ref world.Get(entity); + movement.Speed = 1f; +"); + + [Test] + public Task ReportsUseBetweenStructuralChangeAndRefetch() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + world.Add(entity); + {|DCLA001:movement|}.Speed = 1f; + movement = ref world.Get(entity); + movement.Speed = 2f; +"); + + [Test] + public Task ReportsWhenStructuralAndUseShareABranch() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + if (mode == 0) + { + world.Add(entity); + {|DCLA001:movement|}.Speed = 1f; + } +"); + + [Test] + public Task CleanForCommandBufferStructuralChange() => + VerifyAsync(@" + ref var movement = ref world.Get(entity); + buffer.Add(entity); + movement.Speed = 1f; +"); + } +} diff --git a/Analyzers/DCL.Analyzers/AllocationInSystemUpdateAnalyzer.cs b/Analyzers/DCL.Analyzers/AllocationInSystemUpdateAnalyzer.cs new file mode 100644 index 00000000000..2b5ff583a42 --- /dev/null +++ b/Analyzers/DCL.Analyzers/AllocationInSystemUpdateAnalyzer.cs @@ -0,0 +1,237 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; + +namespace DCL.Analyzers +{ + /// + /// DCLA003: a heap allocation inside a system's per-frame code - the Update() override + /// or a [Query]-attributed method (the generated update path dispatches to those, and + /// they run per entity, hotter than Update itself). + /// Per-frame code runs every frame across multiple world executions, so reference-type + /// construction, capturing lambdas, string interpolation/concatenation, and LINQ + /// calls there accumulate into GC pressure (CLAUDE.md § Performance Constraints). + /// The check is body-only: allocations in callees are not chased. Allocations under a + /// throw statement/expression are exempt - throw paths are cold, not per-frame pressure. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class AllocationInSystemUpdateAnalyzer : DiagnosticAnalyzer + { + public const string DiagnosticId = "DCLA003"; + + private const string BASE_SYSTEM_METADATA_NAME = "ECS.Abstract.BaseUnityLoopSystem"; + private const string BASE_SYSTEM_SIMPLE_NAME = "BaseUnityLoopSystem"; + private const string UPDATE_METHOD_NAME = "Update"; + + // Arch.System.QueryAttribute, matched by name so test stubs and vendored copies work + private const string QUERY_ATTRIBUTE_NAME = "QueryAttribute"; + private const string QUERY_ATTRIBUTE_SHORT_NAME = "Query"; + + private const string HOT_PATH_ATTRIBUTE_NAME = "HotPathAttribute"; + private const string HOT_PATH_ATTRIBUTE_SHORT_NAME = "HotPath"; + + private static readonly ImmutableHashSet LINQ_TYPE_NAMES = + ImmutableHashSet.Create("Enumerable", "Queryable"); + + private static readonly DiagnosticDescriptor RULE = new ( + DiagnosticId, + "Allocation in per-frame system code", + "per-frame system code must be allocation-free: {0}", + "Performance", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "System Update() overrides and the [Query] methods the generated update path dispatches to run every frame " + + "across multiple world executions, so per-frame heap allocations accumulate into GC pressure. " + + "Avoid reference-type construction, capturing lambdas, string interpolation/concatenation, and LINQ in the update path. " + + "See CLAUDE.md § Performance Constraints."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(RULE); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + INamedTypeSymbol? baseSystemType = context.Compilation.GetTypeByMetadataName(BASE_SYSTEM_METADATA_NAME); + + // always register: [HotPath] methods are checked in ANY assembly, including + // ones that never reference the ECS base system (URL handlers, MCP runtime) + context.RegisterSyntaxNodeAction(c => AnalyzeMethod(c, baseSystemType), SyntaxKind.MethodDeclaration); + } + + private static void AnalyzeMethod(SyntaxNodeAnalysisContext context, INamedTypeSymbol? baseSystemType) + { + if (VendoredCode.IsVendored(context.Node.SyntaxTree)) return; + + var method = (MethodDeclarationSyntax)context.Node; + + bool updateShaped = method.Identifier.ValueText == UPDATE_METHOD_NAME + && method.Modifiers.Any(SyntaxKind.OverrideKeyword); + + if (!updateShaped && method.AttributeLists.Count == 0) return; + + SyntaxNode? body = (SyntaxNode?)method.Body ?? method.ExpressionBody; + if (body == null) return; + + IMethodSymbol? symbol = context.SemanticModel.GetDeclaredSymbol(method, context.CancellationToken); + if (symbol == null) return; + + if (!HasAttribute(symbol, HOT_PATH_ATTRIBUTE_NAME, HOT_PATH_ATTRIBUTE_SHORT_NAME)) + { + if (!InheritsFromBaseSystem(symbol.ContainingType, baseSystemType)) + return; + + if (!(updateShaped && symbol.IsOverride) && !HasQueryAttribute(symbol)) + return; + } + + // allocations under a throw are cold-path, not per-frame pressure: skip them + foreach (SyntaxNode node in body.DescendantNodes(static n => n is not ThrowStatementSyntax and not ThrowExpressionSyntax)) + AnalyzeNode(node, context); + } + + private static bool HasQueryAttribute(IMethodSymbol symbol) => + HasAttribute(symbol, QUERY_ATTRIBUTE_NAME, QUERY_ATTRIBUTE_SHORT_NAME); + + private static bool HasAttribute(IMethodSymbol symbol, string name, string shortName) + { + foreach (AttributeData attribute in symbol.GetAttributes()) + { + if (attribute.AttributeClass?.Name == name || attribute.AttributeClass?.Name == shortName) + return true; + } + + return false; + } + + private static void AnalyzeNode(SyntaxNode node, SyntaxNodeAnalysisContext context) + { + SemanticModel model = context.SemanticModel; + CancellationToken ct = context.CancellationToken; + + switch (node) + { + case BaseObjectCreationExpressionSyntax creation: + // exception construction is error-path work regardless of position (thrown, + // wrapped into a Result, logged) - never per-frame pressure + if (model.GetTypeInfo(creation, ct).Type is { IsReferenceType: true } createdType + && !DerivesFromException(createdType, model.Compilation)) + Report(context, node, $"'new {createdType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)}' constructs a reference type"); + break; + + case ArrayCreationExpressionSyntax: + case ImplicitArrayCreationExpressionSyntax: + Report(context, node, "array creation allocates"); + break; + + case AnonymousObjectCreationExpressionSyntax: + Report(context, node, "anonymous object creation allocates"); + break; + + case AnonymousFunctionExpressionSyntax lambda: + DataFlowAnalysis? flow = model.AnalyzeDataFlow(lambda); + + if (flow is { Succeeded: true } && (!flow.Captured.IsEmpty || !flow.CapturedInside.IsEmpty)) + { + ISymbol? captured = flow.Captured.FirstOrDefault() ?? flow.CapturedInside.FirstOrDefault(); + + Report(context, node, captured != null + ? $"lambda captures '{captured.Name}' and allocates a closure" + : "capturing lambda allocates a closure"); + } + + break; + + case InterpolatedStringExpressionSyntax interpolation: + if (model.GetOperation(interpolation, ct)?.ConstantValue.HasValue != true) + Report(context, node, "string interpolation allocates"); + break; + + case BinaryExpressionSyntax binary when binary.IsKind(SyntaxKind.AddExpression): + if (!IsStringConcat(binary, model, ct)) break; + + // compiler-folded constant concat (literals, consts, nameof) never allocates at runtime + if (model.GetOperation(binary, ct)?.ConstantValue.HasValue == true) break; + + // a chain like a + b + c is a single runtime String.Concat: report once, at the top + if (WalkUpParentheses(binary.Parent) is BinaryExpressionSyntax parentConcat && IsStringConcat(parentConcat, model, ct)) break; + + Report(context, node, "string concatenation allocates"); + break; + + // 'label += name' is String.Concat too, but as an AddAssignmentExpression it + // never enters the AddExpression case above + case AssignmentExpressionSyntax assignment when assignment.IsKind(SyntaxKind.AddAssignmentExpression): + if (model.GetTypeInfo(assignment.Left, ct).Type?.SpecialType == SpecialType.System_String + || model.GetTypeInfo(assignment.Right, ct).Type?.SpecialType == SpecialType.System_String) + Report(context, node, "string concatenation allocates"); + + break; + + case InvocationExpressionSyntax invocation: + if (model.GetSymbolInfo(invocation, ct).Symbol is IMethodSymbol { ContainingType: { } linqCandidate } linqMethod + && IsLinqType(linqCandidate)) + Report(context, node, $"LINQ call '{linqCandidate.Name}.{linqMethod.Name}' allocates"); + break; + } + } + + private static bool DerivesFromException(ITypeSymbol type, Compilation compilation) + { + INamedTypeSymbol? exceptionType = compilation.GetTypeByMetadataName("System.Exception"); + if (exceptionType == null) return false; + + for (ITypeSymbol? current = type; current != null; current = current.BaseType) + { + if (SymbolEqualityComparer.Default.Equals(current, exceptionType)) + return true; + } + + return false; + } + + private static bool InheritsFromBaseSystem(INamedTypeSymbol type, INamedTypeSymbol? baseSystemType) + { + // the simple-name fallback only applies when the metadata anchor did not resolve + // (stubs / asmdef splits relocating the base class); with the real symbol present, + // an unrelated type merely sharing the name must not activate the rule + for (INamedTypeSymbol? current = type.BaseType; current != null; current = current.BaseType) + { + if (baseSystemType != null + ? SymbolEqualityComparer.Default.Equals(current.OriginalDefinition, baseSystemType) + : current.Name == BASE_SYSTEM_SIMPLE_NAME) + return true; + } + + return false; + } + + private static bool IsStringConcat(BinaryExpressionSyntax binary, SemanticModel model, CancellationToken ct) => + binary.IsKind(SyntaxKind.AddExpression) + && (model.GetTypeInfo(binary.Left, ct).Type?.SpecialType == SpecialType.System_String + || model.GetTypeInfo(binary.Right, ct).Type?.SpecialType == SpecialType.System_String); + + private static bool IsLinqType(INamedTypeSymbol type) => + LINQ_TYPE_NAMES.Contains(type.Name) + && type.ContainingNamespace is { Name: "Linq", ContainingNamespace: { Name: "System", ContainingNamespace.IsGlobalNamespace: true } }; + + private static SyntaxNode? WalkUpParentheses(SyntaxNode? node) + { + while (node is ParenthesizedExpressionSyntax parenthesized) + node = parenthesized.Parent; + + return node; + } + + private static void Report(SyntaxNodeAnalysisContext context, SyntaxNode node, string kind) => + context.ReportDiagnostic(Diagnostic.Create(RULE, node.GetLocation(), kind)); + } +} diff --git a/Analyzers/DCL.Analyzers/DCL.Analyzers.csproj b/Analyzers/DCL.Analyzers/DCL.Analyzers.csproj new file mode 100644 index 00000000000..27eb91c0722 --- /dev/null +++ b/Analyzers/DCL.Analyzers/DCL.Analyzers.csproj @@ -0,0 +1,24 @@ + + + + + netstandard2.0 + latest + enable + true + true + false + + false + + + + + + + diff --git a/Analyzers/DCL.Analyzers/DetachedUniTaskFlowAnalyzer.cs b/Analyzers/DCL.Analyzers/DetachedUniTaskFlowAnalyzer.cs new file mode 100644 index 00000000000..52b6d061e77 --- /dev/null +++ b/Analyzers/DCL.Analyzers/DetachedUniTaskFlowAnalyzer.cs @@ -0,0 +1,231 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; + +namespace DCL.Analyzers +{ + /// + /// DCLA002: a detached UniTask flow - an async UniTaskVoid method/local function/lambda, + /// or a same-file method detached via a bare UniTask .Forget() - has no exception handling + /// of its own. Detached flows run outside any awaiter, so an unhandled exception is + /// swallowed or crashes the player instead of being reported + /// (CLAUDE.md § Async Flow Guidelines; async-programming skill). + /// A flow counts as guarded when its own body contains a try with catch (System.Exception) + /// or a general catch, or an invocation of SuppressToResultAsync / SuppressToResult; + /// guards inside nested lambdas/local functions do not count (they observe only their own + /// flow), and neither does SuppressCancellationThrow (it only swallows cancellation, so + /// other exceptions still escape unobserved). Forget(exceptionHandler) is guarded at the + /// callsite, and an async UniTaskVoid target of .Forget() is reported once, at its + /// declaration - UniTaskVoid.Forget() is an intent marker, not a second detachment point. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class DetachedUniTaskFlowAnalyzer : DiagnosticAnalyzer + { + public const string DiagnosticId = "DCLA002"; + + private const string UNITASK_VOID_METADATA_NAME = "Cysharp.Threading.Tasks.UniTaskVoid"; + private const string EXCEPTION_METADATA_NAME = "System.Exception"; + private const string FORGET_METHOD_NAME = "Forget"; + + // SuppressCancellationThrow is deliberately absent: it only swallows + // OperationCanceledException, so it does not guard the flow + private static readonly ImmutableHashSet SUPPRESS_METHODS = + ImmutableHashSet.Create("SuppressToResultAsync", "SuppressToResult"); + + private static readonly DiagnosticDescriptor RULE = new ( + DiagnosticId, + "Detached UniTask flow swallows exceptions", + "detached flow '{0}' has no exception handling - wrap the body in try/catch (Exception) (ignore OperationCanceledException, report the rest via ReportHub.LogException) or chain SuppressToResultAsync", + "Correctness", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "async UniTaskVoid flows and .Forget()-detached flows run outside any awaiter: " + + "nothing observes the returned task, so an unhandled exception is silently swallowed. " + + "Detached flows must handle their own exceptions - try/catch (Exception) that ignores " + + "OperationCanceledException and reports the rest via ReportHub.LogException, or a " + + "SuppressToResultAsync chain. See CLAUDE.md § Async Flow Guidelines and the async-programming skill."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(RULE); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + INamedTypeSymbol? uniTaskVoidType = context.Compilation.GetTypeByMetadataName(UNITASK_VOID_METADATA_NAME); + if (uniTaskVoidType == null) return; + + INamedTypeSymbol? exceptionType = context.Compilation.GetTypeByMetadataName(EXCEPTION_METADATA_NAME); + + context.RegisterSyntaxNodeAction( + nodeContext => AnalyzeDeclaredFlow(nodeContext, uniTaskVoidType, exceptionType), + SyntaxKind.MethodDeclaration, + SyntaxKind.LocalFunctionStatement); + + context.RegisterSyntaxNodeAction( + nodeContext => AnalyzeLambda(nodeContext, uniTaskVoidType, exceptionType), + SyntaxKind.ParenthesizedLambdaExpression, + SyntaxKind.SimpleLambdaExpression, + SyntaxKind.AnonymousMethodExpression); + + context.RegisterSyntaxNodeAction( + nodeContext => AnalyzeForget(nodeContext, uniTaskVoidType, exceptionType), + SyntaxKind.InvocationExpression); + } + + /// Rule (a) for named flows: async UniTaskVoid method or local function without its own guard. + private static void AnalyzeDeclaredFlow(SyntaxNodeAnalysisContext context, INamedTypeSymbol uniTaskVoidType, INamedTypeSymbol? exceptionType) + { + if (VendoredCode.IsVendored(context.Node.SyntaxTree)) return; + + (SyntaxTokenList modifiers, SyntaxToken identifier, SyntaxNode? body) = context.Node switch + { + MethodDeclarationSyntax method => (method.Modifiers, method.Identifier, (SyntaxNode?)method.Body ?? method.ExpressionBody?.Expression), + LocalFunctionStatementSyntax localFunction => (localFunction.Modifiers, localFunction.Identifier, (SyntaxNode?)localFunction.Body ?? localFunction.ExpressionBody?.Expression), + _ => default, + }; + + if (body == null || !modifiers.Any(SyntaxKind.AsyncKeyword)) return; + + if (context.SemanticModel.GetDeclaredSymbol(context.Node, context.CancellationToken) is not IMethodSymbol method2 + || !SymbolEqualityComparer.Default.Equals(method2.ReturnType, uniTaskVoidType)) + return; + + if (!IsGuarded(body, context.SemanticModel, exceptionType, context.CancellationToken)) + context.ReportDiagnostic(Diagnostic.Create(RULE, identifier.GetLocation(), identifier.ValueText)); + } + + /// Rule (a) for anonymous flows: async lambda or anonymous method returning UniTaskVoid without its own guard. + private static void AnalyzeLambda(SyntaxNodeAnalysisContext context, INamedTypeSymbol uniTaskVoidType, INamedTypeSymbol? exceptionType) + { + if (VendoredCode.IsVendored(context.Node.SyntaxTree)) return; + + var lambda = (AnonymousFunctionExpressionSyntax)context.Node; + + if (!lambda.Modifiers.Any(SyntaxKind.AsyncKeyword) || lambda.Body == null) return; + + if (context.SemanticModel.GetSymbolInfo(lambda, context.CancellationToken).Symbol is not IMethodSymbol method + || !SymbolEqualityComparer.Default.Equals(method.ReturnType, uniTaskVoidType)) + return; + + if (!IsGuarded(lambda.Body, context.SemanticModel, exceptionType, context.CancellationToken)) + { + SyntaxToken asyncKeyword = lambda.Modifiers.First(m => m.IsKind(SyntaxKind.AsyncKeyword)); + context.ReportDiagnostic(Diagnostic.Create(RULE, asyncKeyword.GetLocation(), "anonymous function")); + } + } + + /// + /// Rule (b): '<invocation>.Forget()' where the invoked method is declared in the same + /// source file and its body has no guard. Cross-file targets are out of scope. + /// + private static void AnalyzeForget(SyntaxNodeAnalysisContext context, INamedTypeSymbol uniTaskVoidType, INamedTypeSymbol? exceptionType) + { + if (VendoredCode.IsVendored(context.Node.SyntaxTree)) return; + + var invocation = (InvocationExpressionSyntax)context.Node; + + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess + || memberAccess.Name.Identifier.ValueText != FORGET_METHOD_NAME + || memberAccess.Expression is not InvocationExpressionSyntax detachedCall) + return; + + // Forget(exceptionHandler) observes exceptions at the callsite - the handler IS the guard + if (invocation.ArgumentList.Arguments.Count > 0) + return; + + // only UniTask's own Forget marks a detached flow; an unrelated domain method that + // happens to be named Forget says nothing about async exception handling + if (context.SemanticModel.GetSymbolInfo(invocation, context.CancellationToken).Symbol is not IMethodSymbol forgetMethod + || !IsDeclaredInCysharp(forgetMethod)) + return; + + if (context.SemanticModel.GetSymbolInfo(detachedCall, context.CancellationToken).Symbol is not IMethodSymbol target) + return; + + // an async UniTaskVoid target is already reported at its declaration by rule (a); + // UniTaskVoid.Forget() is a no-op intent marker, not a second detachment point + if (SymbolEqualityComparer.Default.Equals(target.ReturnType, uniTaskVoidType)) + return; + + // reduced extension methods declare their syntax on the unreduced form, and extended + // partial methods carry the body on the implementation part + IMethodSymbol definition = (target.ReducedFrom ?? target).OriginalDefinition; + definition = definition.PartialImplementationPart ?? definition; + + foreach (SyntaxReference declaration in definition.DeclaringSyntaxReferences) + { + if (declaration.SyntaxTree != invocation.SyntaxTree) continue; + + SyntaxNode? body = declaration.GetSyntax(context.CancellationToken) switch + { + MethodDeclarationSyntax method => (SyntaxNode?)method.Body ?? method.ExpressionBody?.Expression, + LocalFunctionStatementSyntax localFunction => (SyntaxNode?)localFunction.Body ?? localFunction.ExpressionBody?.Expression, + _ => null, + }; + + if (body != null && !IsGuarded(body, context.SemanticModel, exceptionType, context.CancellationToken)) + context.ReportDiagnostic(Diagnostic.Create(RULE, memberAccess.Name.GetLocation(), target.Name)); + + return; + } + } + + /// + /// A body is guarded when it contains a catch of System.Exception (or a general catch), + /// or an invocation of one of the suppress methods (matched by name, so stubs work). + /// Nested lambdas/local functions are separate flows, so their interiors are skipped: + /// a guard that only wraps a nested function does not observe the outer flow's awaits. + /// + private static bool IsGuarded(SyntaxNode body, SemanticModel model, INamedTypeSymbol? exceptionType, CancellationToken ct) + { + foreach (SyntaxNode node in body.DescendantNodesAndSelf(static n => n is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax)) + { + switch (node) + { + case CatchClauseSyntax { Declaration: null }: + return true; + + case CatchClauseSyntax { Declaration.Type: { } caughtTypeSyntax }: + if (exceptionType != null + && SymbolEqualityComparer.Default.Equals(model.GetTypeInfo(caughtTypeSyntax, ct).Type, exceptionType)) + return true; + + break; + + case InvocationExpressionSyntax invocation: + if (SUPPRESS_METHODS.Contains(GetInvokedName(invocation))) + return true; + + break; + } + } + + return false; + } + + private static bool IsDeclaredInCysharp(IMethodSymbol method) => + method.ContainingNamespace is + { + Name: "Tasks", + ContainingNamespace: { Name: "Threading", ContainingNamespace: { Name: "Cysharp", ContainingNamespace.IsGlobalNamespace: true } }, + }; + + private static string GetInvokedName(InvocationExpressionSyntax invocation) => + invocation.Expression switch + { + MemberAccessExpressionSyntax memberAccess => memberAccess.Name.Identifier.ValueText, + MemberBindingExpressionSyntax memberBinding => memberBinding.Name.Identifier.ValueText, + IdentifierNameSyntax identifier => identifier.Identifier.ValueText, + _ => string.Empty, + }; + } +} diff --git a/Analyzers/DCL.Analyzers/FfiEnumUnderlyingTypeAnalyzer.cs b/Analyzers/DCL.Analyzers/FfiEnumUnderlyingTypeAnalyzer.cs new file mode 100644 index 00000000000..e4eab7f096c --- /dev/null +++ b/Analyzers/DCL.Analyzers/FfiEnumUnderlyingTypeAnalyzer.cs @@ -0,0 +1,124 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; + +namespace DCL.Analyzers +{ + /// + /// DCLA005: an enum crossing a [DllImport] boundary (parameter, return type, or a + /// field of a struct parameter) is declared without an explicit underlying type. + /// The native side compiles against a fixed ABI layout; C#'s implicit int default + /// is a convention the enum author can silently change, so FFI enums must pin it + /// (': byte', ': int', ...) - review-enforced in PR #9088. + /// Only source-declared enums are checked: metadata enums cannot reveal whether + /// their base was written explicitly. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class FfiEnumUnderlyingTypeAnalyzer : DiagnosticAnalyzer + { + public const string DiagnosticId = "DCLA005"; + + private const string DLL_IMPORT_METADATA_NAME = "System.Runtime.InteropServices.DllImportAttribute"; + + private static readonly DiagnosticDescriptor RULE = new ( + DiagnosticId, + "FFI enum without explicit underlying type", + "enum '{0}' crosses a DllImport boundary{1} but does not declare its underlying type - pin the ABI with ': byte', ': int', ...", + "Correctness", + // Error by DEFAULT: Unity's csc ignores .editorconfig dotnet_diagnostic severities + // (verified: a probe violation compiled as a warning), so a corruption-class rule + // only fails the Unity build if the descriptor itself says Error. The .editorconfig + // pins still govern IDEs and dotnet builds (including the Tests downgrade). + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "Native code compiles against a fixed layout; an enum without an explicit base relies on " + + "C#'s implicit int, which nothing pins at the interop boundary. Review-enforced (PR #9088)."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(RULE); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + INamedTypeSymbol? dllImportType = context.Compilation.GetTypeByMetadataName(DLL_IMPORT_METADATA_NAME); + if (dllImportType == null) return; + + context.RegisterSyntaxNodeAction(c => AnalyzeMethod(c, dllImportType), SyntaxKind.MethodDeclaration); + } + + private static void AnalyzeMethod(SyntaxNodeAnalysisContext context, INamedTypeSymbol dllImportType) + { + if (VendoredCode.IsVendored(context.Node.SyntaxTree)) return; + + var method = (MethodDeclarationSyntax)context.Node; + + if (context.SemanticModel.GetDeclaredSymbol(method, context.CancellationToken) is not { } symbol + || !HasDllImport(symbol, dllImportType)) + return; + + CheckType(context, symbol.ReturnType, method.ReturnType.GetLocation()); + + foreach (IParameterSymbol parameter in symbol.Parameters) + { + Location location = parameter.DeclaringSyntaxReferences.Length > 0 + ? parameter.DeclaringSyntaxReferences[0].GetSyntax(context.CancellationToken).GetLocation() + : method.Identifier.GetLocation(); + + CheckType(context, parameter.Type, location); + } + } + + private static bool HasDllImport(IMethodSymbol symbol, INamedTypeSymbol dllImportType) + { + foreach (AttributeData attribute in symbol.GetAttributes()) + { + if (SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, dllImportType)) + return true; + } + + return false; + } + + private static void CheckType(SyntaxNodeAnalysisContext context, ITypeSymbol type, Location location) + { + if (LacksExplicitUnderlyingType(type)) + { + context.ReportDiagnostic(Diagnostic.Create(RULE, location, type.Name, "")); + return; + } + + // one level into struct parameters: FFI structs marshal their fields + if (type is INamedTypeSymbol { TypeKind: TypeKind.Struct, IsUnmanagedType: true } structType + && structType.DeclaringSyntaxReferences.Length > 0) + { + foreach (ISymbol member in structType.GetMembers()) + { + if (member is IFieldSymbol { IsStatic: false, IsConst: false } field + && LacksExplicitUnderlyingType(field.Type)) + context.ReportDiagnostic(Diagnostic.Create( + RULE, location, field.Type.Name, $" (field '{field.Name}' of struct '{structType.Name}')")); + } + } + } + + private static bool LacksExplicitUnderlyingType(ITypeSymbol type) + { + if (type.TypeKind != TypeKind.Enum) return false; + + foreach (SyntaxReference reference in type.DeclaringSyntaxReferences) + { + if (reference.GetSyntax() is EnumDeclarationSyntax declaration) + return declaration.BaseList == null; + } + + return false; // metadata enum - explicitness is unknowable, stay silent + } + } +} diff --git a/Analyzers/DCL.Analyzers/PooledRentalLeakAnalyzer.cs b/Analyzers/DCL.Analyzers/PooledRentalLeakAnalyzer.cs new file mode 100644 index 00000000000..6d032ddc8ad --- /dev/null +++ b/Analyzers/DCL.Analyzers/PooledRentalLeakAnalyzer.cs @@ -0,0 +1,304 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; + +namespace DCL.Analyzers +{ + /// + /// DCLA004: a local rented from an object pool (ListPool/HashSetPool/DictionaryPool/ + /// GenericPool/ObjectPool or any IObjectPool implementation) provably leaks: within its + /// declaring scope it is never passed to Release/Return, never returned, never stored in a + /// field/property, never passed to another call, never captured by a nested function, + /// and is not the resource of a using statement/declaration + /// (code-standards skill § Memory; ecs-system-and-component-design skill § cleanup). + /// Deliberately conservative: any escape of the rented value - including a plain + /// local-to-local copy or an is-pattern alias - silences the rule, so only provable leaks + /// are reported. Member invocations on rentals from arbitrary-object pools also silence it: + /// the pooled object's own method can transfer ownership (the self-release idiom, e.g. + /// OneShotAudioSource.Play scheduling pool.Release(this)). BCL collections rented from the + /// CollectionPool family cannot self-release, so member calls on them stay provably local. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class PooledRentalLeakAnalyzer : DiagnosticAnalyzer + { + public const string DiagnosticId = "DCLA004"; + + private const string IOBJECT_POOL_METADATA_NAME = "UnityEngine.Pool.IObjectPool`1"; + + // pools renting BCL collections: the rented List/HashSet/Dictionary cannot release + // itself back to the pool, so member invocations on such rentals stay provably local. + // CollectionPool`2 is required: Unity's static ListPool/HashSetPool/DictionaryPool + // inherit Get/Release from it, so that's where the method symbol actually lives. + private static readonly ImmutableHashSet COLLECTION_POOL_METADATA_NAMES = ImmutableHashSet.Create( + "UnityEngine.Pool.ListPool`1", + "UnityEngine.Pool.HashSetPool`1", + "UnityEngine.Pool.DictionaryPool`2", + "UnityEngine.Pool.CollectionPool`2"); + + // matched against the containing type of the resolved Get() plus its base types and + // interfaces, by metadata name only (test stubs and Unity assemblies both match) + private static readonly ImmutableHashSet POOL_TYPE_METADATA_NAMES = COLLECTION_POOL_METADATA_NAMES.Union(new[] + { + "UnityEngine.Pool.ObjectPool`1", + "UnityEngine.Pool.GenericPool`1", + "UnityEngine.Pool.UnsafeGenericPool`1", + "UnityEngine.Pool.LinkedPool`1", + IOBJECT_POOL_METADATA_NAME, + "DCL.Optimization.Pools.IExtendedObjectPool`1", + }); + + private static readonly DiagnosticDescriptor RULE = new ( + DiagnosticId, + "Pooled rental provably leaks", + "pooled object '{0}' rented from '{1}' is never released and never leaves this method - it provably leaks; release it via '{1}.Release' (ideally in a finally block) or rent through PoolExtensions.AutoScope", + "Correctness", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "A local rented with Get() from an object pool (UnityEngine.Pool.ListPool/HashSetPool/" + + "DictionaryPool/GenericPool/ObjectPool or any IObjectPool implementation such as " + + "DCL.Optimization.Pools.IExtendedObjectPool) that is never passed to Release/Return and never " + + "escapes the method (returned, stored in a field, passed as an argument, captured, or scoped by " + + "a using) permanently removes the instance from the pool. See the code-standards skill § Memory " + + "and the ecs-system-and-component-design skill § cleanup (CLAUDE.md § Component Clean-up Patterns)."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(RULE); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + // anchor on any known pool surface: no pool types, nothing to rent from. + // GetTypesByMetadataName (plural) stays resolvable when a name is ambiguously + // defined in several referenced assemblies (stub/shim packages), where the + // singular GetTypeByMetadataName returns null and would disable the rule. + foreach (string poolTypeName in POOL_TYPE_METADATA_NAMES) + { + if (!context.Compilation.GetTypesByMetadataName(poolTypeName).IsEmpty) + { + context.RegisterCodeBlockAction(AnalyzeBlock); + return; + } + } + } + + private static void AnalyzeBlock(CodeBlockAnalysisContext context) + { + if (VendoredCode.IsVendored(context.CodeBlock.SyntaxTree)) return; + + SyntaxNode block = context.CodeBlock; + SemanticModel model = context.SemanticModel; + + // locals assigned from a parameterless pool Get(); Get(out T) returns a + // PooledObject/scope that releases on Dispose, so it is never a rental here. + // each rental remembers its pool family and the nested function (lambda/local + // function) it was declared in, so uses are judged against the declaring scope + List<(ILocalSymbol local, InvocationExpressionSyntax get)>? rentals = null; + Dictionary? rentalInfo = null; + + foreach (SyntaxNode node in block.DescendantNodes()) + { + switch (node) + { + case VariableDeclaratorSyntax { Initializer: { } initializer } declarator: + if (Unwrap(initializer.Value) is InvocationExpressionSyntax declaratorGet + && IsPoolGet(declaratorGet, model, context.CancellationToken, out bool declaratorCollectionRental) + && !IsUsingResource(declarator) + && model.GetDeclaredSymbol(declarator, context.CancellationToken) is ILocalSymbol declaredLocal) + { + (rentals ??= new List<(ILocalSymbol, InvocationExpressionSyntax)>()).Add((declaredLocal, declaratorGet)); + (rentalInfo ??= new Dictionary(SymbolEqualityComparer.Default))[declaredLocal] = + (declaratorCollectionRental, NearestEnclosingFunction(declarator, block)); + } + + break; + + case AssignmentExpressionSyntax assignment when assignment.IsKind(SyntaxKind.SimpleAssignmentExpression): + // assignment to anything but a local (field, property, ...) is an escape by + // definition. an assignment USED AS AN EXPRESSION (Attach(x = pool.Get())) + // escapes too - the assignment's value flows into the surrounding argument/ + // initializer - so only statement-level assignments register a rental + if (assignment.Parent is ExpressionStatementSyntax + && Unwrap(assignment.Right) is InvocationExpressionSyntax assignmentGet + && IsPoolGet(assignmentGet, model, context.CancellationToken, out bool assignmentCollectionRental) + && assignment.Left is IdentifierNameSyntax target + && model.GetSymbolInfo(target, context.CancellationToken).Symbol is ILocalSymbol assignedLocal) + { + (rentals ??= new List<(ILocalSymbol, InvocationExpressionSyntax)>()).Add((assignedLocal, assignmentGet)); + (rentalInfo ??= new Dictionary(SymbolEqualityComparer.Default))[assignedLocal] = + (assignmentCollectionRental, NearestEnclosingFunction(assignment, block)); + } + + break; + } + } + + if (rentals == null || rentalInfo == null) return; + + var disqualified = new HashSet(SymbolEqualityComparer.Default); + + foreach (IdentifierNameSyntax identifier in block.DescendantNodes().OfType()) + { + if (model.GetSymbolInfo(identifier, context.CancellationToken).Symbol is not ILocalSymbol referenced + || !rentalInfo.TryGetValue(referenced, out (bool isCollectionRental, SyntaxNode? declaringFunction) info) + || IsSafeLocalUse(identifier, block, info.isCollectionRental, info.declaringFunction)) + continue; + + disqualified.Add(referenced); + } + + foreach ((ILocalSymbol local, InvocationExpressionSyntax get) in rentals) + { + if (disqualified.Contains(local)) continue; + + context.ReportDiagnostic(Diagnostic.Create( + RULE, get.GetLocation(), local.Name, PoolDisplayName(get))); + } + } + + /// Uses that keep the rented value inside its declaring scope: member/element access on it, comparisons, reassigning it, iterating it. + private static bool IsSafeLocalUse(IdentifierNameSyntax identifier, SyntaxNode block, bool isCollectionRental, SyntaxNode? declaringFunction) + { + // a reference inside a nested function other than the one declaring the rental is a + // capture: the rented value escapes its declaring scope. references in the declaring + // function itself (including a rental declared and used within one lambda) stay local + if (NearestEnclosingFunction(identifier, block) != declaringFunction) + return false; + + SyntaxNode child = identifier; + SyntaxNode? parent = identifier.Parent; + + while (parent is ParenthesizedExpressionSyntax + || (parent is PostfixUnaryExpressionSyntax suppression && suppression.IsKind(SyntaxKind.SuppressNullableWarningExpression))) + { + child = parent; + parent = parent.Parent; + } + + return parent switch + { + // a member INVOCATION on an arbitrary pooled object can transfer ownership (the + // self-release idiom: the object's own method schedules pool.Release(this)), so it + // silences the rule; BCL collections rented from the CollectionPool family cannot + // self-release, so member calls on them stay provably local. plain property/field + // access is a local read either way + MemberAccessExpressionSyntax memberAccess => memberAccess.Expression == child + && (isCollectionRental || !IsInvocationReceiver(memberAccess)), + ConditionalAccessExpressionSyntax conditionalAccess => conditionalAccess.Expression == child + && (isCollectionRental || conditionalAccess.WhenNotNull is not InvocationExpressionSyntax), + ElementAccessExpressionSyntax elementAccess => elementAccess.Expression == child, + AssignmentExpressionSyntax assignment => assignment.Left == child, + CommonForEachStatementSyntax forEach => forEach.Expression == child, + BinaryExpressionSyntax binary when binary.IsKind(SyntaxKind.EqualsExpression) || binary.IsKind(SyntaxKind.NotEqualsExpression) => true, + + // a designation-free pattern ('is null', 'is { Count: > 0 }') only reads; a + // designation binds an alias the rental can be released through, which must + // silence the rule like any other alias copy + IsPatternExpressionSyntax isPattern => !isPattern.Pattern.DescendantNodesAndSelf().Any(static n => n is SingleVariableDesignationSyntax), + + // everything else - argument (including Release/Return), return, initializer of + // another variable, RHS of an assignment, using resource - silences the rule + _ => false, + }; + } + + private static bool IsInvocationReceiver(MemberAccessExpressionSyntax memberAccess) => + memberAccess.Parent is InvocationExpressionSyntax invocation && invocation.Expression == memberAccess; + + private static SyntaxNode? NearestEnclosingFunction(SyntaxNode node, SyntaxNode block) + { + for (SyntaxNode? current = node.Parent; current != null && current != block; current = current.Parent) + { + if (current is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax) + return current; + } + + return null; + } + + /// Strips parentheses and null-suppression (!) so 'ListPool<T>.Get()!' still reads as the Get() invocation. + private static ExpressionSyntax Unwrap(ExpressionSyntax expression) + { + while (true) + { + switch (expression) + { + case ParenthesizedExpressionSyntax parenthesized: + expression = parenthesized.Expression; + break; + + case PostfixUnaryExpressionSyntax suppression when suppression.IsKind(SyntaxKind.SuppressNullableWarningExpression): + expression = suppression.Operand; + break; + + default: + return expression; + } + } + } + + private static bool IsPoolGet(InvocationExpressionSyntax invocation, SemanticModel model, System.Threading.CancellationToken ct, out bool isCollectionRental) + { + isCollectionRental = false; + + if (invocation.ArgumentList.Arguments.Count != 0) return false; + + return model.GetSymbolInfo(invocation, ct).Symbol is IMethodSymbol { Name: "Get" } method + && IsPoolType(method.ContainingType, out isCollectionRental); + } + + private static bool IsPoolType(INamedTypeSymbol type, out bool isCollectionRental) + { + for (INamedTypeSymbol? current = type; current != null; current = current.BaseType) + { + string metadataName = FullMetadataName(current.OriginalDefinition); + + if (POOL_TYPE_METADATA_NAMES.Contains(metadataName)) + { + isCollectionRental = COLLECTION_POOL_METADATA_NAMES.Contains(metadataName); + return true; + } + } + + foreach (INamedTypeSymbol implemented in type.AllInterfaces) + { + string metadataName = FullMetadataName(implemented.OriginalDefinition); + + if (POOL_TYPE_METADATA_NAMES.Contains(metadataName)) + { + isCollectionRental = COLLECTION_POOL_METADATA_NAMES.Contains(metadataName); + return true; + } + } + + isCollectionRental = false; + return false; + } + + private static string FullMetadataName(INamedTypeSymbol type) + { + string name = type.MetadataName; + + for (INamespaceSymbol? ns = type.ContainingNamespace; ns is { IsGlobalNamespace: false }; ns = ns.ContainingNamespace) + name = ns.Name + "." + name; + + return name; + } + + private static bool IsUsingResource(VariableDeclaratorSyntax declarator) => + declarator.Parent is VariableDeclarationSyntax declaration + && (declaration.Parent is UsingStatementSyntax + || (declaration.Parent is LocalDeclarationStatementSyntax localDeclaration && localDeclaration.UsingKeyword.IsKind(SyntaxKind.UsingKeyword))); + + private static string PoolDisplayName(InvocationExpressionSyntax get) => + get.Expression is MemberAccessExpressionSyntax memberAccess ? memberAccess.Expression.ToString() : get.Expression.ToString(); + } +} diff --git a/Analyzers/DCL.Analyzers/StructuralChangeAfterRefAnalyzer.cs b/Analyzers/DCL.Analyzers/StructuralChangeAfterRefAnalyzer.cs new file mode 100644 index 00000000000..6579de64986 --- /dev/null +++ b/Analyzers/DCL.Analyzers/StructuralChangeAfterRefAnalyzer.cs @@ -0,0 +1,197 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; + +namespace DCL.Analyzers +{ + /// + /// DCLA001: a ref local obtained from Arch's World.Get/TryGetRef is used after a + /// structural change (World.Add/Remove/Create/Destroy) in the same method body. + /// Structural changes relocate entity data in memory, so the outstanding ref points + /// at stale memory - reads are garbage, writes are silently lost + /// (CLAUDE.md § Safe Component Mutation). + /// Ordering is judged by linear source position within the body, refined by two + /// reachability carve-outs calibrated on real compiles: a use inside the structural + /// call's own argument list is pre-call evaluation, and a (call, use) pair in + /// mutually exclusive branches (if/else, switch sections, ternary arms) - the + /// TryGetRef-then-branch idiom - can never both execute. Residual false positives + /// (e.g. exclusive paths via early return) are suppressible with #pragma. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class StructuralChangeAfterRefAnalyzer : DiagnosticAnalyzer + { + public const string DiagnosticId = "DCLA001"; + + private const string ARCH_WORLD_METADATA_NAME = "Arch.Core.World"; + + private static readonly ImmutableHashSet REF_SOURCES = + ImmutableHashSet.Create("Get", "TryGetRef"); + + private static readonly ImmutableHashSet STRUCTURAL_METHODS = + ImmutableHashSet.Create("Add", "Remove", "Create", "Destroy", "AddOrGet", "AddRange", "RemoveRange"); + + private static readonly DiagnosticDescriptor RULE = new ( + DiagnosticId, + "Ref component used after a structural change", + "ref local '{0}' is used after '{1}' - structural changes relocate entity memory and invalidate outstanding refs; complete all ref reads/writes first, or defer the structural change", + "Correctness", + // Error by DEFAULT: Unity's csc ignores .editorconfig dotnet_diagnostic severities + // (verified: a probe violation compiled as a warning), so a corruption-class rule + // only fails the Unity build if the descriptor itself says Error. The .editorconfig + // pins still govern IDEs and dotnet builds (including the Tests downgrade). + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "Structural changes (World.Add/Remove/Create/Destroy) move entity data between archetype chunks. " + + "Any ref obtained from World.Get/TryGetRef before the change points at the old location: " + + "writes are silently lost and reads observe stale data. See CLAUDE.md § Safe Component Mutation."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(RULE); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + INamedTypeSymbol? worldType = context.Compilation.GetTypeByMetadataName(ARCH_WORLD_METADATA_NAME); + if (worldType == null) return; + + context.RegisterCodeBlockAction(blockContext => AnalyzeBlock(blockContext, worldType)); + } + + private static void AnalyzeBlock(CodeBlockAnalysisContext context, INamedTypeSymbol worldType) + { + if (VendoredCode.IsVendored(context.CodeBlock.SyntaxTree)) return; + + SyntaxNode block = context.CodeBlock; + SemanticModel model = context.SemanticModel; + + // ref locals from World.Get/TryGetRef: symbol -> acquisition positions + // (declaration plus every 'x = ref World.Get(...)' re-fetch, which restores + // validity after a structural change - the sanctioned re-acquire idiom) + var refLocals = new Dictionary>(SymbolEqualityComparer.Default); + + // structural World invocations, in source order + var structuralCalls = new List<(InvocationExpressionSyntax node, string name)>(); + + foreach (SyntaxNode node in block.DescendantNodes()) + { + switch (node) + { + case VariableDeclaratorSyntax { Initializer.Value: RefExpressionSyntax { Expression: InvocationExpressionSyntax refInvocation } } declarator: + if (IsWorldInvocation(refInvocation, model, worldType, REF_SOURCES, context.CancellationToken) + && model.GetDeclaredSymbol(declarator, context.CancellationToken) is ILocalSymbol { IsRef: true } local) + (refLocals.TryGetValue(local, out List? dp) ? dp : refLocals[local] = new List()).Add(declarator.SpanStart); + break; + + case AssignmentExpressionSyntax { Right: RefExpressionSyntax { Expression: InvocationExpressionSyntax refetchInvocation }, Left: IdentifierNameSyntax lhs }: + if (IsWorldInvocation(refetchInvocation, model, worldType, REF_SOURCES, context.CancellationToken) + && model.GetSymbolInfo(lhs, context.CancellationToken).Symbol is ILocalSymbol { IsRef: true } refetched) + (refLocals.TryGetValue(refetched, out List? rp) ? rp : refLocals[refetched] = new List()).Add(lhs.SpanStart); + break; + + case InvocationExpressionSyntax invocation: + if (IsWorldInvocation(invocation, model, worldType, STRUCTURAL_METHODS, context.CancellationToken)) + structuralCalls.Add((invocation, GetMethodName(invocation, model, context.CancellationToken))); + break; + } + } + + if (refLocals.Count == 0 || structuralCalls.Count == 0) return; + + foreach (IdentifierNameSyntax identifier in block.DescendantNodes().OfType()) + { + if (model.GetSymbolInfo(identifier, context.CancellationToken).Symbol is not ILocalSymbol referenced + || !refLocals.TryGetValue(referenced, out List? acquisitions)) + continue; + + // the acquisition governing this use is the nearest one before it; the LHS + // of a re-fetch is itself an acquisition, not a use + int declaredAt = int.MinValue; + foreach (int position in acquisitions) + { + if (position <= identifier.SpanStart && position > declaredAt) + declaredAt = position; + } + + if (declaredAt == int.MinValue || identifier.SpanStart <= declaredAt) continue; + + // the first structural call between the declaration and this use invalidates + // the ref - unless the "use" is inside the call's own argument list (arguments + // evaluate before the call runs), or the two sit in mutually exclusive branches + // (if/else, switch sections, ternary arms) and can never both execute. + foreach ((InvocationExpressionSyntax call, string name) in structuralCalls) + { + if (call.SpanStart <= declaredAt || call.SpanStart >= identifier.SpanStart) continue; + if (call.Span.Contains(identifier.Span)) continue; + if (InExclusiveBranches(call, identifier)) continue; + + context.ReportDiagnostic(Diagnostic.Create( + RULE, identifier.GetLocation(), referenced.Name, $"World.{name}")); + break; + } + } + } + + /// + /// True when the two nodes sit in mutually exclusive branches of a common ancestor + /// (then vs else of an if, different switch sections, opposite ternary arms) - + /// execution can reach one or the other, never both. + /// + private static bool InExclusiveBranches(SyntaxNode a, SyntaxNode b) + { + for (SyntaxNode? ancestor = a.Parent; ancestor != null; ancestor = ancestor.Parent) + { + switch (ancestor) + { + case IfStatementSyntax { Else: { } elseClause } ifStatement: + bool aInThen = ifStatement.Statement.Span.Contains(a.Span); + bool bInThen = ifStatement.Statement.Span.Contains(b.Span); + bool aInElse = elseClause.Span.Contains(a.Span); + bool bInElse = elseClause.Span.Contains(b.Span); + if ((aInThen && bInElse) || (aInElse && bInThen)) return true; + break; + + case SwitchStatementSyntax switchStatement when switchStatement.Span.Contains(b.Span): + SwitchSectionSyntax? aSection = a.FirstAncestorOrSelf(); + SwitchSectionSyntax? bSection = b.FirstAncestorOrSelf(); + if (aSection != null && bSection != null && aSection != bSection) return true; + break; + + case ConditionalExpressionSyntax conditional: + bool aTrue = conditional.WhenTrue.Span.Contains(a.Span); + bool bTrue = conditional.WhenTrue.Span.Contains(b.Span); + bool aFalse = conditional.WhenFalse.Span.Contains(a.Span); + bool bFalse = conditional.WhenFalse.Span.Contains(b.Span); + if ((aTrue && bFalse) || (aFalse && bTrue)) return true; + break; + } + } + + return false; + } + + private static bool IsWorldInvocation( + InvocationExpressionSyntax invocation, + SemanticModel model, + INamedTypeSymbol worldType, + ImmutableHashSet methodNames, + System.Threading.CancellationToken ct) + { + if (model.GetSymbolInfo(invocation, ct).Symbol is not IMethodSymbol method) return false; + + return methodNames.Contains(method.Name) + && SymbolEqualityComparer.Default.Equals(method.ContainingType.OriginalDefinition, worldType); + } + + private static string GetMethodName(InvocationExpressionSyntax invocation, SemanticModel model, System.Threading.CancellationToken ct) => + model.GetSymbolInfo(invocation, ct).Symbol?.Name ?? "?"; + } +} diff --git a/Analyzers/DCL.Analyzers/VendoredCode.cs b/Analyzers/DCL.Analyzers/VendoredCode.cs new file mode 100644 index 00000000000..32a5c610b6d --- /dev/null +++ b/Analyzers/DCL.Analyzers/VendoredCode.cs @@ -0,0 +1,24 @@ +using Microsoft.CodeAnalysis; + +namespace DCL.Analyzers +{ + /// + /// Unity feeds a RoslynAnalyzer-labeled DLL to every compilation it drives - + /// including registry/git packages resolved into Library/PackageCache (observed: + /// DCLA005 firing in com.decentraland.pulse.transport and com.unity.cloud.ktx). + /// Vendored sources cannot be fixed in this repo, so every analyzer skips them; + /// project rules bind first-party code only. + /// + internal static class VendoredCode + { + public static bool IsVendored(SyntaxTree tree) + { + string path = tree.FilePath; + + if (string.IsNullOrEmpty(path)) + return false; + + return path.Replace('\\', '/').Contains("/PackageCache/"); + } + } +} diff --git a/Analyzers/README.md b/Analyzers/README.md new file mode 100644 index 00000000000..f52e5d3ad38 --- /dev/null +++ b/Analyzers/README.md @@ -0,0 +1,67 @@ +# DCL.Analyzers — Roslyn analyzers for unity-explorer + +Semantic checks running inside Unity's C# compilation (and Rider/VS), covering +what the regex layer (`scripts/lint/custom-rules.sh`) cannot: symbol +resolution, type checks, statement ordering. Full semantics live in each +analyzer's XML doc. + +Severities: Unity's csc **ignores** `.editorconfig` `dotnet_diagnostic` entries +(verified with a probe violation: it compiled as a warning despite an `error` +pin), so the corruption-class rules (DCLA001, DCLA005) carry +`DiagnosticSeverity.Error` in their descriptors — that is what fails the Unity +build. The pins in `Explorer/.editorconfig` govern IDEs and `dotnet` builds, +including the `**/Tests/**` downgrade, which is therefore IDE-only: Unity test +assemblies get the error severity too. + +## Rules + +| ID | Severity | What it catches | +|---|---|---| +| DCLA001 | error | `ref` local from `World.Get`/`TryGetRef` used after a structural change (`Add/Remove/Create/Destroy`) relocated it. Reachability-aware: exclusive branches, pre-call arguments, and the `x = ref World.Get(...)` re-fetch idiom stay silent; `CommandBuffer` never trips it. | +| DCLA002 | warning | Detached UniTask flow (`async UniTaskVoid`, same-file `.Forget()`) with no exception handling of its own. `Forget(handler)` counts as guarded. | +| DCLA003 | warning | Heap allocation in per-frame code: system `Update()`/`[Query]` bodies, plus any `[Utility.HotPath]` method in any assembly. Throw paths and `Exception` construction are exempt (error-path work). | +| DCLA004 | warning | Pooled `Get()` rental that provably never escapes or releases. Any escape silences it, including self-release member calls on non-collection rentals. | +| DCLA005 | error | Enum crossing a `[DllImport]` boundary (param, return, unmanaged-struct field) without an explicit underlying type. Source-declared enums only. | + +## Integration + +`Explorer/Assets/DCL/DCL.Analyzers.dll` carries the `RoslynAnalyzer` label +(all platforms disabled). In practice Unity feeds it to more compilations +than the label's folder suggests — including registry/git packages resolved +into `Library/PackageCache` (observed: DCLA005 firing inside +`com.decentraland.pulse.transport` and `com.unity.cloud.ktx`). Vendored +sources can't be fixed here, so the analyzers scope themselves: every rule +skips syntax trees whose path contains `/PackageCache/` (`VendoredCode.cs`); +first-party code under `Assets/` is analyzed in full. Built as +netstandard2.0 against Microsoft.CodeAnalysis.CSharp 4.3.1 (loads in any +Roslyn host ≥ 4.3; Unity 6000.x bundles ≥ 4.9). + +## Building + +```bash +bash scripts/build-analyzers.sh # tests + Release build + DLL sync +``` + +Self-wraps in `nix-shell -p dotnet-sdk_10` when dotnet is absent. The SDK is +pinned exactly in `Analyzers/global.json` — the build is deterministic +(`ContinuousIntegrationBuild=true`), and CI's "Fail on DLL drift" step +byte-compares its own rebuild against the committed DLL, so a stale or +out-of-band DLL cannot merge. The DLL is LFS-tracked; commit it with the +source change (CI tells you when you forget). + +Reproducibility notes, each learned from a real drift-gate failure: +`Analyzers/**` is pinned to LF in `.gitattributes` (the deterministic MVID +hashes source bytes, so a CRLF checkout builds different bytes); +`build-analyzers.sh` wipes `bin`/`obj` first (stale incremental state changed +the output); and `IncludeSourceRevisionInInformationalVersion` is off in the +csproj (the SDK's implicit SourceLink otherwise embeds the git HEAD sha, so a +committed DLL could never match a rebuild at any other commit). If the gate +still fails unexpectedly, the job uploads its own build as the +`DCL.Analyzers.dll-canonical` artifact — download, copy over +`Explorer/Assets/DCL/DCL.Analyzers.dll`, commit. + +## Adding a rule + +New `DiagnosticAnalyzer` (next `DCLA00N`) + tests stubbing external types by +metadata name (see any existing test file), `bash scripts/build-analyzers.sh`, +a row here, a severity pin in `Explorer/.editorconfig`. diff --git a/Analyzers/global.json b/Analyzers/global.json new file mode 100644 index 00000000000..1da1303fe7f --- /dev/null +++ b/Analyzers/global.json @@ -0,0 +1,7 @@ +{ + "sdk": { + "version": "10.0.204", + "rollForward": "disable" + }, + "//": "Pinned exactly: the 'Fail on DLL drift' CI step byte-compares a rebuild against the committed Explorer/Assets/DCL/DCL.Analyzers.dll, and byte-identical output requires the same Roslyn compiler. Bump this and rebuild/commit the DLL together (scripts/build-analyzers.sh)." +} diff --git a/CLAUDE.md b/CLAUDE.md index bca32e99590..9e6138b1734 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,6 +10,8 @@ Before writing or modifying any code, follow the code-standards skill for naming A `Stop` hook (`.claude/settings.json` → [`scripts/lint/lint-changed.sh`](scripts/lint/lint-changed.sh)) runs ReSharper InspectCode over the C# files changed in the session **using the exact same scripts, flags, and `.editorconfig` rules as CI** (`scripts/lint/{download-resharper,run-inspectcode,filter-warnings}.sh`, shared with `.github/workflows/test.yml`). Resolve any issues it reports in files you changed before finishing — they are real CI lint findings. If the ReSharper CLI isn't installed it prints how to get it (`bash scripts/lint/download-resharper.sh`) and does not block. It only inspects when `.cs` files changed. +Before ReSharper, the same hook runs [`scripts/lint/custom-rules.sh`](scripts/lint/custom-rules.sh) — deterministic regex rules distilled from this file and the skills (no `Debug.Log`, no new `ObjectProxy`, no `CheckNamespace` suppression, …), applied **only to lines added this session**, so pre-existing violations never block. CI runs the identical script over the PR diff (`custom-lint` job in `test.yml`). `BLOCK` findings stop the flow; `WARN` findings print but don't. For a genuinely sanctioned exception, suppress a single line with a trailing `// lint-ignore: ` — the suppression stays visible for reviewers to challenge. Semantic rules that need symbol resolution (e.g. DCLA001, ref-component use after a structural change) live in the Roslyn analyzer at [`Analyzers/`](Analyzers/README.md), shipped as `Explorer/Assets/DCL/DCL.Analyzers.dll` and rebuilt with `scripts/build-analyzers.sh`. When adding a rule, register it in the `rule` table in that script — one line, cited to its source doc — add a fixture line under `scripts/lint/tests/fixtures/`, and run `bash scripts/lint/tests/selftest.sh --regen` to update the goldens (plain `selftest.sh` to verify; CI runs it before every diff lint, so a dead or broken rule fails the build instead of passing silently). + --- ## Project Code Standards for Claude Reviews @@ -132,6 +134,7 @@ Reviewers have repeatedly identified AI-generated code by these smells. Check yo * **Wiring pooled/virtualized list items per rebind.** For item pools, wire callbacks once when the item is created, not every time `SetItemData` runs. Prefer an `Action` field (single subscriber, direct assignment) over C# `event` (`+=`/`-=` churn) when there is exactly one subscriber. * **Reimplementing primitives that already exist.** Before writing manual atlas UV math, check `TMP_Sprite Asset`. Before hand-batching profile lookups, check the batched `GetProfilesAsync(IReadOnlyList, ct)` overload. Before adding a bespoke event pathway, check `ViewEventBus` / `ChatEvents`. * **Comments that narrate caller/external behavior.** A comment must state only what the annotated code itself does or guarantees ("remove the corrupt file so the next read doesn't hit it"), never what callers or upper layers will do with the result ("so callers treat it as a miss and re-download"). External behavior can change without this code changing, silently turning the comment into a lie. +* **Nullable locals and the null-forgiving `!`.** Make invalid states unrepresentable: never declare a nullable local (`EventId? x = …; if (x == null) …`) — bind a non-nullable local with pattern matching instead (`if (source.Find(id) is not { } item) return;`). Never use `!` to silence nullability. The linter (`nullable-local`, `null-forgiving-suppression`) exempts the sanctioned idioms: `= null!`/`= default!` initializers on wire-format DTOs and `[SerializeField]` members, and the framework-forced split-phase trio `viewInstance!`/`Instance!`/`World!`, which have no NRT-clean rewrite. * **Suppressing `CheckNamespace` with a ReSharper comment.** Never add `// ReSharper disable once CheckNamespace` (or the file-wide variant) — fix the namespace or leave the warning visible. Rationale and full rule: [`docs/code-style-guidelines.md` § Namespaces](docs/code-style-guidelines.md#namespaces). ### Other project-specific rules diff --git a/Explorer/.editorconfig b/Explorer/.editorconfig index ae11f6c248e..ac1a75395af 100644 --- a/Explorer/.editorconfig +++ b/Explorer/.editorconfig @@ -815,3 +815,17 @@ resharper_space_after_operator_keyword = true # resharper_web_config_wrong_module_highlighting = warning # resharper_wrong_indent_size_highlighting = hint + +# DCL.Analyzers (Analyzers/README.md). Corruption-class rules fail the build; +# the rest are advisory while their real-world precision is established. +dotnet_diagnostic.DCLA001.severity = error # ref component used after structural change +dotnet_diagnostic.DCLA002.severity = warning # detached UniTask flow swallows exceptions +dotnet_diagnostic.DCLA003.severity = warning # allocation in system Update +dotnet_diagnostic.DCLA004.severity = warning # pooled rental provably leaks +dotnet_diagnostic.DCLA005.severity = error # FFI enum without explicit underlying type + +# Tests exercise structural-change scenarios deliberately; corruption-class +# rules stay visible there but never fail the build. +[**/Tests/**] +dotnet_diagnostic.DCLA001.severity = warning +dotnet_diagnostic.DCLA005.severity = warning diff --git a/Explorer/Assets/DCL/Audio/csc.rsp b/Explorer/Assets/DCL/Audio/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Audio/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/AvatarRendering/Emotes/Systems/csc.rsp b/Explorer/Assets/DCL/AvatarRendering/Emotes/Systems/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/AvatarRendering/Emotes/Systems/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/AvatarRendering/Emotes/csc.rsp b/Explorer/Assets/DCL/AvatarRendering/Emotes/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/AvatarRendering/Emotes/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/Tests/csc.rsp b/Explorer/Assets/DCL/AvatarRendering/Thumbnails/Tests/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/Tests/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/csc.rsp b/Explorer/Assets/DCL/AvatarRendering/Thumbnails/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/csc.rsp.meta b/Explorer/Assets/DCL/AvatarRendering/Thumbnails/csc.rsp.meta deleted file mode 100644 index 2147b3fd81a..00000000000 --- a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: f6bbd636afa946aa81c7bbcfcffe1b35 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/AvatarRendering/Wearables/Tests/csc.rsp b/Explorer/Assets/DCL/AvatarRendering/Wearables/Tests/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/AvatarRendering/Wearables/Tests/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/AvatarRendering/Wearables/Tests/csc.rsp.meta b/Explorer/Assets/DCL/AvatarRendering/Wearables/Tests/csc.rsp.meta deleted file mode 100644 index 58501000d9d..00000000000 --- a/Explorer/Assets/DCL/AvatarRendering/Wearables/Tests/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: b9d3f4b09bc14d6bac669160f8aee9a9 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Character/CharacterCamera/IExposedCameraData.cs b/Explorer/Assets/DCL/Character/CharacterCamera/IExposedCameraData.cs index 9f105a51e4e..ef49d90674e 100644 --- a/Explorer/Assets/DCL/Character/CharacterCamera/IExposedCameraData.cs +++ b/Explorer/Assets/DCL/Character/CharacterCamera/IExposedCameraData.cs @@ -60,6 +60,13 @@ public interface IExposedCameraData : IExposedTransform CinemachineBrain? CinemachineBrain { get; set; } CameraMode CameraMode { get; set; } + /// + /// The camera the brain currently drives. Lets consumers reach the rendering + /// camera without referencing the Cinemachine assembly (CinemachineBrain in a + /// member signature forces the reference onto every consuming asmdef). + /// + Camera? OutputCamera => CinemachineBrain != null ? CinemachineBrain.OutputCamera : null; + public class Fake : IExposedCameraData { public CanBeDirty WorldPosition { get; } diff --git a/Explorer/Assets/DCL/Character/CharacterCamera/Systems/csc.rsp b/Explorer/Assets/DCL/Character/CharacterCamera/Systems/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Character/CharacterCamera/Systems/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Character/CharacterCamera/Systems/csc.rsp.meta b/Explorer/Assets/DCL/Character/CharacterCamera/Systems/csc.rsp.meta deleted file mode 100644 index 1d79e108bdf..00000000000 --- a/Explorer/Assets/DCL/Character/CharacterCamera/Systems/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: e53f4b7a5c3148309c357c878347f6f4 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Character/CharacterCamera/csc.rsp b/Explorer/Assets/DCL/Character/CharacterCamera/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Character/CharacterCamera/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Character/CharacterCamera/csc.rsp.meta b/Explorer/Assets/DCL/Character/CharacterCamera/csc.rsp.meta deleted file mode 100644 index 2761017873d..00000000000 --- a/Explorer/Assets/DCL/Character/CharacterCamera/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 0b7b28c05f874dd9831add2181ecd475 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Quality/Container/csc.rsp b/Explorer/Assets/DCL/Character/CharacterObject/csc.rsp similarity index 100% rename from Explorer/Assets/DCL/Quality/Container/csc.rsp rename to Explorer/Assets/DCL/Character/CharacterObject/csc.rsp diff --git a/Explorer/Assets/DCL/AvatarRendering/Emotes/csc.rsp.meta b/Explorer/Assets/DCL/Character/CharacterObject/csc.rsp.meta similarity index 74% rename from Explorer/Assets/DCL/AvatarRendering/Emotes/csc.rsp.meta rename to Explorer/Assets/DCL/Character/CharacterObject/csc.rsp.meta index a1a98f1cfce..041039b25bd 100644 --- a/Explorer/Assets/DCL/AvatarRendering/Emotes/csc.rsp.meta +++ b/Explorer/Assets/DCL/Character/CharacterObject/csc.rsp.meta @@ -1,5 +1,5 @@ fileFormatVersion: 2 -guid: c213f15ade4a4625a587ea1e2f08c375 +guid: 4133665308314f4a7e372211f675da19 DefaultImporter: externalObjects: {} userData: diff --git a/Explorer/Assets/DCL/Character/CharacterPreview/CharacterPreviewController.cs b/Explorer/Assets/DCL/Character/CharacterPreview/CharacterPreviewController.cs index 350c880a19b..1de198ae9dd 100644 --- a/Explorer/Assets/DCL/Character/CharacterPreview/CharacterPreviewController.cs +++ b/Explorer/Assets/DCL/Character/CharacterPreview/CharacterPreviewController.cs @@ -92,18 +92,22 @@ public UniTask UpdateAvatarAsync(CharacterPreviewAvatarModel avatarModel, Cancel { ct.ThrowIfCancellationRequested(); - ref AvatarShapeComponent avatarShape = ref globalWorld.Get(characterPreviewEntity); - - avatarShape.SkinColor = avatarModel.SkinColor; - avatarShape.HairColor = avatarModel.HairColor; - avatarShape.EyesColor = avatarModel.EyesColor; - avatarShape.BodyShape = BodyShape.FromStringSafe(avatarModel.BodyShape); - - avatarShape.WearablePromise.ForgetLoading(globalWorld); - - avatarShape.WearablePromise = AssetPromise.Create( + BodyShape bodyShape = BodyShape.FromStringSafe(avatarModel.BodyShape); + + // structural work first, results into locals: promise creation/destruction and + // entity creation relocate archetype chunks and invalidate any outstanding ref + // to this component - including the structural changes hidden inside + // Promise.Create and ForgetLoading. + // The old promise is deliberately COPIED out (not operated via ref): calling + // ForgetLoading on a ref into the chunk would keep 'this' pointing at ECS memory + // while it destroys the promise entity. The copy's mutations are discarded - + // the component's WearablePromise is overwritten wholesale below. + AssetPromise oldWearablePromise = globalWorld.Get(characterPreviewEntity).WearablePromise; + oldWearablePromise.ForgetLoading(globalWorld); + + var wearablePromise = AssetPromise.Create( globalWorld, - WearableComponentsUtils.CreateGetWearablesByPointersIntention(avatarShape.BodyShape, + WearableComponentsUtils.CreateGetWearablesByPointersIntention(bodyShape, avatarModel.Wearables ?? (IReadOnlyCollection)Array.Empty(), avatarModel.ForceRenderCategories), PartitionComponent.TOP_PRIORITY ); @@ -111,10 +115,18 @@ public UniTask UpdateAvatarAsync(CharacterPreviewAvatarModel avatarModel, Cancel Entity emotePromiseEntity = builderEmotesPreview ? Entity.Null : globalWorld.Create(EmotePromise.Create(globalWorld, - EmoteComponentsUtils.CreateGetEmotesByPointersIntention(avatarShape.BodyShape, + EmoteComponentsUtils.CreateGetEmotesByPointersIntention(bodyShape, avatarModel.Emotes ?? (IReadOnlyCollection)Array.Empty()), PartitionComponent.TOP_PRIORITY)); + // the ref is taken only after every structural change has completed (DCLA001) + ref AvatarShapeComponent avatarShape = ref globalWorld.Get(characterPreviewEntity); + + avatarShape.SkinColor = avatarModel.SkinColor; + avatarShape.HairColor = avatarModel.HairColor; + avatarShape.EyesColor = avatarModel.EyesColor; + avatarShape.BodyShape = bodyShape; + avatarShape.WearablePromise = wearablePromise; avatarShape.IsDirty = true; return WaitForAvatarInstantiatedAsync(emotePromiseEntity, ct); diff --git a/Explorer/Assets/DCL/Chat/_Refactor/ChatPanelPresenter.cs b/Explorer/Assets/DCL/Chat/_Refactor/ChatPanelPresenter.cs index 9f65ba94597..d3ba2bf539b 100644 --- a/Explorer/Assets/DCL/Chat/_Refactor/ChatPanelPresenter.cs +++ b/Explorer/Assets/DCL/Chat/_Refactor/ChatPanelPresenter.cs @@ -8,6 +8,7 @@ using DCL.Chat.ChatReactions.Networking; using DCL.Chat.ChatReactions.Presenters; using DCL.Chat.ChatServices; +using DCL.CharacterCamera; using DCL.FeatureFlags; using DCL.Input; using DCL.Chat.ChatServices.ChatContextService; @@ -79,7 +80,8 @@ public ChatPanelPresenter(ChatPanelView view, ChatMessageReactionService messageReactionService, IWeb3IdentityCache web3IdentityCache, IProfileCache profileCache, - IInputBlock inputBlock) + IInputBlock inputBlock, + IExposedCameraData exposedCameraData) { this.chatSharedAreaEventBus = chatSharedAreaEventBus; this.chatMemberListService = chatMemberListService; @@ -229,6 +231,7 @@ public ChatPanelPresenter(ChatPanelView view, reactionSimulation, reactionsConfig, reactionDebugState, + exposedCameraData, reactionDebugController, view.ChatReactionButton.ReactionButton); } diff --git a/Explorer/Assets/DCL/Chat/_Refactor/ChatReactions/Presenters/SituationalReactionPresenter.cs b/Explorer/Assets/DCL/Chat/_Refactor/ChatReactions/Presenters/SituationalReactionPresenter.cs index b6853f36f9c..c1a9be7f55e 100644 --- a/Explorer/Assets/DCL/Chat/_Refactor/ChatReactions/Presenters/SituationalReactionPresenter.cs +++ b/Explorer/Assets/DCL/Chat/_Refactor/ChatReactions/Presenters/SituationalReactionPresenter.cs @@ -1,6 +1,7 @@ using System; using System.Threading; using Cysharp.Threading.Tasks; +using DCL.CharacterCamera; using DCL.Chat.ChatReactions.Configs; using DCL.Chat.ChatReactions.Core; using DCL.Chat.ChatReactions.Debug; @@ -20,6 +21,7 @@ public sealed class SituationalReactionPresenter : IDisposable private readonly ChatReactionDebugState debugState; private readonly SituationalReactionDebugController? debugController; private readonly RectTransform? debugButtonRect; + private readonly IExposedCameraData exposedCameraData; private readonly CancellationTokenSource cts = new (); private Camera? cachedMainCamera; @@ -30,12 +32,14 @@ public sealed class SituationalReactionPresenter : IDisposable internal SituationalReactionPresenter(ISituationalReactionSimulation service, ChatReactionsConfig config, ChatReactionDebugState debugState, + IExposedCameraData exposedCameraData, SituationalReactionDebugController? debugController = null, Button? debugButton = null) { this.service = service; this.config = config; this.debugState = debugState; + this.exposedCameraData = exposedCameraData; this.debugController = debugController; this.debugButtonRect = debugButton != null ? debugButton.GetComponent() : null; @@ -62,7 +66,7 @@ private async UniTask UpdateLoopAsync(CancellationToken ct) float dt = UnityEngine.Time.unscaledDeltaTime; if (cachedMainCamera == null) - cachedMainCamera = Camera.main; + cachedMainCamera = exposedCameraData.OutputCamera; Profiler.BeginSample("ChatReactions.Tick"); service.Tick(dt); diff --git a/Explorer/Assets/DCL/Chat/csc.rsp b/Explorer/Assets/DCL/Chat/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Chat/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Chat/csc.rsp.meta b/Explorer/Assets/DCL/Chat/csc.rsp.meta deleted file mode 100644 index 5710adf7261..00000000000 --- a/Explorer/Assets/DCL/Chat/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: cab872bc1130470a9466b10617c73a66 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/CommunicationData/csc.rsp b/Explorer/Assets/DCL/CommunicationData/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/CommunicationData/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/CommunicationData/csc.rsp.meta b/Explorer/Assets/DCL/CommunicationData/csc.rsp.meta deleted file mode 100644 index 4d312e28b45..00000000000 --- a/Explorer/Assets/DCL/CommunicationData/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 435fc8aa916b48389c7d418905e753fe -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/DCL.Analyzers.dll b/Explorer/Assets/DCL/DCL.Analyzers.dll new file mode 100644 index 00000000000..9886fcee103 --- /dev/null +++ b/Explorer/Assets/DCL/DCL.Analyzers.dll @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d0f55f156ee18a10f55a1bba670f1f3d429a539128111397cfd6a089b1a2aafb +size 35840 diff --git a/Explorer/Assets/DCL/DCL.Analyzers.dll.meta b/Explorer/Assets/DCL/DCL.Analyzers.dll.meta new file mode 100644 index 00000000000..abb3ff62bcc --- /dev/null +++ b/Explorer/Assets/DCL/DCL.Analyzers.dll.meta @@ -0,0 +1,52 @@ +fileFormatVersion: 2 +guid: 43d21fc863d77d732733238d128c3d80 +labels: +- RoslynAnalyzer +PluginImporter: + externalObjects: {} + serializedVersion: 3 + iconMap: {} + executionOrder: {} + defineConstraints: [] + isPreloaded: 0 + isOverridable: 0 + isExplicitlyReferenced: 0 + validateReferences: 1 + platformData: + Any: + enabled: 0 + settings: + Exclude Editor: 1 + Exclude Linux64: 1 + Exclude OSXUniversal: 1 + Exclude Win: 1 + Exclude Win64: 1 + Editor: + enabled: 0 + settings: + CPU: AnyCPU + DefaultValueInitialized: true + OS: AnyOS + Linux64: + enabled: 0 + settings: + CPU: None + OSXUniversal: + enabled: 0 + settings: + CPU: None + Win: + enabled: 0 + settings: + CPU: None + Win64: + enabled: 0 + settings: + CPU: None + WindowsStoreApps: + enabled: 0 + settings: + CPU: AnyCPU + userData: + assetBundleName: + assetBundleVariant: diff --git a/Explorer/Assets/DCL/Donations/csc.rsp b/Explorer/Assets/DCL/Donations/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Donations/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Donations/csc.rsp.meta b/Explorer/Assets/DCL/Donations/csc.rsp.meta deleted file mode 100644 index d99aa5ae20a..00000000000 --- a/Explorer/Assets/DCL/Donations/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: e001d31662e0c49beac06d202c6f417b -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/ECS/GlobalPartitioning/PartitionGlobalAssetEntitiesSystem.cs b/Explorer/Assets/DCL/ECS/GlobalPartitioning/PartitionGlobalAssetEntitiesSystem.cs index 86ee7220afd..48ffe0b4e35 100644 --- a/Explorer/Assets/DCL/ECS/GlobalPartitioning/PartitionGlobalAssetEntitiesSystem.cs +++ b/Explorer/Assets/DCL/ECS/GlobalPartitioning/PartitionGlobalAssetEntitiesSystem.cs @@ -12,7 +12,6 @@ using ECS.Unity.Systems; using UnityEngine; -// ReSharper disable once CheckNamespace (Code generation issues) namespace DCL.Systems { /// diff --git a/Explorer/Assets/DCL/Editor/csc.rsp b/Explorer/Assets/DCL/Editor/csc.rsp new file mode 100644 index 00000000000..2d66b0292b8 --- /dev/null +++ b/Explorer/Assets/DCL/Editor/csc.rsp @@ -0,0 +1 @@ +-nullable:enable diff --git a/Explorer/Assets/DCL/Audio/csc.rsp.meta b/Explorer/Assets/DCL/Editor/csc.rsp.meta similarity index 74% rename from Explorer/Assets/DCL/Audio/csc.rsp.meta rename to Explorer/Assets/DCL/Editor/csc.rsp.meta index db2b89945f2..82bb81b9868 100644 --- a/Explorer/Assets/DCL/Audio/csc.rsp.meta +++ b/Explorer/Assets/DCL/Editor/csc.rsp.meta @@ -1,5 +1,5 @@ fileFormatVersion: 2 -guid: 2792329bb26f40e8bde0340eba946ada +guid: c41f72bbe551d0579d0c56c2804b2a86 DefaultImporter: externalObjects: {} userData: diff --git a/Explorer/Assets/DCL/EmotesWheel/csc.rsp b/Explorer/Assets/DCL/EmotesWheel/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/EmotesWheel/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/EmotesWheel/csc.rsp.meta b/Explorer/Assets/DCL/EmotesWheel/csc.rsp.meta deleted file mode 100644 index b068776e704..00000000000 --- a/Explorer/Assets/DCL/EmotesWheel/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: f4b58eb1e7ad410697df6628a0a79583 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/ExplorePanel/csc.rsp b/Explorer/Assets/DCL/ExplorePanel/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/ExplorePanel/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/ExplorePanel/csc.rsp.meta b/Explorer/Assets/DCL/ExplorePanel/csc.rsp.meta deleted file mode 100644 index 883e2d518ad..00000000000 --- a/Explorer/Assets/DCL/ExplorePanel/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 840f4f99c6374542935f5deca482d825 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/FeatureFlags/FeatureFlagsStrings.cs b/Explorer/Assets/DCL/FeatureFlags/FeatureFlagsStrings.cs index 3ddab4ea6e6..e337eb014b6 100644 --- a/Explorer/Assets/DCL/FeatureFlags/FeatureFlagsStrings.cs +++ b/Explorer/Assets/DCL/FeatureFlags/FeatureFlagsStrings.cs @@ -1,6 +1,5 @@ using System; -// ReSharper disable once CheckNamespace namespace DCL.FeatureFlags { [Serializable] diff --git a/Explorer/Assets/DCL/FeatureFlags/Tests/csc.rsp b/Explorer/Assets/DCL/FeatureFlags/Tests/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/FeatureFlags/Tests/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/FeatureFlags/Tests/csc.rsp.meta b/Explorer/Assets/DCL/FeatureFlags/Tests/csc.rsp.meta deleted file mode 100644 index 492f5816260..00000000000 --- a/Explorer/Assets/DCL/FeatureFlags/Tests/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 0f52f70213964fe79c3496d07bf7dabc -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/FeatureFlags/csc.rsp b/Explorer/Assets/DCL/FeatureFlags/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/FeatureFlags/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/FeatureFlags/csc.rsp.meta b/Explorer/Assets/DCL/FeatureFlags/csc.rsp.meta deleted file mode 100644 index 084153bef6c..00000000000 --- a/Explorer/Assets/DCL/FeatureFlags/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: c3b7ffec8b724eaaa69a9ca2b04a969f -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Friends/csc.rsp b/Explorer/Assets/DCL/Friends/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Friends/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Friends/csc.rsp.meta b/Explorer/Assets/DCL/Friends/csc.rsp.meta deleted file mode 100644 index 756d1a8b602..00000000000 --- a/Explorer/Assets/DCL/Friends/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 56be14701d9843f98517b7ba0fcea7f1 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/InWorldCamera/CameraReelGallery/csc.rsp b/Explorer/Assets/DCL/InWorldCamera/CameraReelGallery/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/InWorldCamera/CameraReelGallery/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/InWorldCamera/CameraReelGallery/csc.rsp.meta b/Explorer/Assets/DCL/InWorldCamera/CameraReelGallery/csc.rsp.meta deleted file mode 100644 index 172764e56e4..00000000000 --- a/Explorer/Assets/DCL/InWorldCamera/CameraReelGallery/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 446864531426406e88f41c601f2384c5 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Tests/csc.rsp b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Tests/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Tests/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Tests/csc.rsp.meta b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Tests/csc.rsp.meta deleted file mode 100644 index fe639f48164..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Tests/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: a021ddf40af14d23bb80716a9256f5fe -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs index 6816291b036..b839affa8b2 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs @@ -1,4 +1,3 @@ -// ReSharper disable once CheckNamespace namespace Global.AppArgs { public static class AppArgsFlags diff --git a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/ChatContainer.cs b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/ChatContainer.cs index fc5c75d6b5c..d62478f2880 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/ChatContainer.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/ChatContainer.cs @@ -228,6 +228,7 @@ public ChatPlugin CreatePlugin( bootstrapContainer.Environment, bootstrapContainer.Analytics.Controller, StreamReactionsChatCommand, + staticContainer.ExposedGlobalDataContainer.ExposedCameraData, CurrentChannelService); public void Dispose() diff --git a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/MainSceneLoader.cs b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/MainSceneLoader.cs index 94f18c29bc5..a771e9d5cf4 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/MainSceneLoader.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/MainSceneLoader.cs @@ -58,7 +58,6 @@ using Utility; using MinimumSpecsScreenView = DCL.ApplicationGuards.MinimumSpecsScreenView; -// ReSharper disable once CheckNamespace namespace Global.Dynamic { public class MainSceneLoader : MonoBehaviour, ICoroutineRunner diff --git a/Explorer/Assets/DCL/Infrastructure/Global/IsExternalInit.cs b/Explorer/Assets/DCL/Infrastructure/Global/IsExternalInit.cs index 297fdbf0aa2..fe45a36d5cd 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/IsExternalInit.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/IsExternalInit.cs @@ -1,4 +1,3 @@ -// ReSharper disable once CheckNamespace namespace System.Runtime.CompilerServices { internal class IsExternalInit { } diff --git a/Explorer/Assets/DCL/Infrastructure/MVC/ViewDependencies/csc.rsp b/Explorer/Assets/DCL/Infrastructure/MVC/ViewDependencies/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/MVC/ViewDependencies/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/MVC/ViewDependencies/csc.rsp.meta b/Explorer/Assets/DCL/Infrastructure/MVC/ViewDependencies/csc.rsp.meta deleted file mode 100644 index 2b3771abd82..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/MVC/ViewDependencies/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: f41d7734838848349f04f0a2d8d34b48 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Infrastructure/ProtobufPartialClasses/csc.rsp b/Explorer/Assets/DCL/Infrastructure/ProtobufPartialClasses/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/ProtobufPartialClasses/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/ProtobufPartialClasses/csc.rsp.meta b/Explorer/Assets/DCL/Infrastructure/ProtobufPartialClasses/csc.rsp.meta deleted file mode 100644 index 8f7b8e4565d..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/ProtobufPartialClasses/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: d9f69180f03c406c97a2d7804fc34297 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/SceneFacade/Systems/csc.rsp b/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/SceneFacade/Systems/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/SceneFacade/Systems/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/SceneFacade/Systems/csc.rsp.meta b/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/SceneFacade/Systems/csc.rsp.meta deleted file mode 100644 index 4542a827287..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/SceneFacade/Systems/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 88e41beb104142c1940b12220f1bad3f -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/Systems/csc.rsp b/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/Systems/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/Systems/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/Systems/csc.rsp.meta b/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/Systems/csc.rsp.meta deleted file mode 100644 index 5ce1e43f81f..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/Systems/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 283a4a1f75b7407aac112cbe216ea859 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Infrastructure/SceneRunner/csc.rsp b/Explorer/Assets/DCL/Infrastructure/SceneRunner/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/SceneRunner/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/SceneRunner/csc.rsp.meta b/Explorer/Assets/DCL/Infrastructure/SceneRunner/csc.rsp.meta deleted file mode 100644 index c578b66d47c..00000000000 --- a/Explorer/Assets/DCL/Infrastructure/SceneRunner/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 0e6b18e23daf408b8b623d9ab0e0a596 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Infrastructure/Utility/HotPathAttribute.cs b/Explorer/Assets/DCL/Infrastructure/Utility/HotPathAttribute.cs new file mode 100644 index 00000000000..054fabad3f1 --- /dev/null +++ b/Explorer/Assets/DCL/Infrastructure/Utility/HotPathAttribute.cs @@ -0,0 +1,14 @@ +using System; + +namespace Utility +{ + /// + /// Marks a method as per-frame/per-call hot: DCLA003 enforces the same + /// allocation-freedom on its body as on system Update() methods. Apply to + /// code invoked at frame rate or per network/URL operation outside ECS systems. + /// Methods only: the analyzer does not inspect constructors, so allowing the + /// attribute there would create unchecked (false-safety) annotations. + /// + [AttributeUsage(AttributeTargets.Method)] + public sealed class HotPathAttribute : Attribute { } +} diff --git a/Explorer/Assets/DCL/Infrastructure/Utility/HotPathAttribute.cs.meta b/Explorer/Assets/DCL/Infrastructure/Utility/HotPathAttribute.cs.meta new file mode 100644 index 00000000000..18b754e36be --- /dev/null +++ b/Explorer/Assets/DCL/Infrastructure/Utility/HotPathAttribute.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d8caae9d0bc4a534950f9d678f6187bc +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Explorer/Assets/DCL/Landscape/Config/Editor/csc.rsp b/Explorer/Assets/DCL/Landscape/Config/Editor/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Landscape/Config/Editor/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Landscape/Config/Editor/csc.rsp.meta b/Explorer/Assets/DCL/Landscape/Config/Editor/csc.rsp.meta deleted file mode 100644 index b4fb5a0fd65..00000000000 --- a/Explorer/Assets/DCL/Landscape/Config/Editor/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 86eff37a4c5148a4b1de64fa15cd957a -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/MarketplaceCredits/MarketplaceCreditsAPIService/csc.rsp b/Explorer/Assets/DCL/MarketplaceCredits/MarketplaceCreditsAPIService/csc.rsp new file mode 100644 index 00000000000..2d66b0292b8 --- /dev/null +++ b/Explorer/Assets/DCL/MarketplaceCredits/MarketplaceCreditsAPIService/csc.rsp @@ -0,0 +1 @@ +-nullable:enable diff --git a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/Tests/csc.rsp.meta b/Explorer/Assets/DCL/MarketplaceCredits/MarketplaceCreditsAPIService/csc.rsp.meta similarity index 74% rename from Explorer/Assets/DCL/AvatarRendering/Thumbnails/Tests/csc.rsp.meta rename to Explorer/Assets/DCL/MarketplaceCredits/MarketplaceCreditsAPIService/csc.rsp.meta index c304216c026..82864df5e61 100644 --- a/Explorer/Assets/DCL/AvatarRendering/Thumbnails/Tests/csc.rsp.meta +++ b/Explorer/Assets/DCL/MarketplaceCredits/MarketplaceCreditsAPIService/csc.rsp.meta @@ -1,5 +1,5 @@ fileFormatVersion: 2 -guid: 48ed40d61924433a9592bea87dc71b03 +guid: 8b5eded193ae4f60be40ae821e73943e DefaultImporter: externalObjects: {} userData: diff --git a/Explorer/Assets/DCL/Multiplayer/Connections/Systems/RoomIndicator/DebugRoomsSystem.Indicator.cs b/Explorer/Assets/DCL/Multiplayer/Connections/Systems/RoomIndicator/DebugRoomsSystem.Indicator.cs index 48b89c52f1c..c57f647f94c 100644 --- a/Explorer/Assets/DCL/Multiplayer/Connections/Systems/RoomIndicator/DebugRoomsSystem.Indicator.cs +++ b/Explorer/Assets/DCL/Multiplayer/Connections/Systems/RoomIndicator/DebugRoomsSystem.Indicator.cs @@ -10,7 +10,6 @@ using ECS.LifeCycle.Components; using UnityEngine.Pool; -// ReSharper disable once CheckNamespace namespace DCL.Multiplayer.Connections.Systems { public partial class DebugRoomsSystem diff --git a/Explorer/Assets/DCL/Multiplayer/Movement/Systems/csc.rsp b/Explorer/Assets/DCL/Multiplayer/Movement/Systems/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Multiplayer/Movement/Systems/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Multiplayer/Movement/Systems/csc.rsp.meta b/Explorer/Assets/DCL/Multiplayer/Movement/Systems/csc.rsp.meta deleted file mode 100644 index 1eb0f10c8ed..00000000000 --- a/Explorer/Assets/DCL/Multiplayer/Movement/Systems/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 55a45bf7cb6246bab5195129626f2d48 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Multiplayer/Movement/Tests/csc.rsp b/Explorer/Assets/DCL/Multiplayer/Movement/Tests/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Multiplayer/Movement/Tests/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Multiplayer/Movement/Tests/csc.rsp.meta b/Explorer/Assets/DCL/Multiplayer/Movement/Tests/csc.rsp.meta deleted file mode 100644 index a20d84dbc88..00000000000 --- a/Explorer/Assets/DCL/Multiplayer/Movement/Tests/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 2680b1905e914864928b60eb0f50ea61 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Navmap/NavmapBus/csc.rsp b/Explorer/Assets/DCL/Navmap/NavmapBus/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Navmap/NavmapBus/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Navmap/NavmapBus/csc.rsp.meta b/Explorer/Assets/DCL/Navmap/NavmapBus/csc.rsp.meta deleted file mode 100644 index e4ba6bb7d7d..00000000000 --- a/Explorer/Assets/DCL/Navmap/NavmapBus/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 7c18839b009f472aa2204807adf9bf9b -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Navmap/csc.rsp b/Explorer/Assets/DCL/Navmap/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Navmap/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Navmap/csc.rsp.meta b/Explorer/Assets/DCL/Navmap/csc.rsp.meta deleted file mode 100644 index baef41337b5..00000000000 --- a/Explorer/Assets/DCL/Navmap/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 01aa1cdf5c7a4dc799cc350501c59d03 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsSource.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsSource.cs index 99d457b8e59..ea4ffd3d5a0 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsSource.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsSource.cs @@ -8,7 +8,6 @@ using System.Linq; using UnityEngine.Pool; -// ReSharper disable once CheckNamespace namespace DCL.Browser.DecentralandUrls { public class DecentralandUrlsSource : IDecentralandUrlsSource diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsUtils.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsUtils.cs index 0bfbc3d0026..c9b7ddda602 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsUtils.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/DecentralandUrlsUtils.cs @@ -2,7 +2,6 @@ using DCL.Optimization.ThreadSafePool; using UnityEngine.Pool; -// ReSharper disable once CheckNamespace namespace DCL.Multiplayer.Connections.DecentralandUrls { public static class DecentralandUrlsUtils diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/GatewayUrlsSource.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/GatewayUrlsSource.cs index 0d53f311d6c..8507249ee91 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/Browser/GatewayUrlsSource.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/GatewayUrlsSource.cs @@ -8,7 +8,6 @@ using System.Linq; using Utility; -// ReSharper disable once CheckNamespace namespace DCL.Browser { public class GatewayUrlsSource : DecentralandUrlsSource diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/SupportRequestService.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/SupportRequestService.cs index 682ef875ebd..e746347dc37 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/Browser/SupportRequestService.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/SupportRequestService.cs @@ -1,7 +1,6 @@ using DCL.Multiplayer.Connections.DecentralandUrls; using System; -// ReSharper disable once CheckNamespace namespace DCL.Browser { public class SupportRequestService diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/DecentralandUrlsSourceShould.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/DecentralandUrlsSourceShould.cs index 0e366985d96..bb02625fd8f 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/DecentralandUrlsSourceShould.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/DecentralandUrlsSourceShould.cs @@ -7,7 +7,6 @@ using System; using System.Collections.Generic; -// ReSharper disable once CheckNamespace namespace DCL.Browser.DecentralandUrls.Tests { public class DecentralandUrlsSourceShould diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs index b442a2fcdd8..0b17245e707 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs @@ -3,7 +3,6 @@ using System; using UnityEngine; -// ReSharper disable once CheckNamespace namespace DCL.Browser { public class UnityAppWebBrowser diff --git a/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionBase.cs b/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionBase.cs index 57a0ba07c9c..9d9d7e95d74 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionBase.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionBase.cs @@ -2,7 +2,6 @@ using System; // ReSharper disable InconsistentNaming -// ReSharper disable once CheckNamespace namespace DCL.Ipfs { /// diff --git a/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionGeneric.cs b/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionGeneric.cs index 0e21666d508..14b98734e3b 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionGeneric.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/EntityDefinitionGeneric.cs @@ -2,7 +2,6 @@ using System.Linq; // ReSharper disable InconsistentNaming -// ReSharper disable once CheckNamespace namespace DCL.Ipfs { // Server schema: decentraland/common-schemas src/platform/entity.ts#/Entity diff --git a/Explorer/Assets/DCL/NetworkDefinitions/LocalIpfsRealm.cs b/Explorer/Assets/DCL/NetworkDefinitions/LocalIpfsRealm.cs index 1d45083870d..adb7822b1dd 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/LocalIpfsRealm.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/LocalIpfsRealm.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; -// ReSharper disable once CheckNamespace namespace DCL.Ipfs { /// diff --git a/Explorer/Assets/DCL/NetworkDefinitions/LogIpfsRealm.cs b/Explorer/Assets/DCL/NetworkDefinitions/LogIpfsRealm.cs index 8965f762cc7..662220d3de8 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/LogIpfsRealm.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/LogIpfsRealm.cs @@ -2,7 +2,6 @@ using DCL.Diagnostics; using System.Collections.Generic; -// ReSharper disable once CheckNamespace namespace DCL.Ipfs { public class LogIpfsRealm : IIpfsRealm diff --git a/Explorer/Assets/DCL/NetworkDefinitions/RealmData.cs b/Explorer/Assets/DCL/NetworkDefinitions/RealmData.cs index 5ccc5c314ea..86d7e61e246 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/RealmData.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/RealmData.cs @@ -4,7 +4,6 @@ using System; using System.Text; -// ReSharper disable once CheckNamespace namespace ECS { /// diff --git a/Explorer/Assets/DCL/NetworkDefinitions/SceneFastLookup.cs b/Explorer/Assets/DCL/NetworkDefinitions/SceneFastLookup.cs index 936d8ff4013..6b731b7aaca 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/SceneFastLookup.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/SceneFastLookup.cs @@ -2,7 +2,6 @@ using System.Linq; using UnityEngine; -// ReSharper disable once CheckNamespace namespace DCL.Ipfs { public readonly struct SceneFastLookup diff --git a/Explorer/Assets/DCL/NetworkDefinitions/SceneMetadata.cs b/Explorer/Assets/DCL/NetworkDefinitions/SceneMetadata.cs index 0c0a2911c0b..44b0986ea65 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/SceneMetadata.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/SceneMetadata.cs @@ -4,7 +4,6 @@ using UnityEngine; using System.ComponentModel; -// ReSharper disable once CheckNamespace namespace DCL.Ipfs { [Serializable] diff --git a/Explorer/Assets/DCL/NetworkDefinitions/SceneParcelsConverter.cs b/Explorer/Assets/DCL/NetworkDefinitions/SceneParcelsConverter.cs index 32ed139ee21..cbcf4a75980 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/SceneParcelsConverter.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/SceneParcelsConverter.cs @@ -5,7 +5,6 @@ using UnityEngine; using UnityEngine.Scripting; -// ReSharper disable once CheckNamespace namespace DCL.Ipfs { /// diff --git a/Explorer/Assets/DCL/NetworkDefinitions/WorldManifest.cs b/Explorer/Assets/DCL/NetworkDefinitions/WorldManifest.cs index 9e4a1c80d93..686710e40e7 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/WorldManifest.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/WorldManifest.cs @@ -2,7 +2,6 @@ using Unity.Collections; using Unity.Mathematics; -// ReSharper disable once CheckNamespace namespace ECS { /// diff --git a/Explorer/Assets/DCL/PerformanceAndDiagnostics/Analytics/Playgrounds/csc.rsp b/Explorer/Assets/DCL/PerformanceAndDiagnostics/Analytics/Playgrounds/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/PerformanceAndDiagnostics/Analytics/Playgrounds/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/PerformanceAndDiagnostics/Analytics/Playgrounds/csc.rsp.meta b/Explorer/Assets/DCL/PerformanceAndDiagnostics/Analytics/Playgrounds/csc.rsp.meta deleted file mode 100644 index 1ff3d6a498e..00000000000 --- a/Explorer/Assets/DCL/PerformanceAndDiagnostics/Analytics/Playgrounds/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 926cb87435bc493189943e10f892995e -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Platforms/csc.rsp b/Explorer/Assets/DCL/Platforms/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Platforms/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Platforms/csc.rsp.meta b/Explorer/Assets/DCL/Platforms/csc.rsp.meta deleted file mode 100644 index e9db6b83d7d..00000000000 --- a/Explorer/Assets/DCL/Platforms/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 4bccd9a3f617458db82dd315005a6be4 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/PluginSystem/Global/ChatPlugin.cs b/Explorer/Assets/DCL/PluginSystem/Global/ChatPlugin.cs index 233e31254cf..79e32fc3b89 100644 --- a/Explorer/Assets/DCL/PluginSystem/Global/ChatPlugin.cs +++ b/Explorer/Assets/DCL/PluginSystem/Global/ChatPlugin.cs @@ -3,6 +3,7 @@ using Arch.SystemGroups; using Cysharp.Threading.Tasks; using DCL.AssetsProvision; +using DCL.CharacterCamera; using DCL.Chat; using DCL.Chat.History; using DCL.Chat.MessageBus; @@ -101,6 +102,7 @@ public class ChatPlugin : IDCLGlobalPlugin private readonly DecentralandEnvironment decentralandEnvironment; private readonly IAnalyticsController analytics; private readonly StreamReactionsChatCommand streamReactionsChatCommand; + private readonly IExposedCameraData exposedCameraData; private readonly CurrentChannelService? externalCurrentChannelService; private readonly DCLInput dclInput; @@ -149,6 +151,7 @@ public ChatPlugin( DecentralandEnvironment decentralandEnvironment, IAnalyticsController analytics, StreamReactionsChatCommand streamReactionsChatCommand, + IExposedCameraData exposedCameraData, CurrentChannelService? externalCurrentChannelService = null) { this.mvcManager = mvcManager; @@ -188,6 +191,7 @@ public ChatPlugin( this.decentralandEnvironment = decentralandEnvironment; this.analytics = analytics; this.streamReactionsChatCommand = streamReactionsChatCommand; + this.exposedCameraData = exposedCameraData; this.externalCurrentChannelService = externalCurrentChannelService; this.dclInput = DCLInput.Instance; @@ -403,7 +407,8 @@ public async UniTask InitializeAsync(ChatPluginSettings settings, CancellationTo messageReactionService, web3IdentityCache, profileCache, - inputBlock + inputBlock, + exposedCameraData ); pluginScope.Add(chatPanelPresenter); diff --git a/Explorer/Assets/DCL/Prefs/DCLPlayerPrefs.cs b/Explorer/Assets/DCL/Prefs/DCLPlayerPrefs.cs index b2d488462c5..557dd2d2e89 100644 --- a/Explorer/Assets/DCL/Prefs/DCLPlayerPrefs.cs +++ b/Explorer/Assets/DCL/Prefs/DCLPlayerPrefs.cs @@ -1,3 +1,4 @@ +using DCL.Diagnostics; using System; using System.Collections.Generic; using System.IO; @@ -35,11 +36,8 @@ private sealed class DisposedDCLPlayerPrefs : IDCLPrefs public void SaveSync() => Warn(); [System.Diagnostics.Conditional("UNITY_EDITOR")] - private static void Warn([System.Runtime.CompilerServices.CallerMemberName] string caller = "") - { - // TODO: Use ReportHub when it properly lives in its own dependency. - Debug.LogWarning( $"[DCLPlayerPrefs] {caller} called after shutdown — ignored."); - } + private static void Warn([System.Runtime.CompilerServices.CallerMemberName] string caller = "") => + ReportHub.LogWarning(ReportCategory.UNSPECIFIED, $"[DCLPlayerPrefs] {caller} called after shutdown — ignored."); } private const string VECTOR2_KEY_FORMAT = "{0}_{1}"; @@ -164,7 +162,7 @@ private static void OnQuitting() (dclPrefs as IDisposable)?.Dispose(); // Avoid any shutdown exceptions, just throw warnings. dclPrefs = new DisposedDCLPlayerPrefs(); - Debug.Log($"[ExitUtils] [DCLPlayerPrefs] cleanup took {stopwatch.ElapsedMilliseconds}ms"); + ReportHub.Log(ReportCategory.UNSPECIFIED, $"[ExitUtils] [DCLPlayerPrefs] cleanup took {stopwatch.ElapsedMilliseconds}ms"); } #if UNITY_EDITOR @@ -185,7 +183,7 @@ private static bool ValidateClearDCLPlayerPrefs() => private static void ResetNearbyVoiceIntroTip() { DeleteKey(DCLPrefKeys.NEARBY_VOICE_TIP_DISMISSED, save: true); - Debug.Log("Nearby Voice Intro Tip has been reset."); + Debug.Log("Nearby Voice Intro Tip has been reset."); // editor MenuItem outside play mode - ReportHub is not initialized // lint-ignore: debug-log } #endif } diff --git a/Explorer/Assets/DCL/Quality/Container/csc.rsp.meta b/Explorer/Assets/DCL/Quality/Container/csc.rsp.meta deleted file mode 100644 index 87aa6c617f2..00000000000 --- a/Explorer/Assets/DCL/Quality/Container/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 49681f3ab5d449acadf1f0e7c9125572 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/Quality/RenderFeatures/csc.rsp b/Explorer/Assets/DCL/Quality/RenderFeatures/csc.rsp new file mode 100644 index 00000000000..2d66b0292b8 --- /dev/null +++ b/Explorer/Assets/DCL/Quality/RenderFeatures/csc.rsp @@ -0,0 +1 @@ +-nullable:enable diff --git a/Explorer/Assets/DCL/AvatarRendering/Emotes/Systems/csc.rsp.meta b/Explorer/Assets/DCL/Quality/RenderFeatures/csc.rsp.meta similarity index 74% rename from Explorer/Assets/DCL/AvatarRendering/Emotes/Systems/csc.rsp.meta rename to Explorer/Assets/DCL/Quality/RenderFeatures/csc.rsp.meta index bb4daf7059f..4af27a31075 100644 --- a/Explorer/Assets/DCL/AvatarRendering/Emotes/Systems/csc.rsp.meta +++ b/Explorer/Assets/DCL/Quality/RenderFeatures/csc.rsp.meta @@ -1,5 +1,5 @@ fileFormatVersion: 2 -guid: b8cf94e5d34844e78e86fe517a3bc201 +guid: 5266feec9f11a2de0cbe5047262ce057 DefaultImporter: externalObjects: {} userData: diff --git a/Explorer/Assets/DCL/Quality/csc.rsp b/Explorer/Assets/DCL/Quality/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/Quality/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/Quality/csc.rsp.meta b/Explorer/Assets/DCL/Quality/csc.rsp.meta deleted file mode 100644 index 03f3ea1714f..00000000000 --- a/Explorer/Assets/DCL/Quality/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 09cd7c46223a5a1479a5f585ab776990 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/SDKComponents/AudioAnalysis/csc.rsp b/Explorer/Assets/DCL/SDKComponents/AudioAnalysis/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/SDKComponents/AudioAnalysis/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/SDKComponents/AudioAnalysis/csc.rsp.meta b/Explorer/Assets/DCL/SDKComponents/AudioAnalysis/csc.rsp.meta deleted file mode 100644 index 3d9b6297f7d..00000000000 --- a/Explorer/Assets/DCL/SDKComponents/AudioAnalysis/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: e737d5e3dbc94470a916691504669df6 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/csc.rsp b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/csc.rsp.meta b/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/csc.rsp.meta deleted file mode 100644 index e4c7bbe8dc3..00000000000 --- a/Explorer/Assets/DCL/SDKComponents/AudioSources/Systems/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: 0659ce9cce654948a411e51940f0d969 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/SDKEntityTriggerArea/csc.rsp b/Explorer/Assets/DCL/SDKEntityTriggerArea/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/SDKEntityTriggerArea/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/SDKEntityTriggerArea/csc.rsp.meta b/Explorer/Assets/DCL/SDKEntityTriggerArea/csc.rsp.meta deleted file mode 100644 index e59f9570144..00000000000 --- a/Explorer/Assets/DCL/SDKEntityTriggerArea/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: ad2974da07f74284946e0c959d38b206 -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/DCL/VoiceChat/csc.rsp b/Explorer/Assets/DCL/VoiceChat/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/VoiceChat/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/VoiceChat/csc.rsp.meta b/Explorer/Assets/DCL/VoiceChat/csc.rsp.meta deleted file mode 100644 index 9124fec7b7c..00000000000 --- a/Explorer/Assets/DCL/VoiceChat/csc.rsp.meta +++ /dev/null @@ -1,3 +0,0 @@ -fileFormatVersion: 2 -guid: 5ce7fe32c8bc44cc9961ab74cb1b3b81 -timeCreated: 1753900529 \ No newline at end of file diff --git a/Explorer/Assets/DCL/WebRequests/csc.rsp b/Explorer/Assets/DCL/WebRequests/csc.rsp deleted file mode 100644 index dcc377f8979..00000000000 --- a/Explorer/Assets/DCL/WebRequests/csc.rsp +++ /dev/null @@ -1 +0,0 @@ --nullable:enable \ No newline at end of file diff --git a/Explorer/Assets/DCL/WebRequests/csc.rsp.meta b/Explorer/Assets/DCL/WebRequests/csc.rsp.meta deleted file mode 100644 index bf924640881..00000000000 --- a/Explorer/Assets/DCL/WebRequests/csc.rsp.meta +++ /dev/null @@ -1,7 +0,0 @@ -fileFormatVersion: 2 -guid: ac6077eea64c4315b83b616f48ec715e -DefaultImporter: - externalObjects: {} - userData: - assetBundleName: - assetBundleVariant: diff --git a/Explorer/Assets/Plugins/UUAV/Packages/UUAV/Runtime/NativeMethods.cs b/Explorer/Assets/Plugins/UUAV/Packages/UUAV/Runtime/NativeMethods.cs index 15c77ef2ad7..588a7fb1481 100644 --- a/Explorer/Assets/Plugins/UUAV/Packages/UUAV/Runtime/NativeMethods.cs +++ b/Explorer/Assets/Plugins/UUAV/Packages/UUAV/Runtime/NativeMethods.cs @@ -26,7 +26,9 @@ public enum UUAVLogLevel Trace = 56, } - public enum UUAVState + // Crosses the DllImport boundary as-is; the underlying type is the ABI contract + // with the native library, so it is pinned explicitly (DCLA005). + public enum UUAVState : int { Closed, Opening, diff --git a/docs/code-style-guidelines.md b/docs/code-style-guidelines.md index 8ccffed80d8..6fd58a1cb8b 100644 --- a/docs/code-style-guidelines.md +++ b/docs/code-style-guidelines.md @@ -250,7 +250,7 @@ List filteredWords = new FilterLogic(listWords). - Folders that are deep in the folders hierarchy should be without namespace. - Never suppress the folder-namespace inspection with `// ReSharper disable once CheckNamespace` (or the file-wide `// ReSharper disable CheckNamespace`). - Namespaces name domains and deliberately survive folder and assembly reshuffles (e.g. `DCL.Ipfs` lives in the `DCL.Network` assembly), so a folder-namespace mismatch is often intentional. - - The lint filter shared by CI and the local hook (`scripts/lint/filter-warnings.sh`) already excludes the `CheckNamespace` inspection, so the comment changes nothing in the warning count — it is pure noise, and it hides genuine cases where a type joined the wrong namespace. + - The lint filter shared by CI and the local hook (`scripts/lint/filter-warnings.sh`) already excludes the `CheckNamespace` inspection, so the comment changes nothing in the warning count — it is pure noise, and it hides genuine cases where a type joined the wrong namespace. Adding the comment is itself a lint violation: the `checknamespace-suppression` rule in `scripts/lint/custom-rules.sh` blocks it in both the CI `custom-lint` job and the local hook. - If the IDE flags a mismatch, either move the type into the domain namespace its closest collaborators live in, or leave the warning visible. ### Whitespaces diff --git a/scripts/build-analyzers.sh b/scripts/build-analyzers.sh new file mode 100755 index 00000000000..48bd1eb25ff --- /dev/null +++ b/scripts/build-analyzers.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Build the DCL.Analyzers Roslyn analyzer, run its tests, and sync the DLL into +# the Unity project (Explorer/Assets/DCL/DCL.Analyzers.dll - the placement makes +# Unity feed it to csc for every asmdef under Assets/DCL, and nothing vendored). +# Run after any change under Analyzers/. CI runs the tests on every such change; +# the DLL itself is synced manually via this script and committed (LFS). +set -euo pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "$ROOT" + +if ! command -v dotnet >/dev/null 2>&1; then + if command -v nix-shell >/dev/null 2>&1; then + exec nix-shell -p dotnet-sdk_10 --run "bash $0 $*" + fi + echo "build-analyzers: dotnet SDK not found (and no nix-shell to provide one)" >&2 + exit 1 +fi + +# Run from Analyzers/ so Analyzers/global.json pins the SDK: the CI drift check +# byte-compares a rebuild against the committed DLL, and byte-identical output +# requires the same Roslyn compiler on every machine. +cd Analyzers +# Always build clean: stale obj/ state can leak into the output and desync it +# from the clean rebuild the CI drift check performs (observed: incremental +# rebuild after an edit produced different bytes than the clean CI build). +rm -rf DCL.Analyzers/bin DCL.Analyzers/obj +dotnet test DCL.Analyzers.Tests -v q --nologo +# ContinuousIntegrationBuild normalizes embedded paths and DebugType=none drops +# the debug directory (whose source hashes differ between CRLF and LF checkouts), +# so this local build is byte-identical to the CI drift check's rebuild +# (workflow: "Fail on DLL drift"). +dotnet build DCL.Analyzers -c Release -v q --nologo \ + -p:ContinuousIntegrationBuild=true -p:DebugType=none +cd "$ROOT" + +src="Analyzers/DCL.Analyzers/bin/Release/netstandard2.0/DCL.Analyzers.dll" +dst="Explorer/Assets/DCL/DCL.Analyzers.dll" +cp "$src" "$dst" +echo "synced: $dst ($(sha256sum "$dst" | cut -c1-16)…, $(stat -c%s "$dst") bytes)" diff --git a/scripts/lint/custom-rules.sh b/scripts/lint/custom-rules.sh new file mode 100755 index 00000000000..5ca3ea7191e --- /dev/null +++ b/scripts/lint/custom-rules.sh @@ -0,0 +1,252 @@ +#!/usr/bin/env bash +# Deterministic project-rule linter: regex rules from CLAUDE.md / .claude/skills, +# enforced ONLY on lines added in the diff under inspection — pre-existing +# violations never block (same ratchet philosophy as the ReSharper warning count). +# +# Usage: +# custom-rules.sh --working-tree # added lines vs HEAD + untracked files (Stop hook) +# custom-rules.sh --diff # added lines in a commit range (CI) +# +# Output: one finding per line -> : +# Exit codes: 0 = clean (or only WARN findings); 2 = BLOCK findings present; +# 3 = a rule pattern is broken (never silently passes). +# +# Escape hatch: a finding is suppressed when its line carries a trailing +# // lint-ignore: [, ...] +# comment naming that rule. Use it for the rare sanctioned exception (e.g. an +# #if UNITY_EDITOR-guarded Debug.Log); the suppression stays visible in the +# diff for reviewers to challenge. +# +# Patterns are POSIX EREs evaluated by awk as dynamic regexes: no \b/\y word +# boundaries (write (^|[^[:alnum:]_.]) guards instead) so they behave the same +# under gawk, mawk, and BSD awk. +set -uo pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "$ROOT" + +mode="${1:?usage: custom-rules.sh --working-tree | --diff }" + +# --------------------------------------------------------------------------- +# Rules. rule [anti-ERE] +# severity BLOCK (exit 2) or WARN (printed, never blocks) +# include rule applies only to paths matching (empty = every .cs file) +# exclude rule never applies to paths matching (empty = no exclusions) +# anti optional: a line matching this is NOT a finding even when the +# pattern matches (carves a sanctioned idiom out of a broad pattern) +# Keep each message pointing at the rule's source doc so findings explain themselves. +# --------------------------------------------------------------------------- +declare -a R_SEV=() R_ID=() R_INC=() R_EXC=() R_PAT=() R_MSG=() R_ANT=() +rule() { R_SEV+=("$1"); R_ID+=("$2"); R_INC+=("$3"); R_EXC+=("$4"); R_PAT+=("$5"); R_MSG+=("$6"); R_ANT+=("${7:-}"); } + +EXCLUDE_NON_PROD='(^|/)([A-Za-z]*Tests?|Editor|Plugins|Demo)/|Editor\.cs$|Should\.cs$|Tests?\.cs$' + +# ReportsHandling is the ReportHub implementation itself - its own error/ANR-dump +# paths cannot log through it. +rule BLOCK debug-log '' "$EXCLUDE_NON_PROD|(^|/)ReportsHandling/" \ + '(^|[^[:alnum:]_])(UnityEngine\.)?Debug\.(Log|LogError|LogWarning|LogException|LogFormat|LogErrorFormat|LogWarningFormat|LogAssertion)\(' \ + 'Use ReportHub instead of Debug.Log (CLAUDE.md; diagnostics-and-logging skill)' + +# instance. Tests constructing the two sanctioned proxies are exempt. +rule BLOCK object-proxy '' "$EXCLUDE_NON_PROD" \ + '(^|[^[:alnum:]_])new +ObjectProxy<' \ + 'ObjectProxy is an anti-pattern - pick a recipe from docs/architecture-overview.md § Deferred dependencies (CLAUDE.md)' + +rule BLOCK checknamespace-suppression '' '' \ + 'ReSharper +disable( +once)? +CheckNamespace' \ + 'Never suppress CheckNamespace - fix the namespace instead (docs/code-style-guidelines.md § Namespaces)' + +rule BLOCK linq-in-system 'System\.cs$' "$EXCLUDE_NON_PROD" \ + 'using +System\.Linq' \ + 'No LINQ in ECS systems - Update() must be allocation-free (CLAUDE.md § Performance Constraints)' + +rule BLOCK camera-main '' "$EXCLUDE_NON_PROD" \ + '(^|[^[:alnum:]_])(UnityEngine\.)?Camera\.main($|[^[:alnum:]_])' \ + 'Use the ECS camera singleton (TryGet in a system), not Camera.main (CLAUDE.md § Anti-Patterns)' + +# NOTE: no null!/default! rule on purpose - `[field: SerializeField] ... = null!` +# is the sanctioned inspector-assigned idiom and the attribute sits on the previous +# line, invisible to a per-line check (310 hits over 200 commits, all legitimate). + +rule BLOCK nullable-disable '' '' \ + '^[ \t]*#nullable +disable' \ + 'Do not add #nullable disable - annotate properly instead (code-standards skill)' + +rule BLOCK interface-prefix '' '' \ + '(public|internal) +(partial +)?interface +([^I[:space:]]|I[a-z_])' \ + 'Interface names must start with I (docs/code-style-guidelines.md § Naming Conventions)' + +rule BLOCK foreign-test-framework '' '' \ + 'using +(Moq|Xunit|FluentAssertions|FakeItEasy)( *;|\.)' \ + 'Tests use NUnit + NSubstitute only (docs/standards.md § Tests)' + +rule BLOCK async-void-in-tests '(^|/)Tests?/|Should\.cs$|Tests\.cs$' '' \ + '(^|[^[:alnum:]_])async +void +' \ + 'Tests must use async Task, not async void (testing-infrastructure skill)' + +rule BLOCK explorer-flag-prefix '' '' \ + 'IsEnabled *\( *"explorer-' \ + 'Flag names drop the explorer- prefix in code (feature-flags-and-configuration skill)' + +rule BLOCK tryaddwidget-unguarded '' '' \ + 'TryAddWidget *\([^)]*\) *\.' \ + 'TryAddWidget returns null when debug is disabled - chain with ?. (debug-widget skill)' + +rule BLOCK thread-affinity-scene-runtime '(^|/)(SceneRunner|SceneRuntime|CrdtEcsBridge)/' "$EXCLUDE_NON_PROD" \ + '\[ThreadStatic\]|ThreadLocal<|Thread\.CurrentThread' \ + 'No thread affinity in scene-runtime code - it hops threads at every await (scene-runtime-and-crdt skill)' + +# WARN: some overloads legitimately take the message first. +rule WARN reporthub-string-category '' '' \ + 'ReportHub\.(Log|LogWarning|LogError|LogException) *\( *"' \ + 'Pass a ReportCategory constant to ReportHub, not a string literal (diagnostics-and-logging skill)' + +rule WARN world-query '' "$EXCLUDE_NON_PROD" \ + '(^|[^[:alnum:]_])[Ww]orld\.Query *\(' \ + 'World.Query is a last resort - prefer source-generated [Query] (CLAUDE.md § Querying)' + +rule WARN raw-http '' "(^|/)WebRequests/|$EXCLUDE_NON_PROD" \ + 'new +(HttpClient|WebClient) *\(|UnityWebRequest\.(Get|Post|Put|Delete|Head) *\(|new +UnityWebRequest *\(' \ + 'Route HTTP through IWebRequestController (web-requests skill)' + +rule WARN nameof-argument-exception '' '' \ + 'new +(ArgumentNullException|ArgumentOutOfRangeException) *\( *"' \ + 'Use nameof(...) for the parameter name (docs/code-style-guidelines.md)' + +rule BLOCK nullable-local '' "$EXCLUDE_NON_PROD" \ + '^[ \t]*[A-Za-z_][A-Za-z0-9_.]*(<[^;={}]*>)?\? +[a-z_][A-Za-z0-9_]* *(=[^;]*)?;' \ + 'Local variables must not be nullable - use pattern matching (is not { } x) to bind a non-nullable local (CLAUDE.md § Anti-Patterns)' + +# The null-forgiving operator lies to the compiler about nullability. The anti +# carves out (a) the sanctioned "= null!"/"= default!" initializer idiom +# (wire-format DTOs, [SerializeField]-assigned members, generic defaults) and +# (b) the three split-phase-initialization idioms the frameworks force - +# viewInstance! (MVC view created after the controller), Instance! (late-init +# singletons), World! (Arch ECS field assigned during system wiring) - where +# no NRT-clean rewrite exists. +rule BLOCK null-forgiving-suppression '' "$EXCLUDE_NON_PROD" \ + '! *[.;,)]' \ + 'Never use the null-forgiving ! to silence nullability - restructure so the value is provably non-null (CLAUDE.md § Anti-Patterns)' \ + '(null|default) *! *[.;,)]|(viewInstance|(^|[^[:alnum:]_])(Instance|World))! *[.;,)]' + +rule BLOCK world-get-copy '' "$EXCLUDE_NON_PROD" \ + '(^|[ \t(])var +[a-z_][A-Za-z0-9_]* *= *([A-Za-z_][A-Za-z0-9_.]*)?[Ww]orld\.Get<' \ + 'Use ref var x = ref World.Get() - a plain var copies the component and mutations are silently lost (CLAUDE.md § Safe Component Mutation)' \ + 'ref +(readonly +)?var' + +rule BLOCK stringbuilder-interpolation '' "$EXCLUDE_NON_PROD" \ + '\.Append(Line)? *\( *\$"' \ + 'Do not interpolate into StringBuilder - use the typed Append overloads (review rule, PR #9339; docs/standards.md § Memory)' + +rule WARN caller-narrating-comment '' "$EXCLUDE_NON_PROD" \ + '// .*so (that )?(the )?(caller|consumer|upper layer|client)s?[^[:alnum:]]' \ + 'Comments must not narrate caller/external behavior - state only what this code does (CLAUDE.md § Anti-Patterns)' + +rule WARN contextmenu '' "$EXCLUDE_NON_PROD" \ + '\[ContextMenu' \ + 'Prefer [Button] from EasyButtons over [ContextMenu] (docs/code-style-guidelines.md § Attribute Usages)' + +# Emit added lines as: \t\t +parse_diff() { + awk ' + /^\+\+\+ b\// { path = substr($0, 7); sub(/\t$/, "", path); next } + /^@@ / { split($0, a, "+"); split(a[2], b, /[ ,]/); line = b[1]; next } + /^\+/ { if (path != "") printf "%s\t%d\t%s\n", path, line, substr($0, 2); line++ } + ' +} + +# The -c/-- flags pin the exact diff format parse_diff expects, immune to user +# gitconfig (diff.noprefix, diff.mnemonicPrefix, diff.external, core.quotePath). +GIT_DIFF=(git -c core.quotePath=off -c diff.noprefix=false diff + --no-ext-diff --src-prefix=a/ --dst-prefix=b/ -U0 --no-color --diff-filter=ACMR) + +# The selftest fixture corpus is violations on purpose - never lint it. +# Assets/Plugins and Packages are vendored/plugin-layer code these project +# rules don't govern (mirrors filter-warnings.sh's ownership boundary). +# Analyzers/ is Roslyn-host code with its own idioms (nullable locals are +# idiomatic there) - governed by its own test suite, not these Unity rules. +PATHSPEC=('*.cs' + ':(exclude)scripts/lint/tests/**' + ':(exclude)Analyzers/**' + ':(exclude)Explorer/Assets/Plugins/**' + ':(exclude)Explorer/Packages/**') + +added_lines() { + case "$mode" in + --working-tree) + "${GIT_DIFF[@]}" HEAD -- "${PATHSPEC[@]}" 2>/dev/null | parse_diff + # untracked .cs files: every line counts as added + git -c core.quotePath=off ls-files --others --exclude-standard -- "${PATHSPEC[@]}" 2>/dev/null | + while IFS= read -r f; do + [ -f "$f" ] && F="$f" awk '{ printf "%s\t%d\t%s\n", ENVIRON["F"], NR, $0 }' < "$f" + done + ;; + --diff) + local base="${2:?--diff needs }" head="${3:?--diff needs }" + "${GIT_DIFF[@]}" "$base" "$head" -- "${PATHSPEC[@]}" 2>/dev/null | parse_diff + ;; + *) + echo "custom-rules: unknown mode '$mode'" >&2 + exit 1 + ;; + esac +} + +# Apply every rule to every added line. A cheap grep -E pass narrows each rule +# to candidate lines (the C regex engine is ~200x faster than awk dynamic +# regexes on big diffs); awk then re-verifies the exact pattern against the +# text column alone and applies the path include/exclude. Patterns reach awk +# through the environment (never -v, which escape-processes backslashes). +# A rule whose pattern fails to compile exits 3 - never a silent pass. +main() { + local tmp found i cpat + tmp="$(mktemp)"; found="$(mktemp)" + trap 'rm -f "$tmp" "$found"' EXIT + + added_lines "$@" > "$tmp" + [ -s "$tmp" ] || exit 0 + + for i in "${!R_ID[@]}"; do + # Candidate pre-filter over the whole TSV line. A leading ^ can never + # match after the path\tline\t prefix, so strip it for the candidate + # pass - the exact pattern is still enforced by awk below. (^|...) + # groups degrade gracefully: the other branch matches the tab. + cpat="${R_PAT[$i]#^}" + printf 'probe\n' | grep -E -- "$cpat" >/dev/null 2>&1 + if [ $? -gt 1 ]; then + echo "custom-rules: rule '${R_ID[$i]}' has an invalid pattern - refusing to pass" >&2 + exit 3 + fi + { grep -E -- "$cpat" "$tmp" || true; } | + INC="${R_INC[$i]}" EXC="${R_EXC[$i]}" PAT="${R_PAT[$i]}" ANT="${R_ANT[$i]}" \ + SEV="${R_SEV[$i]}" ID="${R_ID[$i]}" MSG="${R_MSG[$i]}" \ + awk -F'\t' ' + BEGIN { + inc = ENVIRON["INC"]; exc = ENVIRON["EXC"]; pat = ENVIRON["PAT"]; ant = ENVIRON["ANT"] + } + { + path = $1; ln = $2 + text = $0; sub(/^[^\t]*\t[^\t]*\t/, "", text) + if (inc != "" && path !~ inc) next + if (exc != "" && path ~ exc) next + if (text !~ pat) next + if (ant != "" && text ~ ant) next + # same-line escape hatch: // lint-ignore: rule-a, rule-b + if (text ~ ("lint-ignore:[ a-z0-9,-]*" ENVIRON["ID"] "([^a-z0-9-]|$)")) next + printf "%s:%s %s %s %s\n", path, ln, ENVIRON["SEV"], ENVIRON["ID"], ENVIRON["MSG"] + } + ' >> "$found" || { + echo "custom-rules: rule '${R_ID[$i]}' failed to evaluate - refusing to pass" >&2 + exit 3 + } + done + + [ -s "$found" ] || exit 0 + sort -t: -k1,1 -k2,2n "$found" + # grep -q on a file, not a pipe: a SIGPIPE'd writer under pipefail once + # turned BLOCK findings into exit 0 here. + grep -q ' BLOCK ' "$found" && exit 2 + exit 0 +} + +main "$@" diff --git a/scripts/lint/filter-warnings.sh b/scripts/lint/filter-warnings.sh index 564414fa6ee..e585b5e123a 100755 --- a/scripts/lint/filter-warnings.sh +++ b/scripts/lint/filter-warnings.sh @@ -30,6 +30,10 @@ jq --argjson excludedRules "$(printf '%s\n' "${excluded_rules[@]}" | jq -R . | j | map(select( ((.level // "warning") | IN("warning", "error")) and ((.ruleId // "") | IN($excludedRules[]) | not) + # DCL.Analyzers diagnostics have their own enforcement channel (Unity csc + + # IDE, severities pinned in Explorer/.editorconfig); counting them here would + # double-enforce advisory rules through the unrelated ReSharper ratchet. + and ((.ruleId // "") | startswith("DCLA") | not) and ((.locations[0].physicalLocation.artifactLocation.uri // "") | test($excludedPaths; "i") | not) )) diff --git a/scripts/lint/lint-changed.sh b/scripts/lint/lint-changed.sh index 5dfb2b94ed2..14eaca78ee6 100755 --- a/scripts/lint/lint-changed.sh +++ b/scripts/lint/lint-changed.sh @@ -19,6 +19,21 @@ if [ -n "$input" ] && command -v jq >/dev/null 2>&1; then [ "$(printf '%s' "$input" | jq -r '.stop_hook_active // false' 2>/dev/null)" = "true" ] && exit 0 fi +# Fast deterministic project rules first (CLAUDE.md / skills, added lines only): +# instant feedback, and no point paying the multi-minute ReSharper load while +# these are unresolved. WARN findings print but never block; BLOCK exits 2. +custom_out="$(bash "$LINT_DIR/custom-rules.sh" --working-tree)" +custom_rc=$? +if [ -n "$custom_out" ]; then + { + echo "Project-rule findings on lines added this session (scripts/lint/custom-rules.sh):" + printf '%s\n' "$custom_out" | sed 's/^/ /' + } >&2 +fi +# 2 = BLOCK findings; 3 = a rule pattern is broken - both must stop the flow +# (3 is the "never silently passes" contract, and the hook is where rules get edited). +{ [ "$custom_rc" -eq 2 ] || [ "$custom_rc" -eq 3 ]; } && exit 2 + # Changed C# under Explorer/ (working tree + staged + new untracked). changed="$( { diff --git a/scripts/lint/tests/expected-diff.txt b/scripts/lint/tests/expected-diff.txt new file mode 100644 index 00000000000..05755ce84ba --- /dev/null +++ b/scripts/lint/tests/expected-diff.txt @@ -0,0 +1,2 @@ +Explorer/Assets/DCL/Feature/FooSystem.cs:6 BLOCK debug-log +Explorer/Assets/DCL/Feature/Late.cs:2 BLOCK debug-log diff --git a/scripts/lint/tests/expected-working-tree.txt b/scripts/lint/tests/expected-working-tree.txt new file mode 100644 index 00000000000..940693f075f --- /dev/null +++ b/scripts/lint/tests/expected-working-tree.txt @@ -0,0 +1,29 @@ +Explorer/Assets/DCL/Feature/FooSystem.cs:1 BLOCK linq-in-system +Explorer/Assets/DCL/Feature/Tests/FeatureShould.cs:5 BLOCK async-void-in-tests +Explorer/Assets/DCL/Feature/Violations.cs:1 BLOCK nullable-disable +Explorer/Assets/DCL/Feature/Violations.cs:4 BLOCK foreign-test-framework +Explorer/Assets/DCL/Feature/Violations.cs:6 BLOCK interface-prefix +Explorer/Assets/DCL/Feature/Violations.cs:12 BLOCK object-proxy +Explorer/Assets/DCL/Feature/Violations.cs:14 WARN contextmenu +Explorer/Assets/DCL/Feature/Violations.cs:17 BLOCK debug-log +Explorer/Assets/DCL/Feature/Violations.cs:18 BLOCK debug-log +Explorer/Assets/DCL/Feature/Violations.cs:21 BLOCK camera-main +Explorer/Assets/DCL/Feature/Violations.cs:22 BLOCK explorer-flag-prefix +Explorer/Assets/DCL/Feature/Violations.cs:24 BLOCK tryaddwidget-unguarded +Explorer/Assets/DCL/Feature/Violations.cs:26 WARN reporthub-string-category +Explorer/Assets/DCL/Feature/Violations.cs:28 WARN world-query +Explorer/Assets/DCL/Feature/Violations.cs:29 WARN raw-http +Explorer/Assets/DCL/Feature/Violations.cs:30 WARN nameof-argument-exception +Explorer/Assets/DCL/Feature/Violations.cs:33 BLOCK checknamespace-suppression +Explorer/Assets/DCL/Feature/Violations.cs:35 BLOCK camera-main +Explorer/Assets/DCL/Feature/Violations.cs:41 BLOCK stringbuilder-interpolation +Explorer/Assets/DCL/Feature/Violations.cs:42 BLOCK stringbuilder-interpolation +Explorer/Assets/DCL/Feature/Violations.cs:44 WARN caller-narrating-comment +Explorer/Assets/DCL/Feature/Violations.cs:53 BLOCK nullable-local +Explorer/Assets/DCL/Feature/Violations.cs:54 BLOCK nullable-local +Explorer/Assets/DCL/Feature/Violations.cs:55 BLOCK null-forgiving-suppression +Explorer/Assets/DCL/Feature/Violations.cs:56 BLOCK null-forgiving-suppression +Explorer/Assets/DCL/Feature/Violations.cs:57 BLOCK null-forgiving-suppression +Explorer/Assets/DCL/Feature/Violations.cs:61 BLOCK world-get-copy +Explorer/Assets/DCL/Infrastructure/SceneRunner/Impl.cs:5 BLOCK thread-affinity-scene-runtime +Explorer/Assets/DCL/Infrastructure/SceneRunner/Impl.cs:6 BLOCK thread-affinity-scene-runtime diff --git a/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/FooSystem.cs b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/FooSystem.cs new file mode 100644 index 00000000000..47f354a372f --- /dev/null +++ b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/FooSystem.cs @@ -0,0 +1,5 @@ +using System.Linq; + +public partial class FooSystem +{ +} diff --git a/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/Tests/FeatureShould.cs b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/Tests/FeatureShould.cs new file mode 100644 index 00000000000..9c8c1c3001b --- /dev/null +++ b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/Tests/FeatureShould.cs @@ -0,0 +1,15 @@ +using UnityEngine; + +public class FeatureShould +{ + public async void TestBad() { } + + public async Task TestGood() { } + + public void NonProdExclusionsHoldHere() + { + Debug.Log("allowed in tests"); + var proxy = new ObjectProxy(); + var cam = Camera.main; + } +} diff --git a/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/Violations.cs b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/Violations.cs new file mode 100644 index 00000000000..aeb95d77683 --- /dev/null +++ b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Feature/Violations.cs @@ -0,0 +1,73 @@ +#nullable disable +using System; +using UnityEngine; +using Moq; + +public interface Widget { } + +public interface IWidget { } + +public class Violations +{ + private readonly ObjectProxy proxy = new ObjectProxy(); + + [ContextMenu("run")] + public void Run() + { + Debug.Log("plain"); + UnityEngine.Debug.LogWarning("qualified"); + Debug.Assert(true); + MyDebug.Log("negative"); + var cam = Camera.main; + flags.IsEnabled("explorer-flag"); + flags.IsEnabled("flag"); + builder.TryAddWidget("w").Add(); + builder.TryAddWidget("w")?.Add(); + ReportHub.LogError("Literal", "msg"); + ReportHub.LogError(ReportCategory.ENGINE, "msg"); + world.Query(in desc, Fn); + var http = new HttpClient(); + throw new ArgumentNullException("param"); + } +} +// ReSharper disable once CheckNamespace +class Suppressed { void F() { Debug.Log("s"); } } // lint-ignore: debug-log +class WrongId { void F() { var c = Camera.main; } } // lint-ignore: debug-log + +class Builders +{ + void F(System.Text.StringBuilder b, int n) + { + b.Append($"count={n}"); + b.AppendLine($"row {n}"); + b.Append("literal is fine").Append(n); + int cached = n; // so the caller can retry with the same value + int local = n; // remove the corrupt file so the next read doesn't hit it + } +} + +class NullableStates +{ + void F() + { + EventId? maybeEvent = events[i]; + Dictionary? lookup = TryGetLookup(); + EventId forced = events[i]!; + Process(target!); + Target found = registry.Find(id)!.Target; + viewInstance!.Show(); + DCLInput.Instance!.Shortcuts.Register(); + MoveQuery(World!); + var copied = world.Get(entity); + ref var safe = ref world.Get(entity); + var unrelated = worldService.GetAll(); + EventId fine = events[i]; + string dtoInit = null!; + int score = has ? a : b; + } + + void G( + DebugController? debugController = null, + Button? debugButton = null) + { } +} diff --git a/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Infrastructure/SceneRunner/Impl.cs b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Infrastructure/SceneRunner/Impl.cs new file mode 100644 index 00000000000..aa435aa4f24 --- /dev/null +++ b/scripts/lint/tests/fixtures/Explorer/Assets/DCL/Infrastructure/SceneRunner/Impl.cs @@ -0,0 +1,7 @@ +using System.Threading; + +public class Impl +{ + [ThreadStatic] private static int ts; + private ThreadLocal tl; +} diff --git a/scripts/lint/tests/selftest.sh b/scripts/lint/tests/selftest.sh new file mode 100755 index 00000000000..25d5d3f8083 --- /dev/null +++ b/scripts/lint/tests/selftest.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Selftest for custom-rules.sh: builds a throwaway git repo from fixtures/ and +# asserts both modes against golden findings. Catches dead rules (pattern edits +# that stop matching), engine regressions (parse_diff, pattern transport), and +# whole-script breakage (awk fatal, bash error) - all of which otherwise exit 0. +# +# Goldens pin `path:line SEVERITY rule-id` (not the message text, so message +# rewording doesn't fail the build). Regenerate after an intentional rule +# change with: selftest.sh --regen +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +LINTER="$HERE/../custom-rules.sh" + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +fail() { echo "selftest FAIL: $1" >&2; exit 1; } +# tr strips CR so goldens compare equal when git checked them out with CRLF +# (Windows working copies); the linter itself always emits LF. +norm() { tr -d '\r' | awk -F' ' '{ print $1 " " $2 " " $3 }'; } +golden() { tr -d '\r' < "$1"; } +commit() { git -C "$work" -c user.email=selftest@local -c user.name=selftest -c commit.gpgsign=false commit -q "$@"; } +run_linter() { (cd "$work" && bash "$LINTER" "$@"); } + +cp -R "$HERE/fixtures/." "$work/" +git -C "$work" init -q -b selftest +commit --allow-empty -m base + +out="$(run_linter --working-tree)"; rc=$? +if [ "${1:-}" = "--regen" ]; then + printf '%s\n' "$out" | norm > "$HERE/expected-working-tree.txt" +else + [ "$rc" -eq 2 ] || fail "working-tree rc=$rc, want 2 (BLOCK findings present)" + diff <(printf '%s\n' "$out" | norm) <(golden "$HERE/expected-working-tree.txt") >&2 \ + || fail "working-tree findings diverge from golden (intentional rule change? rerun with --regen)" +fi + +git -C "$work" add -A +commit -m fixtures +out="$(run_linter --working-tree)"; rc=$? +[ "$rc" -eq 0 ] && [ -z "$out" ] || fail "clean tree rc=$rc out='$out', want silent 0" + +printf 'using UnityEngine;\nclass Late { void F() { Debug.Log("l"); } }\n' \ + > "$work/Explorer/Assets/DCL/Feature/Late.cs" # new file (A) +printf ' Debug.LogError("appended");\n' \ + >> "$work/Explorer/Assets/DCL/Feature/FooSystem.cs" # modified file (M) +git -C "$work" add -A +commit -m late +out="$(run_linter --diff HEAD~1 HEAD)"; rc=$? +if [ "${1:-}" = "--regen" ]; then + printf '%s\n' "$out" | norm > "$HERE/expected-diff.txt" + echo "selftest: goldens regenerated - review the diff before committing" + exit 0 +fi +[ "$rc" -eq 2 ] || fail "diff-mode rc=$rc, want 2" +diff <(printf '%s\n' "$out" | norm) <(golden "$HERE/expected-diff.txt") >&2 \ + || fail "diff-mode findings diverge from golden (line numbers test the hunk parser)" + +echo "selftest OK"