✨ Spring 6 migration rules#359
Open
jmle wants to merge 4 commits into
Open
Conversation
This PR adds comprehensive rules to assist with migrating from Spring Framework 5.x to 6.0, addressing 11 open issues. ## New Rules Added ### Removed APIs (spring-framework-5.x-to-6.0-removed-apis.yaml) - 00010: RPC-style remoting removed (RMI, HTTP Invoker, Hessian, JAX-WS) - 00020: Dedicated EJB access removed ### Core Container (spring-framework-5.x-to-6.0-core-container.yaml) - 00040: javax.inject.Inject must migrate to jakarta.inject.Inject - 00050: LocalVariableTableParameterNameDiscoverer deprecated ### Data Access (spring-framework-5.x-to-6.0-data-access.yaml) - 00040: Deprecated exceptions (CannotSerializeTransactionException, DeadlockLoserDataAccessException) - 00050: JDBC exception translator default changed - 00060: Hibernate Validator 7.0+ required ### Web Applications (spring-framework-5.x-to-6.0-web-applications.yaml) - 00050: CommonsMultipartResolver removed - 00060: Apache Tiles integration removed - 00070: FreeMarker JSP support removed - 00080: Servlet mocks require Servlet API 6.0 - 00090: Web server upgrade required (Tomcat 10.1+, Jetty 11+, Undertow 2.2.19+) ### HttpClient (spring-framework-5.x-to-6.0-httpclient.yaml) - NEW FILE - 00010: Apache HttpClient 4.x code must migrate to 5.x - 00020: Apache HttpClient 4.x dependency detected ## Issues Addressed Fixes konveyor#134, konveyor#135, konveyor#137, konveyor#139, konveyor#146, konveyor#147, konveyor#149, konveyor#150, konveyor#155, konveyor#167, konveyor#175 ## Issues Not Addressed See SPRING_MIGRATION_UNADDRESSED.md for issues already assigned to other contributors or covered by existing rules. Signed-off-by: Dylan Murray <dymurray@redhat.com>
Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com>
…ing-framework Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Juan Manuel Leflet Estrada <jleflete@redhat.com>
Contributor
📝 WalkthroughWalkthroughThis pull request adds comprehensive Spring Framework 5.x to 6.0 migration rules across multiple categories (core container, data access, removed APIs, and web applications), including new test data examples and test configurations. Additionally, migration documentation is added to track unaddressed issues. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
stable/java/spring-framework/tests/data/web-applications/src/main/java/org/konveyor/test/ServletMockTest.java (1)
14-29: Clarify that this is fixture data, not a runnable unit test.Line 14 and Line 28 read like an executable test, but there is no
@Testannotation or assertions. If this is intentionally static-analysis fixture code, consider renaming the method or adding an explicit note to avoid confusion for future maintainers.Suggested clarification diff
public class ServletMockTest { - public void testRequestHandling() { + // Fixture method for rule detection; intentionally not a JUnit test. + public void sampleRequestHandling() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); request.setRequestURI("/api/users"); request.addHeader("Accept", "application/json"); @@ - // Perform test assertions... + // Intentionally no assertions: this file is test-data for migration rules. } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stable/java/spring-framework/tests/data/web-applications/src/main/java/org/konveyor/test/ServletMockTest.java` around lines 14 - 29, The method testRequestHandling in class ServletMockTest is ambiguous because it looks like a runnable test but lacks `@Test` and assertions; clarify intent by either renaming the method to a fixture-style name (e.g., createMockRequestFixture or buildMockRequest) or adding an explicit JavaDoc/comment above testRequestHandling stating "fixture data only — not a runnable unit test", and optionally annotate with `@Disabled` or a custom marker to prevent test runners from treating it as a test; update any references to the method name if you rename it and keep the MockHttpServletRequest/Response/Session setup unchanged.stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-httpclient.test.yaml (1)
1-7: This test file is a no-op; add at least one test case or remove it.With
tests: [], there is no executable validation for the referenced ruleset, so regressions can slip through unnoticed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-httpclient.test.yaml` around lines 1 - 7, The YAML test file currently contains an empty tests array ("tests: []"), making it a no-op; either add at least one concrete test case object to the tests array that exercises the ruleset referenced by rulesPath (e.g., include a test entry with name, provider/java, dataPath ./data/httpclient and input/expected assertions) or remove the file entirely; update the tests array in stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-httpclient.test.yaml (referencing rulesPath and providers) so the ruleset is actually validated.stable/java/spring-framework/spring-framework-5.x-to-6.0-httpclient.yaml (1)
1-4: This manifest is currently a no-op; either add concrete rules or remove it.An empty ruleset (
[]) with explanatory comments makes ownership ambiguous and can hide future regressions in dedicated HttpClient migration coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-httpclient.yaml` around lines 1 - 4, This manifest contains an empty ruleset ("[]") with only comments—either populate it with concrete rules covering the RestTemplate/HttpComponentsClientHttpRequestFactory → Apache HttpClient 5 migration (e.g., add rules similar to or referencing spring-framework-5.x-to-6.0-web-applications-00020) or delete the file to avoid silent/no-op coverage; update or add YAML entries instead of leaving the root array empty and ensure the comment is accompanied by at least one rule identifier or a clear link to the existing rule set.stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-web-applications.test.yaml (2)
13-27: Offer to resolve the FIXME for commented-out 00010 and 00030 rules.The FIXME comment at line 13 blocks test coverage for
web-applications-00010andweb-applications-00030. If you'd like, I can help draft test fixtures or test case entries for those rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-web-applications.test.yaml` around lines 13 - 27, Unblock the two commented rules by restoring their rule blocks for ruleID "spring-framework-5.x-to-6.0-web-applications-00010" and "spring-framework-5.x-to-6.0-web-applications-00030": remove the leading comment markers and add proper testCases entries for each rule (for example create a tc-1 entry with analysisParams.mode set to "source-only" and hasIncidents exactly matching the expected counts — 1 for web-applications-00010 and 6 for web-applications-00030), ensuring the YAML syntax is valid and consistent with existing test case entries such as using the same keys (testCases, name, analysisParams, hasIncidents).
28-34: Inconsistent test case naming: 00020 uses generictc-1while peer rules use descriptive names.
web-applications-00050→tc-commons-multipart,00060→tc-tiles,00080→tc-servlet-mocks, butweb-applications-00020still usestc-1. Align for consistency.✏️ Proposed rename
- - name: tc-1 + - name: tc-httpclient5(or another descriptive name matching the HttpClient 5 / RestTemplate migration intent of rule 00020.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-web-applications.test.yaml` around lines 28 - 34, Rename the generic test case name "tc-1" under ruleID spring-framework-5.x-to-6.0-web-applications-00020 to a descriptive identifier consistent with peers (e.g., "tc-httpclient5-resttemplate" or another name reflecting the HttpClient 5 / RestTemplate migration intent), updating the testCases.name value so it matches the naming convention used by other rules like tc-commons-multipart and tc-tiles.stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-core-container.test.yaml (1)
34-47: LGTM — consider pinning exact incident counts once test data stabilizes.Both new test cases use
atLeast: 1, which is fine while test data is still being shaped. Once the fixture files are final, switching toexactly: N(as the pre-existing rules do) would catch future regressions where the rule fires more or fewer times than expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-core-container.test.yaml` around lines 34 - 47, Update the two new test cases (name: tc-javax-inject and name: tc-parameter-discoverer) to assert an exact incident count once fixtures are stable: replace the current hasIncidents: atLeast: 1 entries with hasIncidents: exactly: <N> using the true expected counts for each test; locate the entries by the test case names and change the key from "atLeast" to "exactly" and set the appropriate integer value.stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-removed-apis.test.yaml (1)
12-13: Relaxing 00001 fromexactly: 2toatLeast: 1reduces regression safety.If this is intentional (e.g., because the data path change causes the ehcache fixture to have a different hit count), consider documenting why; otherwise the original exact count is safer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-removed-apis.test.yaml` around lines 12 - 13, The test relaxation changed hasIncidents from exactly: 2 to atLeast: 1 which reduces regression safety; revert the assertion to hasIncidents: exactly: 2 in stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-removed-apis.test.yaml to restore the original guard, or if the change is intentional (e.g., known data-path/fixture variation), keep atLeast: 1 but add an inline comment or accompanying test metadata documenting the reason and the rationale why the expected incident count can vary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SPRING_MIGRATION_UNADDRESSED.md`:
- Line 26: The "Covered By" entry for item `#154` is ambiguous because it uses
"Related to `#175`" as a cross-reference; update the entry for "#154" so it lists
the exact rule ID(s) that implement or cover it (e.g., replace "Related to `#175`"
with the concrete rule identifier(s) such as "httpclient-00010" or
"web-applications-00020"), ensuring traceability by referencing the specific
rule ID(s) that address `#154` rather than a generic related-ticket reference.
- Line 12: The table row for issue `#152` in SPRING_MIGRATION_UNADDRESSED.md
incorrectly lists the PR author "jmle" under the "Issues Assigned to Other
Contributors" section; either move the row to the appropriate "Assigned to PR
Authors" category or change the assignee and add a brief explanatory note —
update the table entry for issue "#152" (the row with "Create rules for
controller detection") to reflect the correct assignee or add a parenthetical
note like "(self-assigned; deferred — see reason)" so the listing is not
self-referential and the document accurately reflects why it was skipped or
reassigned.
In
`@stable/java/spring-framework/spring-framework-5.x-to-6.0-core-container.yaml`:
- Around line 171-174: The rule's when clause only detects java.referenced
pattern "javax.inject.Inject" at ANNOTATION so usages of "javax.inject.Named"
are missed; update the when block for the rule that checks java.referenced to
include an OR combining patterns for "javax.inject.Inject" and
"javax.inject.Named" (both at location ANNOTATION) so that either annotation
triggers the rule; ensure the unique symbol names "javax.inject.Inject" and
"javax.inject.Named" are added as separate alternatives under the
java.referenced condition.
In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-data-access.yaml`:
- Around line 307-323: The rule ruleID
spring-framework-5.x-to-6.0-data-access-00060 currently duplicates rule 00010 by
checking the same dependency; update rule 00060's when clause to avoid overlap
by removing or narrowing the dependency-based trigger (the java.dependency block
for org.hibernate.validator.hibernate-validator) and keep only the import-based
detection (the java.referenced pattern javax.validation* with location IMPORT),
or alternatively add a negation so 00060 only fires when java.dependency does
NOT match the vulnerable range; reference ruleID
spring-framework-5.x-to-6.0-data-access-00060 and the java.dependency /
java.referenced conditions when making the change.
In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-removed-apis.yaml`:
- Around line 139-144: The rule currently matches only Java references (patterns
org.springframework.ejb.access.* and org.springframework.ejb.config.*) so it
misses XML-based EJB config usage; extend the rule to also detect Spring JEe XML
namespace usages by adding checks for the jee namespace (e.g., namespace URI
"http://www.springframework.org/schema/jee" or prefix "jee") and element names
like jee:local-slsb and jee:remote-slsb so XML configs are flagged alongside the
existing Java patterns.
- Around line 118-124: The example uses
RestClient/RestClientAdapter/HttpServiceProxyFactory.builderFor which are only
available in Spring 6.1+, so replace the bean definition for userService and the
usage of RestClient/RestClientAdapter with a Spring 6.0-compatible approach (for
example create a RestTemplate or WebClient bean and use it inside a UserService
implementation or expose a UserService client that delegates to
RestTemplate/WebClient); update references to the factory method names (remove
HttpServiceProxyFactory.builderFor and RestClient.Builder usage) and provide a
clear example that constructs a RestTemplate or WebClient and injects it into
UserService (or a UserServiceClient) so the snippet compiles under Spring 6.0.
In
`@stable/java/spring-framework/spring-framework-5.x-to-6.0-web-applications.yaml`:
- Around line 382-383: Update the Servlet mocks note to reference the correct
API: replace the non-existent MockHttpServletRequestBuilder.setRemoteAddress()
with the actual method MockHttpServletRequestBuilder.remoteAddress(String) and
mention it is available since Spring Framework 6.0.10; locate the note text
mentioning MockHttpServletRequestBuilder and change the method name and
signature accordingly.
In
`@stable/java/spring-framework/tests/data/core-container/src/main/java/org/konveyor/inject/JavaxInjectService.java`:
- Around line 13-19: The class JavaxInjectService currently has mixed injection
on the same field: the private field "repository" is annotated with `@Inject` and
the constructor JavaxInjectService(JavaxInjectRepository repository) is also
annotated with `@Inject` and assigns the same field, causing duplicate injection;
fix this by removing the `@Inject` annotation from the field declaration (leave
the private JavaxInjectRepository repository; field unannotated) and keep the
constructor-level `@Inject` and its assignment, so only constructor injection is
used for JavaxInjectService.
---
Nitpick comments:
In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-httpclient.yaml`:
- Around line 1-4: This manifest contains an empty ruleset ("[]") with only
comments—either populate it with concrete rules covering the
RestTemplate/HttpComponentsClientHttpRequestFactory → Apache HttpClient 5
migration (e.g., add rules similar to or referencing
spring-framework-5.x-to-6.0-web-applications-00020) or delete the file to avoid
silent/no-op coverage; update or add YAML entries instead of leaving the root
array empty and ensure the comment is accompanied by at least one rule
identifier or a clear link to the existing rule set.
In
`@stable/java/spring-framework/tests/data/web-applications/src/main/java/org/konveyor/test/ServletMockTest.java`:
- Around line 14-29: The method testRequestHandling in class ServletMockTest is
ambiguous because it looks like a runnable test but lacks `@Test` and assertions;
clarify intent by either renaming the method to a fixture-style name (e.g.,
createMockRequestFixture or buildMockRequest) or adding an explicit
JavaDoc/comment above testRequestHandling stating "fixture data only — not a
runnable unit test", and optionally annotate with `@Disabled` or a custom marker
to prevent test runners from treating it as a test; update any references to the
method name if you rename it and keep the
MockHttpServletRequest/Response/Session setup unchanged.
In
`@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-core-container.test.yaml`:
- Around line 34-47: Update the two new test cases (name: tc-javax-inject and
name: tc-parameter-discoverer) to assert an exact incident count once fixtures
are stable: replace the current hasIncidents: atLeast: 1 entries with
hasIncidents: exactly: <N> using the true expected counts for each test; locate
the entries by the test case names and change the key from "atLeast" to
"exactly" and set the appropriate integer value.
In
`@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-httpclient.test.yaml`:
- Around line 1-7: The YAML test file currently contains an empty tests array
("tests: []"), making it a no-op; either add at least one concrete test case
object to the tests array that exercises the ruleset referenced by rulesPath
(e.g., include a test entry with name, provider/java, dataPath ./data/httpclient
and input/expected assertions) or remove the file entirely; update the tests
array in
stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-httpclient.test.yaml
(referencing rulesPath and providers) so the ruleset is actually validated.
In
`@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-removed-apis.test.yaml`:
- Around line 12-13: The test relaxation changed hasIncidents from exactly: 2 to
atLeast: 1 which reduces regression safety; revert the assertion to
hasIncidents: exactly: 2 in
stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-removed-apis.test.yaml
to restore the original guard, or if the change is intentional (e.g., known
data-path/fixture variation), keep atLeast: 1 but add an inline comment or
accompanying test metadata documenting the reason and the rationale why the
expected incident count can vary.
In
`@stable/java/spring-framework/tests/spring-framework-5.x-to-6.0-web-applications.test.yaml`:
- Around line 13-27: Unblock the two commented rules by restoring their rule
blocks for ruleID "spring-framework-5.x-to-6.0-web-applications-00010" and
"spring-framework-5.x-to-6.0-web-applications-00030": remove the leading comment
markers and add proper testCases entries for each rule (for example create a
tc-1 entry with analysisParams.mode set to "source-only" and hasIncidents
exactly matching the expected counts — 1 for web-applications-00010 and 6 for
web-applications-00030), ensuring the YAML syntax is valid and consistent with
existing test case entries such as using the same keys (testCases, name,
analysisParams, hasIncidents).
- Around line 28-34: Rename the generic test case name "tc-1" under ruleID
spring-framework-5.x-to-6.0-web-applications-00020 to a descriptive identifier
consistent with peers (e.g., "tc-httpclient5-resttemplate" or another name
reflecting the HttpClient 5 / RestTemplate migration intent), updating the
testCases.name value so it matches the naming convention used by other rules
like tc-commons-multipart and tc-tiles.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
SPRING_MIGRATION_UNADDRESSED.mdstable/java/spring-framework/spring-framework-5.x-to-6.0-core-container.yamlstable/java/spring-framework/spring-framework-5.x-to-6.0-data-access.yamlstable/java/spring-framework/spring-framework-5.x-to-6.0-httpclient.yamlstable/java/spring-framework/spring-framework-5.x-to-6.0-removed-apis.yamlstable/java/spring-framework/spring-framework-5.x-to-6.0-web-applications.yamlstable/java/spring-framework/tests/data/core-container/src/main/java/org/konveyor/ParameterNameDiscovererExample.javastable/java/spring-framework/tests/data/core-container/src/main/java/org/konveyor/inject/JavaxInjectService.javastable/java/spring-framework/tests/data/data-access/src/main/java/com/example/DeprecatedExceptionHandler.javastable/java/spring-framework/tests/data/data-access/src/main/java/com/example/HibernateValidatorExample.javastable/java/spring-framework/tests/data/data-access/src/main/java/com/example/JdbcExceptionTranslatorConfig.javastable/java/spring-framework/tests/data/httpclient/src/main/java/org/konveyor/HttpClient4Example.javastable/java/spring-framework/tests/data/removed-apis/pom.xmlstable/java/spring-framework/tests/data/removed-apis/src/main/java/org/konveyor/EhCache2Application.javastable/java/spring-framework/tests/data/removed-apis/src/main/java/org/konveyor/EjbAccessConfig.javastable/java/spring-framework/tests/data/removed-apis/src/main/java/org/konveyor/RpcRemotingConfig.javastable/java/spring-framework/tests/data/web-applications/pom.xmlstable/java/spring-framework/tests/data/web-applications/src/main/java/org/konveyor/Main.javastable/java/spring-framework/tests/data/web-applications/src/main/java/org/konveyor/multipart/MultipartConfig.javastable/java/spring-framework/tests/data/web-applications/src/main/java/org/konveyor/test/ServletMockTest.javastable/java/spring-framework/tests/data/web-applications/src/main/java/org/konveyor/view/TilesConfig.javastable/java/spring-framework/tests/spring-framework-5.x-to-6.0-core-container.test.yamlstable/java/spring-framework/tests/spring-framework-5.x-to-6.0-data-access.test.yamlstable/java/spring-framework/tests/spring-framework-5.x-to-6.0-httpclient.test.yamlstable/java/spring-framework/tests/spring-framework-5.x-to-6.0-removed-apis.test.yamlstable/java/spring-framework/tests/spring-framework-5.x-to-6.0-web-applications.test.yaml
| | Issue | Title | Assignee | Notes | | ||
| |-------|-------|----------|-------| | ||
| | #145 | Create rules for EclipseLink | billwheatley | In progress by assignee | | ||
| | #152 | Create rules for controller detection | jmle | In progress by assignee | |
Contributor
There was a problem hiding this comment.
Self-referential assignee in "Other Contributors" table
Issue #152 lists jmle as the assignee under the "Issues Assigned to Other Contributors" section, but jmle is the author of this very PR. Either this issue is genuinely being deferred (in which case the note should explain why it was skipped despite being self-assigned), or it should be moved to a different category.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SPRING_MIGRATION_UNADDRESSED.md` at line 12, The table row for issue `#152` in
SPRING_MIGRATION_UNADDRESSED.md incorrectly lists the PR author "jmle" under the
"Issues Assigned to Other Contributors" section; either move the row to the
appropriate "Assigned to PR Authors" category or change the assignee and add a
brief explanatory note — update the table entry for issue "#152" (the row with
"Create rules for controller detection") to reflect the correct assignee or add
a parenthetical note like "(self-assigned; deferred — see reason)" so the
listing is not self-referential and the document accurately reflects why it was
skipped or reassigned.
| | #144 | Create rules for Hibernate Validator upgrade | data-access-00060 (new) | | ||
| | #148 | Create rules for backwards compatibility with database-specific error codes | data-access-00030 | | ||
| | #151 | Create rules for the trailing slash matching configuration | web-applications-00001 | | ||
| | #154 | Create rules for RestTemplate Apache HttpClient 5 requirement | Related to #175 | |
Contributor
There was a problem hiding this comment.
"Covered By" entry for #154 is ambiguous
Related to #175`` reads as a cross-reference, not as evidence that #154 is actually covered. Consider replacing this with the specific rule ID that addresses it (e.g., `httpclient-00010` / `web-applications-00020`) so the traceability is unambiguous.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SPRING_MIGRATION_UNADDRESSED.md` at line 26, The "Covered By" entry for item
`#154` is ambiguous because it uses "Related to `#175`" as a cross-reference; update
the entry for "#154" so it lists the exact rule ID(s) that implement or cover it
(e.g., replace "Related to `#175`" with the concrete rule identifier(s) such as
"httpclient-00010" or "web-applications-00020"), ensuring traceability by
referencing the specific rule ID(s) that address `#154` rather than a generic
related-ticket reference.
Comment on lines
+171
to
+174
| when: | ||
| java.referenced: | ||
| pattern: javax.inject.Inject | ||
| location: ANNOTATION |
Contributor
There was a problem hiding this comment.
Detection gap: javax.inject.Named not covered
The when condition only matches javax.inject.Inject at ANNOTATION location, but javax.inject.Named (also moving to jakarta.inject.Named) is called out in the rule message without being detected. Projects that use @Named without @Inject will never be flagged.
Consider extending the when clause to an or block:
🔧 Proposed fix
when:
- java.referenced:
- pattern: javax.inject.Inject
- location: ANNOTATION
+ or:
+ - java.referenced:
+ pattern: javax.inject.Inject
+ location: ANNOTATION
+ - java.referenced:
+ pattern: javax.inject.Named
+ location: ANNOTATION📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| when: | |
| java.referenced: | |
| pattern: javax.inject.Inject | |
| location: ANNOTATION | |
| when: | |
| or: | |
| - java.referenced: | |
| pattern: javax.inject.Inject | |
| location: ANNOTATION | |
| - java.referenced: | |
| pattern: javax.inject.Named | |
| location: ANNOTATION |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-core-container.yaml`
around lines 171 - 174, The rule's when clause only detects java.referenced
pattern "javax.inject.Inject" at ANNOTATION so usages of "javax.inject.Named"
are missed; update the when block for the rule that checks java.referenced to
include an OR combining patterns for "javax.inject.Inject" and
"javax.inject.Named" (both at location ANNOTATION) so that either annotation
triggers the rule; ensure the unique symbol names "javax.inject.Inject" and
"javax.inject.Named" are added as separate alternatives under the
java.referenced condition.
Comment on lines
+307
to
+323
| - ruleID: spring-framework-5.x-to-6.0-data-access-00060 | ||
| category: mandatory | ||
| effort: 3 | ||
| labels: | ||
| - konveyor.io/source=spring5 | ||
| - konveyor.io/target=spring6+ | ||
| - konveyor.io/source=spring-boot2 | ||
| - konveyor.io/target=spring-boot3+ | ||
| when: | ||
| or: | ||
| - java.dependency: | ||
| name: org.hibernate.validator.hibernate-validator | ||
| upperbound: 6.99.99 | ||
| - java.referenced: | ||
| pattern: javax.validation* | ||
| location: IMPORT | ||
| description: Hibernate Validator must be upgraded to 7.0+ for Spring 6 |
Contributor
There was a problem hiding this comment.
Avoid duplicate Hibernate Validator upgrade incidents.
Rule 00060 overlaps existing 00010 on dependency-based detection, which can emit duplicate findings and conflicting guidance paths for the same project.
🔧 Proposed narrowing for 00060
- when:
- or:
- - java.dependency:
- name: org.hibernate.validator.hibernate-validator
- upperbound: 6.99.99
- - java.referenced:
- pattern: javax.validation*
- location: IMPORT
+ when:
+ java.referenced:
+ pattern: javax.validation.*
+ location: IMPORT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - ruleID: spring-framework-5.x-to-6.0-data-access-00060 | |
| category: mandatory | |
| effort: 3 | |
| labels: | |
| - konveyor.io/source=spring5 | |
| - konveyor.io/target=spring6+ | |
| - konveyor.io/source=spring-boot2 | |
| - konveyor.io/target=spring-boot3+ | |
| when: | |
| or: | |
| - java.dependency: | |
| name: org.hibernate.validator.hibernate-validator | |
| upperbound: 6.99.99 | |
| - java.referenced: | |
| pattern: javax.validation* | |
| location: IMPORT | |
| description: Hibernate Validator must be upgraded to 7.0+ for Spring 6 | |
| - ruleID: spring-framework-5.x-to-6.0-data-access-00060 | |
| category: mandatory | |
| effort: 3 | |
| labels: | |
| - konveyor.io/source=spring5 | |
| - konveyor.io/target=spring6+ | |
| - konveyor.io/source=spring-boot2 | |
| - konveyor.io/target=spring-boot3+ | |
| when: | |
| java.referenced: | |
| pattern: javax.validation.* | |
| location: IMPORT | |
| description: Hibernate Validator must be upgraded to 7.0+ for Spring 6 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-data-access.yaml`
around lines 307 - 323, The rule ruleID
spring-framework-5.x-to-6.0-data-access-00060 currently duplicates rule 00010 by
checking the same dependency; update rule 00060's when clause to avoid overlap
by removing or narrowing the dependency-based trigger (the java.dependency block
for org.hibernate.validator.hibernate-validator) and keep only the import-based
detection (the java.referenced pattern javax.validation* with location IMPORT),
or alternatively add a negation so 00060 only fires when java.dependency does
NOT match the vulnerable range; reference ruleID
spring-framework-5.x-to-6.0-data-access-00060 and the java.dependency /
java.referenced conditions when making the change.
Comment on lines
+118
to
+124
| // After (REST Client) | ||
| @Bean | ||
| public UserService userService(RestClient.Builder builder) { | ||
| RestClient client = builder.baseUrl("http://server").build(); | ||
| return HttpServiceProxyFactory.builderFor(RestClientAdapter.create(client)) | ||
| .build().createClient(UserService.class); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Spring Framework RestClient introduced which version 6.0 6.1
💡 Result:
RestClient was introduced in Spring Framework 6.1 (not 6.0). It’s marked “Since: 6.1” in the official Javadoc, and Spring’s “New in Spring 6.1” announcement also introduces it as a 6.1 feature. [1], [2]
Sources:
[1] Spring Framework Javadoc (RestClient, “Since: 6.1”) (docs.spring.io)
[2] Spring blog: “New in Spring 6.1: RestClient” (spring.io)
Citations:
- 1: https://docs.spring.io/spring-framework/docs/6.1.x/javadoc-api/org/springframework/web/client/RestClient.html?utm_source=openai
- 2: https://spring.io/blog/2023/07/13/new-in-spring-6-1-restclient/?utm_source=openai
🏁 Script executed:
# Let's examine the actual file to understand the context
find . -name "spring-framework-5.x-to-6.0-removed-apis.yaml" -type fRepository: konveyor/rulesets
Length of output: 137
🏁 Script executed:
cat -n ./stable/java/spring-framework/spring-framework-5.x-to-6.0-removed-apis.yaml | sed -n '110,135p'Repository: konveyor/rulesets
Length of output: 1200
🏁 Script executed:
cat -n ./stable/java/spring-framework/spring-framework-5.x-to-6.0-removed-apis.yaml | sed -n '90,130p'Repository: konveyor/rulesets
Length of output: 1980
Replace the code example with a Spring 6.0-compatible migration approach.
This migration ruleset targets Spring 5.x → 6.0, but the example code (lines 118–124) uses RestClient and RestClientAdapter, which were introduced in Spring 6.1. Developers following this guidance for Spring 6.0 will encounter compilation errors.
Suggested fix: Replace the example with one using Spring 6.0-available alternatives, such as:
RestTemplate(available in 6.0)@RestControllerwith HTTP client libraries- gRPC or other modern alternatives listed in the migration guidance
References:
- Spring Framework Javadoc: RestClient marked "Since: 6.1"
- Spring blog: "New in Spring 6.1: RestClient"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-removed-apis.yaml`
around lines 118 - 124, The example uses
RestClient/RestClientAdapter/HttpServiceProxyFactory.builderFor which are only
available in Spring 6.1+, so replace the bean definition for userService and the
usage of RestClient/RestClientAdapter with a Spring 6.0-compatible approach (for
example create a RestTemplate or WebClient bean and use it inside a UserService
implementation or expose a UserService client that delegates to
RestTemplate/WebClient); update references to the factory method names (remove
HttpServiceProxyFactory.builderFor and RestClient.Builder usage) and provide a
clear example that constructs a RestTemplate or WebClient and injects it into
UserService (or a UserServiceClient) so the snippet compiles under Spring 6.0.
Comment on lines
+139
to
+144
| or: | ||
| - java.referenced: | ||
| pattern: org.springframework.ejb.access.* | ||
| - java.referenced: | ||
| pattern: org.springframework.ejb.config.* | ||
| description: Dedicated EJB access has been removed in Spring 6 |
Contributor
There was a problem hiding this comment.
Add XML namespace detection for removed EJB config usage.
Current matching only checks Java references. It misses XML-based usage (<jee:local-slsb>, <jee:remote-slsb>), which is a common path for the removed EJB access support.
🧩 Proposed rule extension
when:
or:
- java.referenced:
pattern: org.springframework.ejb.access.*
- java.referenced:
pattern: org.springframework.ejb.config.*
+ - builtin.xml:
+ namespaces:
+ jee: http://www.springframework.org/schema/jee
+ xpath: "//*[local-name()='local-slsb' or local-name()='remote-slsb']"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stable/java/spring-framework/spring-framework-5.x-to-6.0-removed-apis.yaml`
around lines 139 - 144, The rule currently matches only Java references
(patterns org.springframework.ejb.access.* and org.springframework.ejb.config.*)
so it misses XML-based EJB config usage; extend the rule to also detect Spring
JEe XML namespace usages by adding checks for the jee namespace (e.g., namespace
URI "http://www.springframework.org/schema/jee" or prefix "jee") and element
names like jee:local-slsb and jee:remote-slsb so XML configs are flagged
alongside the existing Java patterns.
Comment on lines
+382
to
+383
| **Note:** If using `MockHttpServletRequestBuilder.setRemoteAddress()`, this is a new method | ||
| available in Spring 6.0. |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Spring Framework 6.0.24 MockHttpServletRequestBuilder remoteAddress method javadoc
💡 Result:
In Spring Framework 6.0.24, MockHttpServletRequestBuilder#remoteAddress is documented as:
- Signature:
public MockHttpServletRequestBuilder remoteAddress(String remoteAddress) - Javadoc: “Set the remote address of the request.”
- Parameter:
remoteAddress– “the remote address (IP)” - Since:
6.0.10[1]
Source: Spring Framework 6.0.24 API Javadoc for MockHttpServletRequestBuilder → method remoteAddress. [1]
Fix the API method name in the Servlet mocks note.
The note incorrectly references MockHttpServletRequestBuilder.setRemoteAddress(), which does not exist. The correct method is remoteAddress(String), available since Spring Framework 6.0.10.
✏️ Proposed text fix
- **Note:** If using `MockHttpServletRequestBuilder.setRemoteAddress()`, this is a new method
- available in Spring 6.0.
+ **Note:** `MockHttpServletRequestBuilder.remoteAddress(String)` is available in Spring Framework 6.0.x.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stable/java/spring-framework/spring-framework-5.x-to-6.0-web-applications.yaml`
around lines 382 - 383, Update the Servlet mocks note to reference the correct
API: replace the non-existent MockHttpServletRequestBuilder.setRemoteAddress()
with the actual method MockHttpServletRequestBuilder.remoteAddress(String) and
mention it is available since Spring Framework 6.0.10; locate the note text
mentioning MockHttpServletRequestBuilder and change the method name and
signature accordingly.
Comment on lines
+13
to
+19
| @Inject | ||
| private JavaxInjectRepository repository; | ||
|
|
||
| @Inject | ||
| public JavaxInjectService(JavaxInjectRepository repository) { | ||
| this.repository = repository; | ||
| } |
Contributor
There was a problem hiding this comment.
Mixed field and constructor injection on the same field
repository is annotated with @Inject on the field (Line 13) and the constructor also declares @Inject (Line 16) and assigns the same field. In any DI container that processes both injection points, the field would be injected twice. While this class is never executed as production code, migration-rule test data is also read by humans, so having an outright injection anti-pattern may mislead developers.
If the intent is to demonstrate both field and constructor injection sites for rule-detection breadth, split them into separate example classes, or remove the field-level @Inject and rely solely on the constructor:
🔧 Minimal fix (constructor injection only)
- `@Inject`
- private JavaxInjectRepository repository;
-
- `@Inject`
public JavaxInjectService(JavaxInjectRepository repository) {
this.repository = repository;
}
+
+ private final JavaxInjectRepository repository;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Inject | |
| private JavaxInjectRepository repository; | |
| @Inject | |
| public JavaxInjectService(JavaxInjectRepository repository) { | |
| this.repository = repository; | |
| } | |
| private final JavaxInjectRepository repository; | |
| public JavaxInjectService(JavaxInjectRepository repository) { | |
| this.repository = repository; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stable/java/spring-framework/tests/data/core-container/src/main/java/org/konveyor/inject/JavaxInjectService.java`
around lines 13 - 19, The class JavaxInjectService currently has mixed injection
on the same field: the private field "repository" is annotated with `@Inject` and
the constructor JavaxInjectService(JavaxInjectRepository repository) is also
annotated with `@Inject` and assigns the same field, causing duplicate injection;
fix this by removing the `@Inject` annotation from the field declaration (leave
the private JavaxInjectRepository repository; field unannotated) and keep the
constructor-level `@Inject` and its assignment, so only constructor injection is
used for JavaxInjectService.
jmle
added
cherry-pick/release-0.9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-creating @dymurray 's PR with all fixes
Issues Addressed (11)
Fixes #134 , #135, #137, #139, #146, #147, #149, #150, #155, #167, #175
Summary by CodeRabbit
Release Notes
New Features
Documentation