Skip to content

Commit

Permalink
SONARPY-2366 Propagate binary expressions of the same type (#2216)
Browse files Browse the repository at this point in the history
  • Loading branch information
maksim-grebeniuk-sonarsource authored Dec 6, 2024
1 parent 4bd143a commit 5a9201b
Show file tree
Hide file tree
Showing 6 changed files with 261 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,18 @@
import org.sonar.python.semantic.v2.UsageV2;
import org.sonar.python.tree.NameImpl;
import org.sonar.python.tree.TreeUtils;
import org.sonar.python.types.HasTypeDependencies;
import org.sonar.python.types.v2.PythonType;
import org.sonar.python.types.v2.UnionType;

public class AstBasedTypeInference {
private final Map<SymbolV2, Set<Propagation>> propagationsByLhs;
private final Propagator propagator;
private final TypeDependenciesCalculator typeDependenciesCalculator;

public AstBasedTypeInference(Map<SymbolV2, Set<Propagation>> propagationsByLhs, TypeTable typeTable) {
this.propagationsByLhs = propagationsByLhs;
this.propagator = new Propagator(typeTable);
this.typeDependenciesCalculator = new TypeDependenciesCalculator();
}

public Map<SymbolV2, Set<PythonType>> process(Set<SymbolV2> trackedVars) {
Expand Down Expand Up @@ -81,8 +82,8 @@ private void computeDependencies(Assignment assignment, Set<SymbolV2> trackedVar
assignment.addVariableDependency(symbol);
propagationsByLhs.get(symbol).forEach(propagation -> propagation.addDependent(assignment));
}
} else if (e instanceof HasTypeDependencies hasTypeDependencies) {
workList.addAll(hasTypeDependencies.typeDependencies());
} else if (typeDependenciesCalculator.hasTypeDependencies(e)) {
workList.addAll(typeDependenciesCalculator.getTypeDependencies(e));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@
package org.sonar.python.semantic.v2.types;

import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
import org.sonar.plugins.python.api.tree.BinaryExpression;
import org.sonar.plugins.python.api.tree.Tree;
import org.sonar.plugins.python.api.tree.UnaryExpression;
import org.sonar.plugins.python.api.types.BuiltinTypes;
import org.sonar.python.semantic.v2.TypeTable;
import org.sonar.python.tree.BinaryExpressionImpl;
import org.sonar.python.tree.UnaryExpressionImpl;
import org.sonar.python.types.v2.ClassType;
import org.sonar.python.types.v2.ObjectType;
import org.sonar.python.types.v2.PythonType;
import org.sonar.python.types.v2.TriBool;
import org.sonar.python.types.v2.TypeCheckBuilder;
import org.sonar.python.types.v2.TypeSource;
import org.sonar.python.types.v2.TypeUtils;
import org.sonar.python.types.v2.UnionType;

public class TrivialTypePropagationVisitor extends BaseTreeVisitor {
private final TypeCheckBuilder isBooleanTypeCheck;
Expand Down Expand Up @@ -57,6 +63,33 @@ public void visitUnaryExpression(UnaryExpression unaryExpr) {
}
}

@Override
public void visitBinaryExpression(BinaryExpression binaryExpression) {
super.visitBinaryExpression(binaryExpression);
if (binaryExpression instanceof BinaryExpressionImpl binaryExpressionImpl) {
var type = calculateBinaryExpressionType(binaryExpression);
binaryExpressionImpl.typeV2(type);
}
}

private static PythonType calculateBinaryExpressionType(BinaryExpression binaryExpression) {
var kind = binaryExpression.getKind();
var leftOperand = binaryExpression.leftOperand();
var rightOperand = binaryExpression.rightOperand();
if (binaryExpression.is(Tree.Kind.AND, Tree.Kind.OR)) {
return UnionType.or(leftOperand.typeV2(), rightOperand.typeV2());
}
if (TypeDependenciesCalculator.SAME_TYPE_PRODUCING_BINARY_EXPRESSION_KINDS.contains(kind)
&& leftOperand.typeV2() instanceof ObjectType leftObjectType
&& leftObjectType.unwrappedType() instanceof ClassType leftClassType
&& rightOperand.typeV2() instanceof ObjectType rightObjectType
&& rightObjectType.unwrappedType() instanceof ClassType rightClassType
&& leftClassType == rightClassType) {
return new ObjectType(leftClassType, TypeSource.min(leftObjectType.typeSource(), rightObjectType.typeSource()));
}
return PythonType.UNKNOWN;
}

private PythonType calculateUnaryExprType(UnaryExpression unaryExpr) {
String operator = unaryExpr.operator().value();
return TypeUtils.map(unaryExpr.expression().typeV2(), type -> mapUnaryExprType(operator, type));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* SonarQube Python Plugin
* Copyright (C) 2011-2024 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 Sonar Source-Available License Version 1, as published by SonarSource SA.
*
* 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 Sonar Source-Available License for more details.
*
* You should have received a copy of the Sonar Source-Available License
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/
package org.sonar.python.semantic.v2.types;

import java.util.EnumSet;
import java.util.List;
import java.util.Set;
import org.sonar.plugins.python.api.tree.BinaryExpression;
import org.sonar.plugins.python.api.tree.Expression;
import org.sonar.plugins.python.api.tree.Tree;
import org.sonar.python.types.HasTypeDependencies;

public class TypeDependenciesCalculator {
static final Set<Tree.Kind> SAME_TYPE_PRODUCING_BINARY_EXPRESSION_KINDS = EnumSet.of(
Tree.Kind.PLUS,
Tree.Kind.MINUS,
Tree.Kind.MULTIPLICATION,
Tree.Kind.DIVISION,
Tree.Kind.FLOOR_DIVISION,
Tree.Kind.MODULO,
Tree.Kind.POWER
);

public boolean hasTypeDependencies(Expression expression) {
return expression instanceof HasTypeDependencies;
}

public List<Expression> getTypeDependencies(Expression expression) {
if (expression instanceof BinaryExpression binaryExpression) {
return calculateBinaryExpressionTypeDependencies(binaryExpression);
} else if (expression instanceof HasTypeDependencies hasTypeDependencies) {
// SONARPY-2417 Once we get rid of v1 type inference -
// we won’t need a HasTypeDependencies interface implemented by a tree model classes.
// The implementation of the logic is still needed for v2 type inference
// but to keep tree model decoupled from the type inference and type model -
// the logic should be moved here as it is done for BinaryExpression
return hasTypeDependencies.typeDependencies();
}
return List.of();
}

private static List<Expression> calculateBinaryExpressionTypeDependencies(BinaryExpression binaryExpression) {
if (SAME_TYPE_PRODUCING_BINARY_EXPRESSION_KINDS.contains(binaryExpression.getKind())
|| binaryExpression.is(Tree.Kind.AND, Tree.Kind.OR)) {
return List.of(binaryExpression.leftOperand(), binaryExpression.rightOperand());
}
return List.of();
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -31,11 +30,7 @@
import org.sonar.plugins.python.api.types.InferredType;
import org.sonar.python.types.HasTypeDependencies;
import org.sonar.python.types.InferredTypes;
import org.sonar.python.types.v2.ClassType;
import org.sonar.python.types.v2.ObjectType;
import org.sonar.python.types.v2.PythonType;
import org.sonar.python.types.v2.TypeSource;
import org.sonar.python.types.v2.UnionType;

import static org.sonar.python.types.InferredTypes.DECL_INT;
import static org.sonar.python.types.InferredTypes.DECL_STR;
Expand All @@ -44,39 +39,38 @@

public class BinaryExpressionImpl extends PyTree implements BinaryExpression, HasTypeDependencies {

private static final Map<String, Kind> KINDS_BY_OPERATOR = kindsByOperator();
private static final Map<String, Kind> KINDS_BY_OPERATOR = Map.ofEntries(
Map.entry("+", Kind.PLUS),
Map.entry("-", Kind.MINUS),
Map.entry("*", Kind.MULTIPLICATION),
Map.entry("/", Kind.DIVISION),
Map.entry("//", Kind.FLOOR_DIVISION),
Map.entry("%", Kind.MODULO),
Map.entry("**", Kind.POWER),
Map.entry("@", Kind.MATRIX_MULTIPLICATION),
Map.entry(">>", Kind.SHIFT_EXPR),
Map.entry("<<", Kind.SHIFT_EXPR),
Map.entry("&", Kind.BITWISE_AND),
Map.entry("|", Kind.BITWISE_OR),
Map.entry("^", Kind.BITWISE_XOR),
Map.entry("and", Kind.AND),
Map.entry("or", Kind.OR),
Map.entry("==", Kind.COMPARISON),
Map.entry("<=", Kind.COMPARISON),
Map.entry(">=", Kind.COMPARISON),
Map.entry("<", Kind.COMPARISON),
Map.entry(">", Kind.COMPARISON),
Map.entry("!=", Kind.COMPARISON),
Map.entry("<>", Kind.COMPARISON),
Map.entry("in", Kind.IN),
Map.entry("is", Kind.IS)
);

private final Kind kind;
private final Expression leftOperand;
private final Token operator;
private final Expression rightOperand;

private static Map<String, Kind> kindsByOperator() {
Map<String, Kind> map = new HashMap<>();
map.put("+", Kind.PLUS);
map.put("-", Kind.MINUS);
map.put("*", Kind.MULTIPLICATION);
map.put("/", Kind.DIVISION);
map.put("//", Kind.FLOOR_DIVISION);
map.put("%", Kind.MODULO);
map.put("**", Kind.POWER);
map.put("@", Kind.MATRIX_MULTIPLICATION);
map.put(">>", Kind.SHIFT_EXPR);
map.put("<<", Kind.SHIFT_EXPR);
map.put("&", Kind.BITWISE_AND);
map.put("|", Kind.BITWISE_OR);
map.put("^", Kind.BITWISE_XOR);
map.put("and", Kind.AND);
map.put("or", Kind.OR);
map.put("==", Kind.COMPARISON);
map.put("<=", Kind.COMPARISON);
map.put(">=", Kind.COMPARISON);
map.put("<", Kind.COMPARISON);
map.put(">", Kind.COMPARISON);
map.put("!=", Kind.COMPARISON);
map.put("<>", Kind.COMPARISON);
return map;
}
private PythonType type = PythonType.UNKNOWN;

public BinaryExpressionImpl(Expression leftOperand, Token operator, Expression rightOperand) {
this.kind = KINDS_BY_OPERATOR.get(operator.value());
Expand Down Expand Up @@ -141,18 +135,12 @@ public InferredType type() {

@Override
public PythonType typeV2() {
if (is(Kind.AND, Kind.OR)) {
return UnionType.or(leftOperand.typeV2(), rightOperand.typeV2());
}
if (is(Kind.PLUS)
&& leftOperand.typeV2() instanceof ObjectType leftObjectType
&& leftObjectType.unwrappedType() instanceof ClassType leftClassType
&& rightOperand.typeV2() instanceof ObjectType rightObjectType
&& rightObjectType.unwrappedType() instanceof ClassType rightClassType
&& leftClassType == rightClassType) {
return new ObjectType(leftClassType, TypeSource.min(leftObjectType.typeSource(), rightObjectType.typeSource()));
}
return PythonType.UNKNOWN;
return type;
}

public BinaryExpressionImpl typeV2(PythonType type) {
this.type = type;
return this;
}

private static boolean shouldReturnDeclaredStr(InferredType leftType, InferredType rightType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3075,6 +3075,59 @@ def foo(x: int):
assertThat(yType.typeSource()).isSameAs(TypeSource.TYPE_HINT);
}

public static Stream<Arguments> binaryExpressionsSource() {
return Stream.of(
Arguments.of("""
1 + 2
""", new ObjectType(INT_TYPE)),
Arguments.of("""
1 - 2
""", new ObjectType(INT_TYPE)),
Arguments.of("""
1 * 2
""", new ObjectType(INT_TYPE)),
Arguments.of("""
1 / 2
""", new ObjectType(INT_TYPE)),
Arguments.of("""
'1' + '2'
""", new ObjectType(STR_TYPE)),
Arguments.of("""
1 + '2'
""", PythonType.UNKNOWN),
Arguments.of("""
1 + 2.0
""", PythonType.UNKNOWN),
Arguments.of("""
a = 1
b = 2
a + b
""", new ObjectType(INT_TYPE)),
Arguments.of("""
a = 1
b = 2
c = a - b
c
""", new ObjectType(INT_TYPE)),
Arguments.of("""
try:
...
except:
...
a = 1
b = 2
c = a - b
c
""", new ObjectType(INT_TYPE))
);
}

@ParameterizedTest
@MethodSource("binaryExpressionsSource")
void binaryExpressionTest(String code, PythonType expectedType) {
assertThat(lastExpression(code).typeV2()).isEqualTo(expectedType);
}

@Test
void assignmentPlusTest() {
var fileInput = inferTypes("""
Expand Down Expand Up @@ -3165,7 +3218,7 @@ def foo():
.orElseGet(List::of);

var fType = ((ExpressionStatement) statements.get(statements.size() - 1)).expressions().get(0).typeV2();
assertThat(fType).isSameAs(PythonType.UNKNOWN);
assertThat(fType).isInstanceOf(ObjectType.class).extracting(PythonType::unwrappedType).isSameAs(INT_TYPE);
}

@Test
Expand Down
Loading

0 comments on commit 5a9201b

Please sign in to comment.