Improve package-sniffing and bind correctly to types in the same package

The original motivation here is to fix #283, which is that if you try to
use `bindings` to bind to a type in the same package as the generated
code, we generate a self-import, which Go doesn't allow. Fixing that is
easy -- the three lines in `imports.go` -- once you know the
package-path of the generated code. (The test that that all fits
together is in integration-tests because that was the easiest place to
set up the right situation.)

Determining the package-path is not too much harder: you ask
`go/packages`, which we already use for `package_bindings`. But once
we're doing that, and handling errors, it's kinda silly that we ask you
to specify the package your generated code will use, because we have
that too, usually. So I rewrote that handling too, making `package` now
rarely necessary (see for example the `example` config), and warning if
it looks wrong. This is the changes in `config.go`, and is the more
substantial non-test change. (I also renamed some of the testdata dirs
to be valid package-names, to exercise more of that code, or in the case
of `find-config`, just for consistency.)

docs/CHANGELOG.md

@@ -27,11 +27,13 @@ When releasing a new version:
 - The new `optional: generic` allows using a generic type to represent optionality. See the [documentation](genqlient.yaml) for details.
 - For schemas with enum values that differ only in casing, it's now possible to disable smart-casing in genqlient.yaml; see the [documentation](genqlient.yaml) for `casing` for details.
 - Support .graphqls and .gql file extensions
+- More accurately guess the package name for generated code (and warn if the config option -- now almost never needed -- looks wrong).
 
 ### Bug fixes:
 - The presence of negative pointer directives, i.e., `# @genqlient(pointer: false)` are now respected even in the when `optional: pointer` is set in the configuration file.
 - Made name collisions between query/mutation arguments and local function variables less likely.
 - Fix generation issue related to golang type implementation of complex graphql union fragments
+- Bind correctly to types in the same package as the generated code.
 
 ## v0.6.0
 

docs/genqlient.yaml

@@ -29,8 +29,12 @@ operations:
 # genqlient.yaml. Default: generated.go.
 generated: generated/genqlient.go
 
-# The package name for the output code; defaults to the directory name of
-# the generated-code file.
+# The package name for the output code; defaults to the package-name
+# corresponding to the setting of `generated`, above.
+#
+# This is rarely needed: only if you want the package-name to differ from the
+# suffix of the package-path, and there are no other Go files in the package
+# already.
 package: mygenerated
 
 # If set, a file at this path (relative to genqlient.yaml) will be generated

example/genqlient.yaml

@@ -2,8 +2,6 @@ schema: schema.graphql
 operations:
 - genqlient.graphql
 generated: generated.go
-# needed since it doesn't match the directory name:
-package: main
 
 # We bind github's DateTime scalar type to Go's time.Time (which conveniently
 # already defines MarshalJSON and UnmarshalJSON).  This means genqlient will

generate/config.go

@@ -47,6 +47,8 @@ type Config struct {
 	// The directory of the config-file (relative to which all the other paths
 	// are resolved).  Set by ValidateAndFillDefaults.
 	baseDir string
+	// The package-path into which we are generating.
+	pkgPath string
 }
 
 // A TypeBinding represents a Go type to which genqlient will bind a particular
@@ -132,6 +134,46 @@ func pathJoin(a, b string) string {
 	return filepath.Join(a, b)
 }
 
+func (c *Config) getPackageNameAndPath() (pkgName, pkgPath string, err error) {
+	abs, err := filepath.Abs(c.Generated)
+	if err != nil {
+		return "", "", err
+	}
+
+	dir := filepath.Dir(abs)
+	pkgNameGuess := filepath.Base(dir)
+	if !token.IsIdentifier(pkgNameGuess) {
+		pkgNameGuess = ""
+	}
+
+	pkgs, err := packages.Load(&packages.Config{Mode: packages.NeedName}, dir)
+	if err != nil {
+		return pkgNameGuess, "", err
+	} else if len(pkgs) != 1 {
+		return pkgNameGuess, "", fmt.Errorf("found %v packages in %v, expected 1", len(pkgs), dir)
+	}
+
+	pkg := pkgs[0]
+	// TODO(benkraft): Can PkgPath ever be empty without error? If so, we could
+	// warn.
+	if pkg.Name != "" {
+		return pkg.Name, pkg.PkgPath, nil
+	}
+
+	// e.g. empty package yet to be created, see if we can just guess a
+	// reasonable name.
+	pathSuffix := filepath.Base(pkg.PkgPath)
+	if token.IsIdentifier(pathSuffix) {
+		pkgNameGuess = pathSuffix
+	}
+
+	if pkgNameGuess != "" {
+		return pkgNameGuess, pkg.PkgPath, nil
+	} else {
+		return "", "", fmt.Errorf("no package found in %v", dir)
+	}
+}
+
 // ValidateAndFillDefaults ensures that the configuration is valid, and fills
 // in any options that were unspecified.
 //
@@ -167,29 +209,40 @@ func (c *Config) ValidateAndFillDefaults(baseDir string) error {
 			"\nExample: \"github.com/Org/Repo/optional.Value\"")
 	}
 
-	if c.Package != "" {
-		if !token.IsIdentifier(c.Package) {
-			// No need for link here -- if you're already setting the package
-			// you know where to set the package.
-			return errorf(nil, "invalid package in genqlient.yaml: '%v' is not a valid identifier", c.Package)
-		}
-	} else {
-		abs, err := filepath.Abs(c.Generated)
-		if err != nil {
-			return errorf(nil, "unable to guess package-name: %v"+
-				"\nSet package name in genqlient.yaml"+
-				"\nExample: https://github.com/Khan/genqlient/blob/main/example/genqlient.yaml#L6", err)
-		}
+	if c.Package != "" && !token.IsIdentifier(c.Package) {
+		// No need for link here -- if you're already setting the package
+		// you know where to set the package.
+		return errorf(nil, "invalid package in genqlient.yaml: '%v' is not a valid identifier", c.Package)
+	}
 
-		base := filepath.Base(filepath.Dir(abs))
-		if !token.IsIdentifier(base) {
-			return errorf(nil, "unable to guess package-name: '%v' is not a valid identifier"+
-				"\nSet package name in genqlient.yaml"+
-				"\nExample: https://github.com/Khan/genqlient/blob/main/example/genqlient.yaml#L6", base)
+	pkgName, pkgPath, err := c.getPackageNameAndPath()
+	if err == nil {
+		if c.Package == pkgName || c.Package == "" {
+			c.Package = pkgName
+		} else {
+			fmt.Printf("warning: package setting in genqlient.yaml '%v' looks wrong "+
+				"('%v' is in package '%v') but proceeding with '%v' anyway\n",
+				c.Package, c.Generated, pkgName, c.Package)
 		}
-
-		c.Package = base
+	} else if c.Package != "" {
+		// If you specified a valid package, at least try to use that.
+		// But we can't set pkgPath, which means you'll run into trouble
+		// binding against the generated package, so at least warn.
+		fmt.Printf("warning: unable to identify current package-path (using 'package' config '%v'): %v\n", c.Package, err)
+	} else if pkgName != "" {
+		// If the directory-name is valid, use that. This is useful if you
+		// somehow can't build, and especially for tests.
+		fmt.Printf("warning: unable to identify current package-path (using directory name '%v': %v\n", pkgName, err)
+		c.Package = pkgName
+	} else {
+		return errorf(nil, "unable to guess package-name: %v"+
+			"\nSet package name in genqlient.yaml"+
+			"\nExample: https://github.com/Khan/genqlient/blob/main/example/genqlient.yaml#L6", err)
 	}
+	// This is not likely to work if we got an error, especially if we did the
+	// c.Package fallback. But it's more likely to work than nothing, so we may
+	// as well.
+	c.pkgPath = pkgPath
 
 	if len(c.PackageBindings) > 0 {
 		for _, binding := range c.PackageBindings {

generate/config_test.go

@@ -11,9 +11,9 @@ import (
 )
 
 const (
-	findConfigDir    = "testdata/find-config"
-	validConfigDir   = "testdata/valid-config"
-	invalidConfigDir = "testdata/invalid-config"
+	findConfigDir    = "testdata/findConfig"
+	validConfigDir   = "testdata/validConfig"
+	invalidConfigDir = "testdata/invalidConfig"
 )
 
 func TestFindCfg(t *testing.T) {

generate/imports.go

@@ -99,6 +99,9 @@ func (g *generator) ref(fullyQualifiedName string) (qualifiedName string, err er
 
 	pkgPath := nameToImport[:i]
 	localName := nameToImport[i+1:]
+	if pkgPath == g.Config.pkgPath {
+		return prefix + localName, nil
+	}
 	alias, ok := g.imports[pkgPath]
 	if !ok {
 		if g.importsLocked {

generate/testdata/snapshots/TestInvalidConfigs-InvalidCasing.yaml

@@ -1 +1 @@
-invalid config file testdata/invalid-config/InvalidCasing.yaml: unknown casing algorithm: bogus
+invalid config file testdata/invalidConfig/InvalidCasing.yaml: unknown casing algorithm: bogus

generate/testdata/snapshots/TestInvalidConfigs-InvalidOptional.yaml

@@ -1 +1 @@
-invalid config file testdata/invalid-config/InvalidOptional.yaml: optional must be one of: 'value' (default), 'pointer', or 'generic'
+invalid config file testdata/invalidConfig/InvalidOptional.yaml: optional must be one of: 'value' (default), 'pointer', or 'generic'

generate/testdata/snapshots/TestInvalidConfigs-InvalidPackage.yaml

@@ -1 +1 @@
-invalid config file testdata/invalid-config/InvalidPackage.yaml: invalid package in genqlient.yaml: 'bogus-package-name' is not a valid identifier
+invalid config file testdata/invalidConfig/InvalidPackage.yaml: invalid package in genqlient.yaml: 'bogus-package-name' is not a valid identifier

generate/testdata/snapshots/TestValidConfigs-Simple.yaml → generate/testdata/snapshots/TestValidConfigs-Empty.yml

@@ -1,7 +1,7 @@
 (*generate.Config)({
   Schema: (generate.StringList) <nil>,
   Operations: (generate.StringList) <nil>,
-  Generated: (string) (len=34) "testdata/valid-config/generated.go",
+  Generated: (string) (len=33) "testdata/validConfig/generated.go",
   Package: (string) (len=11) "validConfig",
   ExportOperations: (string) "",
   ContextType: (string) (len=15) "context.Context",
@@ -17,5 +17,6 @@
   StructReferences: (bool) false,
   Extensions: (bool) false,
   AllowBrokenFeatures: (bool) false,
-  baseDir: (string) (len=21) "testdata/valid-config"
+  baseDir: (string) (len=20) "testdata/validConfig",
+  pkgPath: (string) (len=55) "github.com/Khan/genqlient/generate/testdata/validConfig"
 })

generate/testdata/snapshots/TestValidConfigs-Lists.yaml

@@ -1,13 +1,13 @@
 (*generate.Config)({
   Schema: (generate.StringList) (len=2) {
-    (string) (len=42) "testdata/valid-config/first_schema.graphql",
-    (string) (len=43) "testdata/valid-config/second_schema.graphql"
+    (string) (len=41) "testdata/validConfig/first_schema.graphql",
+    (string) (len=42) "testdata/validConfig/second_schema.graphql"
   },
   Operations: (generate.StringList) (len=2) {
-    (string) (len=46) "testdata/valid-config/first_operations.graphql",
-    (string) (len=47) "testdata/valid-config/second_operations.graphql"
+    (string) (len=45) "testdata/validConfig/first_operations.graphql",
+    (string) (len=46) "testdata/validConfig/second_operations.graphql"
   },
-  Generated: (string) (len=34) "testdata/valid-config/generated.go",
+  Generated: (string) (len=33) "testdata/validConfig/generated.go",
   Package: (string) (len=11) "validConfig",
   ExportOperations: (string) "",
   ContextType: (string) (len=15) "context.Context",
@@ -23,5 +23,6 @@
   StructReferences: (bool) false,
   Extensions: (bool) false,
   AllowBrokenFeatures: (bool) false,
-  baseDir: (string) (len=21) "testdata/valid-config"
+  baseDir: (string) (len=20) "testdata/validConfig",
+  pkgPath: (string) (len=55) "github.com/Khan/genqlient/generate/testdata/validConfig"
 })

generate/testdata/snapshots/TestValidConfigs-Strings.yaml

@@ -1,11 +1,11 @@
 (*generate.Config)({
   Schema: (generate.StringList) (len=1) {
-    (string) (len=36) "testdata/valid-config/schema.graphql"
+    (string) (len=35) "testdata/validConfig/schema.graphql"
   },
   Operations: (generate.StringList) (len=1) {
-    (string) (len=40) "testdata/valid-config/operations.graphql"
+    (string) (len=39) "testdata/validConfig/operations.graphql"
   },
-  Generated: (string) (len=34) "testdata/valid-config/generated.go",
+  Generated: (string) (len=33) "testdata/validConfig/generated.go",
   Package: (string) (len=11) "validConfig",
   ExportOperations: (string) "",
   ContextType: (string) (len=15) "context.Context",
@@ -21,5 +21,6 @@
   StructReferences: (bool) false,
   Extensions: (bool) false,
   AllowBrokenFeatures: (bool) false,
-  baseDir: (string) (len=21) "testdata/valid-config"
+  baseDir: (string) (len=20) "testdata/validConfig",
+  pkgPath: (string) (len=55) "github.com/Khan/genqlient/generate/testdata/validConfig"
 })

generate/testdata/valid-config/Lists.yaml → generate/testdata/validConfig/Lists.yaml

@@ -5,5 +5,3 @@ schema:
 operations:
 - first_operations.graphql
 - second_operations.graphql
-
-package: validConfig

generate/testdata/valid-config/Strings.yaml → generate/testdata/validConfig/Strings.yaml

@@ -1,3 +1,2 @@
 schema: schema.graphql
 operations: operations.graphql
-package: validConfig

internal/integration/genqlient.yaml

@@ -8,3 +8,5 @@ bindings:
     type: time.Time
     marshaler: "github.com/Khan/genqlient/internal/testutil.MarshalDate"
     unmarshaler: "github.com/Khan/genqlient/internal/testutil.UnmarshalDate"
+  MyGreatScalar:
+    type: github.com/Khan/genqlient/internal/integration.MyGreatScalar

internal/integration/integration_test.go

@@ -20,7 +20,7 @@ import (
 
 func TestSimpleQuery(t *testing.T) {
 	_ = `# @genqlient
-	query simpleQuery { me { id name luckyNumber } }`
+	query simpleQuery { me { id name luckyNumber greatScalar } }`
 
 	ctx := context.Background()
 	server := server.RunServer()

internal/integration/schema.graphql

@@ -1,4 +1,5 @@
 scalar Date
+scalar MyGreatScalar
 
 type Query {
   me: User
@@ -23,6 +24,7 @@ type User implements Being & Lucky {
   hair: Hair
   birthdate: Date
   friends: [User!]!
+  greatScalar: MyGreatScalar
 }
 
 input NewUser {