Skip to content

Commit

Permalink
Improve parsing of numeric default values
Browse files Browse the repository at this point in the history
Previously, all integral numbers were parsed as integers. This
caused two problems:

1. Compilation would fail if the default value for a long wasn't a
   valid integer.
2. The default value for a byte or short could be out of range,
   resulting in the generation of invalid metadata and an error
   that could have been caught at compile time not being caught
   until runtime.

This commit updates the parsing of all numeric values to use the
parse method of the target primitive type. For example,
Short.parseShort(String) is now used to parse a short.

Fixes gh-30020
  • Loading branch information
wilkinsona committed Mar 1, 2022
1 parent 355f80a commit a265f15
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 34 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,6 +18,7 @@

import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;

import javax.lang.model.element.AnnotationMirror;
Expand Down Expand Up @@ -106,21 +107,14 @@ private static class DefaultValueCoercionTypeVisitor extends TypeKindVisitor8<Ob

private static final DefaultValueCoercionTypeVisitor INSTANCE = new DefaultValueCoercionTypeVisitor();

private Integer parseInteger(String value) {
private <T extends Number> T parseNumber(String value, Function<String, T> parser,
PrimitiveType primitiveType) {
try {
return Integer.valueOf(value);
return parser.apply(value);
}
catch (NumberFormatException ex) {
throw new IllegalArgumentException(String.format("Invalid number representation '%s'", value));
}
}

private Double parseFloatingPoint(String value) {
try {
return Double.valueOf(value);
}
catch (NumberFormatException ex) {
throw new IllegalArgumentException(String.format("Invalid floating point representation '%s'", value));
throw new IllegalArgumentException(
String.format("Invalid %s representation '%s'", primitiveType, value));
}
}

Expand All @@ -131,22 +125,22 @@ public Object visitPrimitiveAsBoolean(PrimitiveType t, String value) {

@Override
public Object visitPrimitiveAsByte(PrimitiveType t, String value) {
return parseInteger(value);
return parseNumber(value, Byte::parseByte, t);
}

@Override
public Object visitPrimitiveAsShort(PrimitiveType t, String value) {
return parseInteger(value);
return parseNumber(value, Short::parseShort, t);
}

@Override
public Object visitPrimitiveAsInt(PrimitiveType t, String value) {
return parseInteger(value);
return parseNumber(value, Integer::parseInt, t);
}

@Override
public Object visitPrimitiveAsLong(PrimitiveType t, String value) {
return parseInteger(value);
return parseNumber(value, Long::parseLong, t);
}

@Override
Expand All @@ -159,12 +153,12 @@ public Object visitPrimitiveAsChar(PrimitiveType t, String value) {

@Override
public Object visitPrimitiveAsFloat(PrimitiveType t, String value) {
return parseFloatingPoint(value);
return parseNumber(value, Float::parseFloat, t);
}

@Override
public Object visitPrimitiveAsDouble(PrimitiveType t, String value) {
return parseFloatingPoint(value);
return parseNumber(value, Double::parseDouble, t);
}

}
Expand All @@ -180,12 +174,12 @@ public Object visitPrimitiveAsBoolean(PrimitiveType t, Void ignore) {

@Override
public Object visitPrimitiveAsByte(PrimitiveType t, Void ignore) {
return 0;
return (byte) 0;
}

@Override
public Object visitPrimitiveAsShort(PrimitiveType t, Void ignore) {
return 0;
return (short) 0;
}

@Override
Expand All @@ -205,7 +199,7 @@ public Object visitPrimitiveAsChar(PrimitiveType t, Void ignore) {

@Override
public Object visitPrimitiveAsFloat(PrimitiveType t, Void ignore) {
return 0;
return 0F;
}

@Override
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -155,12 +155,13 @@ void constructorParameterPropertyWithPrimitiveTypes() throws IOException {
process(ImmutablePrimitiveProperties.class, (roundEnv, metadataEnv) -> {
TypeElement ownerElement = roundEnv.getRootElement(ImmutablePrimitiveProperties.class);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "flag")).hasDefaultValue(false);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "octet")).hasDefaultValue(0);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "octet")).hasDefaultValue((byte) 0);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "letter")).hasDefaultValue(null);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "number")).hasDefaultValue(0);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "number"))
.hasDefaultValue((short) 0);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "counter")).hasDefaultValue(0);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "value")).hasDefaultValue(0L);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "percentage")).hasDefaultValue(0);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "percentage")).hasDefaultValue(0F);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "ratio")).hasDefaultValue(0D);
});
}
Expand All @@ -170,12 +171,14 @@ void constructorParameterPropertyWithPrimitiveTypesAndDefaultValues() throws IOE
process(ImmutablePrimitiveWithDefaultsProperties.class, (roundEnv, metadataEnv) -> {
TypeElement ownerElement = roundEnv.getRootElement(ImmutablePrimitiveWithDefaultsProperties.class);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "flag")).hasDefaultValue(true);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "octet")).hasDefaultValue(120);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "octet"))
.hasDefaultValue((byte) 120);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "letter")).hasDefaultValue("a");
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "number")).hasDefaultValue(1000);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "number"))
.hasDefaultValue((short) 1000);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "counter")).hasDefaultValue(42);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "value")).hasDefaultValue(2000);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "percentage")).hasDefaultValue(0.5);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "value")).hasDefaultValue(2000L);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "percentage")).hasDefaultValue(0.5F);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "ratio")).hasDefaultValue(42.42);
});
}
Expand All @@ -185,12 +188,14 @@ void constructorParameterPropertyWithPrimitiveWrapperTypesAndDefaultValues() thr
process(ImmutablePrimitiveWrapperWithDefaultsProperties.class, (roundEnv, metadataEnv) -> {
TypeElement ownerElement = roundEnv.getRootElement(ImmutablePrimitiveWrapperWithDefaultsProperties.class);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "flag")).hasDefaultValue(true);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "octet")).hasDefaultValue(120);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "octet"))
.hasDefaultValue((byte) 120);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "letter")).hasDefaultValue("a");
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "number")).hasDefaultValue(1000);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "number"))
.hasDefaultValue((short) 1000);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "counter")).hasDefaultValue(42);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "value")).hasDefaultValue(2000);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "percentage")).hasDefaultValue(0.5);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "value")).hasDefaultValue(2000L);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "percentage")).hasDefaultValue(0.5F);
assertItemMetadata(metadataEnv, createPropertyDescriptor(ownerElement, "ratio")).hasDefaultValue(42.42);
});
}
Expand Down

0 comments on commit a265f15

Please sign in to comment.