Skip to content

Commit 4b7088a

Browse files
JohanLarssonJohanLarsson
committed
BUGFIX: IDISP004 when returning wrapped disposable.
Fix #200
1 parent 853ed8e commit 4b7088a

7 files changed

Lines changed: 113 additions & 20 deletions

File tree

‎IDisposableAnalyzers.Test/Helpers/Disposable.Ignores.cs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,55 @@ public void Dispose()
380380
Assert.AreEqual(false, Disposable.Ignores(value, semanticModel, CancellationToken.None));
381381
}
382382

383+
[TestCase("new StreamReader(File.OpenRead(string.Empty))")]
384+
[TestCase("File.OpenRead(string.Empty).M2()")]
385+
[TestCase("File.OpenRead(string.Empty)?.M2()")]
386+
[TestCase("M2(File.OpenRead(string.Empty))")]
387+
public static void ReturnedStreamWrappedInStreamReader(string expression)
388+
{
389+
var code = @"
390+
namespace N
391+
{
392+
using System.IO;
393+
394+
public static class C
395+
{
396+
public StreamReader M1() => File.OpenRead(string.Empty).M2();
397+
398+
private static StreamReader M2(this Stream stream) => new StreamReader(stream);
399+
}
400+
}".AssertReplace("File.OpenRead(string.Empty).M2()", expression);
401+
var syntaxTree = CSharpSyntaxTree.ParseText(code);
402+
var compilation = CSharpCompilation.Create("test", new[] { syntaxTree }, MetadataReferences.FromAttributes());
403+
var semanticModel = compilation.GetSemanticModel(syntaxTree);
404+
var value = syntaxTree.FindExpression("File.OpenRead(string.Empty)");
405+
Assert.AreEqual(false, Disposable.Ignores(value, semanticModel, CancellationToken.None));
406+
}
407+
408+
[TestCase("await File.OpenRead(string.Empty).ReadAsync(null, 0, 0)")]
409+
[TestCase("await File.OpenRead(string.Empty)?.ReadAsync(null, 0, 0)")]
410+
[TestCase("File.OpenRead(string.Empty).ReadAsync(null, 0, 0)")]
411+
[TestCase("File.OpenRead(string.Empty)?.ReadAsync(null, 0, 0)")]
412+
public static void FileOpenReadReadAsync(string expression)
413+
{
414+
var code = @"
415+
namespace N
416+
{
417+
using System.IO;
418+
using System.Threading.Tasks;
419+
420+
public class C
421+
{
422+
public async Task<int> M() => await File.OpenRead(string.Empty).ReadAsync(null, 0, 0);
423+
}
424+
}".AssertReplace("await File.OpenRead(string.Empty).ReadAsync(null, 0, 0)", expression);
425+
var syntaxTree = CSharpSyntaxTree.ParseText(code);
426+
var compilation = CSharpCompilation.Create("test", new[] { syntaxTree }, MetadataReferences.FromAttributes());
427+
var semanticModel = compilation.GetSemanticModel(syntaxTree);
428+
var value = syntaxTree.FindExpression("File.OpenRead(string.Empty)");
429+
Assert.AreEqual(true, Disposable.Ignores(value, semanticModel, CancellationToken.None));
430+
}
431+
383432
[TestCase("new CompositeDisposable(File.OpenRead(fileName))")]
384433
[TestCase("new CompositeDisposable { File.OpenRead(fileName) }")]
385434
public static void ReturnedInCompositeDisposable(string expression)

‎IDisposableAnalyzers.Test/IDISP004DoNotIgnoreCreatedTests/Valid.ExtensionMethod.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,24 @@ public void Dispose()
223223
RoslynAssert.Valid(Analyzer, code, extCode, DisposableCode, wrappingDisposableCode);
224224
}
225225

226+
[Test]
227+
public static void ExtensionMethodWrappingStreamInStreamReader()
228+
{
229+
var code = @"
230+
namespace N
231+
{
232+
using System.IO;
233+
234+
public static class C
235+
{
236+
public static StreamReader M1() => File.OpenRead(string.Empty).M2();
237+
238+
private static StreamReader M2(this Stream stream) => new StreamReader(stream);
239+
}
240+
}";
241+
RoslynAssert.Valid(Analyzer, code);
242+
}
243+
226244
[Test]
227245
public static void Issue174()
228246
{

‎IDisposableAnalyzers/CodeFixes/DisposeMemberFix.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ void DisposeWhenNoParameter(DocumentEditor editor, CancellationToken cancellatio
131131

132132
break;
133133

134-
case { Identifier: { ValueText: "Dispose" }, ParameterList: { Parameters: { Count: 0 } }, ExpressionBody: { Expression:{} expression } }:
134+
case { Identifier: { ValueText: "Dispose" }, ParameterList: { Parameters: { Count: 0 } }, ExpressionBody: { Expression: { } expression } }:
135135
context.RegisterCodeFix(
136136
$"{symbol.Name}.Dispose() in {method}",
137137
(editor, cancellationToken) => editor.ReplaceNode(

‎IDisposableAnalyzers/Helpers/Disposable.Disposes.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,10 @@ when invocation.IsSymbol(KnownSymbol.SystemWindowsFormsControl.Show, recursion.S
156156
=> true, // disposed by form.Close()
157157
{ Parent: MemberAccessExpressionSyntax { Parent: InvocationExpressionSyntax invocation } }
158158
=> IsDisposeOrReturnValueDisposed(invocation),
159-
{ Parent: ConditionalAccessExpressionSyntax { } parent }
159+
{ Parent: ConditionalAccessExpressionSyntax parent }
160160
=> DisposedByReturnValue(parent, recursion, out var creation) &&
161161
Disposes(creation, recursion),
162-
{ Parent: MemberAccessExpressionSyntax { } parent }
162+
{ Parent: MemberAccessExpressionSyntax parent }
163163
=> DisposedByReturnValue(parent, recursion, out var creation) &&
164164
Disposes(creation, recursion),
165165
{ Parent: AssignmentExpressionSyntax { Left: { } left } assignment }

‎IDisposableAnalyzers/Helpers/Disposable.Ignores.cs‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,36 @@ internal static bool Ignores(ExpressionSyntax candidate, SemanticModel semanticM
2323

2424
private static bool Ignores(ExpressionSyntax candidate, Recursion recursion)
2525
{
26-
if (Disposes(candidate, recursion) ||
27-
Assigns(candidate, recursion, out _) ||
28-
Stores(candidate, recursion, out _) ||
29-
Returns(candidate, recursion))
26+
using (var temp = Recursion.Borrow(recursion.ContainingType, recursion.SemanticModel, recursion.CancellationToken))
3027
{
31-
return false;
28+
if (Disposes(candidate, temp))
29+
{
30+
return false;
31+
}
32+
}
33+
34+
using (var temp = Recursion.Borrow(recursion.ContainingType, recursion.SemanticModel, recursion.CancellationToken))
35+
{
36+
if (Assigns(candidate, temp, out _))
37+
{
38+
return false;
39+
}
40+
}
41+
42+
using (var temp = Recursion.Borrow(recursion.ContainingType, recursion.SemanticModel, recursion.CancellationToken))
43+
{
44+
if (Stores(candidate, temp, out _))
45+
{
46+
return false;
47+
}
48+
}
49+
50+
using (var temp = Recursion.Borrow(recursion.ContainingType, recursion.SemanticModel, recursion.CancellationToken))
51+
{
52+
if (Returns(candidate, temp))
53+
{
54+
return false;
55+
}
3256
}
3357

3458
return candidate switch

‎IDisposableAnalyzers/Helpers/Disposable.Returns.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ private static bool Returns(ExpressionSyntax candidate, Recursion recursion)
5151
=> true,
5252
{ Parent: ArrowExpressionClauseSyntax { Parent: { } parent } }
5353
=> !parent.IsKind(SyntaxKind.ConstructorDeclaration),
54+
{ Parent: MemberAccessExpressionSyntax { Parent: InvocationExpressionSyntax invocation } }
55+
=> DisposedByReturnValue(invocation, recursion, out _) &&
56+
Returns(invocation, recursion),
57+
{ Parent: ConditionalAccessExpressionSyntax { WhenNotNull: InvocationExpressionSyntax invocation } conditionalAccess }
58+
=> DisposedByReturnValue(invocation, recursion, out _) &&
59+
Returns(conditionalAccess, recursion),
5460
{ Parent: EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax variableDeclarator } }
5561
=> recursion.Target(variableDeclarator) is { } target &&
5662
Returns(target, recursion),

‎IDisposableAnalyzers/Helpers/Disposable.cs‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,19 +48,15 @@ internal static bool IsPotentiallyAssignableFrom(ITypeSymbol type, Compilation c
4848

4949
internal static bool IsAssignableFrom(ITypeSymbol type, Compilation compilation)
5050
{
51-
if (type is null)
51+
return type switch
5252
{
53-
return false;
54-
}
55-
56-
// https://blogs.msdn.microsoft.com/pfxteam/2012/03/25/do-i-need-to-dispose-of-tasks/
57-
if (type == KnownSymbol.Task)
58-
{
59-
return false;
60-
}
61-
62-
return type == KnownSymbol.IDisposable ||
63-
type.IsAssignableTo(KnownSymbol.IDisposable, compilation);
53+
null => false,
54+
//// https://blogs.msdn.microsoft.com/pfxteam/2012/03/25/do-i-need-to-dispose-of-tasks/
55+
{ ContainingNamespace: { MetadataName: "Tasks", ContainingNamespace: { MetadataName: "Threading", ContainingNamespace: { MetadataName: "System" } } }, MetadataName: "Task" } => false,
56+
INamedTypeSymbol { ContainingNamespace: { MetadataName: "Tasks", ContainingNamespace: { MetadataName: "Threading", ContainingNamespace: { MetadataName: "System" } } }, MetadataName: "Task`1", TypeArguments: { Length: 1 } arguments }
57+
=> IsAssignableFrom(arguments[0], compilation),
58+
_ => type.IsAssignableTo(KnownSymbol.IDisposable, compilation),
59+
};
6460
}
6561

6662
internal static bool IsNop(ExpressionSyntax candidate, SemanticModel semanticModel, CancellationToken cancellationToken)

0 commit comments

Comments
 (0)