You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by GitBox <gi...@apache.org> on 2022/04/15 16:07:43 UTC
[GitHub] [groovy] eric-milles opened a new pull request, #1714: GROOVY-10570: `@AnnotationCollector`: better error for missing `value()`
eric-milles opened a new pull request, #1714:
URL: https://github.com/apache/groovy/pull/1714
If user applies `@AnnotationCollector` to a Java class, it will be missing the `serializeClass` attribute. Rather than "BUG! exception in phase 'semantic analysis' in source unit ..." produce an error message that indicates what was expected form the collector.
I also managed to produce an example Java annotation that emulates the serialized collected annotations. This might be useful as a workaround for GROOVY-10571.
https://issues.apache.org/jira/browse/GROOVY-10570
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on a diff in pull request #1714: GROOVY-10570: `@AnnotationCollector`: better error for missing `value()`
Posted by GitBox <gi...@apache.org>.
eric-milles commented on code in PR #1714:
URL: https://github.com/apache/groovy/pull/1714#discussion_r851386868
##########
src/main/java/org/codehaus/groovy/transform/AnnotationCollectorTransform.java:
##########
@@ -268,36 +270,43 @@ private static void copyMembers(final Map<String, Expression> members, final Ann
}
}
- private static List<AnnotationNode> getTargetListFromClass(ClassNode alias) {
- alias = getSerializeClass(alias);
- Class<?> c = alias.getTypeClass();
+ private static List<AnnotationNode> getTargetListFromClass(final ClassNode alias) {
+ ClassNode cn = getSerializeClass(alias);
+ Class<?> c = cn.getTypeClass();
Object[][] data;
try {
Method m = c.getMethod("value");
+ if (!Modifier.isStatic(m.getModifiers()))
+ throw new NoSuchMethodException("non-static value()");
+
data = (Object[][]) m.invoke(null);
+ return makeListOfAnnotations(data);
+ } catch (NoSuchMethodException | ClassCastException e) {
+ throw new GroovyRuntimeException("Expecting static method `Object[][] value()`" +
+ " in " + cn.toString(false) + ". Was it compiled from a Java source?");
} catch (Exception e) {
throw new GroovyBugError(e);
}
- return makeListOfAnnotations(data);
}
// 2.5.3 and above gets from annotation attribute otherwise self
- private static ClassNode getSerializeClass(ClassNode alias) {
- List<AnnotationNode> annotations = alias.getAnnotations(ClassHelper.make(AnnotationCollector.class));
- if (!annotations.isEmpty()) {
- AnnotationNode annotationNode = annotations.get(0);
- Expression member = annotationNode.getMember("serializeClass");
- if (member instanceof ClassExpression) {
- ClassExpression ce = (ClassExpression) member;
- if (!ce.getType().getName().equals(AnnotationCollector.class.getName())) {
- alias = ce.getType();
+ private static ClassNode getSerializeClass(final ClassNode alias) {
+ List<AnnotationNode> collectors = alias.getAnnotations(ClassHelper.make(AnnotationCollector.class));
+ if (!collectors.isEmpty()) {
+ assert collectors.size() == 1;
+ AnnotationNode collectorNode = collectors.get(0);
+ Expression serializeClass = collectorNode.getMember("serializeClass");
+ if (serializeClass instanceof ClassExpression) {
+ ClassNode serializeClassType = serializeClass.getType();
+ if (!serializeClassType.getName().equals(AnnotationCollector.class.getName())) {
+ return serializeClassType;
}
}
}
return alias;
}
- private static List<AnnotationNode> makeListOfAnnotations(Object[][] data) {
+ private static List<AnnotationNode> makeListOfAnnotations(final Object[][] data) {
Review Comment:
@sonatype-lift ignore
This is an implementation method. The return value is not exposed to the outside world.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on a diff in pull request #1714: GROOVY-10570: `@AnnotationCollector`: better error for missing `value()`
Posted by GitBox <gi...@apache.org>.
eric-milles commented on code in PR #1714:
URL: https://github.com/apache/groovy/pull/1714#discussion_r851386868
##########
src/main/java/org/codehaus/groovy/transform/AnnotationCollectorTransform.java:
##########
@@ -268,36 +270,43 @@ private static void copyMembers(final Map<String, Expression> members, final Ann
}
}
- private static List<AnnotationNode> getTargetListFromClass(ClassNode alias) {
- alias = getSerializeClass(alias);
- Class<?> c = alias.getTypeClass();
+ private static List<AnnotationNode> getTargetListFromClass(final ClassNode alias) {
+ ClassNode cn = getSerializeClass(alias);
+ Class<?> c = cn.getTypeClass();
Object[][] data;
try {
Method m = c.getMethod("value");
+ if (!Modifier.isStatic(m.getModifiers()))
+ throw new NoSuchMethodException("non-static value()");
+
data = (Object[][]) m.invoke(null);
+ return makeListOfAnnotations(data);
+ } catch (NoSuchMethodException | ClassCastException e) {
+ throw new GroovyRuntimeException("Expecting static method `Object[][] value()`" +
+ " in " + cn.toString(false) + ". Was it compiled from a Java source?");
} catch (Exception e) {
throw new GroovyBugError(e);
}
- return makeListOfAnnotations(data);
}
// 2.5.3 and above gets from annotation attribute otherwise self
- private static ClassNode getSerializeClass(ClassNode alias) {
- List<AnnotationNode> annotations = alias.getAnnotations(ClassHelper.make(AnnotationCollector.class));
- if (!annotations.isEmpty()) {
- AnnotationNode annotationNode = annotations.get(0);
- Expression member = annotationNode.getMember("serializeClass");
- if (member instanceof ClassExpression) {
- ClassExpression ce = (ClassExpression) member;
- if (!ce.getType().getName().equals(AnnotationCollector.class.getName())) {
- alias = ce.getType();
+ private static ClassNode getSerializeClass(final ClassNode alias) {
+ List<AnnotationNode> collectors = alias.getAnnotations(ClassHelper.make(AnnotationCollector.class));
+ if (!collectors.isEmpty()) {
+ assert collectors.size() == 1;
+ AnnotationNode collectorNode = collectors.get(0);
+ Expression serializeClass = collectorNode.getMember("serializeClass");
+ if (serializeClass instanceof ClassExpression) {
+ ClassNode serializeClassType = serializeClass.getType();
+ if (!serializeClassType.getName().equals(AnnotationCollector.class.getName())) {
+ return serializeClassType;
}
}
}
return alias;
}
- private static List<AnnotationNode> makeListOfAnnotations(Object[][] data) {
+ private static List<AnnotationNode> makeListOfAnnotations(final Object[][] data) {
Review Comment:
@sonatype-lift This is an implementation method. The return value is not exposed to the outside world.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] sonatype-lift[bot] commented on a diff in pull request #1714: GROOVY-10570: `@AnnotationCollector`: better error for missing `value()`
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1714:
URL: https://github.com/apache/groovy/pull/1714#discussion_r851384163
##########
src/main/java/org/codehaus/groovy/transform/AnnotationCollectorTransform.java:
##########
@@ -268,36 +270,43 @@ private static void copyMembers(final Map<String, Expression> members, final Ann
}
}
- private static List<AnnotationNode> getTargetListFromClass(ClassNode alias) {
- alias = getSerializeClass(alias);
- Class<?> c = alias.getTypeClass();
+ private static List<AnnotationNode> getTargetListFromClass(final ClassNode alias) {
+ ClassNode cn = getSerializeClass(alias);
+ Class<?> c = cn.getTypeClass();
Object[][] data;
try {
Method m = c.getMethod("value");
+ if (!Modifier.isStatic(m.getModifiers()))
+ throw new NoSuchMethodException("non-static value()");
+
data = (Object[][]) m.invoke(null);
+ return makeListOfAnnotations(data);
+ } catch (NoSuchMethodException | ClassCastException e) {
+ throw new GroovyRuntimeException("Expecting static method `Object[][] value()`" +
+ " in " + cn.toString(false) + ". Was it compiled from a Java source?");
} catch (Exception e) {
throw new GroovyBugError(e);
}
- return makeListOfAnnotations(data);
}
// 2.5.3 and above gets from annotation attribute otherwise self
- private static ClassNode getSerializeClass(ClassNode alias) {
- List<AnnotationNode> annotations = alias.getAnnotations(ClassHelper.make(AnnotationCollector.class));
- if (!annotations.isEmpty()) {
- AnnotationNode annotationNode = annotations.get(0);
- Expression member = annotationNode.getMember("serializeClass");
- if (member instanceof ClassExpression) {
- ClassExpression ce = (ClassExpression) member;
- if (!ce.getType().getName().equals(AnnotationCollector.class.getName())) {
- alias = ce.getType();
+ private static ClassNode getSerializeClass(final ClassNode alias) {
+ List<AnnotationNode> collectors = alias.getAnnotations(ClassHelper.make(AnnotationCollector.class));
+ if (!collectors.isEmpty()) {
+ assert collectors.size() == 1;
+ AnnotationNode collectorNode = collectors.get(0);
+ Expression serializeClass = collectorNode.getMember("serializeClass");
+ if (serializeClass instanceof ClassExpression) {
+ ClassNode serializeClassType = serializeClass.getType();
+ if (!serializeClassType.getName().equals(AnnotationCollector.class.getName())) {
+ return serializeClassType;
}
}
}
return alias;
}
- private static List<AnnotationNode> makeListOfAnnotations(Object[][] data) {
+ private static List<AnnotationNode> makeListOfAnnotations(final Object[][] data) {
Review Comment:
*MixedMutabilityReturnType:* This method returns both mutable and immutable collections or maps from different paths. This may be confusing for users of the method. [(details)](https://errorprone.info/bugpattern/MixedMutabilityReturnType)
```suggestion
private static ImmutableList<AnnotationNode> makeListOfAnnotations(final Object[][] data) {
```
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
---
Was this a good recommendation?
[ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=192715095&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=192715095&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=192715095&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=192715095&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=192715095&lift_comment_rating=5) ]
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles merged pull request #1714: GROOVY-10570: `@AnnotationCollector`: better error for missing `value()`
Posted by GitBox <gi...@apache.org>.
eric-milles merged PR #1714:
URL: https://github.com/apache/groovy/pull/1714
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org