Skip to content

Commit 4bd143a

Browse files
Seppli11Seppli11
authored
SONARPY-2235: UnionType creation should have more guards (#2210)
1 parent 5390bf6 commit 4bd143a

File tree

8 files changed

+109
-49
lines changed

8 files changed

+109
-49
lines changed

‎python-frontend/src/main/java/org/sonar/python/semantic/v2/converter/AmbiguousDescriptorToPythonTypeConverter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public class AmbiguousDescriptorToPythonTypeConverter implements DescriptorToPyt
2626

2727
public PythonType convert(ConversionContext ctx, AmbiguousDescriptor from) {
2828
var candidates = from.alternatives().stream().map(ctx::convert).collect(Collectors.toSet());
29-
return new UnionType(candidates);
29+
return UnionType.or(candidates);
3030
}
3131

3232
@Override

‎python-frontend/src/main/java/org/sonar/python/semantic/v2/types/ProgramStateTypeInferenceVisitor.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@
1818

1919
import java.util.Optional;
2020
import java.util.Set;
21-
import java.util.stream.Stream;
2221
import org.sonar.plugins.python.api.tree.Expression;
2322
import org.sonar.plugins.python.api.tree.FunctionDef;
2423
import org.sonar.plugins.python.api.tree.Name;
2524
import org.sonar.plugins.python.api.tree.QualifiedExpression;
2625
import org.sonar.python.semantic.v2.TypeTable;
2726
import org.sonar.python.tree.NameImpl;
2827
import org.sonar.python.types.v2.PythonType;
29-
import org.sonar.python.types.v2.UnionType;
28+
import org.sonar.python.types.v2.TypeUtils;
3029

3130
/**
3231
* Used in FlowSensitiveTypeInference to update name types based on program state
@@ -44,7 +43,7 @@ public void visitName(Name name) {
4443
Optional.ofNullable(name.symbolV2()).ifPresent(symbol -> {
4544
Set<PythonType> pythonTypes = state.getTypes(symbol);
4645
if (!pythonTypes.isEmpty()) {
47-
((NameImpl) name).typeV2(union(pythonTypes.stream()));
46+
((NameImpl) name).typeV2(union(pythonTypes));
4847
}
4948
});
5049
super.visitName(name);
@@ -66,7 +65,7 @@ public void visitQualifiedExpression(QualifiedExpression qualifiedExpression) {
6665
}
6766
}
6867

69-
private static PythonType union(Stream<PythonType> types) {
70-
return types.reduce(UnionType::or).orElse(PythonType.UNKNOWN);
68+
private static PythonType union(Set<PythonType> types) {
69+
return types.stream().collect(TypeUtils.toUnionType());
7170
}
72-
}
71+
}

‎python-frontend/src/main/java/org/sonar/python/types/v2/LazyUnionType.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121

2222
public class LazyUnionType implements PythonType, ResolvableType {
2323

24-
Set<PythonType> candidates;
24+
private final Set<PythonType> candidates = new HashSet<>();
2525

2626
public LazyUnionType(Set<PythonType> candidates) {
27-
this.candidates = candidates;
27+
this.candidates.addAll(candidates);
2828
}
2929

3030
public PythonType resolve() {
@@ -35,6 +35,6 @@ public PythonType resolve() {
3535
}
3636
resolvedCandidates.add(candidate);
3737
}
38-
return new UnionType(resolvedCandidates);
38+
return UnionType.or(resolvedCandidates);
3939
}
4040
}

‎python-frontend/src/main/java/org/sonar/python/types/v2/TypeUtils.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package org.sonar.python.types.v2;
1818

1919
import java.util.function.UnaryOperator;
20+
import java.util.stream.Collector;
21+
import java.util.stream.Collectors;
2022

2123
public class TypeUtils {
2224

@@ -40,9 +42,13 @@ public static PythonType ensureWrappedObjectType(PythonType pythonType) {
4042

4143
public static PythonType map(PythonType type, UnaryOperator<PythonType> mapper) {
4244
if (type instanceof UnionType unionType) {
43-
return unionType.candidates().stream().map(mapper).reduce(UnionType::or).orElse(PythonType.UNKNOWN);
45+
return unionType.candidates().stream().map(mapper).collect(toUnionType());
4446
} else {
4547
return mapper.apply(type);
4648
}
4749
}
50+
51+
public static Collector<PythonType, ?, PythonType> toUnionType() {
52+
return Collectors.collectingAndThen(Collectors.toSet(), UnionType::or);
53+
}
4854
}

‎python-frontend/src/main/java/org/sonar/python/types/v2/UnionType.java

Lines changed: 53 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,26 @@
2121
import java.util.Comparator;
2222
import java.util.HashSet;
2323
import java.util.List;
24+
import java.util.Objects;
2425
import java.util.Optional;
2526
import java.util.Set;
2627
import java.util.stream.Collectors;
28+
import java.util.stream.Stream;
2729
import javax.annotation.Nullable;
2830
import org.sonar.api.Beta;
2931

3032
@Beta
31-
public record UnionType(Set<PythonType> candidates) implements PythonType {
33+
public class UnionType implements PythonType {
34+
35+
private final Set<PythonType> candidates = new HashSet<>();
36+
37+
private UnionType(Set<PythonType> candidates) {
38+
this.candidates.addAll(candidates);
39+
}
40+
41+
public Set<PythonType> candidates() {
42+
return candidates;
43+
}
3244

3345
@Override
3446
public Optional<String> displayName() {
@@ -69,46 +81,56 @@ public TypeSource typeSource() {
6981
.orElse(TypeSource.EXACT);
7082
}
7183

72-
@Beta
73-
public static PythonType or(Collection<PythonType> candidates) {
74-
ensureCandidatesAreNotLazyTypes(candidates);
75-
if (candidates.isEmpty()) {
76-
return PythonType.UNKNOWN;
77-
}
78-
return candidates
79-
.stream()
80-
.reduce(new UnionType(new HashSet<>()), UnionType::or);
84+
@Override
85+
public boolean equals(Object o) {
86+
if (o == null || getClass() != o.getClass()) return false;
87+
UnionType unionType = (UnionType) o;
88+
return Objects.equals(candidates, unionType.candidates);
8189
}
8290

83-
@Beta
84-
public static PythonType or(@Nullable PythonType type1, @Nullable PythonType type2) {
85-
if (type1 == null) {
86-
return type2;
87-
}
88-
if (type2 == null) {
89-
return type1;
91+
@Override
92+
public int hashCode() {
93+
return Objects.hashCode(candidates);
94+
}
95+
96+
@Override
97+
public String toString() {
98+
return displayName().orElse(super.toString());
99+
}
100+
101+
public static PythonType or(@Nullable PythonType type1, @Nullable PythonType type2, @Nullable PythonType ...types) {
102+
if(types == null) {
103+
types = new PythonType[0];
90104
}
91-
if (type1 == PythonType.UNKNOWN || type2 == PythonType.UNKNOWN) {
105+
Set<PythonType> typeSet = new HashSet<>();
106+
typeSet.add(type1);
107+
typeSet.add(type2);
108+
typeSet.addAll(Set.of(types));
109+
return or(typeSet);
110+
}
111+
112+
public static PythonType or(Collection<PythonType> types) {
113+
types = types.stream().filter(Objects::nonNull).collect(Collectors.toSet());
114+
if(types.isEmpty()) {
92115
return PythonType.UNKNOWN;
93116
}
94-
if (type1.equals(type2)) {
95-
return type1;
96-
}
97-
Set<PythonType> types = new HashSet<>();
98-
addTypes(type1, types);
99-
addTypes(type2, types);
100-
if (types.size() == 1) {
117+
if(types.size() == 1) {
101118
return types.iterator().next();
102119
}
103-
ensureCandidatesAreNotLazyTypes(types);
104-
return new UnionType(types);
120+
121+
Set<PythonType> flatTypes = types.stream().flatMap(UnionType::flattenPythonType).collect(Collectors.toSet());
122+
if(flatTypes.stream().anyMatch(type -> type == PythonType.UNKNOWN)) {
123+
return PythonType.UNKNOWN;
124+
}
125+
ensureCandidatesAreNotLazyTypes(flatTypes);
126+
return new UnionType(flatTypes);
105127
}
106128

107-
private static void addTypes(PythonType type, Set<PythonType> types) {
108-
if (type instanceof UnionType unionType) {
109-
types.addAll(unionType.candidates());
129+
private static Stream<PythonType> flattenPythonType(PythonType type) {
130+
if(type instanceof UnionType unionType) {
131+
return unionType.candidates().stream();
110132
} else {
111-
types.add(type);
133+
return Stream.of(type);
112134
}
113135
}
114136

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,7 +2789,9 @@ void return_type_of_call_expression_inconsistent() {
27892789
CallExpression callExpressionSpy = Mockito.spy(callExpression);
27902790

27912791
// Inconsistent union type, should not happen
2792-
UnionType unionType = new UnionType(Set.of(PythonType.UNKNOWN));
2792+
UnionType unionType = Mockito.mock(UnionType.class);
2793+
Mockito.when(unionType.candidates()).thenReturn(Set.of(PythonType.UNKNOWN));
2794+
27932795
Name mock = Mockito.mock(Name.class);
27942796
Mockito.when(mock.typeV2()).thenReturn(unionType);
27952797
Mockito.doReturn(mock).when(callExpressionSpy).callee();
@@ -2808,7 +2810,9 @@ void return_type_of_call_expression_inconsistent_2() {
28082810
CallExpression callExpressionSpy = Mockito.spy(callExpression);
28092811

28102812
// Inconsistent union type, should not happen
2811-
UnionType unionType = new UnionType(Set.of());
2813+
UnionType unionType = Mockito.mock(UnionType.class);
2814+
Mockito.when(unionType.candidates()).thenReturn(Set.of());
2815+
28122816
Name mock = Mockito.mock(Name.class);
28132817
Mockito.when(mock.typeV2()).thenReturn(unionType);
28142818
Mockito.doReturn(mock).when(callExpressionSpy).callee();
@@ -2827,7 +2831,9 @@ void return_type_of_call_expression_inconsistent_3() {
28272831
CallExpression callExpressionSpy = Mockito.spy(callExpression);
28282832

28292833
// Inconsistent union type, should not happen
2830-
UnionType unionType = new UnionType(Set.of(INT_TYPE));
2834+
UnionType unionType = Mockito.mock(UnionType.class);
2835+
Mockito.when(unionType.candidates()).thenReturn(Set.of(INT_TYPE));
2836+
28312837
Name mock = Mockito.mock(Name.class);
28322838
Mockito.when(mock.typeV2()).thenReturn(unionType);
28332839
Mockito.doReturn(mock).when(callExpressionSpy).callee();

‎python-frontend/src/test/java/org/sonar/python/semantic/v2/converter/PythonTypeToDescriptorConverterTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ void testConvertOtherType() {
187187
void testConvertUnionType() {
188188
ClassType classType = new ClassType("classType", "my_package.classType", Set.of(new Member("aMember", intTypeWrapper.type())), List.of(), List.of(floatTypeWrapper), List.of(intTypeWrapper.type()), true, false, location);
189189
ClassType anotherClassType = new ClassType("classType", "my_package.classType", Set.of(new Member("aMember", intTypeWrapper.type())), List.of(), List.of(floatTypeWrapper), List.of(intTypeWrapper.type()), true, false, location);
190-
UnionType unionType = new UnionType(Set.of(classType, anotherClassType));
190+
PythonType unionType = UnionType.or(classType, anotherClassType);
191191
Descriptor descriptor = converter.convert("foo", new SymbolV2("myUnionType"), Set.of(unionType));
192192

193193
assertThat(descriptor).isInstanceOf(AmbiguousDescriptor.class);
@@ -221,7 +221,7 @@ void testConvertManyTypesWithUnionType() {
221221
ClassType classType = new ClassType("classType", "my_package.classType", Set.of(new Member("aMember", intTypeWrapper.type())), List.of(), List.of(floatTypeWrapper), List.of(intTypeWrapper.type()), true, false, location);
222222
ClassType anotherClassType = new ClassType("classType", "my_package.classType", Set.of(new Member("aMember", intTypeWrapper.type())), List.of(), List.of(floatTypeWrapper), List.of(intTypeWrapper.type()), true, false, location);
223223

224-
UnionType unionType = new UnionType(Set.of(classType, anotherClassType));
224+
PythonType unionType = UnionType.or(classType, anotherClassType);
225225
Descriptor descriptor = converter.convert("foo", new SymbolV2("myUnionType"), Set.of(unionType, classType));
226226

227227
assertThat(descriptor).isInstanceOf(AmbiguousDescriptor.class);

‎python-frontend/src/test/java/org/sonar/python/types/v2/UnionTypeTest.java

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.sonar.python.types.v2;
1818

1919
import java.util.ArrayList;
20+
import java.util.Collections;
2021
import java.util.List;
2122
import java.util.Set;
2223
import org.assertj.core.api.InstanceOfAssertFactories;
@@ -31,6 +32,7 @@
3132
import static org.assertj.core.api.Assertions.assertThat;
3233
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3334
import static org.sonar.python.types.v2.TypesTestUtils.BOOL_TYPE;
35+
import static org.sonar.python.types.v2.TypesTestUtils.FLOAT_TYPE;
3436
import static org.sonar.python.types.v2.TypesTestUtils.INT_TYPE;
3537
import static org.sonar.python.types.v2.TypesTestUtils.STR_TYPE;
3638
import static org.sonar.python.types.v2.TypesTestUtils.parseAndInferTypes;
@@ -44,7 +46,7 @@ void basicUnion() {
4446
PythonType intType = ((ExpressionStatement) fileInput.statements().statements().get(0)).expressions().get(0).typeV2();
4547
PythonType strType = ((ExpressionStatement) fileInput.statements().statements().get(1)).expressions().get(0).typeV2();
4648

47-
UnionType unionType = new UnionType(Set.of(intType, strType));
49+
PythonType unionType = UnionType.or(intType, strType);
4850

4951
assertThat(unionType.isCompatibleWith(intType)).isTrue();
5052
assertThat(unionType.isCompatibleWith(strType)).isTrue();
@@ -60,7 +62,7 @@ void unionWithUnknown() {
6062
FileInput fileInput = parseAndInferTypes("42;foo()");
6163
PythonType intType = ((ExpressionStatement) fileInput.statements().statements().get(0)).expressions().get(0).typeV2();
6264
PythonType strType = ((ExpressionStatement) fileInput.statements().statements().get(1)).expressions().get(0).typeV2();
63-
UnionType unionType = new UnionType(Set.of(intType, strType));
65+
PythonType unionType = UnionType.or(intType, strType);
6466

6567
assertThat(unionType.displayName()).isEmpty();
6668
assertThat(unionType.instanceDisplayName()).isEmpty();
@@ -85,6 +87,9 @@ void or_with_null() {
8587
assertThat(type).isEqualTo(INT_TYPE);
8688
type = UnionType.or(null, INT_TYPE);
8789
assertThat(type).isEqualTo(INT_TYPE);
90+
91+
type = UnionType.or(null, INT_TYPE, (PythonType[]) null);
92+
assertThat(type).isEqualTo(INT_TYPE);
8893
}
8994

9095
@Test
@@ -115,6 +120,19 @@ void or_unresolevdImportType() {
115120
assertThat(((UnionType) unionType).candidates()).containsExactlyInAnyOrder(unresolvedImportType, unresolvedImportType2);
116121
}
117122

123+
@Test
124+
void or_emptySet() {
125+
assertThat(UnionType.or(Collections.emptyList())).isEqualTo(PythonType.UNKNOWN);
126+
}
127+
128+
@Test
129+
void or_singletonSet() {
130+
assertThat(UnionType.or(Set.of(INT_TYPE))).isSameAs(INT_TYPE);
131+
132+
var union = UnionType.or(INT_TYPE, FLOAT_TYPE);
133+
assertThat(UnionType.or(Set.of(union))).isSameAs(union);
134+
}
135+
118136
@Test
119137
void hasMemberUnionType() {
120138
FileInput fileInput = parseAndInferTypes("""
@@ -167,4 +185,13 @@ void noLazyTypeInUnionType() {
167185
.isInstanceOf(IllegalArgumentException.class)
168186
.hasMessage("UnionType cannot contain Lazy types");
169187
}
188+
189+
@Test
190+
void testEquality() {
191+
var union1 = UnionType.or(INT_TYPE, FLOAT_TYPE);
192+
var union2 = UnionType.or(INT_TYPE, FLOAT_TYPE);
193+
assertThat(union1)
194+
.isEqualTo(union2)
195+
.hasSameHashCodeAs(union2);
196+
}
170197
}

0 commit comments

Comments
 (0)