LuciferYang opened a new pull request, #56648:
URL: https://github.com/apache/spark/pull/56648
### What changes were proposed in this pull request?
`StringSubstitutor` (added in SPARK-52990) is the hand-rolled `${name}` /
`${name:-default}` interpolation parser behind every error-message format, but
it has no direct test coverage. This PR adds `StringSubstitutorSuite` to
exercise it on its own.
The suite covers basic substitution, no-placeholder/empty/null input,
non-string values rendered via `toString`, undefined variables (both the
throwing default and the leave-as-is mode), default values (resolved,
defaulted, and empty), substitution that grows the buffer, escaped placeholders
under `preserveEscapes` true/false, nested expansion off/on, cycle detection,
an unclosed placeholder, and an empty default.
While adding the suite I also tidied a small inconsistency in the
default-value branch of `substitute`: it computed `newLength` (the scan length
adjusted for the replacement) but then recursed with the original, unadjusted
`length`, leaving `newLength` dead. The resolved-value branch right above it
already recurses with `newLength`. This makes the two branches symmetric. The
change is behavior-preserving: a default value is always shorter than the
`${name:-default}` placeholder it replaces, so the adjusted length is only ever
smaller, and since `length` serves solely as the upper bound of the scan
(`endPos = offset + length`) both values leave the resumed scan covering the
same region. The suite passes unchanged against the code before and after this
one-line edit.
### Why are the changes needed?
`StringSubstitutor` is on the path of all error-message formatting yet was
only tested indirectly. A dedicated suite documents and locks in its behavior,
including the less-obvious cases (escapes, defaults, nesting, cycles), and the
bookkeeping cleanup removes a confusing dead local.
### Does this PR introduce _any_ user-facing change?
No. The production change is behavior-preserving; the rest is test-only.
### How was this patch tested?
New unit tests. `build/sbt 'common-utils/testOnly *StringSubstitutorSuite'`
passes (13 cases).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
