chore(analyzers): fold the analyzer submodule into the main repo - #152
Merged
Conversation
- Rename project, namespace and assembly UnityFastToolsAnalyzers -> Aspid.FastTools.Analyzers. - Replace the dormant GetComponent/UnityHandler rules (UFT0001-UFT0005) with [TypeSelector] usage rules AFT0001-AFT0003 targeting Aspid.FastTools.Types. - AFT0001 flags [TypeSelector] on a field that is neither a string nor a [SerializeReference] managed reference; AFT0002 flags Allow on a managed reference; AFT0003 flags a base type that shares no concrete type with the field. - Add analyzer release tracking, xUnit tests, and a sample. BREAKING CHANGE: the assembly and root namespace are renamed to Aspid.FastTools.Analyzers and every diagnostic ID changed from UFT* to AFT*.
A sealed class admits no further subtypes, so pairing it with an interface it does not implement provably yields an empty candidate set.
#1) Unity declares the class as SerializeReference : Attribute (no suffix), so AFT0001 fired on every [SerializeReference] [TypeSelector] field. The test stub carried the suffixed name and masked the mismatch; it now mirrors Unity's real declaration, turning the existing managed-reference tests into regression coverage.
* feat(analyzers): add AFT0004 and AFT0005 diagnostics for [TypeSelector] AFT0004 (Error): reports [TypeSelector]+[SerializeReference] on a field whose element type derives from UnityEngine.Object — Unity silently skips serialization of managed references to Object-derived types. AFT0005 (Warning): reports when no concrete, non-UnityEngine.Object class visible in the compilation implements the effective base type (typeof(...) args or the field's element type). Severity is Warning because implementations may live in downstream assemblies. Uses a RegisterCompilationStartAction + per-compilation Lazy<> candidate list for performance. AFT0005 is suppressed for bases already covered by AFT0003. Also fixes a pre-existing bug in Sample.cs where SerializeReferenceAttribute (conventional suffix) was used instead of SerializeReference (Unity's actual no-suffix name), causing AFT0001 to fire on valid sample code. * fix(analyzers): AFT0005 checks intersection of typeof base and field element type HasVisibleCandidate now requires a candidate to be assignable to both the typeof(...) base type AND the field's declared element type, matching the actual picker behaviour which intersects the two constraints. When fieldElementType is System.Object the second condition is skipped. Updated InterfaceBaseType_NoDiagnostic: impl must now derive from Base too (MarkerImpl : Base, IMarker). Added positive test confirming AFT0005 fires when the only IMarker impl does not derive from Base. * perf(analyzers): scan only assemblies that can contain an AFT0005 candidate The AFT0005 check walked Compilation.GlobalNamespace, materialising every symbol of every referenced assembly (all of the BCL and UnityEngine) — up to minutes of csc time per Unity assembly using [TypeSelector] with [SerializeReference]. The candidate search now walks only assemblies that declare or reference both constraint types' assemblies, stops at the first match, and memoises results per (base, field element) pair. In the package sample assembly this cuts csc from ~187s to ~0.3s. * feat: accept SerializableType as a valid [TypeSelector] field shape - AFT0001 no longer fires on a SerializableType / SerializableType<T> field: it is a type-name shape like string, so Allow and base types are meaningful. - Add AspidFastTools.ClassesDescription with the SerializableType full names; matched by the field's original definition (non-generic and open-generic forms). - Cover the new shape with 4 tests (string/generic/list + explicit Allow), 28/28 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R78LEJNg3b9sgDUZhYPdHz --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
InterfaceWithOneConcreteImpl_NoAFT0005 was byte-identical to ManagedReferenceField_NoDiagnostic — same source snippet, same no-diagnostics assertion. Co-authored-by: Claude <noreply@anthropic.com>
- AFT0006 (error): an identifier string that matches no member of the declaring type — the drawer resolves string arguments member-first, so a typo silently drops the constraint - AFT0007 (error): the referenced member is not an instance field/property of type Type, Type[], string, or string[] - AFT0008 (warning): a non-identifier string that is not a plausible assembly-qualified type name (empty comma part, invalid identifier segments) - 17 new analyzer tests; stub gains the string constructors Co-Authored-By: Claude <noreply@anthropic.com>
- AFT0006 (error): an identifier string that matches no member of the declaring type — the drawer resolves string arguments member-first, so a typo silently drops the constraint - AFT0007 (error): the referenced member is not an instance field/property of type Type, Type[], string, or string[] - AFT0008 (warning): a non-identifier string that is not a plausible assembly-qualified type name (empty comma part, invalid identifier segments) - 17 new analyzer tests; stub gains the string constructors Co-authored-by: Claude <noreply@anthropic.com>
- the TypeSelector drawer now reads SerializableType / SerializableType<T> members as base-type sources, so IsSuitableConstraintSource must accept them too — otherwise a valid [TypeSelector(nameof(_serializableTypeMember))] wrongly reported AFT0007 - widen the AFT0007 message accordingly; add three analyzer tests (SerializableType field / generic / array members) Co-Authored-By: Claude <noreply@anthropic.com>
…zabletype-members Accept SerializableType members in AFT0007
Removes the gitlink and the now-empty .gitmodules so the analyzer sources can be grafted in via git subtree in the following commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gqDPXf96xiQ8ehPf3foan
The analyzer was a submodule but had no independent life: IsPackable=false, no own CI, no external consumers, and every rule it ships validates this package's own attributes. Each feature therefore cost two PRs plus a gitlink bump — five of the thirteen superproject commits touching it were pure pointer bookkeeping. Its history is grafted in via git subtree (previous commit), and the analyzer now matches how the generator is already handled: - Directory.Build.targets deploys the DLL into the Unity package on build, so the hook and the build-analyzer skill no longer copy it by hand. The copy is Release-only because the Tests and Sample projects reference the analyzer — CI runs `dotnet test` in Debug and would otherwise overwrite the shipped Release DLL. - tests.yml no longer checks out submodules; the matrix path is unchanged. - Drops the analyzer's LICENSE, a byte-identical copy of the root one. Verified: `dotnet build -c Release` deploys the DLL, `dotnet test` (Debug) leaves it untouched, and all 47 analyzer tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gqDPXf96xiQ8ehPf3foan
VPDPersonal
added
type: chore
VPDPersonal
marked this pull request as ready for review
August 3, 2026 18:23
VPDPersonal
added
status: needs-review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aspid.FastTools.Analyzersfrom a git submodule into this repository viagit subtree, keeping all 13 analyzer commits in the history. The submodule had no independent life —IsPackable=false, no own CI, no external consumers, and every rule it ships validates this package's own attributes — so each feature cost two PRs plus a gitlink bump.Aspid.FastTools.Analyzers/Directory.Build.targets, mirroring the generator: the build now deploys the DLL into the Unity package, so therebuild-analyzers-on-change.shhook and thebuild-analyzerskill no longer copy it by hand.submodules: truefromtests.yml(the matrix path is unchanged) and removes the analyzer'sLICENSE, a byte-identical copy of the root one.CLAUDE.mdand both QA checklists to match.Notes for review
TestsandSampleprojects reference the analyzer project, and CI runsdotnet testwithout-c— i.e. in Debug. Without the guard, every test run would overwrite the shipped Release DLL with a Debug build.dotnet build -c Releasedeploys the DLL, a Debug build leaves it untouched, anddotnet test "Aspid.FastTools.Analyzers"(the exact CI command) passes 47/47.git config --remove-section submodule.Aspid.FastTools.Analyzersandrm -rf .git/modules/Aspid.FastTools.Analyzers), and archiveVPDPersonal/Aspid.FastTools.Analyzerson GitHub.Aspid.FastTools.Analyzers/Aspid.FastTools.Analyzers/Aspid.FastTools.Analyzers/layout inherited from the standalone repo, where the generator has one level less. Flattening it would touch the.sln, the hook, the skill and CI — better as its own change.🇷🇺 Описание на русском
Итог
Aspid.FastTools.Analyzersпереезжает из git-сабмодуля в этот репозиторий черезgit subtree— все 13 его коммитов сохранены в истории. Самостоятельной жизни у сабмодуля не было:IsPackable=false, своего CI нет, внешних потребителей нет, а все правила валидируют атрибуты этого же пакета — поэтому каждая фича стоила двух PR плюс бампа гитлинка.Aspid.FastTools.Analyzers/Directory.Build.targetsпо образцу генератора: сборка сама кладёт DLL в Unity-пакет, так что хукrebuild-analyzers-on-change.shи скиллbuild-analyzerбольше не копируют её руками.tests.ymlубранsubmodules: true(путь матрицы не менялся), удалёнLICENSEаналайзера — байт-в-байт копия корневого.CLAUDE.mdи оба QA-чеклиста приведены в соответствие.На что посмотреть
TestsиSampleссылаются на проект аналайзера, а CI гоняетdotnet testбез-c, то есть в Debug. Без условия каждый прогон тестов затирал бы отгружаемую Release-DLL Debug-сборкой.dotnet build -c Releaseдоставляет DLL, Debug-сборка её не трогает,dotnet test "Aspid.FastTools.Analyzers"(точная команда CI) — 47/47.git config --remove-section submodule.Aspid.FastTools.Analyzersиrm -rf .git/modules/Aspid.FastTools.Analyzers) и заархивироватьVPDPersonal/Aspid.FastTools.Analyzersна GitHub.Aspid.FastTools.Analyzers/Aspid.FastTools.Analyzers/Aspid.FastTools.Analyzers/, доставшаяся от отдельного репо, тогда как у генератора уровнем меньше. Выпрямление затронет.sln, хук, скилл и CI — лучше отдельным изменением.