Skip instrumentation for external pdb with no local sources

src/coverlet.core/Abstracts/IFileSystem.cs

@@ -0,0 +1,7 @@
+namespace Coverlet.Core.Abstracts
+{
+    internal interface IFileSystem
+    {
+        bool Exists(string path);
+    }
+}

src/coverlet.core/Abstracts/InstrumentationHelper.cs → src/coverlet.core/Abstracts/IInstrumentationHelper.cs

@@ -12,7 +12,8 @@ public interface IInstrumentationHelper
         bool IsTypeExcluded(string module, string type, string[] excludeFilters);
         bool IsTypeIncluded(string module, string type, string[] includeFilters);
         void RestoreOriginalModule(string module, string identifier);
-        bool EmbeddedPortablePdbHasLocalSource(string module);
+        bool EmbeddedPortablePdbHasLocalSource(string module, out string firstNotFoundDocument);
+        bool PortablePdbHasLocalSource(string module, out string firstNotFoundDocument);
         bool IsLocalMethod(string method);
     }
 }

src/coverlet.core/DependencyInjection.cs

@@ -28,6 +28,7 @@ private static IServiceProvider InitDefaultServices()
             IServiceCollection serviceCollection = new ServiceCollection();
             serviceCollection.AddTransient<IRetryHelper, RetryHelper>();
             serviceCollection.AddTransient<IProcessExitHandler, ProcessExitHandler>();
+            serviceCollection.AddTransient<IFileSystem, FileSystem>();
 
             // We need to keep singleton/static semantics
             serviceCollection.AddSingleton<IInstrumentationHelper, InstrumentationHelper>();

src/coverlet.core/Helpers/FileSystem.cs

@@ -0,0 +1,13 @@
+using Coverlet.Core.Abstracts;
+using System.IO;
+
+namespace Coverlet.Core.Helpers
+{
+    internal class FileSystem : IFileSystem
+    {
+        public bool Exists(string path)
+        {
+            return File.Exists(path);
+        }
+    }
+}

src/coverlet.core/Helpers/InstrumentationHelper.cs

@@ -9,18 +9,21 @@
 using System.Reflection.PortableExecutable;
 using System.Text.RegularExpressions;
 using Coverlet.Core.Abstracts;
+using Coverlet.Core.Logging;
 
 namespace Coverlet.Core.Helpers
 {
     internal class InstrumentationHelper : IInstrumentationHelper
     {
         private readonly ConcurrentDictionary<string, string> _backupList = new ConcurrentDictionary<string, string>();
         private readonly IRetryHelper _retryHelper;
+        private readonly IFileSystem _fileSystem;
 
-        public InstrumentationHelper(IProcessExitHandler processExitHandler, IRetryHelper retryHelper)
+        public InstrumentationHelper(IProcessExitHandler processExitHandler, IRetryHelper retryHelper, IFileSystem fileSystem)
         {
             processExitHandler.Add((s, e) => RestoreOriginalModules());
             _retryHelper = retryHelper;
+            _fileSystem = fileSystem;
         }
 
         public string[] GetCoverableModules(string module, string[] directories, bool includeTestAssembly)
@@ -93,8 +96,9 @@ public bool HasPdb(string module, out bool embedded)
             }
         }
 
-        public bool EmbeddedPortablePdbHasLocalSource(string module)
+        public bool EmbeddedPortablePdbHasLocalSource(string module, out string firstNotFoundDocument)
         {
+            firstNotFoundDocument = "";
             using (FileStream moduleStream = File.OpenRead(module))
             using (var peReader = new PEReader(moduleStream))
             {
@@ -115,6 +119,7 @@ public bool EmbeddedPortablePdbHasLocalSource(string module)
                                 // Btw check for all possible extension could be weak approach
                                 if (!File.Exists(docName))
                                 {
+                                    firstNotFoundDocument = docName;
                                     return false;
                                 }
                             }
@@ -128,6 +133,41 @@ public bool EmbeddedPortablePdbHasLocalSource(string module)
             return true;
         }
 
+        public bool PortablePdbHasLocalSource(string module, out string firstNotFoundDocument)
+        {
+            firstNotFoundDocument = "";
+            using (var moduleStream = File.OpenRead(module))
+            using (var peReader = new PEReader(moduleStream))
+            {
+                foreach (var entry in peReader.ReadDebugDirectory())
+                {
+                    if (entry.Type == DebugDirectoryEntryType.CodeView)
+                    {
+                        var codeViewData = peReader.ReadCodeViewDebugDirectoryData(entry);
+                        using FileStream pdbStream = new FileStream(codeViewData.Path, FileMode.Open);
+                        using MetadataReaderProvider metadataReaderProvider = MetadataReaderProvider.FromPortablePdbStream(pdbStream);
+                        MetadataReader metadataReader = metadataReaderProvider.GetMetadataReader();
+                        foreach (DocumentHandle docHandle in metadataReader.Documents)
+                        {
+                            Document document = metadataReader.GetDocument(docHandle);
+                            string docName = metadataReader.GetString(document.Name);
+
+                            // We verify all docs and return false if not all are present in local
+                            // We could have false negative if doc is not a source
+                            // Btw check for all possible extension could be weak approach
+                            if (!_fileSystem.Exists(docName))
+                            {
+                                firstNotFoundDocument = docName;
+                                return false;
+                            }
+                        }
+                    }
+                }
+            }
+
+            return true;
+        }
+
         public void BackupOriginalModule(string module, string identifier)
         {
             var backupPath = GetBackupPath(module, identifier);

src/coverlet.core/Instrumentation/Instrumenter.cs

@@ -40,13 +40,13 @@ internal class Instrumenter
         private List<string> _excludedSourceFiles;
 
         public Instrumenter(
-            string module, 
-            string identifier, 
-            string[] excludeFilters, 
-            string[] includeFilters, 
-            string[] excludedFiles, 
-            string[] excludedAttributes, 
-            bool singleHit, 
+            string module,
+            string identifier,
+            string[] excludeFilters,
+            string[] includeFilters,
+            string[] excludedFiles,
+            string[] excludedAttributes,
+            bool singleHit,
             ILogger logger,
             IInstrumentationHelper instrumentationHelper)
         {
@@ -70,19 +70,27 @@ public bool CanInstrument()
                 {
                     if (embeddedPdb)
                     {
-                        if (_instrumentationHelper.EmbeddedPortablePdbHasLocalSource(_module))
+                        if (_instrumentationHelper.EmbeddedPortablePdbHasLocalSource(_module, out string firstNotFoundDocument))
                         {
                             return true;
                         }
                         else
                         {
-                            _logger.LogWarning($"Unable to instrument module: {_module}, embedded pdb without local source files");
+                            _logger.LogWarning($"Unable to instrument module: {_module}, embedded pdb without local source files, [{firstNotFoundDocument}]");
                             return false;
                         }
                     }
                     else
                     {
-                        return true;
+                        if (_instrumentationHelper.PortablePdbHasLocalSource(_module, out string firstNotFoundDocument))
+                        {
+                            return true;
+                        }
+                        else
+                        {
+                            _logger.LogWarning($"Unable to instrument module: {_module}, pdb without local source files, [{firstNotFoundDocument}]");
+                            return false;
+                        }
                     }
                 }
                 else

src/coverlet.core/Properties/AssemblyInfo.cs

@@ -4,4 +4,6 @@
 "2e5bb58a827a3c46c2f959318327ad30d1b52e918321ffbd847fb21565b8576d2a3a24562a93e8" +
 "6c77a298b564a0f1b98f63d7a1441a3a8bcc206da3ed09d5dacc76e122a109a9d3ac608e21a054" +
 "d667a2bae98510a1f0f653c0e6f58f42b4b3934f6012f5ec4a09b3dfd3e14d437ede1424bdb722" +
-"aead64ad")]
+"aead64ad")]
+// Needed to mock internal type https://github.com/Moq/moq4/wiki/Quickstart#advanced-features
+[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]

test/coverlet.core.tests/CoverageTests.cs

@@ -14,7 +14,7 @@ namespace Coverlet.Core.Tests
 {
     public class CoverageTests
     {
-        private readonly InstrumentationHelper _instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper());
+        private readonly InstrumentationHelper _instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), new FileSystem());
         private readonly Mock<ILogger> _mockLogger = new Mock<ILogger>();
 
         [Fact]

test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs

@@ -8,7 +8,7 @@ namespace Coverlet.Core.Helpers.Tests
 {
     public class InstrumentationHelperTests
     {
-        private readonly InstrumentationHelper _instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper());
+        private readonly InstrumentationHelper _instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), new FileSystem());
 
         [Fact]
         public void TestGetDependencies()

test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs

@@ -5,6 +5,7 @@
 using System.Reflection;
 using System.Runtime.InteropServices;
 
+using Coverlet.Core.Abstracts;
 using Coverlet.Core.Helpers;
 using Coverlet.Core.Logging;
 using Coverlet.Core.Samples.Tests;
@@ -20,7 +21,7 @@ namespace Coverlet.Core.Instrumentation.Tests
 {
     public class InstrumenterTests
     {
-        private readonly InstrumentationHelper _instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper());
+        private readonly InstrumentationHelper _instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), new FileSystem());
         private readonly Mock<ILogger> _mockLogger = new Mock<ILogger>();
 
         [Fact(Skip = "To be used only validating System.Private.CoreLib instrumentation")]
@@ -363,6 +364,25 @@ public void SkipEmbeddedPpdbWithoutLocalSource()
             loggerMock.VerifyNoOtherCalls();
         }
 
+        [Fact]
+        public void SkipPpdbWithoutLocalSource()
+        {
+            Mock<IFileSystem> mockFileSystem = new Mock<IFileSystem>();
+            mockFileSystem.Setup(fs => fs.Exists(It.IsAny<string>())).Returns((string path) =>
+             {
+                 return Path.GetExtension(path) != ".cs";
+             });
+            InstrumentationHelper instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), mockFileSystem.Object);
+
+            string coverletLib = Directory.GetFiles(Directory.GetCurrentDirectory(), "coverlet.core.dll").First();
+            var loggerMock = new Mock<ILogger>();
+            Instrumenter instrumenter = new Instrumenter(coverletLib, "_corelib_instrumented", Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), false, loggerMock.Object, instrumentationHelper);
+            Assert.True(_instrumentationHelper.HasPdb(coverletLib, out bool embedded));
+            Assert.False(embedded);
+            Assert.False(instrumenter.CanInstrument());
+            loggerMock.Verify(l => l.LogWarning(It.IsAny<string>()));
+        }
+
         [Fact]
         public void TestInstrument_MissingModule()
         {

test/coverlet.core.tests/InstrumenterHelper.cs

@@ -191,7 +191,7 @@ public static CoverageResult GetCoverageResult(string filePath)
             using (var result = new FileStream(filePath, FileMode.Open))
             {
                 CoveragePrepareResult coveragePrepareResultLoaded = CoveragePrepareResult.Deserialize(result);
-                Coverage coverage = new Coverage(coveragePrepareResultLoaded, new Mock<ILogger>().Object, new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper()));
+                Coverage coverage = new Coverage(coveragePrepareResultLoaded, new Mock<ILogger>().Object, new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), new FileSystem()));
                 return coverage.GetCoverageResult();
             }
         }
@@ -202,6 +202,7 @@ async public static Task<CoveragePrepareResult> Run<T>(Func<dynamic, Task> callM
             DependencyInjection.Set(new ServiceCollection()
             .AddTransient<IRetryHelper, CustomRetryHelper>()
             .AddTransient<IProcessExitHandler, CustomProcessExitHandler>()
+            .AddTransient<IFileSystem, FileSystem>()
             .AddSingleton<IInstrumentationHelper, InstrumentationHelper>()
             .BuildServiceProvider());