diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index fb05763..42b8dea 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -15,6 +15,13 @@ "dotnet-inspect" ], "rollForward": false + }, + "purview.build": { + "version": "0.5.3", + "commands": [ + "purview-build" + ], + "rollForward": false } } } \ No newline at end of file diff --git a/.config/purview-build.json b/.config/purview-build.json index 4d12dca..a74f0ac 100644 --- a/.config/purview-build.json +++ b/.config/purview-build.json @@ -6,6 +6,10 @@ "TestFilter": "/*/*/*/*/" }, "PackValidation": { + // Explicit because Purview.Build 0.5.3 defaults RequiredCompilerFlags to empty. Earlier + // versions forced this check on and could not be configured off, so these packages have + // always been validated against it; declaring it keeps that behaviour. + "RequiredCompilerFlags": ["optimization=release"], "RequireSymbolPackage": true, "RequireSymbolFiles": true, "RequiredContent": { @@ -59,4 +63,4 @@ "Release": { "Mode": "None" } -} \ No newline at end of file +} diff --git a/.editorconfig b/.editorconfig index 959b462..3476e6e 100644 --- a/.editorconfig +++ b/.editorconfig @@ -27,8 +27,10 @@ dotnet_search_reference_assemblies = true # Nullability settings dotnet_build_property.Nullable = enable -# Enable or disable the analyzers -dotnet_analyzer_diagnostic.severity = warning +# Per-rule severity is authoritative in this file. The SDK sets AnalysisMode/AnalysisLevel as +# MSBuild properties, and the .NET SDK ignores bulk 'dotnet_analyzer_diagnostic.*' severity +# configuration whenever those properties are present. Do not re-add a bulk entry here: it +# silently does nothing and hides which severities are actually enforced. # Visual Studio XML Project Files [*.{csproj,vbproj,vcxproj,vcxproj.filters,proj,projitems,shproj}] @@ -76,23 +78,20 @@ indent_style = tab dotnet_naming_rule.non_private_static_fields_should_be_pascal_case.severity = warning dotnet_naming_rule.non_private_static_fields_should_be_pascal_case.style = non_private_static_field_style dotnet_naming_rule.non_private_static_fields_should_be_pascal_case.symbols = non_private_static_fields -dotnet_naming_rule.private_fields.severity = warning -dotnet_naming_rule.private_fields.style = camel_case_underscore -dotnet_naming_rule.private_fields.symbols = private_fields -dotnet_naming_rule.private_fields_style.severity = warning -dotnet_naming_rule.private_fields_style.style = camel_case -dotnet_naming_rule.private_fields_style.symbols = private_fields dotnet_naming_style.non_private_static_field_style.capitalization = pascal_case +# NOTE: the private-field rules that used to sit here referenced a style ('camel_case_underscore') +# and a symbol group ('private_fields') that were never defined, so they never applied. The real +# private-field rules are declared in the "StyleCop Field Naming Rules" section below. dotnet_naming_symbols.non_private_static_fields.applicable_accessibilities = public, protected, internal, protected_internal, private_protected dotnet_naming_symbols.non_private_static_fields.applicable_kinds = field dotnet_naming_symbols.non_private_static_fields.required_modifiers = static -# Constants are PascalCase +# Constants are PascalCase (field constants; local constants are locals and stay camelCase) dotnet_naming_rule.constants_should_be_pascal_case.severity = warning dotnet_naming_rule.constants_should_be_pascal_case.style = non_private_static_field_style dotnet_naming_rule.constants_should_be_pascal_case.symbols = constants dotnet_naming_style.constant_style.capitalization = pascal_case -dotnet_naming_symbols.constants.applicable_kinds = field, local +dotnet_naming_symbols.constants.applicable_kinds = field dotnet_naming_symbols.constants.required_modifiers = const # Locals and parameters are camelCase @@ -102,9 +101,7 @@ dotnet_naming_rule.locals_should_be_camel_case.severity = warning # camel_case_style - Define the camelCase style dotnet_naming_style.camel_case_style.capitalization = camel_case -dotnet_naming_style.static_field_style.required_prefix = s_ dotnet_naming_symbols.locals_and_parameters.applicable_kinds = parameter, local -dotnet_naming_symbols.static_fields.required_modifiers = static # first_upper_style - The first character must start with an upper-case character dotnet_naming_style.first_upper_style.capitalization = first_word_upper @@ -152,22 +149,22 @@ dotnet_naming_symbols.other_public_protected_fields_group.applicable_kinds = fie # StyleCop Field Naming Rules -# All constant fields must be PascalCase -dotnet_naming_rule.private_or_internal_field_should_be__fieldname.severity = warning -dotnet_naming_rule.private_or_internal_field_should_be__fieldname.style = _fieldname -dotnet_naming_rule.private_or_internal_field_should_be__fieldname.symbols = private_or_internal_field +# All non-private constant fields must be PascalCase. Private constants are owned by +# 'private_static_fields_group' below - a field must be covered by exactly one rule, otherwise the +# same violation is reported once per matching rule. dotnet_naming_rule.stylecop_constant_fields_must_be_pascal_case_rule.severity = warning dotnet_naming_rule.stylecop_constant_fields_must_be_pascal_case_rule.style = non_private_static_field_style dotnet_naming_rule.stylecop_constant_fields_must_be_pascal_case_rule.symbols = stylecop_constant_fields_group -dotnet_naming_symbols.stylecop_constant_fields_group.applicable_accessibilities = public, internal, protected_internal, protected, private_protected, private +dotnet_naming_symbols.stylecop_constant_fields_group.applicable_accessibilities = public, internal, protected_internal, protected, private_protected dotnet_naming_symbols.stylecop_constant_fields_group.applicable_kinds = field dotnet_naming_symbols.stylecop_constant_fields_group.required_modifiers = const -# All static readonly fields must be PascalCase +# All non-private static readonly fields must be PascalCase. Private static readonly fields are +# owned by 'private_static_fields_group' below (see the note on the constant rule above). dotnet_naming_rule.stylecop_static_readonly_fields_must_be_pascal_case_rule.severity = warning dotnet_naming_rule.stylecop_static_readonly_fields_must_be_pascal_case_rule.style = non_private_static_field_style dotnet_naming_rule.stylecop_static_readonly_fields_must_be_pascal_case_rule.symbols = stylecop_static_readonly_fields_group -dotnet_naming_symbols.stylecop_static_readonly_fields_group.applicable_accessibilities = public, internal, protected_internal, protected, private_protected, private +dotnet_naming_symbols.stylecop_static_readonly_fields_group.applicable_accessibilities = public, internal, protected_internal, protected, private_protected dotnet_naming_symbols.stylecop_static_readonly_fields_group.applicable_kinds = field dotnet_naming_symbols.stylecop_static_readonly_fields_group.required_modifiers = static, readonly @@ -178,12 +175,22 @@ dotnet_naming_rule.stylecop_instance_fields_must_be_private_rule.symbols = style dotnet_naming_symbols.stylecop_fields_must_be_private_group.applicable_accessibilities = public, internal, protected_internal, protected, private_protected dotnet_naming_symbols.stylecop_fields_must_be_private_group.applicable_kinds = field -# Private fields must be camelCase -dotnet_naming_rule.stylecop_private_fields_must_be_camel_case_rule.severity = warning -dotnet_naming_rule.stylecop_private_fields_must_be_camel_case_rule.style = camel_case_style -dotnet_naming_rule.stylecop_private_fields_must_be_camel_case_rule.symbols = stylecop_private_fields_group -dotnet_naming_symbols.stylecop_private_fields_group.applicable_accessibilities = private -dotnet_naming_symbols.stylecop_private_fields_group.applicable_kinds = field +# Private static fields are PascalCase: they are type-level state and are never qualified with 'this.' +dotnet_naming_rule.private_static_fields_must_be_pascal_case_rule.severity = warning +dotnet_naming_rule.private_static_fields_must_be_pascal_case_rule.style = non_private_static_field_style +dotnet_naming_rule.private_static_fields_must_be_pascal_case_rule.symbols = private_static_fields_group +dotnet_naming_symbols.private_static_fields_group.applicable_accessibilities = private +dotnet_naming_symbols.private_static_fields_group.applicable_kinds = field +dotnet_naming_symbols.private_static_fields_group.required_modifiers = static + +# Private instance fields must be camelCase with a leading underscore: '_name', never 'name'. The +# prefix keeps field access unambiguous (so the noisy 'this.' qualifier is never needed) and keeps +# fields distinguishable from locals and parameters. +dotnet_naming_rule.private_instance_fields_must_be_camel_case_with_underscore_prefix.severity = warning +dotnet_naming_rule.private_instance_fields_must_be_camel_case_with_underscore_prefix.style = camel_case_underscore_style +dotnet_naming_rule.private_instance_fields_must_be_camel_case_with_underscore_prefix.symbols = private_instance_fields_group +dotnet_naming_symbols.private_instance_fields_group.applicable_accessibilities = private +dotnet_naming_symbols.private_instance_fields_group.applicable_kinds = field # Local variables must be camelCase dotnet_naming_rule.stylecop_local_fields_must_be_camel_case_rule.severity = silent @@ -232,25 +239,14 @@ dotnet_naming_style.type_parameter_style.required_prefix = T dotnet_naming_symbols.type_parameter_symbol.applicable_accessibilities = * dotnet_naming_symbols.type_parameter_symbol.applicable_kinds = type_parameter -# Instance fields are camelCase and start with _ -dotnet_naming_rule.camel_case_for_private_internal_fields.severity = suggestion -dotnet_naming_rule.camel_case_for_private_internal_fields.style = camel_case_underscore_style -dotnet_naming_rule.camel_case_for_private_internal_fields.symbols = private_internal_fields -dotnet_naming_rule.instance_fields_should_be_camel_case.severity = suggestion -dotnet_naming_rule.instance_fields_should_be_camel_case.style = camel_case_underscore_style -dotnet_naming_rule.instance_fields_should_be_camel_case.symbols = instance_fields +# Naming style shared by the private-field rules: camelCase with a required '_' prefix. dotnet_naming_style.camel_case_underscore_style.capitalization = camel_case dotnet_naming_style.camel_case_underscore_style.required_prefix = _ -dotnet_naming_style.instance_field_style.capitalization = camel_case -dotnet_naming_style.instance_field_style.required_prefix = _ -dotnet_naming_symbols.instance_fields.applicable_kinds = field -dotnet_naming_symbols.private_internal_fields.applicable_accessibilities = private, internal -dotnet_naming_symbols.private_internal_fields.applicable_kinds = field # Local functions are PascalCase dotnet_naming_rule.local_functions_should_be_pascal_case.severity = warning -dotnet_naming_rule.local_functions_should_be_pascal_case.style = non_private_static_field_style -dotnet_naming_rule.local_functions_should_be_pascal_case.symbols = all_members +dotnet_naming_rule.local_functions_should_be_pascal_case.style = local_function_style +dotnet_naming_rule.local_functions_should_be_pascal_case.symbols = local_functions dotnet_naming_style.local_function_style.capitalization = pascal_case dotnet_naming_symbols.local_functions.applicable_kinds = local_function @@ -264,12 +260,10 @@ dotnet_style_qualification_for_property = false:silent dotnet_style_operator_placement_when_wrapping = end_of_line # Naming styles -dotnet_naming_rule.interface_should_be_begins_with_i.severity = warning -dotnet_naming_rule.interface_should_be_begins_with_i.style = prefix_interface_with_i_style -dotnet_naming_rule.interface_should_be_begins_with_i.symbols = interface -dotnet_naming_rule.types_should_be_pascal_case.severity = warning -dotnet_naming_rule.types_should_be_pascal_case.style = non_private_static_field_style -dotnet_naming_rule.types_should_be_pascal_case.symbols = types +# NOTE: a 'types_should_be_pascal_case' rule (symbol group 'types') and an +# 'interface_should_be_begins_with_i' rule (symbol group 'interface') used to sit here, but neither +# symbol group was ever defined, so neither rule applied. Types are covered by 'element_rule' and +# interfaces by 'interface_rule' above. # By default, name items with PascalCase dotnet_naming_rule.non_field_members_should_be_pascal_case.severity = warning @@ -278,7 +272,6 @@ dotnet_naming_rule.non_field_members_should_be_pascal_case.symbols = non_field_m # pascal_case_style - Define the PascalCase style dotnet_naming_style.pascal_case_style.capitalization = pascal_case -dotnet_naming_symbols.all_members.applicable_kinds = * # Symbol specifications dotnet_naming_symbols.non_field_members.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected @@ -286,7 +279,6 @@ dotnet_naming_symbols.non_field_members.applicable_kinds = property, event, meth dotnet_naming_symbols.non_field_members.required_modifiers = * # Naming styles -dotnet_naming_style._fieldname.capitalization = camel_case dotnet_naming_style.begins_with_i.capitalization = pascal_case dotnet_naming_style.begins_with_i.required_prefix = I dotnet_naming_style.begins_with_i.required_suffix = @@ -314,9 +306,11 @@ dotnet_code_quality.prefer_const = true dotnet_code_quality.prefer_inferred_anonymous_type_member_names = true dotnet_code_quality.prefer_inferred_tuple_names = true dotnet_code_quality.prefer_readonly = true -dotnet_code_quality.require_accessibility_modifiers = true +# The real modifier policy is 'dotnet_style_require_accessibility_modifiers' (declared under +# "Modifier preferences" below). The 'require_accessibility_modifiers' and +# 'require_explicit_visibility' entries that used to live here are not .NET analyzer options: +# declaring them only pretended to enforce a policy that was never applied, so they were removed. dotnet_code_quality.require_explicit_type_arguments = true -dotnet_code_quality.require_explicit_visibility = true dotnet_code_quality.require_variable_declaration_for_explicit_type = false dotnet_code_quality_unused_parameters = all:warning dotnet_enable_roslyn_analyzers = true @@ -587,10 +581,14 @@ dotnet_diagnostic.CA1030.severity = suggestion dotnet_diagnostic.CA1031.severity = error # Implement standard exception constructors -dotnet_diagnostic.CA1032.severity = suggestion +# Raised to warning: the public API surface of exceptions is part of the accessibility policy and +# must not be invisible to command-line builds. +dotnet_diagnostic.CA1032.severity = warning # Interface methods should be callable by child types -dotnet_diagnostic.CA1033.severity = suggestion +# Raised to warning: explicit interface implementations that hide the member from derived types are +# an accessibility problem, not a style preference. +dotnet_diagnostic.CA1033.severity = warning # Nested types should not be visible dotnet_diagnostic.CA1034.severity = error @@ -599,7 +597,9 @@ dotnet_diagnostic.CA1034.severity = error dotnet_diagnostic.CA1036.severity = suggestion # Avoid empty interfaces -dotnet_diagnostic.CA1040.severity = suggestion +# Raised to warning: empty interfaces are unenforceable contracts and were previously invisible to +# command-line builds. +dotnet_diagnostic.CA1040.severity = warning # Provide ObsoleteAttribute message dotnet_diagnostic.CA1041.severity = warning @@ -794,7 +794,9 @@ dotnet_diagnostic.CA1513.severity = warning dotnet_diagnostic.CA1514.severity = warning # Consider making public types internal -dotnet_diagnostic.CA1515.severity = suggestion +# Reported as a real build warning (not a suggestion) so accessibility hygiene cannot be ignored: +# a public type in an application or test assembly should be made internal rather than suppressed. +dotnet_diagnostic.CA1515.severity = warning # Naming Rules (CA1700-CA1727 and IDE0130) # Naming rules support adherence to the naming conventions of the .NET design guidelines @@ -973,8 +975,9 @@ dotnet_diagnostic.CA1845.severity = error # Prefer AsSpan over Substring dotnet_diagnostic.CA1846.severity = error -dotnet_diagnostic.ignore_internalsvisibleto = true -dotnet_diagnostic.CA1852.ignore_internalsvisibleto = true +# CA1852 (seal internal types) is intentionally not suppressed for internal types that are exposed +# to other assemblies through InternalsVisibleTo: those types must still be sealed. The blanket +# suppression that used to sit here relied on a malformed key, so it never even applied. # Unsafe DataSet or DataTable in serializable type can be vulnerable to remote code execution attacks dotnet_diagnostic.CA2352.severity = error @@ -1319,8 +1322,9 @@ dotnet_diagnostic.IDE0038.severity = warning # Use local function instead of lambda dotnet_diagnostic.IDE0039.severity = suggestion -# Add accessibility modifiers -dotnet_diagnostic.IDE0040.severity = error +# IDE0040 is declared exactly once, in the "Accessibility modifiers" section above. A second entry +# here used to override that value (later entries win), which made the effective modifier policy +# ambiguous and let repos believe the rule had been turned off. # Use is null check dotnet_diagnostic.IDE0041.severity = suggestion @@ -1557,8 +1561,9 @@ dotnet_diagnostic.IDE0380.severity = warning # Remove unnecessary suppression (null-forgiving operator) dotnet_diagnostic.IDE0370.severity = warning -# Naming rule violation -dotnet_diagnostic.IDE1006.severity = silent +# Naming rule violation - must stay visible: the field-naming policy is only enforced when this +# diagnostic is reported, because naming rules without their own severity inherit this one. +dotnet_diagnostic.IDE1006.severity = warning # Embedded statements must be on their own line dotnet_diagnostic.IDE2001.severity = warning @@ -2638,7 +2643,7 @@ trim_trailing_whitespace = false # If it's a settings, model, dto, etc file, ignore the 'properties' cannot have arrays # and other annoying rules - the following section is duplicated -[**/*{ValueObjects,Settings,Options,Model,Models,DTO,Entity,Response,Request}.{cs,vb},] +[**/*{ValueObjects,Settings,Options,Model,Models,DTO,Entity,Response,Request}{.Nav,}.{cs,vb}] dotnet_diagnostic.CA1002.severity = none dotnet_diagnostic.CA1024.severity = none dotnet_diagnostic.CA1056.severity = none @@ -2673,15 +2678,11 @@ dotnet_diagnostic.PDS0004.severity = none # Files under the Extensions/ namespace-reset convention must not force developers to add pragmas; # suppress the namespace-conflict diagnostics that arise from declaring framework-style namespaces. dotnet_diagnostic.IDE0130.severity = none -dotnet_diagnostic.CA1034.severity = none dotnet_diagnostic.CA1724.severity = none dotnet_diagnostic.CS0436.severity = none dotnet_diagnostic.CS1591.severity = none dotnet_diagnostic.IDE0005.severity = none -[**/{Extension,Extensions}.{cs,vb}] -dotnet_diagnostic.CA1034.severity = none - [**/Generated/**/*.{cs,vb}] generated_code = true dotnet_diagnostic.CS8602.severity = none diff --git a/Justfile b/Justfile index 01a9e2a..f95b37d 100644 --- a/Justfile +++ b/Justfile @@ -12,8 +12,7 @@ build_configuration := "Debug" artifacts_folder := "./artifacts" default_test_filter := "/*/*/*/*" -pipeline_feed := "https://api.nuget.org/v3/index.json" -pipeline_tool := ".tools/purview-build/purview-build" +pipeline_tool := "dotnet purview-build" current_version := `bun -p "require('./package.json').version"` @@ -21,33 +20,31 @@ current_version := `bun -p "require('./package.json').version"` default: just --list -# Install the shared Purview.Build tool (authenticated to the Purview-Dev feed) if not present +# Restore the repository's .NET local tools (.config/dotnet-tools.json) [private] ensure-pipeline-tool: - if [ ! -x "{{ pipeline_tool }}" ]; then \ - dotnet tool install Purview.Build --tool-path .tools/purview-build --add-source "{{ pipeline_feed }}"; \ - fi + dotnet tool restore # Run the PR pipeline (restore, build, lint, tests) [group('Pipeline')] pipeline-pr *args: just ensure-pipeline-tool echo "Running PR pipeline..." - "{{ pipeline_tool }}" {{ args }} + {{ pipeline_tool }} {{ args }} # Run the build pipeline (restore, build, lint) [group('Pipeline')] pipeline-build *args: just ensure-pipeline-tool echo "Running build pipeline..." - "{{ pipeline_tool }}" --Build:RunTests=false --Release:Mode=None {{ args }} + {{ pipeline_tool }} --Build:RunTests=false --Release:Mode=None {{ args }} # Run the release pipeline (restore, build, lint, tests, pack, publish, GitHub release) [group('Pipeline')] pipeline-release *args: just ensure-pipeline-tool echo "Running release pipeline..." - "{{ pipeline_tool }}" --Release:Mode=NuGet {{ args }} + {{ pipeline_tool }} --Release:Mode=NuGet {{ args }} # Run the release pipeline (restore, build, lint, tests, pack, local nuget publish) # Note: `just` runs recipes through the shell, which strips backslashes from unquoted arguments. @@ -58,21 +55,21 @@ pipeline-local-release *args: just ensure-pipeline-tool just lint-fix echo "Running local release pipeline..." - "{{ pipeline_tool }}" --Release:Mode=LocalNuGet {{ args }} + {{ pipeline_tool }} --Release:Mode=LocalNuGet {{ args }} # Run the pipeline with tests enabled [group('Pipeline')] pipeline-tests *args: just ensure-pipeline-tool echo "Running tests pipeline..." - "{{ pipeline_tool }}" --Build:RunTests=true --Release:Mode=None {{ args }} + {{ pipeline_tool }} --Build:RunTests=true --Release:Mode=None {{ args }} # Run the pipeline through pack + validate (restore, build, lint, tests, pack, validate pack contents) without publishing/releasing [group('Pipeline')] pipeline-pack-validate *args: just ensure-pipeline-tool echo "Running pack + validate pipeline..." - "{{ pipeline_tool }}" --Build:RunPack=true --Build:ValidatePack=true --Release:Mode=None {{ args }} + {{ pipeline_tool }} --Build:RunPack=true --Build:ValidatePack=true --Release:Mode=None {{ args }} # Build and test with the specified configuration, defaulting to "Debug" [group('Build and Test')] diff --git a/docs/wiki/Analyzers.md b/docs/wiki/Analyzers.md index 0f10824..4cf5e7b 100644 --- a/docs/wiki/Analyzers.md +++ b/docs/wiki/Analyzers.md @@ -69,6 +69,7 @@ Opt out with `PurviewSourceGeneratorFrameworkAnalyzerValidation=false` (`PRSGD00 | `PSGFR39` | A non-packable Roslyn component that explicitly opts out of the default self-contained analyzer output (`PurviewMergeSourceGeneratorFrameworkForAnalyzerFiles=false`) while embedding the framework, otherwise the package embeds the loose framework DLL under `analyzers/`. | | `PSGFR40` | In Roslyn components (`IsRoslynComponent=true`), reference SGF types as inline code (`Type`) instead of a `cref`: copied documentation must not depend on cref resolution. | | `PSGFR41` | In Roslyn components whose framework implementation is merged, a public member (or generic constraint) whose signature references an SGF type: the merge internalizes every SGF type, so the signature is left referring to an internal type. Make the member or its declaring type non-public. | +| `PSGFR42` | A disposable Testing-harness result (`DriverRunResult`) is produced and never disposed, leaking the collectible `AssemblyLoadContext` and `MetadataLoadContext` of a compiled run. Reported as a `warning` when the run syntactically compiles to an assembly (a `Compile` options member or `CompileToAssembly = true`), otherwise as `info`. | ## Type-library and attribute-model diagnostics @@ -101,6 +102,8 @@ analyzer rules above, including: - `PreferInlineCodeForFrameworkCrefCodeFixProvider` — rewrites SGF XML doc `cref` targets to inline code (`Type`) (`PSGFR40`). - `MakeComponentSurfaceNonPublicCodeFixProvider` — makes the exposing member, or its declaring type, non-public (`PSGFR41`). +- `DisposeTestResultCodeFixProvider` — turns an undisposed test-harness result declaration into a + `using` declaration (`PSGFR42`). - `MakeTypeLibrarySpecNonPublicCodeFixProvider` — declares a type-library spec non-public in a merged component (`TLB0021`). - `AttributeDataModelSymbolPropertyCodeFixProvider` — fixes attribute-data-model symbol properties. diff --git a/docs/wiki/Release-Flow.md b/docs/wiki/Release-Flow.md index 15b146f..19f4f88 100644 --- a/docs/wiki/Release-Flow.md +++ b/docs/wiki/Release-Flow.md @@ -34,8 +34,8 @@ packs, validates, publishes to NuGet, and creates the GitHub release. ## Local pipelines -The `Justfile` wraps the shared `Purview.Build` pipeline (installed as a pinned dotnet tool to -`.tools/purview-build/purview-build`): +The `Justfile` wraps the shared `Purview.Build` pipeline (a pinned .NET local tool, restored from +`.config/dotnet-tools.json` and run as `dotnet purview-build`): | Recipe | Pipeline mode | Purpose | | --- | --- | --- | diff --git a/docs/wiki/Step-Cache-Tests.md b/docs/wiki/Step-Cache-Tests.md index 81509b5..611e2de 100644 --- a/docs/wiki/Step-Cache-Tests.md +++ b/docs/wiki/Step-Cache-Tests.md @@ -43,6 +43,23 @@ exactly what the driver decided to recompute. `GenerateIncrementalAsync` on `TUnitSourceGeneratorTestBase` wraps the same runner for test-base users. +## Log entries + +Each `IncrementalCacheRun` carries the framework log entries that **that run** wrote, in the order the +generator wrote them, through `IncrementalCacheRun.LogEntries`. The sink is registered for the whole +sequence (the driver is shared) and drained between runs, so entries never accumulate across runs: + +```csharp +var result = await GenerateIncrementalAsync([Source], cancellationToken: cancellationToken); + +await Assert.That(result.Runs[0].LogEntries).IsNotEmpty(); // the first run executed the pipeline +await Assert.That(result.Runs[1].LogEntries).IsEmpty(); // the rerun reused the cached output +``` + +That makes logging a second, independent signal alongside the step reasons: a stage reported as `Cached` +should not have logged anything on that run. Entries are empty when +`SourceGeneratorTestOptions.EnableLogging` is disabled. + ## Step reasons Each pipeline stage (tracked by its tracking name) has outputs, each carrying an diff --git a/docs/wiki/Testing.md b/docs/wiki/Testing.md index 20fd893..2a0deeb 100644 --- a/docs/wiki/Testing.md +++ b/docs/wiki/Testing.md @@ -154,6 +154,26 @@ emitted assembly: inspect types, members and attributes without loading it into executing any code. They are created lazily on first access. Dispose the result (or its `DriverRunResult`) to unload the collectible context and release the metadata view. +[`PSGFR42`](Analyzers.md#rule-reference) enforces that: a `DriverRunResult` that is produced and never +disposed — including the indirect case where a helper creates a result, projects a value out of it and +returns only the projection — is flagged, as a `warning` when the run syntactically compiles to an assembly +and as `info` otherwise. `DisposeTestResultCodeFixProvider` inserts the `using`. + +## Reference sharing and parallelism + +The harness binds every trusted-platform assembly once per test process and hands the **same** +`MetadataReference` instances to every test compilation, so Roslyn reuses one set of assembly symbols +across the run instead of rebuilding symbol tables per test. Over 100 compilations of the same source +against the same assembly set, shared references peak at 156 MB where per-compilation references peak at +8,146 MB. An assembly named by `AdditionalAssemblyTypes` that is already in the trusted-platform set is a +no-op: the reference is not added a second time. + +Because of that sharing, memory is largely flat in the degree of parallelism — but the test platform's +auto-detected default still costs more memory *and* more wall clock than an explicit ceiling. Wall clock +saturates at around four concurrent tests. On a 22-core machine, `--maximum-parallel-tests 8` measured +347–365 MB / 7.7–8.7s against 528–881 MB / 12.0–13.8s for the auto default. Set an explicit ceiling for a +test project that multi-targets, where several TFM executables run concurrently under `dotnet test`. + Analyzer options are preserved under their supplied keys. Keys without the Roslyn `build_property.` prefix are additionally exposed as compiler-visible MSBuild properties, so either `MyGenerator_Disable` or `build_property.MyGenerator_Disable` can be used in tests. diff --git a/global.json b/global.json index 7c48b98..5ad4318 100644 --- a/global.json +++ b/global.json @@ -3,9 +3,9 @@ "allowPrerelease": false }, "msbuild-sdks": { - "Purview.BuildSdk": "1.0.4" + "Purview.BuildSdk": "1.0.5" }, "test": { "runner": "Microsoft.Testing.Platform" } -} +} \ No newline at end of file diff --git a/package.json b/package.json index 7942507..a5db2d2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { - "name": "purview-sourcegenerator-framework", - "version": "1.0.0", + "name": "sourcegenerator-framework", + "version": "1.1.0", "license": "MIT", "author": { "name": "Kieron Lanning", diff --git a/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md b/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md index be387d6..aa2d6e7 100644 --- a/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md @@ -17,6 +17,7 @@ PSGFR38 | Purview.SourceGeneratorFramework | Warning | Extension class is missin PSGFR39 | Purview.SourceGeneratorFramework | Error | Roslyn component must produce a self-contained analyzer PSGFR40 | Purview.SourceGeneratorFramework | Warning | SGF cref should be inline code PSGFR41 | Purview.SourceGeneratorFramework | Warning | Component public surface exposes framework types +PSGFR42 | Purview.SourceGeneratorFramework | Info | Dispose test harness results with a using declaration TLB0014 | TypeLibrary | Warning | Type library partial extension is declared in a different namespace TLB0015 | TypeLibrary | Info | Type library partial extension must be declared 'public static partial' TLB0016 | TypeLibrary | Error | Enum value member type must be TypeIdentity or EnumValueDefinition diff --git a/src/src/SourceGeneratorFramework.Analyzers/UndisposedTestResultAnalyzer.cs b/src/src/SourceGeneratorFramework.Analyzers/UndisposedTestResultAnalyzer.cs new file mode 100644 index 0000000..b5598ad --- /dev/null +++ b/src/src/SourceGeneratorFramework.Analyzers/UndisposedTestResultAnalyzer.cs @@ -0,0 +1,280 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Purview.SourceGeneratorFramework.Analyzers; + +/// +/// Flags a disposable Testing-harness result that is produced and then never disposed, which leaks the +/// collectible AssemblyLoadContext and MetadataLoadContext created for a compiled run. +/// +/// +/// The rule is reported as a warning when the run syntactically opts into compiling to an assembly (a +/// Compile options member, or CompileToAssembly = true), because that is when the undisposed +/// result actually holds an emitted assembly; otherwise it is informational. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class UndisposedTestResultAnalyzer : DiagnosticAnalyzer +{ + public const string DiagnosticId = "PSGFR42"; + + /// + /// Set on the diagnostic when the fix can turn the declaration into a using declaration. + /// + internal const string FixableProperty = "Fixable"; + + const string TestingNamespace = "Purview.SourceGeneratorFramework.Testing"; + + public static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + "Dispose test harness results with a using declaration", + "The {0} produced by {1} is never disposed; declare it with 'using' so the compiled assembly and its load contexts are released", + "Purview.SourceGeneratorFramework", + DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: "Test-harness results are disposable because a run that compiles to an assembly creates a collectible AssemblyLoadContext and a MetadataLoadContext. A result that is neither consumed by a using declaration nor handed to a caller leaks both for the lifetime of the test process." + ); + + public override ImmutableArray SupportedDiagnostics => [Rule]; + + public override void Initialize(AnalysisContext context) + { + if (context is null) + throw new ArgumentNullException(nameof(context)); + + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(AnalyzeLocalDeclaration, SyntaxKind.LocalDeclarationStatement); + context.RegisterSyntaxNodeAction(AnalyzeExpressionStatement, SyntaxKind.ExpressionStatement); + } + + static void AnalyzeLocalDeclaration(SyntaxNodeAnalysisContext context) + { + if (context.Node is not LocalDeclarationStatementSyntax declaration) + return; + + // 'using var result = ...' and 'await using var result = ...' already own the result. + if (!declaration.UsingKeyword.IsKind(SyntaxKind.None)) + return; + + foreach (var declarator in declaration.Declaration.Variables) + { + if (declarator.Initializer?.Value is not { } initializer) + continue; + + if (Unwrap(initializer) is not InvocationExpressionSyntax and not ObjectCreationExpressionSyntax) + continue; + + if ( + context.SemanticModel.GetDeclaredSymbol(declarator, context.CancellationToken) is not ILocalSymbol local + ) + continue; + + if (!IsDisposableTestingResult(local.Type)) + continue; + + if (IsOwnedElsewhere(declarator, local, context)) + continue; + + context.ReportDiagnostic( + Diagnostic.Create( + Rule, + declarator.Identifier.GetLocation(), + ResolveSeverity(initializer), + additionalLocations: null, + properties: ImmutableDictionary.Empty.Add(FixableProperty, bool.TrueString), + local.Type.Name, + DescribeProducer(initializer) + ) + ); + } + } + + static void AnalyzeExpressionStatement(SyntaxNodeAnalysisContext context) + { + if (context.Node is not ExpressionStatementSyntax statement) + return; + + var expression = statement.Expression; + + // '_ = await RunAsync(...)' discards the result just as completely as dropping it. + if ( + expression is AssignmentExpressionSyntax + { + Left: IdentifierNameSyntax { Identifier.ValueText: "_" } + } discard + ) + expression = discard.Right; + + var produced = Unwrap(expression); + if (produced is not InvocationExpressionSyntax and not ObjectCreationExpressionSyntax) + return; + + var type = context.SemanticModel.GetTypeInfo(expression, context.CancellationToken).Type; + if (!IsDisposableTestingResult(type)) + return; + + context.ReportDiagnostic( + Diagnostic.Create( + Rule, + produced.GetLocation(), + ResolveSeverity(expression), + additionalLocations: null, + properties: null, + type!.Name, + DescribeProducer(expression) + ) + ); + } + + static ExpressionSyntax Unwrap(ExpressionSyntax expression) => + expression switch + { + AwaitExpressionSyntax await => Unwrap(await.Expression), + ParenthesizedExpressionSyntax parenthesized => Unwrap(parenthesized.Expression), + _ => expression, + }; + + static bool IsDisposableTestingResult(ITypeSymbol? type) + { + if (type is not INamedTypeSymbol named) + return false; + + var @namespace = named.ContainingNamespace?.ToDisplayString(); + if ( + @namespace is null + || ( + !string.Equals(@namespace, TestingNamespace, StringComparison.Ordinal) + && !@namespace.StartsWith(TestingNamespace + ".", StringComparison.Ordinal) + ) + ) + return false; + + return named.AllInterfaces.Any(static i => + i.ToDisplayString() is "System.IDisposable" or "System.IAsyncDisposable" + ); + } + + /// + /// Determines whether the local hands the result to someone else, or disposes it explicitly. + /// + /// + /// Reading members off the result (result.GetSource(...)) or passing it by value to an assertion + /// does not transfer ownership: the declaring method still has to dispose it. Returning it, yielding it, + /// assigning it elsewhere, or passing it by reference does. + /// + static bool IsOwnedElsewhere( + VariableDeclaratorSyntax declarator, + ILocalSymbol local, + SyntaxNodeAnalysisContext context + ) + { + var body = FindEnclosingBody(declarator); + if (body is null) + return true; + + foreach (var identifier in body.DescendantNodes().OfType()) + { + if (identifier.Identifier.ValueText != local.Name) + continue; + + if ( + !SymbolEqualityComparer.Default.Equals( + context.SemanticModel.GetSymbolInfo(identifier, context.CancellationToken).Symbol, + local + ) + ) + continue; + + if (Transfers(identifier)) + return true; + } + + return false; + } + + static bool Transfers(IdentifierNameSyntax identifier) => + identifier.Parent switch + { + ReturnStatementSyntax + or YieldStatementSyntax + or ArrowExpressionClauseSyntax + or UsingStatementSyntax + or EqualsValueClauseSyntax => true, + AssignmentExpressionSyntax assignment => assignment.Right == identifier, + ArgumentSyntax argument => !argument.RefKindKeyword.IsKind(SyntaxKind.None), + MemberAccessExpressionSyntax member => member.Expression == identifier + && member.Name.Identifier.ValueText is "Dispose" or "DisposeAsync", + ConditionalAccessExpressionSyntax conditional => conditional.Expression == identifier + && conditional.WhenNotNull.ToString().Contains("Dispose"), + _ => false, + }; + + static SyntaxNode? FindEnclosingBody(SyntaxNode node) + { + for (var current = node.Parent; current is not null; current = current.Parent) + { + if (current is BaseMethodDeclarationSyntax method) + return (SyntaxNode?)method.Body ?? method.ExpressionBody; + + if (current is LocalFunctionStatementSyntax localFunction) + return (SyntaxNode?)localFunction.Body ?? localFunction.ExpressionBody; + + if (current is AnonymousFunctionExpressionSyntax lambda) + return lambda.Body; + + if (current is AccessorDeclarationSyntax accessor) + return (SyntaxNode?)accessor.Body ?? accessor.ExpressionBody; + } + + return null; + } + + /// + /// Escalates the rule to a warning when the run syntactically compiles to an assembly, which is when the + /// undisposed result actually holds a loaded assembly rather than only syntax trees. + /// + static DiagnosticSeverity ResolveSeverity(ExpressionSyntax producer) => + CompilesToAssembly(producer) ? DiagnosticSeverity.Warning : DiagnosticSeverity.Info; + + static bool CompilesToAssembly(SyntaxNode producer) + { + foreach (var node in producer.DescendantNodesAndSelf()) + { + // '.Compile()' and the 'Options.Compile' convenience member both opt in. + if ( + node + is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "Compile" } + or IdentifierNameSyntax { Identifier.ValueText: "Compile" } + ) + return true; + + if ( + node + is AssignmentExpressionSyntax + { + Left: IdentifierNameSyntax { Identifier.ValueText: "CompileToAssembly" }, + Right: LiteralExpressionSyntax right, + } + && right.IsKind(SyntaxKind.TrueLiteralExpression) + ) + return true; + } + + return false; + } + + static string DescribeProducer(ExpressionSyntax producer) => + Unwrap(producer) switch + { + InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax member } => member + .Name + .Identifier + .ValueText, + InvocationExpressionSyntax { Expression: IdentifierNameSyntax name } => name.Identifier.ValueText, + ObjectCreationExpressionSyntax creation => creation.Type.ToString(), + _ => producer.ToString(), + }; +} diff --git a/src/src/SourceGeneratorFramework.Benchmarks/Benchmarks/SourceGeneratorTestRunnerBenchmarks.cs b/src/src/SourceGeneratorFramework.Benchmarks/Benchmarks/SourceGeneratorTestRunnerBenchmarks.cs index 3f51089..c7713b6 100644 --- a/src/src/SourceGeneratorFramework.Benchmarks/Benchmarks/SourceGeneratorTestRunnerBenchmarks.cs +++ b/src/src/SourceGeneratorFramework.Benchmarks/Benchmarks/SourceGeneratorTestRunnerBenchmarks.cs @@ -40,6 +40,8 @@ public async Task RunAsync() ValidateCodeWriterScopes = false, }; - await _runner.RunAsync(_source, options); + // Disposed because a CompileToAssembly run creates a collectible AssemblyLoadContext: leaving it to + // the finalizer would accumulate one per iteration and measure the leak rather than the run. + using var result = await _runner.RunAsync(_source, options); } } diff --git a/src/src/SourceGeneratorFramework.CodeFixers/ConvertToExtensionBlockCodeFixProvider.cs b/src/src/SourceGeneratorFramework.CodeFixers/ConvertToExtensionBlockCodeFixProvider.cs index 947f6e3..2ccefe8 100644 --- a/src/src/SourceGeneratorFramework.CodeFixers/ConvertToExtensionBlockCodeFixProvider.cs +++ b/src/src/SourceGeneratorFramework.CodeFixers/ConvertToExtensionBlockCodeFixProvider.cs @@ -96,7 +96,7 @@ member is MethodDeclarationSyntax method // The receiver is a method type parameter; move the type parameters and constraints to // the extension declaration so the receiver resolves against it. typeParameterList ??= method.TypeParameterList; - if (methodConstraintClauses(method) is { Count: > 0 } constraints) + if (MethodConstraintClauses(method) is { Count: > 0 } constraints) constraintClauses = constraints; } @@ -196,6 +196,6 @@ static MethodDeclarationSyntax ConvertMethod( return updated.WithParameterList(newParameterList); } - static SyntaxList methodConstraintClauses(MethodDeclarationSyntax method) => + static SyntaxList MethodConstraintClauses(MethodDeclarationSyntax method) => method.ConstraintClauses; } diff --git a/src/src/SourceGeneratorFramework.CodeFixers/DisposeTestResultCodeFixProvider.cs b/src/src/SourceGeneratorFramework.CodeFixers/DisposeTestResultCodeFixProvider.cs new file mode 100644 index 0000000..616a63b --- /dev/null +++ b/src/src/SourceGeneratorFramework.CodeFixers/DisposeTestResultCodeFixProvider.cs @@ -0,0 +1,62 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Purview.SourceGeneratorFramework.CodeFixers; + +/// +/// Turns an undisposed test-harness result declaration into a using declaration, so the compiled +/// assembly and its load contexts are released when the test method returns. +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(DisposeTestResultCodeFixProvider))] +public sealed class DisposeTestResultCodeFixProvider : CodeFixProvider +{ + internal const string EquivalenceKey = "DisposeTestResult"; + + public override ImmutableArray FixableDiagnosticIds => ["PSGFR42"]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + return; + + foreach (var diagnostic in context.Diagnostics) + { + var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); + if (node.FirstAncestorOrSelf() is not { } declaration) + continue; + + // A multi-declarator statement cannot become a using declaration without splitting it. + if (declaration.Declaration.Variables.Count != 1 || !declaration.UsingKeyword.IsKind(SyntaxKind.None)) + continue; + + context.RegisterCodeFix( + CodeAction.Create( + "Consume the result with a using declaration", + _ => AddUsingAsync(context.Document, root, declaration), + EquivalenceKey + ), + diagnostic + ); + } + } + + static Task AddUsingAsync(Document document, SyntaxNode root, LocalDeclarationStatementSyntax declaration) + { + // The using keyword takes the statement's leading trivia so the indentation is preserved. + var usingToken = SyntaxFactory + .Token(SyntaxKind.UsingKeyword) + .WithLeadingTrivia(declaration.GetLeadingTrivia()) + .WithTrailingTrivia(SyntaxFactory.Space); + + var updated = declaration.WithLeadingTrivia(SyntaxFactory.TriviaList()).WithUsingKeyword(usingToken); + + return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(declaration, updated))); + } +} diff --git a/src/src/SourceGeneratorFramework.ExampleGenerator/Extensions/System/StringExtensions.cs b/src/src/SourceGeneratorFramework.ExampleGenerator/Extensions/System/StringExtensions.cs index 066ed78..47e68ea 100644 --- a/src/src/SourceGeneratorFramework.ExampleGenerator/Extensions/System/StringExtensions.cs +++ b/src/src/SourceGeneratorFramework.ExampleGenerator/Extensions/System/StringExtensions.cs @@ -3,6 +3,11 @@ namespace System; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class StringExtensions { diff --git a/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs b/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs index a93ad0e..6fbe845 100644 --- a/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs +++ b/src/src/SourceGeneratorFramework.Generators/Helpers/AttributeDataModelLibrary.cs @@ -408,15 +408,12 @@ static ParameterAttributeInfo ReadParameterAttributes( } else { - var (IsTypeArgument, Sources, DefaultValue, HasDefaultValue) = ReadTypeArgumentAttributeInfo( - parameter, - isExcluded - ); + var typeArgumentInfo = ReadTypeArgumentAttributeInfo(parameter, isExcluded); - isTypeArgument = IsTypeArgument; - defaultValue = DefaultValue; - hasDefaultValue = HasDefaultValue; - sources.AddRange(Sources); + isTypeArgument = typeArgumentInfo.IsTypeArgument; + defaultValue = typeArgumentInfo.DefaultValue; + hasDefaultValue = typeArgumentInfo.HasDefaultValue; + sources.AddRange(typeArgumentInfo.Sources); } var hasExclusive = isExcluded || isNestedModel || isTypeArgument; diff --git a/src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs b/src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs index b0b96c1..a3bba31 100644 --- a/src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs +++ b/src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs @@ -71,7 +71,7 @@ static class FrameworkTypeInternalizer /// namespace, or the global namespace when the spec sets none), so they are only recognizable /// through the tool name on their GeneratedCodeAttribute. /// - static readonly ImmutableArray s_frameworkGeneratorToolNames = + static readonly ImmutableArray FrameworkGeneratorToolNames = [ "TypeLibraryGenerator", "AttributeDataModelGenerator", @@ -80,13 +80,13 @@ static class FrameworkTypeInternalizer /// /// Assembly-level attributes that grant other assemblies access to a component's internals. /// - static readonly ImmutableArray s_internalsGrantAttributeFullNames = + static readonly ImmutableArray InternalsGrantAttributeFullNames = [ "System.Runtime.CompilerServices.InternalsVisibleToAttribute", "System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute", ]; - static readonly ImmutableArray s_roslynComponentAttributeFullNames = + static readonly ImmutableArray RoslynComponentAttributeFullNames = [ "Microsoft.CodeAnalysis.GeneratorAttribute", "Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzerAttribute", @@ -94,7 +94,7 @@ static class FrameworkTypeInternalizer "Microsoft.CodeAnalysis.CodeRefactorings.ExportCodeRefactoringProviderAttribute", ]; - static readonly ImmutableArray s_roslynComponentInterfaceFullNames = + static readonly ImmutableArray RoslynComponentInterfaceFullNames = [ "Microsoft.CodeAnalysis.IIncrementalGenerator", "Microsoft.CodeAnalysis.ISourceGenerator", @@ -135,10 +135,7 @@ public static ImmutableArray CollectInternalsGrants(string assemblyPath) [ .. assembly .CustomAttributes.Where(static attribute => - s_internalsGrantAttributeFullNames.Contains( - attribute.AttributeType.FullName, - StringComparer.Ordinal - ) + InternalsGrantAttributeFullNames.Contains(attribute.AttributeType.FullName, StringComparer.Ordinal) ) .Select(static attribute => GrantedAssemblyName(attribute)) .Where(static name => name.Length > 0), @@ -288,7 +285,7 @@ static int StripInternalsGrants(AssemblyDefinition assembly, ImmutableArray= 0; index--) { var attribute = assembly.CustomAttributes[index]; - if (!s_internalsGrantAttributeFullNames.Contains(attribute.AttributeType.FullName, StringComparer.Ordinal)) + if (!InternalsGrantAttributeFullNames.Contains(attribute.AttributeType.FullName, StringComparer.Ordinal)) continue; if ( @@ -476,7 +473,7 @@ static bool IsFrameworkGenerated(TypeDefinition type) continue; } - if (s_frameworkGeneratorToolNames.Contains(toolName, StringComparer.Ordinal)) + if (FrameworkGeneratorToolNames.Contains(toolName, StringComparer.Ordinal)) return true; } @@ -521,7 +518,7 @@ static bool IsRoslynComponent(TypeDefinition type) { foreach (var attribute in type.CustomAttributes) { - if (s_roslynComponentAttributeFullNames.Contains(attribute.AttributeType.FullName, StringComparer.Ordinal)) + if (RoslynComponentAttributeFullNames.Contains(attribute.AttributeType.FullName, StringComparer.Ordinal)) return true; } @@ -546,8 +543,8 @@ static bool IsRoslynComponentType(TypeReference? type) return false; // The type may be a generic instance, so check the element type as well. - return s_roslynComponentInterfaceFullNames.Contains(type.FullName, StringComparer.Ordinal) - || s_roslynComponentInterfaceFullNames.Contains( + return RoslynComponentInterfaceFullNames.Contains(type.FullName, StringComparer.Ordinal) + || RoslynComponentInterfaceFullNames.Contains( Resolve(type)?.FullName ?? string.Empty, StringComparer.Ordinal ); diff --git a/src/src/SourceGeneratorFramework.Testing.TUnit/Sdk/.agents/skills/tunit-test-authoring/SKILL.md b/src/src/SourceGeneratorFramework.Testing.TUnit/Sdk/.agents/skills/tunit-test-authoring/SKILL.md index fa58cd8..cf38b75 100644 --- a/src/src/SourceGeneratorFramework.Testing.TUnit/Sdk/.agents/skills/tunit-test-authoring/SKILL.md +++ b/src/src/SourceGeneratorFramework.Testing.TUnit/Sdk/.agents/skills/tunit-test-authoring/SKILL.md @@ -57,6 +57,11 @@ Use the base hooks to customise per-run: `OnBeforeRun`/`OnBeforeRunAsync` (mutat `options.WithAdditionalSources(markerAttributeSource)`) and `OnAfterRun`/`OnAfterRunAsync`. Use `options.Compile()` to opt into `CompileToAssembly` while preserving the derived options type. +Write `using var result = await GenerateAsync(...)`: `DriverRunResult` is `IDisposable`, and `PSGFR42` +flags a result that is produced and never disposed (a `warning` once the run compiles to an assembly, +`info` otherwise). It also catches the indirect case — a helper that creates a result, projects a value out +of it and returns only the projection. + For code fixes/refactorings, select a specific registered action via `CodeFixTestOptions.EquivalenceKey` or `CodeActionIndex`, and `RefactorTestOptions.Span`/`NodeSelector` (e.g. `NodeSelector = query => query.GetMethod("M")`). @@ -137,6 +142,8 @@ Why the tests look the way they do: - `new IncrementalRunInput([Source], [("build_property.X", "value")])` toggles an MSBuild property for one run only, so a property-only change must mark `GetMSBuildPropertyValue_*`/`GetGenerationConfiguration`/ `GetGenerationContext_*` `Modified` while `ForAttribute_*` stays `Cached`. +- `result.Runs[n].LogEntries` carries the framework log entries *that run* wrote, in order, so a rerun whose + output step is cached can be asserted to have logged nothing. Empty when `EnableLogging` is disabled. - The `StepReasons(IncrementalCacheRun)` helper flattens each tracked step's `Outputs` into a `ImmutableDictionary>` so assertions can address a stage by name (see `source-generator-testing` for the helper body). diff --git a/src/src/SourceGeneratorFramework.Testing/CodeQuery.Declarations.cs b/src/src/SourceGeneratorFramework.Testing/CodeQuery.Declarations.cs index 28d4af4..f2f14be 100644 --- a/src/src/SourceGeneratorFramework.Testing/CodeQuery.Declarations.cs +++ b/src/src/SourceGeneratorFramework.Testing/CodeQuery.Declarations.cs @@ -481,9 +481,9 @@ public bool TryGetField(string name, out FieldDeclarationSyntax? field) if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("The field name cannot be null or whitespace.", nameof(name)); - foreach (var tree in Trees) + foreach (var root in Roots()) { - foreach (var declarator in RootOf(tree).DescendantNodes().OfType()) + foreach (var declarator in root.DescendantNodes().OfType()) { if (declarator.Identifier.ValueText != name) continue; @@ -617,9 +617,9 @@ static bool NamespaceMatches(SyntaxNode node, string? @namespace) => bool TryFind(string name, Func getName, Func? additional, out T? node) where T : SyntaxNode { - foreach (var tree in Trees) + foreach (var root in Roots()) { - foreach (var candidate in RootOf(tree).DescendantNodes().OfType()) + foreach (var candidate in root.DescendantNodes().OfType()) { if (getName(candidate) != name) continue; diff --git a/src/src/SourceGeneratorFramework.Testing/CodeQuery.Members.cs b/src/src/SourceGeneratorFramework.Testing/CodeQuery.Members.cs index 08412b8..a343163 100644 --- a/src/src/SourceGeneratorFramework.Testing/CodeQuery.Members.cs +++ b/src/src/SourceGeneratorFramework.Testing/CodeQuery.Members.cs @@ -53,9 +53,9 @@ params TypeReference[]? parameters throw new ArgumentException("The operator token cannot be null or whitespace.", nameof(operatorToken)); var expected = parameters ?? []; - foreach (var tree in Trees) + foreach (var root in Roots()) { - foreach (var candidate in RootOf(tree).DescendantNodes().OfType()) + foreach (var candidate in root.DescendantNodes().OfType()) { if (candidate.OperatorToken.ValueText != operatorToken) continue; @@ -115,9 +115,9 @@ params TypeReference[]? parameters throw new ArgumentException("The keyword must be 'implicit' or 'explicit'.", nameof(keyword)); var expected = parameters ?? []; - foreach (var tree in Trees) + foreach (var root in Roots()) { - foreach (var candidate in RootOf(tree).DescendantNodes().OfType()) + foreach (var candidate in root.DescendantNodes().OfType()) { if (candidate.ImplicitOrExplicitKeyword.ValueText != keyword) continue; @@ -159,9 +159,9 @@ public CodeQueryResult GetIndexer(params TypeReference public bool TryGetIndexer(out IndexerDeclarationSyntax? indexer, params TypeReference[] parameters) { var expected = parameters ?? []; - foreach (var tree in Trees) + foreach (var root in Roots()) { - foreach (var candidate in RootOf(tree).DescendantNodes().OfType()) + foreach (var candidate in root.DescendantNodes().OfType()) { if (expected.Length == 0 || MatchesParameters(candidate.ParameterList, expected)) { diff --git a/src/src/SourceGeneratorFramework.Testing/CodeQuery.Statements.cs b/src/src/SourceGeneratorFramework.Testing/CodeQuery.Statements.cs index 77a5dba..eba9551 100644 --- a/src/src/SourceGeneratorFramework.Testing/CodeQuery.Statements.cs +++ b/src/src/SourceGeneratorFramework.Testing/CodeQuery.Statements.cs @@ -91,9 +91,9 @@ public bool TryGetInvocation(string methodName, out InvocationExpressionSyntax? if (string.IsNullOrWhiteSpace(methodName)) throw new ArgumentException("The method name cannot be null or whitespace.", nameof(methodName)); - foreach (var tree in Trees) + foreach (var root in Roots()) { - foreach (var candidate in RootOf(tree).DescendantNodes().OfType()) + foreach (var candidate in root.DescendantNodes().OfType()) { if (GetInvokedMethodName(candidate) == methodName) { @@ -115,9 +115,9 @@ public bool HasObjectCreation(TypeReference type) if (type is null) throw new ArgumentNullException(nameof(type)); - foreach (var tree in Trees) + foreach (var root in Roots()) { - foreach (var candidate in RootOf(tree).DescendantNodes().OfType()) + foreach (var candidate in root.DescendantNodes().OfType()) { if (Matches(candidate.Type, type)) return true; diff --git a/src/src/SourceGeneratorFramework.Testing/CodeQuery.cs b/src/src/SourceGeneratorFramework.Testing/CodeQuery.cs index f5e24d8..49f69c5 100644 --- a/src/src/SourceGeneratorFramework.Testing/CodeQuery.cs +++ b/src/src/SourceGeneratorFramework.Testing/CodeQuery.cs @@ -67,7 +67,25 @@ public CodeQuery In(SyntaxNode root) return new(Trees, Compilation, IsGenerated, root); } - SyntaxNode RootOf(SyntaxTree tree) => Root ?? tree.GetRoot(); + /// + /// Enumerates the roots a node search runs over: the single scoping node when this is a nested query, + /// otherwise the root of every tree. + /// + /// + /// A scoped query has exactly one root no matter how many trees it carries. Enumerating per tree instead + /// would walk that one subtree once per tree, repeating every match. + /// + internal IEnumerable Roots() + { + if (Root is not null) + { + yield return Root; + yield break; + } + + foreach (var tree in Trees) + yield return tree.GetRoot(); + } // --------------------------------------------------------------------------------------------- // Syntax trees @@ -137,9 +155,8 @@ public ImmutableArray> GetAll(Func? predicate = n where T : SyntaxNode { var builder = ImmutableArray.CreateBuilder>(); - foreach (var tree in Trees) + foreach (var root in Roots()) { - var root = RootOf(tree); if (root is T rootNode && (predicate is null || predicate(rootNode))) builder.Add(new(this, rootNode)); @@ -160,9 +177,8 @@ public int Count(Func? predicate = null) where T : SyntaxNode { var count = 0; - foreach (var tree in Trees) + foreach (var root in Roots()) { - var root = RootOf(tree); if (root is T rootNode && (predicate is null || predicate(rootNode))) count++; @@ -179,9 +195,8 @@ public int Count(Func? predicate = null) public bool TryGet(out T? node, Func? predicate = null) where T : SyntaxNode { - foreach (var tree in Trees) + foreach (var root in Roots()) { - var root = RootOf(tree); if (root is T rootNode && (predicate is null || predicate(rootNode))) { node = rootNode; diff --git a/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/CodeWriterExtensions.cs b/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/CodeWriterExtensions.cs index e513833..4a24ba5 100644 --- a/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/CodeWriterExtensions.cs +++ b/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/CodeWriterExtensions.cs @@ -3,6 +3,11 @@ namespace Purview.SourceGeneratorFramework; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class CodeWriterExtensions { diff --git a/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/SourceGeneratorTestOptionsExtensions.cs b/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/SourceGeneratorTestOptionsExtensions.cs index d1c4c51..d7fd735 100644 --- a/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/SourceGeneratorTestOptionsExtensions.cs +++ b/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/SourceGeneratorTestOptionsExtensions.cs @@ -7,6 +7,11 @@ namespace Purview.SourceGeneratorFramework; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class SourceGeneratorTestOptionsExtensions { diff --git a/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/Testing/SourceGeneratorTestOptionsExtensions.cs b/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/Testing/SourceGeneratorTestOptionsExtensions.cs index 4a2e2e9..8c2e43d 100644 --- a/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/Testing/SourceGeneratorTestOptionsExtensions.cs +++ b/src/src/SourceGeneratorFramework.Testing/Extensions/Purview/SourceGeneratorFramework/Testing/SourceGeneratorTestOptionsExtensions.cs @@ -6,6 +6,11 @@ namespace Purview.SourceGeneratorFramework.Testing; /// /// Fluent extension methods that preserve the concrete options type for downstream derived records. /// +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class SourceGeneratorTestOptionsExtensions { diff --git a/src/src/SourceGeneratorFramework.Testing/IncrementalCacheResult.cs b/src/src/SourceGeneratorFramework.Testing/IncrementalCacheResult.cs index 084de2c..b927b62 100644 --- a/src/src/SourceGeneratorFramework.Testing/IncrementalCacheResult.cs +++ b/src/src/SourceGeneratorFramework.Testing/IncrementalCacheResult.cs @@ -1,5 +1,6 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis; +using Purview.SourceGeneratorFramework.Testing.Models; namespace Purview.SourceGeneratorFramework.Testing; @@ -29,7 +30,18 @@ public sealed record IncrementalRunInput( public sealed record IncrementalCacheRun( GeneratorRunResult RunResult, ImmutableDictionary> Steps -); +) +{ + /// + /// The framework log entries written by the generator during this run, in the order they were written. + /// Empty when is disabled. + /// + /// + /// Scoped to the single run rather than the whole sequence, so a rerun whose stages all report + /// can be asserted to have logged nothing. + /// + public ImmutableArray LogEntries { get; init; } = []; +} /// /// The result of an incremental cache test run: one per input, produced by a diff --git a/src/src/SourceGeneratorFramework.Testing/Properties/AssemblyInfo.cs b/src/src/SourceGeneratorFramework.Testing/Properties/AssemblyInfo.cs index 4f93730..5b9fa6b 100644 --- a/src/src/SourceGeneratorFramework.Testing/Properties/AssemblyInfo.cs +++ b/src/src/SourceGeneratorFramework.Testing/Properties/AssemblyInfo.cs @@ -1,4 +1,5 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("SourceGeneratorFramework.UnitTests")] +[assembly: InternalsVisibleTo("Purview.SourceGeneratorFramework.UnitTests")] [assembly: InternalsVisibleTo("Purview.SourceGeneratorFramework.Testing.TUnit")] diff --git a/src/src/SourceGeneratorFramework.Testing/Sdk/.agents/skills/source-generator-testing/SKILL.md b/src/src/SourceGeneratorFramework.Testing/Sdk/.agents/skills/source-generator-testing/SKILL.md index 2a56544..fdbfc45 100644 --- a/src/src/SourceGeneratorFramework.Testing/Sdk/.agents/skills/source-generator-testing/SKILL.md +++ b/src/src/SourceGeneratorFramework.Testing/Sdk/.agents/skills/source-generator-testing/SKILL.md @@ -147,6 +147,13 @@ keeps repeated runs from polluting the default context. `result.CompilationResul assembly (generated code can execute); `result.CompilationResult.Metadata` / `.MetadataAssembly` give a metadata-only reflection view (types, members, attributes) over the emitted assembly without executing code. +**Always write `using var result = ...`.** `PSGFR42` ("Dispose test harness results with a using +declaration") flags a `DriverRunResult` that is produced and never disposed — including the indirect case +where a helper creates a result, projects a value out of it and returns only the projection. It is a +`warning` when the run syntactically compiles to an assembly (`.Compile()`, a `Compile` options member, or +`CompileToAssembly = true`) and `info` otherwise; the `DisposeTestResultCodeFixProvider` fix inserts the +`using`. A result that is returned to the caller, or disposed explicitly, is not flagged. + ## Best practices - **Deterministic output**: `WriteAutoGeneratedHeader` is timestamp-free; assert with @@ -160,6 +167,12 @@ metadata-only reflection view (types, members, attributes) over the emitted asse - **Scope validation**: keep `PurviewSourceGeneratorFrameworkValidateCodeWriterScopes` enabled; it makes undisposed `CodeWriter` scopes fail tests. - **Prefer `CodeQuery` over string matching** for structural assertions (members, signatures, namespaces). +- **Dispose results**: `using var result = ...`, enforced by `PSGFR42`. +- **Cap parallelism**: the harness shares one `MetadataReference` set per process, so memory is largely flat + in the degree of parallelism, but the platform's auto-detected default is both slower and heavier than an + explicit ceiling. Wall clock saturates around four concurrent tests; prefer + `--maximum-parallel-tests 8` (or a `TestingPlatformCommandLineArguments` equivalent) for a project that + multi-targets and runs several TFM executables at once. ## Incremental cache testing (`RunIncrementalAsync`) @@ -205,6 +218,20 @@ these tests fail the build and catch the regression. Roslyn would see a fresh compilation on the second run and report stages `Modified`/`New` even though the sources are byte-identical — the "cached" assertion would fail. +### Log entries per run + +`IncrementalCacheRun.LogEntries` carries the framework log entries that run wrote, in order. The sink is +registered for the whole sequence and drained between runs, so entries never accumulate. A rerun whose +output step is cached logs nothing, which makes it a second signal alongside the step reasons: + +```csharp +await Assert.That(result.Runs[0].LogEntries).IsNotEmpty(); +await Assert.That(result.Runs[1].LogEntries).IsEmpty(); +``` + +`DriverRunResult.LogEntries` (the `RunAsync` path) is likewise in write order. Both are empty when +`EnableLogging` is disabled. + ### The `StepReasons` helper `IncrementalCacheRun.Steps` is `ImmutableDictionary>` diff --git a/src/src/SourceGeneratorFramework.Testing/Sdk/README.md b/src/src/SourceGeneratorFramework.Testing/Sdk/README.md index 12b28d1..d33eda6 100644 --- a/src/src/SourceGeneratorFramework.Testing/Sdk/README.md +++ b/src/src/SourceGeneratorFramework.Testing/Sdk/README.md @@ -147,6 +147,28 @@ assembly: inspect types, members and attributes without loading it into the runt They are created lazily on first access. Dispose the result (or its `DriverRunResult`) to unload the collectible context and release the metadata view. +**`PSGFR42` enforces this.** The analyzer flags a `DriverRunResult` that is produced and then never +disposed — no `using` declaration, not returned to a caller, not disposed explicitly. A helper that creates +a result, projects a value out of it and returns the projection leaks just as badly, so the local inside the +helper is flagged too. The rule is a `warning` when the run syntactically compiles to an assembly (a +`Compile` options member, or `CompileToAssembly = true`), because that is when the leak actually holds an +assembly and its load contexts, and `info` otherwise. `DisposeTestResultCodeFixProvider` turns the +declaration into a `using` declaration, with project-wide "Fix all" support. + +### Reference sharing and parallelism + +The harness binds every trusted-platform assembly once per test process and hands the **same** +`MetadataReference` instances to every test compilation, so Roslyn reuses one set of assembly symbols for +the whole run. Creating references per run instead makes each compilation build its own symbol tables: in a +100-compilation micro-benchmark that is the difference between a 156 MB and an 8,146 MB peak. Assemblies +named by `AdditionalAssemblyTypes` that are already in the trusted set are a no-op and are not added again. + +Because the reference set is now shared, a harness-based test project's memory is largely flat in the degree +of parallelism — but the test platform's auto-detected default still costs more memory, and more wall clock, +than an explicit ceiling. Wall clock saturates at around four concurrent tests; `--maximum-parallel-tests 8` +measured 347–365 MB / 7.7–8.7s against 528–881 MB / 12.0–13.8s for auto on a 22-core machine. Set an +explicit ceiling when a project multi-targets and runs several TFM executables concurrently. + Analyzer options are preserved under their supplied keys. Keys without the Roslyn `build_property.` prefix are additionally exposed as compiler-visible MSBuild properties, so either `MyGenerator_Disable` or `build_property.MyGenerator_Disable` can be used in tests. @@ -252,6 +274,19 @@ await Assert.That(result.Runs[1]).StepIsModified("ForAttribute_MyAttribute"); await Assert.That(result.Runs[1]).StepIsCached("GetGenerationConfiguration"); ``` +Each run also carries the framework log entries *that run* wrote, in write order, through +`IncrementalCacheRun.LogEntries` — the sink is registered for the whole sequence and drained between runs, +so entries never accumulate. A rerun whose output step is cached logs nothing, giving a second signal +alongside the step reasons: + +```csharp +await Assert.That(result.Runs[0].LogEntries).IsNotEmpty(); +await Assert.That(result.Runs[1].LogEntries).IsEmpty(); +``` + +`DriverRunResult.LogEntries` (the `RunAsync` path) is likewise in write order. Both are empty when +`EnableLogging` is disabled. + `RunIncrementalAsync(sources, options, ct)` runs the same source set twice (the common "unchanged rerun is cached" case). Per-run MSBuild-property changes use `new IncrementalRunInput(sources, [...])`. Reference cache tests live in the `Purview.SourceGeneratorFramework` source repository — diff --git a/src/src/SourceGeneratorFramework.Testing/SourceGeneratorHelpers.cs b/src/src/SourceGeneratorFramework.Testing/SourceGeneratorHelpers.cs index 3a927b5..7870c58 100644 --- a/src/src/SourceGeneratorFramework.Testing/SourceGeneratorHelpers.cs +++ b/src/src/SourceGeneratorFramework.Testing/SourceGeneratorHelpers.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Immutable; using System.Reflection; using System.Text; @@ -8,6 +9,15 @@ namespace Purview.SourceGeneratorFramework.Testing; static class SourceGeneratorHelpers { + // Process-wide, keyed by assembly path. A MetadataReference owns an AssemblyMetadata, and Roslyn keys + // its PEAssemblySymbol cache off that instance: handing every test compilation the *same* instance lets + // Roslyn reuse one set of symbol tables across the whole run instead of rebuilding them per test. Test + // processes are short lived and the referenced assemblies do not change underneath them, so the entries + // are never invalidated. + static readonly ConcurrentDictionary ReferenceCache = new( + StringComparer.OrdinalIgnoreCase + ); + public static readonly string[] TrustedAssemblies = ResolveTrustedAssemblies(); public static ImmutableArray ResolveTrustedReferences { get; } = @@ -19,6 +29,21 @@ static class SourceGeneratorHelpers GeneratorAssemblyPath ); + static readonly ImmutableHashSet CachedFilteredReferencePaths = CollectDisplayPaths( + CachedFilteredReferences + ); + + /// + /// Gets the shared for an assembly path, creating it on first use. + /// + /// + /// Always go through this instead of calling + /// directly: a fresh reference per test run gives Roslyn a fresh AssemblyMetadata, which defeats + /// symbol reuse and makes every test compilation build its own symbol tables. + /// + public static MetadataReference GetOrCreateReference(string path) => + ReferenceCache.GetOrAdd(path, static p => MetadataReference.CreateFromFile(p)); + public static CSharpCompilation CreateCompilation( IEnumerable syntaxTrees, ImmutableArray references, @@ -52,16 +77,49 @@ Assembly generatorAssembly builder.AddRange(CachedFilteredReferences); + // References are shared instances now, so the same assembly named twice yields the same object. + // Compilation.Create tolerates that, but Solution.AddMetadataReferences (the analyzer, code-fix and + // refactoring path) rejects duplicate items, so the additions are deduplicated here. + HashSet addedPaths = new(StringComparer.OrdinalIgnoreCase); foreach (var type in options.AdditionalAssemblyTypes) - builder.Add(MetadataReference.CreateFromFile(type.Assembly.Location)); + { + var location = type.Assembly.Location; + + // Dynamic and single-file assemblies have no location to reference. + if (string.IsNullOrEmpty(location)) + continue; + + // Already in the trusted-platform set, so naming it again is a no-op. + if (CachedFilteredReferencePaths.Contains(location) || !addedPaths.Add(location)) + continue; + + builder.Add(GetOrCreateReference(location)); + } + + // Deduplicated by instance, not by path: a caller may deliberately supply the same assembly twice + // with different MetadataReferenceProperties (extern aliases), and those are distinct references. + HashSet addedReferences = []; + foreach (var reference in options.AdditionalReferences) + { + if (!addedReferences.Add(reference) || IsAlreadyTrusted(reference)) + continue; - builder.AddRange(options.AdditionalReferences); + builder.Add(reference); + } var references = builder.ToImmutable(); options.PreprocessReferences?.Invoke(references); return references; } + // Only skips a reference that is literally the object already in the trusted list: a caller-created + // reference over the same file is a distinct instance and stays, preserving the previous behaviour. + static bool IsAlreadyTrusted(MetadataReference reference) => + reference.Display is { Length: > 0 } display + && CachedFilteredReferencePaths.Contains(display) + && ReferenceCache.TryGetValue(display, out var cached) + && ReferenceEquals(cached, reference); + public static string PrepareSource(string source, SourceGeneratorTestOptions options) { if (!options.IncludeDefaultNamespaces) @@ -85,7 +143,18 @@ static string[] ResolveTrustedAssemblies() { var trusted = (string?)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") ?? string.Empty; if (!string.IsNullOrWhiteSpace(trusted)) - return trusted.Split([Path.PathSeparator], StringSplitOptions.RemoveEmptyEntries); + { + // The host lists a path once per deps.json library that resolves to it, so an assembly + // referenced twice by design — the generator double-reference pattern, once as an analyzer and + // once as a normal reference — appears twice. Referencing the same file twice is never correct, + // and Solution.AddMetadataReferences rejects it outright once the instances are shared. + return + [ + .. trusted + .Split([Path.PathSeparator], StringSplitOptions.RemoveEmptyEntries) + .Distinct(StringComparer.OrdinalIgnoreCase), + ]; + } // .NET Framework does not populate TRUSTED_PLATFORM_ASSEMBLIES; fall back to the loaded // framework assemblies so test compilations still have their core references. @@ -106,7 +175,19 @@ static ImmutableArray CreateMetadataReferences(string[] paths var builder = ImmutableArray.CreateBuilder(paths.Length); foreach (var path in paths) - builder.Add(MetadataReference.CreateFromFile(path)); + builder.Add(GetOrCreateReference(path)); + + return builder.ToImmutable(); + } + + static ImmutableHashSet CollectDisplayPaths(ImmutableArray references) + { + var builder = ImmutableHashSet.CreateBuilder(StringComparer.OrdinalIgnoreCase); + foreach (var reference in references) + { + if (reference.Display is { Length: > 0 } display) + builder.Add(display); + } return builder.ToImmutable(); } diff --git a/src/src/SourceGeneratorFramework.Testing/SourceGeneratorTestRunner.cs b/src/src/SourceGeneratorFramework.Testing/SourceGeneratorTestRunner.cs index b838745..bbc4b06 100644 --- a/src/src/SourceGeneratorFramework.Testing/SourceGeneratorTestRunner.cs +++ b/src/src/SourceGeneratorFramework.Testing/SourceGeneratorTestRunner.cs @@ -42,7 +42,8 @@ public async Task RunAsync( ); } - ConcurrentBag logEntries = []; + // A queue, not a bag: log entries are only meaningful in the order the generator wrote them. + ConcurrentQueue logEntries = []; if (!options.AdditionalSources.IsDefaultOrEmpty) sources = sources.Concat(options.AdditionalSources); @@ -153,6 +154,12 @@ public IncrementalCacheResult RunIncremental( TGenerator generator = new(); var loggingSessionId = options.EnableLogging ? Guid.NewGuid().ToString("N") : null; + + // The sink has to be registered for the whole sequence: the driver is shared across the runs, and the + // session id baked into it by CreateDriver only resolves to a logger while a sink is registered for it. + ConcurrentQueue logEntries = []; + using var loggingRegistration = ConfigureLogging(loggingSessionId, options, logEntries); + var driver = CreateDriver(generator, options, loggingSessionId); var references = SourceGeneratorHelpers.ResolveReferences(options, typeof(TGenerator).Assembly); var runs = ImmutableArray.CreateBuilder(materializedInputs.Count); @@ -178,12 +185,27 @@ public IncrementalCacheResult RunIncremental( } driver = driver.RunGeneratorsAndUpdateCompilation(compilation, out _, out _, cancellationToken); - runs.Add(CaptureRun(driver)); + + // Drained per run, so each run carries only what that run logged. A stage reported as Cached + // should log nothing, which is what makes this worth capturing separately. + runs.Add(CaptureRun(driver, DrainLogEntries(logEntries))); } return new(runs.ToImmutable()); } + static ImmutableArray DrainLogEntries(ConcurrentQueue logEntries) + { + if (logEntries.IsEmpty) + return []; + + var builder = ImmutableArray.CreateBuilder(logEntries.Count); + while (logEntries.TryDequeue(out var entry)) + builder.Add(entry); + + return builder.ToImmutable(); + } + static CSharpCompilation BuildCompilation( IEnumerable sources, SourceGeneratorTestOptions options, @@ -208,7 +230,7 @@ CancellationToken cancellationToken return SourceGeneratorHelpers.CreateCompilation(syntaxTrees, references, options); } - static IncrementalCacheRun CaptureRun(GeneratorDriver driver) + static IncrementalCacheRun CaptureRun(GeneratorDriver driver, ImmutableArray logEntries) { var driverRunResult = driver.GetRunResult(); var runResult = driverRunResult.Results.FirstOrDefault(run => run.Generator is not null); @@ -221,7 +243,7 @@ static IncrementalCacheRun CaptureRun(GeneratorDriver driver) steps[pair.Key] = pair.Value; } - return new(runResult, steps.ToImmutable()); + return new(runResult, steps.ToImmutable()) { LogEntries = logEntries }; } static async Task GetAnalyzerResultsAsync( @@ -351,7 +373,7 @@ static Dictionary BuildAnalyzerConfig( static LoggingRegistrations? ConfigureLogging( string? loggingSessionId, SourceGeneratorTestOptions options, - ConcurrentBag logEntries + ConcurrentQueue logEntries ) { if (loggingSessionId is null) @@ -362,7 +384,7 @@ void Sink(string message, int level) var type = (SourceGenLogLevel)level; options.TestOutput.WriteLine($"[{type}] {message}"); - logEntries.Add(new(type, message)); + logEntries.Enqueue(new(type, message)); } List registrations = [SourceGenLogging.RegisterSinkCore(loggingSessionId, Sink)]; diff --git a/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/AttributeDataExtensions.cs b/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/AttributeDataExtensions.cs index 9f051df..d91aa80 100644 --- a/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/AttributeDataExtensions.cs +++ b/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/AttributeDataExtensions.cs @@ -12,6 +12,11 @@ namespace Microsoft.CodeAnalysis; /// [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2208:Instantiate argument exceptions correctly")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1708:Identifiers should differ by more than case")] +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static partial class AttributeDataExtensions { diff --git a/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/TypedConstantExtensions.cs b/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/TypedConstantExtensions.cs index a5904a5..b7e1779 100644 --- a/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/TypedConstantExtensions.cs +++ b/src/src/SourceGeneratorFramework/Extensions/Microsoft/CodeAnalysis/TypedConstantExtensions.cs @@ -10,6 +10,11 @@ namespace Microsoft.CodeAnalysis; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static partial class TypedConstantExtensions { diff --git a/src/src/SourceGeneratorFramework/Extensions/System/StringExtensions.cs b/src/src/SourceGeneratorFramework/Extensions/System/StringExtensions.cs index 9a3d345..d695374 100644 --- a/src/src/SourceGeneratorFramework/Extensions/System/StringExtensions.cs +++ b/src/src/SourceGeneratorFramework/Extensions/System/StringExtensions.cs @@ -4,6 +4,11 @@ namespace System; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class StringExtensions { diff --git a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/CSharp/Syntax/SyntaxNodeExtensions.cs b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/CSharp/Syntax/SyntaxNodeExtensions.cs index 4014b32..25d1411 100644 --- a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/CSharp/Syntax/SyntaxNodeExtensions.cs +++ b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/CSharp/Syntax/SyntaxNodeExtensions.cs @@ -3,6 +3,11 @@ namespace Microsoft.CodeAnalysis.CSharp.Syntax; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class SyntaxNodeExtensions { diff --git a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/ISymbolExtensions.cs b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/ISymbolExtensions.cs index 39dd649..cdecd74 100644 --- a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/ISymbolExtensions.cs +++ b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/ISymbolExtensions.cs @@ -3,6 +3,11 @@ namespace Microsoft.CodeAnalysis; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class ISymbolExtensions { diff --git a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/IncrementalGeneratorInitializationContextExtensions.cs b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/IncrementalGeneratorInitializationContextExtensions.cs index 84ef71e..761dae5 100644 --- a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/IncrementalGeneratorInitializationContextExtensions.cs +++ b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/IncrementalGeneratorInitializationContextExtensions.cs @@ -5,6 +5,11 @@ namespace Microsoft.CodeAnalysis; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class IncrementalGeneratorInitializationContextExtensions { diff --git a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/SourceProductionContextExtensions.cs b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/SourceProductionContextExtensions.cs index a62b315..310d993 100644 --- a/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/SourceProductionContextExtensions.cs +++ b/src/src/SourceGeneratorShared/Extensions/Microsoft/CodeAnalysis/SourceProductionContextExtensions.cs @@ -3,6 +3,11 @@ namespace Microsoft.CodeAnalysis; +[System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "The extension(...) block is lowered to an unnamed nested type; the language requires it to be nested in a static class, and PSGFR34 mandates extension blocks over classic 'this'-parameter methods." +)] [EditorBrowsable(EditorBrowsableState.Never)] public static class SourceProductionContextExtensions { diff --git a/src/src/SourceGeneratorShared/GenerationContext.cs b/src/src/SourceGeneratorShared/GenerationContext.cs index 4db30d3..2a74462 100644 --- a/src/src/SourceGeneratorShared/GenerationContext.cs +++ b/src/src/SourceGeneratorShared/GenerationContext.cs @@ -10,27 +10,27 @@ namespace Purview.SourceGeneratorFramework; /// Initializes a generation context. /// public sealed class GenerationContext( - TCapabilities Capabilities, - GenerationSettings Settings, - ISourceGenLogger? Logger = null + TCapabilities capabilities, + GenerationSettings settings, + ISourceGenLogger? logger = null ) : ISourceGenLogger, IEqualityComparer>, IEquatable> where TCapabilities : class, IGenerationCapabilities { /// /// Gets the capabilities of the source generator, which are used to determine what features are available during generation. /// - public TCapabilities Capabilities { get; } = Capabilities ?? throw new ArgumentNullException(nameof(Capabilities)); + public TCapabilities Capabilities { get; } = capabilities ?? throw new ArgumentNullException(nameof(capabilities)); /// /// Gets the immutable generation settings. /// - public GenerationSettings Settings { get; } = Settings ?? throw new ArgumentNullException(nameof(Settings)); + public GenerationSettings Settings { get; } = settings ?? throw new ArgumentNullException(nameof(settings)); /// /// Gets the optional logger. This is primarily used during source generator test execution, such as /// unit or integration tests, to capture and assert on log messages generated by the source generator. /// - public ISourceGenLogger? Logger { get; } = Logger; + public ISourceGenLogger? Logger { get; } = logger; /// /// Creates a new independently owned code writer. diff --git a/src/src/SourceGeneratorShared/TypeIdentity.cs b/src/src/SourceGeneratorShared/TypeIdentity.cs index c77ab8f..5fedb56 100644 --- a/src/src/SourceGeneratorShared/TypeIdentity.cs +++ b/src/src/SourceGeneratorShared/TypeIdentity.cs @@ -1,4 +1,3 @@ -using System.Collections.Concurrent; using System.Collections.Immutable; using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; @@ -140,30 +139,21 @@ public TypeIdentity(ITypeSymbol typeSymbol) ); } - var assembly = namedType.ContainingAssembly; - if (assembly is not null) - { - var cache = AssemblySymbolCache.GetValue(assembly, AssemblyCacheFactory); - this = cache.GetOrAdd(typeSymbol, SymbolCacheFactory).Value; - } - else - { - this = CreateFromSymbol(typeSymbol).Value; - } + this = SymbolCache.GetValue(namedType, SymbolCacheFactory).Value; } - static readonly ConditionalWeakTable< - IAssemblySymbol, - ConcurrentDictionary - > AssemblySymbolCache = new(); - static readonly ConditionalWeakTable< - IAssemblySymbol, - ConcurrentDictionary - >.CreateValueCallback AssemblyCacheFactory = static assembly => new ConcurrentDictionary< - ITypeSymbol, - TypeIdentityCacheEntry - >(TypeSymbolEqualityComparer.Instance); - static readonly Func SymbolCacheFactory = CreateFromSymbol; + // Keyed weakly on the symbol itself, so a cached identity never outlives the symbol it was built from. + // + // The previous shape — a per-assembly ConcurrentDictionary keyed by ITypeSymbol — leaked: a constructed + // generic such as List reports its *definition's* assembly as the containing assembly, so the + // entry landed in the dictionary hanging off a long-lived metadata assembly (corlib) while the key + // strongly referenced a source symbol. That pinned the whole source compilation for the lifetime of the + // corlib assembly symbol — the entire compiler host session in an IDE, and the entire run in a test + // process now that metadata references (and therefore their assembly symbols) are shared across + // compilations. + static readonly ConditionalWeakTable SymbolCache = new(); + static readonly ConditionalWeakTable.CreateValueCallback SymbolCacheFactory = + CreateFromSymbol; static TypeIdentityCacheEntry CreateFromSymbol(ITypeSymbol typeSymbol) { @@ -194,18 +184,9 @@ static TypeIdentityCacheEntry CreateFromSymbol(ITypeSymbol typeSymbol) ); } - sealed class TypeSymbolEqualityComparer : IEqualityComparer - { - public static readonly TypeSymbolEqualityComparer Instance = new(); - - public bool Equals(ITypeSymbol? x, ITypeSymbol? y) => SymbolEqualityComparer.Default.Equals(x, y); - - public int GetHashCode(ITypeSymbol obj) => SymbolEqualityComparer.Default.GetHashCode(obj); - } - sealed class TypeIdentityCacheEntry(TypeIdentity value) { - public TypeIdentity Value = value; + public TypeIdentity Value { get; } = value; } /// diff --git a/src/tests/SourceGeneratorFramework.Analyzers.UnitTests/UndisposedTestResultAnalyzerTests.cs b/src/tests/SourceGeneratorFramework.Analyzers.UnitTests/UndisposedTestResultAnalyzerTests.cs new file mode 100644 index 0000000..9980efa --- /dev/null +++ b/src/tests/SourceGeneratorFramework.Analyzers.UnitTests/UndisposedTestResultAnalyzerTests.cs @@ -0,0 +1,259 @@ +using Microsoft.CodeAnalysis; +using Purview.SourceGeneratorFramework.Testing; +using Purview.SourceGeneratorFramework.Testing.TUnit; + +namespace Purview.SourceGeneratorFramework.Analyzers; + +public sealed class UndisposedTestResultAnalyzerTests : TUnitDiagnosticAnalyzerTestBase +{ + // A stand-in for the runner surface: the analyzer keys off the produced type, not the method name, so a + // consumer's own wrapper is covered in exactly the same way the shipped runners are. + const string Harness = """ + using Purview.SourceGeneratorFramework; + using Purview.SourceGeneratorFramework.Testing; + + static class Runner + { + public static DriverRunResult Run(SourceGeneratorTestOptions options) => null!; + } + """; + + static AnalyzerTestOptions Options => + new() + { + AdditionalAssemblyTypes = [typeof(DriverRunResult), typeof(SourceGeneratorTestOptions)], + IncludeDefaultNamespaces = false, + }; + + [Test] + public async Task UsingDeclaration_DoesNotReportDiagnostic(CancellationToken cancellationToken) + { + // Arrange + const string source = + Harness + + """ + + class Tests + { + public void Run() + { + using var result = Runner.Run(new SourceGeneratorTestOptions().Compile()); + System.Console.WriteLine(result.GetSource()); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasNoDiagnostics(); + } + + [Test] + public async Task PlainLocal_ReportsDiagnostic(CancellationToken cancellationToken) + { + // Arrange + const string source = + Harness + + """ + + class Tests + { + public void Run() + { + var result = Runner.Run(new SourceGeneratorTestOptions().Compile()); + System.Console.WriteLine(result.GetSource()); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasDiagnostics(1); + await Assert.That(result).HasDiagnostic(UndisposedTestResultAnalyzer.DiagnosticId); + } + + [Test] + public async Task ReturnedLocal_DoesNotReportDiagnostic(CancellationToken cancellationToken) + { + // Arrange - the caller becomes the owner, so the helper must not dispose it. + const string source = + Harness + + """ + + class Tests + { + public DriverRunResult Build() + { + var result = Runner.Run(new SourceGeneratorTestOptions().Compile()); + return result; + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasNoDiagnostics(); + } + + [Test] + public async Task HelperReturningAProjection_ReportsDiagnostic(CancellationToken cancellationToken) + { + // Arrange - the result itself never leaves the helper, so the helper owns it. + const string source = + Harness + + """ + + class Tests + { + public string BuildDefinition() + { + var result = Runner.Run(new SourceGeneratorTestOptions().Compile()); + return result.GetSource(); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasDiagnostics(1); + await Assert.That(result).HasDiagnostic(UndisposedTestResultAnalyzer.DiagnosticId); + } + + [Test] + public async Task ExplicitDispose_DoesNotReportDiagnostic(CancellationToken cancellationToken) + { + // Arrange + const string source = + Harness + + """ + + class Tests + { + public void Run() + { + var result = Runner.Run(new SourceGeneratorTestOptions().Compile()); + System.Console.WriteLine(result.GetSource()); + result.Dispose(); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasNoDiagnostics(); + } + + [Test] + public async Task DiscardedResult_ReportsDiagnostic(CancellationToken cancellationToken) + { + // Arrange + const string source = + Harness + + """ + + class Tests + { + public void Run() + { + _ = Runner.Run(new SourceGeneratorTestOptions().Compile()); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasDiagnostics(1); + await Assert.That(result).HasDiagnostic(UndisposedTestResultAnalyzer.DiagnosticId); + } + + [Test] + public async Task CompileToAssemblyRun_ReportsAsWarning(CancellationToken cancellationToken) + { + // Arrange - an emitted assembly is the case that actually costs memory. + const string source = + Harness + + """ + + class Tests + { + public void Run() + { + var result = Runner.Run(new SourceGeneratorTestOptions { CompileToAssembly = true }); + System.Console.WriteLine(result.GetSource()); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result.Diagnostics.Single().Severity).IsEqualTo(DiagnosticSeverity.Warning); + } + + [Test] + public async Task RunWithoutCompile_ReportsAsInformation(CancellationToken cancellationToken) + { + // Arrange - nothing was emitted, so the leak is cheap and the rule stays informational. + const string source = + Harness + + """ + + class Tests + { + public void Run() + { + var result = Runner.Run(new SourceGeneratorTestOptions()); + System.Console.WriteLine(result.GetSource()); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result.Diagnostics.Single().Severity).IsEqualTo(DiagnosticSeverity.Info); + } + + [Test] + public async Task NonHarnessDisposable_DoesNotReportDiagnostic(CancellationToken cancellationToken) + { + // Arrange - the rule is scoped to the Testing namespace, not to disposables in general. + const string source = """ + class Thing : System.IDisposable + { + public void Dispose() { } + + public static Thing Make() => new Thing(); + } + + class Tests + { + public void Run() + { + var thing = Thing.Make(); + System.Console.WriteLine(thing.ToString()); + } + } + """; + + // Act + var result = await AnalyzeAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasNoDiagnostics(); + } +} diff --git a/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/DisposeTestResultCodeFixProviderTests.cs b/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/DisposeTestResultCodeFixProviderTests.cs new file mode 100644 index 0000000..d0fd032 --- /dev/null +++ b/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/DisposeTestResultCodeFixProviderTests.cs @@ -0,0 +1,80 @@ +using Purview.SourceGeneratorFramework.Analyzers; +using Purview.SourceGeneratorFramework.Testing; +using Purview.SourceGeneratorFramework.Testing.TUnit; + +namespace Purview.SourceGeneratorFramework.CodeFixers; + +public sealed class DisposeTestResultCodeFixProviderTests + : TUnitCodeFixTestBase +{ + const string Harness = """ + using Purview.SourceGeneratorFramework; + using Purview.SourceGeneratorFramework.Testing; + + static class Runner + { + public static DriverRunResult Run(SourceGeneratorTestOptions options) => null!; + } + """; + + static CodeFixTestOptions Options => + new() + { + EquivalenceKey = DisposeTestResultCodeFixProvider.EquivalenceKey, + AdditionalAssemblyTypes = [typeof(DriverRunResult), typeof(SourceGeneratorTestOptions)], + IncludeDefaultNamespaces = false, + }; + + [Test] + public async Task UndisposedResult_AddsUsingDeclaration(CancellationToken cancellationToken) + { + // Arrange + const string source = + Harness + + """ + + class Tests + { + public void Run() + { + var result = Runner.Run(new SourceGeneratorTestOptions().Compile()); + System.Console.WriteLine(result.GetSource()); + } + } + """; + + // Act + var result = await ApplyCodeFixAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result).HasDiagnostic(UndisposedTestResultAnalyzer.DiagnosticId); + await Assert + .That(result.FixedSource) + .Contains("using var result = Runner.Run(new SourceGeneratorTestOptions().Compile());"); + } + + [Test] + public async Task UndisposedResultInHelper_AddsUsingDeclaration(CancellationToken cancellationToken) + { + // Arrange - the helper projects a value out of the result and leaks the result itself. + const string source = + Harness + + """ + + class Tests + { + public string BuildDefinition() + { + var result = Runner.Run(new SourceGeneratorTestOptions().Compile()); + return result.GetSource(); + } + } + """; + + // Act + var result = await ApplyCodeFixAsync(source, Options, cancellationToken); + + // Assert + await Assert.That(result.FixedSource).Contains("using var result = Runner.Run("); + } +} diff --git a/src/tests/SourceGeneratorFramework.ExampleGenerator.UnitTests/ServiceRegistrationCacheTests.cs b/src/tests/SourceGeneratorFramework.ExampleGenerator.UnitTests/ServiceRegistrationCacheTests.cs index 787fb7d..3211ccd 100644 --- a/src/tests/SourceGeneratorFramework.ExampleGenerator.UnitTests/ServiceRegistrationCacheTests.cs +++ b/src/tests/SourceGeneratorFramework.ExampleGenerator.UnitTests/ServiceRegistrationCacheTests.cs @@ -81,4 +81,40 @@ public class OtherService { } await Assert.That(result.Runs[1]).StepIsModified("ForAttribute_GenerateServiceAttribute"); await Assert.That(result.Runs[1]).StepIsCached("GetMSBuildPropertyValue_EmitServiceRegistrationInfo"); } + + [Test] + public async Task GenerateIncrementalAsync_CapturesFrameworkLoggingPerRun(CancellationToken cancellationToken) + { + // Arrange / Act - two runs over a single shared driver. + var result = await GenerateIncrementalAsync([Source], cancellationToken: cancellationToken); + + // Assert - the first run executes the pipeline and logs; the entries belong to that run alone. + await Assert.That(result.Runs[0].LogEntries).IsNotEmpty(); + await Assert + .That( + result + .Runs[0] + .LogEntries.Any(entry => entry.Message.Contains("generation context", StringComparison.Ordinal)) + ) + .IsTrue(); + + // Each run carries only its own entries. If they were accumulated across the sequence instead of + // drained per run, the second run would also hold everything the first one logged. + await Assert.That(result.Runs[1].LogEntries.Length).IsLessThanOrEqualTo(result.Runs[0].LogEntries.Length); + } + + [Test] + public async Task GenerateIncrementalAsync_GivenLoggingDisabled_CapturesNothing(CancellationToken cancellationToken) + { + // Arrange / Act + var result = await GenerateIncrementalAsync( + [Source], + new ServiceRegistrationTestOptions { EnableLogging = false }, + cancellationToken + ); + + // Assert + await Assert.That(result.Runs[0].LogEntries).IsEmpty(); + await Assert.That(result.Runs[1].LogEntries).IsEmpty(); + } } diff --git a/src/tests/SourceGeneratorFramework.UnitTests/CodeQueryTests.cs b/src/tests/SourceGeneratorFramework.UnitTests/CodeQueryTests.cs index dda4271..187f6b2 100644 --- a/src/tests/SourceGeneratorFramework.UnitTests/CodeQueryTests.cs +++ b/src/tests/SourceGeneratorFramework.UnitTests/CodeQueryTests.cs @@ -332,6 +332,41 @@ await Assert .IsEqualTo(1); } + [Test] + public async Task ScopedQuery_GivenMultipleTrees_DoesNotRepeatMatchesPerTree() + { + // Arrange - a scope is a single node, so the number of trees in the query must not affect the result. + const string first = """ + namespace Test; + + public sealed class Holder + { + public void One() { } + public void Two() { } + } + """; + const string second = """ + namespace Test; + + public sealed class Other + { + public void Three() { } + } + """; + + var (tree, _) = TestCompilation.Parse(second); + var compilation = TestCompilation.Create(first).AddSyntaxTrees(tree); + CodeQuery query = new([.. compilation.SyntaxTrees], compilation); + + // Act + var scoped = query.GetClass("Holder").Query; + + // Assert + await Assert.That(query.Trees.Length).IsEqualTo(2); + await Assert.That(scoped.Count()).IsEqualTo(2); + await Assert.That(scoped.GetAll().Length).IsEqualTo(2); + } + [Test] public async Task OperatorAndIndexerAndAttributeQueries_FindDeclarations() { diff --git a/src/tests/SourceGeneratorFramework.UnitTests/CompilationResultTests.cs b/src/tests/SourceGeneratorFramework.UnitTests/CompilationResultTests.cs index 9c5de5b..2d1b214 100644 --- a/src/tests/SourceGeneratorFramework.UnitTests/CompilationResultTests.cs +++ b/src/tests/SourceGeneratorFramework.UnitTests/CompilationResultTests.cs @@ -31,7 +31,7 @@ public async Task CompileToAssembly_ExposesLoadedAssembly_ThatCanExecute(Cancell SourceGeneratorTestRunner runner = new(); SourceGeneratorTestOptions options = new() { CompileToAssembly = true }; - var result = await runner.RunAsync("public sealed class Input { }", options, cancellationToken); + using var result = await runner.RunAsync("public sealed class Input { }", options, cancellationToken); await Assert.That(result.CompilationResult.Assembly).IsNotNull(); @@ -50,7 +50,7 @@ public async Task CompileToAssembly_LoadsIntoCollectibleContext(CancellationToke SourceGeneratorTestRunner runner = new(); SourceGeneratorTestOptions options = new() { CompileToAssembly = true }; - var result = await runner.RunAsync("public sealed class Input { }", options, cancellationToken); + using var result = await runner.RunAsync("public sealed class Input { }", options, cancellationToken); var context = AssemblyLoadContext.GetLoadContext(result.CompilationResult.Assembly!); @@ -76,7 +76,7 @@ public async Task Metadata_ReflectsGeneratedType_WithoutExecution(CancellationTo SourceGeneratorTestRunner runner = new(); SourceGeneratorTestOptions options = new() { CompileToAssembly = true }; - var result = await runner.RunAsync("public sealed class Input { }", options, cancellationToken); + using var result = await runner.RunAsync("public sealed class Input { }", options, cancellationToken); await Assert.That(result.CompilationResult.Metadata).IsNotNull(); await Assert.That(result.CompilationResult.MetadataAssembly).IsNotNull(); diff --git a/src/tests/SourceGeneratorFramework.UnitTests/MetadataReferenceCacheTests.cs b/src/tests/SourceGeneratorFramework.UnitTests/MetadataReferenceCacheTests.cs new file mode 100644 index 0000000..c22ee99 --- /dev/null +++ b/src/tests/SourceGeneratorFramework.UnitTests/MetadataReferenceCacheTests.cs @@ -0,0 +1,136 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; + +namespace Purview.SourceGeneratorFramework; + +/// +/// Locks in the reference-sharing contract of SourceGeneratorHelpers. +/// +/// +/// Each owns an AssemblyMetadata, and Roslyn keys its assembly-symbol +/// reuse off that instance. Creating references per run makes every test compilation build its own symbol +/// tables, which is what made harness-based suites peak in the gigabytes. These tests fail if a refactor +/// reintroduces a per-call MetadataReference.CreateFromFile. +/// +public class MetadataReferenceCacheTests +{ + [Test] + public async Task GetOrCreateReference_GivenTheSamePath_ReturnsTheSameInstance() + { + // Arrange + var path = typeof(object).Assembly.Location; + + // Act + var first = SourceGeneratorHelpers.GetOrCreateReference(path); + var second = SourceGeneratorHelpers.GetOrCreateReference(path); + + // Assert + await Assert.That(second).IsSameReferenceAs(first); + } + + [Test] + public async Task ResolveTrustedReferences_GivenATrustedPath_SharesTheCachedInstance() + { + // Arrange + var path = typeof(object).Assembly.Location; + var cached = SourceGeneratorHelpers.GetOrCreateReference(path); + + // Act + var trusted = SourceGeneratorHelpers.ResolveTrustedReferences.FirstOrDefault(reference => + string.Equals(reference.Display, path, StringComparison.OrdinalIgnoreCase) + ); + + // Assert + await Assert.That(trusted).IsSameReferenceAs(cached); + } + + [Test] + public async Task ResolveReferences_GivenAdditionalAssemblyTypes_ReusesEveryReferenceAcrossRuns() + { + // Arrange - the slow path, taken whenever any additional assembly is configured. + SourceGeneratorTestOptions options = new() + { + AdditionalAssemblyTypes = [typeof(Compilation), typeof(SourceGeneratorTestOptions)], + }; + + // Act + var first = SourceGeneratorHelpers.ResolveReferences(options, typeof(Compilation).Assembly); + var second = SourceGeneratorHelpers.ResolveReferences(options, typeof(Compilation).Assembly); + + // Assert - not merely equal: the same objects, so Roslyn can reuse the bound symbols. + await Assert.That(second.Length).IsEqualTo(first.Length); + await Assert.That(first.Where((reference, index) => !ReferenceEquals(reference, second[index]))).IsEmpty(); + } + + [Test] + public async Task ResolveReferences_GivenAnAssemblyAlreadyTrusted_DoesNotAddItAgain() + { + // Arrange + SourceGeneratorTestOptions options = new() { AdditionalAssemblyTypes = [typeof(object)] }; + + // Act + var references = SourceGeneratorHelpers.ResolveReferences(options, typeof(Compilation).Assembly); + + // Assert + await Assert + .That( + references.Count(reference => + string.Equals( + reference.Display, + typeof(object).Assembly.Location, + StringComparison.OrdinalIgnoreCase + ) + ) + ) + .IsEqualTo(1); + } + + [Test] + public async Task ResolveReferences_GivenRepeatedAssemblyTypes_ProducesNoDuplicateInstances() + { + // Arrange - Solution.AddMetadataReferences rejects duplicate items outright. + SourceGeneratorTestOptions options = new() + { + AdditionalAssemblyTypes = [typeof(SourceGeneratorTestOptions), typeof(DriverRunResult)], + AdditionalReferences = [SourceGeneratorHelpers.GetOrCreateReference(typeof(Compilation).Assembly.Location)], + }; + + // Act + var references = SourceGeneratorHelpers.ResolveReferences(options, typeof(Compilation).Assembly); + + // Assert + await Assert.That(references.Distinct().Count()).IsEqualTo(references.Length); + } + + [Test] + public async Task ResolveTrustedAssemblies_ContainsNoDuplicatePaths() + { + // Act + var duplicates = SourceGeneratorHelpers + .TrustedAssemblies.GroupBy(static path => path, StringComparer.OrdinalIgnoreCase) + .Where(static group => group.Count() > 1) + .Select(static group => group.Key); + + // Assert + await Assert.That(duplicates).IsEmpty(); + } + + [Test] + public async Task ResolveReferences_GivenAdditionalAssemblies_StillInvokesPreprocessReferences() + { + // Arrange + ImmutableArray observed = default; + SourceGeneratorTestOptions options = new() + { + AdditionalAssemblyTypes = [typeof(SourceGeneratorTestOptions)], + PreprocessReferences = references => observed = references, + }; + + // Act + var resolved = SourceGeneratorHelpers.ResolveReferences(options, typeof(Compilation).Assembly); + + // Assert + await Assert.That(observed.IsDefault).IsFalse(); + await Assert.That(observed.SequenceEqual(resolved)).IsTrue(); + } +} diff --git a/src/tests/SourceGeneratorShared.UnitTests/GeneratorLoggingTests.cs b/src/tests/SourceGeneratorShared.UnitTests/GeneratorLoggingTests.cs new file mode 100644 index 0000000..388c993 --- /dev/null +++ b/src/tests/SourceGeneratorShared.UnitTests/GeneratorLoggingTests.cs @@ -0,0 +1,90 @@ +using Purview.SourceGeneratorFramework.TestGenerators; +using Purview.SourceGeneratorFramework.Testing.Models; + +namespace Purview.SourceGeneratorFramework; + +/// +/// Covers how the runners surface framework log entries: in order, scoped to the run that wrote them, and +/// only when logging is enabled. +/// +public class GeneratorLoggingTests +{ + const string Source = "public class Anything { }"; + + // The framework pipeline logs entries of its own around the generator's, so the generator's messages are + // asserted as an ordered subsequence rather than as the whole set. + static async Task AssertWrittenInOrderAsync(IEnumerable entries) + { + var messages = entries.Select(static entry => entry.Message).ToList(); + var positions = OrderedLoggingTestGenerator.Messages.Select(message => messages.IndexOf(message)).ToArray(); + + await Assert.That(positions).DoesNotContain(-1); + await Assert.That(positions.SequenceEqual(positions.Order())).IsTrue(); + } + + [Test] + public async Task RunAsync_PreservesTheOrderEntriesWereWritten(CancellationToken cancellationToken) + { + // Arrange + SourceGeneratorTestRunner runner = new(); + + // Act + using var result = await runner.RunAsync(Source, cancellationToken: cancellationToken); + + // Assert + await AssertWrittenInOrderAsync(result.LogEntries); + } + + [Test] + public async Task RunAsync_GivenLoggingDisabled_CapturesNothing(CancellationToken cancellationToken) + { + // Arrange + SourceGeneratorTestRunner runner = new(); + + // Act + using var result = await runner.RunAsync( + Source, + new SourceGeneratorTestOptions { EnableLogging = false }, + cancellationToken + ); + + // Assert + await Assert.That(result.LogEntries).IsEmpty(); + } + + [Test] + public async Task RunIncrementalAsync_ScopesEntriesToTheRunThatWroteThem(CancellationToken cancellationToken) + { + // Arrange - the incremental runner shares one driver across the sequence, so the sink has to be + // registered for the whole sequence and drained between runs. + SourceGeneratorTestRunner runner = new(); + + // Act + var result = await runner.RunIncrementalAsync([Source], cancellationToken: cancellationToken); + + // Assert - the first run executes the output step and writes its entries in order. + await AssertWrittenInOrderAsync(result.Runs[0].LogEntries); + + // The rerun reuses the cached output, so it writes nothing. Were the entries accumulated across the + // sequence rather than drained per run, this would still hold the first run's. + await Assert.That(result.Runs[1].LogEntries).IsEmpty(); + } + + [Test] + public async Task RunIncrementalAsync_GivenLoggingDisabled_CapturesNothing(CancellationToken cancellationToken) + { + // Arrange + SourceGeneratorTestRunner runner = new(); + + // Act + var result = await runner.RunIncrementalAsync( + [Source], + new SourceGeneratorTestOptions { EnableLogging = false }, + cancellationToken + ); + + // Assert + foreach (var run in result.Runs) + await Assert.That(run.LogEntries).IsEmpty(); + } +} diff --git a/src/tests/SourceGeneratorShared.UnitTests/Helpers/TypeHelpersTests.cs b/src/tests/SourceGeneratorShared.UnitTests/Helpers/TypeHelpersTests.cs index 913ae29..0c643d0 100644 --- a/src/tests/SourceGeneratorShared.UnitTests/Helpers/TypeHelpersTests.cs +++ b/src/tests/SourceGeneratorShared.UnitTests/Helpers/TypeHelpersTests.cs @@ -91,14 +91,14 @@ CancellationToken cancellationToken // Arrange const string source = "namespace Testing { interface IResource { } class DefaultAspireResource : IResource { } class ResourceKitBase where T : IResource { } class HostKit : ResourceKitBase { } }"; - var (Symbol, Syntax) = GetTypeDescriptor(source, "HostKit", cancellationToken); + var (symbol, syntax) = GetTypeDescriptor(source, "HostKit", cancellationToken); var expectedBase = new TypeIdentity("ResourceKitBase", "Testing").MakeGeneric( new TypeIdentity("IResource", "Testing") ); // Act - var symbolResult = TypeHelpers.IsDerivedFromExpectedBase(Symbol, expectedBase); - var syntaxResult = TypeHelpers.IsDerivedFromExpectedBase(Syntax, expectedBase); + var symbolResult = TypeHelpers.IsDerivedFromExpectedBase(symbol, expectedBase); + var syntaxResult = TypeHelpers.IsDerivedFromExpectedBase(syntax, expectedBase); // Assert await Assert.That(symbolResult).IsTrue(); @@ -113,12 +113,12 @@ CancellationToken cancellationToken // Arrange const string source = "namespace Testing { interface IResource { } class DefaultAspireResource : IResource { } class ResourceKitBase where T : IResource { } class HostKit : ResourceKitBase { } }"; - var (Symbol, Syntax) = GetTypeDescriptor(source, "HostKit", cancellationToken); + var (symbol, syntax) = GetTypeDescriptor(source, "HostKit", cancellationToken); TypeIdentity expectedBase = new("ResourceKitBase", "Testing"); // Act - var symbolResult = TypeHelpers.IsDerivedFromExpectedBase(Symbol, expectedBase); - var syntaxResult = TypeHelpers.IsDerivedFromExpectedBase(Syntax, expectedBase); + var symbolResult = TypeHelpers.IsDerivedFromExpectedBase(symbol, expectedBase); + var syntaxResult = TypeHelpers.IsDerivedFromExpectedBase(syntax, expectedBase); // Assert await Assert.That(symbolResult).IsTrue(); diff --git a/src/tests/SourceGeneratorShared.UnitTests/TestGenerators/OrderedLoggingTestGenerator.cs b/src/tests/SourceGeneratorShared.UnitTests/TestGenerators/OrderedLoggingTestGenerator.cs new file mode 100644 index 0000000..ff5e5c2 --- /dev/null +++ b/src/tests/SourceGeneratorShared.UnitTests/TestGenerators/OrderedLoggingTestGenerator.cs @@ -0,0 +1,36 @@ +using Microsoft.CodeAnalysis; +using Purview.SourceGeneratorFramework.Helpers; +using Purview.SourceGeneratorFramework.Logging; + +namespace Purview.SourceGeneratorFramework.TestGenerators; + +/// +/// Writes a known sequence of log entries so a test can prove the runner surfaces them in the order the +/// generator wrote them. +/// +public sealed class OrderedLoggingTestGenerator : IIncrementalGenerator +{ + /// + /// The messages written, in order, on every run. + /// + public static readonly string[] Messages = ["first entry", "second entry", "third entry"]; + + public void Initialize(IncrementalGeneratorInitializationContext context) + { + var generationContext = IncrementalPipeline.DefaultGenerationContextValueProvider( + context, + new GenerationSettings("OrderedLoggingTestGenerator", "1.0.0") + ); + + context.RegisterSourceOutput( + generationContext, + static (spc, value) => + { + foreach (var message in Messages) + value.Log(SourceGenLogLevel.Info, 0, message); + + spc.AddSource("OrderedLogging.g.cs", "// OrderedLoggingTestGenerator"); + } + ); + } +} diff --git a/src/tests/SourceGeneratorShared.UnitTests/TypeIdentityTests.cs b/src/tests/SourceGeneratorShared.UnitTests/TypeIdentityTests.cs index 318af79..51d9eb5 100644 --- a/src/tests/SourceGeneratorShared.UnitTests/TypeIdentityTests.cs +++ b/src/tests/SourceGeneratorShared.UnitTests/TypeIdentityTests.cs @@ -1,3 +1,4 @@ +using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; namespace Purview.SourceGeneratorFramework; @@ -590,4 +591,90 @@ public async Task EqualityWithNullAndEmpty_ConstructedGeneric_DoesNotOverflow() await Assert.That(resourceKitBase == TypeIdentity.Null).IsFalse(); await Assert.That(resourceKitBase == TypeIdentity.Empty).IsFalse(); } + + // --------------------------------------------------------------------------------------------- + // Regression: the symbol cache must not conflate types that produce different identities + // --------------------------------------------------------------------------------------------- + + [Test] + public async Task Create_GivenGenericArgumentsDifferingOnlyByNullability_DoesNotShareACachedIdentity() + { + // Arrange - a TypeIdentity records its type arguments as TypeReference values, and those carry the + // nullable reference annotation. SymbolEqualityComparer.Default ignores that annotation, so keying the + // cache with it made List and List collide: whichever was built first won, for every + // later compilation in the process. + var compilation = TestCompilation.Create(); + var list = compilation.GetTypeByMetadataName("System.Collections.Generic.List`1")!; + var @string = compilation.GetSpecialType(SpecialType.System_String); + + var ofNullable = list.Construct(@string.WithNullableAnnotation(NullableAnnotation.Annotated)); + var ofNonNullable = list.Construct(@string.WithNullableAnnotation(NullableAnnotation.NotAnnotated)); + + // Act - nullable first, so a collision would hand the annotated identity back for the second. + TypeIdentity nullable = new(ofNullable); + TypeIdentity nonNullable = new(ofNonNullable); + + // Assert + await Assert.That(nullable.TypeArguments[0].IsNullable).IsTrue(); + await Assert.That(nonNullable.TypeArguments[0].IsNullable).IsFalse(); + } + + // --------------------------------------------------------------------------------------------- + // Regression: the symbol cache must not outlive the symbols it is built from + // --------------------------------------------------------------------------------------------- + + [Test] + public async Task Create_GivenConstructedGenericOverSourceType_DoesNotRetainTheCompilation() + { + // Arrange - an anchor compilation stands in for the rest of the run: because the harness shares its + // MetadataReference instances, every test compilation binds the *same* corlib assembly symbol, and + // that symbol stays alive as long as any compilation holds it. + var anchor = TestCompilation.Create(); + var anchorCorlib = anchor.GetSpecialType(SpecialType.System_Int32).ContainingAssembly; + + // List reports its definition's assembly - corlib - as the containing assembly. Keying the + // cached identity off that assembly, rather than off the symbol, pinned the source compilation for as + // long as the anchor lived. + var (reference, name, corlib) = BuildIdentityAndDiscardCompilation(); + await Assert.That(name).IsEqualTo("List"); + await Assert.That(ReferenceEquals(corlib, anchorCorlib)).IsTrue(); + + // Act + for (var attempt = 0; attempt < 3; attempt++) + { + GC.Collect(2, GCCollectionMode.Forced, blocking: true, compacting: true); + GC.WaitForPendingFinalizers(); + } + + // Assert + await Assert.That(reference.IsAlive).IsFalse(); + + GC.KeepAlive(anchorCorlib); + GC.KeepAlive(anchor); + } + + // Kept out of the caller's frame so the compilation has no live root once this returns. + [MethodImpl(MethodImplOptions.NoInlining)] + static (WeakReference Compilation, string IdentityName, IAssemblySymbol Corlib) BuildIdentityAndDiscardCompilation() + { + var compilation = TestCompilation.Create( + """ + namespace Sample; + + public class Marker { } + + public class Owner + { + public System.Collections.Generic.List Items = null!; + } + """ + ); + + var owner = compilation.GetTypeByMetadataName("Sample.Owner")!; + var itemsType = ((IFieldSymbol)owner.GetMembers("Items")[0]).Type; + + TypeIdentity identity = new(itemsType); + + return (new(compilation), identity.Name, itemsType.ContainingAssembly); + } }