Repository: boxed/mutmut

Issue:
Whitelist rsplit
`str.rsplit` behaves exactly like `str.split`, if maxsplit is not provided, so in that case there should be no mutation.

Hints from issue comments:
@fabi321: Upon looking mutations, I also fund another false positive: I have assert False in my code, in order to cover branches that are not intended to be hit. Changing them to true doesn't make much sense.
It also tried to mutate the string of the assert, which also does not make too much sense.

In general, I don't feel like it's that useful to test for mutations in asserts, as they are mostly a defensive measure (they can be disabled, after all).@Otto-AA: > str.rsplit behaves exactly like str.split, if maxsplit is not provided, so in that case there should be no mutation.

Yes, without the maxsplit argument this mutation cannot be detected by the tests. We should change not mutate in this case. If you want to take a shot at this, I would suggest to add another `if m.matches(...)` to the [operator_string_methods_swap](https://github.com/boxed/mutmut/blob/b124c6ad46b0a9a048b5e47c8b7f93d33a9f2e6a/mutmut/node_mutation.py#L118) method (outside of the loop) and remove [this](https://github.com/boxed/mutmut/blob/b124c6ad46b0a9a048b5e47c8b7f93d33a9f2e6a/mutmut/node_mutation.py#L110C10-L110C30) dict entry.

> Upon looking mutations, I also fund another false positive: I have assert False in my code, in order to cover branches that are not intended to be hit. Changing them to true doesn't make much sense.
It also tried to mutate the string of the assert, which also does not make too much sense.

I also do not think mutating `assert false` is useful. We could change this, but I think the change would not be trivial. Currently most (or all?) mutation operators do not rely on the context, so the mutation of `False` to `True` currently does not know if it is the operand of an `assert` statement. We could considert to add the [parent metadata](https://libcst.readthedocs.io/en/latest/metadata.html#parent-node-metadata) as additional context for operators.

So I'm not sure if this would be worth the effort. The mutation for `assert False` should only show up as a mutation that was not executed (similar to uncovered code with e.g. coverage.py).

> In general, I don't feel like it's that useful to test for mutations in asserts, as they are mostly a defensive measure (they can be disabled, after all).

I guess people could use `assert` statements for quick and dirty input validation, for instance:

```python
def save_user(username, age):
  assert 0 <= age <= 150, 'Please provide an age between 0 and 150'
```

While this is not best practice, I could imagine code like this. For these cases, mutating `assert` statements seems reasonable to me. If this is really uncommon, we could consider not mutating `assert` statements, which would be rather simple to implement in mutmut.@fabi321: > So I'm not sure if this would be worth the effort. The mutation for `assert False` should only show up as a mutation that was not executed (similar to uncovered code with e.g. coverage.py).

That assertion comes up as fail. I noticed it due to that.@Otto-AA: Maybe I misunderstood something, do you have a reproducible example?

My thinking is, that if the `assert False` is on unreachable code, then the mutation (`assert True`) should never be executed and thus reported as "no tests" (with the 🫥 emoji). And if it is reachable and executed in the tests, it probably makes sense to mutate it to `assert True` and check if a test is influenced by this.@fabi321: I don't have any minimal example, but mutmut marks https://github.com/stormworks-utils/tumfl/blob/main/tumfl/parser.py#L490 as fail, and coverage reports that that line is never hit.

(Or rather, it would mark it, twice (due to the string mut), if it didn't have the `no mut`)@CHB-0r1s: @fabi321 @Otto-AA Hi! Can you explain your point: split = rsplit without args, so we need check here?

As string mutations was my add-on, I try to help with fix it, if it may be helpful@Otto-AA: The split issue is fixed with #394 .

For the issue regarding `assert` statements, the problem is that we only track coverage per function rather than per line. I have created a separate issue: #395 

Solution patch:
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index 1dc1a04..fdcba4c 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -28,11 +28,23 @@ Running your local version of Mutmut against a test codebase
 
 You can install your local version of Mutmut and run it, including any changes you have made, as normal.
 
+Codebases using pip
+^^^^^^^^^^^^^^^^^^^
+
 .. code-block:: console
 
     python -m pip install --editable <path_to_mutmut_codebase>
 
+Codebases using Poetry
+^^^^^^^^^^^^^^^^^^^^^^
+
+.. code-block:: console
+
+    poetry add --group dev --editable <path_to_mutmut_codebase>
+    # Install dependecies in your Poetry environment
+    pip install -r <path_to_mutmut_codebase>/requirements.txt
+
 Documentation about mutmut's architecture
 -----------------------------------------
 
-Please see ARCHITECTURE.rst for more details on how mutmut works.
\ No newline at end of file
+Please see ARCHITECTURE.rst for more details on how mutmut works.
diff --git a/mutmut/node_mutation.py b/mutmut/node_mutation.py
index 806c42a..d498c1b 100644
--- a/mutmut/node_mutation.py
+++ b/mutmut/node_mutation.py
@@ -96,7 +96,7 @@ def operator_arg_removal(
             yield node.with_changes(args=[*node.args[:i], *node.args[i + 1 :]])
 
 
-supported_str_methods_swap = [
+supported_symmetric_str_methods_swap = [
          ("lower", "upper"),
          ("upper", "lower"),
          ("lstrip", "rstrip"),
@@ -107,24 +107,42 @@ supported_str_methods_swap = [
          ("rjust", "ljust"),
          ("index", "rindex"),
          ("rindex", "index"),
-         ("split", "rsplit"),
-         ("rsplit", "split"),
          ("removeprefix", "removesuffix"),
          ("removesuffix", "removeprefix"),
          ("partition", "rpartition"),
          ("rpartition", "partition")
-     ]
+]
+
+supported_unsymmetrical_str_methods_swap = [
+    ("split", "rsplit"),
+    ("rsplit", "split")
+]
 
-def operator_string_methods_swap(
+def operator_symmetric_string_methods_swap(
      node: cst.Call
  ) -> Iterable[cst.Call]:
      """try to swap string method to opposite e.g. a.lower() -> a.upper()"""
 
-     for old_call, new_call in supported_str_methods_swap:
+     for old_call, new_call in supported_symmetric_str_methods_swap:
          if m.matches(node.func, m.Attribute(value=m.DoNotCare(),  attr=m.Name(value=old_call))):
             func_name = cst.ensure_type(node.func, cst.Attribute).attr
             yield node.with_deep_changes(func_name, value=new_call)
 
+def operator_unsymmetrical_string_methods_swap(
+    node: cst.Call
+) -> Iterable[cst.Call]:
+    """Try to handle specific mutations of string, which useful only in specific args combination."""
+    for old_call, new_call in supported_unsymmetrical_str_methods_swap:
+        if m.matches(node.func, m.Attribute(attr=m.Name(value=old_call))):
+            if old_call in {"split", "rsplit"}:
+                # The logic of this "if" operator described here:
+                # https://github.com/boxed/mutmut/pull/394#issuecomment-2977890188
+                key_args: set[str] = {a.keyword.value for a in node.args if a.keyword} # sep or maxsplit or nothing
+                if len(node.args) == 2 or "maxsplit" in key_args:
+                    func_name = cst.ensure_type(node.func, cst.Attribute).attr
+                    yield node.with_deep_changes(func_name, value=new_call)
+
+
 
 def operator_remove_unary_ops(
     node: cst.UnaryOperation
@@ -238,7 +256,8 @@ mutation_operators: OPERATORS_TYPE = [
     (cst.UnaryOperation, operator_remove_unary_ops),
     (cst.Call, operator_dict_arguments),
     (cst.Call, operator_arg_removal),
-    (cst.Call, operator_string_methods_swap),
+    (cst.Call, operator_symmetric_string_methods_swap),
+    (cst.Call, operator_unsymmetrical_string_methods_swap),
     (cst.Lambda, operator_lambda),
     (cst.CSTNode, operator_keywords),
     (cst.CSTNode, operator_swap_op),


Test patch:
diff --git a/tests/test_mutation.py b/tests/test_mutation.py
index 1cb754c..3295e48 100644
--- a/tests/test_mutation.py
+++ b/tests/test_mutation.py
@@ -62,8 +62,47 @@ def mutated_module(source: str) -> str:
         ]),
         ('a.index("+")', ['a.rindex("+")', 'a.index("XX+XX")', 'a.index(None)']),
         ('a.rindex("+")', ['a.index("+")', 'a.rindex("XX+XX")', 'a.rindex(None)']),
-        ('a.split()', 'a.rsplit()'),
-        ('a.rsplit()', 'a.split()'),
+        ('a.split()', []),
+        ('a.rsplit()', []),
+        ('a.split(" ")', ['a.split("XX XX")', 'a.split(None)']),
+        ('a.rsplit(" ")', ['a.rsplit("XX XX")', 'a.rsplit(None)']),
+        ('a.split(sep="")', ['a.split(sep="XXXX")', 'a.split(sep=None)']),
+        ('a.rsplit(sep="")', ['a.rsplit(sep="XXXX")', 'a.rsplit(sep=None)']),
+        ('a.split(maxsplit=-1)', [
+            'a.rsplit(maxsplit=-1)', 'a.split(maxsplit=+1)', 'a.split(maxsplit=-2)', 'a.split(maxsplit=None)'
+        ]),
+        ('a.rsplit(maxsplit=-1)', [
+            'a.split(maxsplit=-1)', 'a.rsplit(maxsplit=+1)', 'a.rsplit(maxsplit=-2)', 'a.rsplit(maxsplit=None)'
+        ]),
+        ('a.split(" ", maxsplit=-1)', [
+            'a.split(" ", )', 'a.split(" ", maxsplit=+1)', 'a.split(" ", maxsplit=-2)',
+            'a.split(" ", maxsplit=None)', 'a.split("XX XX", maxsplit=-1)', 'a.split(None, maxsplit=-1)',
+            'a.split(maxsplit=-1)', 'a.rsplit(" ", maxsplit=-1)'
+        ]),
+        ('a.rsplit(" ", maxsplit=-1)', [
+            'a.rsplit(" ", )', 'a.rsplit(" ", maxsplit=+1)', 'a.rsplit(" ", maxsplit=-2)',
+            'a.rsplit(" ", maxsplit=None)', 'a.rsplit("XX XX", maxsplit=-1)', 'a.rsplit(None, maxsplit=-1)',
+            'a.rsplit(maxsplit=-1)', 'a.split(" ", maxsplit=-1)'
+        ]),
+        ('a.split(maxsplit=1)', ['a.split(maxsplit=2)', 'a.split(maxsplit=None)', 'a.rsplit(maxsplit=1)']),
+        ('a.rsplit(maxsplit=1)', ['a.rsplit(maxsplit=2)', 'a.rsplit(maxsplit=None)', 'a.split(maxsplit=1)']),
+        ('a.split(" ", 1)', [
+            'a.rsplit(" ", 1)', 'a.split(" ", )', 'a.split(" ", 2)', 'a.split(" ", None)',
+            'a.split("XX XX", 1)', 'a.split(1)', 'a.split(None, 1)'
+        ]),
+        ('a.rsplit(" ", 1)', [
+            'a.rsplit(" ", )', 'a.rsplit(" ", 2)', 'a.rsplit(" ", None)', 'a.rsplit("XX XX", 1)',
+            'a.rsplit(1)', 'a.rsplit(None, 1)', 'a.split(" ", 1)'
+        ]),
+        ('a.split(" ", maxsplit=1)', [
+            'a.rsplit(" ", maxsplit=1)', 'a.split(" ", )', 'a.split(" ", maxsplit=2)', 'a.split(" ", maxsplit=None)',
+            'a.split("XX XX", maxsplit=1)', 'a.split(None, maxsplit=1)', 'a.split(maxsplit=1)'
+        ]),
+        ('a.rsplit(" ", maxsplit=1)', [
+            'a.rsplit(" ", )', 'a.rsplit(" ", maxsplit=2)', 'a.rsplit(" ", maxsplit=None)',
+            'a.rsplit("XX XX", maxsplit=1)', 'a.rsplit(None, maxsplit=1)', 'a.rsplit(maxsplit=1)',
+            'a.split(" ", maxsplit=1)'
+        ]),
         ('a.removeprefix("+")', ['a.removesuffix("+")', 'a.removeprefix("XX+XX")', 'a.removeprefix(None)']),
         ('a.removesuffix("+")', ['a.removeprefix("+")', 'a.removesuffix("XX+XX")', 'a.removesuffix(None)']),
         ('a.partition("++")', ['a.rpartition("++")', 'a.partition("XX++XX")', 'a.partition(None)']),