Skip to content

Fix false positives for Dispose rules #2348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 16, 2019
Merged

Conversation

mavasani
Copy link
Contributor

Fixes #2306: CA2213 (DisposableFieldsShouldBeDisposed)

  1. ba955a4: Ensure that analysis is executed on generated code to account for Dispose implementation in generated code files.

Fixes #2245 (2 scenarios): CA2000 (DisposeObjectsBeforeLosingScope)

  1. 74dd361: Handle escaping of disposable creations through ref/out field arguments.
  2. 5f3209a: Prevent false positives for disposable out arguments of invocations within conditional expression. We assume that an out argument of an invocation returning false within a conditional branch is invalid and does not require disposing.

…ons within conditional expression

We assume that out argument of an invocation returning false within a conditional branch is invalid and does not require disposing.

Fixes another scenario in dotnet#2245
@mavasani mavasani requested review from genlu, sharwell, dotpaul and a team April 16, 2019 20:29
@mavasani mavasani changed the base branch from master to 2.9.x April 16, 2019 20:29
@mavasani
Copy link
Contributor Author

Merging this in as we want to start testing the latest drop from this branch for 2.9.2 release. I'll address any PR feedback with a follow-up change.

operation.Parameter.RefKind == RefKind.Out &&
operation.Parent is IInvocationOperation invocation &&
invocation.TargetMethod.ReturnType.SpecialType == SpecialType.System_Boolean)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't think of anything of the top of my head, but this seems like a heuristic to keep an eye on in the future. Otherwise LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants