SONARPY-1509 Rule S6799: f-strings should not be nested too deeply (#…

…1623)

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

@@ -199,6 +199,7 @@ public static Iterable<Class> getChecks() {
       FileHeaderCopyrightCheck.class,
       FixmeCommentCheck.class,
       FloatingPointEqualityCheck.class,
+      FStringNestingLevelCheck.class,
       FunctionComplexityCheck.class,
       FunctionNameCheck.class,
       FunctionReturnTypeCheck.class,

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

@@ -0,0 +1,113 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
+import javax.annotation.Nullable;
+
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.PythonVersionUtils;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.FormatSpecifier;
+import org.sonar.plugins.python.api.tree.FormattedExpression;
+import org.sonar.plugins.python.api.tree.StringElement;
+import org.sonar.plugins.python.api.tree.StringLiteral;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S6799")
+public class FStringNestingLevelCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Do not nest f-strings too deeply.";
+
+  private static final Set<StringElement> visited = new HashSet<>();
+
+  private static final int MAX_DEPTH = 3;
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> visited.clear());
+    context.registerSyntaxNodeConsumer(Tree.Kind.STRING_ELEMENT, FStringNestingLevelCheck::checkNestingDepthOfFString);
+  }
+
+  private static void checkNestingDepthOfFString(SubscriptionContext ctx) {
+    if (!supportsTypeParameterSyntax(ctx)) {
+      return;
+    }
+    StringElement element = (StringElement) ctx.syntaxNode();
+    if (isFStringNestedTooDeep(element, 0)) {
+      ctx.addIssue(element, MESSAGE);
+    }
+  }
+
+  private static boolean isFStringNestedTooDeep(StringElement element, final int count) {
+    if (!visited.contains(element) && element.isInterpolated()) {
+      visited.add(element);
+      int updatedCount = count + 1;
+      if (updatedCount >= MAX_DEPTH) {
+        return true;
+      }
+      return areFormattedExpressionsNestedTooDeep(element.formattedExpressions(), updatedCount);
+    }
+    return false;
+  }
+
+  private static boolean areFormattedExpressionsNestedTooDeep(List<FormattedExpression> formattedExpressions, int updatedCount) {
+    for (FormattedExpression formattedExpression : formattedExpressions) {
+      if (isTheNestingTooDeepInExpression(formattedExpression.expression(), updatedCount) ||
+        isTheNestingTooDeepInFormatSpecifier(formattedExpression.formatSpecifier(), updatedCount)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private static boolean isTheNestingTooDeepInExpression(Expression expression, int updatedCount) {
+    return Optional.of(expression)
+      .flatMap(TreeUtils.toOptionalInstanceOfMapper(StringLiteral.class))
+      .map(StringLiteral::stringElements)
+      .map(Collection::stream)
+      .map(elements -> elements
+        .anyMatch(sElement -> isFStringNestedTooDeep(sElement, updatedCount)))
+      .orElse(false);
+  }
+
+  private static boolean isTheNestingTooDeepInFormatSpecifier(@Nullable FormatSpecifier formatSpecifier, int updatedCount) {
+    return Optional.ofNullable(formatSpecifier)
+      .map(FormatSpecifier::formatExpressions)
+      .map(formattedExpressions -> areFormattedExpressionsNestedTooDeep(formattedExpressions, updatedCount))
+      .orElse(false);
+  }
+
+  private static boolean supportsTypeParameterSyntax(SubscriptionContext ctx) {
+    PythonVersionUtils.Version required = PythonVersionUtils.Version.V_312;
+
+    // All versions must be greater than or equal to the required version.
+    return ctx.sourcePythonVersions().stream()
+      .allMatch(version -> version.compare(required.major(), required.minor()) >= 0);
+  }
+}

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

@@ -0,0 +1,37 @@
+<p>This rule raises an issue f-strings are deeply nested.</p>
+<h2>Why is this an issue?</h2>
+<p>Through <a href="https://peps.python.org/pep-0701/">PEP 701</a>, Python 3.12 lifts restrictions on how to construct f-strings.</p>
+<p>Prior to Python 3.12, it was not possible to reuse string quotes when nesting f-strings. Therefore, the maximum level of nesting was:</p>
+<pre>
+f"""{f'''{f'{f"{1+1}"}'}'''}"""
+</pre>
+<p>It is now possible to arbitrarily nest f-strings by reusing string quotes. The following snippet is therefore valid:</p>
+<pre>
+f"{f"{f"{f"{f"{f"{1+1}"}"}"}"}"}"
+</pre>
+<p>It is, however, not recommended to nest f-strings too deeply as this would make the code confusing and hard to maintain.</p>
+<p>This rule will raise an issue if f-strings literals are nested more than 3 times.</p>
+<h2>How to fix it</h2>
+<p>To fix this issue, refactor the code to avoid nesting f-string literals too deeply. This may be done by introducing new variables to store
+intermediate results.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+hello = "Hello"
+name = "John"
+my_string = f"{f"{f"{hello}"},"} {name}!" # Noncompliant: deep nesting of f-strings is confusing
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+hello = "Hello"
+name = "John"
+greeting = f"{f"{hello}"},"
+my_string = f"{greeting} {name}!" # Compliant
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> PEP 701 <a href="https://peps.python.org/pep-0701/">Syntactic formalization of f-strings</a> </li>
+  <li> Python Release Notes <a href="https://docs.python.org/3/whatsnew/3.12.html#what-s-new-in-python-3-12">What’s New In Python 3.12</a> </li>
+</ul>
+

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

@@ -0,0 +1,21 @@
+{
+  "title": "f-strings should not be nested too deeply",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6799",
+  "sqKey": "S6799",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

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

@@ -217,6 +217,7 @@
     "S6742",
     "S6792",
     "S6794",
-    "S6796"
+    "S6796",
+    "S6799"
   ]
 }

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

@@ -0,0 +1,46 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.EnumSet;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.plugins.python.api.ProjectPythonVersion;
+import org.sonar.plugins.python.api.PythonVersionUtils;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class FStringNestingLevelCheckTest {
+
+  @Test
+  void test_1() {
+    ProjectPythonVersion.setCurrentVersions(EnumSet.of(PythonVersionUtils.Version.V_312));
+    PythonCheckVerifier.verify("src/test/resources/checks/fstringNestingLevel.py", new FStringNestingLevelCheck());
+  }
+
+  @Test
+  void test_2() {
+    ProjectPythonVersion.setCurrentVersions(EnumSet.of(PythonVersionUtils.Version.V_311, PythonVersionUtils.Version.V_312));
+    var issues = PythonCheckVerifier.issues("src/test/resources/checks/fstringNestingLevel.py", new FStringNestingLevelCheck());
+    assertThat(issues)
+      .isEmpty();
+  }
+}

python-checks/src/test/resources/checks/fstringNestingLevel.py

@@ -0,0 +1,17 @@
+
+def non_compliant(hello, name):
+    my_string = f"{f"{f"{hello}"},"} {name}!" # Noncompliant {{Do not nest f-strings too deeply.}}
+    #           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
+
+    greeting = f"{f"{hello}"},"
+    my_string = f"hello:{greeting}, {name}, {f"end: { f"done" }"}!" # Noncompliant
+#               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
+
+    my_string = f"hello { f"format specifier: {greeting :{F"1"}.{2}}"}" # Noncompliant
+#               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+def compliant(hello, name):
+    greeting = f"{f"{hello}"},"
+    my_string = f"{greeting} {name}!" # Compliant
+
+    my_string = f"{greeting} {name : { f"1" }.{2}}!"