Skip to content

Commit c5ff601

Browse files
Seppli11sonartech
Seppli11
authored and
sonartech
committed
SONARPY-3707 S8412: Add quick fix (#821)
GitOrigin-RevId: 4e8b2b2e3b117ffb93105f051a384bf93e941572
1 parent 7270c92 commit c5ff601

2 files changed

Lines changed: 206 additions & 12 deletions

File tree

‎python-checks/src/main/java/org/sonar/python/checks/FastAPIGenericRouteDecoratorCheck.java‎

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,39 +17,45 @@
1717
package org.sonar.python.checks;
1818

1919
import java.util.List;
20+
import java.util.Locale;
21+
import java.util.Optional;
2022
import java.util.Set;
2123
import javax.annotation.CheckForNull;
2224
import org.sonar.check.Rule;
2325
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2426
import org.sonar.plugins.python.api.SubscriptionContext;
27+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
28+
import org.sonar.plugins.python.api.tree.Argument;
2529
import org.sonar.plugins.python.api.tree.CallExpression;
2630
import org.sonar.plugins.python.api.tree.Decorator;
2731
import org.sonar.plugins.python.api.tree.Expression;
2832
import org.sonar.plugins.python.api.tree.FunctionDef;
2933
import org.sonar.plugins.python.api.tree.ListLiteral;
34+
import org.sonar.plugins.python.api.tree.Name;
35+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
3036
import org.sonar.plugins.python.api.tree.RegularArgument;
3137
import org.sonar.plugins.python.api.tree.StringLiteral;
3238
import org.sonar.plugins.python.api.tree.Tree;
3339
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
3440
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
3541
import org.sonar.python.checks.utils.Expressions;
42+
import org.sonar.python.quickfix.TextEditUtils;
3643
import org.sonar.python.tree.TreeUtils;
3744

3845
@Rule(key = "S8412")
3946
public class FastAPIGenericRouteDecoratorCheck extends PythonSubscriptionCheck {
4047

41-
private static final String MESSAGE =
42-
"Replace this generic \"route()\" decorator with a specific HTTP method decorator.";
48+
private static final String MESSAGE = "Replace this generic \"route()\" decorator with a specific HTTP method decorator.";
49+
50+
private static final String QUICK_FIX_MESSAGE = "Replace with \"%s\"";
4351

4452
private static final TypeMatcher ROUTE_DECORATOR_MATCHER = TypeMatchers.any(
4553
TypeMatchers.isType("fastapi.applications.FastAPI.route"),
46-
TypeMatchers.isType("fastapi.routing.APIRouter.route")
47-
);
54+
TypeMatchers.isType("fastapi.routing.APIRouter.route"));
4855

4956
private static final Set<String> SINGLE_HTTP_METHODS = Set.of(
5057
"GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "HEAD", "TRACE",
51-
"get", "post", "put", "delete", "patch", "options", "head", "trace"
52-
);
58+
"get", "post", "put", "delete", "patch", "options", "head", "trace");
5359

5460
@Override
5561
public void initialize(Context context) {
@@ -81,31 +87,77 @@ private static void checkDecorator(SubscriptionContext ctx, Decorator decorator)
8187
}
8288

8389
Expression methodsExpr = methodsArg.expression();
84-
if (methodsExpr instanceof ListLiteral listLiteral && isSingleHttpMethod(listLiteral)) {
85-
ctx.addIssue(callExpr.callee(), MESSAGE);
90+
if (!(methodsExpr instanceof ListLiteral listLiteral)) {
91+
return;
92+
}
93+
94+
Optional<String> httpMethod = getSingleHttpMethod(listLiteral);
95+
if (httpMethod.isPresent()) {
96+
PreciseIssue issue = ctx.addIssue(callExpr.callee(), MESSAGE);
97+
addQuickFix(issue, callExpr, methodsArg, httpMethod.get());
8698
}
8799
}
88100

89-
private static boolean isSingleHttpMethod(ListLiteral listLiteral) {
101+
private static Optional<String> getSingleHttpMethod(ListLiteral listLiteral) {
90102
List<Expression> elements = listLiteral.elements().expressions();
91103

92104
if (elements.size() != 1) {
93-
return false;
105+
return Optional.empty();
94106
}
95107

96108
Expression element = elements.get(0);
97109
StringLiteral stringLiteral = Expressions.extractStringLiteral(element);
98110
if (stringLiteral == null) {
99-
return false;
111+
return Optional.empty();
100112
}
101113

102114
String methodName = stringLiteral.trimmedQuotesValue();
103-
return SINGLE_HTTP_METHODS.contains(methodName);
115+
if (SINGLE_HTTP_METHODS.contains(methodName)) {
116+
return Optional.of(methodName.toLowerCase(Locale.ROOT));
117+
}
118+
return Optional.empty();
104119
}
105120

106121
@CheckForNull
107122
private static CallExpression getDecoratorCallExpression(Decorator decorator) {
108123
Expression decoratorExpr = decorator.expression();
109124
return decoratorExpr instanceof CallExpression callExpr ? callExpr : null;
110125
}
126+
127+
private static void addQuickFix(PreciseIssue issue, CallExpression callExpr,
128+
RegularArgument methodsArg, String httpMethod) {
129+
Expression callee = callExpr.callee();
130+
if (!(callee instanceof QualifiedExpression qualifiedExpr)) {
131+
return;
132+
}
133+
Name routeName = qualifiedExpr.name();
134+
135+
var builder = PythonQuickFix.newQuickFix(String.format(QUICK_FIX_MESSAGE, httpMethod));
136+
builder.addTextEdit(TextEditUtils.replace(routeName, httpMethod));
137+
addRemoveMethodArgumentEdit(builder, callExpr, methodsArg);
138+
139+
issue.addQuickFix(builder.build());
140+
}
141+
142+
private static void addRemoveMethodArgumentEdit(PythonQuickFix.Builder builder,
143+
CallExpression callExpr,
144+
RegularArgument methodsArg) {
145+
List<Argument> arguments = callExpr.arguments();
146+
int argIndex = arguments.indexOf(methodsArg);
147+
148+
if (argIndex == -1) {
149+
return;
150+
}
151+
152+
if (arguments.size() == 1) {
153+
builder.addTextEdit(TextEditUtils.remove(methodsArg));
154+
} else if (argIndex == arguments.size() - 1) {
155+
Argument previousArg = arguments.get(argIndex - 1);
156+
var lastToken = previousArg.lastToken();
157+
builder.addTextEdit(TextEditUtils.replaceRange(lastToken, methodsArg.lastToken(), lastToken.value()));
158+
} else {
159+
Argument nextArg = arguments.get(argIndex + 1);
160+
builder.addTextEdit(TextEditUtils.removeUntil(methodsArg, nextArg));
161+
}
162+
}
111163
}

‎python-checks/src/test/java/org/sonar/python/checks/FastAPIGenericRouteDecoratorCheckTest.java‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19+
import java.util.stream.Stream;
1920
import org.junit.jupiter.api.Test;
21+
import org.junit.jupiter.params.ParameterizedTest;
22+
import org.junit.jupiter.params.provider.Arguments;
23+
import org.junit.jupiter.params.provider.MethodSource;
24+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
2025
import org.sonar.python.checks.utils.PythonCheckVerifier;
2126

2227
class FastAPIGenericRouteDecoratorCheckTest {
@@ -27,4 +32,141 @@ class FastAPIGenericRouteDecoratorCheckTest {
2732
void test() {
2833
PythonCheckVerifier.verify("src/test/resources/checks/fastAPIGenericRouteDecorator.py", check);
2934
}
35+
36+
@ParameterizedTest
37+
@MethodSource("quickFixWithMessageTestCases")
38+
void quickFixWithMessage(String testName, String before, String after, String expectedMessage) {
39+
PythonQuickFixVerifier.verify(check, before, after);
40+
PythonQuickFixVerifier.verifyQuickFixMessages(check, before, expectedMessage);
41+
}
42+
43+
@ParameterizedTest
44+
@MethodSource("quickFixTestCases")
45+
void quickFix(String testName, String before, String after) {
46+
PythonQuickFixVerifier.verify(check, before, after);
47+
}
48+
49+
static Stream<Arguments> quickFixWithMessageTestCases() {
50+
return Stream.of(
51+
Arguments.of(
52+
"basic GET",
53+
"""
54+
from fastapi import FastAPI
55+
app = FastAPI()
56+
@app.route("/users", methods=["GET"])
57+
def get_users():
58+
return {"users": []}""",
59+
"""
60+
from fastapi import FastAPI
61+
app = FastAPI()
62+
@app.get("/users")
63+
def get_users():
64+
return {"users": []}""",
65+
"Replace with \"get\""),
66+
Arguments.of(
67+
"POST method",
68+
"""
69+
from fastapi import FastAPI
70+
app = FastAPI()
71+
@app.route("/users", methods=["POST"])
72+
def create_user():
73+
return {}""",
74+
"""
75+
from fastapi import FastAPI
76+
app = FastAPI()
77+
@app.post("/users")
78+
def create_user():
79+
return {}""",
80+
"Replace with \"post\""),
81+
Arguments.of(
82+
"router DELETE",
83+
"""
84+
from fastapi import APIRouter
85+
router = APIRouter()
86+
@router.route("/items", methods=["DELETE"])
87+
def delete_item():
88+
return {}""",
89+
"""
90+
from fastapi import APIRouter
91+
router = APIRouter()
92+
@router.delete("/items")
93+
def delete_item():
94+
return {}""",
95+
"Replace with \"delete\""),
96+
Arguments.of(
97+
"lowercase method",
98+
"""
99+
from fastapi import FastAPI
100+
app = FastAPI()
101+
@app.route("/lower", methods=["get"])
102+
def lower_method():
103+
return {}""",
104+
"""
105+
from fastapi import FastAPI
106+
app = FastAPI()
107+
@app.get("/lower")
108+
def lower_method():
109+
return {}""",
110+
"Replace with \"get\""));
111+
}
112+
113+
static Stream<Arguments> quickFixTestCases() {
114+
return Stream.of(
115+
Arguments.of(
116+
"methods first",
117+
"""
118+
from fastapi import FastAPI
119+
app = FastAPI()
120+
@app.route(methods=["GET"], path="/users")
121+
def get_users():
122+
return {}""",
123+
"""
124+
from fastapi import FastAPI
125+
app = FastAPI()
126+
@app.get(path="/users")
127+
def get_users():
128+
return {}"""),
129+
Arguments.of(
130+
"with additional params",
131+
"""
132+
from fastapi import FastAPI
133+
app = FastAPI()
134+
@app.route("/users", methods=["GET"], response_model=None)
135+
def get_users():
136+
return {}""",
137+
"""
138+
from fastapi import FastAPI
139+
app = FastAPI()
140+
@app.get("/users", response_model=None)
141+
def get_users():
142+
return {}"""),
143+
Arguments.of(
144+
"methods middle position",
145+
"""
146+
from fastapi import FastAPI
147+
app = FastAPI()
148+
@app.route("/users", methods=["PUT"], status_code=200, response_model=None)
149+
def update_user():
150+
return {}""",
151+
"""
152+
from fastapi import FastAPI
153+
app = FastAPI()
154+
@app.put("/users", status_code=200, response_model=None)
155+
def update_user():
156+
return {}"""),
157+
Arguments.of(
158+
"only one arg",
159+
"""
160+
from fastapi import FastAPI
161+
app = FastAPI()
162+
@app.route(methods=["GET"])
163+
def get_users():
164+
return {}""",
165+
"""
166+
from fastapi import FastAPI
167+
app = FastAPI()
168+
@app.get()
169+
def get_users():
170+
return {}"""));
171+
}
30172
}

0 commit comments

Comments
 (0)