`make_replacement` fallback returns an unchecked placeholder, can corrupt keyword renames

[mwiebe]

Summary

make_replacement in crates/openjd-expr/src/eval/parse.rs picks a same-length placeholder identifier to stand in for a Python keyword used as an attribute name (Param.if). It tries 26 candidates — the keyword with its first byte replaced by a through z — and accepts one only if it is absent from the source and is not itself a Python keyword:

if replacement.len() == len
    && !source.contains(&replacement)
    && !PYTHON_KEYWORDS.contains(&replacement.as_str())

If all 26 are rejected, it falls through to:

// Fallback: all x's
"x".repeat(len)

The fallback doesn't perform the check, so this can silently corrupt the code.

Reproduction

Both fail on main. Add to crates/openjd-expr/tests/:

use openjd_expr::{ExprValue, ParsedExpression, SymbolTable};

// Exhausts af..zf and bs..zs. "as" and "if" are absent from the fillers
// because they are Python keywords and the loop rejects them anyway.
const S_FILL: &str = "bs cs ds es fs gs hs is js ks ls ms ns os ps qs rs ss ts us vs ws xs ys zs";
const F_FILL: &str = "af bf cf df ef ff gf hf jf kf lf mf nf of pf qf rf sf tf uf vf wf xf yf zf";

#[test]
fn two_keywords_must_not_share_a_placeholder() {
    let mut st = SymbolTable::new();
    st.set("X.as", ExprValue::String("[AS]".into())).unwrap();
    st.set("Y.if", ExprValue::String("[IF]".into())).unwrap();

    let expr = format!("X.as + Y.if + '{S_FILL} {F_FILL}'");
    let got = ParsedExpression::new(&expr).and_then(|p| p.evaluate(&st));
    assert!(got.is_ok(), "{got:?}");
}

#[test]
fn placeholder_must_not_collide_with_a_real_attribute() {
    let mut st = SymbolTable::new();
    st.set("A.xxfoo", ExprValue::String("SIBLING".into())).unwrap();
    st.set("X.if", ExprValue::String("KW".into())).unwrap();

    let expr = format!("'{F_FILL}' + A.xxfoo + X.if");
    let got = ParsedExpression::new(&expr).and_then(|p| p.evaluate(&st));
    assert!(got.is_ok(), "{got:?}");
}

Observed for the first test — both keywords got xx, and the as entry was overwritten:

keyword_renames = {"xx": "if"}
Err(UndefinedVariable { name: "X.xx", suggestion: " Did you mean: X.as" })

Observed for the second test — the placeholder xx collided with the real attribute xxfoo, which the reverse replace rewrote to A.iffoo:

Err(UndefinedVariable { name: "A.xxfoo", suggestion: " Did you mean: A.xxfoo" })

Note the second error suggests the exact symbol it just reported as undefined.

Suggested fix

1. Always check the replacement placeholder against the code and against the existing chosen substitutions. Never return a potential collision.
2. Keep preserving the length, that detail is important.
3. Use a larger placeholder search space. More possible substitution characters. It we use lower and uppercase, also numbers for the second and later character, then we get about 3000 possible placeholder bigrams in the two-letter keyword case.
4. Return an error if no substitution works.

Notes

Found while reviewing #321, decided it's better to file a new issue than expand that PR's scope.