Sonarlint python:S1940 rule does not seem to take Set logic into account


I feel there is a bug in SonarLint’s Python rule python:S1940 in that it gives semantic changing advice in some case when the variables are sets.

As an example, consider:

from typting import Set

def validate(a: Set[str], b: Set[str]) -> None:
    if not (a <= b):
       raise ValueError("a <= b")

In this case, SonarLint will suggest via python:s1940 that I should simplify my logic to if a > b:. However, applying this rewrite would change the semantics as what we are testing here is whether a is a subset of b (i.e., all the values in a are inside the set b). The rewritten on the other hand tests something “entirely different” (namely that b is not a superset of a). This is problematic when b is used as an “approval list”.

I think python:S1940 should account for this case when the variables are “clearly” defined as sets and not suggest this particular rewrite (the not (a == b) variant is still fine for sets AFAICT).

A minimal reproducer where this rewrite goes wrong:

>>> from typing import Set
>>> ingredients: Set[str] = {'beef', 'potatos'}
>>> consumable_by_vegans: Set[str] = {'nut', 'apple', 'potatos'}
>>> if not (ingredients <= consumable_by_vegans):
...     raise ValueError("Sorry, the selected ingredients are not vegan")
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ValueError: Sorry, the selected ingredients are not vegan
>>> if ingredients > consumable_by_vegans:
...     raise ValueError("Sorry, the selected ingredients are not vegan")
>>> # Oops, no exception! :-/

Thanks for considering.

Product info:

  • Operating system: Linux
  • SonarLint plugin version:
  • Programming language you’re coding in: Python
  • Is connected mode used: No

Hi Niels,

Welcome to our community, thanks for reporting the issue, and sorry for the late answer.
Yes, indeed it’s a False Positive. Our current implementation doesn’t take defined type into consideration.
I created a ticket for that: [SONARPY-1251] - Jira and we will plan to implement it soon.

Marcin Stachniuk

1 Like