jaykataria1111 commented on issue #2769:
URL:
https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2458996673
Hi @ppkarwasz
Made some good progress but needed your input on something:
I am not sure if we should add type checking here:
```
if (methodName.startsWith("set")
&& methodElement.getParameters().size() == 1) {
// Check if this is the right setter and if it matches
return
}
```
I added this code:
```
if ((methodName.toLowerCase(Locale.ROOT).startsWith("set") // Typical
pattern for setters
||
methodName.toLowerCase(Locale.ROOT).startsWith("with")) // Typical pattern for
setters
&& methodElement.getParameters().size() == 1 //
It is a weird pattern to not have public setter
) {
// Check if the method parameter matches the type.
boolean methodParamTypeMatchesBoxedFieldType = false;
TypeMirror fieldType = element.asType();
TypeMirror methodParamType =
methodElement.getParameters().get(0).asType();
Types typeUtils = processingEnv.getTypeUtils();
if (fieldType.getKind().isPrimitive() &&
!methodParamType.getKind().isPrimitive()) {
methodParamTypeMatchesBoxedFieldType =
typeUtils.isSameType(methodParamType, typeUtils.boxedClass((PrimitiveType)
fieldType).asType());
} else if (methodParamType.getKind().isPrimitive()
&& !fieldType.getKind().isPrimitive()) {
methodParamTypeMatchesBoxedFieldType =
typeUtils.isSameType(fieldType, typeUtils.boxedClass((PrimitiveType)
methodParamType).asType());
}
boolean methodParamTypeMatchesFieldType =
typeUtils.isSameType(methodParamType, fieldType) ||
methodParamTypeMatchesBoxedFieldType;
// Check if method is public
boolean isPublicMethod =
methodElement.getModifiers().contains(Modifier.PUBLIC);
boolean foundPublicSetter =
methodParamTypeMatchesFieldType && isPublicMethod;
if (foundPublicSetter) {
// Hurray we found a public setter for the field!
return;
}
}
```
But the issue that I face is with such kind of setters, we are throwing an
error in such cases due to type checking :
```
public static final class Builder implements
org.apache.logging.log4j.core.util.Builder<IfLastModified> {
@PluginBuilderAttribute
@Required(message = "No age provided for IfLastModified")
private
org.apache.logging.log4j.core.appender.rolling.action.Duration age;
public Builder setAge(final Duration age) {
this.age =
org.apache.logging.log4j.core.appender.rolling.action.Duration.ofMillis(age.toMillis());
return this;
}
```
As you can see the setAge method has a different parameter type than the
fields type.
if we do have a hard check on the naming convention "setFieldName" (This
becomes more restrictive, do not recommend, I have seen people get really
creative for no reason) that has a number of problems as well.
I do believe we should do some sort of checks, just checking for
`methodName.startsWith("set") && methodElement.getParameters().size() == 1` to
identify
Hence I have come up with the following plan:
1. method name starts with "set" or "with" and method has 1 param
2. method param type is the same as the field type (includes boxed
comparisons)
2.a if the above is not being followed then as a fall back we can check if
there is a method that follows the naming convention like in the above case
after that I believe we can give up, because then people are doing something
really creative with their code.
Hence building on the above statement, I have the following logic, which
seems to work :) :
```
if ((methodName.toLowerCase(Locale.ROOT).startsWith("set") // Typical
pattern for setters
||
methodName.toLowerCase(Locale.ROOT).startsWith("with")) // Typical pattern for
setters
&& methodElement.getParameters().size() == 1 //
It is a weird pattern to not have public setter
) {
// Check if the method parameter matches the type.
boolean methodParamTypeMatchesBoxedFieldType = false;
TypeMirror fieldType = element.asType();
TypeMirror methodParamType =
methodElement.getParameters().get(0).asType();
Types typeUtils = processingEnv.getTypeUtils();
if (fieldType.getKind().isPrimitive() &&
!methodParamType.getKind().isPrimitive()) {
methodParamTypeMatchesBoxedFieldType =
typeUtils.isSameType(methodParamType, typeUtils.boxedClass((PrimitiveType)
fieldType).asType());
} else if (methodParamType.getKind().isPrimitive()
&& !fieldType.getKind().isPrimitive()) {
methodParamTypeMatchesBoxedFieldType =
typeUtils.isSameType(fieldType, typeUtils.boxedClass((PrimitiveType)
methodParamType).asType());
}
boolean methodParamTypeMatchesFieldType =
typeUtils.isSameType(methodParamType, fieldType) ||
methodParamTypeMatchesBoxedFieldType;
boolean containsName =
methodName.toLowerCase(Locale.ROOT).equals(String.format("set%s",fieldName.toLowerCase(Locale.ROOT)))
||
methodName.toLowerCase(Locale.ROOT).equals(String.format("with%s",fieldName.toLowerCase(Locale.ROOT)));
// Check if method is public
boolean isPublicMethod =
methodElement.getModifiers().contains(Modifier.PUBLIC);
boolean foundPublicSetter =
(methodParamTypeMatchesFieldType || containsName) && isPublicMethod;
if (foundPublicSetter) {
// Hurray we found a public setter for the field!
return;
}
}
}
}
```
--
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]
