Replace regex-based interpolation with character scanner#4747
Open
shreyas-goenka wants to merge 14 commits intomainfrom
Open
Replace regex-based interpolation with character scanner#4747shreyas-goenka wants to merge 14 commits intomainfrom
shreyas-goenka wants to merge 14 commits intomainfrom
Conversation
- Move interpolation parser to libs/interpolation/ (independent of dyn)
- Use \$ -> $ and \\ -> \ escape sequences (Bash convention)
- Reject nested ${} constructs as errors
- Replace strings.Replace interpolation with token-based concatenation
- Add WarnMalformedReferences mutator for early parse error warnings
- Add NewRefWithDiagnostics for surfacing parse errors as diagnostics
Co-authored-by: Isaac
5 tasks
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: 033b1a2
17 interesting tests: 10 SKIP, 7 KNOWN
Top 20 slowest tests (at least 2 minutes):
|
The var_in_var test used ${var.foo_${var.tail}} which relied on
undocumented nested reference behavior (the test itself noted:
"not officially supported... might start to reject this in the future").
The new parser now rejects nested ${} constructs as intended.
Co-authored-by: Isaac
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Add acceptance tests for escape sequences, unterminated refs, empty refs, leading digit keys, and dollar-before-non-brace. Consolidate standalone unit tests into table-driven TestParse and TestParseErrors. Add breaking change entry to NEXT_CHANGELOG for nested variable reference rejection. Co-authored-by: Isaac
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
…errors Add ParseError type to interpolation package so callers can access the position offset separately from the message. Update WarnMalformedReferences to include the config path (e.g. bundle.name) in diagnostics and incorporate the position into the column location. Remove redundant Validate method. Co-authored-by: Isaac
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
force-pushed
the
interpolation-parser-pr1
branch
from
ea33af9 to
3ac5b5d
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
The parser now treats outer ${...} prefixes as literal text when a
nested ${ is encountered, allowing inner references to be resolved
first. Multi-round resolution progressively resolves from inside out.
Co-authored-by: Isaac
shreyas-goenka
force-pushed
the
interpolation-parser-pr1
branch
from
3ac5b5d to
fdba4f1
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
force-pushed
the
interpolation-parser-pr1
branch
from
74ff2e2 to
7bf72e3
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Shared test cases in libs/interpolation/testdata/variable_references.json are consumed by both the Go parser (TestParsePureVariableReferences) and the Python regex (test_pure_variable_reference) to verify they agree on which strings are pure variable references. When modifying the parser, add test cases to the JSON file so both languages are validated. Co-authored-by: Isaac
shreyas-goenka
force-pushed
the
interpolation-parser-pr1
branch
from
7bf72e3 to
8cddafa
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Escape sequences (\$ and \\) are incompatible with multi-round variable
resolution: escapes consumed in round N produce bare ${...} text that
round N+1 incorrectly resolves as a real variable reference. Remove
escape support entirely and defer to a follow-up if needed.
Co-authored-by: Isaac
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Co-authored-by: Isaac
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
Author
|
Note: As a followup we'll also add the ability to escape variable references in the syntax. This work enables us to add fancier syntax for interpolation in DABs and unblocks supporting variable interpolation for the script section in DABs. |
shreyas-goenka
requested review from
denik,
pietern and
simonfaltum
as code owners
Contributor
Approval status: pending
|
# Conflicts: # bundle/phases/initialize.go # libs/dyn/dynvar/resolve.go
Co-authored-by: Isaac
Co-authored-by: Isaac
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
- Remove unused NewRefWithDiagnostics public API and inline newRef into NewRef - Drop tests for the removed function - Use read-only dyn.Walk on b.Config.Value() in WarnMalformedReferences (matches the sibling CollectEscapeTelemetry mutator) Co-authored-by: Isaac
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the regex-based variable reference parser with a character scanner. No change in interpolation semantics — all existing configurations produce identical results.
libs/interpolation/with structuredParseErrorfor precise diagnosticsWarnMalformedReferencesmutator emits warnings for invalid references like${foo.bar-}testdata/variable_references.json) keep Go parser and Python regex in syncTest plan
go test ./libs/interpolation/..../libs/dyn/dynvar/..../bundle/config/mutator/...var_in_var,var_in_var_3level,malformed_reference,bad_syntax,unterminated_ref,empty_ref,leading_digit_ref,dollar_before_non_braceThis pull request was AI-assisted by Isaac.