Fix submodule parsing when the gitmodules is missing (#35109)

Follow up #35096, fix #35095, fix #35115 and add more tests

The old code used some fragile behaviors which depend on the "nil"
receiver. This PR should be a complete fix for more edge cases.

modules/git/commit_info.go

@@ -15,8 +15,12 @@ type CommitInfo struct {
 func getCommitInfoSubmoduleFile(repoLink string, entry *TreeEntry, commit *Commit, treePathDir string) (*CommitSubmoduleFile, error) {
 	fullPath := path.Join(treePathDir, entry.Name())
 	submodule, err := commit.GetSubModule(fullPath)
-	if submodule == nil || err != nil {
+	if err != nil {
 		return nil, err
 	}
+	if submodule == nil {
+		// unable to find submodule from ".gitmodules" file
+		return NewCommitSubmoduleFile(repoLink, fullPath, "", entry.ID.String()), nil
+	}
 	return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, entry.ID.String()), nil
 }

modules/git/commit_info_test.go

@@ -129,7 +129,14 @@ func TestEntries_GetCommitsInfo(t *testing.T) {
 		require.NoError(t, err)
 		cisf, err := getCommitInfoSubmoduleFile("/any/repo-link", tree, commit, "")
 		require.NoError(t, err)
-		assert.Nil(t, cisf)
+		assert.Equal(t, &CommitSubmoduleFile{
+			repoLink: "/any/repo-link",
+			fullPath: "file1.txt",
+			refURL:   "",
+			refID:    "e2129701f1a4d54dc44f03c93bca0a2aec7c5449",
+		}, cisf)
+		// since there is no refURL, it means that the submodule info doesn't exist, so it won't have a web link
+		assert.Nil(t, cisf.SubmoduleWebLinkTree(t.Context()))
 	})
 }
 

modules/git/commit_submodule_file.go

@@ -29,12 +29,16 @@ func NewCommitSubmoduleFile(repoLink, fullPath, refURL, refID string) *CommitSub
 	return &CommitSubmoduleFile{repoLink: repoLink, fullPath: fullPath, refURL: refURL, refID: refID}
 }
 
+// RefID returns the commit ID of the submodule, it returns empty string for nil receiver
 func (sf *CommitSubmoduleFile) RefID() string {
+	if sf == nil {
+		return ""
+	}
 	return sf.refID
 }
 
 func (sf *CommitSubmoduleFile) getWebLinkInTargetRepo(ctx context.Context, moreLinkPath string) *SubmoduleWebLink {
-	if sf == nil {
+	if sf == nil || sf.refURL == "" {
 		return nil
 	}
 	if strings.HasPrefix(sf.refURL, "../") {
@@ -53,14 +57,13 @@ func (sf *CommitSubmoduleFile) getWebLinkInTargetRepo(ctx context.Context, moreL
 }
 
 // SubmoduleWebLinkTree tries to make the submodule's tree link in its own repo, it also works on "nil" receiver
+// It returns nil if the submodule does not have a valid URL or is nil
 func (sf *CommitSubmoduleFile) SubmoduleWebLinkTree(ctx context.Context, optCommitID ...string) *SubmoduleWebLink {
-	if sf == nil {
-		return nil
-	}
-	return sf.getWebLinkInTargetRepo(ctx, "/tree/"+util.OptionalArg(optCommitID, sf.refID))
+	return sf.getWebLinkInTargetRepo(ctx, "/tree/"+util.OptionalArg(optCommitID, sf.RefID()))
 }
 
 // SubmoduleWebLinkCompare tries to make the submodule's compare link in its own repo, it also works on "nil" receiver
+// It returns nil if the submodule does not have a valid URL or is nil
 func (sf *CommitSubmoduleFile) SubmoduleWebLinkCompare(ctx context.Context, commitID1, commitID2 string) *SubmoduleWebLink {
 	return sf.getWebLinkInTargetRepo(ctx, "/compare/"+commitID1+"..."+commitID2)
 }

modules/git/commit_submodule_file_test.go

@@ -12,6 +12,8 @@ import (
 func TestCommitSubmoduleLink(t *testing.T) {
 	assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkTree(t.Context()))
 	assert.Nil(t, (*CommitSubmoduleFile)(nil).SubmoduleWebLinkCompare(t.Context(), "", ""))
+	assert.Nil(t, (&CommitSubmoduleFile{}).SubmoduleWebLinkTree(t.Context()))
+	assert.Nil(t, (&CommitSubmoduleFile{}).SubmoduleWebLinkCompare(t.Context(), "", ""))
 
 	t.Run("GitHubRepo", func(t *testing.T) {
 		sf := NewCommitSubmoduleFile("/any/repo-link", "full-path", "git@github.com:user/repo.git", "aaaa")

routers/web/repo/view_home.go

@@ -259,9 +259,10 @@ func handleRepoEmptyOrBroken(ctx *context.Context) {
 }
 
 func handleRepoViewSubmodule(ctx *context.Context, submodule *git.SubModule) {
+	// TODO: it needs to use git.NewCommitSubmoduleFile and SubmoduleWebLinkTree to correctly handle relative paths
 	submoduleRepoURL, err := giturl.ParseRepositoryURL(ctx, submodule.URL)
 	if err != nil {
-		HandleGitError(ctx, "prepareToRenderDirOrFile: ParseRepositoryURL", err)
+		HandleGitError(ctx, "handleRepoViewSubmodule: ParseRepositoryURL", err)
 		return
 	}
 	submoduleURL := giturl.MakeRepositoryWebLink(submoduleRepoURL)

services/repository/files/tree.go

@@ -174,7 +174,9 @@ func newTreeViewNodeFromEntry(ctx context.Context, repoLink string, renderedIcon
 		} else if subModule != nil {
 			submoduleFile := git.NewCommitSubmoduleFile(repoLink, node.FullPath, subModule.URL, entry.ID.String())
 			webLink := submoduleFile.SubmoduleWebLinkTree(ctx)
-			node.SubmoduleURL = webLink.CommitWebLink
+			if webLink != nil {
+				node.SubmoduleURL = webLink.CommitWebLink
+			}
 		}
 	}