Merge pull request #1135 from github/bounds-checked-reverse-RWS-matching

Bounds-checked reverse RWS matching

src/Arguments.hs

@@ -44,7 +44,7 @@ patchDiff :: DiffArguments'
 patchDiff = DiffArguments PatchRenderer identityDecorator
 
 jsonDiff :: DiffArguments'
-jsonDiff = DiffArguments JSONDiffRenderer identityDecorator
+jsonDiff = DiffArguments JSONDiffRenderer (const identifierDecorator)
 
 sExpressionDiff :: DiffArguments'
 sExpressionDiff = DiffArguments (SExpressionDiffRenderer TreeOnly) identityDecorator

src/RWS.hs

@@ -183,17 +183,16 @@ findNearestNeighbourTo editDistance canCompare kdTrees term@(UnmappedTerm j _ b)
   (previous, unmappedA, unmappedB) <- get
   fromMaybe (insertion previous unmappedA unmappedB term) $ do
     -- Look up the nearest unmapped term in `unmappedA`.
-    foundA@(UnmappedTerm i _ a) <- nearestUnmapped editDistance canCompare (IntMap.filterWithKey (\ k _ ->
+    foundA@(UnmappedTerm i _ a) <- nearestUnmapped editDistance canCompare (termsWithinMoveBoundsFrom previous unmappedA) (Both.fst kdTrees) term
-      isInMoveBounds previous k)
-      unmappedA) (Both.fst kdTrees) term
     -- Look up the nearest `foundA` in `unmappedB`
-    UnmappedTerm j' _ _ <- nearestUnmapped editDistance canCompare unmappedB (Both.snd kdTrees) foundA
+    UnmappedTerm j' _ _ <- nearestUnmapped editDistance canCompare (termsWithinMoveBoundsFrom (pred j) unmappedB) (Both.snd kdTrees) foundA
     -- Return Nothing if their indices don't match
     guard (j == j')
     guard (canCompare a b)
     pure $! do
       put (i, IntMap.delete i unmappedA, IntMap.delete j unmappedB)
       pure (These i j, These a b)
+    where termsWithinMoveBoundsFrom bound = IntMap.filterWithKey (\ k _ -> isInMoveBounds bound k)
 
 isInMoveBounds :: Int -> Int -> Bool
 isInMoveBounds previous i = previous < i && i < previous + defaultMoveBound
@@ -391,4 +390,3 @@ instance Listable1 Gram where
 
 instance Listable a => Listable (Gram a) where
   tiers = tiers1
-

src/Renderer.hs

@@ -1,10 +1,11 @@
-{-# LANGUAGE DataKinds, GADTs, MultiParamTypeClasses, TypeOperators #-}
+{-# LANGUAGE DataKinds, GADTs, GeneralizedNewtypeDeriving, MultiParamTypeClasses, TypeOperators #-}
 module Renderer
 ( DiffRenderer(..)
 , SExpressionFormat(..)
 , resolveDiffRenderer
 , runDiffRenderer
 , declarationDecorator
+, identifierDecorator
 , ParseTreeRenderer(..)
 , resolveParseTreeRenderer
 , runParseTreeRenderer
@@ -51,15 +52,8 @@ runDiffRenderer = foldMap . uncurry . resolveDiffRenderer
 declarationDecorator :: Source -> Term (Syntax Text) (Record DefaultFields) -> Term (Syntax Text) (Record (Maybe Declaration ': DefaultFields))
 declarationDecorator = decoratorWithAlgebra . declarationAlgebra
 
-
+identifierDecorator :: Term (Syntax Text) (Record fields) -> Term (Syntax Text) (Record (Maybe Identifier ': fields))
-data ParseTreeRenderer fields output where
+identifierDecorator = decoratorWithAlgebra identifierAlg
-  SExpressionParseTreeRenderer :: (HasField fields Category, HasField fields SourceSpan) => SExpressionFormat -> ParseTreeRenderer fields ByteString
-  JSONParseTreeRenderer :: (ToJSONFields (Record fields), HasField fields Range) => ParseTreeRenderer fields [Value]
-
-resolveParseTreeRenderer :: (Monoid output, StringConv output ByteString) => ParseTreeRenderer fields output -> SourceBlob -> Term (Syntax Text) (Record fields) -> output
-resolveParseTreeRenderer renderer blob = case renderer of
-  SExpressionParseTreeRenderer format -> R.sExpressionParseTree format blob
-  JSONParseTreeRenderer -> R.jsonFile blob . decoratorWithAlgebra identifierAlg
   where identifierAlg :: RAlgebra (CofreeF (Syntax Text) a) (Cofree (Syntax Text) a) (Maybe Identifier)
         identifierAlg (_ :< syntax) = case syntax of
           S.Assignment f _ -> identifier f
@@ -78,9 +72,18 @@ resolveParseTreeRenderer renderer blob = case renderer of
           _ -> Nothing
           where identifier = fmap Identifier . extractLeafValue . unwrap . fst
 
+data ParseTreeRenderer fields output where
+  SExpressionParseTreeRenderer :: (HasField fields Category, HasField fields SourceSpan) => SExpressionFormat -> ParseTreeRenderer fields ByteString
+  JSONParseTreeRenderer :: (ToJSONFields (Record fields), HasField fields Range) => ParseTreeRenderer fields [Value]
+
+resolveParseTreeRenderer :: (Monoid output, StringConv output ByteString) => ParseTreeRenderer fields output -> SourceBlob -> Term (Syntax Text) (Record fields) -> output
+resolveParseTreeRenderer renderer blob = case renderer of
+  SExpressionParseTreeRenderer format -> R.sExpressionParseTree format blob
+  JSONParseTreeRenderer -> R.jsonFile blob . identifierDecorator
+
 
 newtype Identifier = Identifier Text
-  deriving (Eq, Show)
+  deriving (Eq, NFData, Show)
 
 instance ToJSONFields Identifier where
   toJSONFields (Identifier i) = ["identifier" .= i]

test/SemanticCmdLineSpec.hs

@@ -76,8 +76,8 @@ instance Listable DiffFixture where
       patchOutput = "diff --git a/test/fixtures/ruby/method-declaration.A.rb b/test/fixtures/ruby/method-declaration.B.rb\nindex 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644\n--- a/test/fixtures/ruby/method-declaration.A.rb\n+++ b/test/fixtures/ruby/method-declaration.B.rb\n@@ -1,3 +1,4 @@\n-def foo\n+def bar(a)\n+  baz\n end\n\n"
       patchOutput' = "diff --git a/methods.rb b/methods.rb\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..ff7bbbe9495f61d9e1e58c597502d152bab1761e\n--- /dev/null\n+++ b/methods.rb\n+def foo\n+end\n\n"
 
-      jsonOutput = "{\"diff\":{\"after\":{\"category\":\"Program\",\"sourceRange\":[0,21],\"sourceSpan\":{\"start\":[1,1],\"end\":[4,1]}},\"children\":[{\"after\":{\"category\":\"Method\",\"sourceRange\":[0,20],\"sourceSpan\":{\"start\":[1,1],\"end\":[3,4]}},\"children\":[{\"replace\":[{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[4,7],\"sourceSpan\":{\"start\":[1,5],\"end\":[1,8]}},{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[4,7],\"sourceSpan\":{\"start\":[1,5],\"end\":[1,8]}}]},{\"insert\":{\"category\":\"Params\",\"children\":[{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[8,9],\"sourceSpan\":{\"start\":[1,9],\"end\":[1,10]}}],\"sourceRange\":[7,13],\"sourceSpan\":{\"start\":[1,8],\"end\":[2,3]}}},{\"insert\":{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[13,16],\"sourceSpan\":{\"start\":[2,3],\"end\":[2,6]}}}],\"before\":{\"category\":\"Method\",\"sourceRange\":[0,11],\"sourceSpan\":{\"start\":[1,1],\"end\":[2,4]}}}],\"before\":{\"category\":\"Program\",\"sourceRange\":[0,12],\"sourceSpan\":{\"start\":[1,1],\"end\":[3,1]}}},\"oids\":[\"0000000000000000000000000000000000000000\",\"0000000000000000000000000000000000000000\"],\"paths\":[\"test/fixtures/ruby/method-declaration.A.rb\",\"test/fixtures/ruby/method-declaration.B.rb\"]}\n"
+      jsonOutput = "{\"diff\":{\"after\":{\"category\":\"Program\",\"sourceRange\":[0,21],\"sourceSpan\":{\"start\":[1,1],\"end\":[4,1]}},\"children\":[{\"after\":{\"category\":\"Method\",\"identifier\":\"bar\",\"sourceRange\":[0,20],\"sourceSpan\":{\"start\":[1,1],\"end\":[3,4]}},\"children\":[{\"replace\":[{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[4,7],\"sourceSpan\":{\"start\":[1,5],\"end\":[1,8]}},{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[4,7],\"sourceSpan\":{\"start\":[1,5],\"end\":[1,8]}}]},{\"insert\":{\"category\":\"Params\",\"children\":[{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[8,9],\"sourceSpan\":{\"start\":[1,9],\"end\":[1,10]}}],\"sourceRange\":[7,13],\"sourceSpan\":{\"start\":[1,8],\"end\":[2,3]}}},{\"insert\":{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[13,16],\"sourceSpan\":{\"start\":[2,3],\"end\":[2,6]}}}],\"before\":{\"category\":\"Method\",\"identifier\":\"foo\",\"sourceRange\":[0,11],\"sourceSpan\":{\"start\":[1,1],\"end\":[2,4]}}}],\"before\":{\"category\":\"Program\",\"sourceRange\":[0,12],\"sourceSpan\":{\"start\":[1,1],\"end\":[3,1]}}},\"oids\":[\"0000000000000000000000000000000000000000\",\"0000000000000000000000000000000000000000\"],\"paths\":[\"test/fixtures/ruby/method-declaration.A.rb\",\"test/fixtures/ruby/method-declaration.B.rb\"]}\n"
-      jsonOutput' = "{\"diff\":{\"insert\":{\"category\":\"Program\",\"children\":[{\"category\":\"Method\",\"children\":[{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[4,7],\"sourceSpan\":{\"start\":[1,5],\"end\":[1,8]}}],\"sourceRange\":[0,11],\"sourceSpan\":{\"start\":[1,1],\"end\":[2,4]}}],\"sourceRange\":[0,12],\"sourceSpan\":{\"start\":[1,1],\"end\":[3,1]}}},\"oids\":[\"0000000000000000000000000000000000000000\",\"ff7bbbe9495f61d9e1e58c597502d152bab1761e\"],\"paths\":[\"methods.rb\",\"methods.rb\"]}\n"
+      jsonOutput' = "{\"diff\":{\"insert\":{\"category\":\"Program\",\"children\":[{\"category\":\"Method\",\"children\":[{\"category\":\"Identifier\",\"children\":[],\"sourceRange\":[4,7],\"sourceSpan\":{\"start\":[1,5],\"end\":[1,8]}}],\"identifier\":\"foo\",\"sourceRange\":[0,11],\"sourceSpan\":{\"start\":[1,1],\"end\":[2,4]}}],\"sourceRange\":[0,12],\"sourceSpan\":{\"start\":[1,1],\"end\":[3,1]}}},\"oids\":[\"0000000000000000000000000000000000000000\",\"ff7bbbe9495f61d9e1e58c597502d152bab1761e\"],\"paths\":[\"methods.rb\",\"methods.rb\"]}\n"
       sExpressionOutput = "(Program\n  (Method\n  { (Identifier)\n  ->(Identifier) }\n  {+(Params\n      (Identifier))+}\n  {+(Identifier)+}))\n"
       sExpressionOutput' = "{+(Program\n  (Method\n    (Identifier)))+}\n"
       tocOutput = "{\"changes\":{\"test/fixtures/ruby/method-declaration.A.rb -> test/fixtures/ruby/method-declaration.B.rb\":[{\"span\":{\"start\":[1,1],\"end\":[3,4]},\"category\":\"Method\",\"term\":\"bar\",\"changeType\":\"modified\"}]},\"errors\":{}}\n"

test/fixtures/go/channel-types.diffB-A.txt

@@ -5,25 +5,23 @@
     (Identifier)
     (Args)
     (Other "type_declaration"
-    {+(TypeDecl
-        (Identifier)
-        (ChannelTy
-          (ChannelTy
-            (Identifier))))+}
       (TypeDecl
-        (Identifier)
+      { (Identifier)
+      ->(Identifier) }
         (ChannelTy
           (ChannelTy
           { (Identifier)
-          ->(StructTy) })))
-      (TypeDecl
-        (Identifier)
-        (ChannelTy
-          (ChannelTy
-          { (StructTy)
           ->(Identifier) })))
-    {-(TypeDecl
+      (TypeDecl
-        (Identifier)
+      { (Identifier)
+      ->(Identifier) }
         (ChannelTy
           (ChannelTy
-            (Identifier))))-})))
+            (StructTy))))
+      (TypeDecl
+      { (Identifier)
+      ->(Identifier) }
+        (ChannelTy
+          (ChannelTy
+          { (Identifier)
+          ->(Identifier) }))))))

test/fixtures/go/for-statements.diffA-B.txt

@@ -4,11 +4,15 @@
   (Function
     (Identifier)
     (Args)
-  {+(For
+  { (For
-      (FunctionCall
+      (ExpressionStatements
-        (Identifier))
+        (FunctionCall
-      (Other "goto_statement"
+          (Identifier)
-        (Identifier)))+}
+          )(Other"goto_statement"(Identifier))))
+  ->(For
+    (FunctionCall
+      (Identifier))
+    (Other"goto_statement"(Identifier))) }
   {+(For
       (Other "expression_list"
         (Identifier))
@@ -30,14 +34,12 @@
       (FunctionCall
         (Identifier))
       (Continue))+}
-    (For
+  {+(For
       (ExpressionStatements
         (FunctionCall
           (Identifier)
-        {+(Identifier)+})
+          (Identifier))
-      {+(Break)+}
+      (Break)))+}
-      {-(Other "goto_statement"
-          (Identifier))-}))
   {-(For
       (VarDecl
         (Other "expression_list"

test/fixtures/go/for-statements.diffB-A.txt

@@ -4,12 +4,15 @@
   (Function
     (Identifier)
     (Args)
-  {+(For
+  { (For
+      (FunctionCall
+        (Identifier))
+      (Other"goto_statement"(Identifier)))
+  ->(For
       (ExpressionStatements
         (FunctionCall
-          (Identifier))
+          (Identifier)
-        (Other "goto_statement"
+          )(Other"goto_statement"(Identifier)))) }
-          (Identifier))))+}
   {+(For
       (VarDecl
         (Other "expression_list"
@@ -33,20 +36,18 @@
         (Identifier))
       (Continue
         (Identifier)))+}
-    (For
+  {+(For
       (FunctionCall
         (Identifier))
-    { (Other "goto_statement"
+      (Continue))+}
-        (Identifier))
+  (For
-    ->(Continue) })
+    (Other "expression_list"
-    (For
+      (Identifier))
-      (Other "expression_list"
+    (Identifier)
-        (Identifier))
+    (FunctionCall
       (Identifier)
-      (FunctionCall
-        (Identifier)
       {+(Identifier)+})
-      (Break
+    (Break
       {-(Identifier)-}))
   {-(For
       (FunctionCall
@@ -66,4 +67,4 @@
         (FunctionCall
           (Identifier)
           (Identifier))
-        (Break)))-}))
+      (Break)))-}))

test/fixtures/go/map-literals.diffA-B.txt

@@ -14,13 +14,13 @@
               (Identifier)
             { (Identifier)
             ->(Identifier) })
-          {+(Pair
-              (StringLiteral)
-              (StringLiteral))+}
             (Pair
             { (StringLiteral)
             ->(StringLiteral) }
-              (StringLiteral))
+            { (StringLiteral)
-          {-(Pair
+            ->(StringLiteral) })
-              (StringLiteral)
+            (Pair
-              (StringLiteral))-}))))))
+            { (StringLiteral)
+            ->(StringLiteral) }
+            { (StringLiteral)
+            ->(StringLiteral) })))))))

test/fixtures/go/parameter-declarations-with-types.diffB-A.txt

@@ -7,13 +7,13 @@
   (Function
     (Identifier)
     (Args
-    {+(ParameterDecl
-        (Identifier)
-        (Identifier))+}
       (ParameterDecl
-        (Identifier)
+      { (Identifier)
+      ->(Identifier) }
       { (Identifier)
       ->(Identifier) })
-    {-(ParameterDecl
+      (ParameterDecl
-        (Identifier)
+      { (Identifier)
-        (Identifier))-})))
+      ->(Identifier) }
+      { (Identifier)
+      ->(Identifier) }))))

test/fixtures/javascript/export.diffA-B.txt

@@ -12,68 +12,54 @@
     (Other "export_specifier"
     { (Identifier)
     ->(Identifier) }))
-{+(Export
+  (Export
-    (Other "export_specifier"
+      (Other "export_specifier"
-      (Identifier)
+        { (Identifier)
-      (Identifier))
+        ->(Identifier)}
-    (Other "export_specifier"
+        { (Identifier)
-      (Identifier)
+        ->(Identifier)})
-      (Identifier))
+      (Other "export_specifier"
-    (Other "export_specifier"
+      { (Identifier)
-      (Identifier)))+}
+      ->(Identifier)}
-{+(Export
+      { (Identifier)
+      ->(Identifier)})
+      (Other "export_specifier"
+      { (Identifier)
+      ->(Identifier)}))
+(Export
     (VarDecl
-      (Identifier))
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier))
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier)))+}
+    { (Identifier)
-{+(Export
+    ->(Identifier)}))
+(Export
     (VarAssignment
-      (Identifier)
+    { (Identifier)
-      (Identifier))
+    ->(Identifier)}
+    { (Identifier)
+    ->(Identifier)})
     (VarAssignment
-      (Identifier)
+    { (Identifier)
-      (Identifier))
+    ->(Identifier)}
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier))
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier)))+}
+    { (Identifier)
-{+(Export
+    ->(Identifier)}))
-    (Identifier))+}
+(Export
+{ (Identifier)
+->(Identifier)})
 {+(Export
     (Function
       (Identifier)
       (Params)))+}
-{-(Export
-    (Other "export_specifier"
-      (Identifier)
-      (Identifier))
-    (Other "export_specifier"
-      (Identifier)
-      (Identifier))
-    (Other "export_specifier"
-      (Identifier)))-}
-{-(Export
-    (VarDecl
-      (Identifier))
-    (VarDecl
-      (Identifier))
-    (VarDecl
-      (Identifier)))-}
-{-(Export
-    (VarAssignment
-      (Identifier)
-      (Identifier))
-    (VarAssignment
-      (Identifier)
-      (Identifier))
-    (VarDecl
-      (Identifier))
-    (VarDecl
-      (Identifier)))-}
-{-(Export
-    (Identifier))-}
   (Export
     (Function
       (Params)))

test/fixtures/javascript/export.diffB-A.txt

@@ -63,16 +63,17 @@
   (Export
     (Function
       (Params)))
-{+(Export
-    (Function
-      (Identifier)
-      (Params)))+}
   (Export
+      {+(Function
+        (Identifier)
+        (Params))+}
+      {-(Other "export_specifier"
+        (Identifier)
+        (Identifier))-})
+  {+(Export
     (Other "export_specifier"
-    { (Identifier)
+    (Identifier)
-    ->(Identifier) }
+    (Identifier)))+}
-    { (Identifier)
-    ->(Identifier) }))
   (Export
   { (StringLiteral)
   ->(StringLiteral) })

test/fixtures/typescript/export.diffA-B.txt

@@ -12,68 +12,54 @@
     (Other "export_specifier"
     { (Identifier)
     ->(Identifier) }))
-{+(Export
+  (Export
-    (Other "export_specifier"
+      (Other "export_specifier"
-      (Identifier)
+        { (Identifier)
-      (Identifier))
+        ->(Identifier)}
-    (Other "export_specifier"
+        { (Identifier)
-      (Identifier)
+        ->(Identifier)})
-      (Identifier))
+      (Other "export_specifier"
-    (Other "export_specifier"
+      { (Identifier)
-      (Identifier)))+}
+      ->(Identifier)}
-{+(Export
+      { (Identifier)
+      ->(Identifier)})
+      (Other "export_specifier"
+      { (Identifier)
+      ->(Identifier)}))
+(Export
     (VarDecl
-      (Identifier))
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier))
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier)))+}
+    { (Identifier)
-{+(Export
+    ->(Identifier)}))
+(Export
     (VarAssignment
-      (Identifier)
+    { (Identifier)
-      (Identifier))
+    ->(Identifier)}
+    { (Identifier)
+    ->(Identifier)})
     (VarAssignment
-      (Identifier)
+    { (Identifier)
-      (Identifier))
+    ->(Identifier)}
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier))
+    { (Identifier)
+    ->(Identifier)})
     (VarDecl
-      (Identifier)))+}
+    { (Identifier)
-{+(Export
+    ->(Identifier)}))
-    (Identifier))+}
+(Export
+{ (Identifier)
+->(Identifier)})
 {+(Export
     (Function
       (Identifier)
       (Params)))+}
-{-(Export
-    (Other "export_specifier"
-      (Identifier)
-      (Identifier))
-    (Other "export_specifier"
-      (Identifier)
-      (Identifier))
-    (Other "export_specifier"
-      (Identifier)))-}
-{-(Export
-    (VarDecl
-      (Identifier))
-    (VarDecl
-      (Identifier))
-    (VarDecl
-      (Identifier)))-}
-{-(Export
-    (VarAssignment
-      (Identifier)
-      (Identifier))
-    (VarAssignment
-      (Identifier)
-      (Identifier))
-    (VarDecl
-      (Identifier))
-    (VarDecl
-      (Identifier)))-}
-{-(Export
-    (Identifier))-}
   (Export
     (Function
       (Params)))

test/fixtures/typescript/export.diffB-A.txt

@@ -63,16 +63,17 @@
   (Export
     (Function
       (Params)))
-{+(Export
-    (Function
-      (Identifier)
-      (Params)))+}
   (Export
+      {+(Function
+        (Identifier)
+        (Params))+}
+      {-(Other "export_specifier"
+        (Identifier)
+        (Identifier))-})
+  {+(Export
     (Other "export_specifier"
-    { (Identifier)
+    (Identifier)
-    ->(Identifier) }
+    (Identifier)))+}
-    { (Identifier)
-    ->(Identifier) }))
   (Export
   { (StringLiteral)
   ->(StringLiteral) })