improve no-cheat CI check (#2127)

## Changes
improve no-cheat CI check

Currently we get blocked when we change a line of code that already
contains a cheat.
For example, changing from:
`some_function("abc") # pylint: disable=some-code`
to:
`some_function("xyz") # pylint: disable=some-code`
triggers a no-cheat erroneously.
We want to avoid that, and also get better feedback on the new cheats.

The current script diffs the incoming branch with main, not the target
branch.
This PR changes that by using `$GITHUB_BASE_REF`.

The logic is too complex for bash, so delegating the analysis to python.

### Linked issues
None

### Functionality
None

### Tests
- [x] manually tested

---------

Co-authored-by: Eric Vergnaud <[email protected]>

Author: ericvergnaud

.github/workflows/no-cheat.yml

@@ -20,11 +20,13 @@ jobs:
         with:
           fetch-depth: 0
 
-      - name: Verify no lint is disabled in the new code
+      - name: Verify no additional disabled lint in the new code
         run: |
-          NEW_CODE=$(git diff origin/main..$(git branch --show-current) | grep -e '^+')
-          CHEAT=$(echo "${NEW_CODE}" | grep '# pylint: disable' | grep -v "CHEAT" | wc -c)
-          if [ "${CHEAT}" -ne 0 ]; then
-            echo "Do not cheat the linter: ${CHEAT}"
+          git fetch origin $GITHUB_BASE_REF:$GITHUB_BASE_REF
+          git diff $GITHUB_BASE_REF...$(git branch --show-current) >> diff_data.txt
+          python tests/unit/no_cheat.py diff_data.txt >> cheats.txt
+          COUNT=$(cat cheats.txt | wc -c)
+          if [ ${COUNT} -gt 1 ]; then
+            cat cheats.txt
             exit 1
           fi

tests/unit/no_cheat.py

@@ -0,0 +1,37 @@
+import sys
+from pathlib import Path
+
+DISABLE_TAG = '# pylint: disable='
+
+
+def no_cheat(diff_text: str) -> str:
+    lines = diff_text.split('\n')
+    removed: dict[str, int] = {}
+    added: dict[str, int] = {}
+    for line in lines:
+        if not (line.startswith("-") or line.startswith("+")):
+            continue
+        idx = line.find(DISABLE_TAG)
+        if idx < 0:
+            continue
+        codes = line[idx + len(DISABLE_TAG) :].split(',')
+        for code in codes:
+            code = code.strip().strip('\n').strip('"').strip("'")
+            if line.startswith("-"):
+                removed[code] = removed.get(code, 0) + 1
+                continue
+            added[code] = added.get(code, 0) + 1
+    results: list[str] = []
+    for code, count in added.items():
+        count -= removed.get(code, 0)
+        if count > 0:
+            results.append(f"Do not cheat the linter: found {count} additional {DISABLE_TAG}{code}")
+    return '\n'.join(results)
+
+
+if __name__ == "__main__":
+    diff_data = sys.argv[1]
+    path = Path(diff_data)
+    if path.exists():
+        diff_data = path.read_text("utf-8")
+    print(no_cheat(diff_data))

tests/unit/test_no_cheat.py

@@ -0,0 +1,78 @@
+from tests.unit.no_cheat import no_cheat
+
+
+def test_no_cheat_returns_empty_string_for_empty_diff():
+    diff_data = ""
+    result = no_cheat(diff_data)
+    assert not result
+
+
+def test_no_cheat_returns_empty_string_for_no_cheat_diff():
+    diff_data = """
++some code
+-some other code
+"""
+    result = no_cheat(diff_data)
+    assert not result
+
+
+def test_no_cheat_returns_empty_string_for_removed_cheat():
+    diff_data = """
++some code
+-some other code # pylint: disable=some-rule
+"""
+    result = no_cheat(diff_data)
+    assert not result
+
+
+def test_no_cheat_returns_empty_string_for_replaced_cheat():
+    diff_data = """
++some code # pylint: disable=some-rule
+-some other code # pylint: disable=some-rule
+"""
+    result = no_cheat(diff_data)
+    assert not result
+
+
+def test_no_cheat_returns_message_for_single_cheat():
+    diff_data = """
++some code # pylint: disable=some-rule
+-some other code
+"""
+    result = no_cheat(diff_data)
+    assert result == "Do not cheat the linter: found 1 additional # pylint: disable=some-rule"
+
+
+def test_no_cheat_returns_message_for_multiple_same_cheat():
+    diff_data = """
++some code # pylint: disable=some-rule
+-some other code
++some code # pylint: disable=some-rule
+"""
+    result = no_cheat(diff_data)
+    assert result == "Do not cheat the linter: found 2 additional # pylint: disable=some-rule"
+
+
+def test_no_cheat_returns_message_for_multiple_cheats_in_different_lines():
+    diff_data = """
++some code # pylint: disable=some-rule
+-some other code
++some code # pylint: disable=some-other-rule
+"""
+    result = no_cheat(diff_data)
+    assert result == (
+        "Do not cheat the linter: found 1 additional # pylint: disable=some-rule\n"
+        "Do not cheat the linter: found 1 additional # pylint: disable=some-other-rule"
+    )
+
+
+def test_no_cheat_returns_message_for_multiple_cheats_in_same_lines():
+    diff_data = """
++some code # pylint: disable=some-rule, some-other-rule
+-some other code
+"""
+    result = no_cheat(diff_data)
+    assert result == (
+        "Do not cheat the linter: found 1 additional # pylint: disable=some-rule\n"
+        "Do not cheat the linter: found 1 additional # pylint: disable=some-other-rule"
+    )