[issue 1387] Validate final primitive|String fields w/ Option|Parameters annotations #1711
Conversation
acmcelwee
force-pushed
the
issue-1387
branch
2 times, most recently
from
787b9c9
to
3d4b67f
Compare
| boolean isConstantValueField = false; | ||
| for (VariableElement variableElement : ElementFilter.fieldsIn(Collections.singletonList(e))) { | ||
| isConstantValueField = isConstantValueField || variableElement.getConstantValue() != null; | ||
| } | ||
| if ((type.getKind().isPrimitive() || typeUtils.isAssignable(type, stringType)) && isConstantValueField) { |
There was a problem hiding this comment.
While I'd love to also apply this compile-time validation to the boxed primitive types, I don't know if there's actually a way to determine if final Integer foo has a value specified in the class definition. getConstantValue() only returns a non-null value if the defined value is non-null and the type is a true primitive type or string.
| String errorFormat = "Constant (final) primitive and String fields like %s cannot be used as %s: compile-time constant inlining may hide new values written to it."; | ||
| List<String> expectedValidationErrors = new ArrayList<String>(); | ||
| for (String type : types) { | ||
| String titleized = type.substring(0, 1).toUpperCase() + type.substring(1); | ||
| String invalidOptionField = String.format("invalid%s", titleized); | ||
| String invalidParamField = String.format("invalid%sParam", titleized); | ||
|
|
||
| expectedValidationErrors.add(String.format(errorFormat, invalidOptionField, "@Option")); | ||
| expectedValidationErrors.add(String.format(errorFormat, invalidParamField, "@Parameters")); | ||
| } |
There was a problem hiding this comment.
Oh, I also wasn't certain about the java version where these tests get executed, but I demonstrated considerable restraint and wrote this test to only use java 6+ language features.
| boolean isConstantValueField = false; | ||
| for (VariableElement variableElement : ElementFilter.fieldsIn(Collections.singletonList(e))) { | ||
| isConstantValueField = isConstantValueField || variableElement.getConstantValue() != null; | ||
| } |
There was a problem hiding this comment.
In reality, the result of ElementFilter.fieldsIn(...) will either have 0 or 1 item in it, but I just expressed it in a more general sense.
|
Thank you for the contribution! |
|
No worries! I don't currently need this functionality, but I used this open issue as a prompt for a programming exercise with a candidate I interviewed today. Before I asked someone to try it, I took the time to work through the problem, and I figured the best use of that effort was to contribute back to a tool we use at work. |
|
Oh, and BTW, I'll chat with some of the team at work tomorrow to see if anyone has any creative ideas on how to achieve this same functionality for boxed primitive types. |
|
I mis-clicked and accidentally merged before reviewing. 😅 |
|
Please ignore my previous comment; that comment was intended for a different PR: #1708. 😅 |
remkop
added
type: enhancement ✨
theme: annotation-proc
|
Merged. Thank you for the contribution! |
Fixes #1387