Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panicking when handling type aliases in static analysis. #297

Merged
merged 3 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ The format is based on [Keep a Changelog], and this project adheres to
[keep a changelog]: https://keepachangelog.com/en/1.0.0/
[semantic versioning]: https://semver.org/spec/v2.0.0.html

## [0.13.5] - 2024-08-16

### Fixed

- Fixed panicking when handling type aliases in static analysis.

## [0.13.4] - 2024-08-16

### Added
Expand Down Expand Up @@ -308,6 +314,7 @@ interfaces. It does not yet include support for static analysis of the new
[0.13.2]: https://github.com/dogmatiq/configkit/releases/v0.13.2
[0.13.3]: https://github.com/dogmatiq/configkit/releases/v0.13.3
[0.13.4]: https://github.com/dogmatiq/configkit/releases/v0.13.4
[0.13.5]: https://github.com/dogmatiq/configkit/releases/v0.13.5

<!-- version template
## [0.0.1] - YYYY-MM-DD
Expand Down
2 changes: 1 addition & 1 deletion static/adaptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var _ = Describe("func FromPackages() (adaptor function)", func() {
When("the the handler is created by adapting a partial handler implementation", func() {
It("builds the configuration from the adapted type", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/adaptor",
}

Expand Down
9 changes: 6 additions & 3 deletions static/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func addHandlerFromType(
) {
pkg := pkgOfNamedType(typ)
method := prog.LookupMethod(typ, pkg, "Configure")
addHandlerFromConfigureMethod(prog, method, hs, ht)
addHandlerFromConfigureMethod(prog, typ.String(), method, hs, ht)
}

// addHandlersFromAdaptorFunc analyzes the arguments of an "adaptor function" to
Expand Down Expand Up @@ -263,7 +263,7 @@ func addHandlersFromAdaptorFunc(
continue
}

addHandlerFromConfigureMethod(prog, method, hs, ht)
addHandlerFromConfigureMethod(prog, typ.String(), method, hs, ht)
}
}

Expand All @@ -273,13 +273,14 @@ func addHandlersFromAdaptorFunc(
// The handler configuration is added to hs.
func addHandlerFromConfigureMethod(
prog *ssa.Program,
handlerType string,
method *ssa.Function,
hs configkit.HandlerSet,
ht configkit.HandlerType,
) {
hdr := &entity.Handler{
HandlerTypeValue: ht,
TypeNameValue: method.Signature.Recv().Type().String(),
TypeNameValue: handlerType,
MessageNamesValue: configkit.EntityMessageNames{
Produced: message.NameRoles{},
Consumed: message.NameRoles{},
Expand Down Expand Up @@ -462,6 +463,8 @@ func tryPkgOfNamedType(typ types.Type) (_ *types.Package, ok bool) {
switch t := typ.(type) {
case *types.Named:
return t.Obj().Pkg(), true
case *types.Alias:
return t.Obj().Pkg(), true
case *types.Pointer:
return tryPkgOfNamedType(t.Elem())
default:
Expand Down
14 changes: 7 additions & 7 deletions static/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var _ = Describe("func FromPackages() (application detection)", func() {
When("a package contains a single application", func() {
It("returns the application configuration", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/apps/single-app",
}

Expand Down Expand Up @@ -45,7 +45,7 @@ var _ = Describe("func FromPackages() (application detection)", func() {
When("multiple packages contain applications", func() {
It("returns all of the application configurations", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/apps/multiple-apps-in-pkgs",
}

Expand Down Expand Up @@ -77,7 +77,7 @@ var _ = Describe("func FromPackages() (application detection)", func() {
When("a single package contains multiple applications", func() {
It("returns all of the application configurations", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/apps/multiple-apps-in-single-pkg/apps",
}

Expand Down Expand Up @@ -109,7 +109,7 @@ var _ = Describe("func FromPackages() (application detection)", func() {
When("a package contains an application implemented with pointer receivers", func() {
It("returns the application configuration", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/apps/pointer-receiver-app",
}

Expand All @@ -132,7 +132,7 @@ var _ = Describe("func FromPackages() (application detection)", func() {
When("none of the packages contain any applications", func() {
It("returns an empty slice", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/apps/no-app",
}

Expand All @@ -147,7 +147,7 @@ var _ = Describe("func FromPackages() (application detection)", func() {
When("a field within the application type is registered as a handler", func() {
It("includes the handler in the application configuration", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/apps/handler-from-field",
}

Expand All @@ -172,7 +172,7 @@ var _ = Describe("func FromPackages() (application detection)", func() {
When("an application in the package has multiple handlers", func() {
It("returns all messages consumed or produced by all handlers", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/apps/app-level-messages",
}

Expand Down
2 changes: 1 addition & 1 deletion static/constructor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var _ = Describe("func FromPackages() (constructor function)", func() {
When("the handler is created by a call to a 'constructor' function", func() {
It("builds the configuration from the adapted type", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/constructor",
}

Expand Down
168 changes: 160 additions & 8 deletions static/handler_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package static_test

import (
"os"

"github.com/dogmatiq/configkit"
cfixtures "github.com/dogmatiq/configkit/fixtures"
"github.com/dogmatiq/configkit/message"
Expand All @@ -26,7 +28,7 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
When("the application contains a single handler of each type", func() {
It("returns a single configuration for each handler type", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/single",
}

Expand Down Expand Up @@ -156,11 +158,161 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
},
))
})
When("a handler is a type alias", func() {
goDbg := os.Getenv("GODEBUG")

BeforeEach(func() {
// Set the GODEBUG environment variable to enable type alias
// support.
//
// TODO: Remove this setting once the package is
// migrated to Go 1.23 that generates `types.Alias` types by
// default.
os.Setenv("GODEBUG", "gotypesalias=1")
})

AfterEach(func() {
os.Setenv("GODEBUG", goDbg)
})

It("returns a single configuration for each handler type", func() {
cfg := packages.Config{
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/typealias",
}

pkgs := loadPackages(cfg)

apps := FromPackages(pkgs)
Expect(apps).To(HaveLen(1))
Expect(apps[0].Handlers().Aggregates()).To(HaveLen(1))
Expect(apps[0].Handlers().Processes()).To(HaveLen(1))
Expect(apps[0].Handlers().Projections()).To(HaveLen(1))
Expect(apps[0].Handlers().Integrations()).To(HaveLen(1))

aggregate := apps[0].Handlers().Aggregates()[0]
Expect(aggregate.Identity()).To(
Equal(
configkit.Identity{
Name: "<aggregate>",
Key: "92623de9-c9cf-42f3-8338-33c50eeb06fb",
},
),
)
Expect(aggregate.TypeName()).To(
Equal(
"github.com/dogmatiq/configkit/static/testdata/handlers/typealias.AggregateHandlerAlias",
),
)
Expect(aggregate.HandlerType()).To(Equal(configkit.AggregateHandlerType))

Expect(aggregate.MessageNames()).To(Equal(
configkit.EntityMessageNames{
Consumed: message.NameRoles{
cfixtures.MessageATypeName: message.CommandRole,
cfixtures.MessageBTypeName: message.CommandRole,
},
Produced: message.NameRoles{
cfixtures.MessageCTypeName: message.EventRole,
cfixtures.MessageDTypeName: message.EventRole,
},
},
))

process := apps[0].Handlers().Processes()[0]
Expect(process.Identity()).To(
Equal(
configkit.Identity{
Name: "<process>",
Key: "ad9d6955-893a-4d8d-a26e-e25886b113b2",
},
),
)
Expect(process.TypeName()).To(
Equal(
"github.com/dogmatiq/configkit/static/testdata/handlers/typealias.ProcessHandlerAlias",
),
)
Expect(process.HandlerType()).To(Equal(configkit.ProcessHandlerType))

Expect(process.MessageNames()).To(Equal(
configkit.EntityMessageNames{
Consumed: message.NameRoles{
cfixtures.MessageATypeName: message.EventRole,
cfixtures.MessageBTypeName: message.EventRole,
cfixtures.MessageETypeName: message.TimeoutRole,
cfixtures.MessageFTypeName: message.TimeoutRole,
},
Produced: message.NameRoles{
cfixtures.MessageCTypeName: message.CommandRole,
cfixtures.MessageDTypeName: message.CommandRole,
cfixtures.MessageETypeName: message.TimeoutRole,
cfixtures.MessageFTypeName: message.TimeoutRole,
},
},
))

projection := apps[0].Handlers().Projections()[0]
Expect(projection.Identity()).To(
Equal(
configkit.Identity{
Name: "<projection>",
Key: "d012b7ed-3c4b-44db-9276-7bbc90fb54fd",
},
),
)
Expect(projection.TypeName()).To(
Equal(
"github.com/dogmatiq/configkit/static/testdata/handlers/typealias.ProjectionHandlerAlias",
),
)
Expect(projection.HandlerType()).To(Equal(configkit.ProjectionHandlerType))

Expect(projection.MessageNames()).To(Equal(
configkit.EntityMessageNames{
Consumed: message.NameRoles{
cfixtures.MessageATypeName: message.EventRole,
cfixtures.MessageBTypeName: message.EventRole,
},
Produced: message.NameRoles{},
},
))

integration := apps[0].Handlers().Integrations()[0]
Expect(integration.Identity()).To(
Equal(
configkit.Identity{
Name: "<integration>",
Key: "4d8cd3f5-21dc-475b-a8dc-80138adde3f2",
},
),
)
Expect(integration.TypeName()).To(
Equal(
"github.com/dogmatiq/configkit/static/testdata/handlers/typealias.IntergrationHandlerAlias",
),
)
Expect(integration.HandlerType()).To(Equal(configkit.IntegrationHandlerType))

Expect(integration.MessageNames()).To(Equal(
configkit.EntityMessageNames{
Consumed: message.NameRoles{
cfixtures.MessageATypeName: message.CommandRole,
cfixtures.MessageBTypeName: message.CommandRole,
},
Produced: message.NameRoles{
cfixtures.MessageCTypeName: message.EventRole,
cfixtures.MessageDTypeName: message.EventRole,
},
},
))
})
})

When("messages are passed to the *Configurer.Routes() method", func() {
It("includes messages passed as args to *Configurer.Routes() method only", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/only-routes-args",
}

Expand Down Expand Up @@ -295,7 +447,7 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
When("messages are passed to the *Configurer.Routes() method as a dynamically populated splice", func() {
It("returns a single configuration for each handler type", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/dynamic-routes",
}

Expand Down Expand Up @@ -430,7 +582,7 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
When("messages are passed to the *Configurer.Routes() method in conditional branches", func() {
It("returns messages populated in every conditional branch", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/conditional-branches",
}

Expand Down Expand Up @@ -565,7 +717,7 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
When("nil is passed to a call of *Configurer.Routes() methods", func() {
It("does not populate messages", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/nil-routes",
}

Expand Down Expand Up @@ -676,7 +828,7 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
When("the application multiple handlers of each type", func() {
It("returns all of the handler configurations", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/multiple",
}

Expand Down Expand Up @@ -726,7 +878,7 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
When("a nil value is passed as a handler", func() {
It("does not add a handler to the application configuration", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/nil-handler",
}

Expand All @@ -741,7 +893,7 @@ var _ = Describe("func FromPackages() (handler analysis)", func() {
When("a handler with a non-pointer methodset is registered as a pointer", func() {
It("includes the handler in the application configuration", func() {
cfg := packages.Config{
Mode: packages.LoadAllSyntax,
Mode: LoadPackagesConfigMode,
Dir: "testdata/handlers/pointer-handler-with-non-pointer-methodset",
}

Expand Down
Loading