SONARPY-3013 Create QuickFix for Rule S7517 (#314)

GitOrigin-RevId: 85e9c524235197fb536f7c1e27508f59881c0d50

Author: maksim-grebeniuk-sonarsource

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

@@ -16,20 +16,26 @@
  */
 package org.sonar.python.checks;
 
+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.ComprehensionFor;
+import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.ForStatement;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.plugins.python.api.tree.Tuple;
 import org.sonar.plugins.python.api.types.v2.TriBool;
+import org.sonar.python.quickfix.TextEditUtils;
+import org.sonar.python.tree.TreeUtils;
 import org.sonar.python.types.v2.TypeCheckBuilder;
 
 @Rule(key = "S7517")
 public class LoopOverDictKeyValuesCheck extends PythonSubscriptionCheck {
-  public static final String DICT_FQN = "dict";
-  public static final String MESSAGE = "Use items to iterate over key-value pairs";
+  private static final String DICT_FQN = "dict";
+  private static final String MESSAGE = "Use items to iterate over key-value pairs";
+  private static final String QUICK_FIX_MESSAGE = "Replace with items method call";
   private TypeCheckBuilder dictTypeCheck;
 
 
@@ -51,7 +57,9 @@ private void checkForStatement(SubscriptionContext ctx) {
     if (expressions.size() == 2
         && testExpressions.size() == 1
         && dictTypeCheck.check(testExpressions.get(0).typeV2()) == TriBool.TRUE) {
-      ctx.addIssue(testExpressions.get(0), MESSAGE);
+      var dict = testExpressions.get(0);
+      var issue = ctx.addIssue(dict, MESSAGE);
+      createQuickFix(dict).ifPresent(issue::addQuickFix);
     }
   }
 
@@ -60,7 +68,17 @@ private void checkComprehensionFor(SubscriptionContext ctx) {
     if (comprehensionFor.loopExpression() instanceof Tuple tuple
         && tuple.elements().size() == 2
         && dictTypeCheck.check(comprehensionFor.iterable().typeV2()) == TriBool.TRUE) {
-      ctx.addIssue(comprehensionFor.iterable(), MESSAGE);
+      var dict = comprehensionFor.iterable();
+      var issue = ctx.addIssue(dict, MESSAGE);
+      createQuickFix(dict).ifPresent(issue::addQuickFix);
     }
   }
+
+  private static Optional<PythonQuickFix> createQuickFix(Expression dict) {
+    return Optional.ofNullable(TreeUtils.treeToString(dict, false))
+      .map("%s.items()"::formatted)
+      .map(replacementText -> TextEditUtils.replace(dict, replacementText))
+      .map(textEdit -> PythonQuickFix.newQuickFix(QUICK_FIX_MESSAGE).addTextEdit(textEdit))
+      .map(PythonQuickFix.Builder::build);
+  }
 }

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

@@ -16,7 +16,12 @@
  */
 package org.sonar.python.checks;
 
+import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
 import org.sonar.python.checks.utils.PythonCheckVerifier;
 
 class LoopOverDictKeyValuesCheckTest {
@@ -26,4 +31,57 @@ void test() {
     PythonCheckVerifier.verify("src/test/resources/checks/loopOverDictKeyValues.py", new LoopOverDictKeyValuesCheck());
   }
 
+  @ParameterizedTest
+  @MethodSource("quickFixTestCases")
+  void quickFixTest(String before, String after, String expectedMessage) {
+    var check = new LoopOverDictKeyValuesCheck();
+    PythonQuickFixVerifier.verify(check, before, after);
+    PythonQuickFixVerifier.verifyQuickFixMessages(check, before, expectedMessage);
+  }
+
+  @ParameterizedTest
+  @MethodSource("noQuickFixTestCases")
+  void noQuickFixTest(String before) {
+    PythonQuickFixVerifier.verifyNoQuickFixes(new LoopOverDictKeyValuesCheck(), before);
+  }
+
+  static Stream<Arguments> quickFixTestCases() {
+    return Stream.of(
+      Arguments.of(
+        """
+          some_dict = { "a": "b"}
+          for k, v in some_dict:
+            ...
+          """,
+        """
+          some_dict = { "a": "b"}
+          for k, v in some_dict.items():
+            ...
+          """,
+        "Replace with items method call"),
+      Arguments.of(
+        """
+          some_dict = { "hi": "hello"}
+          {k: v for k, v in some_dict}
+          """,
+        """
+          some_dict = { "hi": "hello"}
+          {k: v for k, v in some_dict.items()}
+          """,
+        "Replace with items method call")
+    );
+  }
+
+  static Stream<Arguments> noQuickFixTestCases() {
+    return Stream.of(
+      Arguments.of(
+        """
+          some_dict = {"hi": "hello"}
+          a = {k: v for k, v in {
+              "hi": "hello"
+          }}
+          """)
+    );
+  }
+
 }