Merge pull request #7 from exercism/fixtures

Add recent two-fer solutions as test fixtures

Author: iHiD

.gitignore

@@ -0,0 +1 @@
+/analysis-data

lib/analyze_solution.rb

@@ -4,7 +4,7 @@ class AnalyzeSolution
   initialize_with :exercise_slug, :path
 
   def call
-    code_to_analyze = File.readlines(path / FILENAMES[exercise_slug])
+    code_to_analyze = File.read(path / FILENAMES[exercise_slug])
 
     classified_exercise = exercise_slug.tr('-', '_').classify
     results = "#{classified_exercise}::Analyze".constantize.(code_to_analyze)

lib/analyzer.rb

@@ -13,7 +13,7 @@
 require 'active_support/inflector'
 
 require_relative "generic/helpers"
-require_relative "generic/extract_class_method"
+require_relative "generic/extract_module_method"
 require_relative "generic/extract_instance_method"
 require_relative "generic/extract_module_or_class"
 

lib/analyzers/two_fer/analyze.rb

@@ -1,14 +1,15 @@
 module TwoFer
 
   MESSAGES = {
-    no_module: "No module or class called TwoFer",
-    no_method: "No method called two_fer",
-    splat_args: "Rather than using *%s, how about actually setting a parameter called 'name'?",
-    missing_default_param: "There is no correct default param - the tests will fail",
-    incorrect_default_param: "You could set the default value to 'you' to avoid conditionals",
-    string_building: "Rather than using string building, use interpolation",
-    kernel_format: "Rather than using the format method, use interpolation",
-    string_format: "Rather than using string's format/percentage method, use interpolation"
+    no_module:               "ruby.general.no_target_module",
+    no_method:               "ruby.general.no_target_method",
+    incorrect_indentation:   "ruby.general.incorrect_indentation",
+    splat_args:              "ruby.two_fer.splat_args",              #Rather than using *%s, how about actually setting a parameter called 'name'?",
+    missing_default_param:   "ruby.two_fer.missing_default_param",   #"There is no correct default param - the tests will fail",
+    incorrect_default_param: "ruby.two_fer.incorrect_default_param", #You could set the default value to 'you' to avoid conditionals",
+    string_building:         "ruby.two_fer.avoid_string_building",   # "Rather than using string building, use interpolation",
+    kernel_format:           "ruby.two_fer.avoid_kernel_format",     #"Rather than using the format method, use interpolation",
+    string_format:           "ruby.two_fer.avoid_string_format",     #"Rather than using string's format/percentage method, use interpolation"
   }
 
   class Analyze < ExerciseAnalyzer
@@ -97,7 +98,7 @@ def check_for_optimal_solution!
       # done something weird, so let's get a mentor to look at it!
       refer_to_mentor! unless string_interpolation_has_three_components?
 
-      approve!
+      approve_if_whitespace_is_sensible!
     end
 
     def check_for_correct_solution_without_string_interpolaton!
@@ -112,7 +113,7 @@ def check_for_correct_solution_without_string_interpolaton!
       # "One for " + name + ", one for me."
       if loc.method_name == :+ &&
          loc.arguments[0].type == :str
-        approve!(:string_building)
+        approve_if_whitespace_is_sensible!(:string_building)
       end
 
       # In the case of:
@@ -121,15 +122,15 @@ def check_for_correct_solution_without_string_interpolaton!
          loc.receiver == nil &&
          loc.arguments[0].type == :str &&
          loc.arguments[1].type == :lvar
-        approve!(:kernel_format)
+        approve_if_whitespace_is_sensible!(:kernel_format)
       end
 
       # In the case of:
       # "One for %s, one for me." % name
       if loc.method_name == :% &&
          loc.receiver.type == :str &&
          loc.arguments[0].type == :lvar
-        approve!(:string_format)
+        approve_if_whitespace_is_sensible!(:string_format)
       end
 
       # If we have a one-line method that passes the tests, then it's not
@@ -182,14 +183,38 @@ def string_interpolation_has_three_components?
       target_method.body.children.size == 3
     end
 
+    # REFACTOR: This could be refactored to strip blank
+    # lines and then use each_cons(2).
+    def indentation_is_sensible?
+      previous_line = nil
+      code_to_analyze.lines.each do |line|
+        # If the previous line or this line is
+        # just a whitespace line, don't consider it
+        # when checking for indentation
+        unless previous_line == nil ||
+               previous_line =~ /^\s*\n*$/ ||
+               line =~ /^\s*\n*$/
+
+          previous_line_lspace = previous_line[/^ */].size
+          line_lspace = line[/^ */].size
+
+          return false if (previous_line_lspace - line_lspace).abs > 2
+        end
+
+        previous_line = line
+      end
+
+      true
+    end
+
     memoize
     def target_module
       SA::Helpers.extract_module_or_class(root_node, "TwoFer")
     end
 
     memoize
     def target_method
-      SA::Helpers.extract_class_method(target_module, "two_fer")
+      SA::Helpers.extract_module_method(target_module, "two_fer")
     end
 
     memoize
@@ -220,15 +245,19 @@ def default_argument_value
     # These are totally generic to all exercises and
     # can probably be extracted to parent
     # ###
-    def approve!(msg = nil)
-      if msg
-        self.status = :approve_with_comment
-        self.comments << MESSAGES[msg]
+    def approve_if_whitespace_is_sensible!(msg = nil)
+      if indentation_is_sensible?
+        if msg
+          self.status = :approve_with_comment
+          self.comments << MESSAGES[msg]
+        else
+          self.status = :approve_as_optimal
+        end
+        raise FinishedFlowControlException
+
       else
-        self.status = :approve_as_optimal
+        disapprove!(:incorrect_indentation)
       end
-
-      raise FinishedFlowControlException
     end
 
     def refer_to_mentor!
@@ -239,7 +268,11 @@ def refer_to_mentor!
 
     def disapprove!(msg, *msg_args)
       self.status = :disapprove_with_comment
-      self.comments << (MESSAGES[msg] % msg_args)
+      if msg_args.length > 0
+        self.comments << [MESSAGES[msg], *msg_args]
+      else
+        self.comments << MESSAGES[msg]
+      end
 
       raise FinishedFlowControlException
     end

lib/generic/extract_class_method.rb

@@ -0,0 +1,77 @@
+module SA
+  class ExtractModuleMethod < Parser::AST::Processor
+    include Mandate
+
+    def initialize(node_to_search, name)
+      @node_to_search = node_to_search
+      @name = name.to_sym
+      @found_method = nil
+      @found_instance_method = nil
+
+      @module_function_called = false
+    end
+
+    def call
+      process(node_to_search)
+      found_method
+    end
+
+    def on_sclass(node)
+      # If we're in a class << self context
+      if node.children[0].self_type?
+        @found_method = ExtractInstanceMethod.(node.children[1], name)
+      end
+    end
+
+    def on_def(node)
+      return unless node.method_name == name
+      if @module_function_called
+        @found_method = node
+      else
+        @found_instance_method = node
+      end
+    end
+
+    def on_defs(node)
+      # Return unless the name matches
+      return unless node.method_name == name
+
+      # Is this defined as self (where there is no constant)
+      # or on a specific module, and if so is this module the 
+      # correct one?
+      defined_on = node.children[0].const_name
+      return unless defined_on.nil? || defined_on == module_name
+
+      @found_method = node
+    end
+
+    # We want to see if we have the syntax:
+    #   module_function
+    #   def foobar
+    #   end
+    def on_send(node)
+      return unless node.method_name == :module_function
+      if node.arguments?
+        return unless @found_instance_method
+
+        if node.arguments.map{|a|a.value}.include?(name)
+          @found_method = @found_instance_method
+          @found_instance_method = nil
+        end
+      else
+        @module_function_called = true
+      end
+    end
+
+    private
+    attr_reader :node_to_search, :name,
+                :found_method,
+                :found_instance_method, :module_function_called
+
+    memoize
+    def module_name
+      node_to_search.children[0].const_name
+    end
+  end
+end
+

lib/generic/extract_module_method.rb

@@ -4,8 +4,8 @@ def self.extract_module_or_class(*args)
       ExtractModuleOrClass.(*args)
     end
 
-    def self.extract_class_method(*args)
-      ExtractClassMethod.(*args)
+    def self.extract_module_method(*args)
+      ExtractModuleMethod.(*args)
     end
 
     def self.extract_first_line_from_method(method)

lib/generic/helpers.rb

@@ -0,0 +1,31 @@
+$LOAD_PATH.unshift File.expand_path("../../lib", __FILE__)
+require "analyzer"
+
+output = []
+["two-fer"].each do |slug|
+  output << slug
+  output << "--------"
+
+  path = File.expand_path("#{__FILE__}/../../analysis-data/#{slug}")
+  p path
+  statuses = Hash.new {|h,k|h[k] = 0}
+  Dir.foreach(path) do |dir|
+    next if dir == "." || dir == ".."
+
+    begin
+      Analyzer.analyze(slug, "#{path}/#{dir}")
+      res = JSON.parse(File.read("#{path}/#{dir}/analysis.json"))
+      statuses[res['status']] += 1
+    rescue
+      statuses["exploded"] += 1
+    end
+  end
+
+  statuses.each do |status, score|
+    output << "#{status}: #{score}"
+  end
+
+  output << "\n"
+end
+
+puts output.join("\n")

scripts/generate_fixture_stats.rb

@@ -8,7 +8,7 @@ def test_that_it_constantizes_correctly
     results_json = mock
     results = mock(to_json: results_json)
 
-    File.expects(:readlines).with(SAFE_WRITE_PATH / "two_fer.rb").returns(code)
+    File.expects(:read).with(SAFE_WRITE_PATH / "two_fer.rb").returns(code)
     File.expects(:open).
       with(SAFE_WRITE_PATH / "analysis.json", "w").yields(
         mock(write: results_json)