SONARPY-3996 Rule S8504 Property methods should have a return statement (#1025)

Co-authored-by: Sonar Vibe Bot <vibe-bot@sonarsource.com>
Co-authored-by: David Kunzmann <david.kunzmann@sonarsource.com>
GitOrigin-RevId: 177d15782e00e2aeb45196ab1c23e59ac4a4cfac

Author: 3 people

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

@@ -363,6 +363,7 @@ public Stream<Class<?>> getChecks() {
       PrivilegePolicyCheck.class,
       ProcessSignallingCheck.class,
       PropertyAccessorParameterCountCheck.class,
+      PropertyMethodWithoutReturnCheck.class,
       PytzUsageCheck.class,
       PseudoRandomCheck.class,
       PublicApiIsSecuritySensitiveCheck.class,

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

@@ -0,0 +1,59 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+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.checks.utils.CheckUtils;
+
+@Rule(key = "S8504")
+public class PropertyMethodWithoutReturnCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Add a return statement to this property method.";
+  private static final TypeMatcher PROPERTY_MATCHER = TypeMatchers.isType("builtins.property");
+  private static final TypeMatcher ABSTRACT_METHOD_MATCHER = TypeMatchers.isType("abc.abstractmethod");
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> {
+      FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
+
+      boolean isProperty = functionDef.decorators().stream()
+        .anyMatch(d -> PROPERTY_MATCHER.isTrueFor(d.expression(), ctx));
+      boolean isAbstract = functionDef.decorators().stream()
+        .anyMatch(d -> ABSTRACT_METHOD_MATCHER.isTrueFor(d.expression(), ctx));
+
+      if (!isProperty || isAbstract) {
+        return;
+      }
+
+      if (functionDef.body().statements().stream().allMatch(CheckUtils::isEmptyStatement)) {
+        return;
+      }
+
+      var collector = ReturnCheckUtils.ReturnStmtCollector.collect(functionDef);
+
+      if (collector.getReturnStmts().isEmpty() && !collector.containsYield() && !collector.raisesExceptions()) {
+        ctx.addIssue(functionDef.name(), MESSAGE);
+      }
+    });
+  }
+}

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

@@ -0,0 +1,45 @@
+<p>This rule raises an issue when a method decorated with <code>@property</code> does not contain any <code>return</code> statement.</p>
+<h2>Why is this an issue?</h2>
+<p>In Python, the <code>@property</code> decorator transforms a method into a getter for a computed attribute. When you access a property, you expect
+to receive a value, just like accessing a regular attribute.</p>
+<p>A property method without a <code>return</code> statement will implicitly return <code>None</code>. This makes the property useless because it
+always provides <code>None</code> instead of a meaningful value. This behavior is almost always unintentional and indicates a bug in the code.</p>
+<h3>Exceptions</h3>
+<p>This rule does not raise an issue when the property method raises an exception or is decorated with <code>@abstractmethod</code>, as these are
+intentional patterns where a return value is not expected.</p>
+<h3>What is the potential impact?</h3>
+<p>When a property method lacks a return statement, it always returns <code>None</code>. This leads to:</p>
+<ul>
+  <li><strong>logic errors</strong>: code that depends on the property value will behave incorrectly, potentially causing application failures or
+  incorrect results.</li>
+  <li><strong>difficult debugging</strong>: the implicit <code>None</code> return is not obvious from looking at the property access, making bugs
+  harder to trace.</li>
+  <li><strong>API contract violation</strong>: users of the class expect the property to provide meaningful data, but receive <code>None</code>
+  instead, breaking the expected interface.</li>
+</ul>
+<h2>How to fix it</h2>
+<p>Add a <code>return</code> statement to the property method that returns the intended value.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+@property
+def area(self):
+    3.14159 * self._radius ** 2  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+@property
+def area(self):
+    return 3.14159 * self._radius ** 2
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/functions.html#property"><code>property</code></a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/howto/descriptor.html#properties">Properties</a></li>
+</ul>
+<h3>Related rules</h3>
+<ul>
+  <li>{rule:python:S3801} - Functions should use "return" consistently</li>
+</ul>
+

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

@@ -0,0 +1,23 @@
+{
+  "title": "Property methods should have a return statement",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "pitfall"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-8504",
+  "sqKey": "S8504",
+  "scope": "All",
+  "quickfix": "targeted",
+  "code": {
+    "impacts": {
+      "RELIABILITY": "HIGH"
+    },
+    "attribute": "LOGICAL"
+  }
+}

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

@@ -328,6 +328,7 @@
     "S8413",
     "S8414",
     "S8415",
+    "S8504",
     "S8517"
   ]
 }

python-checks/src/test/java/org/sonar/python/checks/PropertyMethodWithoutReturnCheckTest.java

@@ -0,0 +1,29 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 org.junit.jupiter.api.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class PropertyMethodWithoutReturnCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/propertyMethodWithoutReturn.py", new PropertyMethodWithoutReturnCheck());
+  }
+
+}