fix(lint): flatten every multi-line description form in vale-wrap.sh
Vale locates a frontmatter description by matching the parsed YAML value back against the source text, so any scalar whose value is not spelled out verbatim loses the `text.frontmatter.description` scope entirely. The wrapper only flattened `>` folded scalars, so plain, double-quoted and single-quoted multi-line descriptions silently reported zero alerts and exit 0 — a clean pass indistinguishable from a real one, in a prefilter whose callers are instructed not to re-derive its verdict by judgment. Implementation notes: - Classify the scalar kind after `^description:[ \t]*` and reuse one shared continuation-line generator for every form; `|` literal blocks keep their line breaks, stay verbatim-matchable, and are still left untouched. - Emit the flattened value in whichever scalar form needs no escape at all (plain, then single-quoted, then double-quoted), because any escape breaks the verbatim match. The old blanket `'` -> U+2019 substitution silently made apostrophe-bearing rule tokens unmatchable across 63% of the corpus; it now survives only for the one combination no YAML scalar can carry verbatim. - Terminate continuations at a line flush with the key, not only on a shallower indent — a `description:` followed by a flush-left line previously swallowed the rest of the frontmatter. - Route vale's value-taking flags explicitly instead of inferring targets by file existence, and absolutize relative `--output`/`--path` values the way `--config` already was, since the run `cd`s into the scratch mirror. - Fail loudly on a nonexistent path instead of inheriting bare vale's fallback to stdin, which rendered a typo'd path as `0 errors ... in stdin`, exit 0 — a form the callers' `0 files` NOT-RUN guard cannot match. - Follow symlinks when walking a directory argument, matching bare vale. Refs: #85
This commit is contained in:
@@ -321,7 +321,12 @@ else
|
||||
fail "an absolute path was silently skipped — the bug this test guards against"
|
||||
fi
|
||||
|
||||
# --- 11. A literal (|) block scalar passes through unflattened (no regression) ---
|
||||
# --- 11. A literal (|) block scalar passes through unflattened. Unlike every
|
||||
# other multi-line form, `|` is not broken in Vale: its parsed value keeps the
|
||||
# same line breaks the source has, so the description scope still matches. The
|
||||
# second assertion pins that down — without it, a wrapper that broke `|` and a
|
||||
# Vale that never matched `|` would agree on zero alerts and the comparison
|
||||
# would pass vacuously.
|
||||
echo ""
|
||||
echo "--- leaves a literal (|) block scalar untouched (narrowed >-only scope) ---"
|
||||
FIXTURE11="$(make_raw_fixture <<'EOF'
|
||||
@@ -339,7 +344,9 @@ trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTU
|
||||
REL11="plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
WRAPPED_OUT=$(run_wrap "$FIXTURE11" --config "$VALE_CONFIG" "$REL11")
|
||||
BARE_OUT=$(cd "$FIXTURE11" && vale --config "$VALE_CONFIG" "$REL11" 2>&1 || true)
|
||||
if [[ "$WRAPPED_OUT" == "$BARE_OUT" ]]; then
|
||||
if ! echo "$BARE_OUT" | grep -q "VagueWording"; then
|
||||
fail "bare vale reports nothing for a literal (|) block scalar — the 'literal blocks are not broken' premise is wrong"
|
||||
elif [[ "$WRAPPED_OUT" == "$BARE_OUT" ]]; then
|
||||
pass "literal (|) block scalar output matches bare vale exactly — untouched by flattening"
|
||||
else
|
||||
fail "wrapper altered output for a literal (|) block scalar description — should be left untouched"
|
||||
@@ -425,23 +432,50 @@ else
|
||||
fail "a path with a space was dropped from the directory walk"
|
||||
fi
|
||||
|
||||
# --- 16. No unguarded `"${arr[@]}"` expansion survives in the wrapper. bash
|
||||
# before 4.4 — including the 3.2 that macOS still ships as /bin/bash — treats
|
||||
# that form on an empty array as an unbound variable under `set -u` and aborts.
|
||||
# The portable form is `${arr[@]+"${arr[@]}"}`. This is a static check because
|
||||
# no bash 5 host can reproduce the abort at runtime: the construct is only fatal
|
||||
# on the older shell, so absence of the construct is the property to assert.
|
||||
# `${#arr[@]}` is deliberately not flagged — the count form is safe on 3.2.
|
||||
# --- 16. No unguarded `"${arr[@]}"` expansion survives in any script that runs
|
||||
# on macOS. bash before 4.4 — including the 3.2 that macOS still ships as
|
||||
# /bin/bash — treats that form on an *empty* array as an unbound variable under
|
||||
# `set -u` and aborts. The portable form is `${arr[@]+"${arr[@]}"}`. This is a
|
||||
# static check because no bash 5 host can reproduce the abort at runtime: the
|
||||
# construct is only fatal on the older shell, so absence of the construct is the
|
||||
# property to assert. `${#arr[@]}` is deliberately not flagged — the count form
|
||||
# is safe on 3.2. Neither is an array seeded with at least one element where it
|
||||
# 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.
|
||||
echo ""
|
||||
echo "--- no unguarded array expansion remains in vale-wrap.sh ---"
|
||||
echo "--- no unguarded array expansion remains in the macOS-facing scripts ---"
|
||||
unguarded_expansions() {
|
||||
# Blank out whole-line comments (keeping line numbers), delete every correctly
|
||||
# guarded expansion, then anything still matching is a real hazard.
|
||||
awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$1" \
|
||||
| sed -E 's/\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+"\$\{\1\[@\]\}"\}//g' \
|
||||
| grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' || true
|
||||
local file="$1" hit name
|
||||
while IFS= read -r hit; do
|
||||
name="$(printf '%s\n' "$hit" \
|
||||
| grep -oE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' | head -1 \
|
||||
| sed -E 's/^\$\{//; s/\[@\]\}$//')"
|
||||
if grep -qE "^[[:space:]]*((local|declare|readonly)[[:space:]]+)?(-a[[:space:]]+)?$name=\([^)]" "$file" \
|
||||
&& ! grep -qE "^[[:space:]]*$name=\(\)" "$file"; then
|
||||
continue
|
||||
fi
|
||||
printf '%s:%s\n' "${file##*/}" "$hit"
|
||||
done < <(
|
||||
# Blank out whole-line comments (keeping line numbers), delete every
|
||||
# correctly guarded expansion, then anything still matching is a candidate.
|
||||
awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$file" \
|
||||
| sed -E 's/\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+"\$\{\1\[@\]\}"\}//g' \
|
||||
| grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' || true
|
||||
)
|
||||
}
|
||||
HAZARDS16="$(unguarded_expansions "$SCRIPT")"
|
||||
HAZARDS16=""
|
||||
for BASH32_SCRIPT in \
|
||||
"$SCRIPT" \
|
||||
"$REPO_ROOT/scripts/skill-size-check.sh" \
|
||||
"$REPO_ROOT/scripts/check-release-needed.sh"; do
|
||||
FOUND16="$(unguarded_expansions "$BASH32_SCRIPT")"
|
||||
if [[ -n "$FOUND16" ]]; then
|
||||
HAZARDS16+="$FOUND16 "
|
||||
fi
|
||||
done
|
||||
if [[ -n "$HAZARDS16" ]]; then
|
||||
fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')"
|
||||
else
|
||||
@@ -500,6 +534,238 @@ else
|
||||
fail "a path argument with a space was split by the array expansion: $OUT18"
|
||||
fi
|
||||
|
||||
# The cases below share one cleanup list. The per-case trap rebuilding above
|
||||
# does not scale past the fixture count it already carries, and this trap is
|
||||
# installed last, so it is the one that runs.
|
||||
EXTRA_FIXTURES=()
|
||||
new_fixture() { EXTRA_FIXTURES+=("$1"); }
|
||||
cleanup_all() {
|
||||
rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" \
|
||||
"$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" \
|
||||
"$FIXTURE14" "$FIXTURE17" "$FIXTURE18" \
|
||||
${EXTRA_FIXTURES[@]+"${EXTRA_FIXTURES[@]}"}
|
||||
}
|
||||
trap cleanup_all EXIT
|
||||
|
||||
# --- 19. Every YAML form whose parsed value is joined back out of 2+ physical
|
||||
# lines breaks the `text.frontmatter.description` scope identically, not just
|
||||
# the `>` folded block the flattener originally handled: a plain scalar wrapped
|
||||
# onto continuation lines, a double-quoted one, a single-quoted one, and a bare
|
||||
# `description:` whose value starts on the next line all report zero alerts
|
||||
# under bare vale. Each must come back with the same alerts as the single-line
|
||||
# spelling of the same sentence. Line and column numbers legitimately move (the
|
||||
# value lands on one physical line), so the comparison drops the `line:col`
|
||||
# prefix and compares the alert text — message, matched token, and rule name.
|
||||
REL_SKILL19="plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
DESC19_A="Use when the caller helps with a specific job"
|
||||
DESC19_B="and the second physical line will utilize the wrap"
|
||||
|
||||
# make_form_fixture spells the same two-clause description in one YAML scalar
|
||||
# form: single, folded, plain, dquote, squote, or keyonly.
|
||||
make_form_fixture() {
|
||||
local form="$1" dir
|
||||
dir="$(mktemp -d)"
|
||||
new_fixture "$dir"
|
||||
(cd "$dir" && git init -q)
|
||||
mkdir -p "$dir/plugins/testplugin/skills/zzzskill"
|
||||
{
|
||||
echo "---"
|
||||
echo "name: zzzskill"
|
||||
case "$form" in
|
||||
single) echo "description: $DESC19_A $DESC19_B" ;;
|
||||
folded) echo "description: >"; echo " $DESC19_A"; echo " $DESC19_B" ;;
|
||||
plain) echo "description: $DESC19_A"; echo " $DESC19_B" ;;
|
||||
dquote) echo "description: \"$DESC19_A"; echo " $DESC19_B\"" ;;
|
||||
squote) echo "description: '$DESC19_A"; echo " $DESC19_B'" ;;
|
||||
keyonly) echo "description:"; echo " $DESC19_A"; echo " $DESC19_B" ;;
|
||||
*) echo "make_form_fixture: unknown form '$form'" >&2; exit 1 ;;
|
||||
esac
|
||||
echo "---"
|
||||
echo ""
|
||||
echo "Body."
|
||||
} > "$dir/plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
echo "$dir"
|
||||
}
|
||||
|
||||
# Alert text with the `line:col` prefix and ANSI colouring stripped, sorted.
|
||||
alert_text() {
|
||||
echo "$1" \
|
||||
| sed -E 's/\x1b\[[0-9;]*m//g' \
|
||||
| grep -oE '(error|warning|suggestion)[[:space:]]+.*' \
|
||||
| sed -E 's/[[:space:]]+/ /g' \
|
||||
| sort
|
||||
}
|
||||
|
||||
echo ""
|
||||
echo "--- every multi-line description form reports what its single-line form reports ---"
|
||||
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"
|
||||
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.
|
||||
echo ""
|
||||
echo "--- a style token containing an apostrophe matches in a flattened description ---"
|
||||
APOS_STYLE="$(mktemp -d)"
|
||||
new_fixture "$APOS_STYLE"
|
||||
mkdir -p "$APOS_STYLE/styles/Apostrophe"
|
||||
cat > "$APOS_STYLE/styles/Apostrophe/Token.yml" <<'EOF'
|
||||
extends: existence
|
||||
message: "apostrophe token: '%s'"
|
||||
level: error
|
||||
scope: text.frontmatter.description
|
||||
ignorecase: true
|
||||
tokens:
|
||||
- "user's task"
|
||||
EOF
|
||||
cat > "$APOS_STYLE/.vale.ini" <<'EOF'
|
||||
StylesPath = styles
|
||||
|
||||
[**/SKILL.md]
|
||||
BasedOnStyles = Apostrophe
|
||||
EOF
|
||||
FIXTURE20_PLAIN="$(make_raw_fixture <<'EOF'
|
||||
---
|
||||
name: zzzskill
|
||||
description: >
|
||||
Use when the user's task needs handling, and a second physical
|
||||
line continues the folded scalar.
|
||||
---
|
||||
|
||||
Body.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20_PLAIN"
|
||||
FIXTURE20_QUOTED="$(make_raw_fixture <<'EOF'
|
||||
---
|
||||
name: zzzskill
|
||||
description: >
|
||||
Triggers on: the user's task needing handling, and a second
|
||||
physical line continues the folded scalar.
|
||||
---
|
||||
|
||||
Body.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20_QUOTED"
|
||||
for CASE20 in "unquoted:$FIXTURE20_PLAIN" "double-quoted:$FIXTURE20_QUOTED"; 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"
|
||||
else
|
||||
fail "an apostrophe-bearing token was rewritten out of a flattened ${CASE20%%:*} description"
|
||||
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.
|
||||
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
|
||||
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
|
||||
|
||||
# --- 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
|
||||
# directory), so a `-type f` walk of the tree reported "0 files" where bare vale
|
||||
# reports one — and the audit skills read a "0 files" report as NOT RUN.
|
||||
echo ""
|
||||
echo "--- mirrors a symlinked file reached through a directory argument ---"
|
||||
FIXTURE21="$(make_fixture 2)"
|
||||
new_fixture "$FIXTURE21"
|
||||
mkdir -p "$FIXTURE21/real"
|
||||
mv "$FIXTURE21/$REL_SKILL19" "$FIXTURE21/real/SKILL.md"
|
||||
ln -s ../../../../real/SKILL.md "$FIXTURE21/$REL_SKILL19"
|
||||
BARE21="$(cd "$FIXTURE21" && vale --config "$VALE_CONFIG" plugins 2>&1 || true)"
|
||||
WRAPPED21="$(run_wrap "$FIXTURE21" --config "$VALE_CONFIG" plugins)"
|
||||
BARE21_FILES="$(echo "$BARE21" | sed -E 's/\x1b\[[0-9;]*m//g' | grep -oE 'in [0-9]+ files?' | tail -1)"
|
||||
WRAPPED21_FILES="$(echo "$WRAPPED21" | sed -E 's/\x1b\[[0-9;]*m//g' | grep -oE 'in [0-9]+ files?' | tail -1)"
|
||||
if [[ "$BARE21_FILES" != "in 1 file" ]]; then
|
||||
fail "bare vale did not lint the symlinked file ($BARE21_FILES), so this case can't detect the walk dropping it"
|
||||
elif [[ "$WRAPPED21_FILES" != "$BARE21_FILES" ]]; then
|
||||
fail "the directory walk dropped a symlinked file: wrapper saw '$WRAPPED21_FILES', bare vale '$BARE21_FILES'"
|
||||
elif echo "$WRAPPED21" | grep -q "VagueWording"; then
|
||||
pass "a symlinked file under a directory argument is mirrored, flattened and flagged"
|
||||
else
|
||||
fail "a symlinked file was mirrored but not flattened — no alert came back"
|
||||
fi
|
||||
|
||||
# --- 22. The value of a separated two-argv flag is never treated as a lint
|
||||
# target, however file-like it looks. `--output tmpl.tmpl` names a real
|
||||
# template file: classifying it as input both linted the template and reordered
|
||||
# argv, so vale received `--output --no-wrap` and died on `open :`.
|
||||
echo ""
|
||||
echo "--- a separated flag value that names a real file is not linted as a target ---"
|
||||
FIXTURE22="$(make_fixture 1)"
|
||||
new_fixture "$FIXTURE22"
|
||||
printf 'TMPL{{range .Files}} {{.Path}}{{end}}\n' > "$FIXTURE22/tmpl.tmpl"
|
||||
WRAPPED22="$(run_wrap "$FIXTURE22" --config "$VALE_CONFIG" --output tmpl.tmpl --no-wrap "$REL_SKILL19")"
|
||||
BARE22="$(cd "$FIXTURE22" && vale --config "$VALE_CONFIG" --output tmpl.tmpl --no-wrap "$REL_SKILL19" 2>&1 || true)"
|
||||
# The fixture's description is a single physical line, so flattening is a no-op
|
||||
# and the two invocations must agree byte for byte.
|
||||
if [[ "$WRAPPED22" == "$BARE22" ]]; then
|
||||
pass "a separated --output value is passed through to vale, not linted"
|
||||
else
|
||||
fail "a separated --output value was misrouted: wrapper gave [$WRAPPED22], bare vale [$BARE22]"
|
||||
fi
|
||||
|
||||
# --- 23. A path argument that does not exist is a hard error. Bare vale drops
|
||||
# it, falls back to stdin and prints `0 errors ... in stdin` with exit 0, so a
|
||||
# typo'd target is indistinguishable from a clean run — and the audit skills'
|
||||
# NOT RUN guard string-matches `0 files`, which `in stdin` never produces. This
|
||||
# is a deliberate divergence from bare vale, documented in the wrapper header.
|
||||
echo ""
|
||||
echo "--- a nonexistent path argument fails loudly instead of falling back to stdin ---"
|
||||
set +e
|
||||
OUT23="$(cd "$FIXTURE22" && bash "$SCRIPT" --config "$VALE_CONFIG" plugins/testplugin/skills/zzzskill/SKILLL.md 2>&1)"
|
||||
RC23=$?
|
||||
set -e
|
||||
if [[ $RC23 -eq 0 ]]; then
|
||||
fail "a typo'd path exited 0 — indistinguishable from a clean run, the bug this test guards against"
|
||||
elif echo "$OUT23" | grep -q "in stdin"; then
|
||||
fail "a typo'd path fell back to reading stdin and reported 'in stdin' instead of erroring"
|
||||
elif echo "$OUT23" | grep -q "SKILLL.md"; then
|
||||
pass "a typo'd path exits nonzero with a message naming the path"
|
||||
else
|
||||
fail "a typo'd path exited $RC23 but the message does not name it: $OUT23"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
Reference in New Issue
Block a user