/ devlog / devscripts / check_docblock_edits.sh
#!/bin/sh
# Reads a patch and refuses docblock edits that were made by pattern
# rather than by reading and rewriting.
#
#   check_docblock_edits.sh patch.diff     check a delivery
#   check_docblock_edits.sh --test         run the self-test
#
# WHY THIS EXISTS. A gate asks that every docblock in a file a patch
# touches names its subject. Faced with a file carrying two hundred old
# docblocks, the cheap way to satisfy that gate is a pass that prepends
# the subject's name to whatever sentence is there and swaps words out of
# a list. That produced sentences that are not English -- "yioop_error_
# handler user defined function to perform error handling for" -- and,
# worse, dropped facts a developer needs, such as the name LocalConfig.php
# becoming "the local settings file". Chris caught it after it had run
# over eight files. The pass is gone; this refuses its shape from coming
# back, whoever writes it.
#
# The three shapes it looks for, all read off the patch itself:
#
#   A name a reader needs disappears. A removed comment line
#   names a file, a class or a constant and the line replacing it does
#   not. That is the LocalConfig.php fault exactly.
#
#   A sentence gets a name bolted on the front. The added line
#   is the removed line with a symbol's name inserted at its start and
#   the first letter lowered, which is what a naming pass does and what
#   a person rewriting a sentence never does.
#
#   Too many docblock lines change in one file for the change
#   the patch makes. A patch that fixes one read should not rewrite a
#   hundred and eighty docblocks; where it truly must, DOCBLOCKS_BY_HAND
#   names the file and says the rewriting was read and written.
#
# Exits non-zero when it finds one, so cut_patch.sh can gate on it.
MOST_DOCBLOCK_LINES=${MOST_DOCBLOCK_LINES:-40}

# Reports the docblock edits in one patch that look machine-made.
check_patch()
{
    python3 - "$1" "$MOST_DOCBLOCK_LINES" <<'PYEOF'
import os
import re
import sys

path = sys.argv[1]
most_lines = int(sys.argv[2])
by_hand = set(os.environ.get("DOCBLOCKS_BY_HAND", "").split())
try:
    diff = open(path, encoding="utf-8", errors="replace").read()
except OSError:
    print("check: name a file holding the patch")
    sys.exit(2)
faults = 0
file_now = ""
changed_here = {}
# A large insertion makes git line up a comment taken from one place
# against a comment added in another, and the two have nothing to do
# with each other. A comment line that the patch removes and adds again
# in the same file has moved rather than been rewritten, so it is passed
# over. A line that is rewritten is removed and not added back, so it is
# still caught.
moved = {}
def note_moved():
    where = ""
    for one in diff.split("\n"):
        if one.startswith("+++ b/"):
            where = one[6:].strip()
            continue
        if one.startswith("+") and not one.startswith("+++") and \
            comment_line(one):
            body = one[1:].strip().lstrip("*").strip()
            moved.setdefault(where, {})
            moved[where][body] = moved[where].get(body, 0) + 1
def was_moved(name, line):
    held = moved.get(name, {})
    if held.get(line, 0) > 0:
        held[line] -= 1
        return True
    return False
# a name a reader needs: a file, a class, or a constant said in capitals
NEEDED = re.compile(r'\b\w+\.(?:php|js|css|html|ini|txt)\b|'
    r'\b[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+\b|\b[A-Z][a-z]+[A-Z]\w+\b')

def comment_line(line):
    body = line[1:].strip()
    return body.startswith("*") or body.startswith("/*")

note_moved()
# Every name the patch adds to each file, read before the pairing, so a
# name that moved down a line inside the same docblock is not read as
# lost.
all_added_names = {}
where = ""
for one in diff.split("\n"):
    if one.startswith("+++ b/"):
        where = one[6:].strip()
        continue
    if one.startswith("+") and not one.startswith("+++") and \
        comment_line(one):
        all_added_names.setdefault(where, set())
        all_added_names[where] |= set(NEEDED.findall(one))
removed = []
for line in diff.split("\n"):
    if line.startswith("+++ b/"):
        file_now = line[6:].strip()
        removed = []
        continue
    if line.startswith("@@"):
        removed = []
        continue
    if line.startswith("-") and not line.startswith("---"):
        if comment_line(line):
            body = line[1:].strip().lstrip("*").strip()
            if was_moved(file_now, body):
                continue
            removed.append(body)
            # only a comment line taken away counts toward the tally: a
            # new file adds hundreds of comment lines and removes none,
            # and writing a new file is not rewriting anyone's docblocks
            changed_here[file_now] = changed_here.get(file_now, 0) + 1
        continue
    if line.startswith("+") and not line.startswith("+++"):
        if not comment_line(line):
            continue
        added = line[1:].strip().lstrip("*").strip()
        if file_now in by_hand:
            removed = []
            continue
        for gone in removed:
            names_gone = set(NEEDED.findall(gone))
            names_kept = set(NEEDED.findall(added))
            # A rewritten docblock may say the name further down rather
            # than on the line git happened to line up, so every line the
            # patch adds to this file counts as keeping it.
            names_kept |= all_added_names.get(file_now, set())
            lost = names_gone - names_kept
            if lost and len(gone) > 20:
                print(f"{file_now}: a rewritten comment drops a name a "
                    f"reader needs: {', '.join(sorted(lost))}")
                print(f"  was: {gone[:70]}")
                print(f"  now: {added[:70]}")
                faults += 1
                break
            # a name bolted on the front, with the old first letter lowered
            found = re.match(r'^(\w+)\s+(.*)$', added)
            if found and len(gone) > 20:
                rest = found.group(2)
                # the sentence may have been rewrapped, so both sides are
                # compared as their opening words with case set aside
                opening = " ".join(rest.split()[:8]).lower()
                was = " ".join(gone.split()[:8]).lower()
                same = opening == was or was.startswith(opening) or \
                    opening.startswith(was)
                first_word = gone.split()[0] if gone.split() else ""
                if same and found.group(1).lower() != first_word.lower():
                    print(f"{file_now}: a name was bolted onto the front "
                        f"of a sentence rather than the sentence rewritten")
                    print(f"  was: {gone[:70]}")
                    print(f"  now: {added[:70]}")
                    faults += 1
                    break
        removed = []
for one, count in sorted(changed_here.items()):
    if one in by_hand or count <= most_lines:
        continue
    print(f"{one}: {count} comment lines taken away, past the "
        f"{most_lines} "
        f"a patch changes by reading and rewriting")
    faults += 1
if faults:
    print("check: the docblock edits above look made by pattern; read "
        "each one and rewrite it, or name the file in DOCBLOCKS_BY_HAND")
    sys.exit(1)
print("check: no docblock edit looks machine-made")
sys.exit(0)
PYEOF
}

# Walks a patch of each shape past the check and reports any it misses,
# and an ordinary docblock edit that it must let through.
run_self_test()
{
    scratch=`mktemp`
    missed=0
    cat > "$scratch" <<'DIFFEOF'
diff --git a/src/configs/Config.php b/src/configs/Config.php
--- a/src/configs/Config.php
+++ b/src/configs/Config.php
@@ -1,3 +1,3 @@
- * nsconddefine it should be fair game for tweaking in the LocalConfig.php
+ * nsconddefine it should be fair game for tweaking in the local settings
DIFFEOF
    if check_patch "$scratch" > /dev/null; then
        echo "self-test: a dropped file name was let through"
        missed=`expr $missed + 1`
    fi
    cat > "$scratch" <<'DIFFEOF'
diff --git a/src/configs/Config.php b/src/configs/Config.php
--- a/src/configs/Config.php
+++ b/src/configs/Config.php
@@ -1,3 +1,3 @@
- * User defined function to perform error handling for yioop where a
+ * yioop_error_handler user defined function to perform error handling for
DIFFEOF
    if check_patch "$scratch" > /dev/null; then
        echo "self-test: a name bolted on the front was let through"
        missed=`expr $missed + 1`
    fi
    {
        echo "diff --git a/src/library/Big.php b/src/library/Big.php"
        echo "--- a/src/library/Big.php"
        echo "+++ b/src/library/Big.php"
        echo "@@ -1,3 +1,3 @@"
        count=0
        while [ $count -lt 60 ]; do
            echo "-     * an older line of words about the work"
            echo "+     * a line of ordinary words about the work"
            count=`expr $count + 1`
        done
    } > "$scratch"
    if check_patch "$scratch" > /dev/null; then
        echo "self-test: a hundred rewritten docblocks were let through"
        missed=`expr $missed + 1`
    fi
    cat > "$scratch" <<'DIFFEOF'
diff --git a/src/library/One.php b/src/library/One.php
--- a/src/library/One.php
+++ b/src/library/One.php
@@ -1,4 +1,5 @@
-     * Reads the next packet.
+     * readPacket reads the next packet of sound from the file and hands
+     * back its samples. The caller asks for one packet at a time so a
+     * long recording never sits in memory whole.
DIFFEOF
    if ! check_patch "$scratch" > /dev/null; then
        echo "self-test: an ordinary rewrite was refused"
        missed=`expr $missed + 1`
    fi
    rm -f "$scratch"
    if [ "$missed" -eq 0 ]; then
        echo "self-test: four cases tried, none went wrong"
        return 0
    fi
    echo "self-test: $missed of four cases went wrong"
    return 1
}

if [ "$1" = "--test" ]; then
    run_self_test
    exit $?
fi
if [ -z "${1:-}" ] || [ ! -f "$1" ]; then
    echo "check: name a file holding the patch"
    exit 2
fi
check_patch "$1"
X