Hex: Handle additional dynamic version reading approaches #4442
Conversation
This file contains 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
landongrindheim
force-pushed
the
handle-more-mix-file-version-variations
branch
2 times, most recently
from
fa48db2
to
c1b6fe4
Compare
landongrindheim
changed the title
I _think_ `#sanitized_content` is meant to make a common Elixir pattern
safe for parsing (rather, to prevent code execution while we dependency
management is being performed). Unfortunately, there are no hints in
either Git commit history or in the pull requests that were opened
around Elixir package management.
The pattern that this is handling (at least in the fixtures in this
repo) is the case where there's a top-level file titled VERSION, which
is read in in the Mixfile's project metadata. We're handling a couple of
cases here:
```elixir
String.trim(File.read("VERSION")) # String.trim("0.0.1")
String.trim(File.read!("VERSION")) # String.trim({:ok, "0.0.1"})
```
A lot of Elixir code relies on piping output from one function to
another, which can cause the above pattern to read as:
```elixir
"VERSION" |> File.read() |> String.trim()
"VERSION"
|> File.read()
|> String.trim()
```
We're not handling these properly, which has led to errors like:
```plaintext
(ArgumentError) cannot pipe "VERSION" into "0.0.1", can only pipe (snip)
```
This commit is meant to capture only the former pattern (nested calls),
so that we can properly handle the latter in upcoming commits.
Until this commit, we've only handled the following syntax:
```elixir
String.trim(File.read("VERSION"))
String.trim(File.read!("VERSION"))
```
However, we've encountered users using the following idioms:
```elixir
"VERSION" |> File.read() |> String.trim()
"VERSION" |> File.read!() |> String.trim()
```
and:
```elixir
"VERSION"
|> File.read()
|> String.trim()
"VERSION"
|> File.read!()
|> String.trim()
```
This commit allows us to handle the latter cases, where the pipe
operator is used.
landongrindheim
force-pushed
the
handle-more-mix-file-version-variations
branch
from
c72588d
to
fb042c5
Compare
xlgmokha
approved these changes
Nov 29, 2021
| @@ -11,16 +11,40 @@ def initialize(mixfile_content:) | |||
| @mixfile_content = mixfile_content | |||
| end | |||
|
|
|||
| FILE_READ = /File.read\(.*?\)/.freeze | |||
| FILE_READ_BANG = /File.read!\(.*?\)/.freeze | |||
There was a problem hiding this comment.
question (non-blocking): it looks like we're trying to match File.read. Do we also need to match IO.read and/or Pathname#read?
There was a problem hiding this comment.
We shouldn't. These are matching Elixir functions what share a name with Ruby methods. They do have a very familiar feel as the language's creator came out of the Ruby community 😄
…version-variations
Merged
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.
I think
#sanitized_contentis meant to make a common Elixir patternsafe for parsing (rather, to prevent code execution while we dependency
management is being performed). Unfortunately, there are no hints in
either Git commit history or in the pull requests that were opened
around Elixir package management.
The pattern that this is handling (at least in the fixtures in this
repo) is the case where there's a top-level file titled VERSION, which
is read in in the Mixfile's project metadata. We're handling a couple of
cases here:
A lot of Elixir code relies on piping output from one function to
another, which can cause the above pattern to read as:
We've not been handling these pipelines properly, which has led to errors like:
This pull request is meant to enable us to handle both the nested and piped approaches.