[MNG-5695] document Maven 3.2.5+ scoped components usage

[hboutemy]

clarify something which precise scope was unclear until now (at least to me)
=> update https://maven.apache.org/plugin-tools/maven-plugin-tools-annotations/

[slawekjaranowski]

We have a warning for such components ...

https://github.com/apache/maven-plugin-tools/blob/master/maven-plugin-tools-api/src/main/java/org/apache/maven/tools/plugin/util/PluginUtils.java#L61

and Component is replaced by expression
https://github.com/apache/maven-plugin-tools/blob/master/maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java#L791

[gnodet]

We were discussing and leaning to the exact opposite.
Keep @Parameter for things injected from the config and @Component/@Inject for injecting beans, including those from the session (Session) and mojoExecution scopes (MojoExecution, Project)...

[michael-o]

We were discussing and leaning to the exact opposite. Keep @Parameter for things injected from the config and @Component/@Inject for injecting beans, including those from the session (Session) and mojoExecution scopes (MojoExecution, Project)...

This is exactly what I remember. Use @Parameter exactly for this case.

[gnodet]

We were discussing and leaning to the exact opposite. Keep @Parameter for things injected from the config and @Component/@Inject for injecting beans, including those from the session (Session) and mojoExecution scopes (MojoExecution, Project)...

This is exactly what I remember. Use @Parameter exactly for this case.

But those components can also be injected with @Component, so this is weird to warn about something that is supported by sisu (because those are actually bound to scopes), while pushing users to a custom solution. Why only a few components would be accessible by name then ?
I would agree if we had a single annotation for both use cases, but that's not the case in Maven 3. I suppose we could try for the v4 api though.

[michael-o]

We were discussing and leaning to the exact opposite. Keep @Parameter for things injected from the config and @Component/@Inject for injecting beans, including those from the session (Session) and mojoExecution scopes (MojoExecution, Project)...

This is exactly what I remember. Use @Parameter exactly for this case.

But those components can also be injected with @Component, so this is weird to warn about something that is supported by sisu (because those are actually bound to scopes), while pushing users to a custom solution. Why only a few components would be accessible by name then ? I would agree if we had a single annotation for both use cases, but that's not the case in Maven 3. I suppose we could try for the v4 api though.

I agree that this is inconsistent. If both are valid and are functionally identical then the warning is wrong.

[hboutemy]

so do we all now agree that using @Component is better than using @Parameter( defaultValue = "${session}", readonly = true ) (because smaller and moreover we're sure there is no typo in the default value expression)?

if we agree, the warning could be to promote switch to @Component for ${session}, ${project} and ${mojoExecution}...

[michael-o]

and do we all now agree that using @Component is better than using @Parameter( defaultValue = "${session}", readonly = true ) (because smaller and moreover we're sure there is no typo in the default value expression)?

if we agree, the warning could be to promote switch to @Component...

From my PoV, is this cannot be provided by POM or CLI means and it is unambiguous, then yes.

[hboutemy]

reading git blame on https://github.com/apache/maven-plugin-tools/blob/master/maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java#L791 , I see that in the past, we (I :) ) implemented in plugin-tools a hack to replace @Component with an expression in the plugin descriptor: https://issues.apache.org/jira/browse/MPLUGIN-204

It was was done when we were with Maven 3.0.x, if it was before Maven 3.2.5 MNG-5695. With Maven 3.2.5, this hack is not useful any more for MavenSession, MavenProject nor MojoExecution: it remains useful for a few other types...

[cstamas]

IMHO there is no problem when JSR330 being used. Problem happens when Mojo APIs (java5 anno or javadoc annos are being used).

[michael-o]

We were discussing and leaning to the exact opposite. Keep @Parameter for things injected from the config and @Component/@Inject for injecting beans, including those from the session (Session) and mojoExecution scopes (MojoExecution, Project)...

This is exactly what I remember. Use @Parameter exactly for this case.

[hboutemy]

now that we've found back the MPLUGIN-204 hack, in the proposed update:

@Component // since Maven 3.2.5, thanks to MNG-5695

we could perhaps drop the comment on Maven 3.2.5 because the plugin transforms @Component with @Parameter, then it works with any Maven release
the only improvement that could be useful is to NOT transform but keep a real Plexus component if the plugin is targetting 3.2.5+: I don't know if this is feasible

[hboutemy]

in MPLUGIN-204 we advised using @Component (that would use the trick to transform to parameters in the plugin descriptor XML), but later we advised back: f8a946d
see https://issues.apache.org/jira/browse/MPLUGIN-257

[gnodet]

in MPLUGIN-204 we advised using @Component (that would use the trick to transform to parameters in the plugin descriptor XML), but later we advised back: f8a946d see https://issues.apache.org/jira/browse/MPLUGIN-257

That's exactly what I don't understand. Since they are bound to a scope in plexus, they are plexus components, even if they have not been instantiated by plexus. They can be injected into other Plexus components, especially those from the plugin.

[hboutemy]

before Maven 3.2.5, they were not Plexus components: then implementation in plexus-tools was a hack to replace with parameter in plugin descriptor XML

[hboutemy]

on latest discussions on Slack (I hate non-shared Slack diuscussions), there is also the @Inject options that IIUC completely avoids the plugin descriptor intermediate= <requirement> in https://maven.apache.org/ref/3.9.5/maven-plugin-api/plugin.html

I don't know if it works, never tried

[michael-o]

Are we good to merge this? We have two approvals.

[slawekjaranowski]

We also should remove replacements and worning form code:

https://github.com/apache/maven-plugin-tools/blob/master/maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java#L791

changing only documentation users will be confused when use it

[michael-o]

We also should remove replacements and worning form code:

https://github.com/apache/maven-plugin-tools/blob/master/maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java#L791

changing only documentation users will be confused when use it

Right, it needs to be consistent. @cstamas, your turn.

[michael-o]

I took the stab now and change the code: #253. Both PRs should go together in the next release.