diff --git a/.github/skills/ownership-adoption/SKILL.md b/.github/skills/ownership-adoption/SKILL.md new file mode 100644 index 0000000..a017e5f --- /dev/null +++ b/.github/skills/ownership-adoption/SKILL.md @@ -0,0 +1,85 @@ +--- +name: ownership-adoption +description: >- + Plan and run a staged adoption of ErrorProne.NET disposable ownership in an + existing C# repository. Use for local NuGet pilots, disposable-usage audits, + finding annotation candidates, or evaluating ERP044, ERP045, and ERP046 before + rollout. Coordinate inventory, contract review, and real-project validation. +--- + +# Disposable ownership adoption + +Adoption is an evidence-gathering exercise, not a request to put `using` around +every disposable value. An analyzer finding is an unfulfilled modeled obligation, +not proof of a runtime leak. A clean build is not a proof of ownership safety. + +## Establish the boundary + +Read repository instructions and record the checkout, commit, branch, dirty +files, build entry points, compiler version, and existing analyzer configuration. +Find these facts yourself; ask the user only for material decisions: + +- Is this inventory-only, a package/configuration pilot, or an approved source fix? +- May local branches be created, and which files/projects may change? +- Should the review include real risks outside the current analyzer's coverage? + +For a large repository, agree on a broad inventory plus deeply traced, ranked +candidates. Do not quietly substitute a few interesting files for an inventory. +Independent repositories may be investigated in parallel; keep source-analysis +ownership separate from package/build work to avoid duplicated investigation. + +Keep proprietary source, paths, service names, findings, and build logs in the +consumer repository or an explicitly private artifact directory. Reusable skills +and public analyzer regressions must use independently written, generic examples. +Do not publish, commit, push, change global NuGet settings, or run live integration +tests without the corresponding authorization. + +## Workflow + +1. Read [ERP044](../../../docs/Rules/ERP044.md), + [ERP045](../../../docs/Rules/ERP045.md), and + [ERP046](../../../docs/Rules/ERP046.md). These documents are the policy authority. +2. Use [ownership-inventory](../ownership-inventory/SKILL.md) to map disposable + implementations, producer APIs, consumers, and mixed-cleanup call sites. +3. Use [ownership-pilot](../ownership-pilot/SKILL.md) to publish an immutable local + preview, establish the unchanged baseline, and exercise representative projects. +4. Use [ownership-contract-review](../ownership-contract-review/SKILL.md) to + classify findings and agree on individual ownership boundaries. +5. After source changes are authorized, annotate the smallest justified boundary, + rerun the pilot, and verify both intended diagnostics and negative controls. +6. Generalize demonstrated lessons into the adoption workflow; retain uncertainty + and untested steps rather than presenting a draft procedure as proven. + +## Non-negotiable v1 distinctions + +| Distinction | Adoption consequence | +| --- | --- | +| Disposable capability versus instance ownership | A disposable type does not make every result owning. | +| Unknown versus borrowed | Missing annotations do not prohibit disposal. | +| Input versus output ownership | A consumed parameter and a borrowed result are independent contracts. | +| Recognized transfer versus proven final cleanup | Member storage and explicit contracts are trusted boundaries. | +| Source inference versus library contracts | A simple fresh return may be inferred in one compilation but not across a project/assembly boundary. | +| Diagnostic versus defect | Missing knowledge about a task, callback, collection, or wrapper can produce a finding on correct code. | +| No diagnostic versus safety | Exceptional paths, lifecycle teardown, and general escapes remain outside a full proof. | + +Do not expand the ownership model or invent type-wide attributes as an adoption +shortcut. Separate necessary analyzer improvements from consumer-code changes. + +## Handoff + +Deliver the package IDs/version/feed, reproducible pilot commands, baseline and +pilot outcomes, inventory coverage and gaps, and a ranked review queue. Each +candidate needs a producer, representative callers, the likely lifetime owner, +confidence, current analyzer coverage, and the next decision. + +Use a concrete question, for example: + +```csharp +using var first = provider.Open(); +var second = provider.Open(); +cache.Remember(second); +``` + +Determine whether `Remember` owns and eventually releases its argument before +asking whether to add disposal. Present source evidence and a recommended +contract; never infer approval from a context-only reply. diff --git a/.github/skills/ownership-contract-review/SKILL.md b/.github/skills/ownership-contract-review/SKILL.md new file mode 100644 index 0000000..aaac16b --- /dev/null +++ b/.github/skills/ownership-contract-review/SKILL.md @@ -0,0 +1,150 @@ +--- +name: ownership-contract-review +description: >- + Review disposable ownership boundaries before adding ErrorProne.NET source or + external annotations. Use to classify ownership-oblivious APIs, borrowing, + transfers, mixed-cleanup callers, and suspected ownership analyzer false positives. +--- + +# Review an ownership contract + +Read the current [ERP044](../../../docs/Rules/ERP044.md), +[ERP045](../../../docs/Rules/ERP045.md), and +[ERP046](../../../docs/Rules/ERP046.md) contracts before proposing a change. + +## Build a lifetime record + +For one producer/consumer boundary, document: + +- Which instance is created, returned, passed, stored, or shared. +- Who is responsible before the call, after success, and after failure. +- Whether the result aliases an input, a receiver, a cached value, or a fresh value. +- Who releases retained state, including replacement and shutdown paths. +- Whether source, inherited, or external contracts already apply. +- What today's analyzer actually observes versus what manual tracing establishes. + +Treat task completion, callbacks, aggregate collections, certificate collections, +and configurable wrappers as boundaries to investigate. Do not add early `using` +to silence a finding if the value must remain live after the method returns. + +For a composite result, trace every independently acquired underlying resource. +Disposing a returned wrapper may close a stream but leave its connection owner +unreleased. A local `using` on that connection before returning the wrapper is +not a fix; the composite needs a truthful ownership and teardown design. + +## Choose the smallest truthful contract + +| Established behavior | Candidate annotation | +| --- | --- | +| Result transfers cleanup responsibility to its recipient | `ReturnsOwnership` on method/property/return | +| Recipient must neither dispose nor transfer a result | `DoNotDispose`, preferably return-targeted for methods | +| Callee acquires responsibility for an argument | `AcquiresOwnership` on that parameter | +| Callee only borrows an argument | `DoNotDispose` on that parameter | +| Ownership varies with arguments or remains uncertain | Keep unknown; document the decision or model limitation | + +Do not label all results of a disposable type as owned. Do not label unknown +results borrowed merely to reduce warning volume. Do not annotate generic +collections, task-completion methods, or callback APIs globally based on one +application's lifetime convention. + +An array or list is not automatically an ownership contract for its elements. +Trace element cleanup in the actual aggregate owner. Likewise, successful task +publication and eventual consumption are distinct: a task can be abandoned, and +`TrySetResult` can reject a value. Do not model conditional acceptance as an +unconditional acquiring parameter. + +If one local aliases a borrowed input on one branch and a newly created copy on +another, dispose only the copy. An unconditional `using` on the merged alias can +turn a real cleanup omission into disposal of somebody else's resource. + +Input and result contracts remain independent: + +```csharp +[return: DoNotDispose] +Resource Replace( + [AcquiresOwnership] Resource incoming, + [DoNotDispose] Resource shared) +{ + StoreOwned(incoming); + return shared; +} +``` + +The shared result does not discharge `incoming`; `StoreOwned` must establish +that transfer. A return contract also does not assert that the result aliases a +particular argument. + +Ask one concrete decision at a time. Present representative caller code, +the implementation evidence, the proposed owner, and the recommended answer. +A discussion or context-only reply is not approval to apply attributes. + +## Source versus external annotations + +Use source attributes for owned APIs when changes are approved. The +[annotations generator](../../../src/ErrorProne.NET.Annotations/README.md) +embeds internal types into `RootNamespace` or its explicit override; its public +API usages survive full and reference-assembly emission. No runtime annotation +DLL or public attribute mode is needed. + +Use `.ownership.xml` through the consuming project's `AdditionalFiles` when +the API cannot be edited: + +```xml + + + + + + + +``` + +Obtain declaration IDs from resolved symbols, such as +`DocumentationCommentId.CreateDeclarationId`, rather than guessing overloaded, +generic, constructor, or explicit-interface syntax. Use the assembly's simple +name and actual parameter names. Verify resolution in the intended compilation: +an absent member can be legal in an annotation pack, so no ERP045 does not prove +that an entry matched. Source contracts take precedence over XML. + +The v1 XML schema cannot express arbitrary conditional ownership. Never turn +`leaveOpen: true` or a conditional ownership flag into an unconditional transfer. + +## Validate the decision, not just compilation + +After authorization, require a positive and a negative example for the boundary: +missing cleanup reports, legitimate cleanup/transfer does not, and borrowed +cleanup reports where applicable. Check library consumers separately from +same-compilation inference. Verify independent incoming and outgoing obligations. + +If the issue is in the analyzer, create an independently written minimal +regression in ErrorProne.NET. Reproduce the diagnostic before changing analysis, +then check adjacent ownership cases and the real pilot again. + +Keep negative-control scenarios for retained task results, reusable release +tokens, explicit shutdown-callback cleanup, and collection-element transfers. +Known completed-task wrappers transport result identity; unknown publication, +collection arguments and captures make ownership uncertain and silence ERP044. +Distinguish these outcomes: silence at an unknown handoff does not prove +successful transfer, callback execution or eventual cleanup. Contrast each with +local abandonment, discarded completed wrappers, explicit borrowed boundaries +and ordinary receiver use. Failed-transfer cases can remain intentionally +undetected under the low-noise policy; record that trade-off instead of claiming +that a green build establishes safety. + +Keep fixes to consumer lifetimes, fixes to analyzer modeling, and intentional +trusted contracts separate in the review record. Never close an uncertain case +as a confirmed leak or a proven-safe transfer. + +## Record v1 gaps explicitly + +Separate a supported contract that behaves incorrectly from a documented +analysis limit, missing/incorrect consumer metadata, or an integration problem +such as stale compiler output. Keep both noisy correct lifetimes and missed +unsafe lifetimes in the review queue. + +For each gap, state the expected lifetime, observed diagnostic or absence, +minimal independently written reproduction, affected real-world pattern, +current documented boundary, and the decision needed before expanding v1. +Characterization tests can record a known limitation without changing policy; +name them accordingly. Passing such a test means the limitation was reproduced, +not that the example's lifetime is safe. diff --git a/.github/skills/ownership-inventory/SKILL.md b/.github/skills/ownership-inventory/SKILL.md new file mode 100644 index 0000000..03a9763 --- /dev/null +++ b/.github/skills/ownership-inventory/SKILL.md @@ -0,0 +1,118 @@ +--- +name: ownership-inventory +description: >- + Inventory IDisposable and IAsyncDisposable implementations and trace ownership + across C# producers and consumers. Use to find inconsistent disposal of results, + critical resource types, ownership-transfer boundaries, and adoption candidates. +--- + +# Inventory disposable ownership + +## Define and measure coverage + +Start from tracked source and project membership. Exclude generated outputs, +build caches, secrets, deployment data, and unrelated languages. Classify +production, test-only, generated, and vendored code separately. +Account for case-varied project extensions. Inspect provenance before excluding +a directory merely named `Output`: it can contain real source. A fake in a +shipping-library folder is still a fake, not proof of production ownership. + +Prefer an existing semantic index or Roslyn symbols for type relationships and +references. If only syntax/search is available, label the output accordingly. +Search is a candidate generator, not evidence that two callers invoke the same API. +Before starting an indexer or language server in a read-only audit, check whether +it triggers design-time builds, restores, or writes outside the approved scope. + +Inventory: + +- Direct and inherited `IDisposable` and `IAsyncDisposable` implementations. +- Disposable structs and lease/registration tokens. +- Pattern-disposable types as a separate category; verify analyzer support. +- `SafeHandle` descendants and resource-owning wrappers. +- Raw handles and manual release APIs as a separate, currently unmodeled category. +- Interfaces/abstract bases that carry contracts, separately from instantiable types. + +Follow in-repository base types transitively and distinguish nested types and +generic arity. Merge partial declarations by symbol when binding is available; +otherwise preserve declaration rows and qualify any unique-type count. +Retain unresolved external bases, +ambiguous type names, conditional compilation, and unavailable projects as +coverage gaps. Do not label a direct `: IDisposable` text search exhaustive. + +For every inventory entry record: + +| Field | Evidence | +| --- | --- | +| Identity | Project/assembly, qualified type, arity, declaration locations | +| Classification | Production/test/vendor; direct/inherited/pattern/unknown | +| Lifetime role | Resource owner, borrowed facade, lease, aggregate, marker | +| Resource impact | Native resource, connection, timer, subscription, lock, managed-only, unknown | +| Cleanup | Actual cleanup implementation and owned fields; not merely the method name | +| Usage | Construction/factories, returned values, parameters, member storage, representative callers | +| Confidence | Semantic resolution or bounded syntax/manual tracing; unresolved edges | + +## Find high-value producer/consumer pairs + +Group calls by resolved member identity, overload, and relevant arguments. +Include properties and awaited `Task`/`ValueTask` results. Keep disposal +of a task distinct from disposal of the resource produced by awaiting it. + +Look for an API whose result is: + +- Disposed with `using`, `await using`, `Dispose`, or `DisposeAsync` by some callers. +- Ignored, retained in a local, stored, returned, or passed onward by other callers. +- Transferred through a wrapper, callback, task completion, queue, or collection. + +For each promising pair, trace both call paths to a terminal owner. Check whether +the API allocates, returns shared state, lends a pooled object, or transfers an +existing object. Freshness is evidence, not a substitute for an ownership contract. + +```csharp +using var a = sessions.Get(); +var b = sessions.Get(); +owner.Attach(b); +``` + +This is not a leak report until `Get`, `Attach`, and the owner's teardown have +been investigated. Repeated calls may return the same object, and one caller's +existing `using` may be the bug. + +Caching does not by itself prove borrowing. A cached release token can represent +one disposal duty for each successful lock acquisition, even when several +acquisitions receive the same object. Conversely, constructing that token may not +acquire anything. Trace acquisition/release state, not just allocation and identity. + +Inspect ownership-changing arguments such as `leaveOpen`, `disposeHandler`, and +pool/retention options. Do not generalize one overload or argument combination +into an unconditional contract for every call. + +## Rank without inflating certainty + +Prioritize resource cost, call frequency, retention duration, and affected +production paths. A cancellation source that owns a timer or registrations is +different evidence from an otherwise unused disposable marker. + +Record at least one negative control: a mixed-use pattern that is safe because +of sharing or a verified transfer. Include candidates where `DoNotDispose` +would prevent an inappropriate cleanup, not only missing-dispose candidates. + +Separate results into: + +1. Evidence-backed missing cleanup or misuse. +2. Correct runtime ownership with missing analyzer-visible contracts. +3. Unresolved ownership requiring an API-owner decision. +4. Analyzer false positive or unsupported transport. +5. Real risk outside v1, such as exceptional-path or lifecycle cleanup. + +Deduplicate multi-target findings by rule, source location, and modeled resource; +retain the target frameworks as evidence rather than counting each as a new bug. + +## Deliverables + +Save a private machine-readable inventory and a code-linked review queue. +State files/projects considered, direct and transitive discovery methods, +classification counts, unresolved bases, and manually traced coverage. +Do not confuse the number of matches with the number of audited lifetimes. + +Feed the strongest candidates into +[ownership-contract-review](../ownership-contract-review/SKILL.md). diff --git a/.github/skills/ownership-pilot/SKILL.md b/.github/skills/ownership-pilot/SKILL.md new file mode 100644 index 0000000..89a55ee --- /dev/null +++ b/.github/skills/ownership-pilot/SKILL.md @@ -0,0 +1,187 @@ +--- +name: ownership-pilot +description: >- + Build and consume a local ErrorProne.NET ownership preview in real C# projects. + Use for immutable NuGet folder feeds, opt-in analyzer/source-generator wiring, + isolated MSBuild assets, baseline comparisons, and ERP044/045/046 rollout evidence. +--- + +# Run a real-project ownership pilot + +## Publish a local preview + +Record the analyzer commit and exact emitted package versions. Build and pack +`ErrorProne.Net.Annotations` and `ErrorProne.NET.CoreAnalyzers` using the existing +projects and build tools. Do not change dependency versions to hide restore issues. + +Use an explicit local folder feed. Never omit the destination in a publishing +command, publish to a remote feed, or add a global package source by default. +For this repository, the analyzer package is produced by the CoreAnalyzers +CodeFixes project, not the implementation project's placeholder package ID. + +Account for build-time versioning: inspect the actual `.nupkg` version rather +than assuming a command-line `PackageVersion` override won. Do not overwrite +different package contents under the same ID/version. Record package hashes. +Projects with `GeneratePackageOnBuild` may require an explicit configuration +build before packing; a missing Release DLL is not a NuGet restore problem. + +Inspect the package archive for analyzer DLLs and required build-transitive +assets, especially compiler-property forwarding for the generator. There must +be no `lib`/`runtimes` assets from these build-only packages. + +## Establish a baseline first + +Inspect SDK/compiler versions, central package management, current analyzer +versions, warnings-as-errors, custom output paths, and the repository's supported +build commands. The generator requires Roslyn 4.13 or newer; a project's target +framework and its compiler version are separate facts. + +Build the smallest representative real project without the preview. If the +baseline fails because dependencies are missing, restore through the repository's +existing sources and repeat. Distinguish baseline/environment failures from +preview regressions; do not declare an unchanged failure caused by the analyzer. + +Never run live integration tests, production startup, or deployment targets as +a shortcut to validation. Use existing targeted, local tests. + +## Make rollout opt-in and reversible + +Preserve normal defaults. Prefer an explicit pilot property and, for a large +graph, a project selector. Replace the selected project's existing analyzer +reference rather than loading old and new analyzer assemblies together. + +Pin both preview package versions and keep: + +```xml +PrivateAssets="all" +IncludeAssets="analyzers;build;buildtransitive" +``` + +Respect the repository's actual package-management mechanism. NuGet central +package management and `Microsoft.Build.CentralPackageVersions` do not have +identical import order or global-reference semantics. + +Add the local feed only for the pilot invocation, for example through +`RestoreAdditionalProjectSources`. Retain the established authenticated sources. +Verify NuGet's restored metadata identifies the intended local package. + +Isolate preview restore assets, intermediate compilation, generated sources, +binary outputs, package outputs, and diagnostic logs from the normal build. +Separate package selection from artifact isolation: a test project may consume +a preview-built dependency without enabling the preview analyzer itself, but +its copied dependencies and test output still belong to the isolated invocation. +Set early MSBuild path properties before they are consumed; evaluate the final +paths per project and target framework. Do not globally force all referenced +projects to share one assets file or output directory. + +Trace later output overrides, not just the pilot's first property assignment. +Evaluate representative library, application, and custom-package-ID shapes and +compare `OutputPath`, `OutDir`, `TargetDir`, and package paths with normal builds. +Limit the supported graph explicitly and fail closed for unsupported project +kinds or layouts. Keep pilot builds out of normal release package staging and +deployment targets; preserve normal output precedence when the pilot is off. + +Keep generated files outside source globs. Existing generators, such as +nullability polyfills, may emit additional attributes; count this generator's +output by producer, not every `*Attribute.g.cs` in the directory. + +Check `git status` and ignore rules for every required new import. A successful +local build can hide a broken patch when its new `.props` files are ignored. +Expose only the required files; do not unignore arbitrary build output. + +## Exercise and classify + +For a dirty-source preview, verify the effective package version after the +versioning targets execute, not just at project evaluation. Git-based version +generators can override a command-line `PackageVersion`. Pack into an isolated +staging folder, inspect the archive identity and hashes, then copy to the local +feed without overwriting an existing version. Record the base commit and dirty +source-input/diff hashes; a commit-shaped version alone is insufficient. + +Restore and build selected projects using the preview. Capture structured +diagnostics, preferably separate SARIF files per project/target. Preserve +warnings-as-errors and existing analysis; do not add broad suppressions or +disable analyzers to obtain a green result. + +Changing the project selector or whole-graph scope changes effective package +references. Restore with that same selection, and inspect the selected project's +actual assets and resolved compiler analyzers. A forced rebuild with stale assets +can succeed without loading the requested preview at all. Fresh SARIF and a +correct package archive on disk do not, by themselves, establish analyzer loading. + +A graph may stop at an early ownership diagnostic. Record that failure, then +use a clearly documented project-scoped pilot to inspect other projects without +silently weakening the original graph's policy. + +Build a small provider and a real caller when testing a cross-project contract. +Compiling only the provider can confirm annotation generation but cannot prove +that a downstream call receives the intended ownership diagnostic. + +For an annotation-driven experiment, record the same source and preview with +the contract off and on, then a correct cleanup/transfer control with it on. +An existing unrelated diagnostic can prevent an otherwise useful comparison +from becoming green. Record that baseline separately; if authorized, temporarily +discharge that independent obligation in every comparison, then restore only +your control edits. Never label an unrelated compiler failure as successful +detection of the intended ownership defect. + +Force compilation for these measurements, for example with `--no-incremental`. +Changing a conditional `AdditionalFiles` item set can reuse old successful +compiler output when the newly included file is older than that output. +Where appropriate, expose the contract-mode property through +`CompilerVisibleProperty` so the generated analyzer configuration also changes. +Verify an ordinary incremental off-to-on transition separately. Preserve each +stage's SARIF only after confirming compilation actually ran. + +When the user requests a deliberately failing pilot, retain only the opt-in +contract/configuration and the original source defect, not an injected runtime +leak. Record the expected rule, source span, and exit code. Promote a warning +to an error only in that narrowly selected experiment if needed; do not alter +normal warning policy. Restore temporary cleanup controls and confirm both the +expected opt-in failure and the unaffected normal build. + +If the user subsequently requests permanent source fixes, retain the corrected +source with the contracts enabled; do not reintroduce the original omission. +Keep historical negative-control evidence separate from the corrected build. +Choose cleanup boundaries from actual ownership: materializers consume readers, +returned streams outlive their factory call, borrowed inputs are not owned copies, +and scheduled work or retained registry aliases may outlive the current method. +Unknown contracts and unfinished asynchronous teardown remain separate questions, +not permission to add immediate disposal or claim complete lifetime safety. + +Separate: + +- ERP044/ERP046 ownership findings requiring code/contract review. +- ERP045 or EPANN configuration/annotation failures. +- AD0001 and analyzer loading or compiler compatibility failures. +- Other new diagnostics from upgrading the complete analyzer package. +- Baseline, restore, compiler, test-host, or environment failures. + +Deduplicate repeated target-framework emissions while retaining target coverage. +Route findings to [ownership-contract-review](../ownership-contract-review/SKILL.md); +do not equate the diagnostic count with a leak count. + +ERP044 remains enabled by default but quiets unresolved lifetimes at unknown +argument handoffs and captures. When comparing previews, classify disappearing +diagnostics as proven transport, conservative uncertainty, or an unintended +regression. Include annotated owned-result abandonment and receiver-only use +as positive controls, and explicit borrowed-parameter handoff as a retained +obligation. A lower warning count alone is not increased safety coverage. + +Run existing focused tests against the preview-built dependency. Verify a +nonzero matching test count and the actual assembly used. A successful test +command with no restored test SDK or no matching tests is not passing coverage. + +Inspect runtime output for accidental ErrorProne DLL dependencies. Verify the +normal build still resolves its original packages and output paths with the +pilot disabled. + +## Exit evidence + +Save exact package IDs, versions, hashes and source; baseline and pilot commands; +effective project/TFM coverage; diagnostic classifications; test counts; generated +annotation evidence; runtime-output evidence; and remaining blockers. + +Do not call a preview merge-ready because packages restored or one project +compiled. Unreviewed diagnostics and unresolved ownership decisions remain open, +even when the build treats them as warnings. diff --git a/ReadMe.md b/ReadMe.md index 56daeda..3493b2c 100644 --- a/ReadMe.md +++ b/ReadMe.md @@ -41,6 +41,47 @@ Add the following nuget package to you project: https://www.nuget.org/packages/E | [ERP041](https://github.com/SergeyTeplyakov/ErrorProne.NET/tree/master/docs/Rules/ERP041.md) | EventSource class should be sealed | | [ERP042](https://github.com/SergeyTeplyakov/ErrorProne.NET/tree/master/docs/Rules/ERP042.md) | EventSource implementation is not correct | +### Disposable ownership + +The `ErrorProne.Net.Annotations` source-generator package embeds internal +attributes directly in each project's root namespace, without adding a runtime +DLL. Public API contracts remain visible to consuming analyzers through metadata. +See [source-embedded annotations](src/ErrorProne.NET.Annotations/README.md) for +package setup, namespace overrides, and friend-assembly behavior. + +Ownership analysis uses attributes and bounded inference: a caller trusts a +transfer, and the consuming method is checked for disposal or further transfer. +Third-party contracts can be supplied as `*.ownership.xml` additional files. +Use `DoNotDispose` for borrowed values, including `[return: DoNotDispose]` on +borrowed results; disposing or transferring them reports ERP046. +Unannotated results are ownership-oblivious by default. A simple, non-overridable +source method directly returning a new disposable can establish ownership; +shared, complex, or external results need an explicit contract to establish a +caller obligation. Unknown does not imply `DoNotDispose`. +ERP044 is enabled by default with a low-noise policy: unknown argument handoffs +and captures stop an unresolved local cleanup warning, without proving transfer +or safety. Explicit borrowed arguments and ordinary receiver calls retain the +caller obligation. Known completed-task wrappers carry result ownership rather +than discharging it. +The rules intentionally do not attempt a complete borrow checker or proof of +exception safety; see the documented limitations. This first version focuses +on contracts and simple inference. Dedicated diagnostics for unknown ownership +escapes are deferred; missing knowledge is not itself evidence of unsafe code. + +| Id | Description | +|---|---| +| [ERP044](docs/Rules/ERP044.md) | Dispose owned resources or transfer their ownership | +| [ERP045](docs/Rules/ERP045.md) | Invalid external ownership annotation | +| [ERP046](docs/Rules/ERP046.md) | Obvious use after disposal/transfer or misuse of explicit borrowing | + +For adoption in an existing codebase, start with the +[ownership-adoption skill](.github/skills/ownership-adoption/SKILL.md). +It coordinates a [disposable inventory](.github/skills/ownership-inventory/SKILL.md), +[contract review](.github/skills/ownership-contract-review/SKILL.md), and an +[opt-in local-package pilot](.github/skills/ownership-pilot/SKILL.md). +These workflows distinguish actual lifetime defects from missing contracts and +analysis gaps before applying source changes. + ### Concurrency | Id | Description | diff --git a/docs/Rules/EPC34.md b/docs/Rules/EPC34.md index ce7aaab..a52cc9d 100644 --- a/docs/Rules/EPC34.md +++ b/docs/Rules/EPC34.md @@ -6,6 +6,15 @@ This analyzer detects when methods marked with `MustUseResultAttribute` have the The analyzer warns when the return value of a method decorated with `MustUseResultAttribute` is not used. This attribute indicates that the method's return value contains important information that should always be observed by the caller. +## Getting the attribute + +The [ErrorProne.Net.Annotations source generator](../../src/ErrorProne.NET.Annotations/README.md) +provides `MustUseResultAttribute` and the recognized compatibility name +`MustUseReturnValueAttribute`. It embeds internal types in the project's root +namespace without adding a runtime annotation DLL. Attributes on public APIs +remain visible to downstream analyzers through assembly metadata. +Applications can also continue defining their own attributes, as shown below. + ## Code that triggers the analyzer ```csharp diff --git a/docs/Rules/ERP044.md b/docs/Rules/ERP044.md new file mode 100644 index 0000000..dedb765 --- /dev/null +++ b/docs/Rules/ERP044.md @@ -0,0 +1,287 @@ +# ERP044 - Dispose owned resources before losing scope + +This rule looks for recognized disposal or ownership transfer of an acquired +`IDisposable` or `IAsyncDisposable` resource. A diagnostic means the modeled +obligation was abandoned within the modeled local scope; it is not proof that a +runtime leak occurs. The rule is enabled by default and favors low noise. + +```csharp +var resource = new Resource(); // ERP044: no recognized disposal or transfer. +``` + +Dispose it with `using`, `await using`, `Dispose`, or `DisposeAsync`, or transfer +responsibility to an owning member, return value, `out`/`ref` output, or consuming +API. + +## First-version scope + +This version focuses on ownership contracts, bounded local/callee inference, +and obvious contract violations. It does not perform general escape analysis +or issue a separate diagnostic when ownership tracking becomes unknown. + +An unannotated API may genuinely retain a resource even when the analyzer cannot +infer a transfer. Passing a resource as an unknown argument or capturing it in +a callback makes its cleanup uncertain, so ERP044 stays quiet for that lifetime. +This is not evidence of a successful transfer and does not imply +`AcquiresOwnership` or an ERP046 use-after-transfer diagnostic. Ordinary instance +receiver calls, such as `resource.Use()`, do not hide an outstanding obligation. + +Explicit contracts still matter: passing to a `DoNotDispose` parameter retains +the caller's cleanup obligation, while an acquiring contract establishes a +transfer. Describe known boundaries with attributes or external annotations. +No diagnostics does not establish safety: unknown handoffs may leak, and +recognized ownership boundaries are trusted, not verified transitively. + +Dedicated, opt-in diagnostics for unknown escapes are deferred to a later +iteration. They should distinguish missing knowledge from an ownership bug. + +## Ownership contracts + +Attributes are recognized by their short class names. They can be declared in +your application; no runtime reference to the analyzer assembly is required. + +The [ErrorProne.Net.Annotations source generator](../../src/ErrorProne.NET.Annotations/README.md) +provides these types without another runtime DLL. It generates internal +attributes in the consuming project's `RootNamespace`, with an optional +`ErrorProneAnnotationsNamespace` override. Internal attribute usages on public +APIs survive in both compiled libraries and reference assemblies, where a +downstream analyzer can read them without referencing the generator package. + +Alternatively, applications can define the attributes themselves: + +```csharp +[AttributeUsage(AttributeTargets.Parameter)] +public sealed class AcquiresOwnershipAttribute : Attribute { } + +[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Field + | AttributeTargets.Property | AttributeTargets.Method | AttributeTargets.ReturnValue)] +public sealed class DoNotDisposeAttribute : Attribute { } + +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property + | AttributeTargets.ReturnValue)] +public sealed class ReturnsOwnershipAttribute : Attribute { } + +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property + | AttributeTargets.ReturnValue)] +public sealed class KeepsOwnershipAttribute : Attribute { } + +[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Field + | AttributeTargets.Property)] +public sealed class NoOwnershipAttribute : Attribute { } +``` + +An acquiring parameter transfers responsibility from the caller. The caller +trusts the contract even if the implementation is incorrect; the implementation +gets the warning instead. + +```csharp +void Caller() +{ + var resource = new Resource(); + Consume(resource); // Caller has transferred ownership. +} + +void Consume([AcquiresOwnership] Resource resource) // ERP044 on resource +{ + // Must dispose resource or forward responsibility. +} +``` + +Every acquiring parameter is checked, including constructor and indexer parameters. +An explicit acquiring contract also applies to `object` and unconstrained generic +parameters: erasing the static disposable type does not erase the callee's duty. +Ordinary casts and direct type-pattern aliases can carry that parameter to cleanup. +Overrides and interface implementations inherit ownership contracts. + +An acquiring `Task` or `ValueTask` parameter acquires the eventual result. +Its body must dispose that result after awaiting, or transfer responsibility; +disposing only the Task wrapper does not satisfy the contract. Passing a known +completed-result wrapper to such a parameter transfers its carried resource. +This is a trusted ownership boundary, not proof that asynchronous cleanup completes. + +`ReturnsOwnership` marks an owning method/property result. `DoNotDispose` +marks a borrowed parameter, member, or result; prefer `[return: DoNotDispose]` +for method results. The recipient must not dispose the resource or transfer it +to an owning consumer. Return-target attributes are recognized. Explicit +contracts take priority over factory/fluent heuristics. +A borrowed-result contract does not imply that the result aliases the receiver +or any particular argument. + +`KeepsOwnership` remains supported for borrowed results, and `NoOwnership` +remains supported for borrowed parameters or members. Assigning a resource to +a borrowed member does not discharge responsibility. See [ERP046](ERP046.md) +for disposal and transfer restrictions, including simple local aliases. + +Parameter and return contracts are independent. Returning a borrowed value +does not suppress cleanup obligations for acquired inputs, whether borrowing +is expressed through `DoNotDispose`, `KeepsOwnership`, or an external contract. + +```csharp +[return: DoNotDispose] +Resource BorrowOther( + [AcquiresOwnership] Resource owned, // ERP044: still needs disposal or transfer. + [DoNotDispose] Resource borrowed) +{ + return borrowed; +} +``` + +Disposing `owned` before returning, or removing that acquired parameter, leaves +no unfulfilled input obligation. Returning the acquired resource itself also +counts as a transfer in this bounded model, so an explicit `ReleaseOwnership` +helper remains supported. This is a trusted boundary, not a proof that the +resource will eventually be disposed. + +## Ownership-oblivious APIs + +A parameter or result is **ownership-oblivious** (or **dispose-oblivious**) +when it has no explicit ownership contract from source attributes, inherited +contracts, or external annotations. This terminology follows the analogy of +nullable-oblivious APIs: missing contract information is neither an explicit +owning contract nor an explicit borrowing contract. A method can have an +annotated input and an oblivious result, or vice versa. + +Disposability and ownership answer different questions. `IDisposable` and +`IAsyncDisposable` describe cleanup capabilities; they do not identify who is +responsible for a particular instance: + +```csharp +static Resource Create() => new Resource(); +static Resource GetShared() => shared; +``` + +Both results have the same disposable type and lack an explicit ownership +contract. The type alone cannot distinguish a new instance from a shared or +reused one. Moreover, an existing instance can legitimately be transferred to +a new owner: allocation freshness is not itself an ownership contract. +Annotate the boundary to express the intended responsibility: + +```csharp +[return: ReturnsOwnership] +static Resource Create() => new Resource(); + +[return: DoNotDispose] +static Resource GetShared() => shared; +``` + +A future type-level "must dispose" annotation could describe the cleanup +obligation of an owner, but would not establish that every recipient owns an +instance. Type-level ownership annotations, including equivalent external +annotations, are not implemented in this version. + +Oblivious describes the missing contract, not a promise that analysis is +disabled. Results default to unknown, but the bounded inference below recognizes +the direct fresh return in `Create`. Its caller must dispose or transfer that +result. `GetShared` remains unknown: merely returning a disposable type does +not create an obligation or prohibit disposal. + +An unannotated property result likewise creates no obligation and is not +automatically subject to `DoNotDispose`. Unknown is not the same as borrowed. +Without an explicit borrowing contract, disposal of an unknown result is not +itself an ERP046 violation. The absence of a diagnostic does not establish safety. + +## Good-enough inference + +The analysis intentionally favors useful, bounded inference over an exhaustive +borrow checker: + +- Creating a tracked disposable resource creates a cleanup obligation. +- Unannotated method results default to unknown. Ownership is inferred only for + a non-overridable source method in the current compilation with an expression + body or a single `return` statement directly constructing a tracked disposable. + Ordinary built-in casts and parentheses are accepted; user-defined and dynamic + conversions are not evidence of a fresh result. A direct `new T()` requires a + disposable constraint. Explicit source and external contracts take precedence. +- The same result contract or inference applies after `await`, including framework + `ConfigureAwait` chains. Explicit property contracts also apply to awaited + `Task` and `ValueTask` results. An async method directly returning a new + disposable can therefore establish an obligation for its awaited result. + An explicit result contract on a framework wrapper takes precedence over + the underlying member's contract. +- Mixed/conditional returns, returns through locals, factory forwarding, + additional body statements, iterator yields, overridable methods, and bodies + outside the current compilation do not establish result ownership through + this inference. Use an explicit result contract for such boundaries. +- Unannotated property access does not create a cleanup obligation. Use + `ReturnsOwnership` for owning results and `DoNotDispose` for explicit borrowing. +- Same-type instance fluent methods and generic identity-like extension methods + normally preserve the receiver/argument unless an explicit result contract or + fresh-return inference says otherwise. A simple `Clone() => new Resource()` + creates a separate obligation; disposing that copy does not dispose its receiver. +- For source callees in the current compilation, simple disposal, using, storing + in an owning member, and forwarding to another consumer can infer an acquiring + parameter. Simple local aliases are followed. Recursive inference is capped + at four parameter hops. +- Unannotated invocation, constructor, indexer and user-defined operator inputs without + recognized consumption make ownership uncertain. Dynamic argument handoffs + and lambda/local-function/method-group captures also silence ERP044. + Extension receivers are arguments and follow their parameter contracts, + except for the recognized fluent-alias inference described above. + Other arguments to a fluent call still follow their own contracts. + Uncertainty does not erase explicit borrowing or later definite disposal + and use-after-disposal checks, nor obligations for replacement allocations. +- `StreamReader`, `StreamWriter`, `BinaryReader`, and `BinaryWriter` constructors + are recognized as owning their stream when `leaveOpen` is absent or constant + `false`. Constant `true` retains the caller's obligation; unknown `leaveOpen` + makes cleanup uncertain without proving transfer. +- Returning a resource, assigning it to an owning field/property or an output + parameter, and `Interlocked.Exchange` into an owning member transfer ownership. +- Local aliases retain the same obligation. Discarding an alias does not transfer + it. Unconditional reassignment of a local or parameter does not let cleanup of + the new resource hide an older leaked resource. Conditional reassignments are + handled conservatively and can hide leaks. +- `Task.FromResult`, `ValueTask.FromResult` and `new ValueTask(T result)` + transport the result's responsibility through local aliases, returns, owning + member storage and `await`, including framework `ConfigureAwait`. Discarding + the wrapper, awaiting without cleanup, or disposing the task itself does not + dispose the carried resource. Task and resource identities remain separate. + Supported fluent carrier aliases and `Interlocked.Exchange` into an owning + member preserve that distinction. Bounded source consumption inference also + follows these carriers to actual cleanup or owning storage, not mere wrapping. +- `using` and `await using` capture the resource before their body executes. + Reassigning the original variable inside the body does not dispose the new + value. The framework's `IAsyncDisposable.ConfigureAwait` wrapper preserves + resource identity, including through local aliases. +- User-defined conversions are separate input/result boundaries, not aliases. + Explicit conversion-operator contracts are honored independently, and source + consumption can be inferred for their parameters. An unknown conversion input + makes cleanup uncertain, not discharged. Dynamic conversions do not establish + identity or prove consumption. + +Task objects, `StringReader`, and `MemoryStream` families retain the prototype's +disposal exemptions. This includes derived types, so these exemptions are +heuristics rather than a general statement that disposal can never matter. + +## External annotations + +Ownership contracts for third-party APIs can be supplied through Roslyn +`AdditionalFiles`. See [ERP045](ERP045.md) for the XML schema and validation. +External and source contracts feed the same call-site and callee analysis. + +## Deliberate limits + +This is not a proof of exception safety or a Rust borrow checker. Conditional +cleanup/transfer is accepted even when another path may leak. General loops, +complex aliases, closures, local-function captures, dynamic calls and collection +element ownership are not modeled exhaustively. Nested lambda/local-function +bodies are not analyzed as separate lifetimes. Captured variables make cleanup +uncertain even when the resource is assigned after the callback is created; +it is not evidence that the callback actually runs or disposes anything. + +Calling `DisposeAsync` is accepted as cleanup; the rule does not prove that its +completion is awaited. Only the completed-result wrappers listed above carry +tracked resource identity; arbitrary task/container wrappers are not modeled. +Unknown collection or task-publication arguments can silence a warning even +when the recipient rejects or never disposes the resource. + +Transferring to a member trusts the receiving object's ownership boundary; +the rule does not prove that the containing type eventually disposes its fields. +Similarly, contracts in referenced assemblies cannot be checked against bodies +that are unavailable in the current compilation. + +## Configuration + +```ini +[*.cs] +dotnet_diagnostic.ERP044.severity = warning +``` diff --git a/docs/Rules/ERP045.md b/docs/Rules/ERP045.md new file mode 100644 index 0000000..c39e9fb --- /dev/null +++ b/docs/Rules/ERP045.md @@ -0,0 +1,127 @@ +# ERP045 - Invalid external ownership annotation + +External ownership annotations describe third-party APIs without modifying their +assemblies. Both [ERP044](ERP044.md) and [ERP046](ERP046.md) use the same resolved +contracts as source attributes. + +Add an XML file whose name ends in `.ownership.xml` to your project: + +```xml + + + +``` + +## Schema + +```xml + + + + + + + + + + + +``` + +- `id` is a Roslyn documentation-comment declaration ID, including parameter + types for overloads. Constructors use `#ctor`; generic arity and type parameters + use documentation-ID syntax, not C# display syntax. +- `assembly` is optional. When specified, it is the assembly's simple name, + not its filename or full display identity. +- `returns` is optional and accepts `owned` or `borrowed`. +- Each `parameter` identifies a parameter by name and specifies `owned` or + `borrowed`. Argument order at a call site does not change the contract. +- Source attributes override external declarations; bounded inference applies + only when there is no explicit contract. + +An owning parameter is equivalent to `AcquiresOwnership`; an owning result is +equivalent to `ReturnsOwnership`. Borrowed parameters and results are equivalent +to `DoNotDispose`. The legacy names `NoOwnership` (parameters) and +`KeepsOwnership` (results) remain supported. + +Type-wide "must dispose" annotations are not supported by this schema. +Annotate individual API results and parameters to express ownership boundaries. + +Unannotated external results are ownership-oblivious. Their disposable type +alone does not create a caller cleanup obligation; use `returns="owned"` when +the API transfers ownership. Fresh-return inference is restricted to method +bodies in the current compilation, not referenced projects or assemblies. + +The file is read through Roslyn `AdditionalFiles` once per compilation. No +network fetch, external XML entity resolution, or dynamic assembly loading is +performed. DTDs are prohibited. + +## Using the contracts with a compiled library + +Suppose a referenced `Library.dll` exposes these APIs, with no ownership +attributes of its own. Its `Library.Resource` type implements `IDisposable`. + +| API | Contract to describe | +| --- | --- | +| `Library.Factory.Create()` | Returns an owned `Resource`. | +| `Library.Factory.GetShared()` | Returns a borrowed `Resource`. | +| `Library.Resources.Shared` | A borrowed `Resource` property. | +| `Library.Consumer.Take(IDisposable resource)` | Consumes the argument. | + +Register the XML above as `Library.ownership.xml` in the **consuming project** +using `AdditionalFiles`. The analyzer does not need the library's source or +changes to its assembly. The member IDs must match the actual API signatures, +and `assembly="Library"` must match the assembly's simple name. + +The declared contracts then apply at call sites: + +```csharp +var missingCleanup = Library.Factory.Create(); // ERP044. + +using var owned = Library.Factory.Create(); // Valid: caller disposes it. + +var borrowed = Library.Factory.GetShared(); +var alias = borrowed; +alias.Dispose(); // ERP046: the alias is still borrowed. + +using var shared = Library.Resources.Shared; // ERP046: using would dispose it. +Library.Consumer.Take(Library.Factory.GetShared()); // ERP046: borrowed transfer. + +var transferred = Library.Factory.Create(); +Library.Consumer.Take(transferred); // Valid: responsibility transfers. +transferred.Dispose(); // ERP046: already transferred. + +Library.Consumer.Take(new Library.Resource()); // Valid: explicit transfer. +``` + +Without these annotations, external results remain unknown: their disposable +type alone does not create an ERP044 obligation or an ERP046 borrowing +restriction. Passing a directly created resource to an unknown parameter, as in +`Library.Consumer.Take(new Library.Resource())`, makes cleanup uncertain and is +quiet under ERP044's low-noise policy. It does not establish a successful +transfer or imply ERP046 on subsequent use. An explicit borrowed parameter +retains the caller's obligation; an owned parameter establishes a transfer. +Use contracts to make these distinctions enforceable, not just to silence a +diagnostic. Simply abandoning `new Library.Resource()` still reports ERP044. + +## Diagnostics and limits + +ERP045 reports malformed XML, malformed member identifiers, unsupported schema +content or ownership values, and duplicate/conflicting declarations. Correct the +annotation instead of relying on a silently ignored contract. + +An annotation pack may include members from libraries not present in the current +compilation. An absent member alone does not make the pack invalid. External +annotations are contracts, not proofs of third-party implementation behavior. + +Conditional contracts, such as ownership depending on an arbitrary argument, +are not expressible in this first schema. Common stream-wrapper `leaveOpen` +behavior is handled by the analyzer's bounded built-in inference. Source and +external explicit contracts take precedence over that default. + +## Configuration + +```ini +[*.cs] +dotnet_diagnostic.ERP045.severity = warning +``` diff --git a/docs/Rules/ERP046.md b/docs/Rules/ERP046.md new file mode 100644 index 0000000..d545184 --- /dev/null +++ b/docs/Rules/ERP046.md @@ -0,0 +1,102 @@ +# ERP046 - Respect disposable ownership + +This rule reports obvious violations of the ownership contracts used by +[ERP044](ERP044.md). + +## Use after disposal or transfer + +```csharp +var resource = new Resource(); +Consume(resource); // AcquiresOwnership, or a source callee inferred to consume. +resource.Use(); // ERP046: ownership has already transferred. +``` + +```csharp +var resource = new Resource(); +var alias = resource; +resource.Dispose(); +alias.Use(); // ERP046: alias still refers to the disposed resource. +``` + +The check is deliberately restricted to later statements in the same lexical +block. Reassigning a local to a new resource starts a new lifetime. The rule +does not infer use-after-disposal across arbitrary branches, `finally` regions +or implicit `using` cleanup, and reports only the first obvious invalid use of +a tracked resource. + +Both the cleanup/transfer and the later use must identify the same resource +unambiguously within this bounded model. A condition-dependent alias is not +treated as proof of misuse. Compile-time `nameof` references are not runtime +uses. These exclusions do not establish that the remaining code is safe. + +Unknown argument handoffs and captures are not definite moves. ERP044 can stay +quiet because the recipient might retain ownership without ERP046 claiming +that subsequent use is invalid. A later definite `Dispose` still permits an +obvious use-after-disposal diagnostic. + +## Explicit borrowing + +```csharp +void Inspect([DoNotDispose] Resource resource) +{ + resource.Dispose(); // ERP046: the caller retains responsibility. +} +``` + +```csharp +[return: DoNotDispose] +Resource GetShared() => shared; + +var resource = GetShared(); +var alias = resource; +alias.Dispose(); // ERP046: the result is still borrowed through this alias. +``` + +Disposal or transfer of explicitly borrowed parameters, members, and results +is reported, including transfers into owning members, output parameters, and +owning returns, and `Interlocked.Exchange` into an owning member. This includes +`Dispose`, `Close`, `DisposeAsync`, `using`, and `await using`, including the +framework's async-disposal `ConfigureAwait` wrapper. Explicit property contracts +also apply to awaited `Task` and `ValueTask` results. Ordinary property +getters can return borrowed values; owning properties cannot. + +`DisposeAsync` calls count as cleanup only when they target the actual +`IAsyncDisposable` implementation or the framework's configured-disposal wrapper. +An unrelated method with that name does not discharge ERP044 or invent an ERP046 +violation. Recognizing cleanup still does not prove its asynchronous completion. + +Known completed-result wrappers (`Task.FromResult`, `ValueTask.FromResult`, +and `new ValueTask(T result)`) retain the result's borrowing through local +aliases and `await`, including framework `ConfigureAwait`. Disposing the task +object is not disposing its result. Returning a borrowed carried result through +an owning return, or disposing it after awaiting, still violates borrowing. + +Wrapping a borrowed resource cannot bypass an acquiring parameter contract. +`DoNotDispose` on `Task`/`ValueTask` parameters also preserves borrowing of +their awaited results. Source-inferred async consumers must actually consume the +result; a call that only disposes the Task wrapper is not result acquisition. + +Simple local aliases, assignments, and direct type-pattern bindings preserve borrowing. Replacing a local +or parameter with a new owned resource clears borrowing for that binding, not +for other aliases of the original resource. Conditional assignments are +conservative; arbitrary branching and escape propagation remain out of scope. +User-defined and dynamic conversions do not establish alias identity. A +conversion operator's explicit input and result contracts are checked separately. + +`DoNotDispose` describes the recipient's contract, not a global prohibition on +the actual owner disposing the resource. `NoOwnership` on parameters/members +and `KeepsOwnership` on results remain compatible aliases for borrowing. +Unannotated input parameters are not assumed to be borrowed: simple source +callees can be inferred to consume them. + +An [ownership-oblivious](ERP044.md#ownership-oblivious-apis) parameter or result +has no explicit ownership contract; that absence does not, by itself, forbid +disposal. It is different from an explicit `DoNotDispose` contract. Other +ERP046 checks, such as recognized use after disposal or transfer, still apply. + +## Configuration + +```ini +[*.cs] +dotnet_diagnostic.ERP046.severity = warning +``` diff --git a/src/ErrorProne.NET.Annotations/AnalyzerReleases.Unshipped.md b/src/ErrorProne.NET.Annotations/AnalyzerReleases.Unshipped.md new file mode 100644 index 0000000..45b94a3 --- /dev/null +++ b/src/ErrorProne.NET.Annotations/AnalyzerReleases.Unshipped.md @@ -0,0 +1,9 @@ +; Unshipped generator diagnostics + +### New Rules +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +EPANN001 | Configuration | Error | Annotation namespace is unavailable +EPANN002 | Configuration | Error | Annotation namespace is invalid +EPANN003 | Configuration | Error | Existing annotation definitions are ambiguous +EPANN004 | Configuration | Error | Annotation name is occupied by a non-attribute type diff --git a/src/ErrorProne.NET.Annotations/AnnotationsGenerator.cs b/src/ErrorProne.NET.Annotations/AnnotationsGenerator.cs new file mode 100644 index 0000000..8e12fca --- /dev/null +++ b/src/ErrorProne.NET.Annotations/AnnotationsGenerator.cs @@ -0,0 +1,175 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; + +namespace ErrorProne.NET.Annotations.Generation; + +[Generator(LanguageNames.CSharp)] +public sealed class AnnotationsGenerator : IIncrementalGenerator +{ + private static readonly DiagnosticDescriptor MissingNamespace = new( + "EPANN001", "Annotation namespace is unavailable", + "Set RootNamespace or ErrorProneAnnotationsNamespace and include the annotations package build assets", + "Configuration", DiagnosticSeverity.Error, isEnabledByDefault: true); + private static readonly DiagnosticDescriptor InvalidNamespace = new( + "EPANN002", "Annotation namespace is invalid", + "'{0}' is not a valid annotation namespace", "Configuration", DiagnosticSeverity.Error, isEnabledByDefault: true); + private static readonly DiagnosticDescriptor AmbiguousAttribute = new( + "EPANN003", "Existing annotation definitions are ambiguous", + "Multiple accessible definitions of '{0}' exist in {1}; set ErrorProneAnnotationsNamespace or remove the conflicting reference", + "Configuration", DiagnosticSeverity.Error, isEnabledByDefault: true); + private static readonly DiagnosticDescriptor OccupiedByNonAttribute = new( + "EPANN004", "Annotation name is occupied by a non-attribute type", + "'{0}' already exists in this assembly but does not derive from System.Attribute; rename it or set ErrorProneAnnotationsNamespace", + "Configuration", DiagnosticSeverity.Error, isEnabledByDefault: true); + private static readonly (string Name, string Targets, string Summary)[] Attributes = + { + ("AcquiresOwnershipAttribute", "Parameter", "Transfers cleanup responsibility to the receiving method."), + ("ReturnsOwnershipAttribute", "Method | Property | ReturnValue", "Transfers cleanup responsibility to the recipient of the result."), + ("DoNotDisposeAttribute", "Parameter | Field | Property | Method | ReturnValue", "Marks a borrowed value that the recipient must not dispose or transfer."), + ("KeepsOwnershipAttribute", "Method | Property | ReturnValue", "Compatibility annotation for a borrowed result; prefer DoNotDispose."), + ("NoOwnershipAttribute", "Parameter | Field | Property", "Compatibility annotation for borrowed parameters and members; prefer DoNotDispose."), + ("MustUseResultAttribute", "Method", "Requires the caller to observe the method result."), + ("MustUseReturnValueAttribute", "Method", "Compatibility annotation for a method result that must be observed; prefer MustUseResult."), + ("UseConfigureAwaitFalseAttribute", "Assembly", "Requires ConfigureAwait(false) for awaits in this assembly."), + ("DoNotUseConfigureAwaitAttribute", "Assembly", "Marks ConfigureAwait(false) as redundant for awaits in this assembly."), + }; + + public void Initialize(IncrementalGeneratorInitializationContext context) + { + context.RegisterSourceOutput(context.CompilationProvider.Combine(context.AnalyzerConfigOptionsProvider), static (output, input) => + { + var compilation = input.Left; + var options = input.Right.GlobalOptions; + if (!options.TryGetValue("build_property.ErrorProneAnnotationsNamespace", out var configuredNamespace) + || string.IsNullOrWhiteSpace(configuredNamespace)) + { + if (!options.TryGetValue("build_property.RootNamespace", out configuredNamespace)) + { + output.ReportDiagnostic(Diagnostic.Create(MissingNamespace, Location.None)); + return; + } + } + + if (!TryGetNamespace(configuredNamespace, out var sourceNamespace, out var metadataNamespace)) + { + output.ReportDiagnostic(Diagnostic.Create(InvalidNamespace, Location.None, configuredNamespace)); + return; + } + + // Accessibility alone does not make extern-alias-only types available to generated source. + var globalAssemblies = new HashSet(compilation.References + .Where(reference => reference.Properties.Aliases.IsDefaultOrEmpty + || reference.Properties.Aliases.Contains("global")) + .Select(reference => compilation.GetAssemblyOrModuleSymbol(reference)) + .OfType(), SymbolEqualityComparer.Default); + + foreach (var attribute in Attributes) + { + output.CancellationToken.ThrowIfCancellationRequested(); + var metadataName = metadataNamespace.Length == 0 + ? attribute.Name : metadataNamespace + "." + attribute.Name; + var existing = compilation.GetTypesByMetadataName(metadataName); + var localConflict = existing.FirstOrDefault(type => + SymbolEqualityComparer.Default.Equals(type.ContainingAssembly, compilation.Assembly) + && !IsAttributeType(type, compilation)); + if (localConflict != null) + { + output.ReportDiagnostic(Diagnostic.Create(OccupiedByNonAttribute, + localConflict.Locations.FirstOrDefault(location => location.IsInSource) ?? Location.None, + metadataName)); + continue; + } + + IEnumerable reusableCandidates = + existing.Where(type => IsAttributeType(type, compilation)); + if (attribute.Name is "UseConfigureAwaitFalseAttribute" or "DoNotUseConfigureAwaitAttribute") + { + // These assembly policies also recognize legacy attribute classes without the suffix. + var legacyName = metadataName.Substring(0, metadataName.Length - nameof(Attribute).Length); + reusableCandidates = reusableCandidates.Concat(compilation.GetTypesByMetadataName(legacyName) + .Where(type => IsAttributeType(type, compilation))); + } + + if (reusableCandidates.Any(type => SymbolEqualityComparer.Default.Equals(type.ContainingAssembly, compilation.Assembly))) + { + continue; + } + + var accessible = reusableCandidates.Where(type => globalAssemblies.Contains(type.ContainingAssembly) + && compilation.IsSymbolAccessibleWithin(type, compilation.Assembly)) + .Distinct(SymbolEqualityComparer.Default).ToArray(); + if (accessible.Length > 1) + { + var assemblies = string.Join(", ", accessible.Select(type => type.ContainingAssembly.Identity.ToString()) + .OrderBy(name => name, StringComparer.Ordinal)); + output.ReportDiagnostic(Diagnostic.Create(AmbiguousAttribute, Location.None, metadataName, assemblies)); + continue; + } + + if (accessible.Length == 1) + { + continue; + } + + // Internal copies stay local to each assembly, but their usages on public APIs + // remain visible to downstream analyzers through metadata. + var targets = "global::System.AttributeTargets." + + attribute.Targets.Replace(" | ", " | global::System.AttributeTargets."); + var namespaceStart = sourceNamespace.Length == 0 ? "" : "namespace " + sourceNamespace + "\n{\n"; + var namespaceEnd = sourceNamespace.Length == 0 ? "" : "}\n"; + var source = "// \n" + namespaceStart + @" + /// " + attribute.Summary + @" + [global::System.AttributeUsage(" + targets + @", AllowMultiple = false, Inherited = true)] + internal sealed class " + attribute.Name + @" : global::System.Attribute + { + } +" + namespaceEnd; + output.AddSource(attribute.Name + ".g.cs", SourceText.From(source, Encoding.UTF8)); + } + }); + } + + private static bool IsAttributeType(INamedTypeSymbol type, Compilation compilation) + { + var attributeType = compilation.GetTypeByMetadataName("System.Attribute"); + for (INamedTypeSymbol? current = type; current != null; current = current.BaseType) + { + if (SymbolEqualityComparer.Default.Equals(current, attributeType)) + { + return true; + } + } + + return false; + } + + private static bool TryGetNamespace(string value, out string sourceNamespace, out string metadataNamespace) + { + value = value.Trim(); + sourceNamespace = metadataNamespace = ""; + if (value.Length == 0) + { + return true; + } + + var escaped = string.Join(".", value.Split('.').Select(part => + SyntaxFacts.GetKeywordKind(part) != SyntaxKind.None ? "@" + part : part)); + var name = SyntaxFactory.ParseName(escaped); + if (name.ContainsDiagnostics || name.DescendantNodesAndSelf().Any(node => node is AliasQualifiedNameSyntax or GenericNameSyntax)) + { + return false; + } + + var identifiers = name.DescendantNodesAndSelf().OfType() + .Select(identifier => identifier.Identifier).ToArray(); + sourceNamespace = string.Join(".", identifiers.Select(identifier => identifier.Text)); + metadataNamespace = string.Join(".", identifiers.Select(identifier => identifier.ValueText)); + return identifiers.Length > 0; + } +} diff --git a/src/ErrorProne.NET.Annotations/ErrorProne.NET.Annotations.csproj b/src/ErrorProne.NET.Annotations/ErrorProne.NET.Annotations.csproj new file mode 100644 index 0000000..48c4891 --- /dev/null +++ b/src/ErrorProne.NET.Annotations/ErrorProne.NET.Annotations.csproj @@ -0,0 +1,27 @@ + + + netstandard2.0 + ErrorProne.Net.Annotations + ErrorProne.NET.Annotations + true + false + false + true + true + ErrorProne.Net.Annotations + Sergey Teplyakov + Source-embedded ErrorProne.NET annotations without a runtime assembly dependency. + MIT + https://github.com/SergeyTeplyakov/ErrorProne.NET + README.md + + + + + + + + + + + diff --git a/src/ErrorProne.NET.Annotations/README.md b/src/ErrorProne.NET.Annotations/README.md new file mode 100644 index 0000000..262032c --- /dev/null +++ b/src/ErrorProne.NET.Annotations/README.md @@ -0,0 +1,133 @@ +# ErrorProne.Net.Annotations + +Build-time source generation for ErrorProne.NET attributes, with **no additional +runtime DLL**. The generator embeds internal attribute types directly into each +project that references this package. + +```xml + +``` + +Replace `YOUR_VERSION` with the package version you consume. The generator +requires a Roslyn 4.13 or newer C# compiler. Its generated source supports C# 7.3 +and newer. Reference the analyzer package separately to enable diagnostics. + +```csharp +// With RootNamespace = Azure.Core: +namespace Azure.Core +{ + public interface IResources + { + [return: ReturnsOwnership] + System.IDisposable Open(); + + [return: DoNotDispose] + System.IDisposable GetShared(); + } +} +``` + +## Namespace configuration + +Types are generated directly in the consuming project's `RootNamespace`, not +necessarily its assembly name. Nested namespaces can use the attributes without +adding another namespace import. To override only the annotation namespace: + +```xml + + Azure.Core.InternalAnnotations + +``` + +An empty override uses `RootNamespace`. An explicitly empty `RootNamespace` +generates types in the global namespace. Invalid namespaces produce EPANN002; +missing namespace build configuration produces EPANN001. Include the package's +build assets so these properties reach the compiler. + +## Public contracts without public attribute types + +Internal attributes can annotate public APIs. Their usages remain in both the +compiled library and its reference assembly. A downstream Roslyn analyzer reads +these contracts from metadata; consumer code does not need access to the +attribute class or a runtime reference to the generator. + +The attributes are not conditional and are not stripped from metadata. +Each consuming project that wants to annotate its own code references this +package directly. `PrivateAssets="all"` prevents the generator package from +becoming a transitive dependency of your library. + +No public, shared runtime identity for these attribute types is promised. +Consumers should not depend on using `typeof` or generic reflection APIs with +another assembly's internal attribute types. Metadata-based analysis works +without such access. + +## Generated attributes + +The generator provides: + +- `AcquiresOwnership`: consuming parameters. +- `ReturnsOwnership`: owning method and property results. +- `DoNotDispose`: borrowed parameters, fields, properties, and results. +- `KeepsOwnership` and `NoOwnership`: compatibility ownership names. +- `MustUseResult`: method results that must be observed. +- `MustUseReturnValue`: a recognized compatibility name for `MustUseResult`. +- `UseConfigureAwaitFalse`: assembly-wide policy requiring ConfigureAwait. +- `DoNotUseConfigureAwait`: assembly-wide policy marking `ConfigureAwait(false)` as redundant. + +### Non-ownership annotations + +These annotations work independently of disposable ownership: + +```csharp +// With RootNamespace = MyProject, choose the assembly policy appropriate for the project: +[assembly: MyProject.UseConfigureAwaitFalse] +// Alternatively: [assembly: MyProject.DoNotUseConfigureAwait] + +namespace MyProject +{ + public static class Validation + { + [MustUseResult] + public static bool IsValid(string value) => !string.IsNullOrEmpty(value); + } +} +``` + +Ignoring an annotated method's result reports [EPC34](../../docs/Rules/EPC34.md), +including an ignored awaited result. `MustUseResult` is the preferred name; +`MustUseReturnValue` has the same analyzer behavior. Assembly-level policies +enable [EPC15](../../docs/Rules/EPC15.md) or [EPC14](../../docs/Rules/EPC14.md), +respectively. Generating the attribute types alone does not select a policy: +apply the assembly attribute explicitly. + +All of these types are internal and source-embedded, just like the ownership +attributes. They add no runtime DLL dependency. + +## Existing definitions + +If an accessible attribute type with the same fully qualified name already +exists, its definition is used instead of generating a duplicate. Inaccessible +internal definitions in other assemblies do not prevent generating a local copy, +and referenced non-attribute types with the same name do not suppress local +generation. +For the ConfigureAwait assembly policies, recognized legacy attribute classes +named `UseConfigureAwaitFalse` or `DoNotUseConfigureAwait` without the `Attribute` +suffix are also reused to avoid ambiguous attribute names. +Definitions reachable only through an `extern alias` also do not prevent local +generation, since the generated source cannot use those types by their namespace. +Friend/test assemblies can omit the generator when `InternalsVisibleTo` already +makes the required annotations available. + +If multiple referenced definitions are accessible and ambiguous, EPANN003 +requests a namespace override or removal of the conflicting reference. The +generator does not guess which assembly to reuse, silently change namespace, +or switch to public attribute types. If the current assembly already declares a +same-name non-attribute type in the target namespace, EPANN004 reports that +local conflict instead of generating a duplicate attribute declaration. + +This package does not add type-level ownership markers or generate BCL +nullability attributes. Existing application-defined and legacy ErrorProne.NET +attribute names remain recognized by the analyzers. diff --git a/src/ErrorProne.NET.Annotations/buildTransitive/ErrorProne.Net.Annotations.props b/src/ErrorProne.NET.Annotations/buildTransitive/ErrorProne.Net.Annotations.props new file mode 100644 index 0000000..4adb6bf --- /dev/null +++ b/src/ErrorProne.NET.Annotations/buildTransitive/ErrorProne.Net.Annotations.props @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/ErrorProne.NET.CoreAnalyzers.CodeFixes/ErrorProne.NET.CoreAnalyzers.CodeFixes.csproj b/src/ErrorProne.NET.CoreAnalyzers.CodeFixes/ErrorProne.NET.CoreAnalyzers.CodeFixes.csproj index 49a6aa2..8cd35a2 100644 --- a/src/ErrorProne.NET.CoreAnalyzers.CodeFixes/ErrorProne.NET.CoreAnalyzers.CodeFixes.csproj +++ b/src/ErrorProne.NET.CoreAnalyzers.CodeFixes/ErrorProne.NET.CoreAnalyzers.CodeFixes.csproj @@ -19,6 +19,10 @@ Core .NET analyzers for detecting the most common coding issues 0.9.0 + * Add ERP044: track disposable ownership through attributes and bounded source inference. + * Add ERP045: validate external ownership contracts supplied through .ownership.xml files. + * Add ERP046: warn on obvious use after disposal/transfer and misuse of borrowed values. + * Support source-embedded contracts from ErrorProne.Net.Annotations without a runtime annotation DLL. * Add EPC38: TaskEnumerableReEnumerationAnalyzer (disabled by default). * Add EPC39: QuadraticEnumerationAnalyzer (disabled by default). * Add EPC40: PrivateMethodMultipleEnumerationAnalyzer (disabled by default). diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/Annotations/AnnotationsGeneratorTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/Annotations/AnnotationsGeneratorTests.cs new file mode 100644 index 0000000..5cb7fad --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/Annotations/AnnotationsGeneratorTests.cs @@ -0,0 +1,490 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using ErrorProne.NET.Annotations.Generation; +using ErrorProne.NET.AsyncAnalyzers; +using ErrorProne.NET.DisposableAnalyzers; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Emit; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.Annotations; + +[TestFixture] +public sealed class AnnotationsGeneratorTests +{ + private const int AnnotationCount = 9; + + private const string LibrarySource = @" +[assembly: Library.UseConfigureAwaitFalse] +namespace Library +{ + public sealed class Resource : System.IDisposable { public void Dispose() { } } + public static class Api + { + [return: ReturnsOwnership] public static Resource Create() { return new Resource(); } + [return: DoNotDispose] public static Resource GetShared() { return null; } + public static void Take([AcquiresOwnership] Resource resource) { resource.Dispose(); } + [MustUseResult] public static int Observe() { return 42; } + [MustUseReturnValue] public static int ObserveAlias() { return 42; } + } +}"; + + private static async Task CreateCompilation(string name, string source, + LanguageVersion languageVersion = LanguageVersion.CSharp7_3, MetadataReference? reference = null) + { + var references = await ReferenceAssemblies.Net.Net80.ResolveAsync(LanguageNames.CSharp, CancellationToken.None); + if (reference != null) + { + references = references.Add(reference); + } + + return CSharpCompilation.Create(name, + new[] { CSharpSyntaxTree.ParseText(source, new CSharpParseOptions(languageVersion)) }, + references, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + } + + private static GeneratorDriver CreateDriver(CSharpCompilation compilation, string? rootNamespace = "Library", + string? namespaceOverride = null) + { + return CSharpGeneratorDriver.Create( + new[] { new AnnotationsGenerator().AsSourceGenerator() }, + parseOptions: (CSharpParseOptions)compilation.SyntaxTrees.Single().Options, + optionsProvider: new OptionsProvider(rootNamespace, namespaceOverride)); + } + + private static CSharpCompilation Generate(CSharpCompilation compilation, int expectedCount, + string rootNamespace = "Library", string? namespaceOverride = null) + { + var driver = CreateDriver(compilation, rootNamespace, namespaceOverride); + driver = driver.RunGeneratorsAndUpdateCompilation(compilation, out var updated, out var diagnostics); + Assert.That(diagnostics, Is.Empty); + Assert.That(driver.GetRunResult().GeneratedTrees.Length, Is.EqualTo(expectedCount)); + Assert.That(updated.GetDiagnostics().Where(d => d.Severity >= DiagnosticSeverity.Warning), Is.Empty); + return (CSharpCompilation)updated; + } + + private static PortableExecutableReference Emit(CSharpCompilation compilation, bool referenceAssembly) + { + using var image = new MemoryStream(); + var result = compilation.Emit(image, + options: new EmitOptions(metadataOnly: referenceAssembly, includePrivateMembers: !referenceAssembly)); + Assert.That(result.Success, Is.True, string.Join("\n", result.Diagnostics)); + return MetadataReference.CreateFromImage(image.ToArray()); + } + + [TestCase(LanguageVersion.CSharp7_3)] + [TestCase(LanguageVersion.Latest)] + public async Task Generates_Internal_Annotations_Without_Runtime_Dependencies(LanguageVersion languageVersion) + { + var compilation = Generate(await CreateCompilation("Library", LibrarySource, languageVersion), AnnotationCount); + foreach (var name in new[] { "AcquiresOwnership", "ReturnsOwnership", "DoNotDispose", + "KeepsOwnership", "NoOwnership", "MustUseResult", "MustUseReturnValue", + "UseConfigureAwaitFalse", "DoNotUseConfigureAwait" }) + { + var attribute = compilation.GetTypeByMetadataName("Library." + name + "Attribute"); + Assert.That(attribute, Is.Not.Null); + Assert.That(attribute!.DeclaredAccessibility, Is.EqualTo(Accessibility.Internal)); + Assert.That(attribute.GetAttributes().Any(a => a.AttributeClass?.Name == "ConditionalAttribute"), Is.False); + } + + var reference = Emit(compilation, referenceAssembly: false); + var consumer = await CreateCompilation("Consumer", "", reference: reference); + var library = (IAssemblySymbol)consumer.GetAssemblyOrModuleSymbol(reference)!; + Assert.That(library.Modules.SelectMany(m => m.ReferencedAssemblySymbols) + .Any(a => a.Name.StartsWith("ErrorProne")), Is.False); + } + + [Test] + public async Task Does_Not_Duplicate_An_Existing_Attribute() + { + var compilation = await CreateCompilation("Library", @" +namespace Library +{ + internal sealed class DoNotDisposeAttribute : System.Attribute { } +}"); + Generate(compilation, AnnotationCount - 1); + } + + [TestCase("UseConfigureAwaitFalse")] + [TestCase("DoNotUseConfigureAwait")] + public async Task Reuses_An_Existing_Unsuffixed_ConfigureAwait_Attribute(string name) + { + var compilation = await CreateCompilation("Library", @" +[assembly: Library." + name + @"] +namespace Library +{ + [System.AttributeUsage(System.AttributeTargets.Assembly)] + internal sealed class " + name + @" : System.Attribute { } +}"); + Generate(compilation, AnnotationCount - 1); + } + + [Test] + public async Task Does_Not_Treat_An_Unsuffixed_NonAttribute_Class_As_An_Annotation() + { + var compilation = await CreateCompilation("Library", @" +[assembly: Library.DoNotUseConfigureAwait] +namespace Library +{ + internal sealed class DoNotUseConfigureAwait { } +}"); + Generate(compilation, AnnotationCount); + } + + [Test] + public async Task Generates_A_Local_Attribute_When_Only_A_Referenced_NonAttribute_Exists() + { + var existing = await CreateCompilation("ExistingAnnotations", @" +namespace Library +{ + public sealed class DoNotDisposeAttribute { } +}"); + var compilation = await CreateCompilation("Consumer", @" +namespace Library +{ + public class Api + { + [return: DoNotDispose] + public System.IDisposable GetShared() { return null; } + } +}", reference: Emit(existing, referenceAssembly: false)); + var driver = CreateDriver(compilation).RunGeneratorsAndUpdateCompilation(compilation, + out var updated, out var diagnostics); + + Assert.That(diagnostics, Is.Empty); + Assert.That(driver.GetRunResult().Diagnostics, Is.Empty); + Assert.That(driver.GetRunResult().GeneratedTrees.Length, Is.EqualTo(AnnotationCount)); + Assert.That(updated.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error), Is.Empty); + Assert.That(updated.GetTypeByMetadataName("Library.DoNotDisposeAttribute")! + .ContainingAssembly.Name, Is.EqualTo("Consumer")); + } + + [Test] + public async Task Reports_Local_NonAttribute_That_Occupies_The_Canonical_Annotation_Name() + { + var compilation = await CreateCompilation("Library", @" +namespace Library +{ + internal sealed class DoNotDisposeAttribute { } + + public class Api + { + [return: DoNotDispose] + public System.IDisposable GetShared() { return null; } + } +}"); + var driver = CreateDriver(compilation).RunGeneratorsAndUpdateCompilation(compilation, + out var updated, out var diagnostics); + + Assert.That(diagnostics.Select(d => d.Id), Is.EqualTo(new[] { "EPANN004" })); + Assert.That(driver.GetRunResult().Diagnostics.Select(d => d.Id), Is.EqualTo(new[] { "EPANN004" })); + Assert.That(driver.GetRunResult().GeneratedTrees.Length, Is.EqualTo(AnnotationCount - 1)); + Assert.That(updated.GetDiagnostics().Select(d => d.Id), Does.Contain("CS0616")); + Assert.That(updated.GetDiagnostics().Select(d => d.Id), Does.Not.Contain("CS0101")); + } + + [Test] + public async Task Local_NonAttribute_Takes_Precedence_Over_A_Referenced_Attribute() + { + var reference = Emit(await CreateCompilation("ExistingAnnotations", @" +namespace Library +{ + public sealed class DoNotDisposeAttribute : System.Attribute { } +}"), referenceAssembly: false); + var compilation = await CreateCompilation("Consumer", @" +namespace Library +{ + internal sealed class DoNotDisposeAttribute { } +}", reference: reference); + var driver = CreateDriver(compilation).RunGenerators(compilation); + + Assert.That(driver.GetRunResult().Diagnostics.Select(d => d.Id), Is.EqualTo(new[] { "EPANN004" })); + Assert.That(driver.GetRunResult().GeneratedTrees.Length, Is.EqualTo(AnnotationCount - 1)); + } + + [Test] + public async Task Nested_And_Generic_NonAttributes_Do_Not_Block_Generation() + { + var compilation = await CreateCompilation("Library", @" +namespace Library +{ + internal sealed class Container + { + internal sealed class DoNotDisposeAttribute { } + } + + internal sealed class DoNotDisposeAttribute { } +}"); + Generate(compilation, AnnotationCount); + } + + [TestCase(null, AnnotationCount - 1, "ExistingAnnotations")] + [TestCase("Existing", AnnotationCount, "Consumer")] + [TestCase("global,Existing", AnnotationCount - 1, "ExistingAnnotations")] + public async Task Reuses_Referenced_Attributes_Only_When_Globally_Accessible( + string? aliases, int expectedCount, string expectedAssembly) + { + var library = await CreateCompilation("ExistingAnnotations", @" +namespace Library +{ + public sealed class DoNotDisposeAttribute : System.Attribute { } +}"); + var reference = Emit(library, referenceAssembly: false); + if (aliases != null) + { + reference = reference.WithAliases(aliases.Split(',')); + } + + var consumer = await CreateCompilation("Consumer", @" +namespace Library +{ + public class Api + { + [return: DoNotDispose] + public System.IDisposable GetShared() { return null; } + } +}", reference: reference); + var generated = Generate(consumer, expectedCount); + var attribute = generated.GetTypeByMetadataName("Library.DoNotDisposeAttribute")!; + Assert.That(attribute.ContainingAssembly.Name, Is.EqualTo(expectedAssembly)); + } + + [TestCase(false)] + [TestCase(true)] + public async Task Internal_Annotations_On_Public_APIs_Survive_Assembly_Boundaries(bool referenceAssembly) + { + var library = Generate(await CreateCompilation("Library", LibrarySource), AnnotationCount); + var reference = Emit(library, referenceAssembly); + var consumer = await CreateCompilation("Consumer", @" +public class UseLibrary +{ + public void Run() + { + var owned = Library.Api.Create(); + var borrowed = Library.Api.GetShared(); + borrowed.Dispose(); + var transferred = Library.Api.Create(); + Library.Api.Take(transferred); + transferred.Dispose(); + Library.Api.Observe(); + Library.Api.ObserveAlias(); + } +}", reference: reference); + Assert.That(consumer.GetDiagnostics().Where(d => d.Severity >= DiagnosticSeverity.Warning), Is.Empty); + + var annotations = consumer.GetTypeByMetadataName("Library.Api")!.GetMembers("Create") + .OfType().Single().GetReturnTypeAttributes(); + Assert.That(annotations.Any(a => a.AttributeClass?.ToDisplayString() + == "Library.ReturnsOwnershipAttribute"), Is.True); + var librarySymbol = (IAssemblySymbol)consumer.GetAssemblyOrModuleSymbol(reference)!; + Assert.That(librarySymbol.GetAttributes().Any(a => a.AttributeClass?.Name + == "UseConfigureAwaitFalseAttribute"), Is.True); + + var diagnostics = await consumer.WithAnalyzers(ImmutableArray.Create( + new DisposeBeforeLosingScopeAnalyzer(), new MustUseResultAnalyzer())).GetAnalyzerDiagnosticsAsync(); + Assert.That(diagnostics.Select(d => d.Id).OrderBy(id => id), + Is.EqualTo(new[] { "EPC34", "EPC34", "ERP044", "ERP046", "ERP046" })); + + // The consumer can also annotate its own API without reusing inaccessible library types. + var annotatedConsumer = await CreateCompilation("AnnotatedConsumer", @" +namespace Library +{ +public class ConsumerApi +{ + [return: DoNotDispose] + public System.IDisposable GetShared() { return Library.Api.GetShared(); } +} +}", reference: reference); + Generate(annotatedConsumer, AnnotationCount); + } + + [TestCase("MustUseResult")] + [TestCase("MustUseReturnValue")] + public async Task Generated_Result_Annotations_Require_Observing_Sync_And_Async_Results(string attribute) + { + var compilation = Generate(await CreateCompilation("Library", @" +using System.Threading.Tasks; +namespace Library +{ + public static class Api + { + [" + attribute + @"] public static int Observe() { return 42; } + [" + attribute + @"] public static Task ObserveAsync() { return Task.FromResult(42); } + public static async Task Run() + { + Observe(); + _ = Observe(); + await ObserveAsync(); + _ = await ObserveAsync(); + } + } +}"), AnnotationCount); + + var diagnostics = await compilation.WithAnalyzers(ImmutableArray.Create( + new MustUseResultAnalyzer())).GetAnalyzerDiagnosticsAsync(); + Assert.That(diagnostics.Select(d => d.Id), Is.EqualTo(new[] { "EPC34", "EPC34" })); + } + + [TestCase(null, null)] + [TestCase("UseConfigureAwaitFalse", "EPC15")] + [TestCase("DoNotUseConfigureAwait", "EPC14")] + public async Task Generated_Assembly_Annotations_Configure_Await_Policy(string? attribute, string? expectedDiagnostic) + { + var assemblyAttribute = attribute == null ? "" : "[assembly: Library." + attribute + "]\n"; + var compilation = Generate(await CreateCompilation("Library", "using System.Threading.Tasks;\n" + assemblyAttribute + @" +namespace Library +{ + public static class Api + { + public static async Task Run() + { + await Task.Delay(1); + await Task.Delay(1).ConfigureAwait(false); + } + } +}"), AnnotationCount); + + var diagnostics = await compilation.WithAnalyzers(ImmutableArray.Create( + new ConfigureAwaitRequiredAnalyzer(), new RedundantConfigureAwaitFalseAnalyzer())).GetAnalyzerDiagnosticsAsync(); + Assert.That(diagnostics.Select(d => d.Id), Is.EqualTo(expectedDiagnostic == null + ? System.Array.Empty() : new[] { expectedDiagnostic })); + } + + [TestCase("MustUseResult", System.AttributeTargets.Method)] + [TestCase("MustUseReturnValue", System.AttributeTargets.Method)] + [TestCase("UseConfigureAwaitFalse", System.AttributeTargets.Assembly)] + [TestCase("DoNotUseConfigureAwait", System.AttributeTargets.Assembly)] + public async Task Generated_Non_Ownership_Annotations_Have_Expected_Targets( + string name, System.AttributeTargets expectedTargets) + { + var compilation = Generate(await CreateCompilation("Library", ""), AnnotationCount); + var attribute = compilation.GetTypeByMetadataName("Library." + name + "Attribute")!; + var usage = attribute.GetAttributes().Single(a => a.AttributeClass?.Name == "AttributeUsageAttribute"); + Assert.That(usage.ConstructorArguments.Single().Value, Is.EqualTo((int)expectedTargets)); + } + + [TestCase("Azure.Core", null, "Azure.Core")] + [TestCase("Azure.Core", "Azure.Core.InternalAnnotations", "Azure.Core.InternalAnnotations")] + [TestCase("Azure.Core", "", "Azure.Core")] + [TestCase("", null, "")] + [TestCase("class.namespace", null, "class.namespace")] + public async Task Uses_Configured_Namespace(string rootNamespace, string? namespaceOverride, string expected) + { + var compilation = Generate(await CreateCompilation("DifferentAssemblyName", ""), AnnotationCount, rootNamespace, namespaceOverride); + var prefix = expected.Length == 0 ? "" : expected + "."; + Assert.That(compilation.GetTypeByMetadataName(prefix + "DoNotDisposeAttribute"), Is.Not.Null); + } + + [TestCase(null, "EPANN001")] + [TestCase("Azure..Core", "EPANN002")] + [TestCase("Azure-Core", "EPANN002")] + [TestCase("global::Azure.Core", "EPANN002")] + [TestCase("Azure.Core", "EPANN002")] + public async Task Reports_Missing_Or_Invalid_Namespace(string? rootNamespace, string expectedDiagnostic) + { + var compilation = await CreateCompilation("Library", ""); + var driver = CreateDriver(compilation, rootNamespace).RunGenerators(compilation); + Assert.That(driver.GetRunResult().Diagnostics.Select(d => d.Id), Is.EqualTo(new[] { expectedDiagnostic })); + Assert.That(driver.GetRunResult().GeneratedTrees, Is.Empty); + } + + private static async Task CreateFriendLibrary(string name) + { + var compilation = await CreateCompilation(name, @" +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo(""Consumer"")] +namespace Azure.Core +{ + internal sealed class DoNotDisposeAttribute : System.Attribute { } +}"); + return Emit(compilation, referenceAssembly: false); + } + + [Test] + public async Task Friend_Assembly_Can_Omit_The_Generator() + { + var compilation = await CreateCompilation("Consumer", @" +namespace Azure.Core +{ + public class Api + { + [return: DoNotDispose] + public System.IDisposable GetShared() { return null; } + } +}", reference: await CreateFriendLibrary("Library")); + Assert.That(compilation.GetDiagnostics().Where(d => d.Severity >= DiagnosticSeverity.Warning), Is.Empty); + } + + [Test] + public async Task Reuses_Accessible_Friend_Attribute_Instead_Of_Duplicating_It() + { + var compilation = await CreateCompilation("Consumer", "", reference: await CreateFriendLibrary("Library")); + var generated = Generate(compilation, AnnotationCount - 1, rootNamespace: "Azure.Core"); + Assert.That(generated.GetTypeByMetadataName("Azure.Core.DoNotDisposeAttribute")! + .ContainingAssembly.Name, Is.EqualTo("Library")); + } + + [Test] + public async Task Ambiguous_Friend_Attributes_Require_An_Explicit_Namespace_Override() + { + var compilation = (await CreateCompilation("Consumer", "")) + .AddReferences(await CreateFriendLibrary("FirstLibrary"), await CreateFriendLibrary("SecondLibrary")); + var driver = CreateDriver(compilation, "Azure.Core").RunGenerators(compilation); + Assert.That(driver.GetRunResult().Diagnostics.Select(d => d.Id), Is.EqualTo(new[] { "EPANN003" })); + Assert.That(driver.GetRunResult().GeneratedTrees.Length, Is.EqualTo(AnnotationCount - 1)); + + var generated = Generate(compilation, AnnotationCount, "Azure.Core", "Azure.Core.InternalAnnotations"); + Assert.That(generated.GetTypeByMetadataName("Azure.Core.InternalAnnotations.DoNotDisposeAttribute")! + .ContainingAssembly.Name, Is.EqualTo("Consumer")); + } + + [Test] + public async Task Local_Definition_Takes_Precedence_Over_Ambiguous_Friend_Attributes() + { + var compilation = (await CreateCompilation("Consumer", @" +namespace Azure.Core +{ + internal sealed class DoNotDisposeAttribute : System.Attribute { } +}")).AddReferences(await CreateFriendLibrary("FirstLibrary"), await CreateFriendLibrary("SecondLibrary")); + var generated = Generate(compilation, AnnotationCount - 1, rootNamespace: "Azure.Core"); + Assert.That(generated.GetTypeByMetadataName("Azure.Core.DoNotDisposeAttribute")! + .ContainingAssembly.Name, Is.EqualTo("Consumer")); + } + + private sealed class OptionsProvider : AnalyzerConfigOptionsProvider + { + private readonly Options _options; + public OptionsProvider(string? rootNamespace, string? namespaceOverride) + { + var values = new Dictionary(); + if (rootNamespace != null) values["build_property.RootNamespace"] = rootNamespace; + if (namespaceOverride != null) values["build_property.ErrorProneAnnotationsNamespace"] = namespaceOverride; + _options = new Options(values); + } + public override AnalyzerConfigOptions GlobalOptions => _options; + public override AnalyzerConfigOptions GetOptions(SyntaxTree tree) => _options; + public override AnalyzerConfigOptions GetOptions(AdditionalText textFile) => _options; + } + + private sealed class Options : AnalyzerConfigOptions + { + private readonly IReadOnlyDictionary _values; + public Options(IReadOnlyDictionary values) { _values = values; } + public override bool TryGetValue(string key, out string value) + { + if (_values.TryGetValue(key, out var found)) + { + value = found; + return true; + } + value = ""; + return false; + } + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.AdoptionBoundaries.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.AdoptionBoundaries.cs new file mode 100644 index 0000000..36bbb85 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.AdoptionBoundaries.cs @@ -0,0 +1,155 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + // These characterize v1 boundaries, including known noise and missed obligations. + // A passing test here does not mean the example has a safe resource lifetime. + [Test] + public Task Adoption_Task_Publication_Transfers_The_Carried_Resource() + { + return VerifyAsync(@" +public class Test +{ + [return: ReturnsOwnership] + private static System.Threading.Tasks.Task CreateAsync() + { + var item = new Disposable(); + return System.Threading.Tasks.Task.FromResult(item); + } + + public async System.Threading.Tasks.Task RunAsync() + { + using var item = await CreateAsync(); + } +}"); + } + + [Test] + public Task Adoption_Unannotated_Collection_Makes_Element_Ownership_Unknown() + { + return VerifyAsync(@" +public sealed class Test : System.IDisposable +{ + private readonly System.Collections.Generic.List items = new(); + + public void Add() + { + var item = new Disposable(); + items.Add(item); + } + + public void Dispose() + { + foreach (var item in items) + item.Dispose(); + items.Clear(); + } +}"); + } + + [Test] + public Task Adoption_Shutdown_Capture_Makes_Local_Ownership_Unknown() + { + return VerifyAsync(@" +public class Test +{ + public void Register() + { + var item = new Disposable(); + System.AppDomain.CurrentDomain.ProcessExit += (sender, args) => item.Dispose(); + } +}"); + } + + [Test] + public Task Adoption_Lambda_Local_Leak_Currently_Is_Not_Reported() + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + System.Action callback = () => + { + var item = new Disposable(); + item.ToString(); + }; + callback(); + } +}"); + } + + [Test] + public Task Adoption_Conditional_Cleanup_Currently_Discharges_All_Paths() + { + return VerifyAsync(@" +public class Test +{ + public void Run(bool shouldDispose) + { + var item = new Disposable(); + if (shouldDispose) + item.Dispose(); + } +}"); + } + + [Test] + public Task Adoption_Member_Transfer_Currently_Does_Not_Require_Owner_Teardown() + { + return VerifyAsync(@" +public sealed class Test : System.IDisposable +{ + private readonly Disposable item; + + public Test() + { + item = new Disposable(); + } + + public void Dispose() { } +}"); + } + + [Test] + public Task Adoption_Discarded_Async_Cleanup_Currently_Discharges_Ownership() + { + return VerifyAsync(@" +public sealed class AsyncResource : System.IAsyncDisposable +{ + public async System.Threading.Tasks.ValueTask DisposeAsync() + { + await System.Threading.Tasks.Task.Yield(); + } +} + +public class Test +{ + public void Run() + { + var item = new AsyncResource(); + _ = item.DisposeAsync(); + } +}"); + } + + [TestCase("var {|ERP044:item|} = Create();")] + [TestCase("using var item = Create();")] + [TestCase("Consume(Create());")] + public Task Adoption_Explicit_Owned_Result_Respects_Cleanup_And_Transfer(string body) + { + return VerifyAsync(@" +public class Test +{ + [return: ReturnsOwnership] + private static Disposable Create() => new Disposable(); + + private static void Consume([AcquiresOwnership] Disposable item) => item.Dispose(); + + public void Run() { " + body + @" } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.Borrowing.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.Borrowing.cs new file mode 100644 index 0000000..90afe46 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.Borrowing.cs @@ -0,0 +1,189 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("[return: DoNotDispose]")] + [TestCase("[DoNotDispose]")] + [TestCase("[KeepsOwnership]")] + public Task Borrowed_Returns_Do_Not_Cancel_Acquired_Input_Obligations(string attribute) + { + return VerifyAsync(@" +public class Test +{ + public void Run([DoNotDispose] Disposable borrowed) + { + var owned = new Disposable(); + var result = BorrowOther(owned, borrowed); + } + + " + attribute + @" + private static Disposable BorrowOther( + [AcquiresOwnership] Disposable {|ERP044:owned|}, + [DoNotDispose] Disposable borrowed) => borrowed; + + " + attribute + @" + private static Disposable DisposeAndBorrow( + [AcquiresOwnership] Disposable owned, + [DoNotDispose] Disposable borrowed) + { + owned.Dispose(); + return borrowed; + } + + " + attribute + @" + private static Disposable ReturnTheInput([AcquiresOwnership] Disposable owned) => owned; + + " + attribute + @" + private static Disposable BorrowOnly([DoNotDispose] Disposable borrowed) => borrowed; +}"); + } + + [Test] + public Task Borrowed_Returns_Still_Check_Each_Acquired_Input() + { + return VerifyAsync(@" +public class Test +{ + [return: DoNotDispose] + public Disposable BorrowOther( + [AcquiresOwnership] Disposable first, + [AcquiresOwnership] Disposable {|ERP044:second|}, + [DoNotDispose] Disposable borrowed) + { + first.Dispose(); + return borrowed; + } +}"); + } + + [TestCase("{|ERP046:GetShared()|}.Dispose();")] + [TestCase("var resource = GetShared(); {|ERP046:resource|}.Dispose();")] + [TestCase("var resource = GetShared(); {|ERP046:resource|}?.Dispose();")] + [TestCase("var resource = GetShared(); var alias = resource; {|ERP046:alias|}.Close();")] + [TestCase("using var resource = {|ERP046:GetShared()|};")] + [TestCase("using (var resource = {|ERP046:GetShared()|}) { }")] + [TestCase("var resource = GetShared(); using ({|ERP046:resource|}) { }")] + [TestCase("var resource = GetShared(); using var alias = {|ERP046:resource|};")] + [TestCase("var resource = GetShared(); Consume({|ERP046:resource|});")] + [TestCase("var resource = GetShared(); resource.ToString();")] + [TestCase("var resource = GetShared(); resource = new Disposable(); resource.Dispose();")] + [TestCase("var resource = GetShared(); var alias = resource; resource = new Disposable(); resource.Dispose(); {|ERP046:alias|}.Dispose();")] + [TestCase("var resource = new Disposable(); resource.Dispose(); resource = GetShared(); {|ERP046:resource|}.Dispose();")] + [TestCase("if (condition) { var resource = GetShared(); {|ERP046:resource|}.Dispose(); }")] + [TestCase("var resource = GetShared(); if (condition) resource = new Disposable(); resource.Dispose();")] + [TestCase("var resource = GetShared(); using ({|ERP046:resource|}) { resource = new Disposable(); resource.Dispose(); }")] + [TestCase("var resource = condition ? GetShared() : GetShared(); {|ERP046:resource|}.Dispose();")] + public Task DoNotDispose_Returns_Are_Tracked_Through_Simple_Aliases(string body) + { + return VerifyAsync(@" +public class Test +{ + public void Run(bool condition) { " + body + @" } + [return: DoNotDispose] private static Disposable GetShared() => null; + private static void Consume([AcquiresOwnership] Disposable resource) { resource.Dispose(); } +}"); + } + + [TestCase("parameter")] + [TestCase("_field")] + [TestCase("Property")] + [TestCase("GetterProperty")] + public Task DoNotDispose_Parameters_And_Members_Cannot_Be_Disposed(string expression) + { + return VerifyAsync(@" +public class Test +{ + [DoNotDispose] private Disposable _field; + [DoNotDispose] private Disposable Property => null; + private Disposable GetterProperty { [return: DoNotDispose] get => null; } + public void Run([DoNotDispose] Disposable parameter) + { + var alias = " + expression + @"; + {|ERP046:alias|}.Dispose(); + } +}"); + } + + [TestCase("DoNotDispose")] + [TestCase("NoOwnership")] + public Task Borrowed_Parameter_Reassignment_Starts_A_New_Owned_Value(string attribute) + { + return VerifyAsync(@" +public class Test +{ + public void Run([" + attribute + @"] Disposable parameter) + { + parameter = new Disposable(); + parameter.Dispose(); + } +}"); + } + + [TestCase("[return: DoNotDispose]")] + [TestCase("[DoNotDispose]")] + [TestCase("[KeepsOwnership]")] + public Task Borrowed_Return_Attribute_Names_Share_Disposal_Enforcement(string attribute) + { + return VerifyAsync(@" +public class Test +{ + " + attribute + @" private static Disposable GetShared() => null; + public void Run() { var resource = GetShared(); {|ERP046:resource|}.Dispose(); } +}"); + } + + [TestCase("await {|ERP046:GetShared()|}.DisposeAsync();")] + [TestCase("var resource = await GetSharedAsync(); await {|ERP046:resource|}.DisposeAsync();")] + [TestCase("var resource = await GetSharedAsync().ConfigureAwait(false); await {|ERP046:resource|}.DisposeAsync();")] + [TestCase("await using var resource = {|ERP046:await GetSharedAsync()|};")] + [TestCase("var resource = await GetSharedAsync(); await using ({|ERP046:resource|}) { }")] + [TestCase("var resource = await GetSharedAsync(); await using var alias = {|ERP046:resource|};")] + public Task DoNotDispose_Also_Prohibits_Async_Disposal(string body) + { + return VerifyAsync(@" +public class Resource : System.IAsyncDisposable +{ + public System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Test +{ + [return: DoNotDispose] private static Resource GetShared() => null; + [return: DoNotDispose] private static System.Threading.Tasks.Task GetSharedAsync() => null; + public async System.Threading.Tasks.Task Run() { " + body + @" } +}"); + } + + [Test] + public Task DoNotDispose_Returns_Inherit_Interface_Contracts() + { + return VerifyAsync(@" +public interface IProvider +{ + [return: DoNotDispose] Disposable GetShared(); +} +public class Test : IProvider +{ + public Disposable GetShared() => null; + public void Run() { var resource = GetShared(); {|ERP046:resource|}.Dispose(); } +}"); + } + + [Test] + public Task DoNotDispose_Prevents_Transfers_Into_Owning_Returns() + { + return VerifyAsync(@" +public class Test +{ + [return: DoNotDispose] private static Disposable GetShared() => null; + [return: ReturnsOwnership] + public Disposable Create() + { + var resource = GetShared(); + return {|ERP046:resource|}; + } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.CarriedParameters.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.CarriedParameters.cs new file mode 100644 index 0000000..966f024 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.CarriedParameters.cs @@ -0,0 +1,207 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("Task", "Task.FromResult(item)")] + [TestCase("ValueTask", "ValueTask.FromResult(item)")] + [TestCase("ValueTask", "new ValueTask(item)")] + public Task CarriedParameters_Explicit_Transfer_Preserves_Ownership(string type, string carrier) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + private object saved; + private void Take([AcquiresOwnership] " + type + @" pending) { saved = pending; } + public void Owned() + { + var item = new Disposable(); + Take(" + carrier + @"); + {|ERP046:item|}.ToString(); + } + public void Borrowed([DoNotDispose] Disposable item) + { + Take({|ERP046:" + carrier + @"|}); + } +}"); + } + + [TestCase("Task")] + [TestCase("ValueTask")] + public Task CarriedParameters_Acquired_Result_Is_Cleaned_Up_By_Awaiting(string type) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run([AcquiresOwnership] " + type + @" pending) + { + var alias = pending; + var item = await alias.ConfigureAwait(false); + item.Dispose(); + ({|ERP046:await pending|}).ToString(); + } +}"); + } + + [TestCase("Task", "")] + [TestCase("Task", "pending.Dispose();")] + [TestCase("Task", "using (pending) { }")] + [TestCase("ValueTask", "_ = pending;")] + public Task CarriedParameters_Wrapper_Only_Cleanup_Does_Not_Satisfy_The_Contract(string type, string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public void Run([AcquiresOwnership] " + type + @" {|ERP044:pending|}) + { + " + body + @" + } +}"); + } + + [TestCase("Task")] + [TestCase("ValueTask")] + public Task CarriedParameters_Borrowed_Results_Cannot_Be_Disposed(string type) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run([DoNotDispose] " + type + @" pending) + { + var alias = pending; + ({|ERP046:await alias.ConfigureAwait(false)|}).Dispose(); + } +}"); + } + + [TestCase("new Sink(Task.FromResult(item))")] + [TestCase("sink[Task.FromResult(item)]")] + [TestCase("(Sink)Task.FromResult(item)")] + public Task CarriedParameters_Other_Acquiring_Boundaries_Check_Carriers(string expression) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Sink +{ + private object saved; + public Sink() { } + public Sink([AcquiresOwnership] Task pending) { saved = pending; } + public int this[[AcquiresOwnership] Task pending] + { + get { saved = pending; return 0; } + } + public static implicit operator Sink([AcquiresOwnership] Task pending) => new Sink(pending); +} +public class Test +{ + public void Run(Sink sink, [DoNotDispose] Disposable item) + { + _ = " + expression.Replace("Task.FromResult(item)", "{|ERP046:Task.FromResult(item)|}") + @"; + } + public void Owned(Sink sink) + { + var item = new Disposable(); + _ = " + expression + @"; + {|ERP046:item|}.ToString(); + } +}"); + } + + [TestCase("Task", "Task.FromResult(item)")] + [TestCase("ValueTask", "ValueTask.FromResult(item)")] + public Task CarriedParameters_Source_Inference_Recognizes_Awaited_Cleanup(string type, string carrier) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + private static async Task Cleanup(" + type + @" pending) { (await pending).Dispose(); } + public async Task Run([DoNotDispose] Disposable item) + { + await Cleanup({|ERP046:" + carrier + @"|}); + } + public async Task Owned() + { + var item = new Disposable(); + await Cleanup(" + carrier + @"); + {|ERP046:item|}.ToString(); + } +}"); + } + + [Test] + public Task CarriedParameters_Source_Inference_Follows_Wrapped_Arguments() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + private static object saved; + private static void Take([AcquiresOwnership] Task pending) { saved = pending; } + private static void Forward(Disposable value) { Take(Task.FromResult(value)); } + public void Run([DoNotDispose] Disposable item) { Forward({|ERP046:item|}); } + public void Owned() + { + var item = new Disposable(); + Forward(item); + {|ERP046:item|}.ToString(); + } +}"); + } + + [Test] + public Task CarriedParameters_Disposing_Only_The_Wrapper_Does_Not_Infer_Result_Acquisition() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + private static void WrapperOnly(Task pending) { pending.Dispose(); } + public void Run([DoNotDispose] Disposable item) { WrapperOnly(Task.FromResult(item)); } + public async Task Owned([AcquiresOwnership] Task pending) + { + (await pending).Dispose(); + pending.Dispose(); + } +}"); + } + + [TestCase("Task")] + [TestCase("ValueTask")] + public Task CarriedParameters_Capture_Remains_An_Unknown_Handoff(string type) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public System.Action Run([AcquiresOwnership] " + type + @" pending) + => () => System.GC.KeepAlive(pending); +}"); + } + + [Test] + public Task CarriedParameters_Borrowed_Reassignment_Updates_Result_Binding() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Replace([DoNotDispose] Task pending) + { + pending = Task.FromResult(new Disposable()); + (await pending).Dispose(); + } + public async Task Preserve([DoNotDispose] Task pending, [DoNotDispose] Disposable item, bool replace) + { + if (replace) { pending = Task.FromResult(item); } + ({|ERP046:await pending|}).Dispose(); + } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.FreshReturns.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.FreshReturns.cs new file mode 100644 index 0000000..9fe0ace --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.FreshReturns.cs @@ -0,0 +1,293 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("=> new Disposable();")] + [TestCase("{ return new Disposable(); }")] + [TestCase("=> (System.IDisposable)(new Disposable());")] + public Task Fresh_Return_Creates_A_Caller_Obligation(string implementation) + { + return VerifyAsync(@" +public class Test +{ + private static System.IDisposable Create() " + implementation + @" + public void Run() { var {|ERP044:resource|} = Create(); } +}"); + } + + [TestCase("=> shared;")] + [TestCase("=> null;")] + [TestCase("=> condition ? new Disposable() : shared;")] + [TestCase("{ if (condition) return new Disposable(); return shared; }")] + [TestCase("{ var resource = new Disposable(); return resource; }")] + [TestCase("{ Prepare(); return new Disposable(); }")] + [TestCase("=> Disposable.Create();")] + [TestCase("=> Create();")] + public Task Oblivious_Result_Stays_Unknown_Without_A_Direct_Fresh_Return(string implementation) + { + return VerifyAsync(@" +public class Test +{ + private static Disposable shared; + private static bool condition; + private static void Prepare() { } + private static Disposable Create() " + implementation + @" + public void Run() { var resource = Create(); } +}"); + } + + [TestCase("var resource = System.IO.File.OpenRead(\"path\");")] + [TestCase("using var resource = System.IO.File.OpenRead(\"path\");")] + [TestCase("var resource = GetShared(); resource.Dispose();")] + public Task Oblivious_Result_Does_Not_Imply_Ownership_Or_Borrowing(string body) + { + return VerifyAsync(@" +public class Test +{ + private static Disposable shared; + private static Disposable GetShared() => shared; + public void Run() { " + body + @" } +}"); + } + + [TestCase("public virtual", true, false)] + [TestCase("public override", false, false)] + [TestCase("public sealed override", false, true)] + public Task Fresh_Return_Requires_A_Non_Overridable_Target(string modifiers, bool baseMethod, bool owned) + { + return VerifyAsync(@" +public class Base +{ + public virtual Disposable Create() => new Disposable(); +} +public class Test " + (baseMethod ? "" : ": Base") + @" +{ + " + modifiers + @" Disposable Create() => new Disposable(); + public void Run() + { + var " + (owned ? "{|ERP044:resource|}" : "resource") + @" = Create(); + } +}"); + } + + [Test] + public Task Fresh_Return_Is_Inferred_For_An_Override_In_A_Sealed_Type() + { + return VerifyAsync(@" +public abstract class Base { public abstract Disposable Create(); } +public sealed class Test : Base +{ + public override Disposable Create() => new Disposable(); + public void Run() { var {|ERP044:resource|} = Create(); } +}"); + } + + [TestCase("var {|ERP044:copy|} = original.Clone(); original.Dispose();")] + [TestCase("var copy = original.Clone(); copy.Dispose(); original.Dispose();")] + public Task Fresh_Return_Is_Not_A_Fluent_Receiver_Alias(string body) + { + return VerifyAsync(@" +public class Resource : System.IDisposable +{ + public void Dispose() { } + public Resource Clone() => new Resource(); +} +public class Test +{ + public void Run() + { + var original = new Resource(); + " + body + @" + } +}"); + } + + [Test] + public Task Fresh_Return_Disposal_Does_Not_Discharge_The_Receiver() + { + return VerifyAsync(@" +public class Resource : System.IDisposable +{ + public void Dispose() { } + public Resource Clone() => new Resource(); +} +public class Test +{ + public void Run() + { + var {|ERP044:original|} = new Resource(); + var copy = original.Clone(); + copy.Dispose(); + } +}"); + } + + [TestCase("[return: DoNotDispose]")] + [TestCase("[KeepsOwnership]")] + public Task Fresh_Return_Does_Not_Override_An_Explicit_Borrowing_Contract(string attribute) + { + return VerifyAsync(@" +public class Test +{ + " + attribute + @" + private static Disposable Create() => new Disposable(); + public void Run() { var resource = Create(); {|ERP046:resource|}.Dispose(); } +}"); + } + + [Test] + public Task Oblivious_Result_Can_Be_Made_Explicitly_Owning() + { + return VerifyAsync(@" +public class Test +{ + [return: ReturnsOwnership] + private static Disposable Create() => null; + public void Run() { var {|ERP044:resource|} = Create(); } +}"); + } + + [Test] + public Task Fresh_Return_Supports_Disposable_Structs_And_Constrained_Generics() + { + return VerifyAsync(@" +public struct Resource : System.IDisposable { public void Dispose() { } } +public class Test +{ + private static System.IDisposable CreateStruct() => new Resource(); + private static T Create() where T : System.IDisposable, new() => new T(); + public void Run() + { + var {|ERP044:structure|} = CreateStruct(); + var {|ERP044:generic|} = Create(); + } +}"); + } + + [TestCase("var {|ERP044:resource|} = await Create();")] + [TestCase("await using var resource = await Create().ConfigureAwait(false);")] + public Task Fresh_Return_Supports_Direct_Async_Factories(string body) + { + return VerifyAsync(@" +public class Resource : System.IAsyncDisposable +{ + public System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Test +{ + private static async System.Threading.Tasks.Task Create() => new Resource(); + public async System.Threading.Tasks.Task Run() { " + body + @" } +}"); + } + + [TestCase("new Source()")] + [TestCase("(dynamic){|ERP044:new Source()|}")] + public Task Oblivious_Result_Does_Not_Treat_A_User_Conversion_As_A_Fresh_Return(string expression) + { + return VerifyAsync(@" +public class Source : System.IDisposable +{ + private static Disposable shared; + public void Dispose() { } + public static implicit operator Disposable(Source value) { value.Dispose(); return shared; } +} +public class Test +{ + private static Disposable Create() => " + expression + @"; + public void Run() { var resource = Create(); } +}"); + } + + [Test] + public Task Fresh_Return_Respects_The_Disposable_Type_Exemptions() + { + return VerifyAsync(@" +public class Test +{ + private static System.IDisposable Create() => new System.IO.MemoryStream(); + public void Run() { var resource = Create(); } +}"); + } + + [Test] + public Task Fresh_Return_Supports_Generic_Extensions_Without_Receiver_Aliasing() + { + return VerifyAsync(@" +public static class Factories +{ + public static T FreshCopy([DoNotDispose] this T value) where T : System.IDisposable, new() => new T(); +} +public class Test +{ + public void Run() + { + var {|ERP044:original|} = new Disposable(); + var copy = original.FreshCopy(); + copy.Dispose(); + } +}"); + } + + [Test] + public Task Fresh_Return_Uses_The_Partial_Implementation() + { + return VerifyAsync(@" +public partial class Api { public static partial Disposable Create(); } +public partial class Api { public static partial Disposable Create() => new Disposable(); } +public class Test +{ + public void Run() { var {|ERP044:resource|} = Api.Create(); } +}"); + } + + [Test] + public Task Oblivious_Result_Of_Virtual_Dispatch_Does_Not_Use_A_Derived_Body() + { + return VerifyAsync(@" +public abstract class Base { public abstract Disposable Create(); } +public sealed class Implementation : Base +{ + public override Disposable Create() => new Disposable(); +} +public class Test +{ + public void Run() + { + Base factory = new Implementation(); + var resource = factory.Create(); + } +}"); + } + + [Test] + public Task Oblivious_Result_From_Yield_Is_Not_A_Direct_Factory_Return() + { + return VerifyAsync(@" +public class Test +{ + private static System.Collections.Generic.IEnumerator Create() + { + yield return new Disposable(); + } + public void Run() { var resource = Create(); } +}"); + } + + [Test] + public Task Oblivious_Result_Remains_Unknown_After_Await() + { + return VerifyAsync(@" +public class Test +{ + private static System.Threading.Tasks.Task Create() => null; + public async System.Threading.Tasks.Task Run() + { + var first = await Create(); + var second = await Create().ConfigureAwait(false); + } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.Inference.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.Inference.cs new file mode 100644 index 0000000..4f80b40 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.Inference.cs @@ -0,0 +1,486 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("var {|ERP044:d|} = new Disposable(); var alias = d;")] + [TestCase("var {|ERP044:d|} = new Disposable(); _ = d;")] + [TestCase("var {|ERP044:d|} = new Disposable(); d = new Disposable(); d.Dispose();")] + [TestCase("var d = new Disposable(); var alias = d; alias.Dispose();")] + [TestCase("var d = new Disposable(); var alias = d; d = null; alias.Dispose();")] + [TestCase("new Disposable().Dispose();")] + [TestCase("new Disposable().Close();")] + [TestCase("var d = new Disposable(); d?.Dispose();")] + [TestCase("var d = new Disposable(); using (d) { }")] + [TestCase("var d = new Disposable(); using var alias = d;")] + [TestCase("Disposable d = null; d?.Dispose(); {|ERP044:d|} = new Disposable();")] + public Task Tracks_Resources_Through_Local_Aliases(string body) + { + return VerifyAsync("public class Test { public void Run() { " + body + " } }"); + } + + [Test] + public Task Checks_All_Acquired_Parameters() + { + return VerifyAsync(@" +public class Test +{ + public void Consume([AcquiresOwnership] Disposable first, + [AcquiresOwnership] Disposable {|ERP044:second|}) + { + first.Dispose(); + } +}"); + } + + [Test] + public Task Trusts_Declared_Transfer_And_Reports_In_Callee() + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + var d = new Disposable(); + Consume(d); + } + private void Consume([AcquiresOwnership] Disposable {|ERP044:value|}) { } +}"); + } + + [TestCase("Consume(owned: d, borrowed: null);", false)] + [TestCase("Consume(owned: null, borrowed: d);", true)] + public Task Uses_Bound_Parameters_For_Named_Arguments(string call, bool leaks) + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + var " + (leaks ? "{|ERP044:d|}" : "d") + @" = new Disposable(); + " + call + @" + } + private void Consume([DoNotDispose] Disposable borrowed, [AcquiresOwnership] Disposable owned) + { + owned.Dispose(); + } +}"); + } + + [Test] + public Task Constructor_Receives_And_Discharges_Ownership() + { + return VerifyAsync(@" +public class Test +{ + public void Run() { _ = new Owner(new Disposable()); } +} +public class Owner +{ + public Owner([AcquiresOwnership] Disposable value) { value.Dispose(); } +}"); + } + + [Test] + public Task Constructor_Must_Discharge_Annotated_Parameter() + { + return VerifyAsync(@" +public class Owner +{ + public Owner([AcquiresOwnership] Disposable {|ERP044:value|}) { } +}"); + } + + [Test] + public Task Acquiring_Parameters_Do_Not_Hide_Owning_Returns() + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + var input = new Disposable(); + var {|ERP044:output|} = Replace(input); + } + [ReturnsOwnership] + private static Disposable Replace([AcquiresOwnership] Disposable value) + { + value.Dispose(); + return new Disposable(); + } +}"); + } + + [Test] + public Task Explicit_Owning_Result_Overrides_Fluent_Inference() + { + return VerifyAsync(@" +public class Resource : System.IDisposable +{ + public void Dispose() { } + [ReturnsOwnership] + public Resource Clone() => new Resource(); + public void Run() + { + var {|ERP044:copy|} = Clone(); + } +}"); + } + + [Test] + public Task Return_Target_Attribute_Defines_Owning_Result() + { + return VerifyAsync(@" +public class Test +{ + public void Run() { var {|ERP044:d|} = Create(); } + [return: ReturnsOwnership] + private static Disposable Create() => new Disposable(); +}"); + } + + [Test] + public Task Infers_Forwarding_To_An_Owning_Callee() + { + return VerifyAsync(@" +public class Test +{ + public void Run() { Forward(new Disposable()); } + private static void Forward(Disposable value) { Consume(value); } + private static void Consume([AcquiresOwnership] Disposable value) { value.Dispose(); } +}"); + } + + [Test] + public Task Recursive_Inference_Is_Bounded() + { + return VerifyAsync(@" +public class Test +{ + public void Run([DoNotDispose] Disposable borrowed) { Loop(borrowed); } + private static void Loop(Disposable value) { Loop(value); } +}"); + } + + [Test] + public Task Does_Not_Transfer_To_A_Borrowed_Field() + { + return VerifyAsync(@" +public class Test +{ + [NoOwnership] private Disposable _borrowed; + public void Run() { _borrowed = {|ERP044:new Disposable()|}; } +}"); + } + + [TestCase("var {|ERP044:d|} = new Resource();")] + [TestCase("using var d = new Resource();")] + [TestCase("var d = new Resource(); d.Dispose();")] + public Task Recognizes_Disposable_Structs(string body) + { + return VerifyAsync(@" +public struct Resource : System.IDisposable { public void Dispose() { } } +public class Test { public void Run() { " + body + " } }"); + } + + [Test] + public Task Recognizes_Constrained_Generic_Creation() + { + return VerifyAsync(@" +public class Test +{ + public void Run() where T : System.IDisposable, new() + { + var {|ERP044:value|} = new T(); + } +}"); + } + + [TestCase("var {|ERP044:d|} = new Resource();")] + [TestCase("var d = new Resource(); await d.DisposeAsync();")] + [TestCase("await using var d = new Resource();")] + [TestCase("await using (new Resource()) { }")] + public Task Recognizes_Async_Disposal(string body) + { + return VerifyAsync(@" +public class Resource : System.IAsyncDisposable +{ + public System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Test +{ + public async System.Threading.Tasks.Task Run() { " + body + " } }"); + } + + [TestCase("var {|ERP044:d|} = await Create();")] + [TestCase("using var d = await Create();")] + [TestCase("var {|ERP044:d|} = await Create().ConfigureAwait(false);")] + [TestCase("using var d = await Create().ConfigureAwait(false);")] + public Task Recognizes_Owned_Awaited_Results(string body) + { + return VerifyAsync(@" +public class Test +{ + public async System.Threading.Tasks.Task Run() { " + body + @" } + [ReturnsOwnership] + private static System.Threading.Tasks.Task Create() => null; +}"); + } + + [TestCase(false)] + [TestCase(true)] + public Task Stream_Wrapper_Respects_LeaveOpen(bool leaveOpen) + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + var " + (leaveOpen ? "{|ERP044:stream|}" : "stream") + @" = new System.IO.FileStream(""path"", System.IO.FileMode.Open); + using var reader = new System.IO.StreamReader(stream, System.Text.Encoding.UTF8, true, 1024, " + + (leaveOpen ? "true" : "false") + @"); + } +}"); + } + + [TestCase("var d = new Disposable(); d.Dispose(); {|ERP046:d|}.ToString();")] + [TestCase("var d = new Disposable(); var alias = d; d.Dispose(); {|ERP046:alias|}.ToString();")] + [TestCase("var d = new Disposable(); d.Dispose(); {|ERP046:d|}.Dispose();")] + [TestCase("var d = new Disposable(); Consume(d); {|ERP046:d|}.ToString();")] + [TestCase("var d = new Disposable(); d.Dispose(); d = new Disposable(); d.Dispose();")] + [TestCase("var d = new Disposable(); d.Dispose(); d = null;")] + public Task Checks_Obvious_Uses_After_Disposal_Or_Transfer(string body) + { + return VerifyAsync(@" +public class Test +{ + public void Run() { " + body + @" } + private static void Consume([AcquiresOwnership] Disposable value) { value.Dispose(); } +}"); + } + + [TestCase("{|ERP046:value|}.Dispose();")] + [TestCase("{|ERP046:value|}?.Dispose();")] + public Task Explicitly_Borrowed_Parameter_Cannot_Be_Disposed(string body) + { + return VerifyAsync(@" +public class Test +{ + public void Run([NoOwnership] Disposable value) + { + " + body + @" + } +}"); + } + + [Test] + public Task Explicitly_Borrowed_Field_Cannot_Be_Transferred() + { + return VerifyAsync(@" +public class Test +{ + [NoOwnership] private Disposable _borrowed; + public void Run() { Consume({|ERP046:_borrowed|}); } + private static void Consume([AcquiresOwnership] Disposable value) { value.Dispose(); } +}"); + } + + [Test] + public Task Conditional_Reassignment_Remains_Best_Effort() + { + return VerifyAsync(@" +public class Test +{ + public void Run(bool condition) + { + Disposable d; + if (condition) + d = new Disposable(); + else + d = new Disposable(); + d.Dispose(); + } +}"); + } + + [TestCase("this(value, 0)")] + [TestCase("base(value)")] + public Task Constructor_Initializers_Transfer_Ownership(string initializer) + { + return VerifyAsync(@" +public class Parent { public Parent([AcquiresOwnership] Disposable value) { value?.Dispose(); } } +public class Owner : Parent +{ + public Owner([AcquiresOwnership] Disposable value) : " + initializer + @" { } + public Owner([AcquiresOwnership] Disposable value, int unused) : base(value) { } +}"); + } + + [Test] + public Task Parameter_Reassignment_Does_Not_Erase_An_Acquired_Resource() + { + return VerifyAsync(@" +public class Test +{ + public void Consume([AcquiresOwnership] Disposable {|ERP044:value|}) + { + value = new Disposable(); + value.Dispose(); + } +}"); + } + + [TestCase("var alias = value; alias.Dispose();", true)] + [TestCase("using var alias = value;", true)] + [TestCase("var alias = value; using (alias) { }", true)] + [TestCase("var alias = value; Forward(alias);", true)] + [TestCase("value = new Disposable(); value.Dispose();", false)] + public Task Infers_Disposal_Through_Simple_Callee_Aliases(string body, bool consumes) + { + return VerifyAsync(@" +public class Test +{ + public void Run([DoNotDispose] Disposable borrowed) + { + Consume(" + (consumes ? "{|ERP046:borrowed|}" : "borrowed") + @"); + } + private static void Consume(Disposable value) { " + body + @" } + private static void Forward([AcquiresOwnership] Disposable value) { value.Dispose(); } +}"); + } + + [TestCase("flag ? ConsumeAndReturn(d) : null")] + [TestCase("flag ? d : null", true)] + public Task Conditional_Transfers_Do_Not_Prove_Invalid_Uses(string expression, bool wrap = false) + { + return VerifyAsync(@" +public class Test +{ + public void Run(bool flag) + { + var d = new Disposable(); + " + (wrap ? "ConsumeAndReturn(" + expression + ");" : "_ = " + expression + ";") + @" + d.ToString(); + } + private static object ConsumeAndReturn([AcquiresOwnership] Disposable value) { value?.Dispose(); return null; } +}"); + } + + [TestCase("public void Run() { _owned = {|ERP046:_borrowed|}; }")] + [TestCase("[ReturnsOwnership] public Disposable Create() => {|ERP046:_borrowed|};")] + [TestCase("public Disposable Create() => _borrowed;")] + [TestCase("[KeepsOwnership] public Disposable Borrow() => _borrowed;")] + [TestCase("public Disposable BorrowedProperty => _borrowed;")] + [TestCase("[ReturnsOwnership] public Disposable OwnedProperty => {|ERP046:_borrowed|};")] + [TestCase("public void Create(out Disposable result) { result = {|ERP046:_borrowed|}; }")] + public Task Borrowed_Resources_Cannot_Escape_As_Owned_Resources(string member) + { + return VerifyAsync(@" +public class Test +{ + [NoOwnership] private Disposable _borrowed; + private Disposable _owned; + " + member + @" +}"); + } + + [TestCase("using ({|ERP046:value|}) { }")] + [TestCase("using (var alias = {|ERP046:value|}) { }")] + [TestCase("using var alias = {|ERP046:value|};")] + [TestCase("using var alias = {|ERP046:Borrow()|};")] + public Task Using_Cannot_Dispose_Explicitly_Borrowed_Resources(string body) + { + return VerifyAsync(@" +public class Test +{ + public void Run([NoOwnership] Disposable value) { " + body + @" } + [KeepsOwnership] private static Disposable Borrow() => null; +}"); + } + + [TestCase("resource.Borrow()")] + [TestCase("resource.BorrowGeneric()")] + public Task Explicit_Borrowed_Results_Do_Not_Imply_Receiver_Identity(string expression) + { + return VerifyAsync(@" +public class Resource : System.IDisposable +{ + public void Dispose() { } + [KeepsOwnership] public Resource Borrow() => null; +} +public static class BorrowExtensions +{ + [KeepsOwnership] public static T BorrowGeneric([DoNotDispose] this T resource) => default; +} +public class Test +{ + public void Run() + { + var {|ERP044:resource|} = new Resource(); + var borrowed = " + expression + @"; + {|ERP046:borrowed|}.Dispose(); + } +}"); + } + + [TestCase("new System.IO.StreamReader(stream)", false)] + [TestCase("new System.IO.StreamReader(stream, System.Text.Encoding.UTF8, true, 1024, false)", false)] + [TestCase("new System.IO.StreamReader(stream, System.Text.Encoding.UTF8, true, 1024, true)", true)] + [TestCase("new System.IO.StreamReader(stream, System.Text.Encoding.UTF8, true, 1024, leaveOpen)", true)] + public Task Source_Forwarding_Uses_The_Same_Stream_Wrapper_Policy(string creation, bool retainsStream) + { + return VerifyAsync(@" +public class Test +{ + public void Run(bool leaveOpen, [DoNotDispose] System.IO.Stream borrowed) + { + var stream = new System.IO.FileStream(""path"", System.IO.FileMode.Open); + using var reader = Read(stream, leaveOpen); + using var other = Read(" + (retainsStream ? "borrowed" : "{|ERP046:borrowed|}") + @", leaveOpen); + } + private static System.IO.StreamReader Read(System.IO.Stream stream, bool leaveOpen) => " + creation + @"; +}"); + } + + [TestCase("var alias = flag ? d : other; d.Dispose(); alias.ToString();")] + [TestCase("var alias = flag ? d : other; alias.Dispose(); d.ToString();")] + [TestCase("var alias = flag ? d : other; var copy = alias; d.Dispose(); copy.ToString();")] + [TestCase("var alias = flag ? d : other; alias = d; d.Dispose(); {|ERP046:alias|}.ToString();")] + [TestCase("var alias = d; if (flag) alias = other; d.Dispose(); alias.ToString();")] + [TestCase("var alias = d; if (flag) alias = other; d.Dispose(); alias.ToString(); {|ERP046:d|}.ToString();")] + [TestCase("d.Dispose(); var alias = flag ? d : other; alias.ToString();")] + [TestCase("d.Dispose(); var name = nameof(d);")] + [TestCase("d.Dispose(); var name = nameof(d.Dispose);")] + public Task Invalid_Use_Requires_A_Definite_Runtime_Alias(string body) + { + return VerifyAsync(@" +public class Test +{ + public void Run(bool flag, Disposable other) + { + var d = new Disposable(); + " + body + @" + } +}"); + } + + [Test] + public Task Await_Using_Cannot_Dispose_An_Explicitly_Borrowed_Result() + { + return VerifyAsync(@" +public class Resource : System.IAsyncDisposable +{ + public System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Test +{ + public async System.Threading.Tasks.Task Run() + { + await using var resource = {|ERP046:await Borrow()|}; + } + [KeepsOwnership] private static System.Threading.Tasks.Task Borrow() => null; +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.LowNoise.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.LowNoise.cs new file mode 100644 index 0000000..c342aa5 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.LowNoise.cs @@ -0,0 +1,317 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("Unknown(item); item.ToString();")] + [TestCase("_ = new Holder(item);")] + [TestCase("_ = this[item];")] + [TestCase("var alias = item; Unknown(alias);")] + [TestCase("System.Action callback = () => item.ToString();")] + [TestCase("System.Action callback = item.Dispose;")] + [TestCase("void Callback() { item.ToString(); }")] + [TestCase("var pending = Task.FromResult(item); Unknown(pending);")] + [TestCase("dynamic target = this; target.Unknown(item);")] + public Task LowNoise_Unknown_Handoffs_And_Captures_Are_Not_Leaks_Or_Definite_Moves(string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public void Run() + { + var item = new Disposable(); + " + body + @" + item.ToString(); + } + public static void Unknown(object value) { } + public int this[Disposable value] => 0; +} +public class Holder { public Holder(Disposable value) { } } +"); + } + + [TestCase("item.ToString();")] + [TestCase("_ = item;")] + [TestCase("Borrow(item);")] + [TestCase("_ = new Borrower(item);")] + [TestCase("System.Action callback = () => System.Console.WriteLine(nameof(item));")] + [TestCase("System.Action callback = () => { var other = new Disposable(); other.Dispose(); };")] + [TestCase("var alias = item.ThrowIfNull();")] + public Task LowNoise_Receivers_Borrowing_And_NonCaptures_Keep_The_Obligation(string body) + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + var {|ERP044:item|} = new Disposable(); + " + body + @" + } + public static void Borrow([DoNotDispose] Disposable value) { } +} +public class Borrower { public Borrower([DoNotDispose] Disposable value) { } } +"); + } + + [TestCase("Unknown(item);")] + [TestCase("System.Action callback = () => item.ToString();")] + public Task LowNoise_Uncertainty_Does_Not_Hide_Later_Definite_Misuse(string handoff) + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + var item = new Disposable(); + " + handoff + @" + item.Dispose(); + {|ERP046:item|}.ToString(); + } + private static void Unknown(object value) { } +}"); + } + + [Test] + public Task LowNoise_Unknown_Consumption_Is_Not_Inferred_As_Acquiring() + { + return VerifyAsync(@" +public class Test +{ + public void Run([DoNotDispose] Disposable borrowed) + { + Forward(borrowed); + borrowed.ToString(); + } + private static void Forward(Disposable value) { Unknown(value); } + private static void Unknown(object value) { } +}"); + } + + [TestCase("Task.FromResult(item)")] + [TestCase("new ValueTask(item)")] + [TestCase("ValueTask.FromResult(item)")] + public Task LowNoise_Discarding_A_Completed_Task_Keeps_The_Resource_Obligation(string expression) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public void Run() + { + var {|ERP044:item|} = new Disposable(); + _ = " + expression + @"; + } +}"); + } + + [TestCase("var pending = Task.FromResult(item); pending.Dispose();")] + [TestCase("using var pending = Task.FromResult(item);")] + [TestCase("var pending = Task.FromResult(item); var alias = pending; _ = alias;")] + [TestCase("var pending = Task.FromResult(item); pending = Task.FromResult(null); using var value = await pending;")] + public Task LowNoise_Task_Lifetime_Is_Not_Its_Result_Lifetime(string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run() + { + var {|ERP044:item|} = new Disposable(); + " + body + @" + await Task.CompletedTask; + } +}"); + } + + [TestCase("using var value = await Task.FromResult(item);")] + [TestCase("using var value = await Task.FromResult(item).ConfigureAwait(false);")] + [TestCase("using var value = await new ValueTask(item);")] + [TestCase("using var value = await ValueTask.FromResult(item);")] + [TestCase("var pending = Task.FromResult(item); var alias = pending; pending = null; using var value = await alias;")] + [TestCase("var pending = Task.FromResult(item).ConfigureAwait(false); using var value = await pending;")] + [TestCase("var pending = Task.FromResult(item); var value = await pending; value.Dispose();")] + public Task LowNoise_Completed_Task_Results_Retain_Resource_Identity(string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run() + { + var item = new Disposable(); + " + body + @" + } +}"); + } + + [Test] + public Task LowNoise_Completed_Task_Result_Reports_Use_After_Disposal() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run() + { + var item = new Disposable(); + var pending = Task.FromResult(item); + var value = await pending; + value.Dispose(); + {|ERP046:item|}.ToString(); + } +}"); + } + + [TestCase("using var value = {|ERP046:await Task.FromResult(item)|};")] + [TestCase("var pending = Task.FromResult(item); using var value = {|ERP046:await pending|};")] + [TestCase("var pending = new ValueTask(item); using var value = {|ERP046:await pending.ConfigureAwait(false)|};")] + [TestCase("var pending = Task.FromResult(item); var value = await pending; {|ERP046:value|}.Dispose();")] + [TestCase("using var pending = Task.FromResult(item);")] + [TestCase("var pending = Task.FromResult(item); pending = Task.FromResult(new Disposable()); using var value = await pending;")] + public Task LowNoise_Completed_Task_Results_Preserve_Borrowing_Not_Task_Borrowing(string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run([DoNotDispose] Disposable item) + { + " + body + @" + await Task.CompletedTask; + } +}"); + } + + [Test] + public Task LowNoise_Owning_Task_Return_Cannot_Carry_A_Borrowed_Resource() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + [return: ReturnsOwnership] + public Task Wrap([DoNotDispose] Disposable borrowed) + { + var pending = Task.FromResult(borrowed); + return {|ERP046:pending|}; + } +}"); + } + + [Test] + public Task LowNoise_Unknown_Handoff_Does_Not_Hide_A_Replacement_Lifetime() + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + var item = new Disposable(); + Unknown(item); + {|ERP044:item|} = new Disposable(); + } + private static void Unknown(object value) { } +}"); + } + + [Test] + public Task LowNoise_Acquired_Parameter_May_Be_Handed_To_An_Unknown_Consumer() + { + return VerifyAsync(@" +public class Test +{ + public void Forward([AcquiresOwnership] Disposable item) { Unknown(item); } + private static void Unknown(object value) { } +}"); + } + + [Test] + public Task LowNoise_Explicit_Borrowing_Survives_An_Unknown_Handoff() + { + return VerifyAsync(@" +public class Test +{ + public void Run([DoNotDispose] Disposable item) + { + Unknown(item); + {|ERP046:item|}.Dispose(); + } + private static void Unknown(object value) { } +}"); + } + + [TestCase("Task.FromResult(new Disposable())", "Task")] + [TestCase("new ValueTask(new Disposable())", "ValueTask")] + [TestCase("ValueTask.FromResult(new Disposable())", "ValueTask")] + public Task LowNoise_Completed_Wrappers_Can_Be_Returned_Through_A_Local(string value, string type) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + [return: ReturnsOwnership] + public " + type + @" Create() + { + var pending = " + value + @"; + return pending; + } +}"); + } + + [TestCase("Task.FromResult(item)")] + [TestCase("new ValueTask(item)")] + public Task LowNoise_Awaiting_Without_Cleanup_Does_Not_Discharge_A_Wrapped_Resource(string value) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run() + { + var {|ERP044:item|} = new Disposable(); + var result = await " + value + @"; + result.ToString(); + } +}"); + } + + [Test] + public Task LowNoise_UserDefined_Conversion_Is_An_Unknown_Handoff_Not_An_Alias() + { + return VerifyAsync(@" +public sealed class Other : System.IDisposable +{ + public void Dispose() { } + public static implicit operator Other(Disposable input) => new Other(); +} +public class Test +{ + public void Run() + { + var input = new Disposable(); + using var output = (Other)input; + input.ToString(); + } +}"); + } + + [Test] + public Task LowNoise_Awaiting_A_Known_Disposed_Resource_Is_Still_Misuse() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public async Task Run() + { + var item = new Disposable(); + var pending = Task.FromResult(item); + item.Dispose(); + var value = {|ERP046:await pending|}; + } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.LowNoiseReview.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.LowNoiseReview.cs new file mode 100644 index 0000000..ea3dee1 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.LowNoiseReview.cs @@ -0,0 +1,191 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("registry.Register(item);")] + [TestCase("registry.Identity(item);")] + public Task LowNoiseReview_Fluent_Receivers_Do_Not_Hide_Other_Unknown_Arguments(string body) + { + return VerifyAsync(@" +public interface IRegistry { IRegistry Register(Disposable value); } +public static class Extensions { public static T Identity(this T value, Disposable other) => value; } +public class Test +{ + public void Run(IRegistry registry) + { + var item = new Disposable(); + " + body + @" + item.ToString(); + } +}"); + } + + [TestCase("var item = new Disposable();", "using var result = await alias;")] + [TestCase("var {|ERP044:item|} = new Disposable();", "alias.Dispose();")] + public Task LowNoiseReview_Fluent_Carrier_Identity_Is_Not_Resource_Identity(string creation, string cleanup) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public static class Extensions { public static T Identity(this T value) => value; } +public class Test +{ + public async Task Run() + { + " + creation + @" + var pending = Task.FromResult(item); + var alias = pending.Identity(); + " + cleanup + @" + await Task.CompletedTask; + } +}"); + } + + [Test] + public Task LowNoiseReview_Fluent_Carriers_Preserve_Borrowing() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public static class Extensions { public static T Identity(this T value) => value; } +public class Test +{ + public async Task Run([DoNotDispose] Disposable item) + { + var alias = Task.FromResult(item).Identity(); + using var result = {|ERP046:await alias|}; + } +}"); + } + + [TestCase("System.Action cleanup = () => item.Dispose();")] + [TestCase("void cleanup() { item.Dispose(); }")] + public Task LowNoiseReview_Capture_Before_Acquisition_Is_Unknown(string callback) + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + Disposable item = null; + " + callback + @" + item = new Disposable(); + cleanup(); + } +}"); + } + + [Test] + public Task LowNoiseReview_Nameof_Before_Acquisition_Does_Not_Capture() + { + return VerifyAsync(@" +public class Test +{ + public void Run() + { + Disposable item = null; + System.Action callback = () => System.Console.WriteLine(nameof(item)); + {|ERP044:item|} = new Disposable(); + callback(); + } +}"); + } + + [TestCase("_ = this + item;")] + [TestCase("dynamic target = this; _ = target[item];")] + public Task LowNoiseReview_Operator_And_Dynamic_Indexer_Arguments_Are_Handoffs(string handoff) + { + return VerifyAsync(@" +public class Test +{ + public static Test operator +(Test target, Disposable value) => target; + public void Run() + { + var item = new Disposable(); + " + handoff + @" + item.ToString(); + } +}"); + } + + [Test] + public Task LowNoiseReview_Operator_Input_Contracts_Are_Independent() + { + return VerifyAsync(@" +public class Test +{ + public static int operator +(Test target, [DoNotDispose] Disposable value) => 0; + public static int operator -(Test target, [AcquiresOwnership] Disposable value) { value.Dispose(); return 0; } + public void Run([DoNotDispose] Disposable borrowed) + { + var {|ERP044:item|} = new Disposable(); + _ = this + item; + _ = this - {|ERP046:borrowed|}; + } +}"); + } + + [TestCase("_ = +item;")] + [TestCase("item++;")] + [TestCase("item += 1;")] + public Task LowNoiseReview_Unannotated_Unary_And_Compound_Operators_Are_Handoffs(string body) + { + return VerifyAsync(@" +public class Resource : System.IDisposable +{ + public void Dispose() { } + public static int operator +(Resource value) => 0; + public static Resource operator ++(Resource value) => value; + public static Resource operator +(Resource value, int count) => value; +} +public class Test +{ + public void Run() + { + var item = new Resource(); + " + body + @" + } +}"); + } + + [TestCase("Interlocked.Exchange(ref pending, {|ERP046:Task.FromResult(item)|});", true)] + [TestCase("Interlocked.Exchange(ref pending, Task.FromResult(item)); {|ERP046:item|}.Dispose();", false)] + public Task LowNoiseReview_Interlocked_Carriers_Respect_Ownership(string body, bool borrowed) + { + return VerifyAsync(@" +using System.Threading; +using System.Threading.Tasks; +public class Test +{ + private Task pending; + public void Run(" + (borrowed ? "[DoNotDispose] Disposable item" : "") + @") + { + " + (borrowed ? "" : "var item = new Disposable();") + @" + " + body + @" + } +}"); + } + + [TestCase("(await Task.FromResult(value)).Dispose();", true)] + [TestCase("var pending = Task.FromResult(value); using var result = await pending.ConfigureAwait(false);", true)] + [TestCase("var pending = Task.FromResult(value); var alias = pending; pending = null; (await alias).Dispose();", true)] + [TestCase("var pending = Task.FromResult(value); pending = Task.FromResult(null); (await pending).Dispose();", false)] + [TestCase("var pending = Task.FromResult(value); pending.Dispose(); await Task.CompletedTask;", false)] + [TestCase("_ = Task.FromResult(value); await Task.CompletedTask;", false)] + [TestCase("var pending = Task.FromResult(value); Unknown(pending); await Task.CompletedTask;", false)] + [TestCase("var pending = new ValueTask(value); (await pending).Dispose();", true)] + [TestCase("var pending = ValueTask.FromResult(value); (await pending).Dispose();", true)] + public Task LowNoiseReview_Source_Consumption_Follows_Completed_Carriers_Only_When_Consumed(string body, bool consumes) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + public Task Run([DoNotDispose] Disposable borrowed) => Consume(" + + (consumes ? "{|ERP046:borrowed|}" : "borrowed") + @"); + private static async Task Consume(Disposable value) { " + body + @" } + private static void Unknown(object pending) { } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.MoveSemantics.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.MoveSemantics.cs new file mode 100644 index 0000000..4893379 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.MoveSemantics.cs @@ -0,0 +1,279 @@ +using NUnit.Framework; +using System.Threading.Tasks; +using Verify = ErrorProne.NET.TestHelpers.CSharpCodeFixVerifier< + ErrorProne.NET.DisposableAnalyzers.DisposeBeforeLosingScopeAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers +{ + [TestFixture] + public partial class DisposeBeforeLosingScopeAnalyzerTests + { + private const string AcquiresOwnershipAttribute = + @" +[System.AttributeUsage(System.AttributeTargets.Parameter)] +public class AcquiresOwnershipAttribute : System.Attribute { } + +[System.AttributeUsage(System.AttributeTargets.All)] +public class ReturnsOwnershipAttribute : System.Attribute { } + +[System.AttributeUsage(System.AttributeTargets.Method)] +public class KeepsOwnershipAttribute : System.Attribute { } + +[System.AttributeUsage(System.AttributeTargets.All)] +public class NoOwnershipAttribute : System.Attribute { } + +[System.AttributeUsage(System.AttributeTargets.All)] +public class DoNotDisposeAttribute : System.Attribute { } + +public static class DisposableExtensions +{ + /// + /// A special method that allows to release the current ownership. + /// + [KeepsOwnership] + public static T ReleaseOwnership([AcquiresOwnership]this T disposable) where T : System.IDisposable + { + return disposable; + } + + public static T ThrowIfNull(this T t) => t; +}"; + + private const string Disposable = + @" +public class Disposable : System.IDisposable + { + public void Dispose() { } + public void Close() {} + public static Disposable Create() => new Disposable(); + }"; + + [Test] + public async Task NoWarn_On_Move() + { + var test = @" +public class Test +{ + public static void Moves() + { + var d = new Disposable(); + TakesOwnership(d); + } + + private static void TakesOwnership([AcquiresOwnership] Disposable d2) { d2.Dispose(); } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Out_Variable() + { + var test = @" +public class Test +{ + public void Moves(out Disposable d) + { + d = new Disposable(); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_ActivitySource_StartActivity() + { + var test = @" +public class Test +{ + public static void StartActivityCase() + { + using var source = new System.Diagnostics.ActivitySource(""name""); + using var activity = source.StartActivity(); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Move_To_List() + { + var test = @" +public class Test +{ + public static void Moves() + { + var d = new Disposable(); + var list = new System.Collections.Generic.List(){ d.ReleaseOwnership() }; + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Move_With_ReleaseOwnership() + { + var test = @" +public class Test +{ + public static void Moves() + { + var d = new Disposable().ReleaseOwnership(); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_ThrowIf() + { + var test = @" +public class Test +{ + public static void Moves(Disposable d) + { + d.ThrowIfNull(); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Non_FactoryMethod() + { + var test = @" +public class Test +{ + public static void Moves() + { + var d = NoOwnership(); + } + + [KeepsOwnershipAttribute] + private static Disposable NoOwnership() => new Disposable().ReleaseOwnership(); +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task Warn_On_Taken_Ownership() + { + var test = @" +public class Test +{ + private static void TakesOwnership([AcquiresOwnership] Disposable {|ERP044:d|}) { } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task Warn_On_Taken_Ownership_With_Usage() + { + var test = @" +public class Test +{ + private static string TakesOwnership([AcquiresOwnership] Disposable {|ERP044:d|}) + { + return d.ToString(); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Taken_Ownership_With_Dispose() + { + var test = @" +public class Test +{ + private static string TakesOwnership([AcquiresOwnership] Disposable d) + { + var r = d.ToString(); + d.Dispose(); + return r; + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Taken_Ownership_With_Using() + { + var test = @" +public class Test +{ + private static string TakesOwnership([AcquiresOwnership] Disposable d) + { + using (d) + { + return d.ToString(); + } + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Taken_Ownership_With_Moved_Ownership() + { + var test = @" +public class Test +{ + private static string TakesOwnershipAndDisposes([AcquiresOwnership] Disposable d) + { + using (d) + { + return d.ToString(); + } + } + + private static string TakesOwnership([AcquiresOwnership] Disposable d) + { + return TakesOwnershipAndDisposes(d); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Inferred_Disposal_By_Source_Callee() + { + var test = @" +public class Test +{ + private static string TakesOwnershipAndDisposes(Disposable d) + { + using (d) + { + return d.ToString(); + } + } + + private static string TakesOwnership([AcquiresOwnership] Disposable d) + { + return TakesOwnershipAndDisposes(d); + } +} +"; + await VerifyAsync(test); + } + + private static Task VerifyAsync(string code) + { + code += $"\n{AcquiresOwnershipAttribute}\n{Disposable}"; + return Verify.VerifyAsync(code); + } + } +} \ No newline at end of file diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.PrFeedback.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.PrFeedback.cs new file mode 100644 index 0000000..904ddba --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.PrFeedback.cs @@ -0,0 +1,244 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("int DisposeAsync() => 0")] + [TestCase("void DisposeAsync() => System.GC.KeepAlive(null)")] + [TestCase("System.Threading.Tasks.Task DisposeAsync() => System.Threading.Tasks.Task.CompletedTask")] + [TestCase("System.Threading.Tasks.ValueTask DisposeAsync() => default")] + public Task PrFeedback_Unrelated_DisposeAsync_Does_Not_End_Ownership(string method) + { + return VerifyAsync(@" +public class Resource : System.IDisposable +{ + public void Dispose() { } + public " + method + @"; +} +public class Test +{ + public void Run() + { + var {|ERP044:resource|} = new Resource(); + resource.DisposeAsync(); + resource.ToString(); + } +}"); + } + + [TestCase("int", "0")] + [TestCase("System.Threading.Tasks.ValueTask", "default")] + public Task PrFeedback_Public_Lookalike_Is_Not_An_Explicit_Async_Implementation(string returnType, string result) + { + return VerifyAsync(@" +public class Resource : System.IAsyncDisposable +{ + System.Threading.Tasks.ValueTask System.IAsyncDisposable.DisposeAsync() => default; + public " + returnType + " DisposeAsync() => " + result + @"; +} +public class Test +{ + public void Run() + { + var {|ERP044:resource|} = new Resource(); + resource.DisposeAsync(); + resource.ToString(); + } +}"); + } + + [Test] + public Task PrFeedback_Lookalike_Is_Not_Borrowed_Cleanup_Or_Inferred_Acquisition() + { + return VerifyAsync(@" +public class Resource : System.IDisposable +{ + public void Dispose() { } + public int DisposeAsync() => 0; +} +public class Test +{ + private static void Inspect(Resource resource) { resource.DisposeAsync(); } + public void Run([DoNotDispose] Resource resource) + { + resource.DisposeAsync(); + Inspect(resource); + resource.ToString(); + } +}"); + } + + [TestCase("resource.DisposeAsync();")] + [TestCase("((System.IAsyncDisposable)resource).DisposeAsync();")] + [TestCase("System.Threading.Tasks.TaskAsyncEnumerableExtensions.ConfigureAwait(resource, false).DisposeAsync();")] + public Task PrFeedback_Actual_Async_Disposal_Still_Discharges_Ownership(string cleanup) + { + return VerifyAsync(@" +public class Resource : System.IAsyncDisposable +{ + public System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Test +{ + public void Run() + { + var resource = new Resource(); + " + cleanup + @" + } +}"); + } + + [TestCase("")] + [TestCase("public override System.Threading.Tasks.ValueTask DisposeAsync() => base.DisposeAsync();")] + public Task PrFeedback_Inherited_And_Overridden_Async_Disposal_Are_Recognized(string implementation) + { + return VerifyAsync(@" +public class Base : System.IAsyncDisposable +{ + public virtual System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Resource : Base { " + implementation + @" } +public class Test +{ + public void Run() + { + var resource = new Resource(); + resource.DisposeAsync(); + } +}"); + } + + [Test] + public Task PrFeedback_Reimplemented_Async_Disposal_Does_Not_Trust_An_Inherited_Lookalike() + { + return VerifyAsync(@" +public class Base : System.IAsyncDisposable +{ + public System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Resource : Base, System.IAsyncDisposable +{ + System.Threading.Tasks.ValueTask System.IAsyncDisposable.DisposeAsync() => default; +} +public class Test +{ + public void Run() + { + var {|ERP044:resource|} = new Resource(); + resource.DisposeAsync(); + } +}"); + } + + [Test] + public Task PrFeedback_Generic_DisposeAsync_Lookalike_Does_Not_End_Ownership() + { + return VerifyAsync(@" +public class Resource : System.IAsyncDisposable +{ + public System.Threading.Tasks.ValueTask DisposeAsync() => default; + public System.Threading.Tasks.ValueTask DisposeAsync() => default; +} +public class Test +{ + public void Run() + { + var {|ERP044:resource|} = new Resource(); + resource.DisposeAsync(); + } +}"); + } + + [TestCase("object", "")] + [TestCase("T", "")] + public Task PrFeedback_Explicit_Acquisition_Requires_Callee_Cleanup_For_Erased_Types(string type, string typeParameters) + { + return VerifyAsync(@" +public class Test +{ + private static void Take" + typeParameters + "([AcquiresOwnership] " + type + @" {|ERP044:value|}) { } + public void Run() + { + Take(new Disposable()); + } +}"); + } + + [TestCase("object", "", "((System.IDisposable)value).Dispose();")] + [TestCase("T", "", "(value as System.IDisposable)?.Dispose();")] + [TestCase("object", "", "if (value is System.IDisposable resource) { resource.Dispose(); }")] + [TestCase("T", "", "if (value is not System.IDisposable resource) return; resource.Dispose();")] + public Task PrFeedback_Erased_Acquiring_Parameters_Can_Be_Disposed(string type, string typeParameters, string cleanup) + { + return VerifyAsync(@" +public class Test +{ + private static void Take" + typeParameters + "([AcquiresOwnership] " + type + @" value) + { + " + cleanup + @" + } + public void Run() + { + Take(new Disposable()); + } +}"); + } + + [Test] + public Task PrFeedback_Pattern_Matching_Alone_Does_Not_Discharge_An_Acquiring_Parameter() + { + return VerifyAsync(@" +public class Test +{ + public void Take([AcquiresOwnership] T {|ERP044:value|}) + { + if (value is System.IDisposable resource) { resource.ToString(); } + } +}"); + } + + [Test] + public Task PrFeedback_Pattern_Aliases_Preserve_Borrowing() + { + return VerifyAsync(@" +public class Test +{ + public void Run([DoNotDispose] object value) + { + if (value is System.IDisposable resource) { {|ERP046:resource|}.Dispose(); } + } +}"); + } + + [TestCase("Dispose", "{|ERP046:value|}")] + [TestCase("ToString", "value")] + public Task PrFeedback_Pattern_Based_Source_Acquisition_Requires_Actual_Cleanup(string method, string argument) + { + return VerifyAsync(@" +public class Test +{ + private static void Inspect(object value) + { + if (value is System.IDisposable resource) { resource." + method + @"(); } + } + public void Run([DoNotDispose] Disposable value) { Inspect(" + argument + @"); } +}"); + } + + [TestCase("object", "")] + [TestCase("T", "")] + public Task PrFeedback_Erased_Acquiring_Parameters_Can_Transfer_To_Storage(string type, string typeParameters) + { + return VerifyAsync(@" +public class Test +{ + private object stored; + public void Take" + typeParameters + "([AcquiresOwnership] " + type + @" value) + { + stored = value; + } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.ReviewRegressions.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.ReviewRegressions.cs new file mode 100644 index 0000000..9544839 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.ReviewRegressions.cs @@ -0,0 +1,297 @@ +using System.Threading.Tasks; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +public partial class DisposeBeforeLosingScopeAnalyzerTests +{ + [TestCase("var d = new Disposable(); using (d) { {|ERP044:d|} = new Disposable(); }")] + [TestCase("var d = new Disposable(); using (var captured = d) { {|ERP044:d|} = new Disposable(); }")] + [TestCase("using (new Disposable()) { }")] + [TestCase("Disposable d; using (d = new Disposable()) { }")] + [TestCase("var d = new Disposable(); using (d) { d = null; }")] + public Task Using_Captures_The_Original_Owned_Value(string body) + { + return VerifyAsync("public class Test { public void Run() { " + body + " } }"); + } + + [TestCase("using (d) { d = null; }")] + [TestCase("using (var captured = d) { d = null; }")] + public Task Infers_Using_Consumption_Before_Parameter_Reassignment(string body) + { + return VerifyAsync(@" +public class Test +{ + public void Run() { Consume(new Disposable()); } + private static void Consume(Disposable d) { " + body + @" } +}"); + } + + [TestCase("using (Disposable first = {|ERP046:borrowed|}, second = borrowed = new Disposable()) { }")] + [TestCase("using Disposable first = {|ERP046:borrowed|}, second = borrowed = new Disposable();")] + public Task Using_Initializers_Capture_Borrowing_Independently(string body) + { + return VerifyAsync("public class Test { public void Run([DoNotDispose] Disposable borrowed) { " + body + " } }"); + } + + [TestCase("var d = new Resource(); await using (d) { {|ERP044:d|} = new Resource(); }")] + [TestCase("var d = new Resource(); await using (d.ConfigureAwait(false)) { {|ERP044:d|} = new Resource(); }")] + [TestCase("await using (new Resource()) { }")] + [TestCase("await using (new Resource().ConfigureAwait(false)) { }")] + [TestCase("var d = new Resource(); await using (d.ConfigureAwait(false)) { }")] + [TestCase("var d = new Resource(); await using var captured = d.ConfigureAwait(false);")] + [TestCase("var d = new Resource(); var configured = d.ConfigureAwait(false); await using (configured) { }")] + [TestCase("var d = new Resource(); await d.ConfigureAwait(false).DisposeAsync();")] + [TestCase("var d = new Resource(); await using (TaskAsyncEnumerableExtensions.ConfigureAwait(continueOnCapturedContext: false, source: d)) { }")] + public Task Configured_Async_Disposal_Preserves_Owned_Identity(string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Resource : System.IAsyncDisposable +{ + public ValueTask DisposeAsync() => default; +} +public class Test { public async Task Run() { " + body + @" } }"); + } + + [TestCase("await using (d) { d = null; }")] + [TestCase("await using (d.ConfigureAwait(false)) { d = null; }")] + [TestCase("await using var captured = d.ConfigureAwait(false);")] + [TestCase("var configured = d.ConfigureAwait(false); await using (configured) { }")] + public Task Infers_Configured_Async_Disposal_Of_The_Captured_Parameter(string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Resource : System.IAsyncDisposable +{ + public ValueTask DisposeAsync() => default; +} +public class Test +{ + public async Task Run() { await Consume(new Resource()); } + private static async Task Consume(Resource d) { " + body + @" } +}"); + } + + [TestCase("await using ({|ERP046:d.ConfigureAwait(false)|}) { }")] + [TestCase("await using var captured = {|ERP046:d.ConfigureAwait(false)|};")] + [TestCase("var configured = d.ConfigureAwait(false); await using ({|ERP046:configured|}) { }")] + [TestCase("await {|ERP046:d.ConfigureAwait(false)|}.DisposeAsync();")] + [TestCase("await using ({|ERP046:d.ConfigureAwait(false)|}) { d = new Resource(); await d.DisposeAsync(); }")] + public Task Configured_Async_Disposal_Preserves_Borrowed_Identity(string body) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Resource : System.IAsyncDisposable +{ + public ValueTask DisposeAsync() => default; +} +public class Test { public async Task Run([DoNotDispose] Resource d) { " + body + @" } }"); + } + + [Test] + public Task Unrelated_ConfigureAwait_Methods_Are_Not_Disposal_Aliases() + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Resource : System.IAsyncDisposable +{ + public ValueTask DisposeAsync() => default; + public Other ConfigureAwait(bool unused) => new Other(); +} +public class Other : System.IAsyncDisposable +{ + public ValueTask DisposeAsync() => default; +} +public class Test +{ + public async Task Run([DoNotDispose] Resource borrowed) + { + var {|ERP044:owned|} = new Resource(); + await using (owned.ConfigureAwait(false)) { } + await using (borrowed.ConfigureAwait(false)) { } + } +}"); + } + + [TestCase("using var converted = (Other)borrowed;")] + [TestCase("using var converted = (Other)(dynamic)borrowed;")] + [TestCase("var {|ERP044:owned|} = new Disposable(); using var converted = (Other)owned;")] + [TestCase("var {|ERP044:owned|} = new Disposable(); using var converted = (Other)(dynamic)owned;")] + public Task User_Defined_Conversions_Do_Not_Imply_Resource_Identity(string body) + { + return VerifyAsync(@" +public sealed class Other : System.IDisposable +{ + public void Dispose() { } + public static implicit operator Other([DoNotDispose] Disposable value) => new Other(); +} +public class Test { public void Run([DoNotDispose] Disposable borrowed) { " + body + @" } }"); + } + + [Test] + public Task Inferring_Consumption_Does_Not_Follow_User_Defined_Conversions() + { + return VerifyAsync(@" +public sealed class Other : System.IDisposable +{ + public void Dispose() { } + public static implicit operator Other(Disposable value) => new Other(); +} +public class Test +{ + public void Run([DoNotDispose] Disposable borrowed) { Consume(borrowed); } + private static void Consume(Disposable d) { using var converted = (Other)d; } +}"); + } + + [Test] + public Task Conversion_Input_And_Result_Contracts_Are_Independent() + { + return VerifyAsync(@" +public sealed class Other : System.IDisposable +{ + public void Dispose() { } + [return: ReturnsOwnership] + public static implicit operator Other([AcquiresOwnership] Disposable value) + { + value.Dispose(); + return new Other(); + } +} +public class Test +{ + public void Run([DoNotDispose] Disposable borrowed) + { + var d = new Disposable(); + var {|ERP044:converted|} = (Other)d; + using var invalid = (Other){|ERP046:borrowed|}; + } +}"); + } + + [Test] + public Task Borrowed_Conversion_Results_Cannot_Be_Disposed() + { + return VerifyAsync(@" +public sealed class Other : System.IDisposable +{ + public void Dispose() { } + [return: DoNotDispose] + public static implicit operator Other(Disposable value) => null; +} +public class Test +{ + public void Run(Disposable d) { using var converted = {|ERP046:(Other)d|}; } +}"); + } + + [TestCase("(Other)(Disposable)resource", false)] + [TestCase("(Other)(Disposable)resource", true)] + [TestCase("Other.Replace((Disposable)resource)", false)] + [TestCase("Other.Replace((Disposable)resource)", true)] + public Task Consuming_Expressions_Update_Aliases_In_Their_Containing_Statement(string expression, bool keepOldAlias) + { + return VerifyAsync(@" +public sealed class Other : System.IDisposable +{ + public void Dispose() { } + [return: ReturnsOwnership] + public static implicit operator Other([AcquiresOwnership] Disposable value) => Replace(value); + [return: ReturnsOwnership] + public static Other Replace([AcquiresOwnership] Disposable value) + { + value.Dispose(); + return new Other(); + } +} +public class Test +{ + public void Run() + { + System.IDisposable resource = new Disposable(); + " + (keepOldAlias ? "var old = resource;" : "") + @" + resource = " + expression + @"; + resource.Dispose(); + " + (keepOldAlias ? "{|ERP046:old|}.Dispose();" : "") + @" + } +}"); + } + + [Test] + public Task Indexer_Arguments_Honor_Acquisition_And_Borrowing() + { + return VerifyAsync(@" +public class Test +{ + public int this[[AcquiresOwnership] Disposable value, Disposable other] + { + get { value?.Dispose(); return 0; } + } + public void Run([DoNotDispose] Disposable borrowed) + { + _ = this[other: null, value: new Disposable()]; + _ = this[other: null, value: {|ERP046:borrowed|}]; + var retained = new Disposable(); + _ = this[other: retained, value: null]; + retained.Dispose(); + Forward(new Disposable()); + } + private void Forward(Disposable d) { _ = this[other: null, value: d]; } +}"); + } + + [TestCase("borrowed")] + [TestCase("_borrowed")] + [TestCase("GetBorrowed()")] + [TestCase("alias")] + public Task Borrowed_Values_Cannot_Transfer_Through_Interlocked_Exchange(string value) + { + return VerifyAsync(@" +public class Test +{ + private Disposable _owned; + [DoNotDispose] private Disposable _borrowed; + [return: DoNotDispose] private static Disposable GetBorrowed() => null; + public void Run([DoNotDispose] Disposable borrowed) + { + var alias = borrowed; + System.Threading.Interlocked.Exchange(value: {|ERP046:" + value + @"|}, location1: ref _owned); + System.Threading.Interlocked.Exchange(ref _borrowed, " + value + @"); + } +}"); + } + + [Test] + public Task Infers_Transfers_Through_Interlocked_Exchange() + { + return VerifyAsync(@" +public class Test +{ + private Disposable _owned; + public void Run() { Store(new Disposable()); } + private void Store(Disposable value) { System.Threading.Interlocked.Exchange(ref _owned, value); } +}"); + } + + [TestCase("Task", "")] + [TestCase("Task", ".ConfigureAwait(false)")] + [TestCase("ValueTask", "")] + [TestCase("ValueTask", ".ConfigureAwait(false)")] + public Task Awaited_Properties_Preserve_Explicit_Ownership_Contracts(string taskType, string configure) + { + return VerifyAsync(@" +using System.Threading.Tasks; +public class Test +{ + [ReturnsOwnership] private " + taskType + @" Owned => default; + [DoNotDispose] private " + taskType + @" Borrowed => default; + public async Task Run() + { + var {|ERP044:owned|} = await Owned" + configure + @"; + using var borrowed = {|ERP046:await Borrowed" + configure + @"|}; + using var cleaned = await Owned" + configure + @"; + } +}"); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.cs new file mode 100644 index 0000000..4a68081 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzerTests.cs @@ -0,0 +1,819 @@ +using NUnit.Framework; +using System.Threading.Tasks; +using ErrorProne.NET.DisposableAnalyzers; +using Verify = ErrorProne.NET.TestHelpers.CSharpCodeFixVerifier< + ErrorProne.NET.DisposableAnalyzers.DisposeBeforeLosingScopeAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers +{ + [TestFixture] + public partial class DisposeBeforeLosingScopeAnalyzerTests + { + [Test] + public async Task Warn_On_No_Dispose() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var {|ERP044:d1|} = new Disposable(); + var nd = new NonDisposable(); + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } + + public class NonDisposable { } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task Warn_On_No_Dispose_With_Cast() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var {|ERP044:d1|} = new Disposable() as object; + var {|ERP044:d2|} = (object)new Disposable(); + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task Warn_On_No_Dispose_In_If_Block() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + if ({|ERP044:new Disposable()|} is null) + { + } + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task Warn_On_No_Dispose_With_Factory_Method() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var {|ERP044:d|} = Disposable.Create(); + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + public static Disposable Create() => new Disposable(); + } + + public class NonDisposable { } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_No_Dispose_With_Property() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var d = Instance; + } + + public static Disposable Instance => new Disposable().ReleaseOwnership(); +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task Warn_On_No_Dispose_With_Factory_Property() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var {|ERP044:d|} = Disposable.Instance; + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + + [ReturnsOwnership] + public static Disposable Instance => new Disposable(); + } + + public class NonDisposable { } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_UsingDeclaration() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + using var d = new Disposable(); + using var _ = new Disposable(); + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Manual_Dispose() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var d = new Disposable(); + d.Dispose(); + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Usings() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + using var d1 = new Disposable(); + using var d2 = Disposable.Create(); + using var d3 = Disposable.Instance; + using var _ = new Disposable(); + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + public static Disposable Create() => new Disposable(); + public static Disposable Instance => new Disposable(); + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_SimpleReturn() + { + var test = @" +public class Test +{ + public class Disposable : System.IDisposable + { + public void Dispose() { } + public static Disposable Create() => new Disposable(); + public static Disposable Create2() + { + return new Disposable(); + } + public static Disposable Instance => new Disposable(); + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Conditional_Return() + { + var test = @" +public class Test +{ + public class Disposable : System.IDisposable + { + public void Dispose() { } + public static Disposable Create2(bool check) + { + var result = new Disposable(); + if (check) + { + return result; + } + else + { + return result; + } + } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Conditional_ReturnInCatchFinally() + { + var test = @" +public class Test +{ + public class Disposable : System.IDisposable + { + public void Dispose() { } + public static Disposable Create2(bool check) + { + var result = new Disposable(); + try + { + return result; + } + catch + { + return result; + } + } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Conditional_Return_Conditionally_In_All_Branches() + { + var test = @" +public class Test +{ + public class Disposable : System.IDisposable + { + public void Dispose() { } + public static Disposable Create2(bool check) + { + var result = new Disposable(); + try + { + if (check) + { + return result; + } + else + { + return result; + } + } + catch + { + return result; + } + } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_UsingStatement() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + using (var d = new Disposable()) + using (var d2 = new Disposable()) + using (var d3 = Disposable.Create()) + { + } + + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + public static Disposable Create() => new Disposable(); + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_UsingStatement_In_Local() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + void useInHelper() + { + using (var d4 = Disposable.Create()) + { + } + } + + System.Threading.Tasks.Task.Run(() => + { + using (var d5 = Disposable.Create()) + { + } + }); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_NullableDispose() + { + var test = @" +public class Test +{ + public static void ShouldDispose(bool shouldCreate) + { + Disposable d = null; + try + { + if (shouldCreate) + { + d = new Disposable(); + } + } + finally + { + d?.Dispose(); + } + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Nullable_Factory() + { + var test = @" +public class Test +{ + public static Disposable TryCreate(bool shouldCreate) + { + + try + { + Disposable d = new Disposable(); + return d; + } + catch + { + return null; + } + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_StreamReader() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + using (var fs = new System.IO.FileStream(string.Empty, System.IO.FileMode.Open, System.IO.FileAccess.Read, System.IO.FileShare.ReadWrite)) + using (var sr = new System.IO.StreamReader(fs)) + { + } + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Activity() + { + var test = @" +public class Test +{ + public class Activity : System.IDisposable + { + public Activity SetTag(string key, object value) => this; + public void Dispose() { } + } + + public static void ActivityCase(Activity a) + { + a.SetTag(""42"", 42); + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Dispose_In_Finally() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var d = new Disposable(); + try + { + } + finally + { + d.Dispose(); + } + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Close_In_Finally() + { + var test = @" +public class Test +{ + public static void ShouldDispose(bool create) + { + Disposable d = null; + + try + { + if (create) + { + d = new Disposable(); + } + } + finally + { + if (d != null) + d.Close(); + } + + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Nested_Using() + { + var test = @" +public class Test +{ + public static void ShouldDispose(bool create) + { + Disposable d = null; + + if (create) + { + d = new Disposable(); + } + + using(d) + { + } + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Assigning_To_Field() + { + var test = @" +public class Test +{ + private Disposable _d; + public void ShouldDispose(bool create) + { + Disposable d = null; + + if (create) + { + d = new Disposable(); + } + + _d = d; + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Assigning_To_Field_With_Interlocked() + { + var test = @" +public class Test +{ + private Disposable _d; + public void ShouldDispose(bool create) + { + Disposable d = null; + + if (create) + { + d = new Disposable(); + } + + System.Threading.Interlocked.Exchange(ref _d, d); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Assigning_To_Field_In_Constructor() + { + var test = @" +public class Test +{ + private Disposable _d; + private Disposable _d2; + public Test(bool create) + { + Disposable d = null; + + if (create) + { + d = new Disposable(); + } + + _d = d; + _d2 = new Disposable(); + } +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Dispose_In_Try_And_Catch() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var d = new Disposable(); + try + { + d.Dispose(); + } + catch + { + d.Dispose(); + } + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Disposable_Returned_In_Func() + { + var test = @" +public class Test +{ + internal static System.Func CreateInstance = () => new Disposable(); +} +"; + await VerifyAsync(test); + } + + // await using (var buildCoordinator = new BuildCoordinator( + + [Test] + public async Task NoWarn_On_Disposable_Returned_In_Func_InTask() + { + var test = @" +public class Test +{ + public static void TestTask() + { + System.Threading.Tasks.Task.Run(() => + { + var d = new Disposable(); + return d; + }); + } +}"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Type_Erasure() + { + // This should be covered by another rule. + var test = @" +public interface IFoo { } + +public class Foo : IFoo, System.IDisposable +{ + public void Dispose() { } + public static IFoo Create() => new Foo(); +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Yield_Return() + { + var test = @" +public class Test +{ + public static System.Collections.Generic.IEnumerable Get() + { + yield return new Disposable(); + } +} +"; + await VerifyAsync(test); + } + + // [Test] Not supported yet. + public async Task Warn_On_DisposeInFinally_Conditionally() + { + var test = @" +public class Test +{ + public static void ShouldDispose(bool shouldDispose) + { + var {|ERP044:d|} = new Disposable(); + try + { + } + finally + { + if (shouldDispose) + d.Dispose(); + } + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Dispose_In_Try_Catch() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + var d = new Disposable(); + try + { + } + finally + { + d.Dispose(); + } + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_UsingStatement_With_No_Local() + { + var test = @" +public class Test +{ + public static void ShouldDispose() + { + using (new Disposable()) + { + } + } + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + [Test] + public async Task NoWarn_On_Field() + { + var test = @" +public class Test +{ + private readonly Disposable _d = new Disposable(); + + public class Disposable : System.IDisposable + { + public void Dispose() { } + } +} +"; + await Verify.VerifyAsync(test); + } + + + // Positive test cases: + // * var d = new Disposable() + // * var d = CreateDisposable(); + // * var d = await CreateDisposableAsync(); + // * var d = DisposableProperty; + // * var d = FooBar.DisposableProperty; + + // Disposed only in 'catch' block + + // Dispose if a type has 'Dispose' method and is ref struct? + + // Negative test cases + + // Configure a list of disposable types that should not be disposed. + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeTaskAnalyzerTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeTaskAnalyzerTests.cs new file mode 100644 index 0000000..09673a6 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/DisposeTaskAnalyzerTests.cs @@ -0,0 +1,67 @@ +using NUnit.Framework; +using System.Threading.Tasks; +using ErrorProne.NET.DisposableAnalyzers; +using Verify = ErrorProne.NET.TestHelpers.CSharpCodeFixVerifier< + ErrorProne.NET.AsyncAnalyzers.TaskInUsingBlockAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers +{ + [TestFixture] + public partial class DisposeTaskAnalyzerTests + { + private const string Disposable = + @" +public class Disposable : System.IDisposable + { + public void Dispose() { } + public void Close() {} + public static Disposable Create() => new Disposable(); + }"; + + private static Task VerifyAsync(string code) + { + + code = $"using System.Threading.Tasks;\n{code}\n{Disposable}"; + return Verify.VerifyAsync(code); + } + + [Test] + public async Task Warn_On_Using_Var_On_Task() + { + var test = @" +public class Test +{ + public static async Task ShouldDispose() + { + [|using var x = GetDisposableAsync();|] + await Task.Yield(); + } + + private static Task GetDisposableAsync() => null; +} +"; + await VerifyAsync(test); + } + + [Test] + public async Task Warn_On_Using_On_Task() + { + var test = @" +public class Test +{ + public static async Task ShouldDispose() + { + await Task.Yield(); + [|using (GetDisposableAsync()) + { + }|] + } + + private static Task GetDisposableAsync() => null; +} +"; + await VerifyAsync(test); + } + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/OwnershipContractsTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/OwnershipContractsTests.cs new file mode 100644 index 0000000..f2fc035 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/DisposableAnalyzers/OwnershipContractsTests.cs @@ -0,0 +1,749 @@ +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using ErrorProne.NET.TestHelpers; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; +using Verify = ErrorProne.NET.TestHelpers.CSharpCodeFixVerifier< + ErrorProne.NET.DisposableAnalyzers.DisposeBeforeLosingScopeAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +namespace ErrorProne.NET.CoreAnalyzers.Tests.DisposableAnalyzers; + +[TestFixture] +public sealed class OwnershipContractsTests +{ + private const string Prelude = @" +[System.AttributeUsage(System.AttributeTargets.All)] +public sealed class AcquiresOwnershipAttribute : System.Attribute { } +[System.AttributeUsage(System.AttributeTargets.All)] +public sealed class NoOwnershipAttribute : System.Attribute { } +[System.AttributeUsage(System.AttributeTargets.All)] +public sealed class ReturnsOwnershipAttribute : System.Attribute { } +[System.AttributeUsage(System.AttributeTargets.All)] +public sealed class KeepsOwnershipAttribute : System.Attribute { } +[System.AttributeUsage(System.AttributeTargets.All)] +public sealed class DoNotDisposeAttribute : System.Attribute { } +public sealed class Resource : System.IDisposable +{ + public void Dispose() { } +} +"; + + private static Verify.Test CreateTest(string source, string xml) + { + var test = new Verify.Test + { + LanguageVersion = LanguageVersion.Latest, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + TestCode = source + Prelude, + }.WithoutGeneratedCodeVerification(); + test.TestState.AdditionalFiles.Add(("Contracts.ownership.xml", xml)); + return test; + } + + [TestCase("object", "", "M:Api.Take(System.Object)")] + [TestCase("T", "", "M:Api.Take``1(``0)")] + public Task ExternalAcquisitionChecksErasedCalleeParameters(string type, string typeParameters, string memberId) + { + return CreateTest(@" +class Api +{ + public static void Take" + typeParameters + "(" + type + @" {|ERP044:value|}) { } + public static void Run() { Take(new Resource()); } +}", $@"") + .RunAsync(); + } + + [TestCase("Task", "Task.FromResult")] + [TestCase("ValueTask", "ValueTask.FromResult")] + public Task ExternalAcquisitionTransfersAndChecksAwaitedParameterResults(string type, string factory) + { + return CreateTest(@" +using System.Threading.Tasks; +class Api +{ + public static async Task Take(" + type + @" pending) { (await pending).Dispose(); } + public static async Task Run([DoNotDispose] Resource borrowed) + { + await Take({|ERP046:" + factory + @"(borrowed)|}); + var resource = new Resource(); + await Take(" + factory + @"(resource)); + {|ERP046:resource|}.ToString(); + } +}", @"") + .RunAsync(); + } + + [Test] + public Task ExternalBorrowedReturnsRemainBorrowedThroughLocalAliases() + { + return CreateTest(@" +class Api +{ + public static Resource Read() => null; + public static void Run() + { + var resource = Read(); + var alias = resource; + {|ERP046:alias|}.Dispose(); + } +}", @"").RunAsync(); + } + + [Test] + public Task DoNotDisposeOverridesAnExternalOwningReturn() + { + return CreateTest(@" +class Api +{ + [return: DoNotDispose] public static Resource Read() => null; + public static void Run() { var resource = Read(); {|ERP046:resource|}.Dispose(); } +}", @"").RunAsync(); + } + + [Test] + public Task DoNotDisposeParameterOverridesSourceDisposalInference() + { + return CreateTest(@" +class Api +{ + public static void Inspect([DoNotDispose] Resource resource) { {|ERP046:resource|}.Dispose(); } + public static void Run() + { + var resource = new Resource(); + Inspect(resource); + resource.Dispose(); + } +}", "").RunAsync(); + } + + [TestCase(false)] + [TestCase(true)] + public Task ExternalBorrowedReturnsDoNotSuppressAcquiredParameters(bool dispose) + { + return CreateTest(@" +class Api +{ + public static Resource BorrowOther( + Resource " + (dispose ? "owned" : "{|ERP044:owned|}") + @", Resource borrowed) + { + " + (dispose ? "owned.Dispose();" : "") + @" + return borrowed; + } + public static void Run(Resource borrowed) + { + var result = BorrowOther(new Resource(), borrowed); + } +}", @" + + + + +").RunAsync(); + } + + [Test] + public Task ExternalReturnOwnershipIsHonored() + { + return CreateTest(@" +class Api +{ + public static Resource Read() => null; + public static Resource Create() => null; + public static void Run() + { + var {|ERP044:owned|} = Read(); + var borrowed = Create(); + } +}", @" + + +").RunAsync(); + } + + [Test] + public Task ExternalPropertyOwnershipIsHonored() + { + return CreateTest(@" +class Api +{ + public static Resource Owned => null; + public static Resource Borrowed => null; + public static void Run() + { + var {|ERP044:owned|} = Owned; + var borrowed = Borrowed; + } +}", @" + + +").RunAsync(); + } + + [Test] + public Task ExternalBorrowingOverridesDirectFreshReturnInference() + { + return CreateTest(@" +class Api +{ + public static Resource Create() => new Resource(); + public static void Run() { var resource = Create(); {|ERP046:resource|}.Dispose(); } +}", @"").RunAsync(); + } + + [TestCase("Task", "")] + [TestCase("Task", ".ConfigureAwait(false)")] + [TestCase("ValueTask", "")] + [TestCase("ValueTask", ".ConfigureAwait(false)")] + public Task ExternalAwaitedPropertyContractsAreHonored(string taskType, string configure) + { + return CreateTest(@" +using System.Threading.Tasks; +class Api +{ + public static " + taskType + @" Owned => default; + public static " + taskType + @" Borrowed => default; + public static async Task Run() + { + var {|ERP044:owned|} = await Owned" + configure + @"; + using var borrowed = {|ERP046:await Borrowed" + configure + @"|}; + using var cleaned = await Owned" + configure + @"; + } +}", @" + + +").RunAsync(); + } + + [Test] + public Task ExternalInterlockedParameterContractDoesNotDuplicateBorrowedDiagnostics() + { + return CreateTest(@" +class Api +{ + private Resource _owned; + public void Run([DoNotDispose] Resource borrowed) + { + System.Threading.Interlocked.Exchange(ref _owned, {|ERP046:borrowed|}); + } +}", @" + + + +").RunAsync(); + } + + [Test] + public Task ExternalIndexerParameterContractsAreHonored() + { + return CreateTest(@" +class Api +{ + public int this[Resource value, Resource other] + { + get { value?.Dispose(); return 0; } + } + public void Run([DoNotDispose] Resource borrowed) + { + _ = this[other: null, value: new Resource()]; + _ = this[other: null, value: {|ERP046:borrowed|}]; + var retained = new Resource(); + _ = this[other: retained, value: null]; + retained.Dispose(); + Forward(new Resource()); + } + private void Forward(Resource d) { _ = this[other: null, value: d]; } +}", @" + + + +").RunAsync(); + } + + [TestCase("Task", "owned")] + [TestCase("Task", "borrowed")] + [TestCase("ValueTask", "owned")] + [TestCase("ValueTask", "borrowed")] + public Task ExplicitConfigureAwaitContractsOverrideUnderlyingPropertyContracts(string taskType, string ownership) + { + var body = ownership == "owned" + ? "var {|ERP044:resource|} = await Pending.ConfigureAwait(false);" + : "using var resource = {|ERP046:await Pending.ConfigureAwait(false)|};"; + var attribute = ownership == "owned" ? "DoNotDispose" : "ReturnsOwnership"; + return CreateTest(@" +using System.Threading.Tasks; +class Api +{ + [" + attribute + @"] private " + taskType + @" Pending => default; + public async Task Run() { " + body + @" } +}", @" + +").RunAsync(); + } + + [TestCase(false, false)] + [TestCase(false, true)] + [TestCase(true, false)] + [TestCase(true, true)] + public async Task ExternalLibraryBoundariesNeedExplicitContracts(bool annotated, bool compiled) + { + var references = await ReferenceAssemblies.Net.Net80.ResolveAsync(LanguageNames.CSharp, CancellationToken.None); + var library = CSharpCompilation.Create("Library", + new[] { CSharpSyntaxTree.ParseText(@" +namespace Library +{ + public class Resource : System.IDisposable { public void Dispose() { } } + public static class Factory + { + private static readonly Resource shared = new Resource(); + public static Resource Create() => new Resource(); + public static Resource GetShared() => shared; + } + public static class Resources { public static Resource Shared { get; } = new Resource(); } + public static class Consumer { public static void Take(System.IDisposable resource) { resource.Dispose(); } } +}") }, + references, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + + var test = CreateTest(@" +class Api +{ + public static void Run() + { + var " + (annotated ? "{|ERP044:resource|}" : "resource") + @" = Library.Factory.Create(); + var borrowed = Library.Factory.GetShared(); + var alias = borrowed; + " + (annotated ? "{|ERP046:alias|}" : "alias") + @".Dispose(); + using var property = " + (annotated ? "{|ERP046:Library.Resources.Shared|}" : "Library.Resources.Shared") + @"; + Library.Consumer.Take(" + (annotated ? "{|ERP046:Library.Factory.GetShared()|}" : "Library.Factory.GetShared()") + @"); + var transferred = Library.Factory.Create(); + Library.Consumer.Take(transferred); + " + (annotated ? "{|ERP046:transferred|}" : "transferred") + @".Dispose(); + Library.Consumer.Take(new Library.Resource()); + } +}", annotated + ? @" + + + + + + +" + : ""); + if (compiled) + { + using var image = new MemoryStream(); + var result = library.Emit(image); + Assert.That(result.Success, Is.True, string.Join("\n", result.Diagnostics)); + test.TestState.AdditionalReferences.Add(MetadataReference.CreateFromImage(image.ToArray())); + } + else + { + test.TestState.AdditionalReferences.Add(library.ToMetadataReference()); + } + await test.RunAsync(); + } + + [Test] + public Task ExternalMoveIsTrustedByCallerAndCheckedAtCallee() + { + return CreateTest(@" +class Api +{ + public static void Take(Resource {|ERP044:resource|}) { } + public static void Run() + { + var moved = new Resource(); + Take(moved); + } +}", @" + + + +").RunAsync(); + } + + [Test] + public Task ParameterNamesAndOverloadsUseBoundArguments() + { + return CreateTest(@" +class Api +{ + public static void Take(int count, Resource resource) { resource.Dispose(); } + public static void Take(Resource resource) { } + public static void Run() + { + var moved = new Resource(); + Take(resource: moved, count: 1); + var {|ERP044:retained|} = new Resource(); + Take(retained); + } +}", @" + + + + + + +").RunAsync(); + } + + [Test] + public Task BorrowedParameterDoesNotTransferCallerOwnership() + { + return CreateTest(@" +class Api +{ + public static void Take(Resource resource) { } + public static void Run() + { + var {|ERP044:retained|} = new Resource(); + Take(retained); + } +}", @" + + + +").RunAsync(); + } + + [Test] + public Task SourceReturnAttributesOverrideExternalContracts() + { + return CreateTest(@" +class Api +{ + [ReturnsOwnership] public static Resource Read() => null; + [KeepsOwnership] public static Resource Create() => null; + public static void Run() + { + var {|ERP044:owned|} = Read(); + var borrowed = Create(); + } +}", @" + + +").RunAsync(); + } + + [Test] + public Task SourceParameterAttributesOverrideExternalContractsAndBorrowingWins() + { + return CreateTest(@" +class Api +{ + public static void Take([AcquiresOwnership] Resource resource) { resource.Dispose(); } + public static void Borrow([NoOwnership, AcquiresOwnership] Resource resource) { } + public static void Run() + { + var moved = new Resource(); + Take(moved); + var {|ERP044:retained|} = new Resource(); + Borrow(retained); + } +}", @" + + + + + + +").RunAsync(); + } + + [Test] + public Task ReturnTargetAttributesAreHonored() + { + return CreateTest(@" +class Api +{ + [return: ReturnsOwnership] public static Resource Read() => null; + [return: KeepsOwnership] public static Resource Create() => null; + public static Resource Property { [return: ReturnsOwnership] get => null; } + public static void Run() + { + var {|ERP044:owned|} = Read(); + var borrowed = Create(); + var {|ERP044:property|} = Property; + } +}", "").RunAsync(); + } + + [Test] + public Task InterfaceReturnContractAppliesToConcreteGenericImplementation() + { + return CreateTest(@" +interface IFactory +{ + [return: KeepsOwnership] T Read(); + [ReturnsOwnership] T Value { get; } +} +class Factory : IFactory +{ + public Resource Read() => null; + public Resource Value => null; + public static void Run() + { + var factory = new Factory(); + var borrowed = factory.Read(); + var {|ERP044:owned|} = factory.Value; + } +}", "").RunAsync(); + } + + [Test] + public Task InterfaceParameterOwnershipAlsoAppliesToRenamedImplementationParameter() + { + return CreateTest(@" +interface IConsumer +{ + void Take([AcquiresOwnership] Resource resource); +} +class Consumer : IConsumer +{ + public void Take(Resource {|ERP044:renamed|}) { } + public static void Run() + { + var moved = new Resource(); + new Consumer().Take(moved); + } +}", "").RunAsync(); + } + + [Test] + public Task ExternalInterfaceContractAlsoAppliesToImplementation() + { + return CreateTest(@" +interface IConsumer +{ + void Take(Resource resource); +} +class Consumer : IConsumer +{ + public void Take(Resource {|ERP044:renamed|}) { } + public static void Run() + { + var moved = new Resource(); + new Consumer().Take(moved); + } +}", @" + + + +").RunAsync(); + } + + [Test] + public Task OverriddenParameterOwnershipUsesOrdinalRatherThanName() + { + return CreateTest(@" +abstract class Base +{ + public abstract void Take([AcquiresOwnership] Resource resource); +} +class Derived : Base +{ + public override void Take(Resource {|ERP044:renamed|}) { } + public static void Run() + { + var moved = new Resource(); + new Derived().Take(moved); + } +}", "").RunAsync(); + } + + [Test] + public Task InheritedSourceContractOverridesDirectExternalContract() + { + return CreateTest(@" +interface IConsumer +{ + void Take([NoOwnership] Resource resource); +} +class Consumer : IConsumer +{ + public void Take(Resource renamed) { } + public static void Run() + { + var {|ERP044:retained|} = new Resource(); + new Consumer().Take(retained); + } +}", @" + + + +").RunAsync(); + } + + [Test] + public Task ConstructedGenericReturnAndParameterContractsUseDefinitions() + { + return CreateTest(@" +class Api where T : System.IDisposable +{ + public static T Read() => default(T); + public static T Value => default(T); + public static void Take(T {|ERP044:resource|}) { } +} +class Caller +{ + public static void Run() + { + var borrowed = Api.Read(); + var {|ERP044:owned|} = Api.Value; + var moved = new Resource(); + Api.Take(moved); + } +}", @" + + + + + +").RunAsync(); + } + + [Test] + public Task ReducedGenericExtensionUsesOriginalParameterOrdinal() + { + return CreateTest(@" +static class Extensions +{ + public static void Take(this string label, T {|ERP044:resource|}) where T : System.IDisposable { } + public static T Read(this string label) where T : System.IDisposable => default(T); + public static void Consume([AcquiresOwnership] this T resource) where T : System.IDisposable + { + resource.Dispose(); + } +} +class Caller +{ + public static void Run() + { + var moved = new Resource(); + ""label"".Take(moved); + var borrowed = ""label"".Read(); + var receiver = new Resource(); + receiver.Consume(); + } +}", @" + + + + +").RunAsync(); + } + + [Test] + public Task AssemblyQualifierMatchesCurrentAssembly() + { + var test = CreateTest(@" +class Api +{ + public static Resource Read() => null; + public static void Run() { var {|ERP044:owned|} = Read(); } +}", @" + +"); + test.SolutionTransforms.Add((solution, project) => solution.WithProjectAssemblyName(project, "ContractTarget")); + return test.RunAsync(); + } + + [Test] + public Task AssemblyQualifierDoesNotMatchDifferentAssembly() + { + return CreateTest(@" +class Api +{ + public static Resource Value => null; + public static void Run() { var borrowed = Value; } +}", @" + +").RunAsync(); + } + + [Test] + public Task MetadataMethodCanBeAnnotatedByAssemblyName() + { + return CreateTest(@" +class Caller +{ + public static void Run() + { + var borrowed = System.IO.File.OpenRead(""unused""); + } +}", @" + +").RunAsync(); + } + + [Test] + public Task AbsentLibrariesAndValidComplexIdentifiersAreAllowed() + { + return CreateTest("class Caller { }", @" + + + + + + +").RunAsync(); + } + + [Test] + public Task UnrelatedAdditionalFilesAreIgnored() + { + var test = CreateTest("class Caller { }", ""); + test.TestState.AdditionalFiles.Add(("unrelated.xml", ""); + test.TestState.AdditionalFiles.Add(("Other.ownership.xml", + @"")); + test.ExpectedDiagnostics.Add(Verify.Diagnostic("ERP045") + .WithMessage(null) + .WithLocation("Other.ownership.xml", 1, 13)); + return test.RunAsync(); + } + + [TestCase("<^broken />")] + [TestCase("<^unknown />")] + [TestCase("<^member id=\"M:Api.Read\" returns=\"shared\" />")] + [TestCase("<^member id=\"not-a-documentation-id\" returns=\"owned\" />")] + [TestCase("<^member id=\"M:Api.Read(System.String\" returns=\"owned\" />")] + [TestCase("<^member id=\"M:Api.Read(System.String,)\" returns=\"owned\" />")] + [TestCase("")] + [TestCase("<^parameter name=\"resource\" ownership=\"shared\" />")] + [TestCase("<^member id=\"M:Api.Take(Resource)\">")] + [TestCase("<^parameter name=\"resource\" ownership=\"borrowed\" />")] + [TestCase("<^member id=\"M:Api.Read\" returns=\"borrowed\" />")] + [TestCase("<^member id=\"M:Api.Read\" assembly=\"ApiLibrary\" returns=\"borrowed\" />")] + [TestCase("<^member id=\"M:Api.Read\" />")] + [TestCase("<^member id=\"M:Api.Read\" assembly=\"\" returns=\"owned\" />")] + [TestCase("<^ownership>unexpected text")] + [TestCase("^")] + [TestCase("^]>&dangerous;")] + public Task InvalidAnnotationsReportAtAdditionalFile(string xml) + { + var column = xml.IndexOf('^') + 1; + var test = CreateTest(@" +class Api +{ + public static Resource Read() => null; + public static void Take(Resource resource) { resource.Dispose(); } +}", xml.Remove(column - 1, 1)); + test.ExpectedDiagnostics.Add(Verify.Diagnostic("ERP045") + .WithMessage(null) + .WithLocation("Contracts.ownership.xml", 1, column)); + return test.RunAsync(); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/ErrorProne.NET.CoreAnalyzers.Tests.csproj b/src/ErrorProne.NET.CoreAnalyzers.Tests/ErrorProne.NET.CoreAnalyzers.Tests.csproj index 5a5d079..39a4e13 100644 --- a/src/ErrorProne.NET.CoreAnalyzers.Tests/ErrorProne.NET.CoreAnalyzers.Tests.csproj +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/ErrorProne.NET.CoreAnalyzers.Tests.csproj @@ -13,6 +13,7 @@ + diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/TypeExtensionsTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/TypeExtensionsTests.cs new file mode 100644 index 0000000..3b01689 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/TypeExtensionsTests.cs @@ -0,0 +1,87 @@ +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using ErrorProne.NET.Core; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; + +namespace ErrorProne.NET.CoreAnalyzers.Tests; + +[TestFixture] +public sealed class TypeExtensionsTests +{ + [TestCase("BaseInt", "BaseDefinition", true)] + [TestCase("Derived", "BaseDefinition", true)] + [TestCase("Derived", "BaseInt", true)] + [TestCase("Derived", "BaseString", false)] + [TestCase("BaseInt", "BaseString", false)] + [TestCase("BaseDefinition", "BaseInt", false)] + [TestCase("InterfaceInt", "InterfaceDefinition", true)] + [TestCase("Derived", "InterfaceDefinition", true)] + [TestCase("Derived", "InterfaceInt", true)] + [TestCase("Derived", "InterfaceString", false)] + [TestCase("Derived", "InterfaceDefinition", false, true)] + [TestCase("Constrained", "BaseDefinition", true)] + [TestCase("TransitiveConstraint", "BaseDefinition", true)] + [TestCase("TaskInt", "TaskDefinition", true)] + [TestCase("DerivedTask", "TaskDefinition", true)] + [TestCase("TaskInt", "TaskInt", true)] + [TestCase("TaskInt", "Object", true)] + public async Task DerivesFrom_Respects_Generic_Definitions_And_Constructed_Arguments( + string source, string candidate, bool expected, bool baseTypesOnly = false) + { + var types = await CreateTypes(); + Assert.That(types[source].DerivesFrom(types[candidate], baseTypesOnly), Is.EqualTo(expected)); + } + + [Test] + public async Task DerivesFrom_Rejects_Missing_Types() + { + var types = await CreateTypes(); + ITypeSymbol? missing = null; + Assert.That(missing.DerivesFrom(types["Object"]), Is.False); + Assert.That(types["Object"].DerivesFrom(missing), Is.False); + } + + private static async Task> CreateTypes() + { + var references = await ReferenceAssemblies.Net.Net80.ResolveAsync(LanguageNames.CSharp, CancellationToken.None); + var compilation = CSharpCompilation.Create("TypeRelationships", + new[] { CSharpSyntaxTree.ParseText(@" +public interface IMarker { } +public class GenericBase : IMarker { } +public class Derived : GenericBase { } +public class Constraints where T : GenericBase where U : T { } +public class DerivedTask : System.Threading.Tasks.Task +{ + public DerivedTask() : base(() => 42) { } +}") }, references, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + Assert.That(compilation.GetDiagnostics().Where(d => d.Severity >= DiagnosticSeverity.Warning), Is.Empty); + + var integer = compilation.GetSpecialType(SpecialType.System_Int32); + var text = compilation.GetSpecialType(SpecialType.System_String); + var baseDefinition = compilation.GetTypeByMetadataName("GenericBase`1")!; + var interfaceDefinition = compilation.GetTypeByMetadataName("IMarker`1")!; + var taskDefinition = compilation.GetTypeByMetadataName("System.Threading.Tasks.Task`1")!; + var constraints = compilation.GetTypeByMetadataName("Constraints`2")!; + return new Dictionary + { + ["BaseDefinition"] = baseDefinition, + ["BaseInt"] = baseDefinition.Construct(integer), + ["BaseString"] = baseDefinition.Construct(text), + ["Derived"] = compilation.GetTypeByMetadataName("Derived")!, + ["InterfaceDefinition"] = interfaceDefinition, + ["InterfaceInt"] = interfaceDefinition.Construct(integer), + ["InterfaceString"] = interfaceDefinition.Construct(text), + ["Constrained"] = constraints.TypeParameters[0], + ["TransitiveConstraint"] = constraints.TypeParameters[1], + ["TaskDefinition"] = taskDefinition, + ["TaskInt"] = taskDefinition.Construct(integer), + ["DerivedTask"] = compilation.GetTypeByMetadataName("DerivedTask")!, + ["Object"] = compilation.GetSpecialType(SpecialType.System_Object), + }; + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers/AnalyzerReleases.Unshipped.md b/src/ErrorProne.NET.CoreAnalyzers/AnalyzerReleases.Unshipped.md index 6826c2b..d879c84 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/AnalyzerReleases.Unshipped.md +++ b/src/ErrorProne.NET.CoreAnalyzers/AnalyzerReleases.Unshipped.md @@ -39,3 +39,6 @@ ERP022 | ErrorHandling | Warning | SwallowAllExceptionsAnalyzer ERP031 | Concurrency | Warning | ConcurrentCollectionAnalyzer ERP041 | CodeSmell | Info | EventSourceSealedAnalyzer ERP042 | CodeSmell | Warning | EventSourceAnalyzer +ERP044 | Reliability | Warning | DisposeBeforeLosingScopeAnalyzer +ERP045 | Reliability | Warning | Invalid external ownership annotation +ERP046 | Reliability | Warning | Respect disposable ownership diff --git a/src/ErrorProne.NET.CoreAnalyzers/DiagnosticDescriptors.cs b/src/ErrorProne.NET.CoreAnalyzers/DiagnosticDescriptors.cs index cf56702..6ddb9d4 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/DiagnosticDescriptors.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/DiagnosticDescriptors.cs @@ -9,6 +9,7 @@ internal static class DiagnosticDescriptors private const string CodeSmellCategory = "CodeSmell"; private const string PerformanceCategory = "Performance"; private const string ConcurrencyCategory = "Concurrency"; + private const string ReliabilityCategory = "Reliability"; private const string AsyncCategory = "Async"; @@ -367,6 +368,33 @@ internal static class DiagnosticDescriptors description: "DataContractSerializer fails lazily (on the first serialization attempt) with 'InvalidDataContractException' when a type of a data member is not serializable. A type is serializable when it is marked with 'DataContractAttribute', 'CollectionDataContractAttribute' or 'SerializableAttribute', implements 'ISerializable'/'IXmlSerializable', or is a public type with a parameterless constructor. Types like 'System.Net.IPAddress' or 'System.Net.IPEndPoint' fail this check on .NET Core, and so do positional records.", helpLinkUri: GetHelpUri(nameof(EPC42))); + /// + public static readonly DiagnosticDescriptor ERP044 = new DiagnosticDescriptor( + nameof(ERP044), + title: "Dispose owned resources before losing scope", + messageFormat: "Owned resource '{0}' of type '{1}' must be disposed or its ownership transferred", + ReliabilityCategory, DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: "An acquired disposable resource must be disposed or transferred to another owner.", + helpLinkUri: GetHelpUri(nameof(ERP044))); + + /// + public static readonly DiagnosticDescriptor ERP045 = new DiagnosticDescriptor( + nameof(ERP045), + title: "Invalid external ownership annotation", + messageFormat: "Invalid ownership annotation: {0}", + ReliabilityCategory, DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: "External ownership annotations must describe valid, unambiguous ownership contracts.", + helpLinkUri: GetHelpUri(nameof(ERP045))); + + /// + public static readonly DiagnosticDescriptor ERP046 = new DiagnosticDescriptor( + nameof(ERP046), + title: "Respect disposable ownership", + messageFormat: "Resource '{0}' {1}", + ReliabilityCategory, DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: "Do not dispose or transfer explicitly borrowed resources, or use a resource after disposing or transferring it.", + helpLinkUri: GetHelpUri(nameof(ERP046))); + public static string GetHelpUri(string ruleId) { return $"https://github.com/SergeyTeplyakov/ErrorProne.NET/tree/master/docs/Rules/{ruleId}.md"; diff --git a/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposableAttributes.cs b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposableAttributes.cs new file mode 100644 index 0000000..02ea85e --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposableAttributes.cs @@ -0,0 +1,18 @@ +namespace ErrorProne.NET.DisposableAnalyzers; + +public static class DisposableAttributes +{ + public const string DoNotDisposeAttribute = "DoNotDisposeAttribute"; + + // Attribute contracts are documented in docs/Rules/ERP044.md. + public const string AcquiresOwnershipAttribute = "AcquiresOwnershipAttribute"; + + // For methods that want to emphasize that the ownership is not transferred. + public const string KeepsOwnershipAttribute = "KeepsOwnershipAttribute"; + + // For methods and properties whose results transfer ownership. + public const string ReturnsOwnershipAttribute = "ReturnsOwnershipAttribute"; + + // For borrowed parameters, fields and properties. + public const string NoOwnershipAttribute = "NoOwnershipAttribute"; +} \ No newline at end of file diff --git a/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposeAnalysisHelper.cs b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposeAnalysisHelper.cs new file mode 100644 index 0000000..4753688 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposeAnalysisHelper.cs @@ -0,0 +1,75 @@ +using System.Collections.Generic; +using System.Linq; +using ErrorProne.NET.Core; +using Microsoft.CodeAnalysis; + +namespace ErrorProne.NET.DisposableAnalyzers; + +internal sealed class DisposeAnalysisHelper +{ + private readonly List _disposableExceptions; + + public INamedTypeSymbol? IDisposable { get; } + public INamedTypeSymbol? IAsyncDisposable { get; } + public INamedTypeSymbol? IConfigureAsyncDisposable { get; } + + public DisposeAnalysisHelper(Compilation compilation) + { + _disposableExceptions = CreateExceptions(compilation); + + IDisposable = compilation.GetTypeByFullName(WellKnownTypeNames.SystemIDisposable); + IAsyncDisposable = compilation.GetTypeByFullName(WellKnownTypeNames.SystemIAsyncDisposable); + IConfigureAsyncDisposable = compilation.GetTypeByFullName("System.Runtime.CompilerServices.ConfiguredAsyncDisposable"); + } + + private static List CreateExceptions(Compilation compilation) + { + var result = new List(); + + // Preserve the prototype's low-noise defaults; these are policy exemptions. + addIfNotNull(compilation.TaskType()); + addIfNotNull(compilation.TaskOfTType()); + addIfNotNull(compilation.GetTypeByFullName("System.IO.StringReader")); + addIfNotNull(compilation.GetTypeByFullName("System.IO.MemoryStream")); + + return result; + + void addIfNotNull(INamedTypeSymbol? type) + { + if (type is not null) + { + result.Add(type); + } + } + } + + public bool IsDisposableTypeNotRequiringToBeDisposed(ITypeSymbol? typeSymbol) + { + if (typeSymbol is null) + { + return false; + } + + return _disposableExceptions.Any(e => typeSymbol.DerivesFrom(e)); + } + + public bool IsDisposable(ITypeSymbol? typeSymbol) + { + if (typeSymbol is null) + { + return false; + } + + return typeSymbol.IsDisposable(IDisposable, IAsyncDisposable, IConfigureAsyncDisposable); + } + + public bool ShouldBeDisposed(ITypeSymbol? typeSymbol) + { + if (typeSymbol is null) + { + return false; + } + + return IsDisposable(typeSymbol) && !IsDisposableTypeNotRequiringToBeDisposed(typeSymbol); + } +} \ No newline at end of file diff --git a/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzer.cs b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzer.cs new file mode 100644 index 0000000..f9a79b1 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/DisposeBeforeLosingScopeAnalyzer.cs @@ -0,0 +1,892 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using ErrorProne.NET.Core; +using ErrorProne.NET.CoreAnalyzers; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.Text; + +namespace ErrorProne.NET.DisposableAnalyzers; + +/// +/// Checks local ownership obligations using explicit contracts and bounded source inference. +/// Conditional cleanup is accepted; this is not an exception-safe or path-sensitive proof. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class DisposeBeforeLosingScopeAnalyzer : DiagnosticAnalyzerBase +{ + internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptors.ERP044; + + public override bool ReportDiagnosticsOnGeneratedCode => false; + + public DisposeBeforeLosingScopeAnalyzer() + : base(Rule, DiagnosticDescriptors.ERP045, DiagnosticDescriptors.ERP046) + { + } + + protected override void InitializeCore(AnalysisContext context) + { + context.RegisterCompilationStartAction(start => + { + var contracts = OwnershipContracts.Create(start.Compilation, start.Options, start.CancellationToken); + var helper = new DisposeAnalysisHelper(start.Compilation); + var inference = new OwnershipInference(start.Compilation, contracts, helper); + + start.RegisterOperationBlockAction(block => + { + if (block.OwningSymbol is not IMethodSymbol method || method.IsImplicitlyDeclared) + { + return; + } + + // Attribute/default-value roots are not method implementations. Constructor + // initializers and bodies, however, share the same parameter obligations. + var operations = block.OperationBlocks + .Where(root => root is not (IAttributeOperation or IParameterInitializerOperation)) + .OrderBy(root => root.Syntax.SpanStart) + .SelectMany(EnumerateOperations) + .ToImmutableArray(); + if (operations.IsEmpty || operations.Any(o => o is IInvalidOperation)) + { + return; + } + + ReportBorrowedUses(operations, contracts, inference, block); + var capturedSymbols = new HashSet(SymbolEqualityComparer.Default); + foreach (var closure in operations.Where(o => o is IAnonymousFunctionOperation or ILocalFunctionOperation)) + { + CollectCapturedSymbols(closure, capturedSymbols, block.CancellationToken); + } + + foreach (var parameter in method.Parameters) + { + if (contracts.AcquiresOwnership(parameter)) + { + var lifetime = new Lifetime(null, parameter, operations, capturedSymbols, contracts, inference, block.CancellationToken); + if (lifetime.Analyze() == LifetimeOutcome.Abandoned) + { + block.ReportDiagnostic(Diagnostic.Create(Rule, parameter.Locations.FirstOrDefault(), + parameter.Name, parameter.Type.Name)); + } + else if (lifetime.InvalidUse is { } invalidUse) + { + block.ReportDiagnostic(invalidUse); + } + } + } + + foreach (var operation in operations) + { + block.CancellationToken.ThrowIfCancellationRequested(); + if (!IsAcquisition(operation, helper, contracts, inference, block.CancellationToken)) + { + continue; + } + + var lifetime = new Lifetime(operation, null, operations, capturedSymbols, contracts, inference, block.CancellationToken); + if (lifetime.Analyze() == LifetimeOutcome.Abandoned) + { + var (location, name) = GetDiagnosticTarget(operation); + block.ReportDiagnostic(Diagnostic.Create(Rule, location, name, operation.Type!.Name)); + } + else if (lifetime.InvalidUse is { } invalidUse) + { + block.ReportDiagnostic(invalidUse); + } + } + }); + + start.RegisterCompilationEndAction(end => + { + foreach (var diagnostic in contracts.Diagnostics) + { + end.ReportDiagnostic(diagnostic); + } + }); + }); + } + + private static bool IsAcquisition(IOperation operation, DisposeAnalysisHelper helper, + OwnershipContracts contracts, OwnershipInference inference, CancellationToken cancellationToken) + { + if (!helper.ShouldBeDisposed(operation.Type)) + { + return false; + } + + return operation switch + { + IObjectCreationOperation => true, + ITypeParameterObjectCreationOperation => true, + IInvocationOperation invocation => inference.ReturnsOwnership(invocation.TargetMethod, cancellationToken), + IConversionOperation { OperatorMethod: { } method } => inference.ReturnsOwnership(method, cancellationToken), + IPropertyReferenceOperation property => contracts.GetReturnOwnership(property.Property) == OwnershipKind.Owned, + IAwaitOperation awaited => inference.GetAwaitedMember(awaited.Operation) switch + { + IMethodSymbol method => inference.ReturnsOwnership(method, cancellationToken), + IPropertySymbol property => contracts.GetReturnOwnership(property) == OwnershipKind.Owned, + _ => false, + }, + _ => false, + }; + } + + private static void ReportBorrowedUses(ImmutableArray operations, OwnershipContracts contracts, + OwnershipInference inference, OperationBlockAnalysisContext context) + { + var bindings = new Dictionary(SymbolEqualityComparer.Default); + var carrierBindings = new Dictionary(SymbolEqualityComparer.Default); + var reported = new HashSet(); + foreach (var operation in operations) + { + context.CancellationToken.ThrowIfCancellationRequested(); + if (GetPatternAlias(operation) is { } patternAlias) + { + // A newly declared pattern local has no pre-existing non-borrowed binding to merge. + TrackBinding(patternAlias.Local, patternAlias.Value, conditional: false); + } + + foreach (var input in OwnershipInference.GetOperatorInputs(operation)) + { + if ((IsBorrowedValue(input.Value) || IsBorrowedCarrierValue(input.Value)) + && inference.AcquiresOwnership(input.Parameter, ImmutableArray.Empty, context.CancellationToken)) + { + Report(input.Value); + } + } + if (operation is IInvocationOperation invocation) + { + if (inference.IsDisposeCall(invocation) && IsBorrowedValue(invocation.Instance)) + { + Report(invocation.Instance!); + } + + CheckArguments(invocation.Arguments); + if (inference.GetInterlockedOwningValue(invocation) is { } value + && (IsBorrowedValue(value) || IsBorrowedCarrierValue(value))) + { + Report(value); + } + } + else if (operation is IObjectCreationOperation creation) + { + CheckArguments(creation.Arguments); + } + else if (operation is IPropertyReferenceOperation propertyReference) + { + CheckArguments(propertyReference.Arguments); + } + else if (operation is IUsingDeclarationOperation usingDeclaration) + { + CheckUsingResources(usingDeclaration.DeclarationGroup); + } + else if (operation is IVariableDeclaratorOperation { Initializer: { } initializer } declarator) + { + TrackBinding(declarator.Symbol, initializer.Value, conditional: false); + } + else if (operation is ISimpleAssignmentOperation assignment) + { + if ((IsBorrowedValue(assignment.Value) || IsBorrowedCarrierValue(assignment.Value)) + && IsOwningDestination(Unwrap(assignment.Target), contracts)) + { + Report(assignment.Value); + } + + if (GetAliasSymbol(assignment.Target) is { } target) + { + TrackBinding(target, assignment.Value, IsConditional(assignment)); + } + } + else if (operation is IReturnOperation { ReturnedValue: { } value } + && (IsBorrowedValue(value) || IsBorrowedCarrierValue(value)) + && context.OwningSymbol is IMethodSymbol method + && (method.AssociatedSymbol is IPropertySymbol property + ? contracts.GetReturnOwnership(property) == OwnershipKind.Owned + : inference.ReturnsOwnership(method, context.CancellationToken))) + { + Report(value); + } + + // A using statement captures its resource before its body can reassign the variable. + if (GetUsingCapture(operation) != null) + { + CheckUsingResources(operation); + } + } + + bool IsBorrowedValue(IOperation? value) => IsBorrowed(value, contracts, inference, bindings, carrierBindings); + bool IsBorrowedCarrierValue(IOperation? value) => IsBorrowedCarrier(value, contracts, inference, bindings, carrierBindings); + + void TrackBinding(ISymbol symbol, IOperation value, bool conditional) + { + var borrowed = IsBorrowedValue(value); + var carriedBorrowing = IsBorrowedCarrierValue(value); + if (conditional) + { + borrowed &= bindings.TryGetValue(symbol, out var previous) ? previous : contracts.IsBorrowed(symbol); + carriedBorrowing &= carrierBindings.TryGetValue(symbol, out var previousCarrier) + ? previousCarrier + : symbol is IParameterSymbol parameter && inference.IsTaskResultCarrier(parameter.Type) + && contracts.IsBorrowed(parameter); + } + + bindings[symbol] = borrowed; + carrierBindings[symbol] = carriedBorrowing; + } + + void CheckUsingResources(IOperation resources) + { + if (IsBorrowedValue(resources)) + { + Report(resources); + return; + } + + foreach (var declarator in EnumerateOperations(resources).OfType()) + { + if (declarator.Initializer is { } initializer + && bindings.TryGetValue(declarator.Symbol, out var borrowed) && borrowed) + { + Report(initializer.Value); + } + } + } + + void CheckArguments(ImmutableArray arguments) + { + foreach (var argument in arguments) + { + if (argument.Parameter != null + && (IsBorrowedValue(argument.Value) || IsBorrowedCarrierValue(argument.Value)) + && inference.AcquiresOwnership(argument.Parameter, arguments, context.CancellationToken)) + { + Report(argument.Value); + } + } + } + + void Report(IOperation value) + { + if (reported.Add(value.Syntax.Span)) + { + context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.ERP046, value.Syntax.GetLocation(), + value.Syntax.ToString(), "is borrowed and must not be disposed or transferred")); + } + } + } + + private static bool IsOwningDestination(IOperation operation, OwnershipContracts contracts) + { + return operation switch + { + IMemberReferenceOperation member => !contracts.IsBorrowed(member.Member), + IParameterReferenceOperation parameter => parameter.Parameter.RefKind != RefKind.None + && !contracts.IsBorrowed(parameter.Parameter), + _ => false, + }; + } + + private static bool IsBorrowed(IOperation? operation, OwnershipContracts contracts, + OwnershipInference inference, IReadOnlyDictionary bindings, + IReadOnlyDictionary carrierBindings) + { + if (operation == null) + { + return false; + } + + operation = Unwrap(operation); + if (GetAliasSymbol(operation) is { } symbol && bindings.TryGetValue(symbol, out var borrowed)) + { + return borrowed; + } + + return operation switch + { + IParameterReferenceOperation parameter => contracts.IsBorrowed(parameter.Parameter), + IMemberReferenceOperation member => contracts.IsBorrowed(member.Member) + || contracts.GetReturnOwnership(member.Member) == OwnershipKind.Borrowed, + IInvocationOperation invocation when inference.GetConfiguredDisposableResource(invocation) is { } resource => + IsBorrowed(resource, contracts, inference, bindings, carrierBindings), + IInvocationOperation invocation => contracts.GetReturnOwnership(invocation.TargetMethod) == OwnershipKind.Borrowed, + IConversionOperation { OperatorMethod: { } method } => contracts.GetReturnOwnership(method) == OwnershipKind.Borrowed, + IConditionalAccessInstanceOperation => IsBorrowed(GetConditionalReceiver(operation), contracts, inference, bindings, carrierBindings), + IConditionalOperation conditional => IsBorrowed(conditional.WhenTrue, contracts, inference, bindings, carrierBindings) + && IsBorrowed(conditional.WhenFalse, contracts, inference, bindings, carrierBindings), + ICoalesceOperation coalesce => IsBorrowed(coalesce.Value, contracts, inference, bindings, carrierBindings) + && IsBorrowed(coalesce.WhenNull, contracts, inference, bindings, carrierBindings), + ISimpleAssignmentOperation assignment => IsBorrowed(assignment.Value, contracts, inference, bindings, carrierBindings), + IAwaitOperation awaited => inference.GetAwaitedMember(awaited.Operation) is { } member + && contracts.GetReturnOwnership(member) != OwnershipKind.Unspecified + ? contracts.GetReturnOwnership(member) == OwnershipKind.Borrowed + : IsBorrowedCarrier(awaited.Operation, contracts, inference, bindings, carrierBindings), + _ => false, + }; + } + + private static bool IsBorrowedCarrier(IOperation? operation, OwnershipContracts contracts, + OwnershipInference inference, IReadOnlyDictionary bindings, + IReadOnlyDictionary carrierBindings) + { + if (operation == null) + { + return false; + } + + operation = Unwrap(operation); + if (GetAliasSymbol(operation) is { } symbol && carrierBindings.TryGetValue(symbol, out var borrowed)) + { + return borrowed; + } + + if (inference.GetCompletedTaskValue(operation) is { } value) + { + return IsBorrowed(value, contracts, inference, bindings, carrierBindings); + } + + if (inference.GetConfiguredTask(operation) is { } task) + { + return IsBorrowedCarrier(task, contracts, inference, bindings, carrierBindings); + } + + if (operation is IInvocationOperation invocation && inference.GetFluentReceiver(invocation) is { } receiver) + { + return IsBorrowedCarrier(receiver, contracts, inference, bindings, carrierBindings); + } + + return operation switch + { + IParameterReferenceOperation parameter => inference.IsTaskResultCarrier(parameter.Type) + && contracts.IsBorrowed(parameter.Parameter), + ISimpleAssignmentOperation assignment => IsBorrowedCarrier(assignment.Value, contracts, inference, bindings, carrierBindings), + IConditionalOperation conditional => IsBorrowedCarrier(conditional.WhenTrue, contracts, inference, bindings, carrierBindings) + && IsBorrowedCarrier(conditional.WhenFalse, contracts, inference, bindings, carrierBindings), + ICoalesceOperation coalesce => IsBorrowedCarrier(coalesce.Value, contracts, inference, bindings, carrierBindings) + && IsBorrowedCarrier(coalesce.WhenNull, contracts, inference, bindings, carrierBindings), + _ => false, + }; + } + + internal static IOperation? GetConditionalReceiver(IOperation operation) + { + for (var parent = operation.Parent; parent != null; parent = parent.Parent) + { + if (parent is IConditionalAccessOperation conditional) + { + return conditional.Operation; + } + } + + return null; + } + + private static (Location Location, string Name) GetDiagnosticTarget(IOperation operation) + { + for (var parent = operation.Parent; parent != null; parent = parent.Parent) + { + if (parent is IVariableDeclaratorOperation declarator + && declarator.Syntax is VariableDeclaratorSyntax syntax) + { + return (syntax.Identifier.GetLocation(), declarator.Symbol.Name); + } + + if (parent is ISimpleAssignmentOperation { Target: ILocalReferenceOperation local }) + { + return (local.Syntax.GetLocation(), local.Local.Name); + } + + if (parent is IConversionOperation && ReferenceEquals(Unwrap(parent), parent) + || parent is not (IConversionOperation or IParenthesizedOperation or IAwaitOperation or IVariableInitializerOperation)) + { + break; + } + } + + return (operation.Syntax.GetLocation(), operation.Syntax.ToString()); + } + + internal static IEnumerable EnumerateOperations(IOperation root) + { + // Postorder follows evaluation order for arguments, initializers and their containing call. + foreach (var child in root.ChildOperations) + { + if (child is INameOfOperation) + { + continue; + } + + if (child is IAnonymousFunctionOperation or ILocalFunctionOperation) + { + // Captures can end local certainty, but nested bodies are separate lifetimes. + yield return child; + continue; + } + + foreach (var operation in EnumerateOperations(child)) + { + yield return operation; + } + } + + yield return root; + } + + private static void CollectCapturedSymbols(IOperation operation, HashSet symbols, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + if (operation is INameOfOperation) + { + return; + } + + if (operation is ILocalReferenceOperation or IParameterReferenceOperation + && GetAliasSymbol(operation) is { } symbol) + { + symbols.Add(symbol); + } + + foreach (var child in operation.ChildOperations) + { + CollectCapturedSymbols(child, symbols, cancellationToken); + } + } + + internal static IOperation Unwrap(IOperation operation) + { + while (true) + { + switch (operation) + { + case IConversionOperation conversion when conversion.Conversion.Exists + && !conversion.Conversion.IsUserDefined + && conversion.Type?.TypeKind != TypeKind.Dynamic + && conversion.Operand.Type?.TypeKind != TypeKind.Dynamic: + operation = conversion.Operand; + break; + case IParenthesizedOperation parenthesized: + operation = parenthesized.Operand; + break; + default: + return operation; + } + } + } + + internal static IUsingOperation? GetUsingCapture(IOperation operation) + { + return operation.Parent is IUsingOperation usingOperation && ReferenceEquals(usingOperation.Resources, operation) + ? usingOperation : null; + } + + internal static ISymbol? GetAliasSymbol(IOperation operation) + { + return Unwrap(operation) switch + { + ILocalReferenceOperation local => local.Local, + IParameterReferenceOperation parameter => parameter.Parameter, + _ => null, + }; + } + + internal static (IOperation Value, ILocalSymbol Local)? GetPatternAlias(IOperation operation) + { + if (operation is not IIsPatternOperation isPattern) + { + return null; + } + + var pattern = isPattern.Pattern; + while (pattern is INegatedPatternOperation negated) + { + pattern = negated.Pattern; + } + + // Only a direct type-pattern binding aliases the input, not nested property patterns. + return pattern is IDeclarationPatternOperation { DeclaredSymbol: ILocalSymbol local } + ? (isPattern.Value, local) : null; + } + + internal static bool IsConditional(IOperation operation) + { + for (var parent = operation.Parent; parent != null; parent = parent.Parent) + { + if (parent is IConditionalOperation or IConditionalAccessOperation or ICoalesceOperation + or ISwitchOperation or ILoopOperation or ICatchClauseOperation + or IBinaryOperation { OperatorKind: BinaryOperatorKind.ConditionalAnd or BinaryOperatorKind.ConditionalOr }) + { + return true; + } + } + + return false; + } + + private enum LifetimeOutcome + { + Abandoned, + Discharged, + Unknown, + } + + private sealed class Lifetime + { + private readonly IOperation? _creation; + private readonly ImmutableArray _operations; + private readonly HashSet _capturedSymbols; + private readonly OwnershipContracts _contracts; + private readonly OwnershipInference _inference; + private readonly CancellationToken _cancellationToken; + private readonly HashSet _aliases = new(SymbolEqualityComparer.Default); + private readonly HashSet _uncertainAliases = new(SymbolEqualityComparer.Default); + private readonly HashSet _carriers = new(SymbolEqualityComparer.Default); + private readonly HashSet _uncertainCarriers = new(SymbolEqualityComparer.Default); + + public Diagnostic? InvalidUse { get; private set; } + + public Lifetime(IOperation? creation, IParameterSymbol? parameter, ImmutableArray operations, + HashSet capturedSymbols, + OwnershipContracts contracts, OwnershipInference inference, CancellationToken cancellationToken) + { + _creation = creation; + _operations = operations; + _capturedSymbols = capturedSymbols; + _contracts = contracts; + _inference = inference; + _cancellationToken = cancellationToken; + if (parameter != null) + { + // An acquiring Task/ValueTask parameter owns its result, not wrapper cleanup. + (inference.IsTaskResultCarrier(parameter.Type) ? _carriers : _aliases).Add(parameter); + } + } + + public LifetimeOutcome Analyze() + { + var started = _creation == null; + var ownershipUnknown = _capturedSymbols.Overlaps(_aliases) || _capturedSymbols.Overlaps(_carriers); + foreach (var operation in _operations) + { + _cancellationToken.ThrowIfCancellationRequested(); + if (!started) + { + started = ReferenceEquals(operation, _creation); + if (!started) + { + continue; + } + } + + if (GetPatternAlias(operation) is { } patternAlias) + { + AssignAlias(patternAlias.Local, patternAlias.Value, operation); + } + + foreach (var input in OwnershipInference.GetOperatorInputs(operation)) + { + if ((Matches(input.Value) || Carries(input.Value)) + && _inference.AcquiresOwnership(input.Parameter, ImmutableArray.Empty, _cancellationToken)) + { + return Discharge(operation, Matches(input.Value, requireDefinite: true) + || Carries(input.Value, requireDefinite: true)); + } + if ((Matches(input.Value) || Carries(input.Value)) && !_contracts.IsBorrowed(input.Parameter)) + { + ownershipUnknown = true; + } + } + + switch (operation) + { + case IVariableDeclaratorOperation { Initializer: { } initializer } declarator: + AssignAlias(declarator.Symbol, initializer.Value, operation); + break; + case ISimpleAssignmentOperation assignment: + if (Matches(assignment.Value) || Carries(assignment.Value)) + { + switch (Unwrap(assignment.Target)) + { + case ILocalReferenceOperation local: + AssignAlias(local.Local, assignment.Value, operation); + break; + case IParameterReferenceOperation parameter when IsOwningDestination(parameter, _contracts): + return Discharge(operation, Matches(assignment.Value, requireDefinite: true) + || Carries(assignment.Value, requireDefinite: true)); + case IParameterReferenceOperation parameter: + AssignAlias(parameter.Parameter, assignment.Value, operation); + break; + case IMemberReferenceOperation member when !_contracts.IsBorrowed(member.Member): + return Discharge(operation, Matches(assignment.Value, requireDefinite: true) + || Carries(assignment.Value, requireDefinite: true)); + } + } + else if (GetAliasSymbol(assignment.Target) is { } alias) + { + AssignAlias(alias, assignment.Value, operation); + } + break; + case IInvocationOperation invocation: + if (_inference.IsDisposeCall(invocation) && Matches(invocation.Instance)) + { + return Discharge(operation, Matches(invocation.Instance, requireDefinite: true)); + } + + var published = _inference.GetInterlockedOwningValue(invocation); + if (MovesArgument(invocation.Arguments) || Matches(published) || Carries(published)) + { + return Discharge(operation, MovesArgument(invocation.Arguments, requireDefinite: true) + || Matches(published, requireDefinite: true) || Carries(published, requireDefinite: true)); + } + if (_inference.GetCompletedTaskValue(invocation) == null + && Escapes(invocation.Arguments, _inference.GetFluentReceiver(invocation, _cancellationToken))) + { + ownershipUnknown = true; + } + break; + case IObjectCreationOperation creation when MovesArgument(creation.Arguments): + return Discharge(operation, MovesArgument(creation.Arguments, requireDefinite: true)); + case IObjectCreationOperation creation when _inference.GetCompletedTaskValue(creation) == null + && Escapes(creation.Arguments): + ownershipUnknown = true; + break; + case IPropertyReferenceOperation property when MovesArgument(property.Arguments): + return Discharge(operation, MovesArgument(property.Arguments, requireDefinite: true)); + case IPropertyReferenceOperation property when Escapes(property.Arguments): + ownershipUnknown = true; + break; + case IReturnOperation returned when Matches(returned.ReturnedValue) || Carries(returned.ReturnedValue): + return LifetimeOutcome.Discharged; + case IDelegateCreationOperation { Target: IMethodReferenceOperation method } when Matches(method.Instance): + ownershipUnknown = true; + break; + case IDynamicInvocationOperation dynamicInvocation + when dynamicInvocation.Arguments.Any(a => Matches(a) || Carries(a)): + ownershipUnknown = true; + break; + case IDynamicObjectCreationOperation dynamicCreation + when dynamicCreation.Arguments.Any(a => Matches(a) || Carries(a)): + ownershipUnknown = true; + break; + case IDynamicIndexerAccessOperation dynamicIndexer + when dynamicIndexer.Arguments.Any(a => Matches(a) || Carries(a)): + ownershipUnknown = true; + break; + case IUsingDeclarationOperation declaration when declaration.DeclarationGroup.Declarations + .SelectMany(d => d.Declarators).Any(d => _aliases.Contains(d.Symbol)): + return LifetimeOutcome.Discharged; + } + + // Closures capture variables, including resources assigned after closure creation. + ownershipUnknown |= _capturedSymbols.Overlaps(_aliases) || _capturedSymbols.Overlaps(_carriers); + + if (GetUsingCapture(operation) is { } captured + && (Matches(operation) || captured.Locals.Any(l => _aliases.Contains(l)))) + { + return LifetimeOutcome.Discharged; + } + } + + return ownershipUnknown ? LifetimeOutcome.Unknown : LifetimeOutcome.Abandoned; + } + + private LifetimeOutcome Discharge(IOperation terminal, bool definite) + { + // Restrict invalid-use reports to later statements in the very same lexical block. + // Conditional cleanup, finally blocks and implicit using cleanup remain best effort. + var statement = terminal.Syntax.FirstAncestorOrSelf(); + if (!definite || statement?.Parent is not BlockSyntax block + || IsConditional(terminal) + || EnumerateOperations(terminal).Any(o => o is IConditionalOperation or ICoalesceOperation)) + { + return LifetimeOutcome.Discharged; + } + + var passedTerminal = false; + foreach (var operation in _operations) + { + if (!passedTerminal) + { + passedTerminal = ReferenceEquals(operation, terminal); + continue; + } + + // The enclosing assignment may replace an alias in the terminal statement itself. + if (operation is ISimpleAssignmentOperation assignment + && GetAliasSymbol(assignment.Target) is { } target) + { + AssignAlias(target, assignment.Value, operation); + } + else if (operation is IVariableDeclaratorOperation { Initializer: { } initializer } declarator) + { + AssignAlias(declarator.Symbol, initializer.Value, operation); + } + + if (operation.Syntax.SpanStart < statement.Span.End + || operation is not (ILocalReferenceOperation or IParameterReferenceOperation or IAwaitOperation) + || !Matches(operation, requireDefinite: true) + || IsConditional(operation) + || operation.Syntax.FirstAncestorOrSelf()?.Parent != block) + { + continue; + } + + if (operation.Parent is ISimpleAssignmentOperation { Target: var assignmentTarget } + && ReferenceEquals(assignmentTarget, operation)) + { + continue; + } + + InvalidUse = Diagnostic.Create(DiagnosticDescriptors.ERP046, operation.Syntax.GetLocation(), + operation.Syntax.ToString(), "is used after disposal or ownership transfer"); + break; + } + + return LifetimeOutcome.Discharged; + } + + private void AssignAlias(ISymbol symbol, IOperation value, IOperation assignment) + { + var matches = Matches(value); + var carries = Carries(value); + var definiteMatch = Matches(value, requireDefinite: true); + var definiteCarrier = Carries(value, requireDefinite: true); + Track(_aliases, _uncertainAliases, matches, definiteMatch); + Track(_carriers, _uncertainCarriers, carries, definiteCarrier); + + void Track(HashSet aliases, HashSet uncertain, bool matchesValue, bool definite) + { + if (matchesValue) + { + aliases.Add(symbol); + if (definite && !IsConditional(assignment)) + { + uncertain.Remove(symbol); + } + else + { + uncertain.Add(symbol); + } + } + else if (IsConditional(assignment) && aliases.Contains(symbol)) + { + uncertain.Add(symbol); + } + else + { + aliases.Remove(symbol); + uncertain.Remove(symbol); + } + } + } + + private bool Escapes(ImmutableArray arguments, IOperation? preservedReceiver = null) + { + return arguments.Any(argument => (Matches(argument.Value) || Carries(argument.Value)) + && !ReferenceEquals(argument.Value, preservedReceiver) + && _inference.IsUnknownArgument(argument, arguments)); + } + + private bool Carries(IOperation? value, bool requireDefinite = false) + { + if (value == null) + { + return false; + } + + value = Unwrap(value); + if (GetAliasSymbol(value) is { } symbol) + { + return _carriers.Contains(symbol) && (!requireDefinite || !_uncertainCarriers.Contains(symbol)); + } + + if (_inference.GetCompletedTaskValue(value) is { } result) + { + return Matches(result, requireDefinite); + } + + if (_inference.GetConfiguredTask(value) is { } task) + { + return Carries(task, requireDefinite); + } + + if (value is IInvocationOperation invocation + && _inference.GetFluentReceiver(invocation, _cancellationToken) is { } receiver) + { + return Carries(receiver, requireDefinite); + } + + return value switch + { + ISimpleAssignmentOperation assignment => Carries(assignment.Value, requireDefinite), + IConditionalOperation conditional => requireDefinite + ? Carries(conditional.WhenTrue, true) && Carries(conditional.WhenFalse, true) + : Carries(conditional.WhenTrue) || Carries(conditional.WhenFalse), + ICoalesceOperation coalesce => requireDefinite + ? Carries(coalesce.Value, true) && Carries(coalesce.WhenNull, true) + : Carries(coalesce.Value) || Carries(coalesce.WhenNull), + _ => false, + }; + } + + private bool MovesArgument(ImmutableArray arguments, bool requireDefinite = false) + { + foreach (var argument in arguments) + { + if (argument.Parameter != null + && (Matches(argument.Value, requireDefinite) || Carries(argument.Value, requireDefinite)) + && _inference.AcquiresOwnership(argument.Parameter, arguments, _cancellationToken)) + { + return true; + } + } + + return false; + } + + private bool Matches(IOperation? value, bool requireDefinite = false) + { + if (value == null) + { + return false; + } + + value = Unwrap(value); + if (ReferenceEquals(value, _creation)) + { + return true; + } + + switch (value) + { + case ILocalReferenceOperation local: + return _aliases.Contains(local.Local) && (!requireDefinite || !_uncertainAliases.Contains(local.Local)); + case IParameterReferenceOperation parameter: + return _aliases.Contains(parameter.Parameter) && (!requireDefinite || !_uncertainAliases.Contains(parameter.Parameter)); + case IConditionalAccessInstanceOperation: + return Matches(GetConditionalReceiver(value), requireDefinite); + case IConditionalOperation conditional: + return requireDefinite + ? Matches(conditional.WhenTrue, true) && Matches(conditional.WhenFalse, true) + : Matches(conditional.WhenTrue) || Matches(conditional.WhenFalse); + case ICoalesceOperation coalesce: + return requireDefinite + ? Matches(coalesce.Value, true) && Matches(coalesce.WhenNull, true) + : Matches(coalesce.Value) || Matches(coalesce.WhenNull); + case ISimpleAssignmentOperation assignment: + return Matches(assignment.Value, requireDefinite); + case IAwaitOperation awaited: + return Carries(awaited.Operation, requireDefinite); + case IInvocationOperation invocation when _inference.GetConfiguredDisposableResource(invocation) is { } resource: + return Matches(resource, requireDefinite); + case IInvocationOperation invocation when _inference.GetFluentReceiver(invocation, _cancellationToken) is { } receiver: + return Matches(receiver, requireDefinite); + } + + return false; + } + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/OwnershipContracts.cs b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/OwnershipContracts.cs new file mode 100644 index 0000000..5849183 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/OwnershipContracts.cs @@ -0,0 +1,701 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Threading; +using System.Xml; +using System.Xml.Linq; +using ErrorProne.NET.CoreAnalyzers; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; + +namespace ErrorProne.NET.DisposableAnalyzers; + +internal enum OwnershipKind +{ + Unspecified, + Owned, + Borrowed, +} + +/// +/// Resolves source contracts before external contracts; unspecified results are left to the analyzer. +/// Additional files named *.ownership.xml contain: +/// <ownership><member id="M:Namespace.Type.Method(System.IDisposable)" +/// assembly="AssemblyName" returns="owned|borrowed"> +/// <parameter name="resource" ownership="owned|borrowed"/> +/// </member></ownership>. +/// Assembly and returns are optional. Parameter names belong to the declared member, not its callers. +/// Assembly names are simple names, compared ordinally. Compatible assembly-specific entries supplement +/// unqualified entries; overlapping conflicting contracts are rejected. Unavailable libraries are allowed. +/// +internal sealed class OwnershipContracts +{ + private readonly ImmutableDictionary _external; + + private OwnershipContracts(ImmutableDictionary external, + ImmutableArray diagnostics) + { + _external = external; + Diagnostics = diagnostics; + } + + public ImmutableArray Diagnostics { get; } + + public static OwnershipContracts Create(Compilation compilation, AnalyzerOptions options, + CancellationToken cancellationToken) + { + var contracts = new Dictionary(StringComparer.Ordinal); + var membersById = new Dictionary>(StringComparer.Ordinal); + var diagnostics = ImmutableArray.CreateBuilder(); + foreach (var file in options.AdditionalFiles.OrderBy(file => file.Path, StringComparer.Ordinal)) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!file.Path.EndsWith(".ownership.xml", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + var text = file.GetText(cancellationToken); + void Report(string message, XObject? node = null, int line = 1, int column = 1) + { + if (node is IXmlLineInfo info && info.HasLineInfo()) + { + line = info.LineNumber; + column = info.LinePosition; + } + + var position = 0; + if (text != null && line > 0 && line <= text.Lines.Count) + { + var sourceLine = text.Lines[line - 1]; + position = sourceLine.Start + Math.Min(Math.Max(column - 1, 0), sourceLine.Span.Length); + } + + var span = new TextSpan(position, 0); + var lineSpan = text?.Lines.GetLinePositionSpan(span) ?? default; + diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.ERP045, + Location.Create(file.Path, span, lineSpan), message)); + } + + if (text == null) + { + Report("The additional file could not be read."); + continue; + } + + try + { + using var input = new StringReader(text.ToString()); + using var reader = XmlReader.Create(input, new XmlReaderSettings + { + DtdProcessing = DtdProcessing.Prohibit, + XmlResolver = null, + }); + var document = XDocument.Load(reader, LoadOptions.SetLineInfo); + var root = document.Root!; + if (root.Name != "ownership" || !ValidateShape(root, Array.Empty(), Report)) + { + if (root.Name != "ownership") + { + Report("The root element must be 'ownership' without a namespace.", root); + } + + continue; + } + + foreach (var member in root.Elements()) + { + cancellationToken.ThrowIfCancellationRequested(); + if (member.Name != "member") + { + Report($"Unknown element '{member.Name}'.", member); + continue; + } + + if (!ValidateShape(member, new[] { "id", "assembly", "returns" }, Report)) + { + continue; + } + + var id = (string?)member.Attribute("id"); + if (id == null || !DocumentationIdSyntax.IsValid(id)) + { + Report($"Invalid method or property documentation ID '{id}'.", member); + continue; + } + + var assembly = (string?)member.Attribute("assembly"); + if (assembly != null && (string.IsNullOrWhiteSpace(assembly) + || assembly != assembly.Trim() || assembly.IndexOfAny(new[] { ',', '\\', '/' }) >= 0)) + { + Report("Assembly must be a nonempty simple assembly name.", member); + continue; + } + + var returnsText = (string?)member.Attribute("returns"); + var returns = ParseOwnership(returnsText); + if (returnsText != null && returns == OwnershipKind.Unspecified) + { + Report($"Unknown return ownership '{returnsText}'.", member); + continue; + } + + var parameters = ImmutableDictionary.CreateBuilder(StringComparer.Ordinal); + var valid = true; + foreach (var parameter in member.Elements()) + { + if (parameter.Name != "parameter") + { + Report($"Unknown element '{parameter.Name}'.", parameter); + valid = false; + continue; + } + + if (!ValidateShape(parameter, new[] { "name", "ownership" }, Report)) + { + valid = false; + continue; + } + + var name = (string?)parameter.Attribute("name"); + var ownership = ParseOwnership((string?)parameter.Attribute("ownership")); + if (string.IsNullOrWhiteSpace(name) || name != name!.Trim() + || ownership == OwnershipKind.Unspecified || parameter.HasElements) + { + Report("A parameter needs a name, ownership 'owned' or 'borrowed', and no child elements.", parameter); + valid = false; + } + else if (parameters.ContainsKey(name)) + { + Report($"Duplicate parameter '{name}'.", parameter); + valid = false; + } + else + { + parameters.Add(name, ownership); + } + } + + if (!valid) + { + continue; + } + + if (returns == OwnershipKind.Unspecified && parameters.Count == 0) + { + Report("A member must specify return or parameter ownership.", member); + continue; + } + + // Only validate binding when the target exists. Packs may also contain absent libraries. + var symbols = DocumentationCommentId.GetSymbolsForDeclarationId(id, compilation) + .Where(s => assembly == null || s.ContainingAssembly?.Name == assembly).ToArray(); + if (symbols.Length != 0 && parameters.Keys.Any(name => + !symbols.Any(s => GetParameters(s).Any(p => p.Name == name)))) + { + Report($"A parameter name does not belong to '{id}'.", member); + continue; + } + + var entry = new ExternalContract(id, assembly, returns, parameters.ToImmutable()); + var key = Key(id, assembly); + if (contracts.ContainsKey(key)) + { + Report($"Duplicate member contract '{id}'.", member); + } + else if (membersById.TryGetValue(id, out var existingMembers) && existingMembers.Any(existing => + (existing.Assembly == null || assembly == null || existing.Assembly == assembly) + && Conflicts(existing, entry))) + { + Report($"Conflicting member contract '{id}'.", member); + } + else + { + contracts.Add(key, entry); + if (!membersById.TryGetValue(id, out var entries)) + { + entries = new List(); + membersById.Add(id, entries); + } + + entries.Add(entry); + } + } + } + catch (XmlException ex) + { + Report($"Invalid XML: {ex.Message}", line: ex.LineNumber, column: ex.LinePosition); + } + } + + return new OwnershipContracts(contracts.ToImmutableDictionary(StringComparer.Ordinal), diagnostics.ToImmutable()); + } + + public OwnershipKind GetReturnOwnership(ISymbol methodOrProperty) + { + var members = RelatedMembers(Normalize(methodOrProperty)).ToArray(); + foreach (var member in members) + { + var source = ReturnAttributeOwnership(member); + if (source != OwnershipKind.Unspecified) + { + return source; + } + } + + foreach (var member in members) + { + foreach (var contract in ExternalContracts(member)) + { + if (contract.Returns != OwnershipKind.Unspecified) + { + return contract.Returns; + } + } + } + + return OwnershipKind.Unspecified; + } + + public bool AcquiresOwnership(IParameterSymbol parameter) => ParameterOwnership(parameter) == OwnershipKind.Owned; + + public bool IsBorrowed(ISymbol symbol) + { + return symbol is IParameterSymbol parameter + ? ParameterOwnership(parameter) == OwnershipKind.Borrowed + : HasBorrowedAttribute(symbol.GetAttributes()) + || (symbol is IMethodSymbol or IPropertySymbol && GetReturnOwnership(symbol) == OwnershipKind.Borrowed); + } + + private OwnershipKind ParameterOwnership(IParameterSymbol parameter) + { + var ordinal = parameter.Ordinal; + var owner = parameter.ContainingSymbol; + if (owner is IMethodSymbol { ReducedFrom: { } reduced }) + { + ordinal += reduced.Parameters.Length - ((IMethodSymbol)owner).Parameters.Length; + owner = reduced; + } + + // Accessor parameters and indexer arguments must resolve the same property contract. + if (owner is IMethodSymbol { AssociatedSymbol: IPropertySymbol property } + && ordinal < property.Parameters.Length) + { + owner = property; + } + + owner = owner.OriginalDefinition; + var members = RelatedMembers(owner).ToArray(); + var inherited = OwnershipKind.Unspecified; + for (var i = 0; i < members.Length; i++) + { + var parameters = GetParameters(members[i]); + if (ordinal >= parameters.Length) + { + continue; + } + + var attributes = parameters[ordinal].GetAttributes(); + var source = HasBorrowedAttribute(attributes) + ? OwnershipKind.Borrowed + : HasAttribute(attributes, DisposableAttributes.AcquiresOwnershipAttribute) + ? OwnershipKind.Owned : OwnershipKind.Unspecified; + if (source == OwnershipKind.Borrowed || i == 0 && source != OwnershipKind.Unspecified) + { + return source; + } + + if (source != OwnershipKind.Unspecified) + { + inherited = source; + } + } + + if (inherited != OwnershipKind.Unspecified) + { + return inherited; + } + + foreach (var member in members) + { + var parameters = GetParameters(member); + if (ordinal >= parameters.Length) + { + continue; + } + + foreach (var contract in ExternalContracts(member)) + { + if (contract.Parameters.TryGetValue(parameters[ordinal].Name, out var ownership)) + { + return ownership; + } + } + } + + return OwnershipKind.Unspecified; + } + + private IEnumerable ExternalContracts(ISymbol member) + { + var id = member.GetDocumentationCommentId(); + if (id == null) + { + yield break; + } + + if (_external.TryGetValue(Key(id, member.ContainingAssembly?.Name), out var specific)) + { + yield return specific; + } + + if (_external.TryGetValue(Key(id, null), out var general)) + { + yield return general; + } + } + + private static ISymbol Normalize(ISymbol symbol) + { + if (symbol is IMethodSymbol method) + { + symbol = method.ReducedFrom ?? method; + if (symbol is IMethodSymbol { AssociatedSymbol: IPropertySymbol property }) + { + symbol = property; + } + } + + return symbol.OriginalDefinition; + } + + private static IEnumerable RelatedMembers(ISymbol symbol) + { + var seen = new HashSet(SymbolEqualityComparer.Default); + var chain = new List(); + for (ISymbol? current = symbol; current != null; current = current switch + { + IMethodSymbol method => method.OverriddenMethod?.OriginalDefinition, + IPropertySymbol property => property.OverriddenProperty?.OriginalDefinition, + _ => null, + }) + { + chain.Add(current); + if (seen.Add(current)) + { + yield return current; + } + } + + if (symbol.ContainingType == null) + { + yield break; + } + + foreach (var type in symbol.ContainingType.AllInterfaces) + { + foreach (var member in type.GetMembers()) + { + if (member.Kind != symbol.Kind) + { + continue; + } + + var implementation = symbol.ContainingType.FindImplementationForInterfaceMember(member); + if (implementation != null + && chain.Any(candidate => SymbolEqualityComparer.Default.Equals( + candidate, implementation.OriginalDefinition)) + && seen.Add(member.OriginalDefinition)) + { + yield return member.OriginalDefinition; + } + } + } + } + + private static OwnershipKind ReturnAttributeOwnership(ISymbol member) + { + var attributes = member.GetAttributes(); + if (member is IMethodSymbol method) + { + attributes = attributes.AddRange(method.GetReturnTypeAttributes()); + } + else if (member is IPropertySymbol { GetMethod: { } getter }) + { + attributes = attributes.AddRange(getter.GetAttributes()).AddRange(getter.GetReturnTypeAttributes()); + } + + if (HasAttribute(attributes, DisposableAttributes.KeepsOwnershipAttribute) + || HasAttribute(attributes, DisposableAttributes.DoNotDisposeAttribute) + || member is IPropertySymbol && HasAttribute(attributes, DisposableAttributes.NoOwnershipAttribute)) + { + return OwnershipKind.Borrowed; + } + + return HasAttribute(attributes, DisposableAttributes.ReturnsOwnershipAttribute) + ? OwnershipKind.Owned : OwnershipKind.Unspecified; + } + + private static bool HasBorrowedAttribute(ImmutableArray attributes) => + HasAttribute(attributes, DisposableAttributes.DoNotDisposeAttribute) + || HasAttribute(attributes, DisposableAttributes.NoOwnershipAttribute); + + private static bool HasAttribute(ImmutableArray attributes, string name) => + attributes.Any(attribute => attribute.AttributeClass?.Name == name); + + private static ImmutableArray GetParameters(ISymbol symbol) => symbol switch + { + IMethodSymbol method => method.Parameters, + IPropertySymbol property => property.Parameters, + _ => ImmutableArray.Empty, + }; + + private static OwnershipKind ParseOwnership(string? value) => value switch + { + "owned" => OwnershipKind.Owned, + "borrowed" => OwnershipKind.Borrowed, + _ => OwnershipKind.Unspecified, + }; + + private static bool ValidateShape(XElement element, string[] attributes, + Action report) + { + foreach (var attribute in element.Attributes()) + { + if (!attributes.Contains(attribute.Name.ToString(), StringComparer.Ordinal)) + { + report($"Unknown attribute '{attribute.Name}'.", attribute, 1, 1); + return false; + } + } + + if (element.Nodes().OfType().Any(node => !string.IsNullOrWhiteSpace(node.Value))) + { + report("Text content is not allowed in ownership annotations.", element, 1, 1); + return false; + } + + return true; + } + + private static bool Conflicts(ExternalContract left, ExternalContract right) => + left.Returns != OwnershipKind.Unspecified && right.Returns != OwnershipKind.Unspecified + && left.Returns != right.Returns + || left.Parameters.Any(parameter => right.Parameters.TryGetValue(parameter.Key, out var ownership) + && ownership != parameter.Value); + + private static string Key(string id, string? assembly) => (assembly ?? "") + "\0" + id; + + private sealed class ExternalContract + { + public ExternalContract(string id, string? assembly, OwnershipKind returns, + ImmutableDictionary parameters) + { + Id = id; + Assembly = assembly; + Returns = returns; + Parameters = parameters; + } + + public string Id { get; } + public string? Assembly { get; } + public OwnershipKind Returns { get; } + public ImmutableDictionary Parameters { get; } + } + + // Binding alone cannot validate IDs for libraries absent from the compilation. This small parser + // accepts declaration-ID names, generic arguments/parameters, arrays, pointers, refs and conversions. + private sealed class DocumentationIdSyntax + { + private readonly string _text; + private int _position = 2; + + private DocumentationIdSyntax(string text) => _text = text; + + public static bool IsValid(string text) + { + if (text.Length < 5 || (text[0] != 'M' && text[0] != 'P') || text[1] != ':') + { + return false; + } + + var parser = new DocumentationIdSyntax(text); + var start = parser._position; + if (!parser.Name(0) || text.Substring(start, parser._position - start).IndexOf('.') < 0) + { + return false; + } + + if (parser.Take('(')) + { + if (!parser.Type(0)) + { + return false; + } + + while (parser.Take(',')) + { + if (!parser.Type(0)) + { + return false; + } + } + + if (!parser.Take(')')) + { + return false; + } + } + + if (parser.Take('~') && (text[0] != 'M' || !parser.Type(0))) + { + return false; + } + + return parser._position == text.Length; + } + + private bool Type(int depth) + { + if (depth > 32) + { + return false; + } + + if (Take('`')) + { + Take('`'); + if (!Digits()) + { + return false; + } + } + else if (!Name(depth)) + { + return false; + } + + while (_position < _text.Length) + { + if (Take('*')) + { + continue; + } + + if (!Take('[')) + { + break; + } + + if (Take(']')) + { + continue; + } + + do + { + Take('-'); + Digits(); + if (!Take(':')) + { + return false; + } + + Take('-'); + Digits(); + } while (Take(',')); + if (!Take(']')) + { + return false; + } + } + + Take('@'); + return true; + } + + private bool Name(int depth) + { + if (depth > 32) + { + return false; + } + + do + { + // '#' also encodes explicit-interface separators and constructor names. + Take('#'); + if (_position == _text.Length || !IdentifierStart(_text[_position])) + { + return false; + } + + _position++; + while (_position < _text.Length && + (IdentifierStart(_text[_position]) || char.IsDigit(_text[_position]))) + { + _position++; + } + + if (Take('`')) + { + Take('`'); + if (_position == _text.Length || _text[_position] == '0' || !Digits()) + { + return false; + } + } + + if (Take('{')) + { + if (!Type(depth + 1)) + { + return false; + } + + while (Take(',')) + { + if (!Type(depth + 1)) + { + return false; + } + } + + if (!Take('}')) + { + return false; + } + } + } while (Take('.') || Take('#')); + + return true; + } + + private bool Digits() + { + var start = _position; + while (_position < _text.Length && _text[_position] >= '0' && _text[_position] <= '9') + { + _position++; + } + + return _position != start; + } + + private bool Take(char value) + { + if (_position == _text.Length || _text[_position] != value) + { + return false; + } + + _position++; + return true; + } + + private static bool IdentifierStart(char value) => char.IsLetter(value) || value == '_'; + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/OwnershipInference.cs b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/OwnershipInference.cs new file mode 100644 index 0000000..fcb0a30 --- /dev/null +++ b/src/ErrorProne.NET.CoreAnalyzers/DisposableAnalyzers/OwnershipInference.cs @@ -0,0 +1,533 @@ +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using ErrorProne.NET.Core; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace ErrorProne.NET.DisposableAnalyzers; + +internal sealed class OwnershipInference +{ + private readonly Compilation _compilation; + private readonly OwnershipContracts _contracts; + private readonly DisposeAnalysisHelper _helper; + private readonly IMethodSymbol? _configureAsyncDisposable; + private readonly IMethodSymbol? _disposeAsync; + private readonly IMethodSymbol? _configuredDisposeAsync; + private readonly HashSet _configureAwaitMethods; + private readonly HashSet _completedTaskFactories; + private readonly INamedTypeSymbol? _valueTaskOfT; + private readonly ConcurrentDictionary _acquires = new(SymbolEqualityComparer.Default); + private readonly ConcurrentDictionary _freshReturns = new(SymbolEqualityComparer.Default); + + public OwnershipInference(Compilation compilation, OwnershipContracts contracts, DisposeAnalysisHelper helper) + { + _compilation = compilation; + _contracts = contracts; + _helper = helper; + _disposeAsync = helper.IAsyncDisposable?.GetMembers("DisposeAsync").OfType() + .FirstOrDefault(method => !method.IsStatic && method.Parameters.IsEmpty); + _configuredDisposeAsync = helper.IConfigureAsyncDisposable?.GetMembers("DisposeAsync").OfType() + .FirstOrDefault(method => !method.IsStatic && method.Parameters.IsEmpty); + _configureAsyncDisposable = compilation.GetTypeByMetadataName("System.Threading.Tasks.TaskAsyncEnumerableExtensions") + ?.GetMembers("ConfigureAwait").OfType().FirstOrDefault(method => + SymbolEqualityComparer.Default.Equals(method.ReturnType, helper.IConfigureAsyncDisposable)); + _configureAwaitMethods = new HashSet(new[] + { + "System.Threading.Tasks.Task", "System.Threading.Tasks.Task`1", + "System.Threading.Tasks.ValueTask", "System.Threading.Tasks.ValueTask`1", + }.SelectMany(name => compilation.GetTypeByMetadataName(name)?.GetMembers("ConfigureAwait") + .OfType() ?? Enumerable.Empty()), SymbolEqualityComparer.Default); + _completedTaskFactories = new HashSet(new[] + { + "System.Threading.Tasks.Task", "System.Threading.Tasks.ValueTask", + }.SelectMany(name => compilation.GetTypeByMetadataName(name)?.GetMembers("FromResult") + .OfType() ?? Enumerable.Empty()), SymbolEqualityComparer.Default); + _valueTaskOfT = compilation.GetTypeByMetadataName("System.Threading.Tasks.ValueTask`1"); + } + + public IOperation? GetCompletedTaskValue(IOperation operation) + { + operation = DisposeBeforeLosingScopeAnalyzer.Unwrap(operation); + return operation switch + { + IInvocationOperation invocation when _completedTaskFactories.Contains(invocation.TargetMethod.OriginalDefinition) + && _contracts.GetReturnOwnership(invocation.TargetMethod) == OwnershipKind.Unspecified + => invocation.Arguments.FirstOrDefault(a => a.Parameter?.Ordinal == 0)?.Value, + IObjectCreationOperation { Constructor: { Parameters.Length: 1 } constructor } creation + when SymbolEqualityComparer.Default.Equals(constructor.ContainingType.OriginalDefinition, _valueTaskOfT) + && constructor.OriginalDefinition.Parameters[0].Type is ITypeParameterSymbol + => creation.Arguments.FirstOrDefault(a => a.Parameter?.Ordinal == 0)?.Value, + _ => null, + }; + } + + internal bool IsTaskResultCarrier(ITypeSymbol? type) => + type.IsTaskLike(_compilation, TaskLikeTypes.TaskOfT | TaskLikeTypes.ValueTaskOfT); + + public IOperation? GetConfiguredTask(IOperation operation) + { + return operation is IInvocationOperation { Instance: { } instance } invocation + && _configureAwaitMethods.Contains(invocation.TargetMethod.OriginalDefinition) + && _contracts.GetReturnOwnership(invocation.TargetMethod) == OwnershipKind.Unspecified + ? instance : null; + } + + public IOperation? GetConfiguredDisposableResource(IInvocationOperation invocation) + { + var method = (invocation.TargetMethod.ReducedFrom ?? invocation.TargetMethod).OriginalDefinition; + if (_configureAsyncDisposable == null + || !SymbolEqualityComparer.Default.Equals(method, _configureAsyncDisposable) + || _contracts.GetReturnOwnership(invocation.TargetMethod) != OwnershipKind.Unspecified) + { + return null; + } + + return invocation.Instance ?? invocation.Arguments.FirstOrDefault(a => a.Parameter?.Ordinal == 0)?.Value; + } + + public ISymbol? GetAwaitedMember(IOperation operation) + { + operation = DisposeBeforeLosingScopeAnalyzer.Unwrap(operation); + if (GetConfiguredTask(operation) is { } instance) + { + return GetAwaitedMember(instance); + } + + return operation switch + { + IInvocationOperation call => call.TargetMethod, + IPropertyReferenceOperation property => property.Property, + IConversionOperation conversion => conversion.OperatorMethod, + _ => null, + }; + } + + public IOperation? GetInterlockedOwningValue(IInvocationOperation invocation) + { + if (invocation.TargetMethod.Name != "Exchange" || invocation.Arguments.Length != 2 + || !SymbolEqualityComparer.Default.Equals(invocation.TargetMethod.ContainingType, + _compilation.GetTypeByMetadataName("System.Threading.Interlocked"))) + { + return null; + } + + var target = invocation.Arguments.FirstOrDefault(a => a.Parameter?.Ordinal == 0); + return target != null && DisposeBeforeLosingScopeAnalyzer.Unwrap(target.Value) is IMemberReferenceOperation member + && !_contracts.IsBorrowed(member.Member) + ? invocation.Arguments.FirstOrDefault(a => a.Parameter?.Ordinal == 1)?.Value : null; + } + + internal static IEnumerable<(IParameterSymbol Parameter, IOperation Value)> GetOperatorInputs(IOperation operation) + { + switch (operation) + { + case IConversionOperation { OperatorMethod: { Parameters.Length: 1 } method } conversion: + yield return (method.Parameters[0], conversion.Operand); + break; + case IUnaryOperation { OperatorMethod: { Parameters.Length: 1 } method } unary: + yield return (method.Parameters[0], unary.Operand); + break; + case IBinaryOperation { OperatorMethod: { Parameters.Length: 2 } method } binary: + yield return (method.Parameters[0], binary.LeftOperand); + yield return (method.Parameters[1], binary.RightOperand); + break; + case IIncrementOrDecrementOperation { OperatorMethod: { Parameters.Length: 1 } method } increment: + yield return (method.Parameters[0], increment.Target); + break; + case ICompoundAssignmentOperation { OperatorMethod: { Parameters.Length: 2 } method } assignment: + yield return (method.Parameters[0], assignment.Target); + yield return (method.Parameters[1], assignment.Value); + break; + } + } + + public bool ReturnsOwnership(IMethodSymbol method, CancellationToken cancellationToken) + { + var contract = _contracts.GetReturnOwnership(method); + if (contract != OwnershipKind.Unspecified) + { + return contract == OwnershipKind.Owned; + } + + return HasFreshReturn(method, cancellationToken); + } + + public bool IsFluentAlias(IMethodSymbol method, CancellationToken cancellationToken) + { + if (_contracts.GetReturnOwnership(method) != OwnershipKind.Unspecified) + { + return false; + } + + var candidate = (!method.IsStatic && SymbolEqualityComparer.Default.Equals(method.ReturnType, method.ContainingType)) + || (method.IsExtensionMethod && method.OriginalDefinition.ReturnType is ITypeParameterSymbol + && method.Parameters.Length > 0 + && SymbolEqualityComparer.Default.Equals(method.ReturnType, method.Parameters[0].Type)); + return candidate && !HasFreshReturn(method, cancellationToken); + } + + public IOperation? GetFluentReceiver(IInvocationOperation invocation, CancellationToken cancellationToken = default) + { + return IsFluentAlias(invocation.TargetMethod, cancellationToken) + ? invocation.Instance ?? invocation.Arguments.FirstOrDefault(a => a.Parameter?.Ordinal == 0)?.Value + : null; + } + + private bool HasFreshReturn(IMethodSymbol method, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + var definition = (method.ReducedFrom ?? method).OriginalDefinition; + return _freshReturns.GetOrAdd(definition, m => InferFreshReturn(m, cancellationToken)); + } + + private bool InferFreshReturn(IMethodSymbol method, CancellationToken cancellationToken) + { + method = method.PartialImplementationPart ?? method; + if (method.MethodKind != MethodKind.Ordinary || method.IsAbstract || method.IsExtern + || ((method.IsVirtual || method.IsOverride) && !method.IsSealed && !method.ContainingType.IsSealed)) + { + return false; + } + + foreach (var operation in GetSourceOperations(method, cancellationToken)) + { + if (operation is not IMethodBodyOperation body) + { + continue; + } + + // Only a direct return: no branch joins, local alias tracking, or factory forwarding. + var block = body.BlockBody ?? body.ExpressionBody; + if (block?.Operations.Length == 1 + && block.Operations[0] is IReturnOperation { Kind: OperationKind.Return, ReturnedValue: { } value }) + { + return IsFreshCreation(value); + } + } + + return false; + } + + private IEnumerable GetSourceOperations(ISymbol symbol, CancellationToken cancellationToken) + { + foreach (var reference in symbol.DeclaringSyntaxReferences) + { + cancellationToken.ThrowIfCancellationRequested(); + // Project references can expose syntax from a different compilation. + if (!_compilation.ContainsSyntaxTree(reference.SyntaxTree)) + { + continue; + } + + var syntax = reference.GetSyntax(cancellationToken); + var model = _compilation.GetSemanticModel(syntax.SyntaxTree); + if (model.GetOperation(syntax, cancellationToken) is { } operation) + { + yield return operation; + } + } + } + + private bool IsFreshCreation(IOperation value) + { + while (true) + { + switch (value) + { + case IParenthesizedOperation parenthesized: + value = parenthesized.Operand; + break; + case IConversionOperation conversion when conversion.Conversion.Exists + && !conversion.Conversion.IsUserDefined + && conversion.Type?.TypeKind != TypeKind.Dynamic + && conversion.Operand.Type?.TypeKind != TypeKind.Dynamic: + value = conversion.Operand; + break; + default: + return value is IObjectCreationOperation or ITypeParameterObjectCreationOperation + && _helper.ShouldBeDisposed(value.Type); + } + } + } + + public bool AcquiresOwnership(IParameterSymbol parameter, ImmutableArray arguments, + CancellationToken cancellationToken) + { + return KnownAcquisition(parameter, arguments) ?? _acquires.GetOrAdd(parameter.OriginalDefinition, + p => InferAcquisition(p, new HashSet(SymbolEqualityComparer.Default), cancellationToken)); + } + + public bool IsUnknownArgument(IArgumentOperation argument, ImmutableArray arguments) + { + return argument.Parameter is not { } parameter + || KnownAcquisition(parameter, arguments) == null; + } + + private bool? KnownAcquisition(IParameterSymbol parameter, ImmutableArray arguments) + { + if (_contracts.AcquiresOwnership(parameter)) + { + return true; + } + + if (_contracts.IsBorrowed(parameter)) + { + return false; + } + + if (IsStreamWrapperParameter(parameter)) + { + var leaveOpen = arguments.FirstOrDefault(a => a.Parameter?.Name == "leaveOpen"); + return leaveOpen == null ? true + : leaveOpen.Value.ConstantValue is { HasValue: true, Value: bool value } ? !value + : null; + } + + return null; + } + + private bool InferAcquisition(IParameterSymbol parameter, HashSet visiting, + CancellationToken cancellationToken) + { + if (_contracts.AcquiresOwnership(parameter)) + { + return true; + } + + if (_contracts.IsBorrowed(parameter) || visiting.Count >= 4 || !visiting.Add(parameter)) + { + return false; + } + + try + { + foreach (var root in GetSourceOperations(parameter.ContainingSymbol, cancellationToken)) + { + var aliases = new HashSet(SymbolEqualityComparer.Default); + var carriers = new HashSet(SymbolEqualityComparer.Default); + (IsTaskResultCarrier(parameter.Type) ? carriers : aliases).Add(parameter); + foreach (var operation in DisposeBeforeLosingScopeAnalyzer.EnumerateOperations(root)) + { + cancellationToken.ThrowIfCancellationRequested(); + if (DisposeBeforeLosingScopeAnalyzer.GetPatternAlias(operation) is { } patternAlias) + { + TrackAlias(patternAlias.Local, patternAlias.Value, conditional: true); + } + + if (GetOperatorInputs(operation).Any(input => + (ReferencesResource(input.Value, aliases, carriers) || ReferencesCarrier(input.Value, aliases, carriers)) + && (KnownAcquisition(input.Parameter, ImmutableArray.Empty) + ?? InferAcquisition(input.Parameter.OriginalDefinition, visiting, cancellationToken)))) + { + return true; + } + + if (operation is IInvocationOperation invocation) + { + var published = GetInterlockedOwningValue(invocation); + if ((IsDisposeCall(invocation) && ReferencesResource(invocation.Instance, aliases, carriers)) + || ReferencesResource(published, aliases, carriers) || ReferencesCarrier(published, aliases, carriers)) + { + return true; + } + + if (ForwardsOwnership(invocation.Arguments, aliases, carriers, visiting, cancellationToken)) + { + return true; + } + } + else if (operation is IObjectCreationOperation creation + && ForwardsOwnership(creation.Arguments, aliases, carriers, visiting, cancellationToken)) + { + return true; + } + else if (operation is IPropertyReferenceOperation property + && ForwardsOwnership(property.Arguments, aliases, carriers, visiting, cancellationToken)) + { + return true; + } + else if (operation is IUsingDeclarationOperation declaration && declaration.DeclarationGroup.Declarations + .SelectMany(d => d.Declarators).Any(d => aliases.Contains(d.Symbol))) + { + return true; + } + else if (operation is IVariableDeclaratorOperation { Initializer: { } initializer } declarator) + { + TrackAlias(declarator.Symbol, initializer.Value, conditional: false); + } + else if (operation is ISimpleAssignmentOperation assignment) + { + var target = DisposeBeforeLosingScopeAnalyzer.GetAliasSymbol(assignment.Target); + if ((ReferencesResource(assignment.Value, aliases, carriers) + || ReferencesCarrier(assignment.Value, aliases, carriers)) + && assignment.Target is IMemberReferenceOperation member && !_contracts.IsBorrowed(member.Member)) + { + return true; + } + + if (target != null) + { + TrackAlias(target, assignment.Value, DisposeBeforeLosingScopeAnalyzer.IsConditional(assignment)); + } + } + + if (DisposeBeforeLosingScopeAnalyzer.GetUsingCapture(operation) is { } captured + && (ReferencesResource(operation, aliases, carriers) || captured.Locals.Any(aliases.Contains))) + { + return true; + } + } + + void TrackAlias(ISymbol target, IOperation value, bool conditional) + { + var resource = ReferencesResource(value, aliases, carriers); + var carrier = ReferencesCarrier(value, aliases, carriers); + if (resource) + { + aliases.Add(target); + } + else if (!conditional) + { + aliases.Remove(target); + } + + if (carrier) + { + carriers.Add(target); + } + else if (!conditional) + { + carriers.Remove(target); + } + } + } + } + finally + { + visiting.Remove(parameter); + } + + return false; + } + + private bool ForwardsOwnership(ImmutableArray arguments, HashSet aliases, HashSet carriers, + HashSet visiting, CancellationToken cancellationToken) + { + return arguments.Any(a => a.Parameter != null + && (ReferencesResource(a.Value, aliases, carriers) || ReferencesCarrier(a.Value, aliases, carriers)) + && (KnownAcquisition(a.Parameter, arguments) + ?? InferAcquisition(a.Parameter.OriginalDefinition, visiting, cancellationToken))); + } + + private static bool IsStreamWrapperParameter(IParameterSymbol parameter) + { + return parameter.ContainingSymbol is IMethodSymbol { MethodKind: MethodKind.Constructor } constructor + && parameter.Type.ToDisplayString() == "System.IO.Stream" + && constructor.ContainingType.ToDisplayString() is + "System.IO.StreamReader" or "System.IO.StreamWriter" or "System.IO.BinaryReader" or "System.IO.BinaryWriter"; + } + + internal bool IsDisposeCall(IInvocationOperation invocation) + { + var method = invocation.TargetMethod; + if (method.IsStatic || !invocation.Arguments.IsEmpty) + { + return false; + } + + if (method.Name is "Dispose" or "Close" && method.ReturnsVoid) + { + return true; + } + + if (method.Name != "DisposeAsync") + { + return false; + } + + if (SymbolEqualityComparer.Default.Equals(method.OriginalDefinition, _disposeAsync) + || SymbolEqualityComparer.Default.Equals(method.OriginalDefinition, _configuredDisposeAsync)) + { + return true; + } + + var receiver = invocation.Instance?.Type as INamedTypeSymbol ?? method.ContainingType; + var implementation = _disposeAsync == null ? null : receiver.FindImplementationForInterfaceMember(_disposeAsync); + for (IMethodSymbol? candidate = method; candidate != null; candidate = candidate.OverriddenMethod) + { + if (SymbolEqualityComparer.Default.Equals(candidate.OriginalDefinition, implementation?.OriginalDefinition)) + { + return true; + } + } + + return false; + } + + private bool ReferencesResource(IOperation? operation, HashSet aliases, HashSet carriers) + { + if (operation == null) + { + return false; + } + + operation = DisposeBeforeLosingScopeAnalyzer.Unwrap(operation); + if (DisposeBeforeLosingScopeAnalyzer.GetAliasSymbol(operation) is { } symbol) + { + return aliases.Contains(symbol); + } + + if (operation is IConditionalAccessInstanceOperation) + { + return ReferencesResource(DisposeBeforeLosingScopeAnalyzer.GetConditionalReceiver(operation), aliases, carriers); + } + + if (operation is IInvocationOperation invocation && GetConfiguredDisposableResource(invocation) is { } resource) + { + return ReferencesResource(resource, aliases, carriers); + } + + if (operation is ISimpleAssignmentOperation assignment) + { + return ReferencesResource(assignment.Value, aliases, carriers); + } + + return operation is IAwaitOperation awaited && ReferencesCarrier(awaited.Operation, aliases, carriers); + } + + private bool ReferencesCarrier(IOperation? operation, HashSet aliases, HashSet carriers) + { + if (operation == null) + { + return false; + } + + operation = DisposeBeforeLosingScopeAnalyzer.Unwrap(operation); + if (DisposeBeforeLosingScopeAnalyzer.GetAliasSymbol(operation) is { } symbol) + { + return carriers.Contains(symbol); + } + + if (GetCompletedTaskValue(operation) is { } result) + { + return ReferencesResource(result, aliases, carriers); + } + + if (GetConfiguredTask(operation) is { } task) + { + return ReferencesCarrier(task, aliases, carriers); + } + + if (operation is IInvocationOperation invocation && GetFluentReceiver(invocation) is { } receiver) + { + return ReferencesCarrier(receiver, aliases, carriers); + } + + return operation is ISimpleAssignmentOperation assignment + && ReferencesCarrier(assignment.Value, aliases, carriers); + } +} diff --git a/src/ErrorProne.NET.CoreAnalyzers/SymbolAnalysisContextExtensions.cs b/src/ErrorProne.NET.CoreAnalyzers/SymbolAnalysisContextExtensions.cs index d386b5b..bf80835 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/SymbolAnalysisContextExtensions.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/SymbolAnalysisContextExtensions.cs @@ -1,10 +1,4 @@ -// -------------------------------------------------------------------- -// -// Copyright (c) Microsoft Corporation. All rights reserved. -// -// -------------------------------------------------------------------- - -using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; using System.Diagnostics.CodeAnalysis; diff --git a/src/ErrorProne.NET.CoreAnalyzers/SymbolExtensions.cs b/src/ErrorProne.NET.CoreAnalyzers/SymbolExtensions.cs index bab4c3b..8e42a4a 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/SymbolExtensions.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/SymbolExtensions.cs @@ -20,6 +20,11 @@ public enum SymbolVisibility /// public static class SymbolExtensions { + public static bool HasAttributeWithName(this ISymbol? symbol, string attributeName) + { + return symbol?.GetAttributes().Any(a => a.AttributeClass?.Name == attributeName) == true; + } + public static bool IsPartialDefinition(this INamedTypeSymbol symbol) { return symbol.DeclaringSyntaxReferences @@ -298,5 +303,33 @@ public static bool ExceptionFromCatchBlock(this ISymbol symbol) // Use following code if the trick with DeclaredSyntaxReferences would not work properly! // return (bool?)(symbol.GetType().GetRuntimeProperty("IsCatch")?.GetValue(symbol)) == true; } + + public static bool IsPrivate(this ISymbol symbol) + { + return symbol.DeclaredAccessibility == Accessibility.Private; + } } + + public static class MethodSymbolExtensions + { + /// + /// Checks if the given method matches Dispose method convention and can be recognized by "using". + /// + public static bool HasDisposeSignatureByConvention(this IMethodSymbol method) + { + return method.HasDisposeMethodSignature() + && !method.IsStatic + && !method.IsPrivate(); + } + + /// + /// Checks if the given method has the signature "void Dispose()". + /// + private static bool HasDisposeMethodSignature(this IMethodSymbol method) + { + return method.Name == "Dispose" && method.MethodKind == MethodKind.Ordinary && + method.ReturnsVoid && method.Parameters.IsEmpty; + } + } + } \ No newline at end of file diff --git a/src/ErrorProne.NET.CoreAnalyzers/TypeExtensions.cs b/src/ErrorProne.NET.CoreAnalyzers/TypeExtensions.cs index 38b89b9..e05b583 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/TypeExtensions.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/TypeExtensions.cs @@ -209,5 +209,128 @@ public static bool OverridesToString(this ITypeSymbol type) .Any(t => t.GetMembers(nameof(ToString)).Any(m => m.IsOverride)); } + /// + /// Return true if a given derives from . + /// + public static bool DerivesFrom([NotNullWhen(returnValue: true)] this ITypeSymbol? symbol, [NotNullWhen(returnValue: true)] ITypeSymbol? candidateBaseType, bool baseTypesOnly = false, bool checkTypeParameterConstraints = true) + { + if (candidateBaseType == null || symbol == null) + { + return false; + } + + var candidateIsDefinition = SymbolEqualityComparer.Default.Equals(candidateBaseType.OriginalDefinition, candidateBaseType); + if (!baseTypesOnly && candidateBaseType.TypeKind == TypeKind.Interface) + { + var allInterfaces = symbol.AllInterfaces.OfType(); + if (candidateIsDefinition) + { + // Candidate base type is not a constructed generic type, so use original definition for interfaces. + allInterfaces = allInterfaces.Select(i => i.OriginalDefinition); + } + + if (allInterfaces.Contains(candidateBaseType, SymbolEqualityComparer.Default)) + { + return true; + } + } + + if (checkTypeParameterConstraints && symbol.TypeKind == TypeKind.TypeParameter) + { + var typeParameterSymbol = (ITypeParameterSymbol)symbol; + foreach (var constraintType in typeParameterSymbol.ConstraintTypes) + { + if (constraintType.DerivesFrom(candidateBaseType, baseTypesOnly, checkTypeParameterConstraints)) + { + return true; + } + } + } + + while (symbol != null) + { + if (SymbolEqualityComparer.Default.Equals( + candidateIsDefinition ? symbol.OriginalDefinition : symbol, candidateBaseType)) + { + return true; + } + + symbol = symbol.BaseType; + } + + return false; + } + + /// + /// Indicates if the given is disposable, + /// and thus can be used in a using or await using statement. + /// + public static bool IsDisposable(this ITypeSymbol type, + INamedTypeSymbol? iDisposable, + INamedTypeSymbol? iAsyncDisposable, + INamedTypeSymbol? configuredAsyncDisposable) + { + if (IsInterfaceOrImplementsInterface(type, iDisposable) + || IsInterfaceOrImplementsInterface(type, iAsyncDisposable) + || SymbolEqualityComparer.Default.Equals(type, configuredAsyncDisposable)) + { + return true; + } + + if (type.IsRefLikeType) + { + return type.GetMembers("Dispose").OfType() + .Any(method => method.HasDisposeSignatureByConvention()); + } + + return false; + + static bool IsInterfaceOrImplementsInterface(ITypeSymbol type, INamedTypeSymbol? interfaceType) + { + if (interfaceType == null) + { + return false; + } + + if (SymbolEqualityComparer.Default.Equals(type, interfaceType) + || type.AllInterfaces.Contains(interfaceType, SymbolEqualityComparer.Default)) + { + return true; + } + + if (type is not ITypeParameterSymbol parameter) + { + return false; + } + + var pending = new Stack(parameter.ConstraintTypes); + var visited = new HashSet(SymbolEqualityComparer.Default); + while (pending.Count != 0) + { + var constraint = pending.Pop(); + if (!visited.Add(constraint)) + { + continue; + } + + if (SymbolEqualityComparer.Default.Equals(constraint, interfaceType) + || constraint.AllInterfaces.Contains(interfaceType, SymbolEqualityComparer.Default)) + { + return true; + } + + if (constraint is ITypeParameterSymbol nested) + { + foreach (var next in nested.ConstraintTypes) + { + pending.Push(next); + } + } + } + + return false; + } + } + } } \ No newline at end of file diff --git a/src/ErrorProne.NET.CoreAnalyzers/WellKnownTypesProvider.cs b/src/ErrorProne.NET.CoreAnalyzers/WellKnownTypesProvider.cs index 6d5c184..abbc544 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/WellKnownTypesProvider.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/WellKnownTypesProvider.cs @@ -42,4 +42,10 @@ public static WellKnownTypeProvider GetOrCreate(Compilation compilation) v => Compilation.GetBestTypeByMetadataName(v)); } } + + public static class WellKnownTypeNames + { + public const string SystemIAsyncDisposable = "System.IAsyncDisposable"; + public const string SystemIDisposable = "System.IDisposable"; + } } \ No newline at end of file diff --git a/src/ErrorProne.NET.sln b/src/ErrorProne.NET.sln index df6929e..295db41 100644 --- a/src/ErrorProne.NET.sln +++ b/src/ErrorProne.NET.sln @@ -16,6 +16,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution .editorconfig = .editorconfig EndProjectSection EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ErrorProne.NET.Annotations", ".\ErrorProne.NET.Annotations\ErrorProne.NET.Annotations.csproj", "{CADD4DC8-2D23-4DC5-BBA0-F489CD212710}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -38,6 +40,10 @@ Global {1E47DB73-DAF6-4EB6-B3F2-966FDF8E7179}.Debug|Any CPU.Build.0 = Debug|Any CPU {1E47DB73-DAF6-4EB6-B3F2-966FDF8E7179}.Release|Any CPU.ActiveCfg = Release|Any CPU {1E47DB73-DAF6-4EB6-B3F2-966FDF8E7179}.Release|Any CPU.Build.0 = Release|Any CPU + {CADD4DC8-2D23-4DC5-BBA0-F489CD212710}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {CADD4DC8-2D23-4DC5-BBA0-F489CD212710}.Debug|Any CPU.Build.0 = Debug|Any CPU + {CADD4DC8-2D23-4DC5-BBA0-F489CD212710}.Release|Any CPU.ActiveCfg = Release|Any CPU + {CADD4DC8-2D23-4DC5-BBA0-F489CD212710}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE