Skip to content

Commit cc53436

Browse files
sonar-nigel[bot]Vibe BotSeppli11
authored andcommitted
SONARPY-3985 Fix S3457 false positive for f-strings in marimo SQL cells (#1043)
Co-authored-by: Vibe Bot <vibe-bot@sonarsource.com> Co-authored-by: Sebastian Zumbrunn <sebastian.zumbrunn@sonarsource.com> GitOrigin-RevId: 0f0c179885f2f4b46af00c1ae9b8afcffebdff17
1 parent 8744029 commit cc53436

File tree

5 files changed

+57
-3
lines changed

5 files changed

+57
-3
lines changed

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
import org.sonar.check.Rule;
2727
import org.sonar.plugins.python.api.SubscriptionContext;
2828
import org.sonar.plugins.python.api.symbols.Symbol;
29+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
30+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
31+
import org.sonar.plugins.python.api.tree.ArgList;
2932
import org.sonar.plugins.python.api.tree.BinaryExpression;
3033
import org.sonar.plugins.python.api.tree.CallExpression;
3134
import org.sonar.plugins.python.api.tree.DictionaryLiteral;
@@ -44,6 +47,8 @@
4447
@Rule(key = "S3457")
4548
public class StringFormatCorrectnessCheck extends AbstractStringFormatCheck {
4649

50+
private static final TypeMatcher MARIMO_SQL_MATCHER = TypeMatchers.isType("marimo.sql");
51+
4752
private static final Set<String> LOGGER_FULL_NAMES = new HashSet<>(Arrays.asList(
4853
"logging.debug", "logging.info", "logging.warning", "logging.error", "logging.critical",
4954
"logging.Logger.debug", "logging.Logger.info", "logging.Logger.warning", "logging.Logger.error", "logging.Logger.critical"
@@ -114,11 +119,28 @@ private static void checkFStringLiteral(SubscriptionContext ctx) {
114119
.filter(stringElement -> stringElement.prefix().toLowerCase(Locale.ENGLISH).contains("f"))
115120
.toList();
116121

117-
if (!fStrings.isEmpty() && fStrings.stream().allMatch(str -> str.formattedExpressions().isEmpty())) {
122+
if (!fStrings.isEmpty() && fStrings.stream().allMatch(str -> str.formattedExpressions().isEmpty())
123+
&& !isMarimoSqlArgument(literal, ctx)) {
118124
ctx.addIssue(literal, "Add replacement fields or use a normal string instead of an f-string.");
119125
}
120126
}
121127

128+
private static boolean isMarimoSqlArgument(StringLiteral literal, SubscriptionContext ctx) {
129+
Tree parent = literal.parent();
130+
if (!(parent instanceof RegularArgument)) {
131+
return false;
132+
}
133+
Tree argList = parent.parent();
134+
if (!(argList instanceof ArgList)) {
135+
return false;
136+
}
137+
Tree callParent = argList.parent();
138+
if (!(callParent instanceof CallExpression callExpression)) {
139+
return false;
140+
}
141+
return MARIMO_SQL_MATCHER.isTrueFor(callExpression.callee(), ctx);
142+
}
143+
122144
private static void checkLoggerLog(SubscriptionContext ctx, CallExpression callExpression) {
123145
if (callExpression.arguments().isEmpty() || callExpression.arguments().stream().anyMatch(argument -> !argument.is(Tree.Kind.REGULAR_ARGUMENT))) {
124146
return;

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,3 +119,26 @@ def edge_case():
119119
no_such_function('hello')
120120
l4 = no_such_function('')
121121
l4.error("Foo %s", "Bar", "too many")
122+
123+
124+
import marimo as mo
125+
126+
def marimo_sql():
127+
mo.sql(f"SELECT * FROM table")
128+
mo.sql(
129+
f"""
130+
SELECT plugin_rule_key, COUNT(*) as cnt
131+
FROM issues
132+
GROUP BY plugin_rule_key
133+
"""
134+
)
135+
engine = None
136+
mo.sql(
137+
f"""
138+
SELECT id, name
139+
FROM users
140+
WHERE active = 1
141+
""",
142+
engine=engine
143+
)
144+

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/marimo.protobuf

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ marimo.App*
3131
args
3232
Any*
3333
kwargs
34+
AnyX
35+
sql
36+
marimo.sql"
37+
Any*'
38+
query
39+
builtins.str" builtins.str*
40+
kwargs
3441
Any*g
3542
__path__marimo.__path__J
3643
builtins.list[builtins.str]
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
669032ff87c6a7027931d1987597a4371fc80b9da0f7817762518ec75d1fa357
2-
8a2fb0e2abba9b8e562913a29440ec31c7fd322f0c31d2d64d9a6c9591b444ed
1+
4ed5a5d152958ba1e643059b3f9957815405f44596c64d268aa525aab7c74b5e
2+
4394f7b9850885f1aab48413ba65f330a2c5c83a626202c0075d44b217f37643

python-frontend/typeshed_serializer/resources/custom/marimo/__init__.pyi

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,5 @@ class App(CustomStubBase):
55
def __init__(self, *args: Any, **kwargs: Any) -> None: ...
66
def cell(self, *args: Any, **kwargs: Any) -> Any: ...
77
def function(self, *args: Any, **kwargs: Any) -> Any: ...
8+
9+
def sql(query: str, **kwargs: Any) -> Any: ...

0 commit comments

Comments
 (0)