sergey-chugunov-1985 commented on code in PR #13414:
URL: https://github.com/apache/ignite/pull/13414#discussion_r3682106396


##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,11 +300,27 @@ private List<List<VariableElement>> 
hierarchicalOrderedFields(TypeElement type)
     private void validateEnumFieldMapping(TypeElement type, Element el) {
         CustomMapper custMappAnn = el.getAnnotation(CustomMapper.class);
 
-        if (enumType(processingEnv, el.asType())) {
-            String enumClsFullName = el.asType().toString();
-            String enumMapperClsName = custMappAnn != null ? 
custMappAnn.value() : DLFT_ENUM_MAPPER_CLS;
-            String msgClsName = type.toString();
+        if (!validateEnumType(type.toString(), el, el.asType(), custMappAnn) 
&& custMappAnn != null) {
+            processingEnv.getMessager().printMessage(
+                Diagnostic.Kind.ERROR,
+                "Annotation @CustomMapper must only be used for enum fields or 
enum collections and maps, including nested ones.",
+                el);
+        }
+
+        enumsPerField.clear();
+    }
+
+    /**
+     * @param msgClsName Message class name.
+     * @param field Field.

Review Comment:
   ```suggestion
        * @param field Field being inspected.
   ```



##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,11 +300,27 @@ private List<List<VariableElement>> 
hierarchicalOrderedFields(TypeElement type)
     private void validateEnumFieldMapping(TypeElement type, Element el) {
         CustomMapper custMappAnn = el.getAnnotation(CustomMapper.class);
 
-        if (enumType(processingEnv, el.asType())) {
-            String enumClsFullName = el.asType().toString();
-            String enumMapperClsName = custMappAnn != null ? 
custMappAnn.value() : DLFT_ENUM_MAPPER_CLS;
-            String msgClsName = type.toString();
+        if (!validateEnumType(type.toString(), el, el.asType(), custMappAnn) 
&& custMappAnn != null) {
+            processingEnv.getMessager().printMessage(
+                Diagnostic.Kind.ERROR,
+                "Annotation @CustomMapper must only be used for enum fields or 
enum collections and maps, including nested ones.",
+                el);
+        }
+
+        enumsPerField.clear();
+    }
+
+    /**
+     * @param msgClsName Message class name.
+     * @param field Field.
+     * @param type Type.
+     * @param custMappAnn Cust mapp ann.

Review Comment:
   ```suggestion
        * @param custMappAnn Custom mapper annotation declared for an enum type.
   ```



##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -313,16 +335,45 @@ private void validateEnumFieldMapping(TypeElement type, 
Element el) {
                             otherEnumMapperClsName + " in " + otherMsgClsName 
+ " and " +
                             enumMapperClsName + " in " + msgClsName +
                             ". Only one mapper is allowed per enum type.",
-                        el);
+                        field);
                 }
             }
+
+            String otherEnum = type.toString();

Review Comment:
   This code could be moved to a separate method as the previous comment 
suggests.



##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,11 +300,27 @@ private List<List<VariableElement>> 
hierarchicalOrderedFields(TypeElement type)
     private void validateEnumFieldMapping(TypeElement type, Element el) {
         CustomMapper custMappAnn = el.getAnnotation(CustomMapper.class);
 
-        if (enumType(processingEnv, el.asType())) {
-            String enumClsFullName = el.asType().toString();
-            String enumMapperClsName = custMappAnn != null ? 
custMappAnn.value() : DLFT_ENUM_MAPPER_CLS;
-            String msgClsName = type.toString();
+        if (!validateEnumType(type.toString(), el, el.asType(), custMappAnn) 
&& custMappAnn != null) {
+            processingEnv.getMessager().printMessage(
+                Diagnostic.Kind.ERROR,
+                "Annotation @CustomMapper must only be used for enum fields or 
enum collections and maps, including nested ones.",
+                el);
+        }
+
+        enumsPerField.clear();
+    }
+
+    /**
+     * @param msgClsName Message class name.
+     * @param field Field.
+     * @param type Type.
+     * @param custMappAnn Cust mapp ann.
+     */
+    private boolean validateEnumType(String msgClsName, Element field, 
TypeMirror type, CustomMapper custMappAnn) {

Review Comment:
   ```suggestion
       private boolean inspectFieldForEnumTypes(String msgClsName, Element 
field, TypeMirror type, CustomMapper custMappAnn) {
   ```



##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,11 +300,27 @@ private List<List<VariableElement>> 
hierarchicalOrderedFields(TypeElement type)
     private void validateEnumFieldMapping(TypeElement type, Element el) {
         CustomMapper custMappAnn = el.getAnnotation(CustomMapper.class);
 
-        if (enumType(processingEnv, el.asType())) {
-            String enumClsFullName = el.asType().toString();
-            String enumMapperClsName = custMappAnn != null ? 
custMappAnn.value() : DLFT_ENUM_MAPPER_CLS;
-            String msgClsName = type.toString();
+        if (!validateEnumType(type.toString(), el, el.asType(), custMappAnn) 
&& custMappAnn != null) {
+            processingEnv.getMessager().printMessage(
+                Diagnostic.Kind.ERROR,
+                "Annotation @CustomMapper must only be used for enum fields or 
enum collections and maps, including nested ones.",
+                el);
+        }
+
+        enumsPerField.clear();
+    }
+
+    /**
+     * @param msgClsName Message class name.
+     * @param field Field.
+     * @param type Type.

Review Comment:
   ```suggestion
        * @param type Type that should be inpected for enum type (direct type 
or type parameter).
   ```



##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,11 +300,27 @@ private List<List<VariableElement>> 
hierarchicalOrderedFields(TypeElement type)
     private void validateEnumFieldMapping(TypeElement type, Element el) {
         CustomMapper custMappAnn = el.getAnnotation(CustomMapper.class);
 
-        if (enumType(processingEnv, el.asType())) {
-            String enumClsFullName = el.asType().toString();
-            String enumMapperClsName = custMappAnn != null ? 
custMappAnn.value() : DLFT_ENUM_MAPPER_CLS;
-            String msgClsName = type.toString();
+        if (!validateEnumType(type.toString(), el, el.asType(), custMappAnn) 
&& custMappAnn != null) {
+            processingEnv.getMessager().printMessage(
+                Diagnostic.Kind.ERROR,
+                "Annotation @CustomMapper must only be used for enum fields or 
enum collections and maps, including nested ones.",
+                el);
+        }
+
+        enumsPerField.clear();
+    }
+
+    /**
+     * @param msgClsName Message class name.

Review Comment:
   ```suggestion
        * @param msgClsName Message class name currently being inspected.
   ```



##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,11 +300,27 @@ private List<List<VariableElement>> 
hierarchicalOrderedFields(TypeElement type)
     private void validateEnumFieldMapping(TypeElement type, Element el) {
         CustomMapper custMappAnn = el.getAnnotation(CustomMapper.class);
 
-        if (enumType(processingEnv, el.asType())) {
-            String enumClsFullName = el.asType().toString();
-            String enumMapperClsName = custMappAnn != null ? 
custMappAnn.value() : DLFT_ENUM_MAPPER_CLS;
-            String msgClsName = type.toString();
+        if (!validateEnumType(type.toString(), el, el.asType(), custMappAnn) 
&& custMappAnn != null) {
+            processingEnv.getMessager().printMessage(
+                Diagnostic.Kind.ERROR,
+                "Annotation @CustomMapper must only be used for enum fields or 
enum collections and maps, including nested ones.",
+                el);
+        }
+
+        enumsPerField.clear();
+    }
+
+    /**
+     * @param msgClsName Message class name.
+     * @param field Field.
+     * @param type Type.
+     * @param custMappAnn Cust mapp ann.
+     */
+    private boolean validateEnumType(String msgClsName, Element field, 
TypeMirror type, CustomMapper custMappAnn) {
+        String enumClsFullName = type.toString();
+        String enumMapperClsName = custMappAnn != null ? custMappAnn.value() : 
DLFT_ENUM_MAPPER_CLS;
 
+        if (enumType(processingEnv, type)) {
             IgniteBiTuple<String, String> otherMsgAndMapperClassesNames =

Review Comment:
   I suggest to move these two pieces of code into separate methods: this first 
one is checking that this enum type has only one mapper, either default or 
custom. And another one should perform a check that given type has only enum 
field associated with it.



-- 
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]

Reply via email to