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

Commit

Permalink
Add ability to choose rules applied, refs #4.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Andrew Crump committed Dec 12, 2011
1 parent c735c3b commit 3836620
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 11 deletions.
9 changes: 5 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
17 changes: 14 additions & 3 deletions bin/foodcritic
Original file line number Diff line number Diff line change
@@ -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?

review = FoodCritic::Linter.new.check(ARGV[0], options)
puts review unless review.warnings.empty?
25 changes: 25 additions & 0 deletions features/command_line_help.feature
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions features/step_definitions/command_line_steps.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions foodcritic.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
3 changes: 2 additions & 1 deletion lib/foodcritic/domain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@ 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
#
# @param [String] code The short unique identifier for this rule, e.g. 'FC001'
# @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

Expand Down
7 changes: 7 additions & 0 deletions lib/foodcritic/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 9 additions & 3 deletions lib/foodcritic/linter.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'ripper'
require 'gherkin/tag_expression'

module FoodCritic

Expand All @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions lib/foodcritic/rules.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -7,13 +8,15 @@
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)}
end
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|
Expand All @@ -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 = []
Expand All @@ -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]/
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 = []
Expand All @@ -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
Expand Down

0 comments on commit 3836620

Please sign in to comment.