Fix truncated environment variable expansions#662
Merged
srchase merged 1 commit intosmithy-lang:masterfrom Dec 8, 2020
Merged
Conversation
mtdowling
approved these changes
Dec 8, 2020
| String replacement = expand(node.getSourceLocation(), variable); | ||
| // INLINE over-matches to allow for escaping. If the over-matched first group does not start with | ||
| // '${', we need to prepend the first character from that group on the replacement. | ||
| if (!matcher.group(0).startsWith("${")) { |
Member
There was a problem hiding this comment.
This works, but it would probably be easier to understand if this used https://github.com/awslabs/smithy/blob/master/smithy-utils/src/main/java/software/amazon/smithy/utils/SimpleParser.java. I think we can merge this for now to fix the bug, and then go back and improve this when we have time.
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
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.
Any environment variable expansions in smithy-build.json that did not start at the beginning of the field resulted in dropping the character that preceded the ${NAME} placeholder.
Consider the following
smithy-build.jsonexample, with the environment variable ofFOOwas set toBAZ:Currently, this results in the shapes with the tag
BABAZbeing included, instead ofBARBAZas expected.This CR corrects the expansion behavior to assure preceding characters are included appropriately.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.