Skip to content
This repository has been archived by the owner on Sep 19, 2020. It is now read-only.

Fix FC033 false positives #425

Closed
wants to merge 4 commits into from
Closed

Fix FC033 false positives #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e1c19c
Potentially fix FC033 false positives
tas50 Feb 22, 2016
9fe7a25
Support template sources that are arrays
tas50 Feb 23, 2016
f5f0c9d
Update name and tests
tas50 Feb 23, 2016
7056771
Resolve chefstyle warnings
tas50 Sep 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 19 additions & 22 deletions features/033_check_for_missing_template.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Feature: Check for missing template
Feature: Check for missing template source file(s)

In order to ensure the Chef run is successful
As a developer
Expand All @@ -7,25 +7,22 @@ Feature: Check for missing template
Scenario Outline: Template types
Given a cookbook recipe that <template_type>
When I check the cookbook
Then the missing template warning 033 <warning>
Then the warning 033 should be <warning>
Examples:
| template_type | warning |
| defines a template where both the name and source are complex expressions | should not be displayed |
| defines a template where name and source are both simple expressions | should not be displayed |
| defines a template where name is a complex expression | should not be displayed |
| infers a template with an expression | should not be displayed |
| refers to a hidden template | should not be displayed |
| refers to a local template | should not be displayed |
| refers to a missing template | should be displayed |
| refers to a template in a subdirectory | should not be displayed |
| refers to a template | should not be displayed |
| refers to a template with an expression | should not be displayed |
| refers to a template without an erb extension | should not be displayed |
| uses a missing inferred template | should be displayed |
| uses an inferred template | should not be displayed |
| uses a template from another cookbook | should not be displayed |

Scenario: Template within deploy resource
Given a cookbook recipe with a deploy resource that contains a template resource
When I check the cookbook
Then the missing template warning 033 should not be displayed against the template
| template_type | warning |
| defines a template where both the name and source are complex expressions | valid |
| defines a template where name and source are both simple expressions | valid |
| defines a template where name is a complex expression | valid |
| infers a template with an expression | valid |
| refers to a hidden template | valid |
| refers to a local template | valid |
| refers to a missing template | invalid |
| refers to a template in a subdirectory | valid |
| refers to a template | valid |
| refers to a template with an expression | valid |
| refers to a template without an erb extension | valid |
| uses a missing inferred template | invalid |
| uses an inferred template | valid |
| uses a template from another cookbook | valid |
| includes a deploy resource that contains a template resource | valid |
| includes a template in the root of the templates directory | valid |
23 changes: 21 additions & 2 deletions lib/foodcritic/api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,26 @@ def supported_platforms(ast)
end.sort { |a, b| a[:platform] <=> b[:platform] }
end

# Template filename
# Returns template source filenames if specified or implied by resource name
def template_files(resource)
if resource["source"]
if resource["source"].respond_to?(:xpath) # source is an array
resource["source"].xpath("//array//tstring_content/@value").map { |x| x.to_s }
else # source is a string
[resource["source"]]
end
elsif resource[:name]
if resource[:name].respond_to?(:xpath)
[resource[:name]]
else
["#{File.basename(resource[:name])}.erb"]
end
end
end

# return template source filename
# This is deprecated as it returns a string only which isn't compatible
# with arrays of souce files in Chef 12
def template_file(resource)
if resource["source"]
resource["source"]
Expand Down Expand Up @@ -388,7 +407,7 @@ def templates_included(all_templates, template_path, depth = 1)
end.flatten.uniq.compact
end

# Templates in the current cookbook
# Templates source files in the current cookbook
def template_paths(recipe_path)
Dir.glob(Pathname.new(recipe_path).dirname.dirname + "templates" +
"**/*", File::FNM_DOTMATCH).select do |path|
Expand Down
10 changes: 2 additions & 8 deletions lib/foodcritic/rules.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,14 +498,8 @@ def pry_bindings(ast)
end.reject do |resource|
resource[:file].respond_to?(:xpath)
end.select do |resource|
template_paths(filename).none? do |path|
relative_path = []
Pathname.new(path).ascend do |template_path|
relative_path << template_path.basename
break if template_path.dirname.dirname.basename.to_s == "templates"
end
File.join(relative_path.reverse) == resource[:file]
end
# return true if none of the basenames of the templates match the filename in the resource
! template_paths(filename).any? { |path| File.basename(path) == resource[:file] }
end.map { |resource| resource[:resource] }
end
end
Expand Down
1 change: 1 addition & 0 deletions spec/foodcritic/api_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def parse_ast(str)
:standard_cookbook_subdirs,
:supported_platforms,
:template_file,
:template_files,
:template_paths,
:templates_included,
:valid_query?,
Expand Down