Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

FC007 should not be triggered by include_recipe "#{cookbook_name}::blah" #44

Closed
markjreed opened this Issue · 6 comments

2 participants

@markjreed

To include another recipe from the current cookbook, I always use this syntax: include_recipe "#{cookbook_name}::other", to avoid hard-coding the cookbook name in the recipe code. This incorrectly triggers an FC007.

@acrmp
Owner

Hi Mark,

Thanks for raising this - I can confirm the bug and will have a look into it this evening.

Cheers,

Andrew.

@markjreed

It looks like the included recipe name turns into just "::recipe" by the time the ast is built, so grepping out '^::' works, but I don't understand why that is the case.

@acrmp
Owner

The included_recipes method that FC007 uses only recognises literal strings - so the behaviour of the rule should be to ignore any use of include_recipe that includes an embedded expression.
https://github.com/acrmp/foodcritic/blob/v1.3.0/lib/foodcritic/api.rb#L143

There is a test to check that FC007 doesn't wrongly warn on embedded expressions but the scenario is testing with a dynamic recipe_name rather than the cookbook_name.
https://github.com/acrmp/foodcritic/blob/v1.3.0/features/007_check_for_undeclared_recipe_dependencies.feature#L22-25
https://github.com/acrmp/foodcritic/blob/v1.3.0/features/step_definitions/cookbook_steps.rb#L253-267

It would be good to have scenarios to test:

  • "#{cookbook_name}::other"
  • "foo_#{cookbook_name}::other"

We could optionally add specific support for resolving cookbook_name too which might be nice.

If you feel like having a go at making these changes please let me know.

@acrmp
Owner

Hi Mark,

The fix for this was released in foodcritic 1.4.0. Can you test and let me know if this is still a problem?

Thanks,

Andrew.

@markjreed
@acrmp
Owner

Thank you sir.

@acrmp acrmp closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.