Keep fixture annotations out of ERB blocks (#345)#353
Open
Halvanhelv wants to merge 1 commit into
Open
Conversation
ERB fixtures cannot be parsed by Psych, so the parser used to evaluate the ERB and read Psych line numbers off the *evaluated* output. Those line numbers were then used as indices into the *original* file. When an ERB tag spans multiple lines the offsets no longer line up, so the schema annotation was written inside an ERB tag (e.g. between `<%` and `%>`, or inside a `<% ... do %>` loop). Evaluating the ERB also runs arbitrary code, which could fail outright. Instead, derive the content bounds straight from the original lines: the first non-blank, non-comment line is the start, so annotations land above the ERB body (and after any leading comments), never inside a tag. Fixes drwl#345
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.
Summary
Fixes #345.
Schema annotations were being written inside ERB tags in dynamic fixture files. For example, given a fixture that starts with a multi-line ERB block:
running
annotaterb modelsproduced:i.e. the annotation landed in the middle of the ERB block, corrupting the file.
Root cause
ERB fixtures cannot be parsed by Psych, so
YmlParserevaluated the ERB (ERB.new(input).result) and read Psych's line numbers off the evaluated output. Those line numbers were then used as indices into the original file.When an ERB tag spans multiple lines, the evaluated output has a different line count than the source, so the offsets no longer line up and the annotation is inserted at the wrong position — inside an ERB tag, or inside a
<% ... do %>loop. Evaluating the ERB also executes arbitrary Ruby, which can fail outright.This affected every ERB fixture, not just files starting with
<%— the existing<% 1.upto(100) do |i| %>case also had its annotation written inside the loop.Fix
Stop evaluating the ERB. Instead derive the content bounds directly from the original lines: the first non-blank, non-comment line is the start, so annotations are written above the ERB body (and after any leading comments), never inside a tag. The result is also idempotent across repeated runs.
Tests
YmlParserspec for a fixture that starts with a multi-line ERB block.Generatorspecs covering an ERB fixture that starts with<%and one preceded by a comment.YmlParserandSingleFileAnnotatorwhose expectations encoded the buggy placement.Verified these specs fail without the fix and pass with it. Full unit suite is green.