fix(kyberforge): carry apostrophes verbatim through a |- literal block
The flattener's last-resort branch rewrote ASCII ' to U+2019, justified as the one combination no YAML scalar can carry verbatim. That claim was false: a |- literal block with a single indented content line carries ', ", \ and ": " verbatim and keeps text.frontmatter.description matching — as the wrapper's own docstring already said of literal blocks. The rewrite fired on 12 of 54 in-scope files, silently disabling every rule whose token contains an apostrophe. Case 20 pinned only that the scope stayed alive, so it passed either way. The emission site now splits the emitted scalar on its first newline so a carried-over trailing comment stays on the "description: |-" header line rather than becoming part of the value, and pads by span_lines - 1 - newlines. The pad stays non-negative because the branch is only reachable when the original span is at least two lines. Verified across all 73 in-scope files: no line-count changes, and exactly the 12 expected files take the new branch. One reported position moves: an alert on a description that is itself flagged shifts from the key line to the block's content line, both inside the original span. YAML cannot put a literal block's content on the key's own line, so this is unavoidable; no line at or after the end of any description span moves. Also: --output no longer absolutises the built-in style names line, JSON and CLI, which a same-named file or directory in cwd turned into a template path (exit 2, E100 Runtime error). And case 19's empty-baseline guard no longer lets five dependent comparisons print vacuous passes — while fixing it the guard turned out to be unreachable, since under pipefail an alert-free report aborted the script at the assignment. Refs: #85 ADR: 0014
This commit is contained in:
@@ -443,8 +443,11 @@ fi
|
||||
# is declared and never reset to empty: it cannot be empty at any expansion
|
||||
# site, so the construct is not a hazard there and demanding the guarded form
|
||||
# would be a wrong test. The file list covers every script this repo ships or
|
||||
# runs that a macOS user reaches: the wrapper itself plus the two pre-commit
|
||||
# hook scripts.
|
||||
# runs that a macOS user reaches: the wrapper itself, the two pre-commit hook
|
||||
# scripts, and the test runner AGENTS.md tells contributors to run by hand.
|
||||
# `mapfile` is checked alongside, because it is bash 4.0+ and the expansion scan
|
||||
# cannot see it — run-tests.sh carried one until it was replaced with a
|
||||
# `while read` loop, and nothing would have caught its return.
|
||||
echo ""
|
||||
echo "--- no unguarded array expansion remains in the macOS-facing scripts ---"
|
||||
unguarded_expansions() {
|
||||
@@ -470,11 +473,19 @@ HAZARDS16=""
|
||||
for BASH32_SCRIPT in \
|
||||
"$SCRIPT" \
|
||||
"$REPO_ROOT/scripts/skill-size-check.sh" \
|
||||
"$REPO_ROOT/scripts/check-release-needed.sh"; do
|
||||
"$REPO_ROOT/scripts/check-release-needed.sh" \
|
||||
"$REPO_ROOT/tests/run-tests.sh"; do
|
||||
FOUND16="$(unguarded_expansions "$BASH32_SCRIPT")"
|
||||
if [[ -n "$FOUND16" ]]; then
|
||||
HAZARDS16+="$FOUND16 "
|
||||
fi
|
||||
# `mapfile`/`readarray` are bash 4.0+ builtins with no 3.2 fallback. Whole-line
|
||||
# comments are blanked first so prose naming the builtin is not a hit.
|
||||
FOUND16B="$(awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$BASH32_SCRIPT" \
|
||||
| grep -nE '(^|[^[:alnum:]_])(mapfile|readarray)[[:space:]]' || true)"
|
||||
if [[ -n "$FOUND16B" ]]; then
|
||||
HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16B "
|
||||
fi
|
||||
done
|
||||
if [[ -n "$HAZARDS16" ]]; then
|
||||
fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')"
|
||||
@@ -588,10 +599,15 @@ make_form_fixture() {
|
||||
}
|
||||
|
||||
# Alert text with the `line:col` prefix and ANSI colouring stripped, sorted.
|
||||
# The `|| true` matters under this file's `set -o pipefail`: a report with no
|
||||
# alerts at all makes grep exit 1, which would abort the whole run inside the
|
||||
# command substitutions below — silently, before the empty-baseline guard could
|
||||
# print anything. Returning empty output instead is what makes that guard
|
||||
# reachable.
|
||||
alert_text() {
|
||||
echo "$1" \
|
||||
| sed -E 's/\x1b\[[0-9;]*m//g' \
|
||||
| grep -oE '(error|warning|suggestion)[[:space:]]+.*' \
|
||||
| { grep -oE '(error|warning|suggestion)[[:space:]]+.*' || true; } \
|
||||
| sed -E 's/[[:space:]]+/ /g' \
|
||||
| sort
|
||||
}
|
||||
@@ -601,29 +617,35 @@ echo "--- every multi-line description form reports what its single-line form re
|
||||
FIXTURE19_SINGLE="$(make_form_fixture single)"
|
||||
BASELINE19="$(alert_text "$(run_wrap "$FIXTURE19_SINGLE" --config "$VALE_CONFIG" "$REL_SKILL19")")"
|
||||
if [[ -z "$BASELINE19" ]]; then
|
||||
fail "the single-line baseline reported nothing — the comparison below would be vacuous"
|
||||
# The loop below has to be skipped, not merely reported on: an empty baseline
|
||||
# compares equal to five empty results, so it would print five vacuous PASSes
|
||||
# alongside this one FAIL. The FAIL alone still fails the run at the end.
|
||||
fail "the single-line baseline reported nothing — the comparisons below would be vacuous, so they are skipped"
|
||||
else
|
||||
for FORM19 in folded plain dquote squote keyonly; do
|
||||
DIR19="$(make_form_fixture "$FORM19")"
|
||||
BARE19="$(cd "$DIR19" && vale --config "$VALE_CONFIG" "$REL_SKILL19" 2>&1 || true)"
|
||||
GOT19="$(alert_text "$(run_wrap "$DIR19" --config "$VALE_CONFIG" "$REL_SKILL19")")"
|
||||
if echo "$BARE19" | grep -q "VagueWording"; then
|
||||
fail "bare vale already flags the $FORM19 form, so this case can't detect a silently-skipped flattening"
|
||||
elif [[ "$GOT19" == "$BASELINE19" ]]; then
|
||||
pass "a $FORM19 multi-line description reports the same alerts as its single-line form"
|
||||
else
|
||||
fail "a $FORM19 multi-line description diverged from its single-line form: got [$GOT19]"
|
||||
fi
|
||||
done
|
||||
fi
|
||||
for FORM19 in folded plain dquote squote keyonly; do
|
||||
DIR19="$(make_form_fixture "$FORM19")"
|
||||
BARE19="$(cd "$DIR19" && vale --config "$VALE_CONFIG" "$REL_SKILL19" 2>&1 || true)"
|
||||
GOT19="$(alert_text "$(run_wrap "$DIR19" --config "$VALE_CONFIG" "$REL_SKILL19")")"
|
||||
if echo "$BARE19" | grep -q "VagueWording"; then
|
||||
fail "bare vale already flags the $FORM19 form, so this case can't detect a silently-skipped flattening"
|
||||
elif [[ "$GOT19" == "$BASELINE19" ]]; then
|
||||
pass "a $FORM19 multi-line description reports the same alerts as its single-line form"
|
||||
else
|
||||
fail "a $FORM19 multi-line description diverged from its single-line form: got [$GOT19]"
|
||||
fi
|
||||
done
|
||||
|
||||
# --- 20. A style token containing an ASCII apostrophe matches inside a
|
||||
# flattened description. The flattener used to substitute U+2019 for every `'`
|
||||
# before writing the scratch copy, so no rule whose token carried an apostrophe
|
||||
# could ever fire on a flattened description — a silent, rule-shaped blind spot.
|
||||
# Both branches that can hold an apostrophe verbatim are exercised: a value that
|
||||
# is safe unquoted, and one that must be quoted (it contains `: `) and so has to
|
||||
# land in a double-quoted scalar, since a single-quoted one would need the `''`
|
||||
# escape that kills the scope outright.
|
||||
# All three branches that can hold an apostrophe are exercised: a value that is
|
||||
# safe unquoted; one that must be quoted (it contains `: `) and so lands in a
|
||||
# double-quoted scalar, since a single-quoted one would need the `''` escape
|
||||
# that kills the scope outright; and one that also holds a double quote, which
|
||||
# no inline scalar can spell verbatim and which therefore lands in a `|-`
|
||||
# literal block.
|
||||
echo ""
|
||||
echo "--- a style token containing an apostrophe matches in a flattened description ---"
|
||||
APOS_STYLE="$(mktemp -d)"
|
||||
@@ -638,6 +660,17 @@ ignorecase: true
|
||||
tokens:
|
||||
- "user's task"
|
||||
EOF
|
||||
# A body-scoped companion rule, used by case 20b to read back the line number of
|
||||
# a line *after* the frontmatter — the only way to catch the blank-line pad
|
||||
# being off in either direction.
|
||||
cat > "$APOS_STYLE/styles/Apostrophe/Body.yml" <<'EOF'
|
||||
extends: existence
|
||||
message: "body token: '%s'"
|
||||
level: error
|
||||
scope: text
|
||||
tokens:
|
||||
- flattening marker phrase
|
||||
EOF
|
||||
cat > "$APOS_STYLE/.vale.ini" <<'EOF'
|
||||
StylesPath = styles
|
||||
|
||||
@@ -668,7 +701,23 @@ Body.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20_QUOTED"
|
||||
for CASE20 in "unquoted:$FIXTURE20_PLAIN" "double-quoted:$FIXTURE20_QUOTED"; do
|
||||
# Needs quoting (`: `), holds an apostrophe AND a double quote — the one
|
||||
# combination no inline scalar can carry, so this is the `|-` literal-block
|
||||
# branch. The VagueWording tokens are there for case 20b, which reuses it.
|
||||
FIXTURE20_BLOCK="$(make_raw_fixture <<'EOF'
|
||||
---
|
||||
name: zzzskill
|
||||
description: >
|
||||
Triggers on: the user's task and "audit this" phrasing, which helps
|
||||
with and utilize things across a second physical line.
|
||||
---
|
||||
|
||||
Body carrying a flattening marker phrase for the line-number check.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20_BLOCK"
|
||||
for CASE20 in "unquoted:$FIXTURE20_PLAIN" "double-quoted:$FIXTURE20_QUOTED" \
|
||||
"literal-block:$FIXTURE20_BLOCK"; do
|
||||
if run_wrap "${CASE20#*:}" --config "$APOS_STYLE/.vale.ini" "$REL_SKILL19" \
|
||||
| grep -q "Apostrophe.Token"; then
|
||||
pass "an apostrophe-bearing token matches in a flattened ${CASE20%%:*} description"
|
||||
@@ -677,29 +726,31 @@ for CASE20 in "unquoted:$FIXTURE20_PLAIN" "double-quoted:$FIXTURE20_QUOTED"; do
|
||||
fi
|
||||
done
|
||||
|
||||
# --- 20b. The one combination no verbatim YAML scalar can carry — needs
|
||||
# quoting, holds an apostrophe, and holds a double quote — falls back to the
|
||||
# lossy U+2019 substitution. Apostrophe-bearing tokens are lost there by
|
||||
# design, but the scope must stay alive so every other rule still fires.
|
||||
# --- 20b. The `|-` literal-block branch that case 20 just proved lossless must
|
||||
# also keep the rest of the scope working and keep the line accounting right.
|
||||
# The block is 2 physical lines where every inline form is 1, so the blank-line
|
||||
# pad that preserves later line numbers has to drop by one. The second
|
||||
# assertion pins that arithmetic against the body line's true number: case 3's
|
||||
# `<= original line count` bound would not, since a pad that is one line short
|
||||
# shifts every later line *up*, staying inside the bound while still lying.
|
||||
echo ""
|
||||
echo "--- the unrepresentable combination keeps the description scope alive ---"
|
||||
FIXTURE20C="$(make_raw_fixture <<'EOF'
|
||||
---
|
||||
name: zzzskill
|
||||
description: >
|
||||
Triggers on: the user's "audit this" phrasing, which helps with
|
||||
and utilize things across a second physical line.
|
||||
---
|
||||
|
||||
Body.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20C"
|
||||
if run_wrap "$FIXTURE20C" --config "$VALE_CONFIG" "$REL_SKILL19" | grep -q "VagueWording"; then
|
||||
echo "--- the |- literal-block fallback lints normally and preserves line numbers ---"
|
||||
OUT20B=$(run_wrap "$FIXTURE20_BLOCK" --config "$VALE_CONFIG" "$REL_SKILL19")
|
||||
if echo "$OUT20B" | grep -q "VagueWording"; then
|
||||
pass "a description needing quotes with both an apostrophe and a double quote is still linted"
|
||||
else
|
||||
fail "a description needing quotes with both an apostrophe and a double quote produced no alerts"
|
||||
fi
|
||||
WANT20B_LINE="$(grep -n 'flattening marker phrase' "$FIXTURE20_BLOCK/$REL_SKILL19" | cut -d: -f1)"
|
||||
# `--output line` prints `file:line:col:Rule:message`, so the line number reads
|
||||
# back without any wrapping or colour to strip.
|
||||
GOT20B_LINE="$(run_wrap "$FIXTURE20_BLOCK" --config "$APOS_STYLE/.vale.ini" --output line "$REL_SKILL19" \
|
||||
| grep 'Apostrophe.Body' | head -1 | cut -d: -f2)"
|
||||
if [[ "$GOT20B_LINE" == "$WANT20B_LINE" ]]; then
|
||||
pass "a body line after a |- flattened description keeps its original line number ($WANT20B_LINE)"
|
||||
else
|
||||
fail "the |- block's blank-line pad shifted the body: vale reported line $GOT20B_LINE, the file has it at $WANT20B_LINE"
|
||||
fi
|
||||
|
||||
# --- 21. A symlinked file inside a directory argument is mirrored and linted.
|
||||
# Vale follows symlinks (both a symlinked file and a file under a symlinked
|
||||
@@ -766,6 +817,31 @@ else
|
||||
fail "a typo'd path exited $RC23 but the message does not name it: $OUT23"
|
||||
fi
|
||||
|
||||
# --- 24. `--output`'s built-in style names must not be path-absolutized. The
|
||||
# wrapper rewrites path-valued flag values to absolute form so they still
|
||||
# resolve after the `cd` into the scratch mirror, deciding with an `-e`
|
||||
# existence test — but `line`, `JSON` and `CLI` are style names, not paths. With
|
||||
# a file or directory of that name sitting in the caller's cwd the test hit, the
|
||||
# built-in became `$cwd/line`, and vale flipped into template mode and died with
|
||||
# `E100 [template] Runtime error` where bare vale prints a normal report.
|
||||
echo ""
|
||||
echo "--- a built-in --output style name survives a same-named entry in the cwd ---"
|
||||
FIXTURE24="$(make_fixture 2)"
|
||||
new_fixture "$FIXTURE24"
|
||||
mkdir -p "$FIXTURE24/line"
|
||||
: > "$FIXTURE24/JSON"
|
||||
for FORM24 in "--output line" "--output=line" "--output JSON" "--output=JSON"; do
|
||||
# shellcheck disable=SC2086 # deliberate word splitting of the argv fixture
|
||||
OUT24="$(run_wrap "$FIXTURE24" --config "$VALE_CONFIG" $FORM24 "$REL_SKILL19")"
|
||||
if echo "$OUT24" | grep -q "E100"; then
|
||||
fail "'$FORM24' was rewritten to a cwd path and vale flipped into template mode — the bug this test guards against"
|
||||
elif echo "$OUT24" | grep -q "VagueWording"; then
|
||||
pass "'$FORM24' is passed through as a built-in style name"
|
||||
else
|
||||
fail "'$FORM24' produced no alert: $OUT24"
|
||||
fi
|
||||
done
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
Reference in New Issue
Block a user