Skip to content

Commit 1266fde

Browse files
authored
feat: silence unused_value on fluent-builder method returns (#218)
1 parent efd89d3 commit 1266fde

19 files changed

Lines changed: 398 additions & 1 deletion

File tree

bindgen/bindgen.stdlib.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
"bytes": ["Buffer.WriteByte", "Buffer.WriteRune", "Buffer.WriteString"],
66
"fmt": ["Print", "Printf", "Println", "Fprint", "Fprintf", "Fprintln"],
77
"strings": ["Builder.WriteByte", "Builder.WriteRune", "Builder.WriteString"]
8+
},
9+
"deny_unused_value": {
10+
"go/types": ["TypeParam.Underlying"],
11+
"html/template": ["Template.New"]
812
}
913
},
1014
"types": {

bindgen/internal/config/config.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type Overrides struct {
2121
// LintOverrides holds lint-suppression overrides.
2222
type LintOverrides struct {
2323
AllowUnusedResult map[string][]string `json:"allow_unused_result"`
24+
DenyUnusedValue map[string][]string `json:"deny_unused_value"`
2425
}
2526

2627
// TypeOverrides holds type-conversion overrides.
@@ -77,6 +78,18 @@ func (c *Config) ShouldAllowUnusedResult(pkg, funcName string) bool {
7778
return matchesWildcard(funcs, funcName)
7879
}
7980

81+
// ShouldDenyUnusedValue forces the AST fluent-method heuristic off for curated methods that match its shape but semantically return new values.
82+
func (c *Config) ShouldDenyUnusedValue(pkg, name string) bool {
83+
if c == nil {
84+
return false
85+
}
86+
names, ok := lookupWithGlob(c.Overrides.Lints.DenyUnusedValue, pkg)
87+
if !ok {
88+
return false
89+
}
90+
return matchesWildcard(names, name)
91+
}
92+
8093
// ShouldWrapNilableReturn returns true if the given function or method in the given
8194
// package should be wrapped in Option<> because it can return nil.
8295
// Uses "Type.Method" dot notation for methods.

bindgen/internal/convert/converters.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,118 @@ func (c *Converter) convertMethod(result *ConvertResult, symbolExport extract.Sy
296296
return
297297
}
298298
result.TypeParams = methodSpecs
299+
300+
if isFluentBuilderCandidate(result, symbolExport, signature) {
301+
if fn := c.findFuncDecl(symbolExport.Obj); fn != nil && isFluentMethod(fn, ncGetReceiverName(fn)) {
302+
if c.cfg == nil || !c.cfg.ShouldDenyUnusedValue(c.currentPkgPath, qualifiedName) {
303+
result.BuilderMethod = true
304+
}
305+
}
306+
}
307+
}
308+
309+
// isFluentBuilderCandidate gates AST inspection. Clone/Copy return new values despite the fluent shape.
310+
func isFluentBuilderCandidate(result *ConvertResult, exp extract.SymbolExport, sig *types.Signature) bool {
311+
if result.Receiver == nil || !result.Receiver.IsPointer || exp.IsPromoted {
312+
return false
313+
}
314+
if result.Name == "Clone" || result.Name == "Copy" {
315+
return false
316+
}
317+
return returnIsReceiverShaped(sig)
318+
}
319+
320+
// leftmostIdent walks `recv.A(...).B(...)` chains so the receiver can be detected at the head.
321+
func leftmostIdent(expr ast.Expr) *ast.Ident {
322+
switch e := expr.(type) {
323+
case *ast.Ident:
324+
return e
325+
case *ast.SelectorExpr:
326+
return leftmostIdent(e.X)
327+
case *ast.CallExpr:
328+
return leftmostIdent(e.Fun)
329+
}
330+
return nil
331+
}
332+
333+
// returnIsReceiverShaped filters out delegation that returns unrelated types (e.g. `*Alpha.At -> color.Color`) and Result/Option-wrapped returns where unused_value cannot fire.
334+
func returnIsReceiverShaped(sig *types.Signature) bool {
335+
recv := sig.Recv()
336+
if recv == nil {
337+
return false
338+
}
339+
results := sig.Results()
340+
if results.Len() != 1 {
341+
return false
342+
}
343+
recvPtr, ok := recv.Type().(*types.Pointer)
344+
if !ok {
345+
return false
346+
}
347+
recvNamed, ok := recvPtr.Elem().(*types.Named)
348+
if !ok {
349+
return false
350+
}
351+
352+
if retNamed := singlePointerReturnNamed(sig); retNamed != nil && retNamed == recvNamed {
353+
return true
354+
}
355+
retNamed, ok := results.At(0).Type().(*types.Named)
356+
if !ok {
357+
return false
358+
}
359+
if retNamed == recvNamed {
360+
return true
361+
}
362+
if iface, ok := retNamed.Underlying().(*types.Interface); ok && !iface.Empty() {
363+
return types.Implements(recvPtr, iface)
364+
}
365+
return false
366+
}
367+
368+
// isFluentMethod excludes trivial `return self` getters — real fluent setters either do work before returning or delegate via a method call on the receiver.
369+
func isFluentMethod(fn *ast.FuncDecl, recvName string) bool {
370+
if fn == nil || fn.Body == nil || recvName == "" {
371+
return false
372+
}
373+
374+
hasReturn := false
375+
allMatchRecv := true
376+
anyCallOnRecv := false
377+
ast.Inspect(fn.Body, func(n ast.Node) bool {
378+
if _, ok := n.(*ast.FuncLit); ok {
379+
return false
380+
}
381+
ret, ok := n.(*ast.ReturnStmt)
382+
if !ok {
383+
return true
384+
}
385+
if len(ret.Results) != 1 {
386+
allMatchRecv = false
387+
return true
388+
}
389+
hasReturn = true
390+
switch r := ret.Results[0].(type) {
391+
case *ast.Ident:
392+
if r.Name != recvName {
393+
allMatchRecv = false
394+
}
395+
case *ast.CallExpr:
396+
if id := leftmostIdent(r); id != nil && id.Name == recvName {
397+
anyCallOnRecv = true
398+
return true
399+
}
400+
allMatchRecv = false
401+
default:
402+
allMatchRecv = false
403+
}
404+
return true
405+
})
406+
407+
if !hasReturn || !allMatchRecv {
408+
return false
409+
}
410+
return len(fn.Body.List) > 1 || anyCallOnRecv
299411
}
300412

301413
func (c *Converter) convertType(result *ConvertResult, exp extract.SymbolExport) {

bindgen/internal/convert/dispatch.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ type ConvertResult struct {
3232
// rewrites the return type to Option<int> and emits the matching
3333
// flag-name annotation (e.g. `#[go(sentinel_minus_one)]`).
3434
SentinelInt *int
35+
// BuilderMethod suppresses unused_value on fluent-chain returns the caller typically discards.
36+
BuilderMethod bool
3537
}
3638

3739
type FunctionParameter struct {

bindgen/internal/emit/emitter.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,9 @@ func (e *Emitter) emitMethodInImpl(result convert.ConvertResult) {
346346
e.buf.WriteString(" #[allow(unused_result)]\n")
347347
}
348348
}
349+
if result.BuilderMethod {
350+
e.buf.WriteString(" #[allow(unused_value)]\n")
351+
}
349352

350353
var methodSignature strings.Builder
351354
methodSignature.WriteString(" fn ")
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Fixtures for the builder-method heuristic that emits #[allow(unused_value)]
2+
// when a pointer-receiver method's body returns the receiver itself or
3+
// delegates via a method call on the receiver.
4+
package builder_methods
5+
6+
// Self-return on pointer receiver: classic fluent setter shape.
7+
type Config struct {
8+
name string
9+
tags []string
10+
}
11+
12+
func (c *Config) WithName(n string) *Config { c.name = n; return c }
13+
func (c *Config) WithTag(k, v string) {} // no return: not a builder
14+
func (c *Config) Name() string { return c.name }
15+
16+
// Interface self-return on a real fluent setter (mutates state, returns self).
17+
type Router interface {
18+
Get(path string) Router
19+
Post(path string) Router
20+
}
21+
22+
type Server struct{ routes []string }
23+
24+
func (s *Server) Get(path string) Router { s.routes = append(s.routes, "GET "+path); return s }
25+
func (s *Server) Post(path string) Router { s.routes = append(s.routes, "POST "+path); return s }
26+
27+
// Delegation: body returns a method call on the receiver. Common in Fiber's
28+
// `func (app *App) Get(...) Router { return app.Add(...) }` shape.
29+
type App struct{ server *Server }
30+
31+
func (a *App) Add(method, path string) *App {
32+
a.server.routes = append(a.server.routes, method+" "+path)
33+
return a
34+
}
35+
func (a *App) GetRoute(path string) *App { return a.Add("GET", path) }
36+
func (a *App) PostRoute(path string) *App { return a.Add("POST", path) }
37+
38+
// Pointer receiver returning a *different* concrete type — must not be marked.
39+
type Group struct{}
40+
41+
func (a *App) NewGroup() *Group { return &Group{} }
42+
43+
// Value receiver returning same type — must not be marked (immutable update).
44+
type Coord struct{ X, Y int }
45+
46+
func (c Coord) Translate(dx, dy int) Coord { return Coord{c.X + dx, c.Y + dy} }
47+
48+
// Clone/Copy return a new value — must not be marked even though the body
49+
// returns the receiver name in some path; the Clone/Copy name exclusion catches
50+
// the common case before AST inspection runs.
51+
type Cfg struct{ tags []string }
52+
53+
func (c *Cfg) Clone() *Cfg { return &Cfg{tags: append([]string(nil), c.tags...)} }
54+
func (c *Cfg) Copy() *Cfg { return c.Clone() }
55+
56+
// Trivial single-statement return-self getter — must not be marked. This is
57+
// the structural shape of interface-method implementations like
58+
// `func (b *BasicType) Basic() *BasicType { return b }` in go/debug/dwarf.
59+
type TypeLike interface {
60+
Underlying() TypeLike
61+
Self() TypeLike
62+
}
63+
64+
type Box struct{}
65+
66+
func (b *Box) Underlying() TypeLike { return b }
67+
func (b *Box) Self() TypeLike { return b }
68+
69+
// Copy-and-modify pattern (slog.Logger.With shape) — must not be marked.
70+
// Body assigns to a local and returns the local.
71+
type Logger struct{ attrs []string }
72+
73+
func (l *Logger) With(attr string) *Logger {
74+
clone := &Logger{attrs: append([]string(nil), l.attrs...)}
75+
clone.attrs = append(clone.attrs, attr)
76+
return clone
77+
}
78+
79+
// Multi-path method where one return path is non-receiver (go/types Origin
80+
// shape) — must not be marked because not ALL returns are receiver-like.
81+
type Var struct{ origin *Var }
82+
83+
func (v *Var) Origin() *Var {
84+
if v.origin != nil {
85+
return v.origin
86+
}
87+
return v
88+
}

bindgen/tests/testdata/fixtures/nullability_heuristics/nullability_heuristics.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ type Builder struct{ buf []byte }
2323

2424
func (b *Builder) Reset() *Builder { b.buf = nil; return b }
2525
func (b *Builder) Append(data []byte) *Builder { b.buf = append(b.buf, data...); return b }
26-
func (b *Builder) SetTag(key, val string) *Builder { return b }
26+
func (b *Builder) SetTag(key, val string) *Builder {
27+
b.buf = append(b.buf, []byte(key+"="+val)...)
28+
return b
29+
}
2730

2831
var defaultBuilder = &Builder{}
2932

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Generated by Lisette bindgen
2+
// Source: ./testdata/fixtures/builder_methods (local)
3+
// Go: 0.0.0
4+
// Lisette: 0.0.0
5+
6+
/// Delegation: body returns a method call on the receiver. Common in Fiber's
7+
/// `func (app *App) Get(...) Router { return app.Add(...) }` shape.
8+
pub type App
9+
10+
pub type Box
11+
12+
/// Clone/Copy return a new value — must not be marked even though the body
13+
/// returns the receiver name in some path; the Clone/Copy name exclusion catches
14+
/// the common case before AST inspection runs.
15+
pub type Cfg
16+
17+
/// Self-return on pointer receiver: classic fluent setter shape.
18+
pub type Config
19+
20+
/// Value receiver returning same type — must not be marked (immutable update).
21+
pub struct Coord {
22+
pub X: int,
23+
pub Y: int,
24+
}
25+
26+
/// Pointer receiver returning a *different* concrete type — must not be marked.
27+
pub type Group
28+
29+
/// Copy-and-modify pattern (slog.Logger.With shape) — must not be marked.
30+
/// Body assigns to a local and returns the local.
31+
pub type Logger
32+
33+
/// Interface self-return on a real fluent setter (mutates state, returns self).
34+
pub interface Router {
35+
fn Get(path: string) -> Router
36+
fn Post(path: string) -> Router
37+
}
38+
39+
pub type Server
40+
41+
/// Trivial single-statement return-self getter — must not be marked. This is
42+
/// the structural shape of interface-method implementations like
43+
/// `func (b *BasicType) Basic() *BasicType { return b }` in go/debug/dwarf.
44+
pub interface TypeLike {
45+
fn Self() -> TypeLike
46+
fn Underlying() -> TypeLike
47+
}
48+
49+
/// Multi-path method where one return path is non-receiver (go/types Origin
50+
/// shape) — must not be marked because not ALL returns are receiver-like.
51+
pub type Var
52+
53+
impl App {
54+
#[allow(unused_value)]
55+
fn Add(self: Ref<App>, method: string, path: string) -> Ref<App>
56+
#[allow(unused_value)]
57+
fn GetRoute(self: Ref<App>, path: string) -> Ref<App>
58+
fn NewGroup(self: Ref<App>) -> Ref<Group>
59+
#[allow(unused_value)]
60+
fn PostRoute(self: Ref<App>, path: string) -> Ref<App>
61+
}
62+
63+
impl Box {
64+
fn Self(self: Ref<Box>) -> TypeLike
65+
fn Underlying(self: Ref<Box>) -> TypeLike
66+
}
67+
68+
impl Cfg {
69+
fn Clone(self: Ref<Cfg>) -> Ref<Cfg>
70+
fn Copy(self: Ref<Cfg>) -> Ref<Cfg>
71+
}
72+
73+
impl Config {
74+
fn Name(self: Ref<Config>) -> string
75+
#[allow(unused_value)]
76+
fn WithName(self: Ref<Config>, n: string) -> Ref<Config>
77+
fn WithTag(self: Ref<Config>, k: string, v: string)
78+
}
79+
80+
impl Coord {
81+
fn Translate(self, dx: int, dy: int) -> Coord
82+
}
83+
84+
impl Logger {
85+
fn With(self: Ref<Logger>, attr: string) -> Ref<Logger>
86+
}
87+
88+
impl Server {
89+
#[allow(unused_value)]
90+
fn Get(self: Ref<Server>, path: string) -> Router
91+
#[allow(unused_value)]
92+
fn Post(self: Ref<Server>, path: string) -> Router
93+
}
94+
95+
impl Var {
96+
fn Origin(self: Ref<Var>) -> Ref<Var>
97+
}

bindgen/tests/testdata/snapshots/nullability_heuristics.d.lis

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,11 @@ pub struct Widget {
7474
}
7575

7676
impl Builder {
77+
#[allow(unused_value)]
7778
fn Append(self: Ref<Builder>, data: Slice<uint8>) -> Ref<Builder>
79+
#[allow(unused_value)]
7880
fn Reset(self: Ref<Builder>) -> Ref<Builder>
81+
#[allow(unused_value)]
7982
fn SetTag(self: Ref<Builder>, key: string, val: string) -> Ref<Builder>
8083
}
8184

crates/stdlib/typedefs/container/list.d.lis

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ impl List {
3030
fn Front(self: Ref<List>) -> Option<Ref<Element>>
3131

3232
/// Init initializes or clears list l.
33+
#[allow(unused_value)]
3334
fn Init(self: Ref<List>) -> Ref<List>
3435

3536
/// InsertAfter inserts a new element e with value v immediately after mark and returns e.

0 commit comments

Comments
 (0)