SONARPY-2523: S6735 Fix FP when providing a value for keyword "on" when "how=cross" is used (#362)

GitOrigin-RevId: 9d23ae434fc8066a87ddf714c52c73e1c6cf76c0

Author: joke1196

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

@@ -16,6 +16,8 @@
  */
 package org.sonar.python.checks;
 
+import static org.sonar.python.tree.TreeUtils.toOptionalInstanceOfMapper;
+
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
@@ -30,6 +32,8 @@
 import org.sonar.plugins.python.api.symbols.Symbol;
 import org.sonar.plugins.python.api.tree.Argument;
 import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.StringLiteral;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.python.quickfix.TextEditUtils;
 import org.sonar.python.tree.TreeUtils;
@@ -130,7 +134,8 @@ private static void missingArguments(String fullyQualifiedName, SubscriptionCont
     if (isArgumentMissing(fullyQualifiedName, Keywords.HOW, callExpression.arguments())) {
       missingKeywords.add(Keywords.HOW);
     }
-    if (ON_KEYWORDS.stream().allMatch(keyword -> isArgumentMissing(fullyQualifiedName, keyword, callExpression.arguments()))) {
+    if (ON_KEYWORDS.stream()
+      .allMatch(keyword -> !isCrossJoin(fullyQualifiedName, callExpression) && isArgumentMissing(fullyQualifiedName, keyword, callExpression.arguments()))) {
       missingKeywords.add(Keywords.ON);
     }
     if (isArgumentMissing(fullyQualifiedName, Keywords.VALIDATE, callExpression.arguments())) {
@@ -146,6 +151,15 @@ private static void missingArguments(String fullyQualifiedName, SubscriptionCont
     }
   }
 
+  private static boolean isCrossJoin(String fullyQualifiedName, CallExpression callExpression) {
+    return TreeUtils.nthArgumentOrKeywordOptional(Keywords.HOW.getArgumentPosition(fullyQualifiedName), Keywords.HOW.getKeyword(), callExpression.arguments())
+      .map(RegularArgument::expression)
+      .flatMap(toOptionalInstanceOfMapper(StringLiteral.class))
+      .map(StringLiteral::trimmedQuotesValue)
+      .map("cross"::equals)
+      .orElse(false);
+  }
+
   private static boolean isArgumentMissing(String fullyQualfiedName, Keywords keyword, List<Argument> arguments) {
     return Optional.of(keyword)
       .map(kw -> TreeUtils.nthArgumentOrKeyword(kw.getArgumentPosition(fullyQualfiedName), kw.getKeyword(), arguments))

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

@@ -16,7 +16,6 @@ def non_compliant_merge_1():
 
     _ = age_df.merge(name_df, on="user_id", validate="1:1")  # Noncompliant {{Specify the "how" parameter of this merge.}}
 
-    _ = age_df.merge(name_df, how="cross", validate="1:1")  # Noncompliant {{Specify the "on" parameter of this merge.}}
 
     _ = pd.merge(age_df, name_df, on="user_id")  # Noncompliant {{Specify the "how" and "validate" parameters of this merge.}}
 
@@ -30,8 +29,6 @@ def non_compliant_merge_1():
 
     _ = pd.merge(age_df, name_df, on="user_id", validate="1:1")  # Noncompliant {{Specify the "how" parameter of this merge.}}
 
-    _ = pd.merge(age_df, name_df, how="cross",  # Noncompliant {{Specify the "on" parameter of this merge.}}
-                 validate="1:1")
 
     _ = age_df.join(name_df)  # Noncompliant {{Specify the "how", "on" and "validate" parameters of this join.}}
 
@@ -45,7 +42,6 @@ def non_compliant_merge_1():
 
     _ = age_df.join(name_df, on="user_id", validate="1:1")  # Noncompliant {{Specify the "how" parameter of this join.}}
 
-    _ = age_df.join(name_df, how="cross", validate="1:1")  # Noncompliant {{Specify the "on" parameter of this join.}}
 
 
 def non_compliant_2():
@@ -67,11 +63,11 @@ def non_compliant_2():
 
     _ = merge(age_df, name_df, on="user_id", validate="1:1")  # Noncompliant {{Specify the "how" parameter of this merge.}}
 
-    _ = merge(age_df, name_df, how="cross", validate="1:1")  # Noncompliant {{Specify the "on" parameter of this merge.}}
 
 
 def compliant_1(xx):
     import pandas as pd
+    from pandas import merge
 
     age_df = pd.DataFrame({"user_id": [1, 2, 4], "age": [42, 45, 35]})
     name_df = pd.DataFrame({"user_id": [1, 2, 3, 4], "name": ["a", "b", "c", "d"]})
@@ -98,6 +94,13 @@ def compliant_1(xx):
 
     _ = pd.merge(age_df, name_df, right_on='cat', how='left', validate='m:m')
 
+    _ = pd.merge(age_df, name_df, how="cross",  # Cross should not provide the on argument
+                 validate="1:1")
+
+    _ = age_df.join(name_df, how="cross", validate="1:1")  # Cross should not provide the on argument
+
+    _ = age_df.merge(name_df, how="cross", validate="1:1") # Cross should not provide the on argument
+
     _ = age_df.merge(name_df, right_on='cat', how='left', validate='m:m')
 
     _ = age_df.merge(name_df, right_on='cat', how='left', validate='m:m')