Skip to content

Commit f29d7f1

Browse files
Seppli11sonartech
authored andcommitted
SONARPY-3693 Create rule S8413: Router prefixes should be defined during APIRouter initialization (#818)
GitOrigin-RevId: 07c251b366702d7644da925adba3b0783a7d904a
1 parent 8e230a6 commit f29d7f1

9 files changed

Lines changed: 352 additions & 1 deletion

File tree

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ public Stream<Class<?>> getChecks() {
374374
ReturnAndYieldInOneFunctionCheck.class,
375375
ReturnYieldOutsideFunctionCheck.class,
376376
RobustCipherAlgorithmCheck.class,
377+
RouterPrefixInIncludeRouterCheck.class,
377378
S3BucketBlockPublicAccessCheck.class,
378379
S3BucketGrantedAccessCheck.class,
379380
S3BucketHTTPCommunicationCheck.class,
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import java.util.Optional;
20+
import java.util.stream.Stream;
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
23+
import org.sonar.plugins.python.api.SubscriptionContext;
24+
import org.sonar.plugins.python.api.symbols.v2.SymbolV2;
25+
import org.sonar.plugins.python.api.symbols.v2.UsageV2;
26+
import org.sonar.plugins.python.api.tree.CallExpression;
27+
import org.sonar.plugins.python.api.tree.Expression;
28+
import org.sonar.plugins.python.api.tree.Name;
29+
import org.sonar.plugins.python.api.tree.RegularArgument;
30+
import org.sonar.plugins.python.api.tree.Tree;
31+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
32+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
33+
import org.sonar.python.checks.utils.Expressions;
34+
import org.sonar.python.tree.TreeUtils;
35+
36+
@Rule(key = "S8413")
37+
public class RouterPrefixInIncludeRouterCheck extends PythonSubscriptionCheck {
38+
39+
private static final String MESSAGE =
40+
"Define the prefix in the \"APIRouter\" constructor instead of in \"include_router()\".";
41+
42+
private static final TypeMatcher INCLUDE_ROUTER_MATCHER = TypeMatchers.any(
43+
TypeMatchers.isType("fastapi.applications.FastAPI.include_router"),
44+
TypeMatchers.isType("fastapi.routing.APIRouter.include_router")
45+
);
46+
47+
private static final TypeMatcher API_ROUTER_MATCHER = TypeMatchers.isType("fastapi.routing.APIRouter");
48+
49+
@Override
50+
public void initialize(Context context) {
51+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, RouterPrefixInIncludeRouterCheck::checkIncludeRouterCall);
52+
}
53+
54+
private static void checkIncludeRouterCall(SubscriptionContext ctx) {
55+
CallExpression callExpr = (CallExpression) ctx.syntaxNode();
56+
57+
if (!INCLUDE_ROUTER_MATCHER.isTrueFor(callExpr.callee(), ctx)) {
58+
return;
59+
}
60+
61+
RegularArgument prefixArg = TreeUtils.argumentByKeyword("prefix", callExpr.arguments());
62+
if (prefixArg == null) {
63+
return;
64+
}
65+
66+
Optional<RegularArgument> routerArg = TreeUtils.nthArgumentOrKeywordOptional(0, "router", callExpr.arguments());
67+
if (routerArg.isEmpty()) {
68+
return;
69+
}
70+
71+
Expression routerExpr = routerArg.get().expression();
72+
if (!(routerExpr instanceof Name routerName)) {
73+
return;
74+
}
75+
76+
SymbolV2 routerSymbol = routerName.symbolV2();
77+
if (routerSymbol == null) {
78+
return;
79+
}
80+
81+
if (isRouterWithoutPrefixInConstructor(routerSymbol, ctx)) {
82+
var keyword = prefixArg.keywordArgument();
83+
if (keyword != null) {
84+
ctx.addIssue(keyword, MESSAGE);
85+
}
86+
}
87+
}
88+
89+
private static boolean isRouterWithoutPrefixInConstructor(SymbolV2 routerSymbol, SubscriptionContext ctx) {
90+
return routerSymbol.usages().stream()
91+
.filter(UsageV2::isBindingUsage)
92+
.map(UsageV2::tree)
93+
.flatMap(RouterPrefixInIncludeRouterCheck::getBindingCallExpression)
94+
.filter(call -> API_ROUTER_MATCHER.isTrueFor(call.callee(), ctx))
95+
.anyMatch(RouterPrefixInIncludeRouterCheck::callHasNoPrefixArgument);
96+
}
97+
98+
private static Stream<CallExpression> getBindingCallExpression(Tree bindingTree) {
99+
if (!(bindingTree instanceof Name name)) {
100+
return Stream.empty();
101+
}
102+
return Expressions.singleAssignedNonNameValue(name).stream()
103+
.flatMap(TreeUtils.toStreamInstanceOfMapper(CallExpression.class));
104+
}
105+
106+
private static boolean callHasNoPrefixArgument(CallExpression call) {
107+
return TreeUtils.argumentByKeyword("prefix", call.arguments()) == null;
108+
}
109+
}

python-checks/src/main/java/org/sonar/python/checks/utils/Expressions.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import javax.annotation.Nullable;
3030
import org.sonar.plugins.python.api.symbols.Symbol;
3131
import org.sonar.plugins.python.api.symbols.Usage;
32+
import org.sonar.plugins.python.api.tree.AnnotatedAssignment;
3233
import org.sonar.plugins.python.api.tree.Argument;
3334
import org.sonar.plugins.python.api.tree.AssignmentStatement;
3435
import org.sonar.plugins.python.api.tree.CallExpression;
@@ -152,6 +153,8 @@ public static Expression singleAssignedValue(Name name, Set<Name> visited) {
152153
parent.parent().is(Kind.ASSIGNMENT_STMT)) {
153154

154155
result = ((AssignmentStatement) parent.parent()).assignedValue();
156+
} else if (parent instanceof AnnotatedAssignment annotatedAssignment) {
157+
result = annotatedAssignment.assignedValue();
155158
} else {
156159
return null;
157160
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<p>An issue is raised when a router prefix is defined in the <code>include_router()</code> call instead of in the <code>APIRouter()</code>
2+
initialization.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>In FastAPI applications, routers help organize endpoints into logical groups. Each router can have a URL prefix that gets prepended to all its
5+
routes.</p>
6+
<p>When you define the prefix in the <code>include_router()</code> call, the router’s URL structure is separated from its definition. This means
7+
developers need to search through the application setup code to understand where the router’s endpoints will be accessible.</p>
8+
<p>Modern FastAPI versions support defining the prefix directly in the <code>APIRouter()</code> constructor. This approach keeps the router’s
9+
configuration self-contained and makes the code easier to understand at a glance.</p>
10+
<p>By defining the prefix at initialization time, you create a single source of truth for the router’s configuration. Anyone reading the router file
11+
immediately knows its URL structure without having to trace through the application setup.</p>
12+
<h3>What is the potential impact?</h3>
13+
<p>This pattern reduces code maintainability and readability. In larger applications with multiple routers, it becomes increasingly difficult to
14+
understand the application’s URL structure without examining the main application file.</p>
15+
<p>When routers are defined in separate modules (a common practice), developers working on a specific router module cannot see its full URL path
16+
without switching to a different file. This cognitive overhead slows down development and increases the likelihood of mistakes when adding or
17+
modifying routes.</p>
18+
<h2>How to fix it</h2>
19+
<p>Move the <code>prefix</code> parameter from the <code>include_router()</code> call to the <code>APIRouter()</code> initialization. This makes the
20+
router’s URL structure immediately visible in its definition.</p>
21+
<h3>Code examples</h3>
22+
<h4>Noncompliant code example</h4>
23+
<pre data-diff-id="1" data-diff-type="noncompliant">
24+
from fastapi import APIRouter, FastAPI
25+
26+
router = APIRouter() # Noncompliant
27+
28+
@router.get("/users")
29+
def list_users():
30+
return ["user1", "user2"]
31+
32+
app = FastAPI()
33+
app.include_router(router, prefix="/api/v1")
34+
</pre>
35+
<h4>Compliant solution</h4>
36+
<pre data-diff-id="1" data-diff-type="compliant">
37+
from fastapi import APIRouter, FastAPI
38+
39+
router = APIRouter(prefix="/api/v1")
40+
41+
@router.get("/users")
42+
def list_users():
43+
return ["user1", "user2"]
44+
45+
app = FastAPI()
46+
app.include_router(router)
47+
</pre>
48+
<h2>Resources</h2>
49+
<h3>Documentation</h3>
50+
<ul>
51+
<li> FastAPI - APIRouter - <a href="https://fastapi.tiangolo.com/tutorial/bigger-applications/">Official FastAPI documentation on using APIRouter
52+
for larger applications</a> </li>
53+
<li> FastAPI - APIRouter Reference - <a href="https://fastapi.tiangolo.com/reference/apirouter/">API reference for the APIRouter class and its
54+
parameters</a> </li>
55+
</ul>
56+
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"title": "Router prefixes should be defined during \"APIRouter\" initialization",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "2min"
8+
},
9+
"tags": [
10+
"fastapi",
11+
"convention",
12+
"readability"
13+
],
14+
"defaultSeverity": "Blocker",
15+
"ruleSpecification": "RSPEC-8413",
16+
"sqKey": "S8413",
17+
"scope": "Main",
18+
"quickfix": "unknown",
19+
"code": {
20+
"impacts": {
21+
"MAINTAINABILITY": "BLOCKER"
22+
},
23+
"attribute": "CLEAR"
24+
}
25+
}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@
323323
"S8409",
324324
"S8410",
325325
"S8411",
326-
"S8412"
326+
"S8412",
327+
"S8413"
327328
]
328329
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class RouterPrefixInIncludeRouterCheckTest {
23+
@Test
24+
void test() {
25+
PythonCheckVerifier.verify(
26+
"src/test/resources/checks/routerPrefixInIncludeRouter.py",
27+
new RouterPrefixInIncludeRouterCheck());
28+
}
29+
}

python-checks/src/test/java/org/sonar/python/checks/utils/ExpressionsTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ void singleAssignedValue() {
269269
assertThat(lastNameValue("x = 42; import x; x")).isNull();
270270
assertThat(lastNameValue("x = 42; x = 43; x")).isNull();
271271
assertThat(lastNameValue("x = 42; y")).isNull();
272+
assertThat(lastNameValue("x: int = 42; x").getKind()).isEqualTo(Kind.NUMERIC_LITERAL);
272273
}
273274

274275
private Expression lastNameValue(String code) {
@@ -290,6 +291,7 @@ void singleAssignedNonNameValue() {
290291
assertThat(lastNameNonNameValue("x = 42; import x; x")).isNull();
291292
assertThat(lastNameNonNameValue("x = 42; x = 43; x")).isNull();
292293
assertThat(lastNameNonNameValue("x = 42; y")).isNull();
294+
assertThat(lastNameNonNameValue("x: int = 42; x").getKind()).isEqualTo(Kind.NUMERIC_LITERAL);
293295
}
294296

295297
@Test
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
from fastapi import APIRouter, FastAPI
2+
3+
# --- Noncompliant Cases ---
4+
5+
def noncompliant_basic():
6+
"""Basic case: prefix in include_router instead of APIRouter constructor"""
7+
router = APIRouter()
8+
app = FastAPI()
9+
app.include_router(router, prefix="/api/v1") # Noncompliant {{Define the prefix in the "APIRouter" constructor instead of in "include_router()".}}
10+
# ^^^^^^
11+
12+
def noncompliant_router_to_router():
13+
"""Prefix when adding router to another router"""
14+
parent = APIRouter()
15+
child = APIRouter()
16+
parent.include_router(child, prefix="/users") # Noncompliant
17+
# ^^^^^^
18+
19+
def noncompliant_keyword_argument():
20+
"""Using keyword argument for router parameter"""
21+
router = APIRouter()
22+
app = FastAPI()
23+
app.include_router(router=router, prefix="/api") # Noncompliant
24+
# ^^^^^^
25+
26+
def noncompliant_multiple_routers():
27+
"""Multiple routers with prefix in include_router"""
28+
router1 = APIRouter()
29+
router2 = APIRouter()
30+
app = FastAPI()
31+
app.include_router(router1, prefix="/v1") # Noncompliant
32+
# ^^^^^^
33+
app.include_router(router2, prefix="/v2") # Noncompliant
34+
# ^^^^^^
35+
36+
def noncompliant_prefix_with_tags():
37+
"""Prefix combined with other arguments"""
38+
router = APIRouter()
39+
app = FastAPI()
40+
app.include_router(router, prefix="/api", tags=["api"]) # Noncompliant
41+
# ^^^^^^
42+
43+
# --- Compliant Cases ---
44+
45+
def compliant_prefix_in_constructor():
46+
router = APIRouter(prefix="/api/v1")
47+
app = FastAPI()
48+
app.include_router(router) # Compliant
49+
50+
def compliant_no_prefix():
51+
router = APIRouter()
52+
app = FastAPI()
53+
app.include_router(router) # Compliant
54+
55+
def compliant_both_prefixes():
56+
"""Prefix in both places - still compliant (user's choice to combine)"""
57+
router = APIRouter(prefix="/api")
58+
app = FastAPI()
59+
app.include_router(router, prefix="/v1") # Compliant
60+
61+
def compliant_prefix_in_constructor_with_tags():
62+
"""Prefix in constructor, other args in include_router"""
63+
router = APIRouter(prefix="/api")
64+
app = FastAPI()
65+
app.include_router(router, tags=["api"]) # Compliant
66+
67+
def compliant_only_tags():
68+
"""Only tags, no prefix"""
69+
router = APIRouter()
70+
app = FastAPI()
71+
app.include_router(router, tags=["api"]) # Compliant
72+
73+
def fn_router_without_symbol():
74+
"""Router created inline - edge case"""
75+
app = FastAPI()
76+
app.include_router(APIRouter(), prefix="/api") # FN
77+
78+
def fn_unknown_router_origin():
79+
"""Router comes from function call"""
80+
app = FastAPI()
81+
app.include_router(get_router(), prefix="/api") # FN
82+
83+
def fn_router_from_import():
84+
"""Router imported from another module"""
85+
from other_module import router
86+
app = FastAPI()
87+
app.include_router(router, prefix="/api") # FN
88+
89+
# --- Edge Cases ---
90+
91+
def edge_case_reassigned_router():
92+
"""Router reassigned to another variable"""
93+
router = APIRouter()
94+
r = router
95+
app = FastAPI()
96+
app.include_router(r, prefix="/api") # Noncompliant
97+
98+
def edge_case_conditional():
99+
router = APIRouter()
100+
app = FastAPI()
101+
if True:
102+
app.include_router(router, prefix="/api") # Noncompliant
103+
# ^^^^^^
104+
105+
def edge_case_annotated_assignment():
106+
router: APIRouter = APIRouter()
107+
app = FastAPI()
108+
app.include_router(router, prefix="/api") # Noncompliant
109+
# ^^^^^^
110+
111+
def edge_case_router_as_function_parameter():
112+
def setup_routes(router: APIRouter):
113+
app = FastAPI()
114+
app.include_router(router, prefix="/api") # potential FN
115+
116+
def edge_case_multiple_assignments():
117+
router = APIRouter()
118+
router = APIRouter(prefix="/api")
119+
app = FastAPI()
120+
app.include_router(router, prefix="/v1") # FN
121+
122+
def edge_case_no_router():
123+
router = APIRouter()
124+
app = FastAPI()
125+
app.include_router(prefix="/api", router)

0 commit comments

Comments
 (0)