Fix and Update S3966: Rule should not throw cast exception + detect D…

…ispose on this (#592)
SonarSource · Jul 24, 2017 · 7e9108f · 7e9108f
1 parent 7a9ae39
commit 7e9108f
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 7 deletions.
diff --git a/...ranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/ObjectsShouldNotBeDisposedMoreThanOnce.cs b/...ranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/ObjectsShouldNotBeDisposedMoreThanOnce.cs
@@ -144,10 +144,21 @@ private ProgramState VisitInvocationExpression(InvocationExpressionSyntax instru
                 var disposeMethodSymbol = semanticModel.GetSymbolInfo(instruction).Symbol as IMethodSymbol;
                 if (disposeMethodSymbol.IsIDisposableDispose())
                 {
-                    var disposable = ((MemberAccessExpressionSyntax)instruction.Expression).Expression;
-                    var disposableSymbol = semanticModel.GetSymbolInfo(disposable).Symbol;
-
-                    newProgramState = ProcessDisposableSymbol(newProgramState, disposable, disposableSymbol);
+                    var disposedObject =
+                        // Direct call to Dispose()
+                        instruction.Expression as IdentifierNameSyntax
+                        // Call to Dispose on local variable, field or this
+                        ?? (instruction.Expression as MemberAccessExpressionSyntax)?.Expression;
+                    if (disposedObject != null)
+                    {
+                        var disposableSymbol = semanticModel.GetSymbolInfo(disposedObject).Symbol;
+                        if (disposableSymbol is IMethodSymbol ||
+                            disposableSymbol is IParameterSymbol)
+                        {
+                            disposableSymbol = disposableSymbol.ContainingType;
+                        }
+                        newProgramState = ProcessDisposableSymbol(newProgramState, disposedObject, disposableSymbol);
+                    }
                 }
 
                 return newProgramState;
@@ -172,7 +183,8 @@ private ProgramState ProcessStreamDisposingTypes(ProgramState programState, Synt
                 return newProgramState;
             }
 
-            private ProgramState ProcessDisposableSymbol(ProgramState programState, SyntaxNode instruction, ISymbol disposableSymbol)
+            private ProgramState ProcessDisposableSymbol(ProgramState programState, SyntaxNode disposeInstruction,
+                ISymbol disposableSymbol)
             {
                 if (disposableSymbol == null) // DisposableSymbol is null when we invoke an array element
                 {
@@ -181,7 +193,8 @@ private ProgramState ProcessDisposableSymbol(ProgramState programState, SyntaxNo
 
                 if (disposableSymbol.HasConstraint(DisposableConstraint.Disposed, programState))
                 {
-                    ObjectDisposed?.Invoke(this, new ObjectDisposedEventArgs(disposableSymbol.Name, instruction.GetLocation()));
+                    ObjectDisposed?.Invoke(this, new ObjectDisposedEventArgs(disposableSymbol.Name,
+                        disposeInstruction.GetLocation()));
                     return programState;
                 }
 
@@ -192,7 +205,15 @@ private ProgramState ProcessDisposableSymbol(ProgramState programState, SyntaxNo
                     return programState;
                 }
 
-                return disposableSymbol.SetConstraint(DisposableConstraint.Disposed, programState);
+                var newProgramState = programState;
+                if (disposableSymbol is INamedTypeSymbol &&
+                    newProgramState.GetSymbolValue(disposableSymbol) == null)
+                {
+                    // Dispose is called on current instance but we don't usually store a symbol for this
+                    // so we store it and then associate the Disposed constraint.
+                    newProgramState = newProgramState.StoreSymbolicValue(disposableSymbol, SymbolicValue.This);
+                }
+                return disposableSymbol.SetConstraint(DisposableConstraint.Disposed, newProgramState);
             }
 
             private static ArgumentSyntax FirstArgumentOrDefault(ObjectCreationExpressionSyntax objectCreation) =>

diff --git a/...tnet/src/Tests/SonarAnalyzer.UnitTest/TestCases/ObjectsShouldNotBeDisposedMoreThanOnce.cs b/...tnet/src/Tests/SonarAnalyzer.UnitTest/TestCases/ObjectsShouldNotBeDisposedMoreThanOnce.cs
@@ -115,4 +115,21 @@ public class Disposable : IDisposable
     {
         public void Dispose() { }
     }
+
+    public class MyClass : IDisposable
+    {
+        public void Dispose() { }
+
+        public void DisposeMultipleTimes()
+        {
+            Dispose();
+            this.Dispose(); // Noncompliant
+            Dispose(); // Noncompliant
+        }
+
+        public void DoSomething()
+        {
+            Dispose();
+        }
+    }
 }