IDISP003 should not warn when assigning out parameter in if return. Fix

#124
DotNetAnalyzers · Nov 4, 2018 · 8820ebe · 8820ebe
1 parent e27558b
commit 8820ebe
Show file tree

Hide file tree

Showing 5 changed files with 118 additions and 44 deletions.
diff --git a/.paket/Paket.Restore.targets b/.paket/Paket.Restore.targets
@@ -11,20 +11,26 @@
     <PaketRootPath>$(MSBuildThisFileDirectory)..\</PaketRootPath>
     <PaketRestoreCacheFile>$(PaketRootPath)paket-files\paket.restore.cached</PaketRestoreCacheFile>
     <PaketLockFilePath>$(PaketRootPath)paket.lock</PaketLockFilePath>
+    <PaketBootstrapperStyle>classic</PaketBootstrapperStyle>
+    <PaketBootstrapperStyle Condition="Exists('$(PaketToolsPath)paket.bootstrapper.proj')">proj</PaketBootstrapperStyle>
+    <PaketExeImage>assembly</PaketExeImage>
+    <PaketExeImage Condition=" '$(PaketBootstrapperStyle)' == 'proj' ">native</PaketExeImage>
     <MonoPath Condition="'$(MonoPath)' == '' And Exists('/Library/Frameworks/Mono.framework/Commands/mono')">/Library/Frameworks/Mono.framework/Commands/mono</MonoPath>
     <MonoPath Condition="'$(MonoPath)' == ''">mono</MonoPath>
     <!-- Paket command -->
+    <PaketExePath Condition=" '$(PaketExePath)' == '' AND Exists('$(PaketToolsPath)paket')">$(PaketToolsPath)paket</PaketExePath>
     <PaketExePath Condition=" '$(PaketExePath)' == '' AND Exists('$(PaketRootPath)paket.exe')">$(PaketRootPath)paket.exe</PaketExePath>
-    <PaketExePath Condition=" '$(PaketExePath)' == '' ">$(PaketToolsPath)paket.exe</PaketExePath>
-    <PaketCommand Condition=" '$(OS)' == 'Windows_NT'">"$(PaketExePath)"</PaketCommand>
-    <PaketCommand Condition=" '$(OS)' != 'Windows_NT' ">$(MonoPath) --runtime=v4.0.30319 "$(PaketExePath)"</PaketCommand>
 
-    <!-- .net core fdd -->
-    <_PaketExeExtension>$([System.IO.Path]::GetExtension("$(PaketExePath)"))</_PaketExeExtension>
-    <PaketCommand Condition=" '$(_PaketExeExtension)' == '.dll' ">dotnet "$(PaketExePath)"</PaketCommand>
+    <PaketExePath Condition=" '$(PaketExePath)' == '' AND '$(OS)' == 'Windows_NT' ">$(PaketToolsPath)paket.exe</PaketExePath>
+    <PaketExePath Condition=" '$(PaketExePath)' == '' AND '$(OS)' != 'Windows_NT' AND '$(PaketExeImage)' == 'assembly' ">$(PaketToolsPath)paket.exe</PaketExePath>
+    <PaketExePath Condition=" '$(PaketExePath)' == '' AND '$(OS)' != 'Windows_NT' AND '$(PaketExeImage)' == 'native' ">$(PaketToolsPath)paket</PaketExePath>
 
-    <!-- no extension is a shell script -->
-    <PaketCommand Condition=" '$(_PaketExeExtension)' == '' ">"$(PaketExePath)"</PaketCommand>
+    <!-- Paket command -->
+    <_PaketExeExtension>$([System.IO.Path]::GetExtension("$(PaketExePath)"))</_PaketExeExtension>
+    <PaketCommand Condition=" '$(PaketCommand)' == '' AND '$(_PaketExeExtension)' == '.dll' ">dotnet "$(PaketExePath)"</PaketCommand>
+    <PaketCommand Condition=" '$(PaketCommand)' == '' AND '$(OS)' == 'Windows_NT'">"$(PaketExePath)"</PaketCommand>
+    <PaketCommand Condition=" '$(PaketCommand)' == '' AND '$(OS)' != 'Windows_NT' AND '$(_PaketExeExtension)' == '.exe' ">$(MonoPath) --runtime=v4.0.30319 "$(PaketExePath)"</PaketCommand>
+    <PaketCommand Condition=" '$(PaketCommand)' == '' AND '$(OS)' != 'Windows_NT'">"$(PaketExePath)"</PaketCommand>
 
     <PaketBootStrapperExePath Condition=" '$(PaketBootStrapperExePath)' == '' AND Exists('$(PaketRootPath)paket.bootstrapper.exe')">$(PaketRootPath)paket.bootstrapper.exe</PaketBootStrapperExePath>
     <PaketBootStrapperExePath Condition=" '$(PaketBootStrapperExePath)' == '' ">$(PaketToolsPath)paket.bootstrapper.exe</PaketBootStrapperExePath>
@@ -38,7 +44,11 @@
     <DisableImplicitSystemValueTupleReference>true</DisableImplicitSystemValueTupleReference>
   </PropertyGroup>
 
-  <Target Name="PaketRestore" Condition="'$(PaketRestoreDisabled)' != 'True'" BeforeTargets="_GenerateDotnetCliToolReferenceSpecs;_GenerateProjectRestoreGraphPerFramework;_GenerateRestoreGraphWalkPerFramework;CollectPackageReferences" >
+  <Target Name="PaketBootstrapping" Condition="Exists('$(PaketToolsPath)paket.bootstrapper.proj')">
+    <MSBuild Projects="$(PaketToolsPath)paket.bootstrapper.proj" Targets="Restore" />
+  </Target>
+
+  <Target Name="PaketRestore" Condition="'$(PaketRestoreDisabled)' != 'True'" BeforeTargets="_GenerateDotnetCliToolReferenceSpecs;_GenerateProjectRestoreGraphPerFramework;_GenerateRestoreGraphWalkPerFramework;CollectPackageReferences" DependsOnTargets="PaketBootstrapping">
 
     <!-- Step 1 Check if lockfile is properly restored -->
     <PropertyGroup>
@@ -77,7 +87,7 @@
     </PropertyGroup>
 
     <!-- Do a global restore if required -->
-    <Exec Command='$(PaketBootStrapperCommand)' Condition="Exists('$(PaketBootStrapperExePath)') AND !(Exists('$(PaketExePath)'))" ContinueOnError="false" />
+    <Exec Command='$(PaketBootStrapperCommand)' Condition=" '$(PaketBootstrapperStyle)' == 'classic' AND Exists('$(PaketBootStrapperExePath)') AND !(Exists('$(PaketExePath)'))" ContinueOnError="false" />
     <Exec Command='$(PaketCommand) restore' Condition=" '$(PaketRestoreRequired)' == 'true' " ContinueOnError="false" />
 
     <!-- Step 2 Detect project specific changes -->

diff --git a/IDisposableAnalyzers.Test/IDISP003DisposeBeforeReassigningTests/ValidCode.RefAndOut.cs b/IDisposableAnalyzers.Test/IDISP003DisposeBeforeReassigningTests/ValidCode.RefAndOut.cs
@@ -271,6 +271,64 @@ private static bool TryGetStream(out Stream stream)
             return true;
         }
     }
+}";
+            AnalyzerAssert.Valid(Analyzer, DisposableCode, testCode);
+        }
+
+        [Test]
+        public void AssigningReturnOut()
+        {
+            var testCode = @"
+namespace RoslynSandbox
+{
+    using System.IO;
+
+    class Foo
+    {
+        private static bool TryGetStream(string fileName, out Stream result)
+        {
+            if (File.Exists(fileName))
+            {
+                result = File.OpenRead(fileName);
+                return true;
+            }
+
+            result = null;
+            return false;
+        }
+    }
+}";
+            AnalyzerAssert.Valid(Analyzer, DisposableCode, testCode);
+        }
+
+        [Test]
+        public void AssigningReturnOutTwice()
+        {
+            var testCode = @"
+namespace RoslynSandbox
+{
+    using System.IO;
+
+    class Foo
+    {
+        private static bool TryGetStream(string fileName, out Stream result)
+        {
+            if (File.Exists(fileName))
+            {
+                result = File.OpenRead(fileName);
+                return true;
+            }
+
+            if (File.Exists(fileName))
+            {
+                result = File.OpenRead(fileName);
+                return true;
+            }
+
+            result = null;
+            return false;
+        }
+    }
 }";
             AnalyzerAssert.Valid(Analyzer, DisposableCode, testCode);
         }

diff --git a/IDisposableAnalyzers/Helpers/Disposable.IsCreation.cs b/IDisposableAnalyzers/Helpers/Disposable.IsCreation.cs
@@ -103,8 +103,9 @@ parenthesizedExpression.Parent is BinaryExpressionSyntax binary &&
             bool IsReturnedBefore(ExpressionSyntax expression)
             {
                 return expression.TryFirstAncestor(out BlockSyntax block) &&
-                       block.Statements.TryFirstOfType(out ReturnStatementSyntax returnStatement) &&
-                       disposable.IsExecutedBefore(returnStatement) == ExecutedBefore.No;
+                       block.Statements.TryFirstOfType(out ReturnStatementSyntax _) &&
+                       !block.Contains(disposable) &&
+                       block.SharesAncestor(disposable, out MemberDeclarationSyntax _);
             }
         }
 

diff --git a/ValidCode/OutParameter.cs b/ValidCode/OutParameter.cs
@@ -33,5 +33,23 @@ private static bool TryGetStreamCore(out Stream stream)
             stream = File.OpenRead(string.Empty);
             return true;
         }
+
+        private static bool TryGetStream(string fileName, out Stream result)
+        {
+            if (File.Exists(fileName))
+            {
+                result = File.OpenRead(fileName);
+                return true;
+            }
+
+            if (File.Exists(fileName))
+            {
+                result = File.OpenRead(fileName);
+                return true;
+            }
+
+            result = null;
+            return false;
+        }
     }
 }