Fix dead code and side-effect bug in BeanPropertyWriter.toString()

[Copilot]

BeanPropertyWriter.toString() had two bugs: a dead null-check on _accessor (a final field, never null) that made the "virtual" branch unreachable, and a call to _accessor.toString() that inadvertently triggered lazy MethodHandle initialization — undesirable in a diagnostic method, especially before fixAccess() is called.

Changes

• Replace _accessor != null with _member == null — the correct semantic check for virtual properties (those with no backing field or method)
• Remove _accessor.toString() call — instead describe the member directly from _member (field name or method name) without touching the MethodHandle

// Before — dead code: _accessor is final and never null; _accessor.toString() triggers MethodHandle init
if (_accessor != null) {
    sb.append("via methodhandle ").append(_accessor);
} else {
    sb.append("virtual"); // unreachable
}

// After — correct virtual detection; no MethodHandle side-effect
if (_member == null) {
    sb.append("virtual");
} else if (_member instanceof AnnotatedField) {
    sb.append("field '").append(_member.getName()).append("'");
} else {
    sb.append("method '").append(_member.getName()).append("()'");
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• central.sonatype.com
  • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.13/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.13/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.13 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.13/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/jackson-databind/jackson-databind org.codehaus.plexus.classworlds.launcher.Launcher compile -q (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

Fix subtle bug in BeanPropertyWriter.toString()

Problem

In BeanPropertyWriter.toString(), there is a dead code null-check on _accessor and a potentially problematic call to _accessor.toString():

@Override
public String toString() {
    StringBuilder sb = new StringBuilder(40);
    sb.append("property '").append(getName()).append("' (");
    if (_accessor != null) {
        sb.append("via methodhandle ")
                .append(_accessor);
    } else {
        sb.append("virtual");
    }
    ...
}

There are two bugs here:

1. Dead code null-check: _accessor is declared as protected final GetterHolder _accessor = new GetterHolder(); — it is a final field initialized inline and can never be null. The if (_accessor != null) branch is dead code, and the else branch (sb.append("virtual")) is therefore unreachable. Virtual properties (where _member is null) are never correctly identified as "virtual" in the string output.

2. Unintended side-effect in a diagnostic method: _accessor.toString() delegates to UnreflectHandleSupplier.toString(), which calls get().toString() on the underlying MethodHandle. This triggers lazy initialization of the MethodHandle — which is undesirable in a diagnostic/debugging method, especially if fixAccess() has not yet been called, potentially causing IllegalAccessException or other unexpected behavior.

Fix

The toString() method should be updated to:

1. Remove the dead _accessor != null null-check.
2. Instead, check _member to determine whether the property is "virtual" (i.e., _member == null), which is the actual semantic distinction.
3. Avoid calling _accessor.toString() (which triggers MethodHandle initialization). Instead, describe the accessor based on _member type (e.g., field name or method signature) without touching the MethodHandle.

Location

File: src/main/java/tools/jackson/databind/ser/BeanPropertyWriter.java

The toString() method currently reads:

@Override
public String toString() {
    StringBuilder sb = new StringBuilder(40);
    sb.append("property '").append(getName()).append("' (");
    if (_accessor != null) {
        sb.append("via methodhandle ")
                .append(_accessor);
    } else {
        sb.append("virtual");
    }
    if (_serializer == null) {
        sb.append(", no static serializer");
    } else {
        sb.append(", static serializer of type "
                + _serializer.getClass().getName());
    }
    sb.append(')');
    return sb.toString();
}

It should be fixed to properly distinguish virtual properties (where _member == null) from real field/method-backed properties, and must not trigger MethodHandle initialization as a side-effect of being called.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: try again

Assistant: Here's a walkthrough of the key parts of BeanPropertyWriter.java and how they fit together:

---

BeanPropertyWriter — Key Concepts

1. Core Purpose

BeanPropertyWriter is the workhorse for serializing a single named property of a Java bean. It holds all configuration — name, type info, serializers, filtering rules — and drives the actual JSON output for that property.

---

2. GetterHolder — Lazy MethodHandle for Property Access

class GetterHolder extends UnreflectHandleSupplier {
    public GetterHolder() {
        super(methodType(Object.class, Object.class));
    }

    @Override
    protected MethodHandle unreflect() throws IllegalAccessException {
        if (_member instanceof AnnotatedField) {
            return MethodHandles.lookup().unreflectGetter((Field) _member.getMember());
        } else if (_member instanceof AnnotatedMethod method) {
            return MethodHandles.lookup().unreflect(method.getMember());
        } else {
            // 01-Dec-2014, tatu: Used to be illegal, but now explicitly allowed
            // for virtual props
            return null;
        }
    }
}

GetterHolder extends UnreflectHandleSupplier, which is a lazy, memoized, thread-safe wrapper for a MethodHandle:

public UnreflectHandleSupplier(MethodType asType) {
    this.asType = asType;
}

@Override
public MethodHandle get() {
    return delegate.get();
}

synchronized MethodHandle initialize() {
    if (!initialized) {
        MethodHandle mh;
        try {
            mh = postprocess(unreflect());
        } catch (IllegalAccessException e) {
            throw ClassUtil.sneakyThro...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 Send tasks to Copilot coding agent from [Slack](https://gh.io/cca-slack-docs) and [Teams](https://gh.io/cca-teams-docs) to turn conversations into code. Copilot posts an update in your thread when it's finished.

[github-actions]

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	81.34%	📉 -0.010%
Branches	74.57%	📉 -0.010%

Coverage data generated from JaCoCo test results

[github-actions]

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	81.34%	📉 -0.010%
Branches	74.57%	📉 -0.010%

Coverage data generated from JaCoCo test results