Rewrite next/previous word motions and objects

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

@@ -1044,10 +1044,19 @@ two
   @TestWithoutNeovim(reason = SkipNeovimReason.UNCLEAR)
   @Test
   fun testLastWord() {
-    typeTextInFile(injector.parser.parseKeys("w"), "${c}one\n")
+    typeTextInFile(injector.parser.parseKeys("w"), "${c}one")
     assertOffset(2)
   }
 
+  @TestWithoutNeovim(reason = SkipNeovimReason.UNCLEAR)
+  @Test
+  fun testLastWord2() {
+    typeTextInFile(injector.parser.parseKeys("w"), "${c}one\n")
+    // Counter-intuitive, but correct. There is no next word, so we get to the end of the current word, skip whitespace
+    // and move past the end of the file. Behaviour matches Vim - `w` will move to the next line (which is empty)
+    assertOffset(4)
+  }
+
   // |b|
   @Test
   fun testWordBackwardsAtFirstLineWithWhitespaceInFront() {

src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertDeletePreviousWordActionTest.kt

@@ -128,7 +128,7 @@ class InsertDeletePreviousWordActionTest : VimTestCase() {
   fun `test delete starting from the last character of the file`() {
     // This test was originally trying to delete the previous word with the caret positioned at the end of line and
     // recorded different behaviour to Vim. The problem was that the caret was incorrectly positioned _passed_ the end
-    // of the line, and indeed passed the end of the file.
+    // of the line, and indeed past the end of the file.
     // This placement is valid, both in IdeaVim and Vim, but only when `:set virtualedit=onemore` is set. The test was
     // showing a bug in the implementation in this scenario. The test is now explicit in what it's trying to do, and
     // matches Vim's behaviour.

src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterWordActionTest.kt

@@ -788,19 +788,12 @@ class MotionOuterWordActionTest : VimTestCase() {
     )
   }
 
-  // TODO: Fix this bug
-  @VimBehaviorDiffers(originalVimAfter =
-    "Lorem${s}    ipsum --dolor${c} ${se}sit amet, consectetur adipiscing elit",
-    description = "First aw should select whitespace+'ipsum' " +
-      "second should select whitespace+'--' " +
-      "third should select 'dolor' and following whitespace",
-  )
   @Test
   fun `test select multiple outer words starting in whitespace`() {
     doTest(
       "v3aw",
       "Lorem  ${c}  ipsum --dolor sit amet, consectetur adipiscing elit",
-      "Lorem${s}    ipsum --dolo${c}r${se} sit amet, consectetur adipiscing elit",
+      "Lorem${s}    ipsum --dolor${c} ${se}sit amet, consectetur adipiscing elit",
       Mode.VISUAL(SelectionType.CHARACTER_WISE),
     )
   }

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

@@ -28,8 +28,7 @@ class SearchHelperTest : VimTestCase() {
   fun testFindNextWord() {
     val text = "first second"
     configureByText(text)
-    val nextWordPosition =
+    val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 1, bigWord = true)
-      injector.searchHelper.findNextWord(fixture.editor.vim, 0, 1, bigWord = true, spaceWords = false)
     kotlin.test.assertEquals(nextWordPosition, text.indexOf("second"))
   }
 
@@ -38,7 +37,7 @@ class SearchHelperTest : VimTestCase() {
   fun testFindSecondNextWord() {
     val text = "first second third"
     configureByText(text)
-    val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 2, bigWord = true, false)
+    val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 2, bigWord = true)
     kotlin.test.assertEquals(nextWordPosition, text.indexOf("third"))
   }
 
@@ -47,7 +46,7 @@ class SearchHelperTest : VimTestCase() {
   fun testFindAfterLastWord() {
     val text = "first second"
     configureByText(text)
-    val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 3, bigWord = true, false)
+    val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 3, bigWord = true)
     kotlin.test.assertEquals(nextWordPosition, text.length)
   }
 
@@ -57,7 +56,7 @@ class SearchHelperTest : VimTestCase() {
     val text = "first second"
     configureByText(text)
     val previousWordPosition =
-      injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("second"), -1, bigWord = true, false)
+      injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("second"), -1, bigWord = true)
     kotlin.test.assertEquals(previousWordPosition, text.indexOf("first"))
   }
 
@@ -67,13 +66,7 @@ class SearchHelperTest : VimTestCase() {
     val text = "first second third"
     configureByText(text)
     val previousWordPosition =
-      injector.searchHelper.findNextWord(
+      injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("third"), -2, bigWord = true)
-        fixture.editor.vim,
-        text.indexOf("third"),
-        -2,
-        bigWord = true,
-        spaceWords = false,
-      )
     kotlin.test.assertEquals(previousWordPosition, text.indexOf("first"))
   }
 
@@ -83,13 +76,7 @@ class SearchHelperTest : VimTestCase() {
     val text = "first second"
     configureByText(text)
     val previousWordPosition =
-      injector.searchHelper.findNextWord(
+      injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("second"), -3, bigWord = true)
-        fixture.editor.vim,
-        text.indexOf("second"),
-        -3,
-        bigWord = true,
-        spaceWords = false,
-      )
     kotlin.test.assertEquals(previousWordPosition, text.indexOf("first"))
   }
 
@@ -98,8 +85,7 @@ class SearchHelperTest : VimTestCase() {
   fun testFindPreviousWordWhenCursorOutOfBound() {
     val text = "first second"
     configureByText(text)
-    val previousWordPosition =
+    val previousWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, text.length, -1, bigWord = true)
-      injector.searchHelper.findNextWord(fixture.editor.vim, text.length, -1, bigWord = true, spaceWords = false)
     kotlin.test.assertEquals(previousWordPosition, text.indexOf("second"))
   }
 

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/DeletePrevWordAction.kt

@@ -35,7 +35,7 @@ class DeletePrevWordAction : VimActionHandler.SingleExecution() {
 
     val oldText = commandLine.actualText
     val motion =
-      injector.motion.findOffsetOfNextWord(oldText, oldText.length, commandLine.caret.offset, -1, true, editor)
+      injector.motion.findOffsetOfNextWord(oldText, commandLine.caret.offset, -1, true, editor)
     when (motion) {
       is Motion.AbsoluteOffset -> {
         val newText = oldText.substring(0, motion.offset) + oldText.substring(caretOffset)
@@ -48,4 +48,4 @@ class DeletePrevWordAction : VimActionHandler.SingleExecution() {
 
     return true
   }
-}
+}

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToNextWordAction.kt

@@ -31,7 +31,7 @@ class MoveToNextWordAction : VimActionHandler.SingleExecution() {
     val commandLine = injector.commandLine.getActiveCommandLine() ?: return false
 
     val text = commandLine.actualText
-    val motion = injector.motion.findOffsetOfNextWord(text, text.length, commandLine.caret.offset, 1, true, editor)
+    val motion = injector.motion.findOffsetOfNextWord(text, commandLine.caret.offset, 1, true, editor)
     when (motion) {
       is Motion.AbsoluteOffset -> {
         commandLine.caret.offset = motion.offset

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToPreviousWordAction.kt

@@ -31,7 +31,7 @@ class MoveToPreviousWordAction : VimActionHandler.SingleExecution() {
     val commandLine = injector.commandLine.getActiveCommandLine() ?: return false
 
     val text = commandLine.actualText
-    val motion = injector.motion.findOffsetOfNextWord(text, text.length, commandLine.caret.offset, -1, true, editor)
+    val motion = injector.motion.findOffsetOfNextWord(text, commandLine.caret.offset, -1, true, editor)
     when (motion) {
       is Motion.AbsoluteOffset -> {
         commandLine.caret.offset = motion.offset
@@ -41,4 +41,4 @@ class MoveToPreviousWordAction : VimActionHandler.SingleExecution() {
     }
     return true
   }
-}
+}

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroup.kt

@@ -102,7 +102,6 @@ interface VimMotionGroup {
    * Find the offset of the start of the next/previous word/WORD in some text outside the editor (e.g., command line)
    *
    * @param text        The text to search in
-   * @param textLength  The text length (there is no guarantee that calling [text.length] will be a constant time operation)
    * @param searchFrom  The buffer offset to start searching from
    * @param count       The number of words to skip
    * @param bigWord     If true then find WORD, if false then find word
@@ -111,7 +110,6 @@ interface VimMotionGroup {
    */
   fun findOffsetOfNextWord(
     text: CharSequence,
-    textLength: Int = text.length,
     searchFrom: Int,
     count: Int,
     bigWord: Boolean,

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroupBase.kt

@@ -97,29 +97,20 @@ abstract class VimMotionGroupBase : VimMotionGroup {
    * @return position
    */
   override fun findOffsetOfNextWord(editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean): Motion {
-    return findOffsetOfNextWord(editor.text(), editor.fileSize().toInt(), searchFrom, count, bigWord, editor)
+    return findOffsetOfNextWord(editor.text(), searchFrom, count, bigWord, editor)
   }
 
   override fun findOffsetOfNextWord(
     text: CharSequence,
-    textLength: Int,
     searchFrom: Int,
     count: Int,
     bigWord: Boolean,
     editor: VimEditor,
   ): Motion {
-    if ((searchFrom == 0 && count < 0) || (searchFrom >= textLength - 1 && count > 0)) {
+    if ((searchFrom == 0 && count < 0) || (searchFrom >= text.length - 1 && count > 0)) {
       return Motion.Error
     }
-    return (injector.searchHelper.findNextWord(
+    return (injector.searchHelper.findNextWord(text, editor, searchFrom, count, bigWord)).toMotionOrError()
-      text,
-      textLength,
-      editor,
-      searchFrom,
-      count,
-      bigWord,
-      false
-    )).toMotionOrError()
   }
 
   override fun getHorizontalMotion(

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelper.kt

@@ -99,36 +99,35 @@ interface VimSearchHelper {
   /**
    * Find the next word in the editor's document, from the given starting point
    *
+   * Note that this function can return an out of bounds index when there is no next word!
+   *
    * @param editor The editor's document to search in. Editor is required because word boundaries depend on
    *               local-to-buffer options
    * @param searchFrom The offset in the document to search from
    * @param count      Return an offset to the [count] word from the starting position. Will search backwards if negative
    * @param bigWord    Use WORD instead of word boundaries
-   * @param spaceWords Include whitespace as part of a word, e.g. the difference between `iw` and `aw` motions
    * @return The offset of the [count] next word, or `0` or the offset of the end of file if not found
    */
-  fun findNextWord(editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean, spaceWords: Boolean): Int
+  fun findNextWord(editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean): Int
 
   /**
    * Find the next word in some text outside the editor (e.g., command line), from the given starting point
    *
+   * Note that this function can return an out of bounds index when there is no next word!
+   *
    * @param text        The text to search in
-   * @param textLength  The text length
    * @param editor Required because word boundaries depend on local-to-buffer options
    * @param searchFrom The offset in the document to search from
    * @param count      Return an offset to the [count] word from the starting position. Will search backwards if negative
    * @param bigWord    Use WORD instead of word boundaries
-   * @param spaceWords Include whitespace as part of a word, e.g. the difference between `iw` and `aw` motions
    * @return The offset of the [count] next word, or `0` or the offset of the end of file if not found
    */
   fun findNextWord(
     text: CharSequence,
-    textLength: Int,
     editor: VimEditor,
     searchFrom: Int,
     count: Int,
     bigWord: Boolean,
-    spaceWords: Boolean,
   ): Int
 
   /**

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelperBase.kt

@@ -98,25 +98,26 @@ abstract class VimSearchHelperBase : VimSearchHelper {
     searchFrom: Int,
     count: Int,
     bigWord: Boolean,
-    spaceWords: Boolean,
   ): Int {
-    return findNextWord(editor.text(), editor.fileSize().toInt(), editor, searchFrom, count, bigWord, spaceWords)
+    return findNextWord(editor.text(), editor, searchFrom, count, bigWord)
   }
 
-  // TODO: Get rid of this overload when rewriting findNextWordOne
   override fun findNextWord(
     text: CharSequence,
-    textLength: Int,
     editor: VimEditor,
     searchFrom: Int,
     count: Int,
     bigWord: Boolean,
-    spaceWords: Boolean,
   ): Int {
-    val step = if (count >= 0) 1 else -1
     var pos = searchFrom
     repeat(abs(count)) {
-      pos = findNextWordOne(text, editor, pos, textLength, step, bigWord, spaceWords)
+      pos = if (count > 0) {
+        findNextWordOne(text, editor, pos, bigWord)
+      } else {
+        findPreviousWordOne(text, editor, pos, bigWord)
+      }
+
+      if (pos >= text.length) return pos
     }
     return pos
   }
@@ -287,7 +288,96 @@ abstract class VimSearchHelperBase : VimSearchHelper {
     ).map { it.range }
   }
 
+  /**
+   * Find the next word from the current starting position, skipping current word and whitespace
+   *
+   * Note that this will return an out of bound index if there is no next word! This is necessary to distinguish between
+   * no next word and the next word being on the last character of the file.
+   *
+   * Also remember that two different "word" types can butt up against each other - e.g. KEYWORD followed by PUNCTUATION
+   */
   private fun findNextWordOne(
+    chars: CharSequence,
+    editor: VimEditor,
+    start: Int,
+    bigWord: Boolean,
+    stopAtEndOfLine: Boolean = false,
+  ): Int {
+    var pos = start
+    if (pos >= chars.length) return chars.length
+
+    val startingCharType = charType(editor, chars[start], bigWord)
+
+    // It is important to move first, to properly handle stopping at the end of a line and moving from an empty line
+    pos++
+
+    // If we're on a word, move past the end of it
+    if (pos < chars.length && startingCharType != CharacterHelper.CharacterType.WHITESPACE) {
+      pos = skipWhileCharacterType(editor, chars, pos, 1, startingCharType, bigWord)
+    }
+
+    // Skip following whitespace, optionally stopping at the end of the line (on the newline char).
+    // An empty line is a word, so stop when the offset is at the newline char of an empty line.
+    while (pos < chars.length && isWhitespace(editor, chars[pos], bigWord)) {
+      if (isEmptyLine(chars, pos) || (chars[pos] == '\n' && stopAtEndOfLine)) return pos
+      pos++
+    }
+
+    // We're now on the first character of a word or just past the end of the file
+    return pos.coerceAtMost(chars.length)
+  }
+
+  /**
+   * Find the start of the current word, skipping current whitespace
+   *
+   * This function will always return an in-bounds index. If there is no previous word (because we're at the start of
+   * the file), the offset will be `0`, the start of the current word or preceding whitespace.
+   */
+  private fun findPreviousWordOne(
+    chars: CharSequence,
+    editor: VimEditor,
+    start: Int,
+    bigWord: Boolean,
+  ): Int {
+    var pos = start
+
+    // Always move back one to make sure that we don't get stuck on the start of a word
+    pos--
+
+    // Skip any intermediate whitespace, stopping at an empty line (offset is the newline char of the empty line).
+    // This will leave us on the last character of the previous word.
+    while (pos >= 0 && isWhitespace(editor, chars[pos], bigWord)) {
+      if (isEmptyLine(chars, pos)) return pos
+      pos--
+    }
+
+    // We're now on a word character, or at the start of the file. Move back until we're past the start of the word,
+    // then move forward to the start of the word
+    if (pos >= 0) {
+      pos = skipWhileCharacterType(editor, chars, pos, -1, charType(editor, chars[pos], bigWord), bigWord) + 1
+    }
+
+    return pos.coerceAtLeast(0)
+  }
+
+  @Suppress("SameParameterValue")
+  private fun findCharBeforeNextWord(editor: VimEditor, pos: Int, isBig: Boolean, stopAtEndOfLine: Boolean): Int {
+    val chars = editor.text()
+
+    // Find the next word, and take the character before it. If there is no next word, we're at the end of the file, and
+    // offset will be chars.length. Subtracting one gives us an in-bounds index again
+    var offset = findNextWordOne(chars, editor, pos, isBig, stopAtEndOfLine) - 1
+
+    // Don't back up to the end of a non-empty line
+    if (chars[offset] == '\n' && !isEmptyLine(chars, offset)) {
+      offset--
+    }
+
+    return offset
+  }
+
+  // TODO: Remove this once findWordUnderCursor has been properly rewritten
+  private fun oldFindNextWordOne(
     chars: CharSequence,
     editor: VimEditor,
     pos: Int,
@@ -358,7 +448,7 @@ abstract class VimSearchHelperBase : VimSearchHelper {
     return res
   }
 
-  @Suppress("GrazieInspection", "StructuralWrap")
+  @Suppress("GrazieInspection")
   private fun findNextWordEndOne(
     chars: CharSequence,
     editor: VimEditor,
@@ -1440,10 +1530,10 @@ abstract class VimSearchHelperBase : VimSearchHelper {
     // TODO: This could be simplified to move backwards until char type changes
     if ((!onWordStart && !(onSpace && isOuter)) || hasSelection || (count > 1 && dir == -1)) {
       start = if (dir == 1) {
-        findNextWord(editor, pos, -1, isBig, !isOuter)
+        oldFindNextWordOne(editor.text(), editor, pos, editor.text().length, -1, isBig, spaceWords = !isOuter)
       } else {
         val c = -(count - if (onWordStart && !hasSelection) 1 else 0)
-        findNextWord(editor, pos, c, isBig, !isOuter)
+        oldFindNextWordOne(editor.text(), editor, pos, editor.text().length, c, isBig, spaceWords = !isOuter)
       }
       start = editor.normalizeOffset(start, false)
     }
@@ -1460,6 +1550,7 @@ abstract class VimSearchHelperBase : VimSearchHelper {
     // TODO: Figure out the logic of this going backwards
     if (dir == 1) {
       var count = count
+      var shouldEndOnWhitespace = false
 
       // Selecting word/WORDs (forwards):
       // If there's no selection, we need to calculate the first range:
@@ -1508,16 +1599,18 @@ abstract class VimSearchHelperBase : VimSearchHelper {
 
           // We're on a word, move to the end, and include following whitespace by moving to the character before the
           // next word
-          isOuter && !onSpace ->            // "${s}word ${se}word"
+          isOuter && !onSpace -> {          // "${s}word ${se}word"
-            findNextWord(chars, chars.length, editor, start, 1, isBig, spaceWords = true) - 1
+            shouldEndOnWhitespace = true
+            findCharBeforeNextWord(editor, start, isBig, stopAtEndOfLine = true)
+          }
 
           // We're on a word, move to the end, not including trailing whitespace
           !isOuter && !onSpace ->           // "${s}word${se} word"
             findNextWordEnd(editor, start, 1, isBig, stopOnEmptyLine = true, allowMoveFromWordEnd = false)
 
           // We're on preceding whitespace, move to the character before the next word
-          else /* !isOuter && onSpace */ -> // "${s}     ${se}word"
+          else /* !isOuter && onSpace */ ->   // "${s}     ${se}word"
-            findNextWord(chars, chars.length, editor, start, 1, isBig, spaceWords = true) - 1
+            findCharBeforeNextWord(editor, start, isBig, stopAtEndOfLine = true)
         }
 
         count--
@@ -1585,7 +1678,7 @@ abstract class VimSearchHelperBase : VimSearchHelper {
           }
           else {
             // Move to one before start of next word (skips following whitespace)
-            findNextWord(chars, chars.length, editor, end, 1, isBig, spaceWords = true) - 1
+            findCharBeforeNextWord(editor, end, isBig, stopAtEndOfLine = true)
           }
 
         } else {
@@ -1657,6 +1750,23 @@ abstract class VimSearchHelperBase : VimSearchHelper {
           }
         }
       }
+
+      if (isOuter && shouldEndOnWhitespace && start > 0
+        && !isWhitespace(editor, chars[end], isBig)
+        && !isWhitespace(editor, chars[start], isBig)) {
+
+        // Outer word objects normally include following whitespace. But if there's no following whitespace to include,
+        // we should extend the range to include preceding whitespace. However, Vim doesn't select whitespace at the
+        // start of a line
+        var offset = start - 1
+        while (offset >= 0 && chars[offset] != '\n' && isWhitespace(editor, chars[offset], isBig)) {
+          offset--
+        }
+        if (offset > 0 && chars[offset] != '\n') start = offset + 1
+      }
+
+      if (start == end && chars[start] == '\n') end++
+      return TextRange(start, end + 1)
     }
     else if (!onWordEnd || hasSelection || (count > 1 && dir == 1) || (onSpace && isOuter)) {
       end = if (dir == 1) {