Merge pull request #219 from fan-tom/bugifx/1008

Fix block actions (i.e ci{) in presence of quotes (VIM-1008)

src/com/maddyhome/idea/vim/helper/SearchHelper.java

@@ -37,12 +37,13 @@ import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Stack;
+import java.util.*;
+import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static com.maddyhome.idea.vim.helper.SearchHelperKtKt.checkInString;
+
 /**
  * Helper methods for searching text
  */
@@ -98,10 +99,10 @@ public class SearchHelper {
     int pos = caret.getOffset();
     int loc = blockChars.indexOf(type);
     // What direction should we go now (-1 is backward, 1 is forward)
-    int dir = loc % 2 == 0 ? -1 : 1;
+    Direction dir = loc % 2 == 0 ? Direction.BACK : Direction.FORWARD;
     // Which character did we find and which should we now search for
     char match = blockChars.charAt(loc);
-    char found = blockChars.charAt(loc - dir);
+    char found = blockChars.charAt(loc - dir.toInt());
 
     return findBlockLocation(chars, found, match, dir, pos, count, false);
   }
@@ -152,10 +153,10 @@ public class SearchHelper {
         int endOffset = quoteRange.getEndOffset();
         CharSequence subSequence = chars.subSequence(startOffset, endOffset);
         int inQuotePos = pos - startOffset;
-        int inQuoteStart = findBlockLocation(subSequence, close, type, -1, inQuotePos, count, false);
+        int inQuoteStart = findBlockLocation(subSequence, close, type, Direction.BACK, inQuotePos, count, false);
         if (inQuoteStart != -1) {
           startPosInStringFound = true;
-          int inQuoteEnd = findBlockLocation(subSequence, type, close, 1, inQuoteStart, 1, false);
+          int inQuoteEnd = findBlockLocation(subSequence, type, close, Direction.FORWARD, inQuoteStart, 1, false);
           if (inQuoteEnd != -1) {
             bstart = inQuoteStart + startOffset;
             bend = inQuoteEnd + startOffset;
@@ -165,9 +166,9 @@ public class SearchHelper {
     }
 
     if (!startPosInStringFound) {
-      bstart = findBlockLocation(chars, close, type, -1, pos, count, false);
+      bstart = findBlockLocation(chars, close, type, Direction.BACK, pos, count, false);
       if (bstart != -1) {
-        bend = findBlockLocation(chars, type, close, 1, bstart, 1, false);
+        bend = findBlockLocation(chars, type, close, Direction.FORWARD, bstart, 1, false);
       }
     }
 
@@ -281,10 +282,10 @@ public class SearchHelper {
     // If we found one ...
     if (loc >= 0) {
       // What direction should we go now (-1 is backward, 1 is forward)
-      int dir = loc % 2 == 0 ? 1 : -1;
+      Direction dir = loc % 2 == 0 ? Direction.FORWARD : Direction.BACK;
       // Which character did we find and which should we now search for
       char found = getPairChars().charAt(loc);
-      char match = getPairChars().charAt(loc + dir);
+      char match = getPairChars().charAt(loc + dir.toInt());
       res = findBlockLocation(chars, found, match, dir, pos, 1, true);
     }
 
@@ -308,21 +309,29 @@ public class SearchHelper {
   private static int findBlockLocation(@NotNull CharSequence chars,
                                        char found,
                                        char match,
-                                       int dir,
+                                       @NotNull Direction dir,
                                        int pos,
                                        int cnt,
                                        boolean allowInString) {
     int res = -1;
-    final int inCheckPos = dir < 0 && pos > 0 ? pos - 1 : pos;
+    int initialPos = pos;
+    Function<Integer, Integer> inCheckPosF = x -> dir == Direction.BACK && x > 0 ? x - 1 : x + 1;
+    final int inCheckPos = inCheckPosF.apply(pos);
     boolean inString = checkInString(chars, inCheckPos, true);
     boolean initialInString = inString;
     boolean inChar = checkInString(chars, inCheckPos, false);
-    boolean initial = true;
     int stack = 0;
     // Search to start or end of file, as appropriate
+    Set<Character> charsToSearch = new HashSet<>(Arrays.asList('\'', '"', '\n', match, found));
     while (pos >= 0 && pos < chars.length() && cnt > 0) {
+      Pair<Character, Integer> ci = findPositionOfFirstCharacter(chars, pos, charsToSearch,  false, dir);
+      if (ci == null) {
+        return -1;
+      }
+      Character c = ci.first;
+      pos = ci.second;
       // If we found a match and we're not in a string...
-      if (chars.charAt(pos) == match && (allowInString ? initialInString == inString : !inString) && !inChar) {
+      if (c == match && (allowInString ? initialInString == inString : !inString) && !inChar) {
         // We found our match
         if (stack == 0) {
           res = pos;
@@ -334,26 +343,24 @@ public class SearchHelper {
         }
       }
       // End of line - mark not in a string any more (in case we started in the middle of one
-      else if (chars.charAt(pos) == '\n') {
+      else if (c == '\n') {
         inString = false;
         inChar = false;
       }
-      else if (!initial) {
+      else if (pos != initialPos) {
         // We found another character like our original - belongs to another pair
-        if (!inString && !inChar && chars.charAt(pos) == found) {
+        if (!inString && !inChar && c == found) {
           stack++;
         }
         // We found the start/end of a string
-        else if (!inChar && isQuoteWithoutEscape(chars, pos, '"')) {
-          inString = !inString;
+        else if (!inChar) {
+          inString = checkInString(chars, inCheckPosF.apply(pos), true);
         }
-        else if (!inString && isQuoteWithoutEscape(chars, pos, '\'')) {
-          inChar = !inChar;
+        else if (!inString) {
+          inChar = checkInString(chars, inCheckPosF.apply(pos), false);
         }
       }
-
-      pos += dir;
-      initial = false;
+      pos += dir.toInt();
     }
 
     return res;
@@ -366,18 +373,13 @@ public class SearchHelper {
     if (chars.charAt(pos) != quote) return false;
 
     int backslashCounter = 0;
-    while (pos-- > 0) {
-      if (chars.charAt(pos) == '\\') {
-        backslashCounter++;
-      }
-      else {
-        break;
-      }
+    while (pos-- > 0 && chars.charAt(pos) == '\\') {
+      backslashCounter++;
     }
     return backslashCounter % 2 == 0;
   }
 
-  private enum Direction {
+  public enum Direction {
     BACK(-1), FORWARD(1);
 
     private final int value;
@@ -386,7 +388,7 @@ public class SearchHelper {
       value = i;
     }
 
-    private int toInt() {
+    public int toInt() {
       return value;
     }
   }
@@ -419,10 +421,28 @@ public class SearchHelper {
     return cnt;
   }
 
+  public static Pair<Character, Integer> findPositionOfFirstCharacter(
+    @NotNull CharSequence chars,
+    int pos,
+    final Set<Character> needles,
+    boolean searchEscaped,
+    @NotNull Direction direction
+  ) {
+    int dir = direction.toInt();
+    while (pos >= 0 && pos < chars.length()) {
+      final char c = chars.charAt(pos);
+      if (needles.contains(c) && (pos == 0 || searchEscaped || isQuoteWithoutEscape(chars, pos, c))) {
+        return Pair.create(c, pos);
+      }
+      pos += dir;
+    }
+    return null;
+  }
+
   private static int findCharacterPosition(@NotNull CharSequence chars, int pos, final char c, boolean currentLineOnly,
                                            boolean searchEscaped, @NotNull Direction direction) {
     while (pos >= 0 && pos < chars.length() && (!currentLineOnly || chars.charAt(pos) != '\n')) {
-      if (chars.charAt(pos) == c && (pos == 0 || searchEscaped || chars.charAt(pos - 1) != '\\')) {
+      if (chars.charAt(pos) == c && (pos == 0 || searchEscaped || isQuoteWithoutEscape(chars, pos, c))) {
         return pos;
       }
       pos += direction.toInt();
@@ -660,27 +680,6 @@ public class SearchHelper {
     return new TextRange(start, end + 1);
   }
 
-  private static boolean checkInString(@NotNull CharSequence chars, int pos, boolean str) {
-    if (chars.length() == 0) return false;
-    int offset = pos;
-    while (offset > 0 && chars.charAt(offset) != '\n') {
-      offset--;
-    }
-
-    boolean inString = false;
-    boolean inChar = false;
-    for (int i = offset; i <= pos; i++) {
-      if (!inChar && isQuoteWithoutEscape(chars, i, '"')) {
-        inString = !inString;
-      }
-      else if (!inString && isQuoteWithoutEscape(chars, i, '\'')) {
-        inChar = !inChar;
-      }
-    }
-
-    return str ? inString : inChar;
-  }
-
   public static int findNextCamelStart(@NotNull Editor editor, @NotNull Caret caret, int count) {
     CharSequence chars = editor.getDocument().getCharsSequence();
     int pos = caret.getOffset();

src/com/maddyhome/idea/vim/helper/SearchHelperKt.kt

@@ -0,0 +1,163 @@
+/*
+ * IdeaVim - Vim emulator for IDEs based on the IntelliJ platform
+ * Copyright (C) 2003-2020 The IdeaVim authors
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package com.maddyhome.idea.vim.helper
+
+import com.maddyhome.idea.vim.helper.SearchHelper.Direction
+import com.maddyhome.idea.vim.helper.SearchHelper.findPositionOfFirstCharacter
+
+data class State(val position: Int, val trigger: Char, val inQuote: Boolean?, val lastOpenSingleQuotePos: Int)
+
+// bounds are considered inside corresponding quotes
+fun checkInString(chars: CharSequence, currentPos: Int, str: Boolean): Boolean {
+  val begin = findPositionOfFirstCharacter(chars, currentPos, setOf('\n'), false, Direction.BACK)?.second ?: 0
+  val changes = quoteChanges(chars, begin)
+  // TODO: here we need to keep only the latest element in beforePos (if any) and
+  // don't need atAndAfterPos to be eagerly collected
+  var (beforePos, atAndAfterPos) = changes.partition { it.position < currentPos }
+
+  var (atPos, afterPos) = atAndAfterPos.partition { it.position == currentPos }
+  assert(atPos.size <= 1) { "Multiple characters at position $currentPos in string $chars" }
+  if (atPos.isNotEmpty()) {
+    val atPosChange = atPos[0]
+    if (afterPos.isEmpty()) {
+      // it is situation when cursor is on closing quote, so we must consider that we are inside quotes pair
+      afterPos = afterPos.toMutableList()
+      afterPos.add(atPosChange)
+    } else {
+      // it is situation when cursor is on opening quote, so we must consider that we are inside quotes pair
+      beforePos = beforePos.toMutableList()
+      beforePos.add(atPosChange)
+    }
+  }
+
+  val lastBeforePos = beforePos.lastOrNull()
+
+  // if opening quote was found before pos (inQuote=true), it doesn't mean pos is in string, we need
+  // to find closing quote to be sure
+  var posInQuote = lastBeforePos?.inQuote?.let { if (it) null else it }
+
+  val lastOpenSingleQuotePosBeforeCurrentPos = lastBeforePos?.lastOpenSingleQuotePos ?: -1
+  var posInChar = if (lastOpenSingleQuotePosBeforeCurrentPos == -1) false else null
+
+  var inQuote: Boolean? = null
+
+  for((_, trigger, inQuoteAfter, lastOpenSingleQuotePosAfter) in afterPos) {
+    inQuote = inQuoteAfter
+    if (posInQuote != null && posInChar != null) break
+    if (posInQuote == null && inQuoteAfter != null) {
+      // if we found double quote
+      if (trigger == '"') {
+        // then previously it has opposite value
+        posInQuote = !inQuoteAfter
+        // if we found single quote
+      } else if (trigger == '\'') {
+        // then we found closing single quote
+        posInQuote = inQuoteAfter
+      }
+    }
+    if (posInChar == null && lastOpenSingleQuotePosAfter != lastOpenSingleQuotePosBeforeCurrentPos) {
+      // if we found double quote and we reset position of last single quote
+      if (trigger == '"' && lastOpenSingleQuotePosAfter == -1) {
+        // then it means previously there supposed to be open single quote
+        posInChar = false
+        // if we found single quote
+      } else if (trigger == '\'') {
+        // if we reset position of last single quote
+        // it means we found closing single quote
+        // else it means we found opening single quote
+        posInChar = lastOpenSingleQuotePosAfter == -1
+      }
+    }
+  }
+
+  return if (str) posInQuote != null && posInQuote && (inQuote == null || !inQuote) else posInChar != null && posInChar
+}
+
+// yields changes of inQuote and lastOpenSingleQuotePos during while iterating over chars
+// rules are that:
+// escaped quotes are skipped
+// single quoted group may enclose only one character, maybe escaped,
+// so distance between opening and closing single quotes cannot be more than 3
+// bounds are considered inside corresponding quotes
+private fun quoteChanges(chars: CharSequence, begin: Int) = sequence {
+  // position of last found unpaired single quote
+  var lastOpenSingleQuotePos = -1
+  // whether we are in double quotes
+  // true - definitely yes
+  // false - definitely no
+  // null - maybe yes, in case we found such combination: '"
+  // in that situation it may be double quote inside single quotes, so we cannot threat it as double quote pair open/close
+  var inQuote: Boolean? = false
+  val charsToSearch = setOf('\'', '"', '\n')
+  var found = findPositionOfFirstCharacter(chars, begin, charsToSearch, false, Direction.FORWARD)
+  while (found != null && found.first != '\n') {
+    val i = found.second
+
+    val c = found.first
+    when (c) {
+      '"' -> {
+        // if [maybe] in quote, then we know we found closing quote, so now we surely are not in quote
+        if (inQuote == null || inQuote) {
+          // we just found closing double quote
+          inQuote = false
+          // reset last found single quote, as it was in string literal
+          lastOpenSingleQuotePos = -1
+          // if we previously found unclosed single quote
+        } else if (lastOpenSingleQuotePos >= 0) {
+          // ...but we are too far from it
+          if (i - lastOpenSingleQuotePos > 2) {
+            // then it definitely was not opening single quote
+            lastOpenSingleQuotePos = -1
+            // and we found opening double quote
+            inQuote = true
+          } else {
+            // else we don't know if we inside double or single quotes or not
+            inQuote = null
+          }
+          // we were not in double nor in single quote, so now we are in double quote
+        } else {
+          inQuote = true
+        }
+      }
+      '\'' -> {
+        // if we previously found unclosed single quote
+        if (lastOpenSingleQuotePos >= 0) {
+          // ...but we are too far from it
+          if (i - lastOpenSingleQuotePos > 3) {
+            // ... forget about it and threat current one as unclosed
+            lastOpenSingleQuotePos = i
+          } else {
+            // else we found closing single quote
+            lastOpenSingleQuotePos = -1
+            // and if we didn't know whether we are in double quote or not
+            if (inQuote == null) {
+              // then now we are definitely not in
+              inQuote = false
+            }
+          }
+        } else {
+          // we found opening single quote
+          lastOpenSingleQuotePos = i
+        }
+      }
+    }
+    yield(State(i, c, inQuote, lastOpenSingleQuotePos))
+    found = findPositionOfFirstCharacter(chars, i + Direction.FORWARD.toInt(), charsToSearch, false, Direction.FORWARD)
+  }
+}

test/org/jetbrains/plugins/ideavim/action/MotionActionTest.java

@@ -292,6 +292,27 @@ public class MotionActionTest extends VimTestCase {
     myFixture.checkResult("a{<caret>}b}");
   }
 
+  // VIM-1008 |c| |v_i{|
+  public void testDeleteInsideDoubleQuotesSurroundedBlockWithSingleQuote() {
+    configureByText("\"{do<caret>esn't work}\"");
+    typeText(parseKeys("ci{"));
+    myFixture.checkResult("\"{<caret>}\"");
+  }
+
+  // VIM-1008 |c| |v_i{|
+  public void testDeleteInsideSingleQuotesSurroundedBlock() {
+    configureByText("'{does n<caret>ot work}'");
+    typeText(parseKeys("ci{"));
+    myFixture.checkResult("'{<caret>}'");
+  }
+
+  // VIM-1008 |c| |v_i{|
+  public void testDeleteInsideDoublySurroundedBlock() {
+    configureByText("<p class=\"{{ $ctrl.so<caret>meClassName }}\"></p>");
+    typeText(parseKeys("ci{"));
+    myFixture.checkResult("<p class=\"{{<caret>}}\"></p>");
+  }
+
   // |d| |v_i>|
   public void testDeleteInnerAngleBracketBlock() {
     typeTextInFile(parseKeys("di>"),
@@ -348,6 +369,31 @@ public class MotionActionTest extends VimTestCase {
     myFixture.checkResult("foo = [\"\", \"two\", \"three\"];\n");
   }
 
+  public void testDeleteDoubleQuotedStringOddNumberOfQuotes() {
+    typeTextInFile(parseKeys("di\""),
+                   "abc\"def<caret>\"gh\"i");
+    myFixture.checkResult("abc\"\"gh\"i");
+  }
+
+  public void testDeleteDoubleQuotedStringBetweenEvenNumberOfQuotes() {
+    typeTextInFile(parseKeys("di\""),
+                   "abc\"def\"g<caret>h\"ijk\"l");
+    myFixture.checkResult("abc\"def\"\"ijk\"l");
+  }
+
+  public void testDeleteDoubleQuotedStringOddNumberOfQuotesOnLast() {
+    typeTextInFile(parseKeys("di\""),
+                   "abcdef\"gh\"ij<caret>\"kl");
+    myFixture.checkResult("abcdef\"gh\"ij\"kl");
+  }
+
+  public void testDeleteDoubleQuotedStringEvenNumberOfQuotesOnLast() {
+    typeTextInFile(parseKeys("di\""),
+                   "abc\"def\"gh\"ij<caret>\"kl");
+    myFixture.checkResult("abc\"def\"gh\"\"kl");
+  }
+
+
   // VIM-132 |v_i"|
   public void testInnerDoubleQuotedStringSelection() {
     typeTextInFile(parseKeys("vi\""),

test/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerBlockParenActionTest.kt

@@ -102,6 +102,22 @@ class MotionInnerBlockParenActionTest : VimTestCase() {
     myFixture.checkResult("foo()\n")
   }
 
+  // VIM-1008 |d| |v_ib|
+  fun testDeleteInnerBlockWithQuote() {
+    typeTextInFile(parseKeys("di)"),
+      "(abc${c}def'ghi)"
+    )
+    myFixture.checkResult("()")
+  }
+
+  // VIM-1008 |d| |v_ib|
+  fun testDeleteInnerBlockWithDoubleQuote() {
+    typeTextInFile(parseKeys("di)"),
+      """(abc${c}def"ghi)"""
+    )
+    myFixture.checkResult("()")
+  }
+
   // VIM-326 |d| |v_ib|
   fun testDeleteInnerBlockCaretBeforeString() {
     typeTextInFile(parseKeys("di)"),

test/org/jetbrains/plugins/ideavim/helper/SearchHelperTest.java

@@ -19,6 +19,7 @@
 package org.jetbrains.plugins.ideavim.helper;
 
 import com.maddyhome.idea.vim.helper.SearchHelper;
+import com.maddyhome.idea.vim.helper.SearchHelperKtKt;
 import org.jetbrains.plugins.ideavim.VimTestCase;
 
 import static com.maddyhome.idea.vim.helper.StringHelper.parseKeys;
@@ -77,4 +78,100 @@ public class SearchHelperTest extends VimTestCase {
     typeTextInFile(parseKeys("v", "a("), "((int) nu<caret>m)");
     myFixture.checkResult("<selection>((int) num)</selection>");
   }
+
+  public void testCheckInStringInsideDoubleQuotes() {
+    String text = "abc\"def\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertTrue(inString);
+  }
+
+  public void testCheckInStringWithoutClosingDoubleQuote() {
+    String text = "abcdef\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringOnUnpairedSingleQuote() {
+    String text = "abc\"d'ef\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertTrue(inString);
+  }
+
+  public void testCheckInStringOutsideOfDoubleQuotesPair() {
+    String text = "abc\"def\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 2, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringEscapedDoubleQuote() {
+    String text = "abc\\\"def\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringOddNumberOfDoubleQuotes() {
+    String text = "abc\"def\"gh\"i";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringInsideSingleQuotesPair() {
+    String text = "abc\"d'e'f\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 6, false);
+    assertTrue(inString);
+  }
+
+  public void testCheckInStringOnOpeningDoubleQuote() {
+    String text = "abc\"def\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 3, true);
+    assertTrue(inString);
+  }
+
+  public void testCheckInStringOnClosingDoubleQuote() {
+    String text = "abc\"def\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 7, true);
+    assertTrue(inString);
+  }
+
+  public void testCheckInStringWithoutQuotes() {
+    String text = "abcdefghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringDoubleQuoteInsideSingleQuotes() {
+    String text = "abc'\"'ef\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringSingleQuotesAreTooFarFromEachOtherToMakePair() {
+    String text = "abc'\"de'f\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 5, true);
+    assertTrue(inString);
+  }
+
+  public void testCheckInStringDoubleQuoteInsideSingleQuotesIsInsideSingleQuotedString() {
+    String text = "abc'\"'def\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 4, false);
+    assertTrue(inString);
+  }
+
+  public void testCheckInStringAfterClosingDoubleQuote() {
+    String text = "abc\"def\"ghi";
+    boolean inString = SearchHelperKtKt.checkInString(text, 9, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringOnMiddleDoubleQuote() {
+    String text = "abc\"def\"gh\"i";
+    boolean inString = SearchHelperKtKt.checkInString(text, 7, true);
+    assertFalse(inString);
+  }
+
+  public void testCheckInStringBetweenPairs() {
+    String text = "abc\"def\"gh\"ij\"k";
+    boolean inString = SearchHelperKtKt.checkInString(text, 8, true);
+    assertFalse(inString);
+  }
 }