diff --git a/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java b/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java index 7e65f5fbd9..1bf2a56742 100644 --- a/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java +++ b/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java @@ -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; @@ -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() @@ -115,22 +116,25 @@ void setX(Map 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 { @@ -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 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 diff --git a/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotationReader.java b/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotationReader.java index f3f1b00acc..1e97b63df2 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotationReader.java +++ b/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotationReader.java @@ -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; } @@ -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); @@ -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; } @@ -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); } } @@ -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; @@ -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; } @@ -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); } } @@ -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); } } @@ -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); } } @@ -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; @@ -692,7 +694,9 @@ 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; } @@ -700,13 +704,13 @@ public void annotationDefault(MethodDef defined, Object value) { 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); } } @@ -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 { @@ -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; } } @@ -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); } } @@ -1389,22 +1394,10 @@ private void doComponent(Component comp, Annotation annotation) throws Exception if (!mismatchedAnnotations.isEmpty()) { for (Entry> 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; @@ -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); diff --git a/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotations.java b/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotations.java index bae7c54c02..abb6dfeed0 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotations.java +++ b/biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotations.java @@ -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); } diff --git a/biz.aQute.bndlib/src/aQute/bnd/component/error/DeclarativeServicesAnnotationError.java b/biz.aQute.bndlib/src/aQute/bnd/component/error/DeclarativeServicesAnnotationError.java index f014a0b597..39b6ef7927 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/component/error/DeclarativeServicesAnnotationError.java +++ b/biz.aQute.bndlib/src/aQute/bnd/component/error/DeclarativeServicesAnnotationError.java @@ -52,4 +52,19 @@ public DeclarativeServicesAnnotationError(String className, String fieldName, Er this.fieldName = fieldName; this.errorType = errorType; } + + public String location() { + if (fieldName != null) { + return String.format("%s.%s", className, fieldName); + } + if (methodName != null) { + return String.format("%s.%s%s", className, methodName, methodSignature); + } + return className; + } + + @Override + public String toString() { + return location() + " " + errorType; + } } diff --git a/biz.aQute.bndlib/src/aQute/bnd/component/error/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/component/error/package-info.java index 1de11718ff..d089b24b55 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/component/error/package-info.java +++ b/biz.aQute.bndlib/src/aQute/bnd/component/error/package-info.java @@ -1,7 +1,7 @@ /** * This package provides Declarative Service Annotation Error information. */ -@Version("1.6.0") +@Version("1.7.0") package aQute.bnd.component.error; import org.osgi.annotation.versioning.Version;