refactor: fix go-coding-style violations in manually written code

xaionaro@dx.center · xaionaro@dx.center · commit 0238362e34b2 · 2026-03-17T22:13:38.000Z
- Rename cstringLiteral→newCString (semantic integrity: not a literal)
- Extract findClassWithFallback to flatten 5-level nesting and remove
  duplicated ClassLoader fallback blocks in proxy.go
- Rename ExtractalarmClockInfo→ExtractAlarmClockInfo (broken PascalCase)
- Rename files with prohibited "helpers" suffix:
  bundle_helpers.go→extract_bundle.go, intent_helpers.go→intent_extra.go,
  os/build/helpers.go→split into build_info.go, version_info.go,
  get_build_info.go
- Move types to own files (AlarmClockInfo, BuildInfo, VersionInfo)
- Fix silently discarded errors in os/build (use errors.Join) and
  app/context.ContentResolver (now returns error)
- Deduplicate readString closure into readStaticStringField
- Simplify jnierr cached strings: sync.Once+4 vars → 3 sync.OnceValue
- Remove dead unsafe imports and sentinels
- Format multi-param functions per style (one param per line)
- Fix mismatched doc comment (Location→ExtractedLocation)
- Remove what-not-why comments in bluetooth/new_adapter.go
- Fix "helper" in testjvm package comment
diff --git a/app/alarm/alarm_clock_info.go b/app/alarm/alarm_clock_info.go
@@ -0,0 +1,6 @@
+package alarm
+
+// AlarmClockInfo holds extracted fields from AlarmManager.AlarmClockInfo.
+type AlarmClockInfo struct {
+	TriggerTime int64
+}
diff --git a/app/alarm/extract.go b/app/alarm/extract.go
@@ -2,18 +2,15 @@ package alarm
 
 import (
 	"fmt"
-	"unsafe"
 
 	"github.com/AndroidGoLab/jni"
 )
 
-// AlarmClockInfo holds extracted fields from AlarmManager.AlarmClockInfo.
-type AlarmClockInfo struct {
-	TriggerTime int64
-}
-
-// ExtractalarmClockInfo extracts fields from an AlarmManager.AlarmClockInfo JNI object.
-func ExtractalarmClockInfo(env *jni.Env, obj *jni.Object) (*AlarmClockInfo, error) {
+// ExtractAlarmClockInfo extracts fields from an AlarmManager.AlarmClockInfo JNI object.
+func ExtractAlarmClockInfo(
+	env *jni.Env,
+	obj *jni.Object,
+) (*AlarmClockInfo, error) {
 	if err := ensureInit(env); err != nil {
 		return nil, err
 	}
@@ -33,5 +30,3 @@ func ExtractalarmClockInfo(env *jni.Env, obj *jni.Object) (*AlarmClockInfo, erro
 
 	return &AlarmClockInfo{TriggerTime: triggerTime}, nil
 }
-
-var _ = unsafe.Pointer(nil)
diff --git a/app/context.go b/app/context.go
@@ -85,10 +85,12 @@ func (m *Context) PackageManager() (*jni.Object, error) {
 }
 
 // ContentResolver calls android.content.Context.getContentResolver().
-func (m *Context) ContentResolver() *jni.Object {
+func (m *Context) ContentResolver() (*jni.Object, error) {
 	var result *jni.Object
+	var callErr error
 	m.VM.Do(func(env *jni.Env) error {
 		if err := ensureContextInit(env); err != nil {
+			callErr = err
 			return err
 		}
 		mid, err := env.GetMethodID(
@@ -97,12 +99,13 @@ func (m *Context) ContentResolver() *jni.Object {
 			"()Landroid/content/ContentResolver;",
 		)
 		if err != nil {
-			return err
+			callErr = fmt.Errorf("get getContentResolver: %w", err)
+			return callErr
 		}
-		result, _ = env.CallObjectMethod(m.Obj, mid)
-		return nil
+		result, callErr = env.CallObjectMethod(m.Obj, mid)
+		return callErr
 	})
-	return result
+	return result, callErr
 }
 
 // GetSystemService calls android.content.Context.getSystemService.
diff --git a/app/extract_bundle.go b/app/extract_bundle.go
diff --git a/app/intent_extra.go b/app/intent_extra.go
@@ -2,7 +2,6 @@ package app
 
 import (
 	"fmt"
-	"unsafe"
 
 	"github.com/AndroidGoLab/jni"
 )
@@ -38,7 +37,10 @@ func NewIntent(vm *jni.VM) (*Intent, error) {
 }
 
 // PutStringExtra calls Intent.putExtra(key, value) for string values.
-func (i *Intent) PutStringExtra(key, value string) {
+func (i *Intent) PutStringExtra(
+	key string,
+	value string,
+) {
 	i.VM.Do(func(env *jni.Env) error {
 		cls := env.GetObjectClass(i.Obj)
 		mid, err := env.GetMethodID(cls, "putExtra",
@@ -61,7 +63,10 @@ func (i *Intent) PutStringExtra(key, value string) {
 }
 
 // PutIntExtra calls Intent.putExtra(key, value) for int values.
-func (i *Intent) PutIntExtra(key string, value int32) {
+func (i *Intent) PutIntExtra(
+	key string,
+	value int32,
+) {
 	i.VM.Do(func(env *jni.Env) error {
 		cls := env.GetObjectClass(i.Obj)
 		mid, err := env.GetMethodID(cls, "putExtra",
@@ -80,7 +85,10 @@ func (i *Intent) PutIntExtra(key string, value int32) {
 }
 
 // PutBoolExtra calls Intent.putExtra(key, value) for boolean values.
-func (i *Intent) PutBoolExtra(key string, value bool) {
+func (i *Intent) PutBoolExtra(
+	key string,
+	value bool,
+) {
 	i.VM.Do(func(env *jni.Env) error {
 		cls := env.GetObjectClass(i.Obj)
 		mid, err := env.GetMethodID(cls, "putExtra",
@@ -103,9 +111,10 @@ func (i *Intent) PutBoolExtra(key string, value bool) {
 }
 
 // GetBoolExtra calls Intent.getBooleanExtra(key, defaultValue).
-func (i *Intent) GetBoolExtra(key string, defaultValue bool) bool {
+func (i *Intent) GetBoolExtra(
+	key string,
+	defaultValue bool,
+) bool {
 	result, _ := i.GetBooleanExtra(key, defaultValue)
 	return result
 }
-
-var _ = unsafe.Pointer(nil)
diff --git a/bluetooth/new_adapter.go b/bluetooth/new_adapter.go
@@ -21,7 +21,6 @@ func NewAdapter(ctx *app.Context) (*Adapter, error) {
 			return err
 		}
 
-		// Get BluetoothManager via getSystemService("bluetooth").
 		svc, err := ctx.GetSystemService("bluetooth")
 		if err != nil {
 			return fmt.Errorf("get bluetooth service: %w", err)
@@ -30,7 +29,6 @@ func NewAdapter(ctx *app.Context) (*Adapter, error) {
 			return fmt.Errorf("bluetooth service not available")
 		}
 
-		// Call BluetoothManager.getAdapter() to get the BluetoothAdapter.
 		bmClass, err := env.FindClass("android/bluetooth/BluetoothManager")
 		if err != nil {
 			return fmt.Errorf("find BluetoothManager: %w", err)
@@ -68,5 +66,4 @@ func (m *Adapter) Close() {
 	}
 }
 
-// Ensure imports are used.
 var _ = unsafe.Pointer(nil)
diff --git a/examples/app_framework/main.go b/examples/app_framework/main.go
@@ -106,8 +106,12 @@ func run(vm *jni.VM) error {
 	}
 
 	// Content resolver and package manager.
-	resolver := ctx.ContentResolver()
-	fmt.Fprintf(&output, "content resolver: %v\n", resolver)
+	resolver, err := ctx.ContentResolver()
+	if err != nil {
+		fmt.Fprintf(&output, "  ContentResolver: %v\n", err)
+	} else {
+		fmt.Fprintf(&output, "content resolver: %v\n", resolver)
+	}
 
 	pkgMgr, err := ctx.PackageManager()
 	if err != nil {
diff --git a/internal/jnierr/exception.go b/internal/jnierr/exception.go
@@ -146,25 +146,12 @@ func extractGoString(env *capi.Env, jstr capi.String) string {
 }
 
 var (
-	cachedStringsOnce sync.Once
-	bGetName          *byte
-	bGetMessage       *byte
-	bVoidToString     *byte
+	cstrGetName      = sync.OnceValue(func() *byte { return newCString("getName") })
+	cstrGetMessage   = sync.OnceValue(func() *byte { return newCString("getMessage") })
+	cstrVoidToString = sync.OnceValue(func() *byte { return newCString("()Ljava/lang/String;") })
 )
 
-func initCachedStrings() {
-	cachedStringsOnce.Do(func() {
-		bGetName = cstringLiteral("getName")
-		bGetMessage = cstringLiteral("getMessage")
-		bVoidToString = cstringLiteral("()Ljava/lang/String;")
-	})
-}
-
-func cstrGetName() *byte      { initCachedStrings(); return bGetName }
-func cstrGetMessage() *byte   { initCachedStrings(); return bGetMessage }
-func cstrVoidToString() *byte { initCachedStrings(); return bVoidToString }
-
-func cstringLiteral(s string) *byte {
+func newCString(s string) *byte {
 	b := make([]byte, len(s)+1)
 	copy(b, s)
 	return &b[0]
diff --git a/internal/testjvm/testjvm.go b/internal/testjvm/testjvm.go
@@ -1,4 +1,4 @@
-// Package testjvm provides a helper to create a JVM for testing.
+// Package testjvm creates and manages a JVM instance for use in tests.
 package testjvm
 
 // #cgo CFLAGS: -I/usr/lib/jvm/java-25-openjdk-amd64/include -I/usr/lib/jvm/java-25-openjdk-amd64/include/linux
diff --git a/jni_proxy_test.go b/jni_proxy_test.go
@@ -50,19 +50,19 @@ func TestLookupProxyNotFound(t *testing.T) {
 	}
 }
 
-// --- cstringLiteral ---
+// --- newCString ---
 
-func TestCstringLiteral(t *testing.T) {
-	ptr := cstringLiteral("hello")
+func TestNewCString(t *testing.T) {
+	ptr := newCString("hello")
 	if ptr == nil {
-		t.Fatal("cstringLiteral returned nil")
+		t.Fatal("newCString returned nil")
 	}
 }
 
-func TestCstringLiteralEmpty(t *testing.T) {
-	ptr := cstringLiteral("")
+func TestNewCStringEmpty(t *testing.T) {
+	ptr := newCString("")
 	if ptr == nil {
-		t.Fatal("cstringLiteral returned nil for empty string")
+		t.Fatal("newCString returned nil for empty string")
 	}
 }
 
diff --git a/location/extract.go b/location/extract.go
@@ -7,7 +7,7 @@ import (
 	"github.com/AndroidGoLab/jni"
 )
 
-// Location holds extracted fields from android.location.Location.
+// ExtractedLocation holds extracted fields from android.location.Location.
 type ExtractedLocation struct {
 	Latitude  float64
 	Longitude float64
diff --git a/os/build/build_info.go b/os/build/build_info.go
@@ -0,0 +1,12 @@
+package build
+
+// BuildInfo holds static field values from android.os.Build.
+type BuildInfo struct {
+	Device       string
+	Model        string
+	Product      string
+	Manufacturer string
+	Brand        string
+	Board        string
+	Hardware     string
+}
diff --git a/os/build/get_build_info.go b/os/build/get_build_info.go
@@ -0,0 +1,85 @@
+package build
+
+import (
+	"errors"
+	"fmt"
+	"unsafe"
+
+	"github.com/AndroidGoLab/jni"
+)
+
+func readStaticStringField(
+	env *jni.Env,
+	cls *jni.Class,
+	name string,
+) (string, error) {
+	fid, err := env.GetStaticFieldID(cls, name, "Ljava/lang/String;")
+	if err != nil {
+		return "", fmt.Errorf("get field %s: %w", name, err)
+	}
+	obj := env.GetStaticObjectField(cls, fid)
+	return env.GoString((*jni.String)(unsafe.Pointer(obj))), nil
+}
+
+// GetBuildInfo reads static fields from android.os.Build.
+func GetBuildInfo(vm *jni.VM) (*BuildInfo, error) {
+	var info BuildInfo
+	err := vm.Do(func(env *jni.Env) error {
+		cls, err := env.FindClass("android/os/Build")
+		if err != nil {
+			return fmt.Errorf("find Build: %w", err)
+		}
+
+		var errs []error
+		info.Device, err = readStaticStringField(env, cls, "DEVICE")
+		errs = append(errs, err)
+		info.Model, err = readStaticStringField(env, cls, "MODEL")
+		errs = append(errs, err)
+		info.Product, err = readStaticStringField(env, cls, "PRODUCT")
+		errs = append(errs, err)
+		info.Manufacturer, err = readStaticStringField(env, cls, "MANUFACTURER")
+		errs = append(errs, err)
+		info.Brand, err = readStaticStringField(env, cls, "BRAND")
+		errs = append(errs, err)
+		info.Board, err = readStaticStringField(env, cls, "BOARD")
+		errs = append(errs, err)
+		info.Hardware, err = readStaticStringField(env, cls, "HARDWARE")
+		errs = append(errs, err)
+		return errors.Join(errs...)
+	})
+	if err != nil {
+		return nil, err
+	}
+	return &info, nil
+}
+
+// GetVersionInfo reads static fields from android.os.Build.VERSION.
+func GetVersionInfo(vm *jni.VM) (*VersionInfo, error) {
+	var info VersionInfo
+	err := vm.Do(func(env *jni.Env) error {
+		cls, err := env.FindClass("android/os/Build$VERSION")
+		if err != nil {
+			return fmt.Errorf("find Build.VERSION: %w", err)
+		}
+
+		var errs []error
+		info.Release, err = readStaticStringField(env, cls, "RELEASE")
+		errs = append(errs, err)
+		info.Codename, err = readStaticStringField(env, cls, "CODENAME")
+		errs = append(errs, err)
+		info.Incremental, err = readStaticStringField(env, cls, "INCREMENTAL")
+		errs = append(errs, err)
+
+		sdkFid, err := env.GetStaticFieldID(cls, "SDK_INT", "I")
+		if err != nil {
+			return fmt.Errorf("get SDK_INT: %w", err)
+		}
+		info.SDKInt = env.GetStaticIntField(cls, sdkFid)
+
+		return errors.Join(errs...)
+	})
+	if err != nil {
+		return nil, err
+	}
+	return &info, nil
+}
diff --git a/os/build/helpers.go b/os/build/helpers.go
diff --git a/os/build/version_info.go b/os/build/version_info.go
diff --git a/proxy.go b/proxy.go
diff --git a/tests/e2e/e2e_svc2_tests.go b/tests/e2e/e2e_svc2_tests.go
diff --git a/tests/e2e/e2e_tier1_tests.go b/tests/e2e/e2e_tier1_tests.go

Original file line number	Diff line number	Diff line change
`@@ -85,10 +85,12 @@ func (m Context) PackageManager() (jni.Object, error) {`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`// ContentResolver calls android.content.Context.getContentResolver().`
`88`		`-func (m Context) ContentResolver() jni.Object {`
	`88`	`+func (m Context) ContentResolver() (jni.Object, error) {`
`89`	`89`	`var result *jni.Object`
	`90`	`+ var callErr error`
`90`	`91`	`m.VM.Do(func(env *jni.Env) error {`
`91`	`92`	`if err := ensureContextInit(env); err != nil {`
	`93`	`+ callErr = err`
`92`	`94`	`return err`
`93`	`95`	`}`
`94`	`96`	`mid, err := env.GetMethodID(`
`@@ -97,12 +99,13 @@ func (m Context) ContentResolver() jni.Object {`
`97`	`99`	`"()Landroid/content/ContentResolver;",`
`98`	`100`	`)`
`99`	`101`	`if err != nil {`
`100`		`- return err`
	`102`	`+ callErr = fmt.Errorf("get getContentResolver: %w", err)`
	`103`	`+ return callErr`
`101`	`104`	`}`
`102`		`- result, _ = env.CallObjectMethod(m.Obj, mid)`
`103`		`- return nil`
	`105`	`+ result, callErr = env.CallObjectMethod(m.Obj, mid)`
	`106`	`+ return callErr`
`104`	`107`	`})`
`105`		`- return result`
	`108`	`+ return result, callErr`
`106`	`109`	`}`
`107`	`110`
`108`	`111`	`// GetSystemService calls android.content.Context.getSystemService.`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,6 @@ func NewAdapter(ctx app.Context) (Adapter, error) {`
`21`	`21`	`return err`
`22`	`22`	`}`
`23`	`23`
`24`		`- // Get BluetoothManager via getSystemService("bluetooth").`
`25`	`24`	`svc, err := ctx.GetSystemService("bluetooth")`
`26`	`25`	`if err != nil {`
`27`	`26`	`return fmt.Errorf("get bluetooth service: %w", err)`
`@@ -30,7 +29,6 @@ func NewAdapter(ctx app.Context) (Adapter, error) {`
`30`	`29`	`return fmt.Errorf("bluetooth service not available")`
`31`	`30`	`}`
`32`	`31`
`33`		`- // Call BluetoothManager.getAdapter() to get the BluetoothAdapter.`
`34`	`32`	`bmClass, err := env.FindClass("android/bluetooth/BluetoothManager")`
`35`	`33`	`if err != nil {`
`36`	`34`	`return fmt.Errorf("find BluetoothManager: %w", err)`
`@@ -68,5 +66,4 @@ func (m *Adapter) Close() {`
`68`	`66`	`}`
`69`	`67`	`}`
`70`	`68`
`71`		`-// Ensure imports are used.`
`72`	`69`	`var _ = unsafe.Pointer(nil)`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// Package testjvm provides a helper to create a JVM for testing.`
	`1`	`+// Package testjvm creates and manages a JVM instance for use in tests.`
`2`	`2`	`package testjvm`
`3`	`3`
`4`	`4`	`// #cgo CFLAGS: -I/usr/lib/jvm/java-25-openjdk-amd64/include -I/usr/lib/jvm/java-25-openjdk-amd64/include/linux`
Original file line number	Diff line number	Diff line change
`@@ -50,19 +50,19 @@ func TestLookupProxyNotFound(t *testing.T) {`
`50`	`50`	`}`
`51`	`51`	`}`
`52`	`52`
`53`		`-// --- cstringLiteral ---`
	`53`	`+// --- newCString ---`
`54`	`54`
`55`		`-func TestCstringLiteral(t *testing.T) {`
`56`		`- ptr := cstringLiteral("hello")`
	`55`	`+func TestNewCString(t *testing.T) {`
	`56`	`+ ptr := newCString("hello")`
`57`	`57`	`if ptr == nil {`
`58`		`- t.Fatal("cstringLiteral returned nil")`
	`58`	`+ t.Fatal("newCString returned nil")`
`59`	`59`	`}`
`60`	`60`	`}`
`61`	`61`
`62`		`-func TestCstringLiteralEmpty(t *testing.T) {`
`63`		`- ptr := cstringLiteral("")`
	`62`	`+func TestNewCStringEmpty(t *testing.T) {`
	`63`	`+ ptr := newCString("")`
`64`	`64`	`if ptr == nil {`
`65`		`- t.Fatal("cstringLiteral returned nil for empty string")`
	`65`	`+ t.Fatal("newCString returned nil for empty string")`
`66`	`66`	`}`
`67`	`67`	`}`
`68`	`68`