Feedback for records: Accessors name() vs. getName()

[Brian Goetz]

The method PropertyDescriptor::getWriteMethod is allowed to return 
`null` for read-only properties.  Similarly, the base class 
SimpleBeanInfo basically says "nope" to all the questions -- which 
suggests that most of the bean meta-properties (default event, default 
property, event sets, properties, icon, etc) are intended to be 
optional. Introspector says further that the analysis for properties, 
events, and methods are independently performed.

The bigger question here is, then, whether we should interpret "Bean" 
strictly as the JavaBean Spec (1997) says, for purposes of extending 
Introspector:

     “A Java Bean is a reusable software component that can be 
manipulated visually
     in a builder tool.”

or as people would like to use it (POJO with getters and setters using 
the JavaBean naming convention.)  Clearly, no one has used JavaBean to 
mean what the spec has said for many years, but the naming conventions 
have lived on.

Personally, your approach looks good to me.  The implementation, as you 
observe, is pretty trivial.  More important would be the spec, to make 
it clear what we're doing.  It might be worthwhile to clarify the spec 
to say that Introspector does not test every requirement of being a Java 
Bean, but still makes a good-faith effort to try and extract things like 
property names, and then specifically say what it does for records.



On 8/5/2020 11:08 AM, Johannes Kuhn wrote:
> On 05-Aug-20 15:41, Brian Goetz wrote:
>>> 1. java.beans.Introspector not recognizing accessors
>>
>> This one was raised on this list before; there's no reason 
>> Introspector can't learn about records -- all the reflective 
>> machinery is there to do so.  Someone at one point offered to 
>> contribute this change, but so far I haven't seen it.  We would be 
>> happy to take back such a patch.
>>
> While I was not the one who did propose such a thing, I just 
> implemented that[1].
>
> It is based on Java 14 and used via --patch-module. (My IDE somewhat 
> supports developing with --patch-module, so this was the easy path.)
>
> It was easier than anticipated. But before discussing that, I want to 
> address the elephant in the room:
>
> **Records are not JavaBeans**.
> The JavaBeans spec requires that a bean has a public no-args constructor.
> After creating the object, it's state is then set using the various 
> setters.
>
> Records on the other hand are immutable - their contents need to be 
> known at creation. (duh)
> While that patch exposes the record components as properties, it can 
> not be used to gain compatibility w.r.t. creation or modification.
>
> And before I put more work into that (porting it to tip, writing 
> tests, get rid of spaces vs tabs...), I want to be sure that this kind 
> of risk is acceptable.
>
> ---
>
> About the patch:
>
> For Introspector it needs to be in the JDK itself - if it should work 
> for all records.
> The Introspector has a search path to find additonal BeanInfo for a 
> given class.
> Adding a RecordBeanInfo to the default search path is not a working 
> approach as the record class is not passed to the BeanInfo.
> Therefore I had decided to add that hook in the BeanInfoFinder class.
>
> Conceptually, it looks like there is an explicit BeanInfo for every 
> record class on the default BeanInfo search path.
>
> - Johannes
>
> [1]: https://gist.github.com/DasBrain/71c7318c911df307e43f4961a46dc56b
>