Skip to content

Add validation for Plexus-based plugin dependency injection - #12577

Merged
slawekjaranowski merged 3 commits into
apache:masterfrom
slawekjaranowski:desc-requirements
Jul 30, 2026
Merged

Add validation for Plexus-based plugin dependency injection#12577
slawekjaranowski merged 3 commits into
apache:masterfrom
slawekjaranowski:desc-requirements

Conversation

@slawekjaranowski

@slawekjaranowski slawekjaranowski commented Jul 28, 2026

Copy link
Copy Markdown
Member

What changed

  • Added a new validation that detects outdated dependency injection usage in plugins.
  • Added a clear warning for plugin developers when this pattern is found.
  • Pointed users to the recommended modern injection approach.

What changed

- Added a new validation that detects outdated dependency injection usage in plugins.
- Added a clear warning for plugin developers when this pattern is found.
- Pointed users to the recommended modern injection approach.
@slawekjaranowski slawekjaranowski self-assigned this Jul 28, 2026
@slawekjaranowski slawekjaranowski added the enhancement New feature or request label Jul 28, 2026
@slawekjaranowski

Copy link
Copy Markdown
Member Author

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor issues confirmed in the warning message string — the validation logic and test coverage look solid.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

gnodet added a commit to gnodet/maven that referenced this pull request Jul 28, 2026
gnodet added a commit to gnodet/maven that referenced this pull request Jul 28, 2026
@slawekjaranowski
slawekjaranowski requested a review from gnodet July 28, 2026 21:50

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both issues from the prior review have been cleanly addressed in the "Apply review comments" commit:

  • {@code @Component} Javadoc syntax removed from runtime string — now uses (@Component annotation)
  • Grammar/phrasing improved — now reads naturally
  • Unused mock import in the test was also removed

The validation logic is correct, tests cover both positive and negative paths, and CI is green.

Very minor (non-blocking): the constructor does not use requireNonNull(pluginValidationManager) unlike the abstract base classes in this package (AbstractMavenPluginParametersValidator, AbstractMavenPluginDependenciesValidator). With JSR 330 injection this is a non-issue in practice.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest commit adds requireNonNull(pluginValidationManager) in the constructor, cleanly addressing the minor observation from our previous review. The import style (import static java.util.Objects.requireNonNull) and usage pattern exactly match AbstractMavenPluginParametersValidator and AbstractMavenPluginDependenciesValidator in the same package. Consistent and correct.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

@slawekjaranowski
slawekjaranowski merged commit 2a8b6b2 into apache:master Jul 30, 2026
22 checks passed
@github-actions github-actions Bot added this to the 4.1.0 milestone Jul 30, 2026
@slawekjaranowski
slawekjaranowski deleted the desc-requirements branch July 30, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants