> Hm... I have to think about what future requirements we could run
> into, and if therefore it's actually a better idea to go the other way
> around, that is, to address the existence of Java Bean properties
> inside ZeroArgumentNonVoidMethodPolicy (i.e., include BEAN_PROPS
> this-and-that in the enum names). 

I'm also still thinking things through, including 
BeansWrapper.EXPOSE_PROPERTIES_ONLY

> .... the main issue with this
> new setback in the Java language design is exactly that I can only
> tell that it's a zero argument method that returns non-void, but I
> don't know if it's an accessor, or it sends out mails to all users if
> I call it, or what. (When these started with "get"/"is", at least you
> had a strong guess.)

Good point: there's no language difference between e.g.
an adjective/state method "active" and a verb/operation method "activate".
(Well, there are components in records, but nothing to distinguish things in 
normal classes.)

> Fixed it, and thanks for spotting this!

Great! Thank you very much.


Cheers,
Simon



On Wednesday, 3 April 2024 at 17:03:54 BST, Daniel Dekany 
<daniel.dek...@gmail.com> wrote: 





> NonBeanAccessorsPolicy.XXX (as opposed to ZeroArgumentNonVoidMethodPolicy.XXX)

Hm... I have to think about what future requirements we could run
into, and if therefore it's actually a better idea to go the other way
around, that is, to address the existence of Java Bean properties
inside ZeroArgumentNonVoidMethodPolicy (i.e., include BEAN_PROPS
this-and-that in the enum names). And also, the main issue with this
new setback in the Java language design is exactly that I can only
tell that it's a zero argument method that returns non-void, but I
don't know if it's an accessor, or it sends out mails to all users if
I call it, or what. (When these started with "get"/"is", at least you
had a strong guess.)

> For 2.3.32:
> * non-bean class accessors could only be methods
> * bean class accessors could be methods and properties

Some mostly terminology related clarifications for this. The
FreeMarker template language doesn't have anything called properties.
The Java Bean specification talks about properties, and hence
BeansWrapper (and hence DefaultObjectWrapper, which is its subclass).
In tempataes, Java Bean properties are only exposed as sub-variables
(like foo.bar), not as methods with that(!) name (i.e. "bar"). That
the Java Beans property read method (foo.getBar(), or foo.isBar()) is
also exposed is unrelated to the fact that it's a Java Bean property
read method. It's because Java Beans expose them as "actions" (I don't
know why, but that what they did). The name of these actions (usually)
differs from the property name ("foo" VS "getFoo").

> * non-bean record accessors could only be methods

By default, yes. Though the MethodAppearanceFineTuner was always pluggable.

> * bean record accessors are generally a mistake

Actually, they are probably often there on purpose, since existing
libraries expect "properties" exposed via getXxx/isXxx methods.
Because they decided to break this pattern (instead of doing what
people have been asking for since forever), now we will sometimes end
up with hacks like @lombok.Getter public record MyRecord(String bar,
boolean accessible) { }. In this case, myRecrod.bar() and
myRecord.getBar() are both accessor methods to get the value of "bar",
and there was no intention to create a "getBar"
property/sub-variable/component/attribute/whatever-we-call-it.

> With the current implementation, both of the following cause an error when 
> the "active" property is used!
> record MyRecord(boolean isActive) {
>    public boolean active() {

Maybe people who do that deserve their fate. :) So, if you have no
active() there, then myRecord.active gives a boolean, because that's
just an accidental Java Bean property in this case, and then
myRecord.isActive will not give a boolean (but a method), since that
method now was taken over by the Java Beans logic. Not great, but
maybe not too bad. But, if you add MyRecord.active() to that class as
well, then that's a Java Bean "action" (the term for things meant to
be exposed as methods), and by default methods shadow properties with
clashing names (although that's configurable via
BeansWrapper.methodsShadowItems).

> record MyRecord(boolean active) {
>  public boolean isActive() {

That I expect to be a typical use-case, because of the backward
compatibility hacks needed sometimes. I missed that case in the JUnit
tests, and as the usual punishment for that, of course it was broken.
Fixed it, and thanks for spotting this!
https://github.com/apache/freemarker/commit/9cc2b9cb6d1196fffa010430920331a36e7cbf49
Also deployed to the snapshot repo
(https://repository.apache.org/content/repositories/snapshots/org/freemarker/freemarker/2.3.33-SNAPSHOT/).

On Mon, Apr 1, 2024 at 6:49 PM Simon Hartley
<scrhart...@yahoo.co.uk.invalid> wrote:
>
> In terms of language, for a record's read methods, Java calls them accessors.
> For beans, I'm used to the term getter, but when expanding to also describe 
> how boolean return methods are handled,
> the term accessor is again used. It seems that zero-argument non-void is 
> trying to be distinct from the term accessor,
> because that's what we believe is necessary for backwards compatibility and 
> compatibility with beans
> (and a project could have a mixture of bean and non-bean style classes).
>
> But trying to move the problem slightly, what do you think of the following 
> idea?
> NonBeanAccessorsPolicy.XXX (as opposed to ZeroArgumentNonVoidMethodPolicy.XXX)
> Note: I've used a plural here to try to highlight we're saying the methods 
> are non-bean, rather than the classes.
> So how does this help us when there's a conflict?
> We've implied that logic for bean accessors may be being applied as well; 
> this can be supplemented with documentation.
> Also not every future option should hang off ZeroArgumentNonVoidMethodPolicy,
> since our original intent was to describe non-bean related behavior.
>
>
> For 2.3.32:
> * non-bean class accessors could only be methods
> * bean class accessors could be methods and properties
> * non-bean record accessors could only be methods
> * bean record accessors are generally a mistake
>
> For 2.3.33:
> * non-bean class accessors were added to address use cases for both records 
> and classes.
> * we haven't introduced an option to turn-off bean accessors for records to 
> solve the problem for records
> * for some projects, an option to turn-off bean accessors for classes may be 
> useful,
>  but some projects would want a mixture
> * where there is a mixture here are some approaches:
>  - prefer exposing bean properties (more likely to be backwards compatible?)
>  - prefer exposing non-bean properties (would work better with records?)
>  - error (force the user to resolve ambiguity by either using a method 
>invocation or changing their Java code)
>  - expose both (collisions?)
>
>
> Property collisions
> ---------------
>
> What happens when a record has fields/components of both active and isActive?
> record Evil(boolean active, boolean isActive) {}
>
> With the current implementation, both of the following cause an error when 
> the "active" property is used!
> record MyRecord(boolean isActive) {
>    public boolean active() {
>        return false;
>    }
>    // implicit method isActive()
> }
> record MyRecord(boolean active) {
>    public boolean isActive() {
>        return false;
>    }
>    // implicit method active()
> }
>
> So collisions are already there, not a potential risk.
>
> Simon
>
>
>
>
>
> On Monday, 1 April 2024 at 12:35:44 BST, Daniel Dekany 
> <daniel.dek...@gmail.com> wrote:
>
>
>
>
>
> I think we need the current behavior for the
> almost-backward-compatible behavior that incompatibleImprovements
> 2.3.33 promises. (Although it's "incompatible", there's a promise that
> as far as it's 2.3.x, it only enables changes that are most likely
> won't break your application, hence when someone discovers that they
> need some of the improvements it brings, they can usually crank it up,
> and get all the other improvements too, instead of people having some
> unique permutation of "improvements" enabled.) So maybe
> ZeroArgumentNonVoidMethodPolicy.PROPERTY_ONLY should be called
> PROPERTY_UNLESS_BEAN_PROP_READ_METHOD (oh my...), and
> BOTH_PROPERTY_AND_METHOD should called
> PROPERTY_UNLESS_BEAN_PROP_READ_METHOD_AND_METHOD (even funnier name).
> And then later we can introduce other policies if needed, which are
> less backward compatible, but might be seen cleaner or more obvious.
>
> On Sun, Mar 31, 2024 at 4:49 PM Simon Hartley
> <scrhart...@yahoo.co.uk.invalid> wrote:
> >
> > If I was trying to do something here's an idea or two
> > (please don't feel any pressure to implement this, I'm just thinking out 
> > loud):
> >
> > How properties are implemented is from two sources: beans and simple 
> > non-voids.
> > When the logic of both is being applied, then it's not a surprise that 
> > there are some strange interactions.
> > If I could enable them separately, then I could turn off bean properties on 
> > records and have it work as I hoped.
> >
> > Otherwise perhaps I would need to have a backwards compatibility mode which 
> > is only enabled when using the default values.
> > Or perhaps a setting which said whether beans or simple non-voids take 
> > precedence.
> >
> > I'm more than happy with the status quo; a few rough edges aren't the end 
> > of the world.
> > I very much appreciate the effort you've already put in and what's been 
> > done is a huge improvement.
> >
> > Thank you so much,
> > Simon
> >
> >
> >
> >
> > On Sunday, 31 March 2024 at 02:05:32 BST, Daniel Dekany 
> > <daniel.dek...@gmail.com> wrote:
> >
> >
> >
> >
> >
> > Yeah, it is confusing/complicated if somebody digs into it. So you
> > have `boolean isAccessible()` in a record. Because it's in a record,
> > it's a good question what the intent of the author was (plus, now that
> > the Java language makers make a mess, people will start using the same
> > pattern in all kind of classes, randomly mixed with old-style
> > getters). Like maybe it looks as a Java Beans property read method
> > only accidentally, and they want this property-like thing to be called
> > "isAccessible". But I think the most likely, by far, is that the
> > authors are just confused about what the convention now is. Also,
> > Java's java.beans.Introspector tells us that that's what it is: a Java
> > Beans property called "accessible". FreeMarker doesn't try to guess if
> > from the name. Also, this happens to be the most backward compatible
> > behavior too.
> >
> > The end result is as below.
> >
> > These will work:
> > myRecord.accessible - boolean value, because it's a Java Beans
> > property called "accessible"
> > myRecord.isAccessible() - which returns boolean, because a such a
> > public method exists in Java, and therefore people expect it to work
> > like in Java.
> >
> > This doesn't work:
> > myRecord.accessible() - because there's no such method in Java, so it
> > would be quite absurd
> > myRecord.isAccessible - is not a boolean value, but a method, because
> > that's how it always was with JavaBean property read methods.
> >
> > And then, you say, maybe myRecord.isAccessible should also be a
> > boolean? I don't know... It's more backward incompatibility risk for
> > sure.
> >
> > Also, note that if something is related to JavaBean properties, the
> > ZeroArgumentNonVoidMethodPolicy doesn't apply. We decided that it's a
> > JavaBean property read method, and end of story.
> >
> > That the ZeroArgumentNonVoidMethodPolicy name doesn't convey the exact
> > meaning is true. Not sure what a better name could be though, that's
> > not comically long.
> >
> > On Sat, Mar 30, 2024 at 1:11 AM Simon Hartley
> > <scrhart...@yahoo.co.uk.invalid> wrote:
> > >
> > > Similarly, for a record with a field (component) called isActive,
> > > its property name is currently "active" and not "isActive".
> > > Additionally, this exposed property is callable as a method even when the 
> > > setting is PROPERTY_ONLY,
> > > due to it being considered a bean property and not a record property.
> > >
> > > Is this the compromise we're making and something we'll live with,
> > > or should we add more complexity so that this doesn't always apply?
> > >
> > >
> > >
> > > On Friday, 29 March 2024 at 23:29:31 GMT, Simon Hartley 
> > > <scrhart...@yahoo.co.uk.invalid> wrote:
> > >
> > >
> > >
> > > I've worked out what my confusion was:
> > >
> > > I was setting it to METHOD_ONLY and then becoming confused that the bean 
> > > get method still worked as a property.
> > > Of course, this setting only affects zero-argument non-void methods which 
> > > ALSO aren't bean getter methods.
> > >
> > > I think that means that if a class was using non-bean style and has a 
> > > field called isActive,
> > > then if you create a method for it called isActive, then its property 
> > > name is "active", not "isActive"
> > > (even when set to BOTH_PROPERTY_AND_METHOD or PROPERTY_ONLY).
> > > i.e. It's not exposed as both "active" (from the bean property syntax) 
> > > and "isActive" (from the non-bean style).
> > >
> > > I hope that's right and all as intended!
> > >
> > > Cheers,
> > > Simon
> >
> >
> >
> >
> > --
> > Best regards,

>
> > Daniel Dekany
>
>
>
> --
> Best regards,
> Daniel Dekany



-- 
Best regards,
Daniel Dekany

Reply via email to