changes per feedback

hunterjm · hunterjm · commit 85fa0f35ebb9 · 2018-05-05T12:51:30.000-04:00
diff --git a/src/coverlet.core/CoverageDetails.cs b/src/coverlet.core/CoverageDetails.cs
@@ -0,0 +1,14 @@
+using System;
+
+namespace Coverlet.Core
+{
+    public class CoverageDetails
+    {
+        public double Covered { get; internal set; }
+        public int Total { get; internal set; }
+        public double Percent
+        {
+            get => Math.Round(Total == 0 ? Total : Covered / Total, 3);
+        }
+    }
+}
diff --git a/src/coverlet.core/CoverageSummary.cs b/src/coverlet.core/CoverageSummary.cs
@@ -4,21 +4,13 @@
 
 namespace Coverlet.Core
 {
-    public class CoverageDetails
-    {
-        public double Covered { get; set; }
-        public int Total { get; set; }
-        public double Percent { get; set; }
-    }
     public class CoverageSummary
     {
         public CoverageDetails CalculateLineCoverage(Lines lines)
         {
             var details = new CoverageDetails();
             details.Covered = lines.Where(l => l.Value.Hits > 0).Count();
             details.Total = lines.Count;
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -31,9 +23,6 @@ public CoverageDetails CalculateLineCoverage(Methods methods)
                 details.Covered += methodCoverage.Covered;
                 details.Total += methodCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -46,9 +35,6 @@ public CoverageDetails CalculateLineCoverage(Classes classes)
                 details.Covered += classCoverage.Covered;
                 details.Total += classCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -61,9 +47,6 @@ public CoverageDetails CalculateLineCoverage(Documents documents)
                 details.Covered += documentCoverage.Covered;
                 details.Total += documentCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -76,9 +59,6 @@ public CoverageDetails CalculateLineCoverage(Modules modules)
                 details.Covered += moduleCoverage.Covered;
                 details.Total += moduleCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -87,8 +67,6 @@ public CoverageDetails CalculateBranchCoverage(List<BranchInfo> branchInfo)
             var details = new CoverageDetails();
             details.Covered = branchInfo.Count(bi => bi.Hits > 0);
             details.Total = branchInfo.Count;
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -97,8 +75,6 @@ public CoverageDetails CalculateBranchCoverage(Branches branches)
             var details = new CoverageDetails();
             details.Covered = branches.Sum(b => b.Value.Where(bi => bi.Hits > 0).Count());
             details.Total = branches.Sum(b => b.Value.Count());
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -111,9 +87,6 @@ public CoverageDetails CalculateBranchCoverage(Methods methods)
                 details.Covered += methodCoverage.Covered;
                 details.Total += methodCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -126,9 +99,6 @@ public CoverageDetails CalculateBranchCoverage(Classes classes)
                 details.Covered += classCoverage.Covered;
                 details.Total += classCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -141,9 +111,6 @@ public CoverageDetails CalculateBranchCoverage(Documents documents)
                 details.Covered += documentCoverage.Covered;
                 details.Total += documentCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -156,9 +123,6 @@ public CoverageDetails CalculateBranchCoverage(Modules modules)
                 details.Covered += moduleCoverage.Covered;
                 details.Total += moduleCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -167,7 +131,6 @@ public CoverageDetails CalculateMethodCoverage(Lines lines)
             var details = new CoverageDetails();
             details.Covered = lines.Any(l => l.Value.Hits > 0) ? 1 : 0;
             details.Total = 1;
-            details.Percent = details.Covered;
             return details;
         }
 
@@ -181,9 +144,6 @@ public CoverageDetails CalculateMethodCoverage(Methods methods)
                 details.Covered += methodCoverage.Covered;
             }
             details.Total = methodsWithLines.Count();
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -196,9 +156,6 @@ public CoverageDetails CalculateMethodCoverage(Classes classes)
                 details.Covered += classCoverage.Covered;
                 details.Total += classCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -211,9 +168,6 @@ public CoverageDetails CalculateMethodCoverage(Documents documents)
                 details.Covered += documentCoverage.Covered;
                 details.Total += documentCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
 
@@ -226,9 +180,6 @@ public CoverageDetails CalculateMethodCoverage(Modules modules)
                 details.Covered += moduleCoverage.Covered;
                 details.Total += moduleCoverage.Total;
             }
-
-            double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;
-            details.Percent = Math.Round(coverage, 3);
             return details;
         }
     }
diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs
@@ -126,28 +126,25 @@ private void InstrumentIL(MethodDefinition method)
                     index += 3;
                 }
 
-                if (targetedBranchPoints.Count() > 0)
+                foreach (var _branchTarget in targetedBranchPoints)
                 {
-                    foreach (var _branchTarget in targetedBranchPoints)
-                    {
-                        /*
-                         * Skip branches with no sequence point reference for now.
-                         * In this case for an anonymous class the compiler will dynamically create an Equals 'utility' method.
-                         * The CecilSymbolHelper will create branch points with a start line of -1 and no document, which
-                         * I am currently not sure how to handle.
-                         */
-                        if (_branchTarget.StartLine == -1 || _branchTarget.Document == null)
-                            continue;
-                        
-                        var target = AddInstrumentationCode(method, processor, instruction, _branchTarget);
-                        foreach (var _instruction in processor.Body.Instructions)
-                            ReplaceInstructionTarget(_instruction, instruction, target);
-
-                        foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers)
-                            ReplaceExceptionHandlerBoundary(handler, instruction, target);
-
-                        index += 3;
-                    }
+                    /*
+                        * Skip branches with no sequence point reference for now.
+                        * In this case for an anonymous class the compiler will dynamically create an Equals 'utility' method.
+                        * The CecilSymbolHelper will create branch points with a start line of -1 and no document, which
+                        * I am currently not sure how to handle.
+                        */
+                    if (_branchTarget.StartLine == -1 || _branchTarget.Document == null)
+                        continue;
+                    
+                    var target = AddInstrumentationCode(method, processor, instruction, _branchTarget);
+                    foreach (var _instruction in processor.Body.Instructions)
+                        ReplaceInstructionTarget(_instruction, instruction, target);
+
+                    foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers)
+                        ReplaceExceptionHandlerBoundary(handler, instruction, target);
+
+                    index += 3;
                 }
 
                 index++;
@@ -171,7 +168,6 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
                     document.Lines.Add(new Line { Number = i, Class = method.DeclaringType.FullName, Method = method.FullName });
             }
 
-            // string flag = branchPoints.Count > 0 ? "B" : "L";
             string marker = $"L,{document.Path},{sequencePoint.StartLine},{sequencePoint.EndLine}";
 
             var pathInstr = Instruction.Create(OpCodes.Ldstr, _result.HitsFilePath);
diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs
@@ -19,6 +19,7 @@ public static class CecilSymbolHelper
     {
         private const int StepOverLineCode = 0xFEEFEE;
         private static readonly Regex IsMovenext = new Regex(@"\<[^\s>]+\>\w__\w(\w)?::MoveNext\(\)$", RegexOptions.Compiled | RegexOptions.ExplicitCapture);
+
         public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
         {
             var list = new List<BranchPoint>();
@@ -56,7 +57,7 @@ private static void GetBranchPoints(MethodDefinition methodDefinition, List<Bran
                     var branchingInstructionLine = closestSeqPt.Maybe(x => x.StartLine, -1);
                     var document = closestSeqPt.Maybe(x => x.Document.Url);
 
-                    if (null == instruction.Next)
+                    if (instruction.Next == null)
                         return;
 
                     if (!BuildPointsForConditionalBranch(list, instruction, branchingInstructionLine, document, branchOffset, pathCounter, instructions, ref ordinal, methodDefinition)) 

Original file line number	Diff line number	Diff line change
`@@ -4,21 +4,13 @@`
`4`	`4`
`5`	`5`	`namespace Coverlet.Core`
`6`	`6`	`{`
`7`		`- public class CoverageDetails`
`8`		`- {`
`9`		`- public double Covered { get; set; }`
`10`		`- public int Total { get; set; }`
`11`		`- public double Percent { get; set; }`
`12`		`- }`
`13`	`7`	`public class CoverageSummary`
`14`	`8`	`{`
`15`	`9`	`public CoverageDetails CalculateLineCoverage(Lines lines)`
`16`	`10`	`{`
`17`	`11`	`var details = new CoverageDetails();`
`18`	`12`	`details.Covered = lines.Where(l => l.Value.Hits > 0).Count();`
`19`	`13`	`details.Total = lines.Count;`
`20`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`21`		`- details.Percent = Math.Round(coverage, 3);`
`22`	`14`	`return details;`
`23`	`15`	`}`
`24`	`16`
`@@ -31,9 +23,6 @@ public CoverageDetails CalculateLineCoverage(Methods methods)`
`31`	`23`	`details.Covered += methodCoverage.Covered;`
`32`	`24`	`details.Total += methodCoverage.Total;`
`33`	`25`	`}`
`34`		`-`
`35`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`36`		`- details.Percent = Math.Round(coverage, 3);`
`37`	`26`	`return details;`
`38`	`27`	`}`
`39`	`28`
`@@ -46,9 +35,6 @@ public CoverageDetails CalculateLineCoverage(Classes classes)`
`46`	`35`	`details.Covered += classCoverage.Covered;`
`47`	`36`	`details.Total += classCoverage.Total;`
`48`	`37`	`}`
`49`		`-`
`50`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`51`		`- details.Percent = Math.Round(coverage, 3);`
`52`	`38`	`return details;`
`53`	`39`	`}`
`54`	`40`
`@@ -61,9 +47,6 @@ public CoverageDetails CalculateLineCoverage(Documents documents)`
`61`	`47`	`details.Covered += documentCoverage.Covered;`
`62`	`48`	`details.Total += documentCoverage.Total;`
`63`	`49`	`}`
`64`		`-`
`65`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`66`		`- details.Percent = Math.Round(coverage, 3);`
`67`	`50`	`return details;`
`68`	`51`	`}`
`69`	`52`
`@@ -76,9 +59,6 @@ public CoverageDetails CalculateLineCoverage(Modules modules)`
`76`	`59`	`details.Covered += moduleCoverage.Covered;`
`77`	`60`	`details.Total += moduleCoverage.Total;`
`78`	`61`	`}`
`79`		`-`
`80`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`81`		`- details.Percent = Math.Round(coverage, 3);`
`82`	`62`	`return details;`
`83`	`63`	`}`
`84`	`64`
`@@ -87,8 +67,6 @@ public CoverageDetails CalculateBranchCoverage(List<BranchInfo> branchInfo)`
`87`	`67`	`var details = new CoverageDetails();`
`88`	`68`	`details.Covered = branchInfo.Count(bi => bi.Hits > 0);`
`89`	`69`	`details.Total = branchInfo.Count;`
`90`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`91`		`- details.Percent = Math.Round(coverage, 3);`
`92`	`70`	`return details;`
`93`	`71`	`}`
`94`	`72`
`@@ -97,8 +75,6 @@ public CoverageDetails CalculateBranchCoverage(Branches branches)`
`97`	`75`	`var details = new CoverageDetails();`
`98`	`76`	`details.Covered = branches.Sum(b => b.Value.Where(bi => bi.Hits > 0).Count());`
`99`	`77`	`details.Total = branches.Sum(b => b.Value.Count());`
`100`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`101`		`- details.Percent = Math.Round(coverage, 3);`
`102`	`78`	`return details;`
`103`	`79`	`}`
`104`	`80`
`@@ -111,9 +87,6 @@ public CoverageDetails CalculateBranchCoverage(Methods methods)`
`111`	`87`	`details.Covered += methodCoverage.Covered;`
`112`	`88`	`details.Total += methodCoverage.Total;`
`113`	`89`	`}`
`114`		`-`
`115`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`116`		`- details.Percent = Math.Round(coverage, 3);`
`117`	`90`	`return details;`
`118`	`91`	`}`
`119`	`92`
`@@ -126,9 +99,6 @@ public CoverageDetails CalculateBranchCoverage(Classes classes)`
`126`	`99`	`details.Covered += classCoverage.Covered;`
`127`	`100`	`details.Total += classCoverage.Total;`
`128`	`101`	`}`
`129`		`-`
`130`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`131`		`- details.Percent = Math.Round(coverage, 3);`
`132`	`102`	`return details;`
`133`	`103`	`}`
`134`	`104`
`@@ -141,9 +111,6 @@ public CoverageDetails CalculateBranchCoverage(Documents documents)`
`141`	`111`	`details.Covered += documentCoverage.Covered;`
`142`	`112`	`details.Total += documentCoverage.Total;`
`143`	`113`	`}`
`144`		`-`
`145`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`146`		`- details.Percent = Math.Round(coverage, 3);`
`147`	`114`	`return details;`
`148`	`115`	`}`
`149`	`116`
`@@ -156,9 +123,6 @@ public CoverageDetails CalculateBranchCoverage(Modules modules)`
`156`	`123`	`details.Covered += moduleCoverage.Covered;`
`157`	`124`	`details.Total += moduleCoverage.Total;`
`158`	`125`	`}`
`159`		`-`
`160`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`161`		`- details.Percent = Math.Round(coverage, 3);`
`162`	`126`	`return details;`
`163`	`127`	`}`
`164`	`128`
`@@ -167,7 +131,6 @@ public CoverageDetails CalculateMethodCoverage(Lines lines)`
`167`	`131`	`var details = new CoverageDetails();`
`168`	`132`	`details.Covered = lines.Any(l => l.Value.Hits > 0) ? 1 : 0;`
`169`	`133`	`details.Total = 1;`
`170`		`- details.Percent = details.Covered;`
`171`	`134`	`return details;`
`172`	`135`	`}`
`173`	`136`
`@@ -181,9 +144,6 @@ public CoverageDetails CalculateMethodCoverage(Methods methods)`
`181`	`144`	`details.Covered += methodCoverage.Covered;`
`182`	`145`	`}`
`183`	`146`	`details.Total = methodsWithLines.Count();`
`184`		`-`
`185`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`186`		`- details.Percent = Math.Round(coverage, 3);`
`187`	`147`	`return details;`
`188`	`148`	`}`
`189`	`149`
`@@ -196,9 +156,6 @@ public CoverageDetails CalculateMethodCoverage(Classes classes)`
`196`	`156`	`details.Covered += classCoverage.Covered;`
`197`	`157`	`details.Total += classCoverage.Total;`
`198`	`158`	`}`
`199`		`-`
`200`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`201`		`- details.Percent = Math.Round(coverage, 3);`
`202`	`159`	`return details;`
`203`	`160`	`}`
`204`	`161`
`@@ -211,9 +168,6 @@ public CoverageDetails CalculateMethodCoverage(Documents documents)`
`211`	`168`	`details.Covered += documentCoverage.Covered;`
`212`	`169`	`details.Total += documentCoverage.Total;`
`213`	`170`	`}`
`214`		`-`
`215`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`216`		`- details.Percent = Math.Round(coverage, 3);`
`217`	`171`	`return details;`
`218`	`172`	`}`
`219`	`173`
`@@ -226,9 +180,6 @@ public CoverageDetails CalculateMethodCoverage(Modules modules)`
`226`	`180`	`details.Covered += moduleCoverage.Covered;`
`227`	`181`	`details.Total += moduleCoverage.Total;`
`228`	`182`	`}`
`229`		`-`
`230`		`- double coverage = details.Total == 0 ? details.Total : details.Covered / details.Total;`
`231`		`- details.Percent = Math.Round(coverage, 3);`
`232`	`183`	`return details;`
`233`	`184`	`}`
`234`	`185`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ public static class CecilSymbolHelper`
`19`	`19`	`{`
`20`	`20`	`private const int StepOverLineCode = 0xFEEFEE;`
`21`	`21`	`private static readonly Regex IsMovenext = new Regex(@"\<[^\s>]+\>\w__\w(\w)?::MoveNext\(\)$", RegexOptions.Compiled \| RegexOptions.ExplicitCapture);`
	`22`	`+`
`22`	`23`	`public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)`
`23`	`24`	`{`
`24`	`25`	`var list = new List<BranchPoint>();`
`@@ -56,7 +57,7 @@ private static void GetBranchPoints(MethodDefinition methodDefinition, List<Bran`
`56`	`57`	`var branchingInstructionLine = closestSeqPt.Maybe(x => x.StartLine, -1);`
`57`	`58`	`var document = closestSeqPt.Maybe(x => x.Document.Url);`
`58`	`59`
`59`		`- if (null == instruction.Next)`
	`60`	`+ if (instruction.Next == null)`
`60`	`61`	`return;`
`61`	`62`
`62`	`63`	`if (!BuildPointsForConditionalBranch(list, instruction, branchingInstructionLine, document, branchOffset, pathCounter, instructions, ref ordinal, methodDefinition))`