More progress, now with `byte` coercions

5 files changed, 177 insertions, 99 deletions

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java
index 5cf873927..efe015368 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/NumberDeserializers.java
@@ -254,7 +254,7 @@ public class NumberDeserializers
 
         final static ByteDeserializer primitiveInstance = new ByteDeserializer(Byte.TYPE, (byte) 0);
         final static ByteDeserializer wrapperInstance = new ByteDeserializer(Byte.class, null);
-        
+
         public ByteDeserializer(Class<Byte> cls, Byte nvl)
         {
             super(cls, LogicalType.Integer, nvl, (byte) 0);
@@ -266,58 +266,7 @@ public class NumberDeserializers
             if (p.hasToken(JsonToken.VALUE_NUMBER_INT)) {
                 return p.getByteValue();
             }
-            return _parseByte(p, ctxt);
-        }
-
-        protected Byte _parseByte(JsonParser p, DeserializationContext ctxt) throws IOException
-        {
-            JsonToken t = p.currentToken();
-            if (t == JsonToken.VALUE_STRING) { // let's do implicit re-parse
-                String text = p.getText();
-                CoercionAction act = _checkFromStringCoercion(ctxt, text);
-                if (act == CoercionAction.AsNull) {
-                    return (Byte) getNullValue(ctxt);
-                }
-                if (act == CoercionAction.AsEmpty) {
-                    return (Byte) getEmptyValue(ctxt);
-                }
-                text = text.trim();
-                if (_hasTextualNull(text)) {
-                    return (Byte) _coerceTextualNull(ctxt, _primitive);
-                }
-                int value;
-                try {
-                    value = NumberInput.parseInt(text);
-                } catch (IllegalArgumentException iae) {
-                    return (Byte) ctxt.handleWeirdStringValue(_valueClass, text,
-                            "not a valid Byte value");
-                }
-                // So far so good: but does it fit?
-                // as per [JACKSON-804], allow range up to 255, inclusive
-                if (_byteOverflow(value)) {
-                    return (Byte) ctxt.handleWeirdStringValue(_valueClass, text,
-                            "overflow, value cannot be represented as 8-bit value");
-                    // fall-through for deferred fails
-                }
-                return Byte.valueOf((byte) value);
-            }
-            if (t == JsonToken.VALUE_NUMBER_FLOAT) {
-                if (!ctxt.isEnabled(DeserializationFeature.ACCEPT_FLOAT_AS_INT)) {
-                    _failDoubleToIntCoercion(p, ctxt, "Byte");
-                }
-                return p.getByteValue();
-            }
-            if (t == JsonToken.VALUE_NULL) {
-                return (Byte) _coerceNullToken(ctxt, _primitive);
-            }
-            // [databind#381]
-            if (t == JsonToken.START_ARRAY) {
-                return _deserializeFromArray(p, ctxt);
-            }
-            if (t == JsonToken.VALUE_NUMBER_INT) { // shouldn't usually be called with it but
-                return p.getByteValue();
-            }
-            return (Byte) ctxt.handleUnexpectedToken(_valueClass, p);
+            return _parseByte(p, ctxt, _valueClass);
         }
     }
 
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java
index 35dc22c76..20c87d35b 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java
@@ -508,7 +508,7 @@ public abstract class PrimitiveArrayDeserializers<T> extends StdDeserializer<T>
                             _verifyNullForPrimitive(ctxt);
                             value = (byte) 0;
                         } else {
-                            value = _parseBytePrimitive(p, ctxt);
+                            value = _parseBytePrimitive(ctxt, p, handledType());
                         }
                     }
                     if (ix >= chunk.length) {
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java
index 678e516f2..7f15e81a5 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java
@@ -7,6 +7,7 @@ import com.fasterxml.jackson.annotation.JsonFormat;
 import com.fasterxml.jackson.annotation.Nulls;
 
 import com.fasterxml.jackson.core.*;
+import com.fasterxml.jackson.core.JsonParser.NumberType;
 import com.fasterxml.jackson.core.exc.InputCoercionException;
 import com.fasterxml.jackson.core.io.NumberInput;
 
@@ -369,7 +370,8 @@ public abstract class StdDeserializer<T>
             JsonParser p, Class<?> targetType)
         throws IOException
     {
-        JsonToken t = p.currentToken();
+        final JsonToken t = p.currentToken();
+        // usually caller should have handled but:
         if (t == JsonToken.VALUE_TRUE) return true;
         if (t == JsonToken.VALUE_FALSE) return false;
         if (t == JsonToken.VALUE_NULL) {
@@ -393,7 +395,7 @@ public abstract class StdDeserializer<T>
                 return false;
             }
             if (act == CoercionAction.AsEmpty) {
-                return (Boolean) getEmptyValue(ctxt);
+                return false;
             }
             text = text.trim();
             // [databind#422]: Allow aliases
@@ -411,14 +413,13 @@ public abstract class StdDeserializer<T>
                     "only \"true\" or \"false\" recognized");
             return Boolean.TRUE.equals(b);
         }
-        // [databind#381]
+        // 12-Jun-2020, tatu: For some reason calling `_deserializeFromArray()` won't work so:
         if (t == JsonToken.START_ARRAY && ctxt.isEnabled(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS)) {
             p.nextToken();
             final boolean parsed = _parseBooleanPrimitive(ctxt, p, targetType);
             _verifyEndArrayForSingle(p, ctxt);
             return parsed;
         }
-        // Otherwise, no can do:
         return ((Boolean) ctxt.handleUnexpectedToken(targetType, p)).booleanValue();
     }
 
@@ -426,7 +427,10 @@ public abstract class StdDeserializer<T>
             JsonParser p, Class<?> targetType)
         throws IOException
     {
-        JsonToken t = p.currentToken();
+        final JsonToken t = p.currentToken();
+        // usually caller should have handled but:
+        if (t == JsonToken.VALUE_TRUE) return true;
+        if (t == JsonToken.VALUE_FALSE) return false;
         if (t == JsonToken.VALUE_NULL) {
             return (Boolean) _coerceNullToken(ctxt, false);
         }
@@ -459,14 +463,7 @@ public abstract class StdDeserializer<T>
             return (Boolean) ctxt.handleWeirdStringValue(_valueClass, text,
                     "only \"true\" or \"false\" recognized");
         }
-        // usually caller should have handled but:
-        if (t == JsonToken.VALUE_TRUE) {
-            return Boolean.TRUE;
-        }
-        if (t == JsonToken.VALUE_FALSE) {
-            return Boolean.FALSE;
-        }
-        if (t == JsonToken.START_ARRAY) { // unwrapping?
+        if (t == JsonToken.START_ARRAY) { // unwrapping / from-empty-array coercion?
             return (Boolean) _deserializeFromArray(p, ctxt);
         }
         // Otherwise, no can do:
@@ -487,22 +484,129 @@ public abstract class StdDeserializer<T>
         return !"0".equals(p.getText());
     }
 
-    protected final byte _parseBytePrimitive(JsonParser p, DeserializationContext ctxt)
+    protected final byte _parseBytePrimitive(DeserializationContext ctxt, JsonParser p,
+            Class<?> targetType)
         throws IOException
     {
-        int value = _parseIntPrimitive(p, ctxt);
-        // So far so good: but does it fit?
-        if (_byteOverflow(value)) {
-            Number v = (Number) ctxt.handleWeirdStringValue(_valueClass, String.valueOf(value),
-                    "overflow, value cannot be represented as 8-bit value");
-            return _nonNullNumber(v).byteValue();
+        final JsonToken t = p.currentToken();
+        if (t == JsonToken.VALUE_NUMBER_INT) return p.getByteValue();
+        if (t == JsonToken.VALUE_NULL) {
+            return (Byte) _coerceNullToken(ctxt, true);
+        }
+        if (t == JsonToken.VALUE_STRING) { // let's do implicit re-parse
+            String text = p.getText();
+            CoercionAction act = _checkFromStringCoercion(ctxt, text,
+                    LogicalType.Integer, targetType);
+            if (act == CoercionAction.AsNull) {
+//                _verifyNullForPrimitiveCoercion(ctxt, text);
+                return (byte) 0; // no need to check as does not come from `null`, explicit coercion
+            }
+            if (act == CoercionAction.AsEmpty) {
+                return (byte) 0;
+            }
+            text = text.trim();
+            if (_hasTextualNull(text)) {
+                _verifyNullForPrimitiveCoercion(ctxt, text);
+                return (byte) 0;
+            }
+            int value;
+            try {
+                value = NumberInput.parseInt(text);
+            } catch (IllegalArgumentException iae) {
+                return (Byte) ctxt.handleWeirdStringValue(_valueClass, text,
+                        "not a valid Byte value");
+            }
+            // So far so good: but does it fit?
+            // as per [JACKSON-804], allow range up to 255, inclusive
+            if (_byteOverflow(value)) {
+                return (Byte) ctxt.handleWeirdStringValue(_valueClass, text,
+                        "overflow, value cannot be represented as 8-bit value");
+                // fall-through for deferred fails
+            }
+            return (byte) value;
+        }
+        if (t == JsonToken.VALUE_NUMBER_FLOAT) {
+            CoercionAction act = _checkFloatToIntCoercion(ctxt, p, targetType);
+            if (act == CoercionAction.AsNull) {
+                return (byte) 0;
+            }
+            if (act == CoercionAction.AsEmpty) {
+                return (byte) 0;
+            }
+            return p.getByteValue();
+        }
+        // 12-Jun-2020, tatu: For some reason calling `_deserializeFromArray()` won't work so:
+        if (t == JsonToken.START_ARRAY && ctxt.isEnabled(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS)) {
+            p.nextToken();
+            final byte parsed = _parseBytePrimitive(ctxt, p, targetType);
+            _verifyEndArrayForSingle(p, ctxt);
+            return parsed;
+        }
+        return ((Byte) ctxt.handleUnexpectedToken(targetType, p)).byteValue();
+    }
+
+    protected Byte _parseByte(JsonParser p, DeserializationContext ctxt,
+            Class<?> targetType) throws IOException
+    {
+        final JsonToken t = p.currentToken();
+        if (t == JsonToken.VALUE_NUMBER_INT) return p.getByteValue();
+        if (t == JsonToken.VALUE_NULL) {
+            return (Byte) _coerceNullToken(ctxt, false);
+        }
+        if (t == JsonToken.VALUE_STRING) { // let's do implicit re-parse
+            String text = p.getText();
+            CoercionAction act = _checkFromStringCoercion(ctxt, text,
+                    LogicalType.Integer, targetType);
+            if (act == CoercionAction.AsNull) {
+                return (Byte) getNullValue(ctxt);
+            }
+            if (act == CoercionAction.AsEmpty) {
+                return (Byte) getEmptyValue(ctxt);
+            }
+            text = text.trim();
+            if (_hasTextualNull(text)) {
+                return (Byte) _coerceTextualNull(ctxt, false);
+            }
+            int value;
+            try {
+                value = NumberInput.parseInt(text);
+            } catch (IllegalArgumentException iae) {
+                return (Byte) ctxt.handleWeirdStringValue(_valueClass, text,
+                        "not a valid Byte value");
+            }
+            // So far so good: but does it fit?
+            // as per [JACKSON-804], allow range up to 255, inclusive
+            if (_byteOverflow(value)) {
+                return (Byte) ctxt.handleWeirdStringValue(_valueClass, text,
+                        "overflow, value cannot be represented as 8-bit value");
+                // fall-through for deferred fails
+            }
+            return Byte.valueOf((byte) value);
         }
-        return (byte) value;
+        if (t == JsonToken.VALUE_NUMBER_FLOAT) {
+            CoercionAction act = _checkFloatToIntCoercion(ctxt, p, targetType);
+            if (act == CoercionAction.AsNull) {
+                return (Byte) getNullValue(ctxt);
+            }
+            if (act == CoercionAction.AsEmpty) {
+                return (Byte) getEmptyValue(ctxt);
+            }
+            return p.getByteValue();
+        }
+        if (t == JsonToken.START_ARRAY) { // [databind#381]
+            return (Byte) _deserializeFromArray(p, ctxt);
+        }
+        return (Byte) ctxt.handleUnexpectedToken(_valueClass, p);
     }
 
     protected final short _parseShortPrimitive(JsonParser p, DeserializationContext ctxt)
         throws IOException
     {
+        final JsonToken t = p.currentToken();
+        if (t == JsonToken.VALUE_NUMBER_INT) return p.getShortValue();
+        if (t == JsonToken.VALUE_NULL) {
+            return (Byte) _coerceNullToken(ctxt, true);
+        }
         int value = _parseIntPrimitive(p, ctxt);
         // So far so good: but does it fit?
         if (_shortOverflow(value)) {
@@ -942,23 +1046,16 @@ public abstract class StdDeserializer<T>
         if (value.length() == 0) {
             act = ctxt.findCoercionAction(logicalType, rawTargetType,
                     CoercionInputShape.EmptyString);
-            if (act == CoercionAction.Fail) {
-                ctxt.reportInputMismatch(this,
-"Cannot coerce empty String (\"\") to %s (but could if enabling coercion using `CoercionConfig`)",
-_coercedTypeDesc());
-            }
+            return _checkCoercionActionFail(ctxt, act, "empty String (\"\")");
         } else if (_isBlank(value)) {
             act = ctxt.findCoercionFromBlankString(logicalType, rawTargetType, CoercionAction.Fail);
-            if (act == CoercionAction.Fail) {
-                ctxt.reportInputMismatch(this,
-"Cannot coerce blank String (all whitespace) to %s (but could if enabling coercion using `CoercionConfig`)",
-_coercedTypeDesc());
-            }
+            return _checkCoercionActionFail(ctxt, act, "blank String (all whitespace)");
         } else {
             act = ctxt.findCoercionAction(logicalType, rawTargetType, CoercionInputShape.String);
             if (act == CoercionAction.Fail) {
+                // since it MIGHT (but might not), create desc here, do not use helper
                 ctxt.reportInputMismatch(this,
-"Cannot coerce String value (\"%s\") to %s (but might if enabling coercion using `CoercionConfig`)",
+"Cannot coerce String value (\"%s\") to %s (but might if coercion using `CoercionConfig` was enabled)",
 value, _coercedTypeDesc());
             }
         }
@@ -968,17 +1065,30 @@ value, _coercedTypeDesc());
     /**
      * @since 2.12
      */
-    protected Boolean _coerceBooleanFromInt(DeserializationContext ctxt, JsonParser p,
+    protected CoercionAction _checkFloatToIntCoercion(DeserializationContext ctxt, JsonParser p,
             Class<?> rawTargetType)
         throws IOException
     {
+        final CoercionAction act = ctxt.findCoercionAction(LogicalType.Integer,
+                rawTargetType, CoercionInputShape.Float);
+        if (act == CoercionAction.Fail) {
+            _checkCoercionActionFail(ctxt, act, "Floating-point value ("+p.getText()+")");
+        }
+        return act;
+    }
 
-        final CoercionAction act = ctxt.findCoercionAction(LogicalType.Boolean, rawTargetType, CoercionInputShape.Integer);
+    /**
+     * @since 2.12
+     */
+    protected Boolean _coerceBooleanFromInt(DeserializationContext ctxt, JsonParser p,
+            Class<?> rawTargetType)
+        throws IOException
+    {
+        CoercionAction act = ctxt.findCoercionAction(LogicalType.Boolean, rawTargetType, CoercionInputShape.Integer);
         switch (act) {
         case Fail:
-            ctxt.reportInputMismatch(this,
-"Cannot coerce Integer value (%s) to %s (but might if enabling coercion using `CoercionConfig`)",
-p.getText(), _coercedTypeDesc());
+            _checkCoercionActionFail(ctxt, act, "Integer value ("+p.getText()+")");
+            break;
         case AsNull:
             return null;
         case AsEmpty:
@@ -988,8 +1098,22 @@ p.getText(), _coercedTypeDesc());
         // 13-Oct-2016, tatu: As per [databind#1324], need to be careful wrt
         //    degenerate case of huge integers, legal in JSON.
         //    Also note that number tokens can not have WS to trim:
-        boolean b = !"0".equals(p.getText());
-        return b;
+        if (p.getNumberType() == NumberType.INT) {
+            // but minor optimization for common case is possible:
+            return p.getIntValue() != 0;
+        }
+        return !"0".equals(p.getText());
+    }
+
+    protected CoercionAction _checkCoercionActionFail(DeserializationContext ctxt,
+            CoercionAction act, String inputDesc) throws IOException
+    {
+        if (act == CoercionAction.Fail) {
+            ctxt.reportInputMismatch(this,
+"Cannot coerce %s to %s (but could if coercion was enabled using `CoercionConfig`)",
+inputDesc, _coercedTypeDesc());
+        }
+        return act;
     }
 
     /*
diff --git a/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java b/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java
index fbc4751bb..33117d4aa 100644
--- a/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java
+++ b/src/test/java/com/fasterxml/jackson/databind/convert/TestArrayConversions.java
@@ -12,7 +12,7 @@ public class TestArrayConversions
     extends com.fasterxml.jackson.databind.BaseMapTest
 {
     final static String OVERFLOW_MSG_BYTE = "out of range of Java byte";
-    final static String OVERFLOW_MSG = "overflow";
+    final static String OVERFLOW_MSG_SHORT = "out of range of Java short";
 
     final static String OVERFLOW_MSG_INT = "out of range of int";
     final static String OVERFLOW_MSG_LONG = "out of range of long";
@@ -103,7 +103,7 @@ public class TestArrayConversions
         try {
             MAPPER.convertValue(new int[] { -99999 }, short[].class);
         } catch (IllegalArgumentException e) {
-            verifyException(e, OVERFLOW_MSG);
+            verifyException(e, OVERFLOW_MSG_SHORT);
         }
         // Int overflow
         try {
diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java
index b5b049872..0d334f2e1 100644
--- a/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java
+++ b/src/test/java/com/fasterxml/jackson/databind/deser/jdk/JDKScalarsTest.java
@@ -118,7 +118,11 @@ public class JDKScalarsTest
         public Void value;
     }
 
-    private final ObjectMapper MAPPER = new ObjectMapper();
+    private final ObjectMapper MAPPER = newJsonMapper();
+
+    final ObjectMapper MAPPER_NO_COERCION =jsonMapperBuilder()
+            .disable(MapperFeature.ALLOW_COERCION_OF_SCALARS)
+            .build();
 
     /*
     /**********************************************************
@@ -854,14 +858,14 @@ public class JDKScalarsTest
         final String JSON_WITH_NULL = "[ null ]";
         final String SIMPLE_NAME = "`"+cls.getSimpleName()+"`";
         final ObjectReader readerCoerceOk = MAPPER.readerFor(cls);
-        final ObjectReader readerNoCoerce = readerCoerceOk
+        final ObjectReader readerNoNulls = readerCoerceOk
                 .with(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES);
 
         Object ob = readerCoerceOk.forType(cls).readValue(JSON_WITH_NULL);
         assertEquals(1, Array.getLength(ob));
         assertEquals(defValue, Array.get(ob, 0));
         try {
-            readerNoCoerce.readValue(JSON_WITH_NULL);
+            readerNoNulls.readValue(JSON_WITH_NULL);
             fail("Should not pass");
         } catch (JsonMappingException e) {
             verifyException(e, "Cannot coerce `null`");
@@ -873,8 +877,9 @@ public class JDKScalarsTest
             assertEquals(1, Array.getLength(ob));
             assertEquals(defValue, Array.get(ob, 0));
 
+            final ObjectReader readerNoEmpty = MAPPER_NO_COERCION.readerFor(cls);
             try {
-                readerNoCoerce.readValue(EMPTY_STRING_JSON);
+                readerNoEmpty.readValue(EMPTY_STRING_JSON);
                 fail("Should not pass");
             } catch (JsonMappingException e) {
                 // 07-Jun-2020, tatu: during transition, two acceptable alternatives