Skip to content

Commit 03d16ed

Browse files
joke1196sonartech
authored andcommitted
SONARPY-3927 Migrated S5445 to TypeV2 (#967)
GitOrigin-RevId: 7015cf8f784fb3abd60fcb23bbefb6f861d8ecaa
1 parent 7ef0e84 commit 03d16ed

File tree

5 files changed

+58
-21
lines changed

5 files changed

+58
-21
lines changed

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

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,34 +16,38 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19-
import java.util.Arrays;
20-
import java.util.List;
2119
import java.util.Optional;
22-
import javax.annotation.Nullable;
2320
import org.sonar.check.Rule;
2421
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
22+
import org.sonar.plugins.python.api.SubscriptionContext;
2523
import org.sonar.plugins.python.api.tree.CallExpression;
2624
import org.sonar.plugins.python.api.tree.Tree;
27-
import org.sonar.plugins.python.api.symbols.Symbol;
25+
import org.sonar.plugins.python.api.types.v2.FullyQualifiedNameHelper;
26+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
27+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
2828

2929
@Rule(key = "S5445")
3030
public class TempFileCreationCheck extends PythonSubscriptionCheck {
3131

32-
private static final List<String> SUSPICIOUS_CALLS = Arrays.asList("os.tempnam", "os.tmpnam", "tempfile.mktemp");
32+
// os.tempnam and os.tmpnam were removed in Python 3 and have no stubs.
33+
// We use withFQN to match both qualified (os.tempnam) and direct import (from os import tempnam) forms,
34+
// as the callee gets typed as UnresolvedImportType("os.tempnam") in both cases.
35+
private static final TypeMatcher INSECURE_CALLS = TypeMatchers.any(
36+
TypeMatchers.isType("tempfile.mktemp"),
37+
TypeMatchers.withFQN("os.tempnam"),
38+
TypeMatchers.withFQN("os.tmpnam")
39+
);
3340

3441
@Override
3542
public void initialize(Context context) {
36-
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
37-
CallExpression callExpr = (CallExpression) ctx.syntaxNode();
38-
Symbol symbol = callExpr.calleeSymbol();
39-
isInsecureTempFile(symbol).ifPresent(s -> ctx.addIssue(callExpr, String.format("'%s' is insecure. Use 'tempfile.TemporaryFile' instead", s)));
40-
});
43+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, TempFileCreationCheck::checkCallExpression);
4144
}
4245

43-
private static Optional<String> isInsecureTempFile(@Nullable Symbol symbol) {
44-
if (symbol == null) {
45-
return Optional.empty();
46-
}
47-
return SUSPICIOUS_CALLS.stream().filter(call -> call.equals(symbol.fullyQualifiedName())).findFirst();
46+
private static void checkCallExpression(SubscriptionContext ctx) {
47+
CallExpression callExpr = (CallExpression) ctx.syntaxNode();
48+
Optional.of(callExpr.callee())
49+
.filter(callee -> INSECURE_CALLS.isTrueFor(callee, ctx))
50+
.flatMap(callee -> FullyQualifiedNameHelper.getFullyQualifiedName(callee.typeV2()))
51+
.ifPresent(name -> ctx.addIssue(callExpr.callee(), String.format("'%s' is insecure. Use 'tempfile.TemporaryFile' instead", name)));
4852
}
4953
}
Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,25 @@
11
import tempfile
22
import os
3+
from os import tempnam
4+
from os import tmpnam
35

46
filename = os.tempnam() # Noncompliant {{'os.tempnam' is insecure. Use 'tempfile.TemporaryFile' instead}}
5-
# ^^^^^^^^^^^^
7+
# ^^^^^^^^^^
68
tmp_file = open(filename, "w+b")
79

810
filename = os.tmpnam() # Noncompliant {{'os.tmpnam' is insecure. Use 'tempfile.TemporaryFile' instead}}
9-
# ^^^^^^^^^^^
11+
# ^^^^^^^^^
1012
tmp_file = open(filename, "w+b")
1113

1214
filename = tempfile.mktemp() # Noncompliant {{'tempfile.mktemp' is insecure. Use 'tempfile.TemporaryFile' instead}}
13-
# ^^^^^^^^^^^^^^^^^
15+
# ^^^^^^^^^^^^^^^
1416

1517
tmp_file = open(filename, "w+b")
1618

19+
filename = tempnam() # Noncompliant {{'os.tempnam' is insecure. Use 'tempfile.TemporaryFile' instead}}
20+
# ^^^^^^^
21+
22+
filename = tmpnam() # Noncompliant {{'os.tmpnam' is insecure. Use 'tempfile.TemporaryFile' instead}}
23+
# ^^^^^^
1724

1825
tmp_file = tempfile.TemporaryFile() # Compliant

python-frontend/src/main/java/org/sonar/python/semantic/v2/types/typecalculator/QualifiedExpressionCalculator.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import org.sonar.plugins.python.api.tree.Expression;
2121
import org.sonar.plugins.python.api.tree.Name;
2222
import org.sonar.plugins.python.api.tree.QualifiedExpression;
23+
import org.sonar.plugins.python.api.types.v2.FullyQualifiedNameHelper;
2324
import org.sonar.plugins.python.api.types.v2.FunctionType;
2425
import org.sonar.plugins.python.api.types.v2.PythonType;
26+
import org.sonar.plugins.python.api.types.v2.UnknownType;
2527
import org.sonar.python.semantic.v2.types.TypeInferenceMatcher;
2628
import org.sonar.python.semantic.v2.types.TypeInferenceMatchers;
2729
import org.sonar.python.types.v2.matchers.TypePredicateContext;
@@ -38,10 +40,16 @@ public QualifiedExpressionCalculator(TypePredicateContext typePredicateContext)
3840

3941
public PythonType calculate(QualifiedExpression qualifiedExpression) {
4042
Name name = qualifiedExpression.name();
41-
return Optional.of(qualifiedExpression.qualifier())
42-
.map(Expression::typeV2)
43+
PythonType qualifierType = qualifiedExpression.qualifier().typeV2();
44+
return Optional.of(qualifierType)
4345
.flatMap(t -> t.resolveMember(name.name()))
4446
.map(this::handlePropertyFunction)
47+
.orElseGet(() -> unresolvedMemberType(qualifierType, name.name()));
48+
}
49+
50+
private static PythonType unresolvedMemberType(PythonType qualifierType, String memberName) {
51+
return FullyQualifiedNameHelper.getFullyQualifiedName(qualifierType)
52+
.map(qualifierFqn -> (PythonType) new UnknownType.UnresolvedImportType(qualifierFqn + "." + memberName))
4553
.orElse(PythonType.UNKNOWN);
4654
}
4755

python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4041,8 +4041,10 @@ void reexportedWildcardImport() {
40414041
var actualFoo = tupleExpr.elements().get(1);
40424042

40434043
assertThat(importedFoo.typeV2())
4044-
.isEqualTo(PythonType.UNKNOWN)
4044+
.isInstanceOf(UnknownType.UnresolvedImportType.class)
40454045
.isNotEqualTo(actualFoo.typeV2());
4046+
assertThat(((UnknownType.UnresolvedImportType) importedFoo.typeV2()).importPath())
4047+
.isEqualTo("mod2.foo");
40464048
}
40474049

40484050
@Test

python-frontend/src/test/java/org/sonar/python/semantic/v2/types/typecalculator/QualifiedExpressionCalculatorTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.sonar.plugins.python.api.tree.Tree;
2323
import org.sonar.plugins.python.api.types.v2.FunctionType;
2424
import org.sonar.plugins.python.api.types.v2.PythonType;
25+
import org.sonar.plugins.python.api.types.v2.UnknownType;
2526
import org.sonar.python.PythonTestUtils;
2627
import org.sonar.python.types.v2.matchers.TypePredicateContext;
2728

@@ -114,6 +115,21 @@ class A:
114115
assertThat(result).isEqualTo(PythonType.UNKNOWN);
115116
}
116117

118+
@Test
119+
void calculate_unknownMemberOnKnownModule_returnsUnresolvedImportType() {
120+
FileInput fileInput = parseAndInferTypes("""
121+
import os
122+
os.nonexistent_member
123+
""");
124+
125+
var qualifiedExpression = getQualifiedExpressionFromStatement(fileInput);
126+
PythonType result = calculator.calculate(qualifiedExpression);
127+
128+
assertThat(result).isInstanceOf(UnknownType.UnresolvedImportType.class);
129+
var unresolvedType = (UnknownType.UnresolvedImportType) result;
130+
assertThat(unresolvedType.importPath()).isEqualTo("os.nonexistent_member");
131+
}
132+
117133
private static QualifiedExpression getQualifiedExpressionFromStatement(FileInput fileInput) {
118134
return PythonTestUtils.getLastDescendant(fileInput, t -> t.is(Tree.Kind.QUALIFIED_EXPR));
119135
}

0 commit comments

Comments
 (0)