SteveNewson commented on issue #20752:
URL: https://github.com/apache/beam/issues/20752#issuecomment-1294845137
The "problem" is here:
```java
@Override
public List<FieldValueTypeInformation> get(Class<?> clazz) {
// If the generated class is passed in, we want to look at the base
class to find the getters.
Class<?> targetClass = AutoValueUtils.getBaseAutoValueClass(clazz);
List<Method> methods =
ReflectUtils.getMethods(targetClass).stream()
.filter(ReflectUtils::isGetter)
// All AutoValue getters are marked abstract.
.filter(m -> Modifier.isAbstract(m.getModifiers()))
.filter(m -> !Modifier.isPrivate(m.getModifiers()))
.filter(m -> !Modifier.isProtected(m.getModifiers()))
.filter(m -> !m.isAnnotationPresent(SchemaIgnore.class))
.collect(Collectors.toList());
List<FieldValueTypeInformation> types =
Lists.newArrayListWithCapacity(methods.size());
for (int i = 0; i < methods.size(); ++i) {
types.add(FieldValueTypeInformation.forGetter(methods.get(i), i));
}
types.sort(Comparator.comparing(FieldValueTypeInformation::getNumber));
validateFieldNumbers(types);
return types;
}
```
The first filter removes anything that does not start with "get" or "is":
```java
public static boolean isGetter(Method method) {
if (Void.TYPE.equals(method.getReturnType())) {
return false;
}
if (method.getName().startsWith("get") && method.getName().length() > 3)
{
return true;
}
return (method.getName().startsWith("is")
&& method.getName().length() > 2
&& method.getParameterCount() == 0
&& (Boolean.TYPE.equals(method.getReturnType())
|| Boolean.class.equals(method.getReturnType())));
}
```
Clearly a trivial change to correct this. However, that might be
controversial as changing this behaviour may pick up methods that previously
would not have been detected. If there's another Abstract method on the class
that is not a getter then this would cause a problem. However, I can't see how
that could occur without also breaking AutoValue itself.
```java
@AutoValue
public abstract Bob {
abstract int id();
abstract int notARealGetter();
static Bob create(int id) {};
}
```
AutoValue would interpret the `notARealGetter` method as a getter, so this
is fine.
The problem would be `toString` (or any `Object` method that AutoValue can
redefine - currently only `toString`, `hashCode` and `equals`):
```java
@AutoValue
class PleaseOverrideExample extends SuperclassThatDefinesToString {
...
// cause AutoValue to generate this even though the superclass has it
@Override public abstract String toString();
}
```
Removing the `isGetter` filter would pick this method up as a property,
which it clearly is not. It is likely that people have done this in existing
code, so changing this behaviour would potentially break their code (without a
compilation or runtime warning of the fact).
So, I propose we remove the filter `isGetter` and also explicitly exclude
the `toString`, `hashCode` and `equals` methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
