Skip to content

Commit

Permalink
SONARPY-1609 Fix FP on S5886 when returning optional unions of unknow…
Browse files Browse the repository at this point in the history
…n symbols (#1707)
  • Loading branch information
guillaume-dequenne-sonarsource authored Feb 16, 2024
1 parent 783c308 commit 999884a
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 4 deletions.
18 changes: 18 additions & 0 deletions python-checks/src/test/resources/checks/functionReturnType.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,3 +334,21 @@ def echo_round() -> Generator[int, float, str]:
@contextlib.contextmanager
def test() -> contextlib.AbstractContextManager[str]:
yield "this is an example"


def no_fp_on_optional_of_unknown_symbol(cond) -> Optional[Unknown]:
if cond == 42:
return 10 # OK
if cond == 24:
return "hello" # OK
if cond == 5:
return None


def no_fp_on_optional_union_of_unknown_symbol(cond) -> Optional[Union[str, Unknown]]:
if cond == 42:
return 10 # OK
if cond == 24:
return "hello" # OK
if cond == 5:
return None
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,9 @@ private static DeclaredType declaredTypeFromTypeAnnotationSubscription(Subscript
.map(exp -> declaredTypeFromTypeAnnotation(exp, builtinSymbols))
.collect(Collectors.toList());
if (args.stream().anyMatch(Objects::isNull)) {
args = Collections.emptyList();
// null args indicate something was wrong in the resolution of some of the alternatives
// returning null here will ensure the resulting type will be AnyType, which will avoid potential FPs
return null;
}
String builtinFqn = ALIASED_ANNOTATIONS.get(symbol.fullyQualifiedName());
return builtinFqn != null ? new DeclaredType(builtinSymbols.get(builtinFqn), args) : new DeclaredType(symbol, args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,7 @@ void test_optional_type_annotations() {
);
assertThat(fromTypeshedTypeAnnotation(typeAnnotation)).isEqualTo(InferredTypes.anyType());
declaredType = fromTypeAnnotation(typeAnnotation);
assertThat(declaredType).isInstanceOf(DeclaredType.class);
assertThat(((DeclaredType) declaredType).alternativeTypeSymbols()).extracting(Symbol::fullyQualifiedName)
.containsExactlyInAnyOrder("typing.Optional");
assertThat(declaredType).isInstanceOf(AnyType.class);
}

@Test
Expand Down

0 comments on commit 999884a

Please sign in to comment.