sergey-chugunov-1985 commented on code in PR #13414:
URL: https://github.com/apache/ignite/pull/13414#discussion_r3683140457
##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,34 +297,99 @@ 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();
+ Map<Element, String> enumsPerField = new HashMap<>();
- IgniteBiTuple<String, String> otherMsgAndMapperClassesNames =
- enumMappersInUse.put(enumClsFullName, new
IgniteBiTuple<>(msgClsName, enumMapperClsName));
+ if (!inspectFieldForEnumTypes(type.toString(), el, el.asType(),
custMappAnn, enumsPerField) && 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);
+ }
+ }
+
+ /**
+ * @param msgClsName Message class name currently being inspected.
+ * @param field Field being inspected.
+ * @param type Type that should be inpected for enum type (direct type or
type parameter).
+ * @param custMappAnn Custom mapper annotation declared for an enum type.
Review Comment:
```suggestion
* @param custMappAnn Custom mapper annotation declared for the enum
type.
```
##########
modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java:
##########
@@ -343,6 +390,25 @@ public void
testDifferentMappersForTheSameEnumAreProhibited() {
assertThat(compilation).hadErrorContaining(errMsg);
}
+ /**
+ * Negative test for a coflict situation when two enum mappers are used
for the same enum in different messages.
Review Comment:
```suggestion
* Negative test for a conflict situation when two enum mappers are used
for the same enum in different messages.
```
##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,34 +297,99 @@ 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();
+ Map<Element, String> enumsPerField = new HashMap<>();
- IgniteBiTuple<String, String> otherMsgAndMapperClassesNames =
- enumMappersInUse.put(enumClsFullName, new
IgniteBiTuple<>(msgClsName, enumMapperClsName));
+ if (!inspectFieldForEnumTypes(type.toString(), el, el.asType(),
custMappAnn, enumsPerField) && 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);
+ }
+ }
+
+ /**
+ * @param msgClsName Message class name currently being inspected.
+ * @param field Field being inspected.
+ * @param type Type that should be inpected for enum type (direct type or
type parameter).
+ * @param custMappAnn Custom mapper annotation declared for an enum type.
+ * @param enumsPerField Map for collecting enum types related to a
partiular field.
Review Comment:
```suggestion
* @param enumsPerField Map for collecting enum types related to a
particular field.
```
##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java:
##########
@@ -294,34 +297,99 @@ 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();
+ Map<Element, String> enumsPerField = new HashMap<>();
- IgniteBiTuple<String, String> otherMsgAndMapperClassesNames =
- enumMappersInUse.put(enumClsFullName, new
IgniteBiTuple<>(msgClsName, enumMapperClsName));
+ if (!inspectFieldForEnumTypes(type.toString(), el, el.asType(),
custMappAnn, enumsPerField) && 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);
+ }
+ }
+
+ /**
+ * @param msgClsName Message class name currently being inspected.
+ * @param field Field being inspected.
+ * @param type Type that should be inpected for enum type (direct type or
type parameter).
+ * @param custMappAnn Custom mapper annotation declared for an enum type.
+ * @param enumsPerField Map for collecting enum types related to a
partiular field.
+ */
+ private boolean inspectFieldForEnumTypes(String msgClsName, Element field,
TypeMirror type, CustomMapper custMappAnn,
+ Map<Element, String> enumsPerField) {
+ String enumClsFullName = type.toString();
+ String enumMapperClsName = custMappAnn != null ? custMappAnn.value() :
DLFT_ENUM_MAPPER_CLS;
- if (otherMsgAndMapperClassesNames != null) {
- String otherMsgClsName = otherMsgAndMapperClassesNames.get1();
- String otherEnumMapperClsName =
otherMsgAndMapperClassesNames.get2();
+ if (enumType(processingEnv, type)) {
+ inspectForDuplicatedMappers(msgClsName, field, enumClsFullName,
enumMapperClsName);
- if (!otherEnumMapperClsName.equals(enumMapperClsName)) {
- processingEnv.getMessager().printMessage(
- Diagnostic.Kind.ERROR,
- "Enum " + enumClsFullName + " is declared with
different mappers: " +
- otherEnumMapperClsName + " in " + otherMsgClsName
+ " and " +
- enumMapperClsName + " in " + msgClsName +
- ". Only one mapper is allowed per enum type.",
- el);
- }
+ inspectForDuplicatedEnums(msgClsName, field, type, enumsPerField);
+
+ return true;
+ }
+ else if (assignableFrom(erasedType(type),
type(Collection.class.getName()))) {
+ List<? extends TypeMirror> typeArgs =
((DeclaredType)type).getTypeArguments();
+
+ assert typeArgs.size() == 1 : type.toString();
+
+ TypeMirror typeArg = typeArgs.get(0);
+
+ return inspectFieldForEnumTypes(msgClsName, field, typeArg,
custMappAnn, enumsPerField);
+ }
+ else if (assignableFrom(erasedType(type), type(Map.class.getName()))) {
+ List<? extends TypeMirror> typeArgs =
((DeclaredType)type).getTypeArguments();
+
+ assert typeArgs.size() == 2 : type.toString();
+
+ TypeMirror keyType = typeArgs.get(0);
+ TypeMirror valType = typeArgs.get(1);
+
+ return inspectFieldForEnumTypes(msgClsName, field, keyType,
custMappAnn, enumsPerField) |
+ inspectFieldForEnumTypes(msgClsName, field, valType,
custMappAnn, enumsPerField);
+ }
+
+ return false;
+ }
+
+ /**
+ * Checks, that only single type of mapper is used for an enum type.
Review Comment:
```suggestion
* Checks, that only single type of mapper is used for the enum type.
```
--
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]
