object: fix Change.Files() method behavior (fix #317)

- If 'from' or 'to' are tree entries that aren't files, Files() method will return nil instead of object not found error.
- Added a test checking this using modules fixture.

2 files changed, 56 insertions, 0 deletions

diff --git a/plumbing/object/change.go b/plumbing/object/change.go
index d29836d..2f702e4 100644
--- a/plumbing/object/change.go
+++ b/plumbing/object/change.go
@@ -46,6 +46,10 @@ func (c *Change) Files() (from, to *File, err error) {
 
 	if action == merkletrie.Insert || action == merkletrie.Modify {
 		to, err = c.To.Tree.TreeEntryFile(&c.To.TreeEntry)
+		if !c.To.TreeEntry.Mode.IsFile() {
+			return nil, nil, nil
+		}
+
 		if err != nil {
 			return
 		}
@@ -53,6 +57,10 @@ func (c *Change) Files() (from, to *File, err error) {
 
 	if action == merkletrie.Delete || action == merkletrie.Modify {
 		from, err = c.From.Tree.TreeEntryFile(&c.From.TreeEntry)
+		if !c.From.TreeEntry.Mode.IsFile() {
+			return nil, nil, nil
+		}
+
 		if err != nil {
 			return
 		}
diff --git a/plumbing/object/change_test.go b/plumbing/object/change_test.go
index 3bab5d2..005ceb0 100644
--- a/plumbing/object/change_test.go
+++ b/plumbing/object/change_test.go
@@ -198,6 +198,54 @@ func (s *ChangeSuite) TestEmptyChangeFails(c *C) {
 	c.Assert(str, Equals, "malformed change")
 }
 
+// test reproducing bug #317
+func (s *ChangeSuite) TestNoFileFilemodes(c *C) {
+	s.Suite.SetUpSuite(c)
+	f := fixtures.ByURL("https://github.com/git-fixtures/submodule.git").One()
+
+	sto, err := filesystem.NewStorage(f.DotGit())
+	c.Assert(err, IsNil)
+
+	iter, err := sto.IterEncodedObjects(plumbing.AnyObject)
+	c.Assert(err, IsNil)
+	var commits []*Commit
+	iter.ForEach(func(o plumbing.EncodedObject) error {
+		if o.Type() == plumbing.CommitObject {
+			commit, err := GetCommit(sto, o.Hash())
+			c.Assert(err, IsNil)
+			commits = append(commits, commit)
+
+		}
+
+		return nil
+	})
+
+	c.Assert(len(commits), Not(Equals), 0)
+
+	var prev *Commit
+	for _, commit := range commits {
+		if prev == nil {
+			prev = commit
+			continue
+		}
+		tree, err := commit.Tree()
+		c.Assert(err, IsNil)
+		prevTree, err := prev.Tree()
+		c.Assert(err, IsNil)
+		changes, err := DiffTree(tree, prevTree)
+		c.Assert(err, IsNil)
+		for _, change := range changes {
+			_, _, err := change.Files()
+			if err != nil {
+				panic(err)
+				c.Assert(err, IsNil)
+			}
+		}
+
+		prev = commit
+	}
+}
+
 func (s *ChangeSuite) TestErrorsFindingChildsAreDetected(c *C) {
 	// Commit 7beaad711378a4daafccc2c04bc46d36df2a0fd1 of the go-git
 	// fixture modified "examples/latest/latest.go".