New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audit: Detect multiline and interpolated strings in formula desc cop #2628

Merged
merged 1 commit into from May 15, 2017

Conversation

Projects
None yet
2 participants
@GauthamGoli
Copy link
Member

GauthamGoli commented May 13, 2017

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew tests with your changes locally?

Fixes #2611
Note: It assumes there is no expression/variables in the multiline string. Ex: '#{var1} is a formula' Although var1 can be substituted it doesn't make sense, as rubocop is a static analyser, so I'm ignoring that part of string .
Also does not support here document , let me know if it's necessary for formula desc

def string_content(node)
node.str_content if node.type == :str
return node.str_content if node.type == :str
node.each_child_node(:str).map(&:str_content).join("") if node.type == :dstr

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid May 14, 2017

Member

Are there any other types of strings beyond these two?

This comment has been minimized.

@GauthamGoli

GauthamGoli May 14, 2017

Member

There is execute-string https://github.com/whitequark/parser/blob/master/doc/AST_FORMAT.md#execute-string
I'm not sure if we should consider that here. Can you please take a look and confirm that?

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid May 15, 2017

Member

Yeh, I think we can ignore that, thanks!

@MikeMcQuaid MikeMcQuaid merged commit 9889b42 into Homebrew:master May 15, 2017

3 checks passed

codecov/patch 100% of diff hit (target 64.42%)
Details
codecov/project 64.42% (+<.01%) compared to 459fef3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented May 15, 2017

Thanks again @GauthamGoli!

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.