martin-g commented on code in PR #3150:
URL: https://github.com/apache/avro/pull/3150#discussion_r1749835366
##########
lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java:
##########
@@ -1004,27 +1005,43 @@ public String conversionInstance(Schema schema) {
*/
public String[] javaAnnotations(JsonProperties props) {
final Object value = props.getObjectProp("javaAnnotation");
- if (value == null)
- return new String[0];
- if (value instanceof String)
+ if (value instanceof String && isValidAsAnnotation((String) value))
return new String[] { value.toString() };
if (value instanceof List) {
final List<?> list = (List<?>) value;
final List<String> annots = new ArrayList<>(list.size());
for (Object o : list) {
- annots.add(o.toString());
+ if (isValidAsAnnotation(o.toString()))
+ annots.add(o.toString());
}
return annots.toArray(new String[0]);
}
return new String[0];
}
+ private static final String PATTERN_IDENTIFIER_PART =
"\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*";
+ private static final String PATTERN_IDENTIFIER =
String.format("(?:%s(?:\\.%s)*)", PATTERN_IDENTIFIER_PART,
+ PATTERN_IDENTIFIER_PART);
+ private static final String PATTERN_STRING =
"\"(?:\\\\[\\\\\"ntfb]|(?<!\\\\).)*\"";
+ private static final String PATTERN_NUMBER =
"(?:\\((?:byte|char|short|int|long|float|double)\\))?[x0-9_.]*[fl]?";
+ private static final String PATTERN_LITERAL_VALUE =
String.format("(?:%s|%s|true|false)", PATTERN_STRING,
+ PATTERN_NUMBER);
+ private static final String PATTERN_PARAMETER_LIST = String.format(
+ "\\(\\s*(?:%s|%s\\s*=\\s*%s(?:\\s*,\\s*%s\\s*=\\s*%s)*)?\\s*\\)",
PATTERN_LITERAL_VALUE, PATTERN_IDENTIFIER,
+ PATTERN_LITERAL_VALUE, PATTERN_IDENTIFIER, PATTERN_LITERAL_VALUE);
+ private static final Pattern VALID_AS_ANNOTATION = Pattern
+ .compile(String.format("%s(?:%s)?", PATTERN_IDENTIFIER,
PATTERN_PARAMETER_LIST));
+
+ private boolean isValidAsAnnotation(String value) {
Review Comment:
Is this new method being used somewhere ?
It is `private` and I do not see any usages below.
##########
lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java:
##########
@@ -1045,7 +1062,7 @@ public String javaSplit(String s) throws IOException {
/**
* Utility for template use. Escapes quotes and backslashes.
*/
- public static String javaEscape(String o) {
+ public static String escapeForJavaString(String o) {
Review Comment:
This is an API break.
Please keep the old method as deprecated for the lifecycle of 1.12.
##########
lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java:
##########
@@ -17,14 +17,19 @@
*/
package org.apache.avro.compiler.specific;
-import static org.hamcrest.CoreMatchers.equalTo;
Review Comment:
IMO static imports are usually before the non-static ones. Please update
your IDE settings to not do these changes automatically.
--
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]
