Linting with ruff

[Snooz82]

Hi @pekkaklarck

I have taken ruff and let it run over the Robot Framework code (src folder)
It had found a huge amount, of things, but not all of them are relevant, because they are more naming decisions.
Others may be real issues.
Others just New vs Old technique. (like pathlib.Path vs os.path)
Others are considered better style...

I have applied some of the rules and excluded others.
I have a pack of rules i would consider worth fixing, but want to talk to you first.

• PTH100 os.path.abspath() should be replaced by Path.resolve() (And their siblings)
• B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
• UP031 Use format specifiers instead of percent format
• DTZ001 The use of datetime.datetime() without tzinfo argument is not allowed

Maybe you can find this valuable.

[pekkaklarck]

First of all, I'm not entirely sure is using an automatic formatter a good idea in general, because the code they produce isn't always that great. A formatter has a lot of benefits in environment where there are different people working in the same code base, though. Benefits are probably even bigger with an open source project where communicating project conventions to contributors is challenging.

I'm personally not too big fan of Black because it has so little configuration options. I find it annoying that projects are forced to same style even when their original style would be reasonable. Even bigger problem is that Black rules change now and then, and the lack of configuration options means that code changes as well. I hope the ruff formatter gets some more configuration options and would that way suite better for us.

Anyway, this is something we can discuss but time for that discussion is after RF 7.0 is out. Due to the time and budget constraints I don't want to spend much time for this now. Due to the amount of changes I believe the best time to take a formatter into use, assuming we consider it useful, is when RF 8.0 development starts.

---

UPDATE: I now understand that you used ruff for linting, not for formatting. That's less invasive but I don't think we have time for that either now. As the example with not ... == ... vs ... != ... illustrates (see the comment below), these changes need to be carefully reviewed.

[pekkaklarck]

I was browsing through the changes and noticed this one. This change is not ok. == and != call different methods, __eq__ and __ne__, respectively, and thus there's a significant difference between ... != ... and not ... == .... It typically doesn't matter, but it matters in this particular context.

This is a rather minor thing, but shows that formatting changes must be reviewed very carefully to avoid accidental functional changes. We don't have time for that before RF 7.0 and, because we can miss something, I believe we'd be too late in RF 7.0 development cycle also otherwise.

[Snooz82]

@pekkaklarck

I think we have just found a bug in Robot Framework DotDict. 😉
This not a == b vs a != b was the reason the utest fail in the branch.
And i would consider this an actual bug in DotDict implementation?

from collections import OrderedDict
from robot.utils import DotDict

# Normal dict: the order isn't relevant
norm_dict = {'a': 1, 'b': 2, 'c': 3, 'd': 4, 'e': 5, 'f': 6, 'g': 7}
norm_dict_reverse = {'g': 7, 'f': 6, 'e': 5, 'd': 4, 'c': 3, 'b': 2, 'a': 1}

norm_dict == norm_dict_reverse    # True
norm_dict != norm_dict_reverse    # False
not norm_dict == norm_dict_reverse    # False
not norm_dict != norm_dict_reverse    # True

# OrderedDict: the order is important
order_dict = OrderedDict([('a', 1), ('b', 2), ('c', 3), ('d', 4), ('e', 5), ('f', 6), ('g', 7)])
order_dict_reverse = OrderedDict([('g', 7), ('f', 6), ('e', 5), ('d', 4), ('c', 3), ('b', 2), ('a', 1)])

order_dict == order_dict_reverse    # False
order_dict != order_dict_reverse    # True
not order_dict == order_dict_reverse    # True
not order_dict != order_dict_reverse    # False

#DotDict : the order isn't relevant with == but is relevant with !=
dot_dict = DotDict({'a': 1, 'b': 2, 'c': 3, 'd': 4, 'e': 5, 'f': 6, 'g': 7})
dot_dict_reverse = DotDict({'g': 7, 'f': 6, 'e': 5, 'd': 4, 'c': 3, 'b': 2, 'a': 1})
dot_dict == dot_dict_reverse    # True because order is not relevant with ==
dot_dict != dot_dict_reverse    # True because order is relevant with !=
not dot_dict == dot_dict_reverse    # False
not dot_dict != dot_dict_reverse    # False