Skip to content

Commit

Permalink
Merge pull request #297 from dogmatiq/fix-static-analysis-type-alias
Browse files Browse the repository at this point in the history
Fix panicking when handling type aliases in static analysis.
  • Loading branch information
jmalloc authored Aug 16, 2024
2 parents df687b9 + b88e05a commit a765c9c
Show file tree
Hide file tree
Showing 14 changed files with 447 additions and 24 deletions.
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

0 comments on commit a765c9c

Please sign in to comment.