beanshell / beanshell

Beanshell scripting language
Apache License 2.0
819 stars 183 forks source link

Primitive 3.0 #687

Open nickl- opened 1 year ago

nickl- commented 1 year ago

Primitive 3.0

Note: This is a breaking update which cannot be helped, every effort was taken to try and minimise the impact but only so much is possible. Please go through the relevant points below and let us know if you agree with the decisions or not and lets discuss. Perhaps you can see benefits or problems which I did not.

Quick history:

As recently explained at https://github.com/beanshell/beanshell/issues/609#issuecomment-1366294050 we have a legacy, for debug reasons, of an exposed bsh.Primitive in the scripting environment. Making it accessible instead of the value.

Intended purpose:

A utility that retains the state and type of JAVA primitives to maintain a distinction from boxed number types, which would otherwise be impossible due to autoboxing. The only reason is to have primitives in the scripting environment so their value should be exposed and not the utility's.

Problem revealed:

The problem is revealed when trying to call a method on a variable the error reports that the method was not found on bsh.Primitive. see #609

// Error: Evaluation Error: Error in method invocation: Method setScale(int, RoundingMode) not found in class'bsh.Primitive'

This may not have been a problem in the past, so the impact may be limited, because there are no instance methods on primitives in JAVA. But ever since we've promoted BigInteger and BigDecimal to primitive status this has caused the math methods to be inaccessible.

Solution:

Instead of exposing the wrapped instance of type bsh.Primitive, which is incorrect, rather expose the actual value of primitive type, as is intended.

Impact:

As the type and value of primitive variables have now changed this will be a breaking change to any existing scripts which are expecting variables to be of type bsh.Primitive and those who invoke methods on primitive variables of this class type.

The following is a detailed update of the public instance methods and other usage, with explanations of the change and possible workarounds.

instanceof

Many legacy scripts may have relied on instanceof to report true for type bsh.Primitive, this has not been changed. Like before it will also report true as an instance of boxed type but reports false on primitive type (should this also return true?).

BeanShell 3.0.0-SNAPSHOT.2801
bsh % int i = 1;
--> $0 = 1I :int
bsh % i instanceof Primitive;
--> $1 = true :boolean
bsh % i instanceof Integer;
--> $2 = true :boolean
bsh % i instanceof int;
--> $3 = false :boolean

getType

Many legacy scripts may have relied on the public method getType to assess the primitive type of a variable. To reduce the breaking this functionality has been retained and along with getClass exposed on primitive variables. Both will return the same primitive type and the choice was taken to include getClass for when compairing to boxed types or other objects as getType is not available on all instances but getClass is.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % int i = 1;
--> $0 = 1I :int
bsh % i.getType();
--> $1 = int :Class
bsh % i.getType() == i.getClass();
--> $2 = true :boolean
bsh % i.getClass() == Integer.TYPE;
--> $3 = true :boolean
bsh % i.getClass() == Integer.class;
--> $4 = false :boolean

getClass

Any legacy script which used this method would expect the value to be equal to bsh.Primitive.class, this will be breaking. Instead these scripts should rather enquire the isPrimitive method to assertain if this is a primitive type.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % int i = 1;
--> $7 = 1I :int
bsh % i.getClass() == Primitive.class;
--> $8 = false :boolean
bsh % i.getClass().isPrimitive();
--> $9 = true :boolean

equals

The bsh.Primitive class has a public equals method which compares by value, this differs from the boxed types, and may have been relied upon by legacy scripts. In order to reduce the breaking this functionality has been retained and exposed on primitive variables.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % int i = 1;
--> $8 = 1I :int
bsh % i.equals(1o);
--> $9 = true :Boolean
bsh % i.equals(1s);
--> $0 = true :Boolean
bsh % i.equals(1l);
--> $1 = true :Boolean
bsh % i.equals(1w);
--> $2 = true :Boolean
bsh % i.equals(1.0f);
--> $3 = true :Boolean
bsh % i.equals(1.0d);
--> $4 = true :Boolean
bsh % i.equals(1.0w);
--> $5 = true :Boolean
bsh % Integer.valueOf(i).equals(1o);
--> $6 = false :boolean
bsh % Integer.valueOf(i).equals(1s);
--> $7 = false :boolean
bsh % Integer.valueOf(i).equals(1i);
--> $8 = true :boolean

getValue

Existing scripts may have been forced to use the public method getValue to gain access to the utility wrapper's obfuscated value, and will be breaking. This will be problematic since the method returns a boxed type, due to autoboxing, and not the primitive value it encloses. Because of this type conversion and due to the fact that the value itself is now being exposed, this method is not available anymore, and will be breaking to all scripts trying to use it. To fix this existing scripts can instead use the variable value itself.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % int i = 1;
--> $0 = 1I :int
bsh % i.getValue();
// Error: Evaluation Error: Error in method invocation: Method getValue() not found in class'int' : at Line: 26 : in file: <unknown file> : i .getValue ( )

bsh % i;
--> $1 = 1I :int

isNumber

Because there are primitive types other than numbers some legacy scripts may have used this method to determine whether a variable is a number type, and will be breaking. This is only nescessary before unwrapping a number see numberValue and since the value is already unwrapped this is not nescessary. Scripts that did use this method can instead query instanceof Number however this will exclude char types, which BeanShell also treats like numbers. For scripts that want exactly the same reply as from the isNumber method can use Types.isNumeric instead.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % int i = 1;
--> $1 = 1I :int
bsh % i instanceof Number;
--> $2 = true :boolean
bsh % Types.isNumeric(i);
--> $3 = true :boolean
bsh % c = 'a';
--> $4 = 'a' :char
bsh % c instanceof Number;
--> $5 = false :boolean
bsh % Types.isNumeric(c);
--> $6 = true :boolean

numberValue

This method is used to unwrap the value as a Number type but since the value is already unwrapped this functionality was deemed unnescessary, and will be breaking. For any scripts that did use this method can simply assign the value to a Number type, or alternatively cast the value to a Number type, as required.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % int i = 1;
--> $2 = 1I :int
bsh % Number in = i;
--> $3 = 1I :int
bsh % cin = (Number)i;
--> $4 = 1I :int
bsh % c = 'a';
--> $5 = 'a' :char
bsh % Number cn = c;
--> $6 = 97I :int
bsh % ccn = (Number)c;
--> $7 = 97I :int

intValue

Typically an artifact as a result of the exposure of the wrapper utility, which should never have been there, it has no internal BeanShell utility and has therefor been removed. This is not breaking because intValue, along with all the other typed value methods from the parent Number class is naturally exposed, as a result of exposing the value instead of the wrapper utility, with the help of some autoboxing magic of our own, only temporary for method exposure as the type returns to its primitive form after the call.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % c = 'a';
--> $4 = 'a' :char
bsh % c.intValue();
--> $9 = 97I :int
bsh % c.longValue();
--> $0 = 97L :long
bsh % c.doubleValue();
--> $1 = 97.0d :double

booleanValue

Typically an artifact as a result of the exposure of the wrapper utility, which should never have been there, it has no internal BeanShell utility and has therefor been removed. This is not breaking because booleanValue from the boxed type Boolean class is naturally exposed, as a result of exposing the value instead of the wrapper utility, with the help of some autoboxing magic of our own, only temporary for method exposure as the type returns to its primitive form after the call.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % b = true;
--> $0 = true :boolean
bsh % b.booleanValue();
--> $1 = true :boolean

Primitive magic math methods

Now that we have the wide numbers BigInteger and BigDecimal promoted to primitive status, and properly exposed as primitive values, with all their instance math methods accessible. It seemed unfair if all the other primitives does not also implement these methods. All the math methods from both BigDecimal and BigInteger visa versa, for both floating point types and fixed, has been normalised across the complete range of numeric types for BeanShell.

If a method was not found it will cast the value up to its wide type representative and try the method again for resolution. If the return type is of the math type used, important because a method might return other types like boolean, String, etc. depending on its utility, the return value gets cast down again to the exposed type it was called on. Making the math methods completely transparent to the scripting environment regardless of which numeric type was used.

Chaining math methods

As with the math type methods, anything which returns itself can be chained and works for all numeric types. There might be similar methods for fixed and floating point numbers and some may require additional arguments like a MathContext or RoundingMode. These classes are also imported by default to simplify usage as needed.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % s = 5s;
--> $0 = 5s :short
bsh % s = s.add(2).subtract(6).pow(2).divide(1).multiply(1).sqrt();
--> $1 = 1s :short
bsh % f = 5f;
--> $2 = 5.0f :float
bsh % f = f.add(2).subtract(6).pow(2).divide(1).multiply(1).sqrt(MathContext.DECIMAL32);
--> $3 = 1.0f :float
bsh % c = 'e';
--> $4 = 'e' :char
bsh % c = c.add(2).subtract(6).pow(2).divide(1).multiply(1).sqrt();
--> $5 = 'a' :char

All methods available

We can call floating point math methods on fixed numbers and visa versa, if the method is not available for one it will look for it on the other. However if the method is found for the specific type then it will error unless the correct parameter arguments are supplied.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % s.testBit(0);
--> $6 = true :boolean
bsh % f.testBit(0);
--> $7 = true :boolean
bsh % c.testBit(0);
--> $8 = true :boolean
bsh % s.movePointRight(1);
--> $9 = 10s :short
bsh % f.movePointRight(1);
--> $0 = 10.0f :float
bsh % c.movePointRight(1);
--> $1 = 'ϊ' :char

Comparing numbers

All numeric types can be compared to each other as the types are widened for a value comparison by referencing the compareTo method on the corresponding math classes, in the same way that the other math methods are exposed.

BeanShell 3.0.0-SNAPSHOT.2801
bsh % 10i.compareTo(10s);
--> $0 = 0I :int
bsh % 10l.compareTo(10.0f);
--> $1 = 0I :int
bsh % 'a'.compareTo(97);
--> $2 = 0I :int
bsh % 97o.compareTo('a');
--> $3 = 0I :int
opeongo commented 1 year ago

Please go through the relevant points below and let us know if you agree with the decisions or not

I had a quick review of your write up and a quick look at the commit. I agree with your analysis. bsh.Primitive should not have been exposed in the first place, it is an internal data structure. From my point of view I think it is safe to consider everything up until now regarding Primitive 3.0 as preview feature subject to change. If Primitive was exposed in BeanShell 2.* then I think others will have to make a case for why it should remain available, but probably there should be a high bar on this one.

Perhaps the magic primitive methods getType() and equals() should be marked as deprecated, so that older scripts will still run but further use is discouraged.

Interesting addition of the magic methods, although I don't think I will every use them.

jimjag commented 1 year ago

Agreed!

Sailsman63 commented 1 year ago

Just wondering how much, or if at all, design here should be influenced by JEPs 401 and 402? (Parts of project Valhalla, breaking down some of the distinctions between Java primitives and their autoboxed counterparts)

nickl- commented 1 year ago

Just wondering how much, or if at all, design here should be influenced by JEPs 401 and 402? (Parts of project Valhalla, breaking down some of the distinctions between Java primitives and their autoboxed counterparts)

I made a very quick scan of the two JEPs referenced and I don't see anything of concern, instead it seems things will only improve. Remember that internally, due to the nature of JAVA, all JAVA primitives are boxed, since BeanShell naturally uses Object for all values, regardless of type. All that is happening here is the representation of primitives within the scripting engine to appear to be primitive, even though they really aren't. The major change referred to here is that previously the wrapper utility class bsh.Primitive was exposed, which should not be visible externally, instead of the unwrapped values themselves.

It is interesting to know where JAVA is going but BeanShell is still so far behind that actually doing anything about JEPs makes very little sense while we are playing catch up. Even if we were up to date I speculate, very wishful thinking =), any compatibility updates will still remain after the fact, only when JEPs have been applied to a new version, and not before.

Hope this addresses your concerns..

@Sailsman63 Thank you for your input!

Sailsman63 commented 1 year ago

^ That does make sense.