From ab1aaed845bed53aba3834d9e59229ac7fd5d510 Mon Sep 17 00:00:00 2001
From: Ben Kraft <benkraft@khanacademy.org>
Date: Wed, 22 Sep 2021 17:00:27 -0700
Subject: [PATCH] Fix a bug in type-name collapsing introduced by #71 (#110)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary:
We typically name our types `OperationFieldTypeFieldType`, but if a
type's name matches the preceding field-name, we omit the type-name.
In #71 I changed the behavior such that we no longer do that in the case
where the type's name matches some suffix of the name-so-far that's
longer than just the leaf field-name.

This was semi-intentional; I assumed it didn't matter and would be more
predictable this way.  But it turns out that was a feature, both in the
sense that almost any change to the type-name-generator is breaking, and
in the sense that it made the names uglier.  Plus, now that we have
better conflict-detection (#94), the possibility that some tricksy
type-names could cause problems is no longer as much of an issue, so we
can be a little less careful here.  (Although I think this is no less
safe than before; the field-names are the important part.)  So in this
commit I revert the change.

Specifically, this comes up a lot at Khan where we do
```
mutation ForcePhantom {
    forcePhantom {     # type: ForcePhantom
        error { ... }  # type: ForcePhantomError
    }
}
```
Before #71, and again after this change, we'll generate
`ForcePhantomForcePhantomError` for `error`; before we'd generate
`ForcePhantomForcePhantomErrorForcePhantomError`.

Issue: https://github.com/Khan/genqlient/issues/109

## Test plan:
make tesc


Author: benjaminjkraft

Reviewers: csilvers, aberkan, dnerdy, jvoll, mahtabsabet, MiguelCastillo, StevenACoffman

Required Reviewers:

Approved By: csilvers

Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint

Pull Request URL: https://github.com/Khan/genqlient/pull/110
---
 docs/CHANGELOG.md      | 2 ++
 generate/names.go      | 4 ++--
 generate/names_test.go | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 19ddb43b..f4c2e740 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -26,6 +26,8 @@ When releasing a new version:
 
 ### Bug fixes:
 
+- Generated type-names now abbreviate across multiple components; for example if the path to a type is `(MyOperation, Outer, Outer, Inner, OuterInner)`, it will again be called `MyOperationOuterInner`.  (This regressed in a pre-v0.1.0 refactor.) (#109)
+
 ## v0.1.0
 
 First open-sourced version.
diff --git a/generate/names.go b/generate/names.go
index e9bc5003..24f8bde7 100644
--- a/generate/names.go
+++ b/generate/names.go
@@ -143,9 +143,9 @@ func typeNameParts(prefix *prefixList, typeName string) *prefixList {
 	// If the prefix has just one part, that's the operation-name.  There's no
 	// need to add "Query" or "Mutation".  (Zero should never happen.)
 	if prefix == nil || prefix.tail == nil ||
-		// If the last prefix field ends with this type's name, omit the
+		// If the name-so-far ends with this type's name, omit the
 		// type-name (see the "shortened" case in the top-of-file comment).
-		strings.HasSuffix(prefix.head, typeName) {
+		strings.HasSuffix(joinPrefixList(prefix), typeName) {
 		return prefix
 	}
 	return &prefixList{typeName, prefix}
diff --git a/generate/names_test.go b/generate/names_test.go
index befb6559..1e3d2fc2 100644
--- a/generate/names_test.go
+++ b/generate/names_test.go
@@ -33,8 +33,8 @@ func TestTypeNames(t *testing.T) {
 		[]*ast.Field{fakeField("Query", "operationUser")},
 		"User",
 	}, {
-		// We don't shorten across multiple prefixes.
-		"OperationUserOperationUser",
+		// We do shorten across multiple prefixes.
+		"OperationUser",
 		[]*ast.Field{fakeField("Query", "user")},
 		"OperationUser",
 	}, {