Permalink
Browse files

Now you can use regexp "^=", "$=" and "*=" matchers for method names

  • Loading branch information...
1 parent c1cc416 commit 19adf030414d5b3cd03af78eb34a485a18365527 @LTe committed Jun 8, 2012
Showing with 108 additions and 0 deletions.
  1. +8 −0 lib/machete/matchers.rb
  2. +25 −0 lib/machete/parser.y
  3. +51 −0 spec/machete/matchers_spec.rb
  4. +7 −0 spec/machete/parser_spec.rb
  5. +17 −0 spec/machete/patterns_spec.rb
View
@@ -136,6 +136,7 @@ def matches?(node)
end
end
+
@dmajda

dmajda Jun 8, 2012

Added accidentally?

# @private
class StringRegexpMatcher
attr_reader :regexp
@@ -154,6 +155,13 @@ def matches?(node)
end
# @private
+ class SymbolRegexpMatcher < StringRegexpMatcher
@dmajda

dmajda Jun 8, 2012

I don't like this inheritance. SymbolRegexpMatcher isn't really a specialized version of StringRegexpMatcher. If you think sharing the common parts makes sense, I'd recommend putting them into a RegexpMatcher class and inherit both SymbolRegexpMatcher and StringRegexpMatcher from it.

@LTe

LTe Jun 12, 2012

Owner

Make sense for me

+ def matches?(node)
+ node.is_a?(Symbol) && node.to_s =~ @regexp
+ end
+ end
+
+ # @private
class AnyMatcher
def ==(other)
other.instance_of?(self.class)
View
@@ -41,6 +41,27 @@ attrs : attr
| attrs "," attr { result = val[0].merge(val[2]) }
attr : method_name "=" expression { result = { val[0].to_sym => val[2] } }
+ | method_name "^=" SYMBOL {
+ result = {
+ val[0].to_sym => SymbolRegexpMatcher.new(
+ Regexp.new("^" + Regexp.escape(symbol_value(val[2])))
+ )
+ }
+ }
+ | method_name "$=" SYMBOL {
+ result = {
+ val[0].to_sym => SymbolRegexpMatcher.new(
+ Regexp.new(Regexp.escape(symbol_value(val[2])) + "$")
+ )
+ }
+ }
+ | method_name "*=" SYMBOL {
+ result = {
+ val[0].to_sym => SymbolRegexpMatcher.new(
+ Regexp.new(Regexp.escape(symbol_value(val[2])))
+ )
+ }
+ }
| method_name "^=" STRING {
result = {
val[0].to_sym => StringRegexpMatcher.new(
@@ -173,6 +194,10 @@ def string_value(value)
end
end
+def symbol_value(value)
+ value.to_s[1..-1]
+end
@dmajda

dmajda Jun 8, 2012

This function can be used also in the following line:

literal : SYMBOL  { result = LiteralMatcher.new(val[0][1..-1].to_sym) }
@LTe

LTe Jun 12, 2012

Owner

I have not noticed this. I will update this

+
# "^" needs to be here because if it were among operators recognized by
# METHOD_NAME, "^=" would be recognized as two tokens.
SIMPLE_TOKENS = [
@@ -409,6 +409,57 @@ class SubclassedLiteralMatcher < LiteralMatcher
end
end
+ describe SymbolRegexpMatcher do
+ before do
+ @matcher = SymbolRegexpMatcher.new(/abcd/)
+ end
+
+ describe "initialize" do
+ it "sets attributes correctly" do
+ @matcher.regexp.should == /abcd/
+ end
+ end
+
+ describe "==" do
+ it "returns true when passed the same object" do
+ @matcher.should == @matcher
+ end
+
+ it "returns true when passed a StartsWithMatcher initialized with the same parameters" do
+ @matcher.should == SymbolRegexpMatcher.new(/abcd/)
+ end
+
+ it "returns false when passed some random object" do
+ @matcher.should_not == Object.new
+ end
+
+ it "returns false when passed a subclass of StartsWithMatcher initialized with the same parameters" do
+ class SubclassedSymbolRegexpMatcher < SymbolRegexpMatcher
+ end
+
+ @matcher.should_not == SubclassedSymbolRegexpMatcher.new(/abcd/)
+ end
+
+ it "returns false when passed a StartsWithMatcher initialized with different parameters" do
+ @matcher.should_not == SymbolRegexpMatcher.new(/efgh/)
+ end
+ end
@dmajda

dmajda Jun 8, 2012

Generally you should test just methods implemented in the class directly. So if initialize and == was implemented in StringRegexpMatcher already, there is no need to test it in tests of SymbolRegexpMatcher. (If you'll split these into 3 classes as I suggest elsewhere, then of course 3 blocks of tests would be appropriate, each testing methods defined in a corresponding class.)

@LTe

LTe Jun 12, 2012

Owner

I will create 3 classes and I will update specs

+
+ describe "matches?" do
+ it "matches a string matching the regexp" do
+ @matcher.matches?(:efghabcdijkl).should be_true
+ end
+
+ it "does not match a string not matching the regexp" do
+ @matcher.matches?(:efghijkl).should be_false
+ end
+
+ it "does not match some random object" do
+ @matcher.matches?(Object.new).should be_false
+ end
+ end
+ end
+
describe StringRegexpMatcher do
before :each do
@matcher = StringRegexpMatcher.new(/abcd/)
@@ -72,6 +72,13 @@ def array_matcher_with_quantifier(min, max, step)
# Canonical attr is "a = 42".
it "parses attr" do
+ 'Foo<a ^= :symbol>'.should be_parsed_as(
+ NodeMatcher.new(:Foo, :a => SymbolRegexpMatcher.new(/^symbol/) ))
+ 'Foo<a $= :symbol>'.should be_parsed_as(
+ NodeMatcher.new(:Foo, :a => SymbolRegexpMatcher.new(/symbol$/) ))
+ 'Foo<a *= :symbol>'.should be_parsed_as(
+ NodeMatcher.new(:Foo, :a => SymbolRegexpMatcher.new(/symbol/) ))
@dmajda

dmajda Jun 8, 2012

Please add these below the 'Foo<a = 42 | 43>' test. (The parser tests mirror the parser definition and in the parser the method_name "=" expression case is defined first. Empty lines between 'Foo<a = 42 | 43>' test, symbol tests and string tests would probably be good readability-wise.

@LTe

LTe Jun 12, 2012

Owner

Updated

+
'Foo<a = 42 | 43>'.should be_parsed_as(NodeMatcher.new(:Foo, :a => @ch4243))
'Foo<a ^= "abcd">'.should be_parsed_as(
NodeMatcher.new(:Foo, :a => StringRegexpMatcher.new(/^abcd/))
@@ -0,0 +1,17 @@
+require "spec_helper"
+
+describe "patterns" do
+ describe "method name" do
+ it "returns true when method name is equal" do
+ Machete.matches?('find_by_id'.to_ast, 'Send<name = :find_by_id>').should be_true
+ end
+
+ it "returns true when method name begins with find" do
+ Machete.matches?('find_by_id'.to_ast, 'Send<name ^= :find>').should be_true
+ end
+
+ it "return false when method name does not begin with find" do
@dmajda

dmajda Jun 8, 2012

The description does not match the code.

@LTe

LTe Jun 12, 2012

Owner

Corrected

+ Machete.matches?('find_by_id'.to_ast, 'Send<name ^= :_find>').should be_false
+ end
+ end
+end
@dmajda

dmajda Jun 8, 2012

I was thinking about adding "high-level" tests like this (the current tests test just whether a language is parser into a correct matcher and whether matchers behave correctly, but not both in combination like this test does). However I'd do it in a separate commit and for all patterns. But of course this depends on how much time do you want to spend with this :-)

2 comments on commit 19adf03

One small thing: Please make sure that wherever some string-related stuff and symbol-related stuff is placed together at one place, they are placed in the same order. This helps readers to orient better in the code a bit.

As usual, the code is pretty much OK, the only "real" thing I have a problem with is the inheritance between matchers.

Please sign in to comment.