Warn when annotations placed on group definition

[jadams-tresys]

Annotations placed on group definitions will be ignored, so we should warn users that this is occurring.

DAFFODIL-2668

[mbeckerle]

Need to check that the annotation is specifically an appinfo annotation with source="http://www.ogf.org/dfdl/".
Those should create the warning.

You might want to tolerate not a perfect match on the source URI.

See annotatedSchemaComponent.scala

Look for lazy val dfdlAppInfos = { ... for this logic.

The common logic for finding a DFDL appinfo, tolerating small imperfections in the URI, should be shared between here and there. I'd suggest the common code belongs in XMLUtils, called from both places.

[tuxji]

Agreed. Will expect a new commit to check appinfo and fix formatting.

[jadams-tresys]

I've moved the logic to XMLUtils via a getDFDLAppinfos function. However, it loses the ability to send schema definition warnings and I'm not sure what a good solution is to get that functionality back. Should there just be another pass through all the appinfos in AnnotatedSchemaComponent to issue warnings as needed? I thought about having my function return a list of warnings in addition to the list of appinfos and then doing SDW's back in AnnotatedSchemaComponent, but it felt very hacky to have a function returning a tuple with a list containing a tuple for WarnID, String.

[mbeckerle]

Suggest make dfdlAppInfos return a Either[Seq[(WarnID, String)], Seq[DFDLAppInfo]]
where Left is warnings to issue, Right is the appinfo object(s) if all goes well.

[stevedlawrence]

Maybe another alternative, doesn't GlobalGroupDef implement AnnotatedSchemaComponet so it has access to dfdlAppInfos? Can the new check function just do something like:

if (dfdlAppInfos.length > 0) SDW(...)

So you reuse the existing dfdlAppInfos logic to find any dfdl annotations (which might warn), but then we can warn that they will be ignored in the check?

[jadams-tresys]

So it does implement AnnotatedSchemaComponent, but only on the groupXML, which is just the group members I believe. It doesn't include the actual group definition, which is how the annotations for group definitions were getting ignored before.

[stevedlawrence]

I see. Maybe instead of making it an XMLUtils function it becomes a public function in AnnotatedMixin that accepts the XML node to look for annotations as a paramter? This way it could still create warnings like normal, and dfdlAppInfo could pass in xml while the GroupDef check could pass in defXML? Just trying to avoid having a complex return type.

[jadams-tresys]

I like this suggestion from Steve. Changes for it have been pushed up for review.

[mbeckerle]

+1

[stevedlawrence]

+1

[stevedlawrence]

As an alternative to the group reference, could annotations also be placed on the model group (i.e. choice or sequence) of the group definition? I imagine if someone put an annotation on the group def that's maybe what they meant to do? Is it worth saying something like ... should be placed on the group reference or model group?

[tuxji]

Saying "or model group" may confuse a user who isn't familiar with the meaning of "model group." I suggest either leaving the message the way it is now or changing it to ... should be placed on the group reference, choice, or sequence if we really want to list all the possible places.

[mbeckerle]

+1. This is cleaner than what I was suggesting.

[tuxji]

+1

[tuxji]

Saying "or model group" may confuse a user who isn't familiar with the meaning of "model group." I suggest either leaving the message the way it is now or changing it to ... should be placed on the group reference, choice, or sequence if we really want to list all the possible places.