Skip to content

Derive source file from builder when processing DS annotations #6458

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

Merged
pkriens merged 1 commit into from
Feb 14, 2025
Merged

Derive source file from builder when processing DS annotations #6458

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 85 additions & 109 deletions biz.aQute.bndlib/src/aQute/bnd/component/DSAnnotationReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,9 @@ private ComponentDef getDef() throws Exception {
if (clazz.is(ANNOTATED, COMPONENT_INSTR, analyzer)) {
DeclarativeServicesAnnotationError details = new DeclarativeServicesAnnotationError(clazz.getFQN(),
null, ErrorType.INVALID_COMPONENT_TYPE);
analyzer
.error("[%s] The type is not a class and therefore not suitable for the @Component annotation",
details.location())
.details(details);
details.addError(analyzer,
"[%s] The type is not a class and therefore not suitable for the @Component annotation",
details.location());
}
return null;
}
Expand All @@ -183,8 +182,8 @@ private ComponentDef getDef() throws Exception {
if (ec == null) {
DeclarativeServicesAnnotationError details = new DeclarativeServicesAnnotationError(
className.getFQN(), null, null, ErrorType.UNABLE_TO_LOCATE_SUPER_CLASS);
analyzer.error("[%s] Missing super class for DS annotations %s", details.location(), extendsClass)
.details(details);
details.addError(analyzer, "[%s] Missing super class for DS annotations %s", details.location(),
extendsClass);
break;
} else {
ec.parseClassFileWithCollector(this);
Expand All @@ -196,11 +195,11 @@ private ComponentDef getDef() throws Exception {
rdef.unbind = referredMethod(analyzer, rdef, rdef.unbind, unbind1, unbind2);
rdef.updated = referredMethod(analyzer, rdef, rdef.updated, updated1, updated2);

if (rdef.policy == ReferencePolicy.DYNAMIC && rdef.unbind == null)
analyzer
.error("In component class %s, reference %s is dynamic but has no unbind method.",
className.getFQN(), rdef.name)
.details(getDetails(rdef, ErrorType.DYNAMIC_REFERENCE_WITHOUT_UNBIND));
if (rdef.policy == ReferencePolicy.DYNAMIC && rdef.unbind == null) {
addError(rdef, ErrorType.DYNAMIC_REFERENCE_WITHOUT_UNBIND,
"In component class %s, reference %s is dynamic but has no unbind method.",
className.getFQN(), rdef.name);
}
}
}
return component;
Expand Down Expand Up @@ -415,22 +414,20 @@ private void doActivate(Annotation annotation) {
break;
}
}
analyzer.error("[%s] Invalid activation object type %s", details.location(), member.descriptor())
.details(details);

details.addError(analyzer, "[%s] Invalid activation object type %s", details.location(),
member.descriptor());
break;
}
case CONSTRUCTOR : {
DeclarativeServicesAnnotationError details = new DeclarativeServicesAnnotationError(className.getFQN(),
member.getName(), memberDescriptor, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR);
if (component.init != null) {
analyzer.error("[%s] Multiple constructors are annotated @Activate.", details.location())
.details(details);
details.addError(analyzer, "[%s] Multiple constructors are annotated @Activate.",
details.location());
break;
}
if (!member.isPublic()) {
analyzer.error("[%s] Constructors must be public access.", details.location())
.details(details);
details.addError(analyzer, "[%s] Constructors must be public access.", details.location());
break;
}
constructorSig = methodSig;
Expand Down Expand Up @@ -501,8 +498,8 @@ private void processMethodActivationArgs(String propertyDefKeyFormat, String mem
} else if (deactivate && (type == BaseType.I)) {
component.updateVersion(V1_1, "deactivate(int)");
} else {
analyzer.error("[%s] Invalid activation object type %s for parameter %s", details.location(), type, arg)
.details(details);
details.addError(analyzer, "[%s] Invalid activation object type %s for parameter %s",
details.location(), type, arg);
}
}
Result resultType = resolver.resolveResult();
Expand All @@ -515,8 +512,7 @@ private void processMethodActivationArgs(String propertyDefKeyFormat, String mem
if ((resultType instanceof ClassTypeSignature param) && param.binary.equals("java/util/Map")) {
checkMapReturnType(details);
} else {
analyzer.error("[%s] Invalid return type type %s", details.location(), resultType)
.details(details);
details.addError(analyzer, "[%s] Invalid return type type %s", details.location(), resultType);
}
}

Expand All @@ -535,10 +531,8 @@ private void processConstructorActivationArgs(int toArg) {
String propertyDefKey = String.format(ComponentDef.PROPERTYDEF_CONSTRUCTORFORMAT, arg);
processActivationObject(propertyDefKey, param, memberDescriptor, details, false);
} else {
analyzer
.error("[%s] Invalid activation object type %s for constructor parameter %s", details.location(),
type, arg)
.details(details);
details.addError(analyzer, "[%s] Invalid activation object type %s for constructor parameter %s",
details.location(), type, arg);
}
}
}
Expand Down Expand Up @@ -569,10 +563,9 @@ private void processActivationObject(String propertyDefKey, ClassTypeSignature p
} else if (clazz.isInterface() && options.contains(Options.felixExtensions)) {
component.updateVersion(V1_3, "Felix interface type??");
} else {
analyzer
.error("[%s] Non annotation type for activation object with descriptor %s, type %s",
details.location(), memberDescriptor, param.binary)
.details(details);
details.addError(analyzer,
"[%s] Non annotation type for activation object with descriptor %s, type %s",
details.location(), memberDescriptor, param.binary);
}
} catch (Exception e) {
analyzer.exception(e,
Expand Down Expand Up @@ -949,18 +942,17 @@ private void doReference(Reference reference, Annotation annotation) throws Exce
if (m.matches()) {
def.name = m.group("name");
} else {
analyzer.error("In component '%s', invalid name for bind method '%s'", className, def.bind)
.details(getDetails(def, ErrorType.INVALID_REFERENCE_BIND_METHOD_NAME));
addError(def, ErrorType.INVALID_REFERENCE_BIND_METHOD_NAME,
"In component '%s', invalid name for bind method '%s'", className, def.bind);
}
}

def.service = determineMethodReferenceType(def, (MethodDef) member, methodSig, annoService);

if (def.service == null) {
analyzer
.error("In component '%s', cannot recognize the signature of method '%s': %s",
className, def.bind, methodSig)
.details(getDetails(def, ErrorType.REFERENCE));
addError(def, ErrorType.REFERENCE,
"In component '%s', cannot recognize the signature of method '%s': %s", className, def.bind,
methodSig);
}
break;
}
Expand Down Expand Up @@ -992,10 +984,9 @@ private void doReference(Reference reference, Annotation annotation) throws Exce
}
if (def.isOptional && (def.cardinality != ReferenceCardinality.OPTIONAL)
&& (def.cardinality != ReferenceCardinality.MANDATORY)) {
analyzer.error(
addError(def, ErrorType.OPTIONAL_FIELD_WITH_MULTIPLE,
"In component '%s', field '%s' is 'Optional' but cardinality is not '0..1' or '1..1'.",
className, def.field)
.details(getDetails(def, ErrorType.OPTIONAL_FIELD_WITH_MULTIPLE));
className, def.field);
}
}
if ((def.fieldOption == null) && (def.policy == ReferencePolicy.DYNAMIC)
Expand All @@ -1006,62 +997,52 @@ private void doReference(Reference reference, Annotation annotation) throws Exce
}
if (def.fieldOption == FieldOption.UPDATE) {
if (def.policy != ReferencePolicy.DYNAMIC) {
analyzer
.error(
addError(def, ErrorType.UPDATE_FIELD_WITH_STATIC,
"In component '%s', field '%s' fieldOption is 'update' but policy is not 'dynamic'.",
className, def.field)
.details(getDetails(def, ErrorType.UPDATE_FIELD_WITH_STATIC));
className, def.field);
}
if ((def.cardinality != ReferenceCardinality.MULTIPLE)
&& (def.cardinality != ReferenceCardinality.AT_LEAST_ONE)) {
analyzer.error(
addError(def, ErrorType.UPDATE_FIELD_WITH_UNARY,
"In component '%s', field '%s' fieldOption is 'update' but cardinality is not '0..n' or '1..n'.",
className, def.field)
.details(getDetails(def, ErrorType.UPDATE_FIELD_WITH_UNARY));
className, def.field);
}
} else { // def.fieldOption == FieldOption.REPLACE
if (member.isFinal()) {
analyzer
.error(
addError(def, ErrorType.FINAL_FIELD_WITH_REPLACE,
"In component '%s', field '%s' has fieldOption 'replace' but the field is final.",
className,
def.field)
.details(getDetails(def, ErrorType.FINAL_FIELD_WITH_REPLACE));
def.field);
}
if ((def.policy == ReferencePolicy.DYNAMIC) && !member.isVolatile()) {
analyzer
.error("In component '%s', field '%s' policy is 'dynamic' and field is not volatile.",
className, def.field)
.details(getDetails(def, ErrorType.DYNAMIC_FIELD_NOT_VOLATILE));
addError(def, ErrorType.DYNAMIC_FIELD_NOT_VOLATILE,
"In component '%s', field '%s' policy is 'dynamic' and field is not volatile.",
className, def.field);
}
if (def.isNotDirectlyListOrCollection) {
analyzer.error(
addError(def, ErrorType.COLLECTION_SUBCLASS_FIELD_WITH_REPLACE,
"In component '%s', field '%s' is a collection type with fieldOption 'replace' but the type of the field is not declared as 'List' or 'Collection'.",
className, def.field)
.details(getDetails(def, ErrorType.COLLECTION_SUBCLASS_FIELD_WITH_REPLACE));
className, def.field);
}
}
} else {
analyzer
.error("In component '%s', cannot recognize the signature of field '%s': %s",
className, def.field, (type != null) ? type : member.descriptor())
.details(getDetails(def, ErrorType.REFERENCE));
addError(def, ErrorType.REFERENCE,
"In component '%s', cannot recognize the signature of field '%s': %s", className, def.field,
(type != null) ? type : member.descriptor());
}
break;
}
case PARAMETER : {
if (!"<init>".equals(member.getName())) {
analyzer.error(
addError(def, ErrorType.REFERENCE,
"In component '%s', @Reference can be used on parameters only for constructors; method '%s' is not a constructor",
className, member)
.details(getDetails(def, ErrorType.REFERENCE));
className, member);
return;
}
if (constructorSig == null) {
analyzer.error(
addError(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR,
"In component '%s', @Reference can only be used for parameters on the constructor annotated @Activate",
className)
.details(getDetails(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR));
className);
return;
}

Expand All @@ -1086,8 +1067,8 @@ private void doReference(Reference reference, Annotation annotation) throws Exce
def.service = determineReferenceType(def, type, resolver, annoService);
if (def.service != null) {
if (def.policy == ReferencePolicy.DYNAMIC) {
analyzer.error("In component '%s', constructor parameters may not be dynamic", className)
.details(getDetails(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR));
addError(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR,
"In component '%s', constructor parameters may not be dynamic", className);
def.policy = ReferencePolicy.STATIC;
}
if (def.isCollection) {
Expand All @@ -1100,40 +1081,35 @@ private void doReference(Reference reference, Annotation annotation) throws Exce
}
if (def.isOptional && (def.cardinality != ReferenceCardinality.OPTIONAL)
&& (def.cardinality != ReferenceCardinality.MANDATORY)) {
analyzer.error(
addError(def, ErrorType.OPTIONAL_FIELD_WITH_MULTIPLE,
"In component '%s', constructor argument %s is 'Optional' but cardinality is not '0..1' or '1..1'.",
className, def.parameter)
.details(getDetails(def, ErrorType.OPTIONAL_FIELD_WITH_MULTIPLE));
className, def.parameter);
}
if (def.isNotDirectlyListOrCollection) {
analyzer.error(
addError(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR,
"In component '%s', constructor argument %s is a subclass of Collection but it must be declared as either List or Collection",
className, def.parameter)
.details(getDetails(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR));
className, def.parameter);
}
}
if (def.fieldOption == FieldOption.UPDATE) {
analyzer.error(
addError(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR,
"In component '%s', constructor argument %s is marked with 'update' fieldOption. Changing this to 'replace'.",
className, def.parameter)
.details(getDetails(def, ErrorType.CONSTRUCTOR_SIGNATURE_ERROR));
className, def.parameter);
def.fieldOption = null;
}
} else {
analyzer.error(
addError(def, ErrorType.REFERENCE,
"In component '%s', cannot recognize the signature of constructor argument %s: %s", className,
def.parameter, type)
.details(getDetails(def, ErrorType.REFERENCE));
def.parameter, type);
}
break;
}
case TYPE : { // reference element of Component annotation
def.service = annoService;
if (def.name == null) {
analyzer.error(
addError(def, ErrorType.MISSING_REFERENCE_NAME,
"In component '%s', 'name' must be specified for a @Reference specified in the @Component annotation. Service: %s",
className, def.service)
.details(getDetails(def, ErrorType.MISSING_REFERENCE_NAME));
className, def.service);
return;
}
break;
Expand All @@ -1146,35 +1122,31 @@ private void doReference(Reference reference, Annotation annotation) throws Exce
if (def.target != null) {
String error = Verifier.validateFilter(def.target);
if (error != null) {
analyzer
.error("In component '%s', invalid target filter %s for %s: %s", className, def.target, def.name,
error)
.details(getDetails(def, ErrorType.INVALID_TARGET_FILTER));
addError(def, ErrorType.INVALID_TARGET_FILTER, "In component '%s', invalid target filter %s for %s: %s",
className, def.target, def.name, error);
}
}

if ("org.osgi.service.component.AnyService".equals(def.service)) {
def.updateVersion(V1_5, "use of AnyService");
if (def.target == null) {
analyzer
.error("In component '%s', @Reference name '%s' uses 'AnyService' without specifying 'target'.",
className, def.name)
.details(getDetails(def, ErrorType.ANYSERVICE_NO_TARGET));
addError(def, ErrorType.ANYSERVICE_NO_TARGET,
"In component '%s', @Reference name '%s' uses 'AnyService' without specifying 'target'.", className,
def.name);
}
}

if (component.references.containsKey(def.name)) {
analyzer
.error("In component '%s', multiple references with the same name: %s. Previous def: %s, this def: %s",
className, component.references.get(def.name), def.service, "")
.details(getDetails(def, ErrorType.MULTIPLE_REFERENCES_SAME_NAME));
addError(def, ErrorType.MULTIPLE_REFERENCES_SAME_NAME, "In component '%s', multiple references with the same name: %s. Previous def: %s, this def: %s",
className, component.references.get(def.name), def.service, "");
} else {
component.references.put(def.name, def);
}
}

private DeclarativeServicesAnnotationError getDetails(ReferenceDef def, ErrorType type) {
if (def == null) {

return null;
}

Expand Down Expand Up @@ -1406,12 +1378,10 @@ private String determineMethodReferenceType(ReferenceDef def, MethodDef method,

private void checkMapReturnType(DeclarativeServicesAnnotationError details) {
if (!options.contains(Options.felixExtensions)) {
analyzer
.error(
"[%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.",
details.location())
.details(details);
details.addError(analyzer,
"[%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.",
details.location());
}
}

Expand All @@ -1426,10 +1396,9 @@ 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()) {
analyzer.error(
errorDetails.addError(analyzer,
"[%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);
errorDetails.location(), componentName, e.getKey());
}
}
return;
Expand Down Expand Up @@ -1505,10 +1474,8 @@ private void doComponent(Component comp, Annotation annotation) throws Exception
try {
Clazz service = analyzer.findClass(typeRef);
if (!analyzer.assignable(clazz, service)) {
analyzer
.error("[%s] The component is not assignable to specified service %s",
details.location(), typeRef.getFQN())
.details(details);
details.addError(analyzer, "[%s] The component is not assignable to specified service %s",
details.location(), typeRef.getFQN());
}
} catch (Exception e) {
analyzer
Expand Down Expand Up @@ -1642,4 +1609,13 @@ public String toString() {
}
return clazz + "#" + member;
}

private void addError(ReferenceDef def, ErrorType type, String message, Object... params) {
DeclarativeServicesAnnotationError details = getDetails(def, type);
if (details == null) {
analyzer.error(message, params);
} else {
details.addError(analyzer, message, params);
}
}
}
Loading