search

bndtools / bnd

Bnd/Bndtools. Tooling to build OSGi bundles including Eclipse, Maven, and Gradle plugins.
https://bndtools.org
Other
532 stars 305 forks source link

[ds] Tooling support to catch property vs properties confusion #6265

Closed kriegfrj closed 2 months ago

kriegfrj commented 2 months ago

Hi all,

I pray that you are well.

I just wasted a good 15-30m because I created a component, and because I hadn't done it for a while I used the "properties" field rather than the "property" field. When I figured out the problem I remembered that I had done the same thing before.

It would be great if the Bnd DS code generation was able to detect this and issue a warning. A generic solution could be:

pkriens commented 2 months ago

No PR? :-(

kriegfrj commented 2 months ago

I'll have a quick look and if it's easy I'll give it a go...

Sorry for not being around as much recently...

kriegfrj commented 2 months ago

Some thoughts:

  • Inspect all filenames specified in the properties annotation field and warn if they do not exist in the finally assembled bundle.

Upon reflection, I don't think that this is feasible. Correct me if I am wrong (I'm a bit rusty), but at the time that the DS generation code runs the bundle has not yet been assembled, so the code would not be able to reliably report on whether the file is present or not.

If this were to be implemented, it would need to be in some kind of post-bundle-assembly verification phase. Does such a thing already exist? If not it might be a nice thing to add, but it would also be getting a bit complicated for my limited time.

  • For any that do not exist, if they are of the form "A=B" then add the suggestion to the warning along the lines of: "Did you mean to use "property" attribute instead?"

In light of the previous reflection, I have reflected further on this one too. I have come up with the following simplified proposal:

In theory, this could generate a warning in some situations that are legal. In practice:

  1. It is highly unlikely that people would intentionally name their property files in this way.
  2. In those rare cases where it was intentional, the warning can probably be fixed easily by renaming the file to something more sensible.
  3. In the even rarer case that it is intentional and necessary, the warning can be suppressed using a -fixupmessage.

If anyone has any alternative thoughts/suggestions, let me know.

kriegfrj commented 2 months ago
  • If a properties attribute contains any filenames of the form "A=B", issue a warning.

Ok, so I implemented something... and in the process I discovered this:

https://github.com/bndtools/bnd/blob/318805bde143048e60edbf0b3ab2d8116cade068/biz.aQute.bndlib/src/aQute/bnd/component/ComponentDef.java#L130-L137

I have confirmed that this is in fact working; however it is a warning only, and it appears on the project's bnd.bnd file only (not in the source where the error actually occurs), so it is easily missed. It also doesn't have any context as to where in your source the property missing property file is specified.

PR to come.