Skip to content

Gate dead code in CI with a deliberate-keep attribute, and clear the current findings #1987

Description

@hatayama

Summary

The dead-code scanner already finds unreferenced C# symbols, but nothing runs it and nothing
distinguishes "unreferenced by accident" from "unreferenced on purpose". Three pieces are needed:
a general keep-attribute, a CI job, and the cleanup of what is currently flagged.

Found while auditing docs/ (#TBD): two symbols with zero references in the repository had been
sitting undetected because the scanner is only ever run by hand, at the moment someone is already
about to delete something.

Current state

  • tools/UnityCliLoop.DeadCodeScanner works and scripts/check-dead-code.sh wraps it.
  • Nothing in .github/workflows/ references either. grep -rn "check-dead-code" .github/ is empty.
  • A keep mechanism exists but is framework-specific: UnityKeeperClassifier.KeptAttributeNames is a
    hardcoded set of UnityCliLoopTool, InitializeOnLoad, MenuItem, SerializeField,
    JsonProperty, Serializable and friends, plus KeptBaseTypeNames for MonoBehaviour,
    ScriptableObject, EditorWindow, Editor. Symbols matching those are reported as
    KeptByUnityOrReflection and hidden by --include-kept false.
  • There is no way to say "I am keeping this deliberately, for a reason the classifier cannot infer".
    That is the gap: a public extension API or a facade method kept for an onion boundary has no
    attribute to carry, so it stays in the report forever.
  • --fail-on high-confidence already exists and gates on categories Unused,
    UnusedPrivateMember, UnusedLocal.

Current scan output (--scope public --include-types true --include-members true --include-locals true --include-test-only true --include-kept false):

Category Count
PublicCandidate 187
TestOnly 57
Total 244

Nothing lands in the three high-confidence categories, so --fail-on high-confidence exits 0
today
— a gating CI job can be added without cleaning anything up first.

Tasks

1. Add a deliberate-keep attribute

Introduce an attribute (working name [UnityCliLoopKeep(string reason)]) that the scanner treats as
a keep signal, and require a reason string so the justification lives at the symbol instead of in a
reviewer's memory. Wire it into UnityKeeperClassifier alongside the existing sets, and surface the
reason in the report row so --include-kept true explains itself.

A reason string is what makes this better than the status quo. docs/dead-code-scanner.md currently
tells the reader not to add explanatory comments where the attribute or base type already makes the
reason obvious; this attribute is for the cases where it is not obvious.

Decide whether it belongs in ToolContracts (visible to third-party extension authors, since their
tools face the same problem) or stays internal to the package. Note the repository rule that public
package/assembly/extension API identifiers are not renamed later, so the placement should be settled
before it ships.

2. Run the scanner in CI

Add a workflow calling scripts/check-dead-code.sh with --fail-on high-confidence on pull requests
touching Packages/src/**/*.cs. Model it on .github/workflows/code-complexity.yml, which already
solves the same shape: path-filtered PR trigger, .NET setup, report, upload artifact.

Open question worth deciding explicitly: whether the job also reports PublicCandidate as a
non-blocking artifact. Gating on it is not viable — most of the 187 are genuine extension API such as
UnityCliLoopToolRegistrar.RegisterCustomTool — but a diffable artifact would let a reviewer notice
when a PR adds a new one. Consider whether an advisory-only report actually gets read, since the
complexity check has the same advisory shape and its report is not part of anyone's routine.

3. Clean up what is already flagged

Two confirmed cases from the audit, deliberately left untouched there because that branch was
docs-only:

  • DynamicCodeForegroundWarmupRunner.TryRunBackgroundSequenceAsync
    (Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs:35)
    internal static, zero references, no non-C# references either. Straightforward deletion.

  • FirstPartyToolsEditorStartup.CreateExecuteDynamicCodeReadinessProbeCode
    (Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs:40) — do not delete
    without deciding what it was for.
    It is public static and carries its own rationale: "composition
    root can only depend on the bundled-tool facade assembly, so the dynamic-code assembly keeps
    ownership of the actual probe source shape." Its two neighbours on the same facade
    (ResetServerScopedServices, ResetServerScopedServicesBeforeDomainReload) are both called from
    UnityCliLoopFirstPartyServerLifecycleBinding; only this one is not. Server readiness currently
    warms the transport with get-version alone and never compiles the dynamic-code probe shape, so
    this may be an unwired path rather than a leftover — in which case the fix is to connect it, not
    remove it. Resolve that first; if it is intentionally kept, it becomes the first user of the
    attribute from task 1.

The remaining 185 PublicCandidate entries are out of scope here. Per docs/dead-code-scanner.md
they need case-by-case review against non-C# references before anyone touches them.

Done when

  • A keep-attribute with a required reason exists, is honoured by the scanner, and its reason appears
    in the report.
  • A CI job runs the scanner on pull requests touching package C# and fails on high-confidence findings.
  • TryRunBackgroundSequenceAsync is gone; CreateExecuteDynamicCodeReadinessProbeCode is either
    wired up or annotated with the keep-attribute.
  • docs/dead-code-scanner.md documents the attribute and the CI gate.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions