Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ds: Add details to error messages for DS annotations processing #4225

Merged
merged 1 commit into from Jul 10, 2020
Merged

ds: Add details to error messages for DS annotations processing #4225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
93 changes: 44 additions & 49 deletions biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java
View file
Open in desktop
Expand Up @@ -14,7 +14,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -94,15 +93,17 @@ public static class ValidNSVersion {
}

public void testExceedsVersion() throws Exception {
Builder b = new Builder();
b.setProperty(Constants.DSANNOTATIONS, "test.component.*ValidNSVersion");
b.setProperty(Constants.DSANNOTATIONS_OPTIONS, "version;maximum=1.2.0");
b.setProperty("Private-Package", "test.component");
b.addClasspath(new File("bin_test"));
b.addClasspath(new File("jar/osgi.jar")); // v1.0.0
Jar jar = b.build();
assertTrue(b.check(
"component 1.3.0 version test.component.DSAnnotationTest.ValidNSVersion exceeds -dsannotations-options version;maximum version 1.2.0 because base"));
try (Builder b = new Builder()) {
b.setProperty(Constants.DSANNOTATIONS, "test.component.*ValidNSVersion");
b.setProperty(Constants.DSANNOTATIONS_OPTIONS, "version;maximum=1.2.0");
b.setProperty("Private-Package", "test.component");
b.addClasspath(new File("bin_test"));
b.addClasspath(new File("jar/osgi.jar")); // v1.0.0
Jar jar = b.build();
assertThat(b.getErrors()).anyMatch(e -> e.matches(
".*ValidNSVersion.*version 1\\.3\\.0 exceeds -dsannotations-options version;maximum version 1\\.2\\.0 because base.*"))
.hasSize(1);
}
}

@Component()
Expand All @@ -115,22 +116,25 @@ void setX(Map<String, Object> map) {
}

public void testRequires1_3() throws Exception {
Builder b = new Builder();
b.setProperty(Constants.DSANNOTATIONS, "test.component.*RequiresV1_3");
b.setProperty(Constants.DSANNOTATIONS_OPTIONS, "version;minimum=1.2.0;maximum=1.2.0");
b.setProperty("Private-Package", "test.component");
b.addClasspath(new File("bin_test"));
b.addClasspath(new File("jar/osgi.jar")); // v1.0.0
Jar jar = b.build();
assertTrue(b.check("component 1.3.0 version test.component.DSAnnotationTest\\$RequiresV1_3 exceeds"));
try (Builder b = new Builder()) {
b.setProperty(Constants.DSANNOTATIONS, "test.component.*RequiresV1_3");
b.setProperty(Constants.DSANNOTATIONS_OPTIONS, "version;minimum=1.2.0;maximum=1.2.0");
b.setProperty("Private-Package", "test.component");
b.addClasspath(new File("bin_test"));
b.addClasspath(new File("jar/osgi.jar")); // v1.0.0
Jar jar = b.build();
assertThat(b.getErrors()).anyMatch(e -> e.matches(
".*RequiresV1_3.*version 1\\.3\\.0 exceeds -dsannotations-options version;maximum version 1\\.2\\.0.*"))
.hasSize(1);

System.out.println(jar.getResources()
.keySet());
System.out.println(jar.getResources()
.keySet());

Resource resource = jar.getResource("OSGI-INF/test.component.DSAnnotationTest$RequiresV1_3.xml");
assertThat(resource).isNotNull();
String s = IO.collect(resource.openInputStream());
System.out.println(s);
Resource resource = jar.getResource("OSGI-INF/test.component.DSAnnotationTest$RequiresV1_3.xml");
assertThat(resource).isNotNull();
String s = IO.collect(resource.openInputStream());
System.out.println(s);
}
}

public void testValidNamespaceVersion() throws Exception {
Expand Down Expand Up @@ -3508,31 +3512,22 @@ void stop() {}
}

public void testMixedStandardBnd() throws Exception {
Builder b = new Builder();
b.setProperty(Constants.DSANNOTATIONS, "test.component.DSAnnotationTest$MixedStdBnd");
b.setProperty("Private-Package", "test.component");
b.addClasspath(new File("bin_test"));
Jar build = b.build();
System.err.println(b.getErrors());
System.err.println(b.getWarnings());
assertEquals(4, b.getErrors()
.size());
List<String> errors = new ArrayList<>(b.getErrors());
Collections.sort(errors);
assertEquals(
"The DS component mixed-std-bnd uses standard annotations to declare it as a component, but also uses the bnd DS annotation: aQute.bnd.annotation.component.Activate on method start with signature ()V. It is an error to mix these two types of annotations",
errors.get(0));
assertEquals(
"The DS component mixed-std-bnd uses standard annotations to declare it as a component, but also uses the bnd DS annotation: aQute.bnd.annotation.component.Deactivate on method stop with signature ()V. It is an error to mix these two types of annotations",
errors.get(1));
assertEquals(
"The DS component mixed-std-bnd uses standard annotations to declare it as a component, but also uses the bnd DS annotation: aQute.bnd.annotation.component.Modified on method update with signature (Ljava/util/Map;)V. It is an error to mix these two types of annotations",
errors.get(2));
assertEquals(
"The DS component mixed-std-bnd uses standard annotations to declare it as a component, but also uses the bnd DS annotation: aQute.bnd.annotation.component.Reference on method setLog with signature (Lorg/osgi/service/log/LogService;)V. It is an error to mix these two types of annotations",
errors.get(3));
assertEquals(0, b.getWarnings()
.size());
try (Builder b = new Builder()) {
b.setProperty(Constants.DSANNOTATIONS, "test.component.DSAnnotationTest$MixedStdBnd");
b.setProperty("Private-Package", "test.component");
b.addClasspath(new File("bin_test"));
Jar build = b.build();
System.err.println(b.getErrors());
System.err.println(b.getWarnings());
assertThat("foo");
assertThat(b.getErrors())
.anyMatch(e -> e.matches(".*MixedStdBnd\\.stop.*aQute\\.bnd\\.annotation\\.component\\.Deactivate.*"))
.anyMatch(e -> e.matches(".*MixedStdBnd\\.update.*aQute\\.bnd\\.annotation\\.component\\.Modified.*"))
.anyMatch(e -> e.matches(".*MixedStdBnd\\.start.*aQute\\.bnd\\.annotation\\.component\\.Activate.*"))
.anyMatch(e -> e.matches(".*MixedStdBnd\\.setLog.*aQute\\.bnd\\.annotation\\.component\\.Reference.*"))
.hasSize(4);
assertThat(b.getWarnings()).isEmpty();
}
}

@Component
Expand Down
112 changes: 53 additions & 59 deletions biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotationReader.java
View file
Open in desktop
Expand Up @@ -152,11 +152,12 @@ private ComponentDef getDef() throws Exception {
// These types cannot be components so don't bother scanning them

if (clazz.is(ANNOTATED, COMPONENT_INSTR, analyzer)) {
DeclarativeServicesAnnotationError details = new DeclarativeServicesAnnotationError(clazz.getFQN(),
null, ErrorType.INVALID_COMPONENT_TYPE);
analyzer
.error("The type %s is not a class and therfore not suitable for the @Component annotation",
clazz.getFQN())
.details(
new DeclarativeServicesAnnotationError(clazz.getFQN(), null, ErrorType.INVALID_COMPONENT_TYPE));
.error("[%s] The type is not a class and therefore not suitable for the @Component annotation",
details.location())
.details(details);
}
return null;
}
Expand All @@ -179,10 +180,11 @@ private ComponentDef getDef() throws Exception {

Clazz ec = analyzer.findClass(extendsClass);
if (ec == null) {
DeclarativeServicesAnnotationError details = new DeclarativeServicesAnnotationError(
className.getFQN(), null, null, ErrorType.UNABLE_TO_LOCATE_SUPER_CLASS);
analyzer
.error("Missing super class for DS annotations: %s from %s", extendsClass, clazz.getClassName())
.details(new DeclarativeServicesAnnotationError(className.getFQN(), null, null,
ErrorType.UNABLE_TO_LOCATE_SUPER_CLASS));
.error("[%s] Missing super class for DS annotations %s", details.location(), extendsClass)
.details(details);
break;
} else {
ec.parseClassFileWithCollector(this);
Expand Down Expand Up @@ -326,10 +328,9 @@ private void handlePossibleComponentPropertyAnnotation(Annotation annotation) th

if (clazz == null) {
analyzer.warning(
"Unable to determine whether the annotation %s applied to type %s is a component property type as it is not on the project build path. If this annotation is a component property type then it must be present on the build path in order to be processed",
annotation.getName()
.getFQN(),
className.getFQN())
"[%s] Unable to determine whether the annotation %s is a component property type as it is not on the project build path. If this annotation is a component property type then it must be present on the build path in order to be processed",
details.location(), annotation.getName()
.getFQN())
.details(details);
return;
}
Expand All @@ -341,16 +342,15 @@ private void handlePossibleComponentPropertyAnnotation(Annotation annotation) th
new ComponentPropertyTypeDataCollector(propertyDefKey, annotation, details));
} else {
logger.debug(
"The annotation {} on component type {} will not be used for properties as the annotation is not annotated with @ComponentPropertyType",
clazz.getFQN(), className.getFQN());
"The annotation {} on component {} will not be used for properties as the annotation is not annotated with @ComponentPropertyType",
clazz.getFQN(), details.location());
return;
}
} catch (Exception e) {
analyzer
.exception(e, "An error occurred when attempting to process annotation %s, applied to component %s",
.exception(e, "[%s] An error occurred when attempting to process annotation %s", details.location(),
annotation.getName()
.getFQN(),
className.getFQN())
.getFQN())
.details(details);
}
}
Expand Down Expand Up @@ -420,7 +420,7 @@ private void doActivate(Annotation annotation) {
}
}
analyzer
.error("Invalid activation object, type %s for field %s", member.descriptor(), details.fieldName)
.error("[%s] Invalid activation object type %s", details.location(), member.descriptor())
.details(details);

break;
Expand All @@ -429,12 +429,12 @@ private void doActivate(Annotation annotation) {
DeclarativeServicesAnnotationError details = new DeclarativeServicesAnnotationError(className.getFQN(),
member.getName(), memberDescriptor, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR);
if (component.init != null) {
analyzer.error("Multiple constructors for %s are annotated @Activate.", details.className)
analyzer.error("[%s] Multiple constructors are annotated @Activate.", details.location())
.details(details);
break;
}
if (!member.isPublic()) {
analyzer.error("Constructors must be public access.")
analyzer.error("[%s] Constructors must be public access.", details.location())
.details(details);
break;
}
Expand Down Expand Up @@ -507,7 +507,7 @@ private void processMethodActivationArgs(String propertyDefKeyFormat, String mem
} else if (deactivate && (type == BaseType.I)) {
component.updateVersion(V1_1, "deactivate(int)");
} else {
analyzer.error("Invalid activation object type %s for parameter %s", type, arg)
analyzer.error("[%s] Invalid activation object type %s for parameter %s", details.location(), type, arg)
.details(details);
}
}
Expand All @@ -522,7 +522,7 @@ private void processMethodActivationArgs(String propertyDefKeyFormat, String mem
&& ((ClassTypeSignature) resultType).binary.equals("java/util/Map")) {
checkMapReturnType(details);
} else {
analyzer.error("Invalid return type type %s", resultType)
analyzer.error("[%s] Invalid return type type %s", details.location(), resultType)
.details(details);
}
}
Expand All @@ -543,7 +543,9 @@ private void processConstructorActivationArgs(int toArg) {
String propertyDefKey = String.format(ComponentDef.PROPERTYDEF_CONSTRUCTORFORMAT, arg);
processActivationObject(propertyDefKey, param, memberDescriptor, details, false);
} else {
analyzer.error("Invalid activation object type %s for constructor parameter %s", type, arg)
analyzer
.error("[%s] Invalid activation object type %s for constructor parameter %s", details.location(),
type, arg)
.details(details);
}
}
Expand Down Expand Up @@ -576,15 +578,15 @@ private void processActivationObject(String propertyDefKey, ClassTypeSignature p
component.updateVersion(V1_3, "Felix interface type??");
} else {
analyzer
.error("Non annotation type for activation object with descriptor %s, type %s",
memberDescriptor, param.binary)
.error("[%s] Non annotation type for activation object with descriptor %s, type %s",
details.location(), memberDescriptor, param.binary)
.details(details);
}
} catch (Exception e) {
analyzer
.exception(e,
"Exception looking at annotation type for activation object with descriptor %s, type %s",
memberDescriptor, param.binary)
"[%s] Exception looking at annotation type for activation object with descriptor %s, type %s",
details.location(), memberDescriptor, param.binary)
.details(details);
}
break;
Expand Down Expand Up @@ -692,21 +694,23 @@ public void annotationDefault(MethodDef defined, Object value) {
try {
Clazz r = analyzer.findClass(type);
if (r.isAnnotation()) {
analyzer.warning("Nested annotation type found in member %s, %s", name, type.getFQN())
analyzer
.warning("[%s] Nested annotation type found in member %s, %s", details.location(), name,
type.getFQN())
.details(details);
return;
}
} catch (Exception e) {
if (memberDescriptor != null) {
analyzer
.exception(e,
"Exception looking at annotation type on member with descriptor %s, type %s",
memberDescriptor, type)
"[%s] Exception looking at annotation type on member with descriptor %s, type %s",
details.location(), memberDescriptor, type)
.details(details);
} else {
analyzer
.exception(e, "Exception looking at annotation %s applied to type %s", typeRef.getFQN(),
className.getFQN())
.exception(e, "[%s] Exception looking at annotation %s", details.location(),
typeRef.getFQN())
.details(details);
}
}
Expand Down Expand Up @@ -740,8 +744,8 @@ public void classEnd() throws Exception {
} else {
prefix = null;
analyzer.warning(
"Field PREFIX_ in %s is not a static final String field with a compile-time constant value: %s",
typeRef.getFQN(), c)
"[%s] Field PREFIX_ in %s is not a static final String field with a compile-time constant value: %s",
details.location(), typeRef.getFQN(), c)
.details(details);
}
} else {
Expand Down Expand Up @@ -873,7 +877,8 @@ private String identifierToPropertyName(String name) {
break;
default : // unknown!
sb.append(m.group());
analyzer.error("unknown mapping %s in property name %s", m.group(), name);
analyzer.error("[%s] unknown mapping %s in property name %s", details.location(), m.group(),
name);
break;
}
}
Expand Down Expand Up @@ -1371,9 +1376,9 @@ private String determineMethodReferenceType(ReferenceDef def, MethodDef method,
private void checkMapReturnType(DeclarativeServicesAnnotationError details) {
if (!options.contains(Options.felixExtensions)) {
analyzer.error(
"In component %s, to use a return type of Map you must specify the -dsannotations-options felixExtensions flag "
"[%s] To use a return type of Map you must specify the -dsannotations-options felixExtensions flag "
+ " and use a felix extension attribute or explicitly specify the appropriate xmlns.",
className)
details.location())
.details(details);
}
}
Expand All @@ -1389,22 +1394,10 @@ private void doComponent(Component comp, Annotation annotation) throws Exception
if (!mismatchedAnnotations.isEmpty()) {
for (Entry<String, List<DeclarativeServicesAnnotationError>> e : mismatchedAnnotations.entrySet()) {
for (DeclarativeServicesAnnotationError errorDetails : e.getValue()) {
if (errorDetails.fieldName != null) {
analyzer.error(
"The DS component %s uses standard annotations to declare it as a component, but also uses the bnd DS annotation: %s on field %s. It is an error to mix these two types of annotations",
componentName, e.getKey(), errorDetails.fieldName)
.details(errorDetails);
} else if (errorDetails.methodName != null) {
analyzer.error(
"The DS component %s uses standard annotations to declare it as a component, but also uses the bnd DS annotation: %s on method %s with signature %s. It is an error to mix these two types of annotations",
componentName, e.getKey(), errorDetails.methodName, errorDetails.methodSignature)
.details(errorDetails);
} else {
analyzer.error(
"The DS component %s uses standard annotations to declare it as a component, but also uses the bnd DS annotation: %s. It is an error to mix these two types of annotations",
componentName, e.getKey())
.details(errorDetails);
}
analyzer.error(
"[%s] The DS component %s uses standard annotations to declare it as a component, but also uses the bnd DS annotation: %s. It is an error to mix these two types of annotations",
errorDetails.location(), componentName, e.getKey())
.details(errorDetails);
}
}
return;
Expand Down Expand Up @@ -1478,22 +1471,23 @@ private void doComponent(Component comp, Annotation annotation) throws Exception
component.service = Stream.of(declaredServices)
.map(TypeRef.class::cast)
.peek(typeRef -> {
DeclarativeServicesAnnotationError details = new DeclarativeServicesAnnotationError(
className.getFQN(), null, null, ErrorType.INCOMPATIBLE_SERVICE);
try {
Clazz service = analyzer.findClass(typeRef);
if (!analyzer.assignable(clazz, service)) {
analyzer
.error("Class %s is not assignable to specified service %s", clazz.getFQN(),
.error("[%s] The component is not assignable to specified service %s",
details.location(),
typeRef.getFQN())
.details(new DeclarativeServicesAnnotationError(className.getFQN(), null, null,
ErrorType.INCOMPATIBLE_SERVICE));
.details(details);
}
} catch (Exception e) {
analyzer
.exception(e,
"An error occurred when attempting to process service %s, applied to component %s",
typeRef.getFQN(), className.getFQN())
.details(new DeclarativeServicesAnnotationError(className.getFQN(), null, null,
ErrorType.INCOMPATIBLE_SERVICE));
"[%s] An error occurred when attempting to process service %s", details.location(),
typeRef.getFQN())
.details(details);
}
})
.toArray(TypeRef[]::new);
Expand Down
5 changes: 3 additions & 2 deletions biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotations.java
View file
Open in desktop
Expand Up @@ -235,8 +235,9 @@ private void checkVersionConflicts(Analyzer analyzer, ComponentDef definition, V
DeclarativeServicesAnnotationError dse = new DeclarativeServicesAnnotationError(
definition.implementation.getFQN(), null, ErrorType.VERSION_MISMATCH);
analyzer
.error("component %s version %s exceeds -dsannotations-options version;maximum version %s because %s",
definition.version, definition.name, settings.maxVersion, definition.versionReason)
.error(
"[%s] component %s version %s exceeds -dsannotations-options version;maximum version %s because %s",
dse.location(), definition.name, definition.version, settings.maxVersion, definition.versionReason)
.details(dse);
}

Expand Down