FC030: Cookbook contains breakpoints, refs #36.

features/030_check_for_debugger_breakpoints.feature

@@ -0,0 +1,25 @@
+Feature: Check for debugger breakpoints
+
+  In order to avoid halting a converge
+  As a developer
+  I want to identify debugger breakpoints that have not been removed
+
+  Scenario Outline: Debugger breakpoints
+    Given a cookbook with a <component> that <includes> a breakpoint
+    When I check the cookbook
+    Then the debugger breakpoint warning 030 should be <show_warning> against the <component>
+
+  Examples:
+    | component | includes         | show_warning |
+    | library   | does not include | not shown    |
+    | library   | includes         | shown        |
+    | metadata  | does not include | not shown    |
+    | metadata  | includes         | shown        |
+    | provider  | does not include | not shown    |
+    | provider  | includes         | shown        |
+    | recipe    | does not include | not shown    |
+    | recipe    | includes         | shown        |
+    | resource  | does not include | not shown    |
+    | resource  | includes         | shown        |
+    | template  | does not include | not shown    |
+    | template  | includes         | shown        |

features/step_definitions/cookbook_steps.rb

@@ -543,6 +543,24 @@
   }
 end
 
+Given /^a cookbook with a ([^ ]+) that (includes|does not include) a breakpoint$/ do |component,includes|
+  content = case component
+    when 'template' then includes == 'includes' ? "Hello <% require 'pry'; binding.pry %>" : 'Hello World'
+    else includes == 'includes' ? 'binding.pry' : '# No breakpoint'
+  end
+  write_recipe ''
+  case component
+    when 'library' then write_library 'foo', content
+    when 'metadata' then write_metadata content
+    when 'provider' then write_provider 'foo', content
+    when 'recipe' then write_recipe content
+    when 'resource' then write_resource 'foo', content
+    when 'template' then write_file 'cookbooks/example/templates/default/foo.erb',
+      content
+    else fail "Unrecognised component: #{component}"
+  end
+end
+
 Given /^a cookbook with a single recipe for which the first hash (key|value) is an interpolated string$/ do |key_or_value|
   write_recipe case key_or_value
     when 'key' then %q{{"#{foo}" => 'bar', 'bar' => 'foo'}}
@@ -986,6 +1004,18 @@ def search(bag_name, query=nil, sort=nil, start=0, rows=1000, &block)
   expect_output("foodcritic #{FoodCritic::VERSION}")
 end
 
+Then /^the debugger breakpoint warning 030 should be (not )?shown against the (.*)$/ do |should_not, component|
+  filename = case component
+    when 'library' then 'libraries/foo.rb'
+    when 'metadata' then 'metadata.rb'
+    when 'provider' then 'providers/foo.rb'
+    when 'recipe' then 'recipes/default.rb'
+    when 'resource' then 'resources/foo.rb'
+    when 'template' then 'templates/default/foo.erb'
+  end
+  expect_warning('FC030', :line => nil, :expect_warning => should_not.nil?, :file => filename)
+end
+
 Then /^the file mode warning 006 should be (valid|invalid)$/ do |valid|
   valid == 'valid' ? expect_no_warning('FC006') : expect_warning('FC006')
 end

features/support/command_helpers.rb

@@ -36,6 +36,7 @@ module CommandHelpers
       'FC027' => 'Resource sets internal attribute',
       'FC028' => 'Incorrect #platform? usage',
       'FC029' => 'No leading cookbook name in recipe metadata',
+      'FC030' => 'Cookbook contains debugger breakpoints',
       'FCTEST001' => 'Test Rule'
     }
 

lib/foodcritic/linter.rb

@@ -69,7 +69,7 @@ def check(cookbook_paths, options)
             rule_matches += matches(rule.provider, ast, file)
           end
           if File.basename(File.dirname(file)) == 'resources'
-          rule_matches += matches(rule.resource, ast, file)
+            rule_matches += matches(rule.resource, ast, file)
           end
           if last_dir != cookbook_dir
             rule_matches += matches(rule.cookbook, cookbook_dir)

lib/foodcritic/rules.rb

@@ -414,3 +414,23 @@
     end.compact.map {|m| match(m).merge(:filename => metadata_path.to_s) }
   end
 end
+
+rule "FC030", "Cookbook contains debugger breakpoints" do
+  tags %w{annoyances}
+  cookbook do |cookbook_dir|
+    Dir[cookbook_dir + '**/*.rb'].map do |ruby_file|
+      read_ast(ruby_file).xpath('//call[(vcall|var_ref)/ident/@value="binding"]
+        [ident/@value="pry"]').map do |bp|
+        match(bp).merge({:filename => ruby_file})
+      end
+    end +
+    Dir[cookbook_dir + 'templates/**/*.erb'].map do |template_file|
+      IO.read(template_file).lines.with_index(1).map do |line, line_number|
+        # Not properly parsing the template
+        if line =~ /binding\.pry/
+          {:filename => template_file, :line => line_number}
+        end
+      end.compact
+    end
+  end
+end