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

[v3] Fix binding generator bugs #4001

Merged
merged 24 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7cc0b66
Add some clarifying comments
fbbdev Jan 6, 2025
0e4a052
Remove special handling of window parameters
fbbdev Jan 9, 2025
6e04897
Improve internal method exclusion
fbbdev Jan 9, 2025
f5f06c1
Add test for internal method exclusion
fbbdev Jan 15, 2025
5d3dc0e
Remove useless blank field from app options
fbbdev Jan 15, 2025
d25886d
Remove redundant godebug setting
fbbdev Jan 15, 2025
804406a
Use new range for syntax to simplify code
fbbdev Jan 15, 2025
c49e103
Remove generator dependency on github.com/samber/lo
fbbdev Jan 15, 2025
3200713
Ensure generator testing tasks do not use the test cache
fbbdev Jan 15, 2025
a6f318f
Rename cyclic types test
fbbdev Jan 15, 2025
215d861
Test for cyclic imports
fbbdev Jan 15, 2025
720661e
Fix import cycle between model files
fbbdev Jan 10, 2025
5fcce72
Sort class aliases after their aliased class
fbbdev Jan 11, 2025
cb51682
Test class aliases
fbbdev Jan 11, 2025
f94a34a
Fix length of default value for array types
fbbdev Jan 14, 2025
6e366ed
Test array initialization
fbbdev Jan 14, 2025
7415949
Add changelog
fbbdev Jan 15, 2025
47aa75c
Update changelog
fbbdev Jan 15, 2025
f496fb7
Merge branch 'v3-alpha' into v3/bindgen-fixes
leaanthony Jan 16, 2025
be9b31c
Merge branch 'v3-alpha' into v3/bindgen-fixes
leaanthony Jan 16, 2025
5e84f8a
Fix contrived marking technique in model sorting algorithm
fbbdev Jan 16, 2025
f69420f
Merge branch 'v3-alpha' into v3/bindgen-fixes
leaanthony Jan 16, 2025
c5e99f5
Update binding example
fbbdev Jan 15, 2025
ee4ed99
Update test data
fbbdev Jan 15, 2025
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 docs/src/content/docs/changelog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### Breaking Changes

- Renamed Service methods: `Name` -> `ServiceName`, `OnStartup` -> `ServiceStartup`, `OnShutdown` -> `ServiceShutdown` by [@leaanthony](https://github.com/leaanthony)

### Added

- Support aarch64 AppImage builds by [@AkshayKalose](https://github.com/AkshayKalose) in [#3981](https://github.com/wailsapp/wails/pull/3981)
- Add diagnostics section to `wails doctor` by [@leaanthony](https://github.com/leaanthony)
- Add window to context when calling a service method by [@leaanthony](https://github.com/leaanthony)
- Add `window-call` example to demonstrate how to know which window is calling a service by [@leaanthony](https://github.com/leaanthony)

### Fixed

- Fixed Windows+Linux Edit Menu issues by [@leaanthony](https://github.com/leaanthony) in [#3f78a3a](https://github.com/wailsapp/wails/commit/3f78a3a8ce7837e8b32242c8edbbed431c68c062)
- Updated the minimum system version in macOS .plist files from 10.13.0 to 10.15.0 by [@AkshayKalose](https://github.com/AkshayKalose) in [#3981](https://github.com/wailsapp/wails/pull/3981)
- Window ID skip issue by [@leaanthony](https://github.com/leaanthony)
- Fix nil menu issue when calling RegisterContextMenu by [@leaanthony](https://github.com/leaanthony)
- Fixed dependency cycles in binding generator output by [@fbbdev](https://github.com/fbbdev) in [#4001](https://github.com/wailsapp/wails/pull/4001)
- Fixed use-before-define errors in binding generator output by [@fbbdev](https://github.com/fbbdev) in [#4001](https://github.com/wailsapp/wails/pull/4001)

### Changed

- Removed `application.WindowIDKey` and `application.WindowNameKey` (replaced by `application.WindowKey`) by [@leaanthony](https://github.com/leaanthony)
- In JS/TS bindings, class fields of fixed-length array types are now initialized with their expected length instead of being empty by [@fbbdev](https://github.com/fbbdev) in [#4001](https://github.com/wailsapp/wails/pull/4001)

## v3.0.0-alpha.9 - 2025-01-13

Expand Down
11 changes: 6 additions & 5 deletions v3/internal/generator/Taskfile.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,26 @@ tasks:

test:
cmds:
- go test -v .
- go test -count=1 -v .
- task: test:check

test:analyse:
cmds:
- go test -v -run ^TestAnalyser .
- go test -count=1 -v -run ^TestAnalyser .

test:constants:
cmds:
- go test -v -run ^TestGenerateConstants .
- go test -v -count=1 -run ^TestGenerateConstants .

test:generate:
cmds:
- go test -v -run ^TestGenerator .
- go test -v -count=1 -run ^TestGenerator .
- task: test:check

test:regenerate:
cmds:
- cmd: rm -rf ./testdata/output/*
- cmd: go test -v -run ^TestGenerator .
- cmd: go test -v -count=1 -run ^TestGenerator .
ignore_error: true
- task: test:generate

Expand All @@ -40,6 +40,7 @@ tasks:
- install-deps
cmds:
- npx tsc
- npx madge --circular output/

install-deps:
internal: true
Expand Down
119 changes: 60 additions & 59 deletions v3/internal/generator/analyse.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"go/token"
"go/types"
"iter"

"github.com/wailsapp/wails/v3/internal/generator/config"
"golang.org/x/tools/go/packages"
Expand All @@ -19,7 +20,7 @@ import (
// Results are deduplicated, i.e. yield is called at most once per object.
//
// If yield returns false, FindBoundTypes returns immediately.
func FindServices(pkgs []*packages.Package, systemPaths *config.SystemPaths, logger config.Logger, yield func(*types.TypeName) bool) error {
func FindServices(pkgs []*packages.Package, systemPaths *config.SystemPaths, logger config.Logger) (iter.Seq[*types.TypeName], error) {
type instanceInfo struct {
args *types.TypeList
pos token.Position
Expand Down Expand Up @@ -122,7 +123,7 @@ func FindServices(pkgs []*packages.Package, systemPaths *config.SystemPaths, log
signature := fn.Type().(*types.Signature)
if signature.Params().Len() > 2 || signature.Results().Len() != 1 || tp.Len() != 1 || tp.At(0).Obj() == nil {
logger.Warningf("Param Len: %d, Results Len: %d, tp.Len: %d, tp.At(0).Obj(): %v", signature.Params().Len(), signature.Results().Len(), tp.Len(), tp.At(0).Obj())
return ErrBadApplicationPackage
return nil, ErrBadApplicationPackage
}

// Schedule unique type param for analysis.
Expand All @@ -136,71 +137,71 @@ func FindServices(pkgs []*packages.Package, systemPaths *config.SystemPaths, log
// found tracks service types that have been found so far, for deduplication.
found := make(map[*types.TypeName]bool)

// Process targets.
for len(next) > 0 {
// Pop one target off the next list.
tgt := next[len(next)-1]
next = next[:len(next)-1]

// Prepare indirect binding message.
indirectMsg := ""
if tgt.cause.IsValid() {
indirectMsg = fmt.Sprintf(" (indirectly bound at %s)", tgt.cause)
}

for _, instance := range instances[tgt.obj] {
// Retrieve type argument.
serviceType := types.Unalias(instance.args.At(tgt.param))
return func(yield func(*types.TypeName) bool) {
// Process targets.
for len(next) > 0 {
// Pop one target off the next list.
tgt := next[len(next)-1]
next = next[:len(next)-1]

// Prepare indirect binding message.
indirectMsg := ""
if tgt.cause.IsValid() {
indirectMsg = fmt.Sprintf(" (indirectly bound at %s)", tgt.cause)
}

var named *types.Named
for _, instance := range instances[tgt.obj] {
// Retrieve type argument.
serviceType := types.Unalias(instance.args.At(tgt.param))

var named *types.Named

switch t := serviceType.(type) {
case *types.Named:
// Process named type.
named = t.Origin()

case *types.TypeParam:
// Schedule type parameter for analysis.
newtgt := target{owner[t.Obj()], t.Index()}
if !scheduled[newtgt] {
scheduled[newtgt] = true

// Retrieve position of call to application.NewService
// that caused this target to be scheduled.
cause := tgt.cause
if !tgt.cause.IsValid() {
// This _is_ a call to application.NewService.
cause = instance.pos
}

switch t := serviceType.(type) {
case *types.Named:
// Process named type.
named = t.Origin()

case *types.TypeParam:
// Schedule type parameter for analysis.
newtgt := target{owner[t.Obj()], t.Index()}
if !scheduled[newtgt] {
scheduled[newtgt] = true

// Retrieve position of call to application.NewService
// that caused this target to be scheduled.
cause := tgt.cause
if !tgt.cause.IsValid() {
// This _is_ a call to application.NewService.
cause = instance.pos
// Push on next list.
next = append(next, targetInfo{newtgt, cause})
}
continue

// Push on next list.
next = append(next, targetInfo{newtgt, cause})
default:
logger.Warningf("%s: ignoring anonymous service type %s%s", instance.pos, serviceType, indirectMsg)
continue
}
continue

default:
logger.Warningf("%s: ignoring anonymous service type %s%s", instance.pos, serviceType, indirectMsg)
continue
}

// Reject interfaces and generic types.
if types.IsInterface(named.Underlying()) {
logger.Warningf("%s: ignoring interface service type %s%s", instance.pos, named, indirectMsg)
continue
} else if named.TypeParams() != nil {
logger.Warningf("%s: ignoring generic service type %s", instance.pos, named, indirectMsg)
continue
}
// Reject interfaces and generic types.
if types.IsInterface(named.Underlying()) {
logger.Warningf("%s: ignoring interface service type %s%s", instance.pos, named, indirectMsg)
continue
} else if named.TypeParams() != nil {
logger.Warningf("%s: ignoring generic service type %s", instance.pos, named, indirectMsg)
continue
}

// Record and yield type object.
if !found[named.Obj()] {
found[named.Obj()] = true
if !yield(named.Obj()) {
return nil
// Record and yield type object.
if !found[named.Obj()] {
found[named.Obj()] = true
if !yield(named.Obj()) {
return
}
}
}
}
}

return nil
}, nil
fbbdev marked this conversation as resolved.
Show resolved Hide resolved
}
9 changes: 5 additions & 4 deletions v3/internal/generator/analyse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,15 @@ func TestAnalyser(t *testing.T) {

got := make([]string, 0)

err = FindServices(pkgs, systemPaths, config.DefaultPtermLogger(nil), func(tn *types.TypeName) bool {
got = append(got, types.TypeString(tn.Type(), nil))
return true
})
services, err := FindServices(pkgs, systemPaths, config.DefaultPtermLogger(nil))
if err != nil {
t.Error(err)
}

for obj := range services {
got = append(got, types.TypeString(obj.Type(), nil))
}
fbbdev marked this conversation as resolved.
Show resolved Hide resolved

slices.Sort(got)

if diff := cmp.Diff(test.want, got); diff != "" {
Expand Down
12 changes: 1 addition & 11 deletions v3/internal/generator/collect/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ type (

// ImportModels records whether models from the current package may be needed.
ImportModels bool
// ImportInternal records whether internal models from the current package may be needed.
ImportInternal bool

// External records information about each imported package,
// keyed by package path.
Expand Down Expand Up @@ -68,9 +66,6 @@ func (imports *ImportMap) Merge(other *ImportMap) {
if other.ImportModels {
imports.ImportModels = true
}
if other.ImportInternal {
imports.ImportInternal = true
}

for path, info := range other.External {
if _, ok := imports.External[path]; ok {
Expand Down Expand Up @@ -151,12 +146,7 @@ func (imports *ImportMap) addTypeImpl(typ types.Type, visited map[*types.TypeNam
}

if obj.Pkg().Path() == imports.Self {
// Record self import.
if obj.Exported() {
imports.ImportModels = true
} else {
imports.ImportInternal = true
}
imports.ImportModels = true
}

// Record model.
Expand Down
49 changes: 22 additions & 27 deletions v3/internal/generator/collect/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,30 @@ import (
"strings"
)

// PackageIndex lists all bindings, models and unexported models
// generated from a package.
// PackageIndex lists all services, models and unexported models
// to be generated from a single package.
//
// When obtained through a call to [PackageInfo.Index],
// each binding and model name appears at most once.
// each service and model appears at most once;
// services are sorted by name;
// exported models precede all unexported ones
// and both ranges are sorted by name.
type PackageIndex struct {
Package *PackageInfo

Services []*ServiceInfo
Models []*ModelInfo
Internal []*ModelInfo

Models []*ModelInfo
HasExportedModels bool // If true, there is at least one exported model.
}

// Index computes a [PackageIndex] for the selected language from the list
// of generated services and models and regenerates cached stats.
//
// Services and models appear at most once in the returned slices,
// which are sorted by name.
// Services and models appear at most once in the returned slices;
// services are sorted by name;
// exported models precede all unexported ones
// and both ranges are sorted by name.
//
// Index calls info.Collect, and therefore provides the same guarantees.
// It is safe for concurrent use.
Expand All @@ -38,7 +44,7 @@ func (info *PackageInfo) Index(TS bool) (index *PackageIndex) {
}

// Gather services.
info.services.Range(func(key, value any) bool {
for _, value := range info.services.Range {
service := value.(*ServiceInfo)
if !service.IsEmpty() {
if service.Object().Exported() {
Expand All @@ -49,8 +55,7 @@ func (info *PackageInfo) Index(TS bool) (index *PackageIndex) {
stats.NumServices++
stats.NumMethods += len(service.Methods)
}
return true
})
}

// Sort services by name.
slices.SortFunc(index.Services, func(b1 *ServiceInfo, b2 *ServiceInfo) int {
Expand All @@ -61,19 +66,22 @@ func (info *PackageInfo) Index(TS bool) (index *PackageIndex) {
})

// Gather models.
info.models.Range(func(key, value any) bool {
for _, value := range info.models.Range {
model := value.(*ModelInfo)
index.Models = append(index.Models, model)
// Mark presence of exported models
if model.Object().Exported() {
index.HasExportedModels = true
}
// Update model stats.
if len(model.Values) > 0 {
stats.NumEnums++
} else {
stats.NumModels++
}
return true
})
}

// Sort models by internal property (non-internal first), then by name.
// Sort models by exported property (exported first), then by name.
slices.SortFunc(index.Models, func(m1 *ModelInfo, m2 *ModelInfo) int {
if m1 == m2 {
return 0
Expand All @@ -91,19 +99,6 @@ func (info *PackageInfo) Index(TS bool) (index *PackageIndex) {
return strings.Compare(m1.Name, m2.Name)
})

// Find first internal model.
split, _ := slices.BinarySearchFunc(index.Models, struct{}{}, func(m *ModelInfo, _ struct{}) int {
if m.Object().Exported() {
return -1
} else {
return 1
}
})

// Separate internal and non-internal models.
index.Internal = index.Models[split:]
index.Models = index.Models[:split]

// Cache stats
info.stats.Store(stats)

Expand Down
4 changes: 2 additions & 2 deletions v3/internal/generator/collect/properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ func IsClass(typ types.Type) bool {
typ = types.Unalias(typ)

if _, isNamed := typ.(*types.Named); !isNamed {
// Not a model type.
// Unnamed types are never rendered as classes.
return false
}

// Struct types without custom marshaling are rendered as classes.
// Struct named types without custom marshaling are rendered as classes.
_, isStruct := typ.Underlying().(*types.Struct)
return isStruct && !MaybeJSONMarshaler(typ) && !MaybeTextMarshaler(typ)
}
Expand Down
Loading
Loading