Skip to content

S6856: Support Spring 7 path-segment API versioning#5755

Open
patpatpat123 wants to merge 1 commit into
SonarSource:masterfrom
patpatpat123:fix/s6856-spring-7-api-versioning
Open

S6856: Support Spring 7 path-segment API versioning#5755
patpatpat123 wants to merge 1 commit into
SonarSource:masterfrom
patpatpat123:fix/s6856-spring-7-api-versioning

Conversation

@patpatpat123

Copy link
Copy Markdown

Problem

Spring Framework 7 supports resolving an API version from a request path segment:

@Override
public void configureApiVersioning(ApiVersionConfigurer configurer) {
  Predicate<RequestPath> versionedPaths = this::isVersionedPath;
  configurer.usePathSegment(1, versionedPaths);
}

@GetMapping(value = "/api/v{apiVersion}/resources/{id}", version = "2")
Resource getResource(@PathVariable UUID id) {
  // ...
}

PathApiVersionResolver reads the raw path segment (v2 in this example) independently of controller argument binding. The {apiVersion} placeholder is structural: it makes the route match the version
segment, but it does not need to be exposed as a controller method parameter.

S6856 currently reports {apiVersion} as unbound. Adding an unused @PathVariable merely to satisfy the rule would make the controller signature misleading.

## Proposed solution

Teach S6856 to recognize source-visible Spring MVC path-version configuration and defer its final decision until all source files in the module have been inspected.

The exception is deliberately conservative. It applies only when:

- Spring Web 7 or later is present;
- usePathSegment(...) is called directly from an actual configureApiVersioning callback;
- the segment index is a non-negative compile-time constant;
- only one distinct path-version index is configured;
- the controller mapping declares a nonblank version;
- no fallback resolver, default version, optional-version configuration, dynamic index, or escaped configurer makes the configuration ambiguous; and
- the placeholder occurs only in the configured segment and is not a catch-all variable.

Other missing variables are still reported. For example, /api/v{apiVersion}/resources/{id} continues to report an unbound {id}.

## Scope

This proposal handles direct, source-visible Spring MVC Java configuration. It intentionally does not infer WebFlux configuration, configuration supplied by dependencies or properties, delegated
configurer mutations, or programmatic path prefixes.

## Tests

This change adds a Spring Web 7.0.8 fixture module and regression coverage for:

- both usePathSegment overloads;
- method-reference predicates;
- configuration declared after controller sources;
- class-level paths;
- ordinary missing variables;
- versionless mappings and catch-all variables;
- fallback resolvers;
- inactive configuration methods;
- multiple and dynamic indices; and
- escaped configurers.

Verified locally:

- Spring Web 7 fixture module: successful
- S6856 focused suite: 29 tests, 0 failures

Spring API-versioning documentation:
https://docs.spring.io/spring-framework/reference/web/webmvc/mvc-config/api-version.html

Comment on lines +336 to +350
private void reportOrDeferMissingTemplateVariables(UriInfo uri, MappingInfo classMapping, Set<String> unboundVariables) {
if (springWebVersion != SpringWebVersion.START_FROM_7_0) {
addPendingIssue(uri.request(), missingTemplateVariablesMessage(unboundVariables));
return;
}

pendingMappingIssues.add(new PendingMappingIssue(
context.getInputFile(),
AnalyzerMessage.textSpanFor(uri.request()),
Set.copyOf(unboundVariables),
combinePaths(classMapping.paths(), uri.mapping().paths()),
classMapping.versioned() || uri.mapping().versioned()));
}

private void addPendingIssue(Tree tree, String message) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Quality: All issues buffered to endOfAnalysis even without Spring 7

The refactor now routes every S6856 issue (including the ordinary "Bind method parameter" and template-variable issues) through pendingIssues/pendingMappingIssues and flushes them only in endOfAnalysis, for all Spring versions. Cross-file deferral is only needed for START_FROM_7_0; for the far more common non-Spring-7 case this unnecessarily holds every module's S6856 issues in memory until the end of analysis and reports them via the module context instead of inline.

This is functionally correct (verified endOfAnalysis runs in batch and file-by-file/SonarLint modes), but for the non-Spring-7 path you could report inline to avoid buffering. For example, in reportOrDeferMissingTemplateVariables you already branch on springWebVersion != START_FROM_7_0 — the addPendingIssue calls in that branch and in checkParametersAndPathTemplate could instead call reportIssue(...) directly so no state needs to be retained. Minor; consider only if memory of large modules is a concern.

Only buffer when deferral is actually required (Spring 7); otherwise report inline as before.:

// For non-Spring-7, report inline instead of buffering:
private void reportMissing(Tree tree, String message) {
  if (springWebVersion == SpringWebVersion.START_FROM_7_0) {
    pendingIssues.add(new AnalyzerMessage(this, context.getInputFile(),
      AnalyzerMessage.textSpanFor(tree), message, 0));
  } else {
    reportIssue(tree, message);
  }
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Implements Spring 7 path-segment API versioning support for S6856 to prevent false positives on unbound placeholders. Note that all analysis results are now buffered until the end of the analysis, which may impact performance even for non-Spring 7 projects.

💡 Quality: All issues buffered to endOfAnalysis even without Spring 7

📄 java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java:336-350 📄 java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java:383-397

The refactor now routes every S6856 issue (including the ordinary "Bind method parameter" and template-variable issues) through pendingIssues/pendingMappingIssues and flushes them only in endOfAnalysis, for all Spring versions. Cross-file deferral is only needed for START_FROM_7_0; for the far more common non-Spring-7 case this unnecessarily holds every module's S6856 issues in memory until the end of analysis and reports them via the module context instead of inline.

This is functionally correct (verified endOfAnalysis runs in batch and file-by-file/SonarLint modes), but for the non-Spring-7 path you could report inline to avoid buffering. For example, in reportOrDeferMissingTemplateVariables you already branch on springWebVersion != START_FROM_7_0 — the addPendingIssue calls in that branch and in checkParametersAndPathTemplate could instead call reportIssue(...) directly so no state needs to be retained. Minor; consider only if memory of large modules is a concern.

Only buffer when deferral is actually required (Spring 7); otherwise report inline as before.
// For non-Spring-7, report inline instead of buffering:
private void reportMissing(Tree tree, String message) {
  if (springWebVersion == SpringWebVersion.START_FROM_7_0) {
    pendingIssues.add(new AnalyzerMessage(this, context.getInputFile(),
      AnalyzerMessage.textSpanFor(tree), message, 0));
  } else {
    reportIssue(tree, message);
  }
}
🤖 Prompt for agents
Code Review: Implements Spring 7 path-segment API versioning support for S6856 to prevent false positives on unbound placeholders. Note that all analysis results are now buffered until the end of the analysis, which may impact performance even for non-Spring 7 projects.

1. 💡 Quality: All issues buffered to endOfAnalysis even without Spring 7
   Files: java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java:336-350, java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java:383-397

   The refactor now routes every S6856 issue (including the ordinary "Bind method parameter" and template-variable issues) through `pendingIssues`/`pendingMappingIssues` and flushes them only in `endOfAnalysis`, for all Spring versions. Cross-file deferral is only needed for `START_FROM_7_0`; for the far more common non-Spring-7 case this unnecessarily holds every module's S6856 issues in memory until the end of analysis and reports them via the module context instead of inline.
   
   This is functionally correct (verified `endOfAnalysis` runs in batch and file-by-file/SonarLint modes), but for the non-Spring-7 path you could report inline to avoid buffering. For example, in `reportOrDeferMissingTemplateVariables` you already branch on `springWebVersion != START_FROM_7_0` — the `addPendingIssue` calls in that branch and in `checkParametersAndPathTemplate` could instead call `reportIssue(...)` directly so no state needs to be retained. Minor; consider only if memory of large modules is a concern.

   Fix (Only buffer when deferral is actually required (Spring 7); otherwise report inline as before.):
   // For non-Spring-7, report inline instead of buffering:
   private void reportMissing(Tree tree, String message) {
     if (springWebVersion == SpringWebVersion.START_FROM_7_0) {
       pendingIssues.add(new AnalyzerMessage(this, context.getInputFile(),
         AnalyzerMessage.textSpanFor(tree), message, 0));
     } else {
       reportIssue(tree, message);
     }
   }

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@patpatpat123 patpatpat123 marked this pull request as ready for review July 7, 2026 10:04
@patpatpat123

Copy link
Copy Markdown
Author

Hello team, may i get some feedback on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant