Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Converted to ruby_parser instead of ParseTree.

Got tests to pass again.
  • Loading branch information...
commit db0c64dcb171b59e45ac4abf90162e32f1cd9f98 1 parent 7f37f2a
Marty Andrews martinjandrews authored
Showing with 141 additions and 72 deletions.
  1. +4 −0 History.txt
  2. +1 −1  lib/roodi.rb
  3. +5 −5 lib/roodi/checks/check.rb
  4. +1 −2  lib/roodi/checks/control_coupling_check.rb
  5. +2 −2 lib/roodi/checks/empty_rescue_body_check.rb
  6. +1 −4 lib/roodi/checks/line_count_check.rb
  7. +1 −1  lib/roodi/checks/parameter_number_check.rb
  8. +1 −0  lib/roodi/core.rb
  9. +1 −2  lib/roodi/core/checking_visitor.rb
  10. +1 −1  lib/roodi/core/iterator_visitor.rb
  11. +2 −2 lib/roodi/core/parse_tree_runner.rb
  12. +4 −7 lib/roodi/core/{parser.rb → ruby_parser_parser.rb}
  13. +78 −0 lib/roodi/core/ruby_parser_runner.rb
  14. +15 −19 lib/roodi/core/visitable_sexp.rb
  15. +1 −1  lib/roodi_task.rb
  16. +1 −1  spec/roodi/checks/abc_metric_method_check_spec.rb
  17. +2 −4 spec/roodi/checks/assignment_in_conditional_check_spec.rb
  18. +2 −2 spec/roodi/checks/case_missing_else_check_spec.rb
  19. +1 −1  spec/roodi/checks/class_line_count_check_spec.rb
  20. +1 −1  spec/roodi/checks/class_name_check_spec.rb
  21. +2 −2 spec/roodi/checks/class_variable_check_spec.rb
  22. +2 −2 spec/roodi/checks/control_coupling_check_spec.rb
  23. +2 −2 spec/roodi/checks/cyclomatic_complexity_block_check_spec.rb
  24. +1 −1  spec/roodi/checks/cyclomatic_complexity_method_check_spec.rb
  25. +3 −3 spec/roodi/checks/empty_rescue_body_check_spec.rb
  26. +1 −1  spec/roodi/checks/for_loop_check_spec.rb
  27. +1 −1  spec/roodi/checks/method_line_count_check_spec.rb
  28. +1 −1  spec/roodi/checks/method_name_check_spec.rb
  29. +1 −1  spec/roodi/checks/module_line_count_check_spec.rb
  30. +1 −1  spec/roodi/checks/module_name_check_spec.rb
  31. +1 −1  spec/roodi/checks/parameter_number_check_spec.rb
4 History.txt
View
@@ -1,3 +1,7 @@
+= 1.3.7
+
+* Fixed a bug in the rake task where it always failed even if no errors existed.
+
= 1.3.6
* Added nil as a valid response for an empty rescue block
2  lib/roodi.rb
View
@@ -2,5 +2,5 @@
require 'roodi/core'
module Roodi
- VERSION = '1.3.6'
+ VERSION = '1.3.7'
end
10 lib/roodi/checks/check.rb
View
@@ -10,16 +10,16 @@ def initialize
def position(offset = 0)
"#{@line[2]}:#{@line[1] + offset}"
end
-
- def evaluate_node_at_line(node, line)
- @line = line
+
+ def evaluate_node(node)
+ @node = node
eval_method = "evaluate_#{node.node_type}"
self.send(eval_method, node) if self.respond_to? eval_method
evaluate(node) if self.respond_to? :evaluate
end
- def add_error(error, offset = 0)
- @errors << Roodi::Core::Error.new("#{@line[2]}", "#{@line[1] + offset}", error)
+ def add_error(error)
+ @errors << Roodi::Core::Error.new("#{@node.file}", "#{@node.line}", error)
end
def errors
3  lib/roodi/checks/control_coupling_check.rb
View
@@ -9,8 +9,7 @@ def interesting_nodes
def evaluate_defn(node)
@method_name = node[1]
- @arguments = node[2][1][1]
- @arguments.delete_at 0
+ @arguments = node[2][1..-1]
end
def evaluate_lvar(node)
4 lib/roodi/checks/empty_rescue_body_check.rb
View
@@ -7,14 +7,14 @@ module Checks
# When the body of a rescue block is empty, exceptions can get caught and swallowed without
# any feedback to the user.
class EmptyRescueBodyCheck < Check
- STATEMENT_NODES = [:fcall, :return, :attrasgn, :vcall, :nil]
+ STATEMENT_NODES = [:fcall, :return, :attrasgn, :vcall, :nil, :call]
def interesting_nodes
[:resbody]
end
def evaluate(node)
- add_error("Rescue block should not be empty.", 1) unless has_statement?(node)
+ add_error("Rescue block should not be empty.") unless has_statement?(node)
end
private
5 lib/roodi/checks/line_count_check.rb
View
@@ -22,10 +22,7 @@ def evaluate(node)
protected
def count_lines(node)
- count = 0
- count = count + 1 if node.node_type == :newline
- node.children.each {|node| count += count_lines(node)}
- count
+ node.last.line - 2
end
end
end
2  lib/roodi/checks/parameter_number_check.rb
View
@@ -21,7 +21,7 @@ def interesting_nodes
def evaluate(node)
method_name = node[1]
- arguments = node[2][1][1]
+ arguments = node[2]
parameter_count = arguments.inject(-1) { |count, each| count = count + (each.class == Symbol ? 1 : 0) }
add_error "Method name \"#{method_name}\" has #{parameter_count} parameters. It should have #{@parameter_count} or less." unless parameter_count <= @parameter_count
end
1  lib/roodi/core.rb
View
@@ -1 +1,2 @@
require 'roodi/core/parse_tree_runner'
+require 'roodi/core/ruby_parser_runner'
3  lib/roodi/core/checking_visitor.rb
View
@@ -14,9 +14,8 @@ def initialize(*checks)
end
def visit(node)
- @last_newline = node if node.node_type == :newline
checks = @checks[node.node_type]
- checks.each {|check| check.evaluate_node_at_line(node, @last_newline)} unless checks.nil?
+ checks.each {|check| check.evaluate_node(node)} unless checks.nil?
nil
end
end
2  lib/roodi/core/iterator_visitor.rb
View
@@ -9,7 +9,7 @@ def visit(visited)
visited.accept(@payload)
visitable_nodes = visited.is_language_node? ? visited.sexp_body : visited
visitable_nodes.each do |child|
- if child.class == VisitableSexp then
+ if child.class == Sexp then
child.accept(self)
end
end
4 lib/roodi/core/parse_tree_runner.rb
View
@@ -3,7 +3,7 @@
require 'roodi/core/checking_visitor'
require 'roodi/core/iterator_visitor'
-require 'roodi/core/parser'
+require 'roodi/core/parse_tree_parser'
require 'roodi/core/visitable_sexp'
module Roodi
@@ -16,7 +16,7 @@ class ParseTreeRunner
def initialize(*checks)
@config = DEFAULT_CONFIG
@checks = checks unless checks.empty?
- @parser = Parser.new
+ @parser = ParseTreeParser.new
end
def check(filename, content)
11 lib/roodi/core/parser.rb → lib/roodi/core/ruby_parser_parser.rb
View
@@ -1,11 +1,11 @@
require 'rubygems'
-require 'parse_tree'
+require 'ruby_parser'
require 'facets'
module Roodi
module Core
- class Parser
+ class RubyParserParser
def parse(content, filename)
silence_stream(STDERR) do
return silent_parse(content, filename)
@@ -15,11 +15,8 @@ def parse(content, filename)
private
def silent_parse(content, filename)
- @parser ||= ParseTree.new(true)
- node = @parser.parse_tree_for_string(content, filename)
- sexp = VisitableSexp.from_array node
- sexp.filename = filename
- sexp
+ @parser ||= RubyParser.new
+ @parser.parse(content, filename)
end
end
end
78 lib/roodi/core/ruby_parser_runner.rb
View
@@ -0,0 +1,78 @@
+require 'pp'
+require 'yaml'
+
+require 'roodi/core/checking_visitor'
+require 'roodi/core/iterator_visitor'
+require 'roodi/core/ruby_parser_parser'
+require 'roodi/core/visitable_sexp'
+
+module Roodi
+ module Core
+ class RubyParserRunner
+ DEFAULT_CONFIG = File.join(File.dirname(__FILE__), "..", "..", "..", "roodi.yml")
+
+ attr_writer :config
+
+ def initialize(*checks)
+ @config = DEFAULT_CONFIG
+ @checks = checks unless checks.empty?
+ @parser = RubyParserParser.new
+ end
+
+ def check(filename, content)
+ @checks ||= load_checks
+ node = parse(filename, content)
+ node.accept(IteratorVisitor.new(CheckingVisitor.new(@checks))) if node
+ end
+
+ def check_content(content)
+ check("dummy-file.rb", content)
+ end
+
+ def check_file(filename)
+ check(filename, File.read(filename))
+ end
+
+ def print(filename, content)
+ node = @parser.parse(content, filename)
+ puts "Line: #{node.line}"
+ pp node
+ end
+
+ def print_content(content)
+ print("dummy-file.rb", content)
+ end
+
+ def print_file(filename)
+ print(filename, File.read(filename))
+ end
+
+ def errors
+ @checks ||= []
+ all_errors = @checks.collect {|check| check.errors}
+ all_errors.flatten
+ end
+
+ private
+
+ def parse(filename, content)
+ begin
+ @parser.parse(content, filename)
+ rescue Exception => e
+ puts "#{filename} looks like it's not a valid Ruby file. Skipping..." if ENV["ROODI_DEBUG"]
+ nil
+ end
+ end
+
+ def load_checks
+ check_objects = []
+ checks = YAML.load_file @config
+ checks.each do |check|
+ klass = eval("Roodi::Checks::#{check[0]}")
+ check_objects << (check[1].empty? ? klass.new : klass.new(check[1]))
+ end
+ check_objects
+ end
+ end
+ end
+end
34 lib/roodi/core/visitable_sexp.rb
View
@@ -1,24 +1,20 @@
require 'rubygems'
require 'sexp'
-module Roodi
- module Core
- class VisitableSexp < Sexp
- def accept(visitor)
- visitor.visit(self)
- end
-
- def node_type
- first
- end
-
- def children
- sexp_body.select {|each| each.class == VisitableSexp }
- end
-
- def is_language_node?
- first.class == Symbol
- end
- end
+class Sexp
+ def accept(visitor)
+ visitor.visit(self)
+ end
+
+ def node_type
+ first
+ end
+
+ def children
+ find_all { | sexp | Sexp === sexp }
+ end
+
+ def is_language_node?
+ first.class == Symbol
end
end
2  lib/roodi_task.rb
View
@@ -28,7 +28,7 @@ def define
runner.errors.each {|error| puts error}
- raise "Found #{runner.errors.size} errors." if runner.errors
+ raise "Found #{runner.errors.size} errors." unless runner.errors.empty?
end
self
end
2  spec/roodi/checks/abc_metric_method_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::AbcMetricMethodCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::AbcMetricMethodCheck.new({'score' => 0}))
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::AbcMetricMethodCheck.new({'score' => 0}))
end
def verify_content_score(content, a, b, c)
6 spec/roodi/checks/assignment_in_conditional_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::AssignmentInConditionalCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::AssignmentInConditionalCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::AssignmentInConditionalCheck.new)
end
it "should accept an assignment before an if clause" do
@@ -55,9 +55,7 @@
end
it "should reject an assignment inside a a ternary operator check clause" do
- content = <<-END
- call_foo (bar = bam) ? baz : bad
- END
+ content = 'call_foo (bar = bam) ? baz : bad'
@roodi.check_content(content)
errors = @roodi.errors
errors.should_not be_empty
4 spec/roodi/checks/case_missing_else_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::CaseMissingElseCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::CaseMissingElseCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::CaseMissingElseCheck.new)
end
it "should accept case statements that do have an else" do
@@ -27,6 +27,6 @@
@roodi.check_content(content)
errors = @roodi.errors
errors.should_not be_empty
- errors[0].to_s.should eql("dummy-file.rb:1 - Case statement is missing an else clause.")
+ errors[0].to_s.should match(/dummy-file.rb:[1-2] - Case statement is missing an else clause./)
end
end
2  spec/roodi/checks/class_line_count_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ClassLineCountCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ClassLineCountCheck.new({'line_count' => 1}))
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ClassLineCountCheck.new({'line_count' => 1}))
end
it "should accept classes with less lines than the threshold" do
2  spec/roodi/checks/class_name_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ClassNameCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ClassNameCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ClassNameCheck.new)
end
it "should accept camel case class names starting in capitals" do
4 spec/roodi/checks/class_variable_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ClassVariableCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ClassVariableCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ClassVariableCheck.new)
end
it "should reject class variables" do
@@ -12,6 +12,6 @@
@roodi.check_content(content)
errors = @roodi.errors
errors.should_not be_empty
- errors[0].to_s.should eql("dummy-file.rb:1 - Don't use class variables. You might want to try a different design.")
+ errors[0].to_s.should match(/dummy-file.rb:[1-2] - Don't use class variables. You might want to try a different design./)
end
end
4 spec/roodi/checks/control_coupling_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ControlCouplingCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ControlCouplingCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ControlCouplingCheck.new)
end
it "should reject methods with if checks using a parameter" do
@@ -18,6 +18,6 @@ def write(quoted, foo)
@roodi.check_content(content)
errors = @roodi.errors
errors.should_not be_empty
- errors[0].to_s.should eql("dummy-file.rb:2 - Method \"write\" uses the argument \"quoted\" for internal control.")
+ errors[0].to_s.should match(/dummy-file.rb:[2-3] - Method \"write\" uses the argument \"quoted\" for internal control./)
end
end
4 spec/roodi/checks/cyclomatic_complexity_block_check_spec.rb
View
@@ -2,14 +2,14 @@
describe Roodi::Checks::CyclomaticComplexityBlockCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::CyclomaticComplexityBlockCheck.new({'complexity' => 0}))
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::CyclomaticComplexityBlockCheck.new({'complexity' => 0}))
end
def verify_content_complexity(content, complexity)
@roodi.check_content(content)
errors = @roodi.errors
errors.should_not be_empty
- errors[0].to_s.should eql("dummy-file.rb:2 - Block cyclomatic complexity is #{complexity}. It should be 0 or less.")
+ errors[0].to_s.should match(/dummy-file.rb:[2-4] - Block cyclomatic complexity is #{complexity}. It should be 0 or less./)
end
it "should find a simple block" do
2  spec/roodi/checks/cyclomatic_complexity_method_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::CyclomaticComplexityMethodCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::CyclomaticComplexityMethodCheck.new({'complexity' => 0}))
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::CyclomaticComplexityMethodCheck.new({'complexity' => 0}))
end
def verify_content_complexity(content, complexity)
6 spec/roodi/checks/empty_rescue_body_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::EmptyRescueBodyCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::EmptyRescueBodyCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::EmptyRescueBodyCheck.new)
end
it "should accept a rescue body with content and no parameter" do
@@ -87,7 +87,7 @@
@roodi.check_content(content)
errors = @roodi.errors
errors.should_not be_empty
- errors[0].to_s.should eql("dummy-file.rb:3 - Rescue block should not be empty.")
+ errors[0].to_s.should match(/dummy-file.rb:[3-4] - Rescue block should not be empty./)
end
it "should accept a rescue block with an explicit nil" do
@@ -109,6 +109,6 @@
@roodi.check_content(content)
errors = @roodi.errors
errors.should_not be_empty
- errors[0].to_s.should eql("dummy-file.rb:3 - Rescue block should not be empty.")
+ errors[0].to_s.should match(/dummy-file.rb:[3-4] - Rescue block should not be empty./)
end
end
2  spec/roodi/checks/for_loop_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ForLoopCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ForLoopCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ForLoopCheck.new)
end
it "should reject for loops" do
2  spec/roodi/checks/method_line_count_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::MethodLineCountCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::MethodLineCountCheck.new({'line_count' => 1}))
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::MethodLineCountCheck.new({'line_count' => 1}))
end
it "should accept methods with less lines than the threshold" do
2  spec/roodi/checks/method_name_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::MethodNameCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::MethodNameCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::MethodNameCheck.new)
end
it "should accept method names with underscores" do
2  spec/roodi/checks/module_line_count_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ModuleLineCountCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ModuleLineCountCheck.new({'line_count' => 1}))
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ModuleLineCountCheck.new({'line_count' => 1}))
end
it "should accept modules with less lines than the threshold" do
2  spec/roodi/checks/module_name_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ModuleNameCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ModuleNameCheck.new)
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ModuleNameCheck.new)
end
it "should accept camel case module names starting in capitals" do
2  spec/roodi/checks/parameter_number_check_spec.rb
View
@@ -2,7 +2,7 @@
describe Roodi::Checks::ParameterNumberCheck do
before(:each) do
- @roodi = Roodi::Core::ParseTreeRunner.new(Roodi::Checks::ParameterNumberCheck.new({'parameter_count' => 1}))
+ @roodi = Roodi::Core::RubyParserRunner.new(Roodi::Checks::ParameterNumberCheck.new({'parameter_count' => 1}))
end
it "should accept methods with less lines than the threshold" do
Please sign in to comment.
Something went wrong with that request. Please try again.