From 383662055b5a4473e36bbaabc9fd1dec5953c9af Mon Sep 17 00:00:00 2001 From: Andrew Crump Date: Mon, 12 Dec 2011 23:33:40 +0000 Subject: [PATCH] Add ability to choose rules applied, refs #4. The DSL now accepts a tags array to define the set of tags available to choose whether or not to apply the rule. This is lifted straight from Cucumber tag expressions. --- Gemfile.lock | 9 +++--- bin/foodcritic | 17 +++++++++-- features/command_line_help.feature | 25 ++++++++++++++++ .../step_definitions/command_line_steps.rb | 29 +++++++++++++++++++ foodcritic.gemspec | 1 + lib/foodcritic/domain.rb | 3 +- lib/foodcritic/dsl.rb | 7 +++++ lib/foodcritic/linter.rb | 12 ++++++-- lib/foodcritic/rules.rb | 9 ++++++ 9 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 features/command_line_help.feature create mode 100644 features/step_definitions/command_line_steps.rb diff --git a/Gemfile.lock b/Gemfile.lock index 8d834ef5..4b9e5844 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,12 +3,13 @@ PATH specs: foodcritic (0.4.0) chef (~> 0.10.4) + gherkin (~> 2.7.1) nokogiri (~> 1.5.0) GEM remote: http://rubygems.org/ specs: - aruba (0.4.8) + aruba (0.4.9) childprocess (>= 0.2.3) cucumber (>= 1.1.1) ffi (= 1.0.11) @@ -33,16 +34,16 @@ GEM uuidtools childprocess (0.2.3) ffi (~> 1.0.6) - cucumber (1.1.3) + cucumber (1.1.4) builder (>= 2.1.2) diff-lcs (>= 1.1.2) - gherkin (~> 2.6.7) + gherkin (~> 2.7.1) json (>= 1.4.6) term-ansicolor (>= 1.0.6) diff-lcs (1.1.3) erubis (2.7.0) ffi (1.0.11) - gherkin (2.6.9) + gherkin (2.7.1) json (>= 1.4.6) guard (0.8.8) thor (~> 0.14.6) diff --git a/bin/foodcritic b/bin/foodcritic index c2a0dddb..2021239b 100755 --- a/bin/foodcritic +++ b/bin/foodcritic @@ -1,8 +1,19 @@ #!/usr/bin/env ruby require 'foodcritic' +require 'optparse' + +options = {} +options[:tags] = [] +parser = OptionParser.new do |opts| + opts.banner = 'foodcritic [cookbook_path]' + opts.on("-t", "--tags TAGS", "Only check against rules with the specified tags.") {|t|options[:tags] << t} +end +parser.parse! + unless ARGV.length == 1 and Dir.exists?(ARGV[0]) - STDERR.puts 'foodcritic [cookbook_path]' + puts parser.help exit 1 end -review = FoodCritic::Linter.new.check(ARGV[0]) -puts review unless review.warnings.empty? \ No newline at end of file + +review = FoodCritic::Linter.new.check(ARGV[0], options) +puts review unless review.warnings.empty? diff --git a/features/command_line_help.feature b/features/command_line_help.feature new file mode 100644 index 00000000..488b5a14 --- /dev/null +++ b/features/command_line_help.feature @@ -0,0 +1,25 @@ +Feature: Command line help + + In order to be able to learn about the options available for checking my cookbooks + As a developer + I want to be able to interactively get help on the options from the command line + + Scenario: No arguments + Given I have installed the lint tool + When I run it on the command line with no arguments + Then the simple usage text should be displayed along with a non-zero exit code + + Scenario: Too many arguments + Given I have installed the lint tool + When I run it on the command line with too many arguments + Then the simple usage text should be displayed along with a non-zero exit code + + Scenario: Non-existent cookbook directory + Given I have installed the lint tool + When I run it on the command line specifying a cookbook that does not exist + Then the simple usage text should be displayed along with a non-zero exit code + + Scenario: Command help + Given I have installed the lint tool + When I run it on the command line with the help option + Then the simple usage text should be displayed along with a zero exit code \ No newline at end of file diff --git a/features/step_definitions/command_line_steps.rb b/features/step_definitions/command_line_steps.rb new file mode 100644 index 00000000..8acec291 --- /dev/null +++ b/features/step_definitions/command_line_steps.rb @@ -0,0 +1,29 @@ +Given /^I have installed the lint tool$/ do + +end + +When /^I run it on the command line with no arguments$/ do + run_simple('foodcritic', false) +end + +When /^I run it on the command line with too many arguments$/ do + run_simple('foodcritic example example', false) +end + +When /^I run it on the command line specifying a cookbook that does not exist$/ do + run_simple('foodcritic no-such-cookbook', false) +end + +When /^I run it on the command line with the help option$/ do + run_simple('foodcritic --help', false) +end + +Then /^the simple usage text should be displayed along with a (non-)?zero exit code$/ do |non_zero| + assert_partial_output 'foodcritic [cookbook_path]', all_output + assert_matching_output('( )+-t, --tags TAGS( )+Only check against rules with the specified tags.', all_output) + if non_zero.nil? + assert_exit_status 0 + else + assert_not_exit_status 0 + end +end \ No newline at end of file diff --git a/foodcritic.gemspec b/foodcritic.gemspec index 1bdbfbb8..9bd66509 100644 --- a/foodcritic.gemspec +++ b/foodcritic.gemspec @@ -11,6 +11,7 @@ Gem::Specification.new do |s| s.license = 'MIT' s.executables << 'foodcritic' s.add_dependency('chef', '~> 0.10.4') + s.add_dependency('gherkin', '~> 2.7.1') s.add_dependency('nokogiri', '~> 1.5.0') s.files = Dir['lib/**/*.rb'] s.required_ruby_version = '>= 1.9.3' diff --git a/lib/foodcritic/domain.rb b/lib/foodcritic/domain.rb index 79e2eaec..3b0e0d88 100644 --- a/lib/foodcritic/domain.rb +++ b/lib/foodcritic/domain.rb @@ -38,7 +38,7 @@ def to_s # A rule to be matched against. class Rule - attr_accessor :code, :name, :description, :recipe + attr_accessor :code, :name, :description, :recipe, :tags # Create a new rule # @@ -46,6 +46,7 @@ class Rule # @param [String] name The short descriptive name of this rule presented to the end user. def initialize(code, name) @code, @name = code, name + @tags = [code] end end diff --git a/lib/foodcritic/dsl.rb b/lib/foodcritic/dsl.rb index 27ab6928..fdd6b067 100644 --- a/lib/foodcritic/dsl.rb +++ b/lib/foodcritic/dsl.rb @@ -19,6 +19,13 @@ def rule(code, name, &block) yield self end + # Add tags to the rule which can be used to filter the rules to be applied. + # + # @param [Array] tags The tags associated with this rule. + def tags(tags) + rules.last.tags += tags + end + # Set the rule description # # @param [String] description Set the rule description. diff --git a/lib/foodcritic/linter.rb b/lib/foodcritic/linter.rb index a4b9bb66..bdfa468b 100644 --- a/lib/foodcritic/linter.rb +++ b/lib/foodcritic/linter.rb @@ -1,4 +1,5 @@ require 'ripper' +require 'gherkin/tag_expression' module FoodCritic @@ -15,14 +16,19 @@ def initialize # Review the cookbooks at the provided path, identifying potential improvements. # # @param [String] cookbook_path The file path to an individual cookbook directory + # @param [Hash] options Options to apply to the linting + # @option options [Array] tags The tags to filter rules based on # @return [FoodCritic::Review] A review of your cookbooks, with any warnings issued. - def check(cookbook_path) + def check(cookbook_path, options) warnings = [] + tag_expr = Gherkin::TagExpression.new(options[:tags]) files_to_process(cookbook_path).each do |file| ast = read_file(file) @rules.each do |rule| - matches = rule.recipe.yield(ast, file) - matches.each{|match| warnings << Warning.new(rule, {:filename => file}.merge(match))} unless matches.nil? + if tag_expr.eval(rule.tags) + matches = rule.recipe.yield(ast, file) + matches.each{|match| warnings << Warning.new(rule, {:filename => file}.merge(match))} unless matches.nil? + end end end Review.new(warnings) diff --git a/lib/foodcritic/rules.rb b/lib/foodcritic/rules.rb index 26847b1e..9260637c 100644 --- a/lib/foodcritic/rules.rb +++ b/lib/foodcritic/rules.rb @@ -1,4 +1,5 @@ rule "FC002", "Avoid string interpolation where not required" do + tags %w{style} description "When setting a resource value avoid string interpolation where not required." recipe do |ast| ast.xpath(%q{//string_literal[count(descendant::string_embexpr) = 1 and @@ -7,6 +8,7 @@ end rule "FC003", "Check whether you are running with chef server before using server-specific features" do + tags %w{portability solo} description "Ideally your cookbooks should be usable without requiring chef server." recipe do |ast| checks_for_chef_solo?(ast) ? [] : searches(ast).map{|s| match(s)} @@ -14,6 +16,7 @@ end rule "FC004", "Use a service resource to start and stop services" do + tags %w{style} description "Avoid use of execute to control services - use the service resource instead." recipe do |ast| find_resources(ast, 'execute').find_all do |cmd| @@ -24,6 +27,7 @@ end rule "FC005", "Avoid repetition of resource declarations" do + tags %w{style} description "Where you have a lot of resources that vary in only a single attribute wrap them in a loop for brevity." recipe do |ast| matches = [] @@ -39,6 +43,7 @@ end rule "FC006", "Mode should be quoted or fully specified when setting file permissions" do + tags %w{correctness} description "Not quoting mode when setting permissions can lead to incorrect permissions being set." recipe do |ast| ast.xpath(%q{//ident[@value='mode']/parent::command/descendant::int[string-length(@value) < 4]/ @@ -47,6 +52,7 @@ end rule "FC007", "Ensure recipe dependencies are reflected in cookbook metadata" do + tags %w{correctness metadata} description "You are including a recipe that is not in the current cookbook and not defined as a dependency in your cookbook metadata." recipe do |ast,filename| metadata_path = Pathname.new(File.join(File.dirname(filename), '..', 'metadata.rb')).cleanpath @@ -60,6 +66,7 @@ end rule "FC008", "Generated cookbook metadata needs updating" do + tags %w{style metadata} description "The cookbook metadata for this cookbook is boilerplate output from knife generate cookbook and needs updating with the real details of your cookbook." recipe do |ast,filename| metadata_path = Pathname.new(File.join(File.dirname(filename), '..', 'metadata.rb')).cleanpath @@ -74,6 +81,7 @@ end rule "FC009", "Resource attribute not recognised" do + tags %w{correctness} description "You appear to be using an unrecognised attribute on a standard Chef resource. Please check for typos." recipe do |ast| matches = [] @@ -93,6 +101,7 @@ end rule "FC010", "Invalid search syntax" do + tags %w{correctness search} description "The search expression in the recipe could not be parsed. Please check your syntax." recipe do |ast| # This only works for literal search strings