#!/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"