Decorate Diff.Compare generics with interface

This allows for a typesafe Diff.Compare model.

This commit introduces a new IDiffResult interface which marks classes
which can be passed to Diff.Compare<T> so we get rid of the runtime
exception.

Fixes #1176.

Author: Therzok

LibGit2Sharp.Tests/DiffTreeToTreeFixture.cs

@@ -1123,17 +1123,6 @@ public void RetrievingDiffChangesMustAlwaysBeCaseSensitive()
             }
         }
 
-        [Fact]
-        public void CallingCompareWithAnUnsupportedGenericParamThrows()
-        {
-            var path = SandboxStandardTestRepoGitDir();
-            using (var repo = new Repository(path))
-            {
-                Assert.Throws<LibGit2SharpException>(() => repo.Diff.Compare<string>(default(Tree), default(Tree)));
-                Assert.Throws<LibGit2SharpException>(() => repo.Diff.Compare<string>());
-            }
-        }
-
         [Fact]
         public void UsingPatienceAlgorithmCompareOptionProducesPatienceDiff()
         {

LibGit2Sharp.Tests/MetaFixture.cs

@@ -17,7 +17,7 @@ public class MetaFixture
     {
         private static readonly HashSet<Type> explicitOnlyInterfaces = new HashSet<Type>
         {
-            typeof(IBelongToARepository),
+            typeof(IBelongToARepository), typeof(IDiffResult),
         };
 
         [Fact]
@@ -401,6 +401,20 @@ where method.IsDefined(typeof(ExtensionAttribute), false)
                         select method;
             return query;
         }
+
+        [Fact]
+        public void AllIDiffResultsAreInChangesBuilder()
+        {
+            var diff = typeof(Diff).GetField("ChangesBuilders", BindingFlags.NonPublic | BindingFlags.Static);
+            var changesBuilders = (System.Collections.IDictionary)diff.GetValue(null);
+
+            IEnumerable<Type> diffResults = typeof(Diff).Assembly.GetExportedTypes()
+                .Where(type => type.GetInterface("IDiffResult") != null);
+
+            var nonBuilderTypes = diffResults.Where(diffResult => !changesBuilders.Contains(diffResult));
+            Assert.False(nonBuilderTypes.Any(), "Classes which implement IDiffResult but are not registered under ChangesBuilders in Diff:" + Environment.NewLine +
+                string.Join(Environment.NewLine, nonBuilderTypes.Select(type => type.FullName)));
+        }
     }
 
     internal static class TypeExtensions

LibGit2Sharp/Diff.cs

@@ -102,6 +102,21 @@ private static IDictionary<DiffTargets, Func<Repository, TreeComparisonHandleRet
             { typeof(PatchStats), diff => new PatchStats(diff) },
         };
 
+
+        private static T BuildDiffResult<T>(DiffSafeHandle diff) where T : class, IDiffResult
+        {
+            Func<DiffSafeHandle, object> builder;
+
+            if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
+            {
+                throw new LibGit2SharpException(CultureInfo.InvariantCulture,
+                    "User-defined types passed to Compare are not supported. Supported values are: {0}",
+                    string.Join(", ", ChangesBuilders.Keys.Select(x => x.Name)));
+            }
+
+            return (T)builder(diff);
+        }
+
         /// <summary>
         /// Show changes between two <see cref="Blob"/>s.
         /// </summary>
@@ -134,7 +149,7 @@ public virtual ContentChanges Compare(Blob oldBlob, Blob newBlob, CompareOptions
         /// <param name="oldTree">The <see cref="Tree"/> you want to compare from.</param>
         /// <param name="newTree">The <see cref="Tree"/> you want to compare to.</param>
         /// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
-        public virtual T Compare<T>(Tree oldTree, Tree newTree) where T : class
+        public virtual T Compare<T>(Tree oldTree, Tree newTree) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, newTree, null, null, null);
         }
@@ -146,7 +161,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree) where T : class
         /// <param name="newTree">The <see cref="Tree"/> you want to compare to.</param>
         /// <param name="paths">The list of paths (either files or directories) that should be compared.</param>
         /// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
-        public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths) where T : class
+        public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, newTree, paths, null, null);
         }
@@ -163,7 +178,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
         /// </param>
         /// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
         public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths,
-            ExplicitPathsOptions explicitPathsOptions) where T : class
+            ExplicitPathsOptions explicitPathsOptions) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, newTree, paths, explicitPathsOptions, null);
         }
@@ -176,7 +191,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
         /// <param name="paths">The list of paths (either files or directories) that should be compared.</param>
         /// <param name="compareOptions">Additional options to define patch generation behavior.</param>
         /// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
-        public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths, CompareOptions compareOptions) where T : class
+        public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths, CompareOptions compareOptions) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, newTree, paths, null, compareOptions);
         }
@@ -188,7 +203,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
         /// <param name="newTree">The <see cref="Tree"/> you want to compare to.</param>
         /// <param name="compareOptions">Additional options to define patch generation behavior.</param>
         /// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
-        public virtual T Compare<T>(Tree oldTree, Tree newTree, CompareOptions compareOptions) where T : class
+        public virtual T Compare<T>(Tree oldTree, Tree newTree, CompareOptions compareOptions) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, newTree, null, null, compareOptions);
         }
@@ -206,19 +221,8 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, CompareOptions compareOp
         /// <param name="compareOptions">Additional options to define patch generation behavior.</param>
         /// <returns>A <see cref="TreeChanges"/> containing the changes between the <paramref name="oldTree"/> and the <paramref name="newTree"/>.</returns>
         public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> paths, ExplicitPathsOptions explicitPathsOptions,
-            CompareOptions compareOptions) where T : class
+                               CompareOptions compareOptions) where T : class, IDiffResult
         {
-            Func<DiffSafeHandle, object> builder;
-
-            if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
-            {
-                throw new LibGit2SharpException(CultureInfo.InvariantCulture,
-                                                "Unexpected type '{0}' passed to Compare. Supported values are either '{1}' or '{2}'.",
-                                                typeof(T),
-                                                typeof(TreeChanges),
-                                                typeof(Patch));
-            }
-
             var comparer = TreeToTree(repo);
             ObjectId oldTreeId = oldTree != null ? oldTree.Id : null;
             ObjectId newTreeId = newTree != null ? newTree.Id : null;
@@ -236,7 +240,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
 
             using (DiffSafeHandle diff = BuildDiffList(oldTreeId, newTreeId, comparer, diffOptions, paths, explicitPathsOptions, compareOptions))
             {
-                return (T)builder(diff);
+                return BuildDiffResult<T>(diff);
             }
         }
 
@@ -252,7 +256,7 @@ public virtual T Compare<T>(Tree oldTree, Tree newTree, IEnumerable<string> path
         /// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
-        public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets) where T : class
+        public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, diffTargets, null, null, null);
         }
@@ -270,7 +274,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets) where T : cla
         /// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
-        public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths) where T : class
+        public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, diffTargets, paths, null, null);
         }
@@ -293,7 +297,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
         public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths,
-            ExplicitPathsOptions explicitPathsOptions) where T : class
+            ExplicitPathsOptions explicitPathsOptions) where T : class, IDiffResult
         {
             return Compare<T>(oldTree, diffTargets, paths, explicitPathsOptions, null);
         }
@@ -317,19 +321,8 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the <see cref="Tree"/> and the selected target.</returns>
         public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<string> paths,
-            ExplicitPathsOptions explicitPathsOptions, CompareOptions compareOptions) where T : class
+            ExplicitPathsOptions explicitPathsOptions, CompareOptions compareOptions) where T : class, IDiffResult
         {
-            Func<DiffSafeHandle, object> builder;
-
-            if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
-            {
-                throw new LibGit2SharpException(CultureInfo.InvariantCulture,
-                                                "Unexpected type '{0}' passed to Compare. Supported values are either '{1}' or '{2}'.",
-                                                typeof(T),
-                                                typeof(TreeChanges),
-                                                typeof(Patch));
-            }
-
             var comparer = HandleRetrieverDispatcher[diffTargets](repo);
             ObjectId oldTreeId = oldTree != null ? oldTree.Id : null;
 
@@ -349,7 +342,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s
 
             using (DiffSafeHandle diff = BuildDiffList(oldTreeId, null, comparer, diffOptions, paths, explicitPathsOptions, compareOptions))
             {
-                return (T)builder(diff);
+                return BuildDiffResult<T>(diff);
             }
         }
 
@@ -363,7 +356,7 @@ public virtual T Compare<T>(Tree oldTree, DiffTargets diffTargets, IEnumerable<s
         /// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
-        public virtual T Compare<T>() where T : class
+        public virtual T Compare<T>() where T : class, IDiffResult
         {
             return Compare<T>(DiffModifiers.None);
         }
@@ -379,7 +372,7 @@ public virtual T Compare<T>() where T : class
         /// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
-        public virtual T Compare<T>(IEnumerable<string> paths) where T : class
+        public virtual T Compare<T>(IEnumerable<string> paths) where T : class, IDiffResult
         {
             return Compare<T>(DiffModifiers.None, paths);
         }
@@ -396,7 +389,7 @@ public virtual T Compare<T>(IEnumerable<string> paths) where T : class
         /// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
-        public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked) where T : class
+        public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked) where T : class, IDiffResult
         {
             return Compare<T>(includeUntracked ? DiffModifiers.IncludeUntracked : DiffModifiers.None, paths);
         }
@@ -417,7 +410,7 @@ public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked) wh
         /// <typeparam name="T">Can be either a <see cref="TreeChanges"/> if you are only interested in the list of files modified, added, ..., or
         /// a <see cref="Patch"/> if you want the actual patch content for the whole diff and for individual files.</typeparam>
         /// <returns>A <typeparamref name="T"/> containing the changes between the working directory and the index.</returns>
-        public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked, ExplicitPathsOptions explicitPathsOptions) where T : class
+        public virtual T Compare<T>(IEnumerable<string> paths, bool includeUntracked, ExplicitPathsOptions explicitPathsOptions) where T : class, IDiffResult
         {
             return Compare<T>(includeUntracked ? DiffModifiers.IncludeUntracked : DiffModifiers.None, paths, explicitPathsOptions);
         }
@@ -443,7 +436,7 @@ public virtual T Compare<T>(
             IEnumerable<string> paths,
             bool includeUntracked,
             ExplicitPathsOptions explicitPathsOptions,
-            CompareOptions compareOptions) where T : class
+            CompareOptions compareOptions) where T : class, IDiffResult
         {
             return Compare<T>(includeUntracked ? DiffModifiers.IncludeUntracked : DiffModifiers.None, paths, explicitPathsOptions, compareOptions);
         }
@@ -452,19 +445,8 @@ internal virtual T Compare<T>(
             DiffModifiers diffOptions,
             IEnumerable<string> paths = null,
             ExplicitPathsOptions explicitPathsOptions = null,
-            CompareOptions compareOptions = null) where T : class
+            CompareOptions compareOptions = null) where T : class, IDiffResult
         {
-            Func<DiffSafeHandle, object> builder;
-
-            if (!ChangesBuilders.TryGetValue(typeof(T), out builder))
-            {
-                throw new LibGit2SharpException(CultureInfo.InvariantCulture,
-                                                "Unexpected type '{0}' passed to Compare. Supported values are either '{1}' or '{2}'.",
-                                                typeof(T),
-                                                typeof(TreeChanges),
-                                                typeof(Patch));
-            }
-
             var comparer = WorkdirToIndex(repo);
 
             if (explicitPathsOptions != null)
@@ -479,7 +461,7 @@ internal virtual T Compare<T>(
 
             using (DiffSafeHandle diff = BuildDiffList(null, null, comparer, diffOptions, paths, explicitPathsOptions, compareOptions))
             {
-                return (T)builder(diff);
+                return BuildDiffResult<T>(diff);
             }
         }
 

LibGit2Sharp/IDiffResult.cs

@@ -0,0 +1,5 @@
+namespace LibGit2Sharp
+{
+    public interface IDiffResult
+    { }
+}

LibGit2Sharp/LibGit2Sharp.csproj

@@ -380,6 +380,7 @@
     <Compile Include="Core\GitCertificateSsh.cs" />
     <Compile Include="Core\GitCertificateSshType.cs" />
     <Compile Include="CertificateSsh.cs" />
+    <Compile Include="IDiffResult.cs" />
   </ItemGroup>
   <ItemGroup>
     <CodeAnalysisDictionary Include="CustomDictionary.xml" />

LibGit2Sharp/Patch.cs

@@ -16,7 +16,7 @@ namespace LibGit2Sharp
     /// deleted, modified, ..., then consider using a simpler <see cref="TreeChanges"/>.</para>
     /// </summary>
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
-    public class Patch : IEnumerable<PatchEntryChanges>
+    public class Patch : IEnumerable<PatchEntryChanges>, IDiffResult
     {
         private readonly StringBuilder fullPatchBuilder = new StringBuilder();
 
@@ -41,7 +41,6 @@ internal Patch(DiffSafeHandle diff)
                     AddFileChange(delta);
                     Proxy.git_patch_print(patch, PrintCallBack);
                 }
-
             }
         }
 

LibGit2Sharp/PatchStats.cs

@@ -13,7 +13,7 @@ namespace LibGit2Sharp
     /// <para>The individual patches for each file can be accessed through the indexer of this class.</para>
     /// </summary>
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
-    public class PatchStats : IEnumerable<ContentChangeStats>
+    public class PatchStats : IEnumerable<ContentChangeStats>, IDiffResult
     {
         private readonly IDictionary<FilePath, ContentChangeStats> changes = new Dictionary<FilePath, ContentChangeStats>();
         private readonly int totalLinesAdded;

LibGit2Sharp/TreeChanges.cs

@@ -15,7 +15,7 @@ namespace LibGit2Sharp
     /// <para>To obtain the actual patch of the diff, use the <see cref="Patch"/> class when calling Compare.</para>.
     /// </summary>
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
-    public class TreeChanges : IEnumerable<TreeEntryChanges>
+    public class TreeChanges : IEnumerable<TreeEntryChanges>, IDiffResult
     {
         private readonly List<TreeEntryChanges> changes = new List<TreeEntryChanges>();
         private readonly List<TreeEntryChanges> added = new List<TreeEntryChanges>();
@@ -91,7 +91,6 @@ IEnumerator IEnumerable.GetEnumerator()
 
         #endregion
 
-
         /// <summary>
         /// List of <see cref="TreeEntryChanges"/> that have been been added.
         /// </summary>