SONARPY-3613 Extend S6552 to support Flask route decorators alongside Django receiver (#816)

GitOrigin-RevId: e89907f1d61c48975fc396e0ea5ac510c79ea1a6

Author: marc-jasper-sonarsource

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

@@ -41,7 +41,6 @@
 import org.sonar.python.checks.django.DjangoModelFormFieldsCheck;
 import org.sonar.python.checks.django.DjangoModelStrMethodCheck;
 import org.sonar.python.checks.django.DjangoModelStringFieldCheck;
-import org.sonar.python.checks.django.DjangoReceiverDecoratorCheck;
 import org.sonar.python.checks.hotspots.ClearTextProtocolsCheck;
 import org.sonar.python.checks.hotspots.CommandLineArgsCheck;
 import org.sonar.python.checks.hotspots.CorsCheck;
@@ -484,7 +483,7 @@ public Stream<Class<?>> getChecks() {
       XMLParserXXEVulnerableCheck.class,
       XMLSignatureValidationCheck.class,
       DjangoModelFormFieldsCheck.class,
-      DjangoReceiverDecoratorCheck.class,
+      WebEntryPointDecoratorCheck.class,
       DjangoModelStringFieldCheck.class,
       DjangoModelStrMethodCheck.class,
       HardcodedCredentialsCallCheck.class);

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

@@ -0,0 +1,109 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks;
+
+import java.util.List;
+import java.util.Optional;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Decorator;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.FunctionDef;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
+import org.sonar.python.quickfix.TextEditUtils;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S6552")
+public class WebEntryPointDecoratorCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE_TEMPLATE = "Move this '%s' decorator to the top of the other decorators.";
+  private static final String QUICK_FIX_TEMPLATE = "Move the '%s' decorator to the top";
+
+  private record DecoratorMatcher(TypeMatcher matcher, String decoratorName) {}
+
+  private static final List<DecoratorMatcher> DECORATOR_MATCHERS = List.of(
+    new DecoratorMatcher(TypeMatchers.withFQN("django.dispatch.receiver"), "@receiver"),
+    new DecoratorMatcher(TypeMatchers.any(
+      TypeMatchers.isType("flask.app.Flask.route"),
+      TypeMatchers.isType("flask.blueprints.Blueprint.route")
+    ), "@route")
+  );
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, WebEntryPointDecoratorCheck::checkFunctionDef);
+  }
+
+  private static void checkFunctionDef(SubscriptionContext ctx) {
+    var functionDef = (FunctionDef) ctx.syntaxNode();
+    var decorators = functionDef.decorators();
+
+    if (decorators.size() < 2) {
+      return;
+    }
+
+    for (var matcher : DECORATOR_MATCHERS) {
+      decorators.stream()
+        .filter(decorator -> matchesDecorator(decorator, matcher.matcher(), ctx))
+        .findFirst()
+        .ifPresent(decorator -> checkDecoratorPosition(ctx, functionDef, decorator, matcher.decoratorName()));
+    }
+  }
+
+  private static boolean matchesDecorator(Decorator decorator, TypeMatcher matcher, SubscriptionContext ctx) {
+    Expression expression = decorator.expression();
+    if (!(expression instanceof CallExpression callExpr)) {
+      return false;
+    }
+    return matcher.isTrueFor(callExpr.callee(), ctx);
+  }
+
+  private static void checkDecoratorPosition(SubscriptionContext ctx, FunctionDef functionDef, Decorator decorator, String decoratorName) {
+    var decorators = functionDef.decorators();
+    if (decorators.indexOf(decorator) != 0) {
+      String message = String.format(MESSAGE_TEMPLATE, decoratorName);
+      String quickFixMessage = String.format(QUICK_FIX_TEMPLATE, decoratorName);
+      var issue = ctx.addIssue(decorator, message);
+      addQuickFix(issue, functionDef, decorator, quickFixMessage);
+    }
+  }
+
+  private static void addQuickFix(PreciseIssue issue, FunctionDef functionDef, Decorator decorator, String quickFixMessage) {
+    var builder = PythonQuickFix.newQuickFix(quickFixMessage);
+
+    var decorators = functionDef.decorators();
+    var decoratorPosition = decorators.indexOf(decorator);
+    var removeTo = decorators.size() == decoratorPosition + 1 ? functionDef.defKeyword()
+      : decorators.get(decoratorPosition + 1);
+
+    Optional.of(decorator)
+      .map(d -> TreeUtils.treeToString(d, true))
+      .map(decoratorString -> TextEditUtils.insertBefore(decorators.get(0), decoratorString))
+      .ifPresent(builder::addTextEdit);
+
+    var removeEdit = TextEditUtils.removeUntil(decorator, removeTo);
+    builder.addTextEdit(removeEdit);
+
+    issue.addQuickFix(builder.build());
+  }
+}
+

python-checks/src/main/java/org/sonar/python/checks/django/DjangoReceiverDecoratorCheck.java

@@ -1,26 +1,30 @@
-<p>This rule enforces that the '@receiver' decorator is placed on top of all other decorators in Django functions.</p>
+<p>This rule enforces that the <code>@receiver</code> decorator in Django and the <code>@route</code> decorator in Flask are placed on top of all
+other decorators.</p>
 <h2>Why is this an issue?</h2>
-<p>In Django, the '@receiver' decorator is used to register signal handlers. These handlers are used to respond to events that occur in the
-application, such as a user logging in or a database record being saved.</p>
-<p>The order in which decorators are applied can have a significant impact on their behavior. In the case of the @receiver decorator, it is important
-that it is applied first, before any other decorators, in order to ensure that the signal handler is registered correctly.</p>
-<p>If the '@receiver' decorator is not applied first, the decorators placed above it will be ignored, which can result in unexpected behavior or even
-errors in the application.</p>
-<h2>How to fix it</h2>
-<p>To fix this issue, simply move the '@receiver' decorator to the top of the list of decorators used to decorate the function.</p>
+<p>The order in which decorators are applied can have a significant impact on their behavior. When decorators are applied in Python, they are
+processed from bottom to top. The outermost decorator (the one written first) is applied last and receives the result of all inner decorators.</p>
+<p>In Django, the <code>@receiver</code> decorator is used to register signal handlers. It must be the outermost decorator (written at the top) so
+that all other decorators are applied to the function before it gets registered as a signal handler. If <code>@receiver</code> is not outermost, it
+registers the original function, and the decorators written above it will not affect the signal handler, resulting in unexpected behavior or
+errors.</p>
+<p>In Flask, the <code>@app.route()</code> or <code>@blueprint.route()</code> decorator must be the outermost decorator for view functions. If other
+decorators are placed outside the route decorator, the route may not be registered correctly, leading to 404 errors or routes that don’t respond to
+requests as expected.</p>
+<h2>How to fix it in Django</h2>
+<p>Move the <code>@receiver</code> decorator to the top of the list of decorators used to decorate the function.</p>
 <h3>Code examples</h3>
 <h4>Noncompliant code example</h4>
-<pre>
+<pre data-diff-id="1" data-diff-type="noncompliant">
 from django.dispatch import receiver
 from django.views.decorators.csrf import csrf_exempt
 
 @csrf_exempt
-@receiver(some_signal)
+@receiver(some_signal)  # Noncompliant: @receiver should be the outermost decorator
 def my_handler(sender, **kwargs):
     ...
 </pre>
 <h4>Compliant solution</h4>
-<pre>
+<pre data-diff-id="1" data-diff-type="compliant">
 from django.dispatch import receiver
 from django.views.decorators.csrf import csrf_exempt
 
@@ -29,9 +33,28 @@ <h4>Compliant solution</h4>
 def my_handler(sender, **kwargs):
     ...
 </pre>
+<h2>How to fix it in Flask</h2>
+<p>Move the <code>@app.route()</code> or <code>@blueprint.route()</code> decorator to be the outermost (first) decorator.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+@login_required
+@app.route('/secret_page')  # Noncompliant: @route should be the outermost decorator
+def secret_page():
+    return "Secret content"
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+@app.route('/secret_page')
+@login_required
+def secret_page():
+    return "Secret content"
+</pre>
 <h2>Resources</h2>
 <h3>Documentation</h3>
 <ul>
   <li> <a href="https://docs.djangoproject.com/en/4.1/topics/signals/">Django signals</a> </li>
+  <li> <a href="https://flask.palletsprojects.com/en/stable/patterns/viewdecorators/">Flask View Decorators</a> </li>
+  <li> <a href="https://flask.palletsprojects.com/en/stable/quickstart/#routing">Flask Routing</a> </li>
 </ul>
 

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6552.html

@@ -1,5 +1,5 @@
 {
-  "title": "Django signal handler functions should have the \u0027@receiver\u0027 decorator on top of all other decorators",
+  "title": "The \u0027@receiver\u0027 (Django) and \u0027@route\u0027 (Flask) decorators should be the outermost decorators",
   "type": "BUG",
   "code": {
     "impacts": {
@@ -14,11 +14,12 @@
   },
   "tags": [
     "pitfall",
-    "django"
+    "django",
+    "flask"
   ],
   "defaultSeverity": "Major",
   "ruleSpecification": "RSPEC-6552",
   "sqKey": "S6552",
   "scope": "All",
-  "quickfix": "unknown"
+  "quickfix": "covered"
 }

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

@@ -14,33 +14,34 @@
  * You should have received a copy of the Sonar Source-Available License
  * along with this program; if not, see https://sonarsource.com/license/ssal/
  */
-package org.sonar.python.checks.django;
+package org.sonar.python.checks;
 
 import org.junit.jupiter.api.Test;
 import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
 import org.sonar.python.checks.utils.PythonCheckVerifier;
 
-class DjangoReceiverDecoratorCheckTest {
+class WebEntryPointDecoratorCheckTest {
+
   @Test
-  void test() {
-    PythonCheckVerifier.verify("src/test/resources/checks/django/djangoReceiverDecoratorCheck.py", new DjangoReceiverDecoratorCheck());
+  void testDjangoReceiver() {
+    PythonCheckVerifier.verify("src/test/resources/checks/webEntryPointDecorator/djangoReceiverDecorator.py", new WebEntryPointDecoratorCheck());
   }
 
   @Test
-  void testRenamedImport() {
-    PythonCheckVerifier.verify("src/test/resources/checks/django/djangoReceiverDecoratorCheck_renamed_import.py",
-      new DjangoReceiverDecoratorCheck());
+  void testDjangoReceiverRenamedImport() {
+    PythonCheckVerifier.verify("src/test/resources/checks/webEntryPointDecorator/djangoReceiverDecoratorRenamedImport.py",
+      new WebEntryPointDecoratorCheck());
   }
 
   @Test
-  void testWrongImport() {
-    PythonCheckVerifier.verifyNoIssue("src/test/resources/checks/django/djangoReceiverDecoratorCheck_wrong_import.py",
-      new DjangoReceiverDecoratorCheck());
+  void testDjangoReceiverWrongImport() {
+    PythonCheckVerifier.verifyNoIssue("src/test/resources/checks/webEntryPointDecorator/djangoReceiverDecoratorWrongImport.py",
+      new WebEntryPointDecoratorCheck());
   }
 
   @Test
-  void quickFixTest() {
-    var check = new DjangoReceiverDecoratorCheck();
+  void djangoReceiverQuickFixTest() {
+    var check = new WebEntryPointDecoratorCheck();
     var before = """
       from django.dispatch import receiver
       @csrf_exempt
@@ -76,4 +77,31 @@ def my_handler(sender, **kwargs):
     PythonQuickFixVerifier.verifyQuickFixMessages(check, before, "Move the '@receiver' decorator to the top");
   }
 
+  @Test
+  void testFlaskRoute() {
+    PythonCheckVerifier.verify("src/test/resources/checks/webEntryPointDecorator/flaskRouteDecorator.py", new WebEntryPointDecoratorCheck());
+  }
+
+  @Test
+  void flaskRouteQuickFixTest() {
+    var check = new WebEntryPointDecoratorCheck();
+    var before = """
+      from flask import Flask
+      app = Flask(__name__)
+      @login_required
+      @app.route('/secret')
+      def secret_page():
+          return "Secret" """;
+
+    var after = """
+      from flask import Flask
+      app = Flask(__name__)
+      @app.route('/secret')
+      @login_required
+      def secret_page():
+          return "Secret" """;
+    PythonQuickFixVerifier.verify(check, before, after);
+    PythonQuickFixVerifier.verifyQuickFixMessages(check, before, "Move the '@route' decorator to the top");
+  }
 }
+