Keep the type arguments of generic column types and recognise their interfaces by dschreij · Pull Request #66 · go-gorm/cli

dschreij · 2026-09-05T11:33:29Z

Problem

A field of an instantiated generic type came out wrong twice over. datatypes.JSONType[map[string]string], for one, generated field.Struct[datatypes.JSONType[any]]:

The type arguments were lost. The type printer had no case for a map type, an interface type or a second argument (Pair[K, V]), so they rendered as any.
The type was not recognised as a column. The package and type names were split at the last dot of the whole expression, which for Box[example.com/sample.Plain] falls inside the argument, so the named type was never found and its driver.Valuer and sql.Scanner implementations were never checked. The field fell through to the association helper.

Fix

The package and type split happens on the part before the arguments and the lookup asks for the bare generic name. ImplementsAllowedInterfaces instantiates a generic type with any for each parameter before asking types.Implements, whose behaviour is unspecified for an uninstantiated generic type; constraints are not checked, since the question is which methods the type has and Value and Scan do not mention the parameters. The printer renders map types, multi-argument instantiations and the empty interface. The helper keeps the arguments as the field spelled them, shortened per component so a package path inside an argument does not break the type: field.Field[datatypes.JSONType[map[string]string]], field.Struct[sample.Pair[sample.Plain, int]].

Test

TestGenericColumnTypesKeepTheirArgumentsAndInterfaces generates over a generic Box[T] implementing Valuer and Scanner, instantiated with a map, a local struct and through a pointer, and over a generic Pair[K, V] that implements neither and stays an association with both arguments intact. TestShortTypeName gains the multi-argument cases.

This branch is stacked on #65, whose shortTypeName it extends; merging #65 first leaves this one with a single fix commit. The test helper commit is the one shared by the other pull requests from this fork.

🤖 Generated with Claude Code

Copilot

🟡 Changes recommended

There’s a confirmed incorrect type-shortening path for slice relation element types (generic args can be truncated), and the new shared test helper should create parent directories for nested relative paths.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves the generator’s type handling for generic Go types so that (1) instantiated type arguments are preserved when rendering helper types and (2) generic named types are correctly recognized as “column” types when they implement driver.Valuer / sql.Scanner (or other allowed interfaces), rather than being misclassified as associations.

Changes:

Instantiate uninstantiated generic named types with any before checking types.Implements, to reliably detect interface implementations.
Preserve and correctly shorten complex type expressions (maps, multi-arg generic instantiations, empty interface) so generated helpers keep the original type arguments.
Add focused regression tests using a temp-module generator harness for serializer-tagged fields and generic column/association classification.

File summaries

File	Description
internal/gen/utils.go	Adds generic instantiation for interface checks and introduces robust `shortTypeName` (maps, multi-arg generics) + serializer tag detection.
internal/gen/generator.go	Fixes package/type splitting for generics, uses `shortTypeName` for generated helper types, and extends type printer for maps, multi-arg generics, and empty interface.
internal/gen/serializer_columns_test.go	Adds regression tests ensuring serializer-tagged fields generate `field.Field[...]` and expands `shortTypeName` coverage.
internal/gen/generic_column_types_test.go	Adds regression tests ensuring generic column types keep their type args and are classified by interface implementation.
internal/gen/generator_support_test.go	Adds shared test helper to generate from ad-hoc sources in a temp module and assert generated helper field types.

Review details

Files reviewed: 5/5 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@@ -464,7 +474,7 @@ func (f Field) Type() string {
 		elementType := filepath.Base(strings.TrimPrefix(goType, "[]"))
 		return fmt.Sprintf("field.Slice[%s]", elementType)


+	for name, content := range files {
+		if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
+			t.Fatalf("write %s: %v", name, err)
+		}
+	}


dschreij · 2026-09-05T15:39:08Z

Addressed the review. The shared test helper now creates parent directories before writing, so a nested key like models/model.go works as its doc comment promises.

The slice finding was a real defect and is fixed. field.Slice still took filepath.Base of the raw element while the struct branch had moved to shortTypeName, so an element carrying an import path inside its type arguments was truncated at the last slash: []Box[example.com/sample.Plain] rendered as field.Slice[sample.Plain]]. The element now goes through shortTypeName, and the leading * is still dropped, as you noted it should be.

TestGenericColumnTypesKeepTheirArgumentsAndInterfaces gained a []Box[Plain] field asserting field.Slice[sample.Box[sample.Plain]]; it fails on the previous code.

Copilot

🟡 Changes recommended

Field.Type() can unnecessarily invoke packages.Load for slice/map types due to the new strings.Cut base-type extraction, potentially causing significant performance regressions during generation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

 	if typ := loadNamedType(f.file.goModDir, f.file.getFullImportPath(pkgName), typName); typ != nil {
 		if ImplementsAllowedInterfaces(typ) { // For interface-implementing types, use generic Field
-			return fmt.Sprintf("field.Field[%s]", filepath.Base(goType))
+			return fmt.Sprintf("field.Field[%s]", shortTypeName(goType))
 		}
 	}


dschreij · 2026-09-05T20:00:46Z

Addressed the second round, and this one was a real regression rather than a nit.

You are right that the strings.Cut base-type extraction made the lookup run for containers. []T cuts to an empty base name and map[K]V to map, and where the old code passed a nonsense package path that failed fast, the new one passed the file's own package, so packages.Load did real work before finding nothing. On a struct with many slice fields that is not free.

The lookup is now guarded by isNamedTypeCandidate, which skips an empty base name, map, and anything starting with [] or map[. A named slice or map still reaches the lookup, since its name is its own and a named byte slice has to keep resolving to field.Bytes.

The shared test helper also reads the whole output tree rather than its top level.

Copilot

🟡 Changes recommended

The interface-implementation lookup still resolves the package path incorrectly for fully-qualified types, so external interface-implementing types may remain misclassified despite the intended fix.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

+	// Only a plausible named type is worth loading a package for. A container has none to find:
+	// "[]T" leaves an empty base name and "map[K]V" leaves "map", and the lookup would shell out
+	// to the go command for a package it then finds nothing in. A named slice or map still
+	// reaches this, its name being its own.
+	if isNamedTypeCandidate(goType, typName) {
+		if typ := loadNamedType(f.file.goModDir, f.file.getFullImportPath(pkgName), typName); typ != nil {
+			if ImplementsAllowedInterfaces(typ) { // For interface-implementing types, use generic Field
+				return fmt.Sprintf("field.Field[%s]", shortTypeName(goType))
+			}
 		}
 	}


dschreij · 2026-09-05T20:45:19Z

I do not think this one holds, and I have added a test rather than just argue it.

getFullImportPath returns its argument unchanged when it matches no import, not "":

func (p *File) getFullImportPath(shortName string) string {
	for _, i := range p.Imports {
		if i.Name == shortName {
			return i.Path
		}
	}
	return shortName
}

So for gorm.io/datatypes.JSONType[map[string]string] the base type splits into gorm.io/datatypes and JSONType, no import is named gorm.io/datatypes, and the full path is handed to loadNamedType unchanged. That is the path the load needs, so the type resolves and the interface check runs.

TestGenericColumnTypesResolveThroughAFullImportPath now pins it: a generic Valuer declared in another package of the fixture module, referenced through its full import path, has to come out as field.Field[box.Box[map[string]string]]. It would be field.Struct if the lookup returned nothing.

Independently of the test, this is also what the change was written for. Running the generator over a real model package, a datatypes.JSONType[map[string]string] column went from field.Struct[datatypes.JSONType[any]] to field.Field[datatypes.JSONType[map[string]string]], which only happens when the external type resolves and ImplementsAllowedInterfaces sees its Valuer.

Copilot

🟡 Changes recommended

The updated map type rendering can cause map[...]... fields containing package-qualified element types to be misclassified as field.Struct[...] associations, which can generate incorrect helpers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

 	} else if strings.Contains(goType, ".") {
-		return fmt.Sprintf("field.Struct[%s]", filepath.Base(goType))
+		return fmt.Sprintf("field.Struct[%s]", shortTypeName(goType))
 	}


…nterfaces A field of an instantiated generic type, datatypes.JSONType[map[string]string] for one, came out wrong twice over. Its type arguments were lost: the type printer had no case for a map type, an interface type or a second argument, so they rendered as any. And the package and type names were split at the last dot of the whole expression, which for Box[example.com/sample.Plain] falls inside the argument, so the named type was never found, its Valuer and Scanner interfaces never checked, and the column became an association helper: field.Struct[datatypes.JSONType[any]]. The split now happens on the part before the arguments, the lookup asks for the bare generic name, and the type is instantiated with any for each parameter before types.Implements is asked, which is unspecified for an uninstantiated generic type. The printer renders map types, multi-argument instantiations and the empty interface, and the helper keeps the arguments as the field spelled them, shortened per component so a package path inside an argument does not break the type. TestGenericColumnTypesKeepTheirArgumentsAndInterfaces covers a generic Valuer with a map, a local struct and a pointer instantiation, and a generic struct that stays an association with its two arguments intact. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

dschreij · 2026-09-06T05:29:56Z

Right, and this one was mine. Fixed.

Rendering the map type is what exposed it: the association branch tests for a dot anywhere in the type, so map[string]sample.Attr matched and came out as field.Struct[map[string]sample.Attr]. Before this pull request an unnamed map rendered as any and fell through to field.Field[any], wrong in a different way but at least a column.

A map is now its own branch, ahead of the association test, and the default return uses shortTypeName too, as you suggested, so it does not truncate a generic either.

TestMapColumnsAreNotAssociations pins it over a plain map, one with a qualified value, and one whose value is a slice of a qualified type. It fails without the new branch.

Copilot

🟢 Approval recommended

The changes directly address the described generic-type and interface-detection issues and are covered by targeted new tests for both behavior and formatting.

Review details

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Lite

Copilot AI lite review requested due to automatic review settings September 5, 2026 11:33

Copilot started reviewing on behalf of dschreij September 5, 2026 11:34 View session

Copilot AI reviewed Sep 5, 2026

View reviewed changes

dschreij force-pushed the fix/generic-column-types branch from 873c987 to b085164 Compare September 5, 2026 15:38

dschreij requested a lite review from Copilot September 5, 2026 19:47

Copilot started reviewing on behalf of dschreij September 5, 2026 19:47 View session

Copilot AI reviewed Sep 5, 2026

View reviewed changes

Daniel Schreij added 2 commits September 5, 2026 21:59

Add a test helper that runs the generator over ad hoc sources

e95d801

Treat serializer-tagged fields as columns, not associations

25b4dee

dschreij force-pushed the fix/generic-column-types branch from b085164 to ae3038d Compare September 5, 2026 20:00

dschreij requested a lite review from Copilot September 5, 2026 20:37

Copilot started reviewing on behalf of dschreij September 5, 2026 20:38 View session

Copilot AI reviewed Sep 5, 2026

View reviewed changes

dschreij force-pushed the fix/generic-column-types branch from ae3038d to 4f1549f Compare September 5, 2026 20:45

dschreij requested a lite review from Copilot September 6, 2026 05:22

Copilot started reviewing on behalf of dschreij September 6, 2026 05:22 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

Comment thread internal/gen/generator.go

Comment on lines 484 to 486

} else if strings.Contains(goType, ".") {

return fmt.Sprintf("field.Struct[%s]", filepath.Base(goType))

return fmt.Sprintf("field.Struct[%s]", shortTypeName(goType))

}

dschreij force-pushed the fix/generic-column-types branch from 4f1549f to d4461ee Compare September 6, 2026 05:29

dschreij requested a lite review from Copilot September 6, 2026 05:33

Copilot started reviewing on behalf of dschreij September 6, 2026 05:33 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

Navigation Menu

Uh oh!

Keep the type arguments of generic column types and recognise their interfaces - #66

Keep the type arguments of generic column types and recognise their interfaces#66
dschreij wants to merge 3 commits into
go-gorm:masterfrom
dschreij:fix/generic-column-types

dschreij commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

dschreij commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

dschreij commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

dschreij commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

dschreij commented Sep 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -464,7 +474,7 @@ func (f Field) Type() string {
		elementType := filepath.Base(strings.TrimPrefix(goType, "[]"))
		return fmt.Sprintf("field.Slice[%s]", elementType)

Uh oh!

Conversation

dschreij commented Sep 5, 2026

Problem

Fix

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

dschreij commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

dschreij commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

dschreij commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

dschreij commented Sep 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants