Skip to content

Commit

Permalink
fix: Formatting fails when position end is out of range of the document
Browse files Browse the repository at this point in the history
Fixes redhat-developer#183

Signed-off-by: azerr <[email protected]>
  • Loading branch information
angelozerr committed Mar 26, 2024
1 parent 67324b3 commit a05f0c8
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 4 deletions.
31 changes: 28 additions & 3 deletions src/main/java/com/redhat/devtools/lsp4ij/LSPIJUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,34 @@ public static Module getModule(@Nullable VirtualFile file, @NotNull Project proj
return ReadAction.compute(() -> ProjectFileIndex.getInstance(project).getModuleForFile(file, false));
}

public static int toOffset(Position start, Document document) throws IndexOutOfBoundsException {
int lineStartOffset = document.getLineStartOffset(start.getLine());
return lineStartOffset + start.getCharacter();
/**
* Returns a valid offset from the given position in the given document even if position is invalid.
*
* <ul>
* <li>If a line number is negative, it defaults to 0.</li>
* <li>If a line number is greater than the number of lines in a document, it defaults back to the number of lines in the document.</li>
* <li>If the character value is greater than the line length it defaults back to the line length</li>
* </ul>
*
* @param position the LSP position.
* @param document the IJ document.
* @return a valid offset from the given position in the given document.
*/
public static int toOffset(@NotNull Position position, @NotNull Document document) {
// Adjust position line/character according to this comment https://github.com/microsoft/vscode-languageserver-node/blob/ed3cd0f78c1495913bda7318ace2be7f968008af/textDocument/src/main.ts#L26
int line = position.getLine();
if (line < 0) {
// If a line number is negative, it defaults to 0.
line = 0;
} else {
// If a line number is greater than the number of lines in a document, it defaults back to the number of lines in the document.
line = Math.min(line, document.getLineCount() - 1);
}
int lineStartOffset = document.getLineStartOffset(line);
int lineEndOffset = document.getLineEndOffset(line);
int offset = lineStartOffset + position.getCharacter();
// If the character value is greater than the line length it defaults back to the line length
return Math.min(offset, lineEndOffset);
}

public static Position toPosition(int offset, Document document) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*******************************************************************************
* Copyright (c) 2024 Red Hat, Inc.
* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution,
* and is available at http://www.eclipse.org/legal/epl-v20.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
******************************************************************************/
package com.redhat.devtools.lsp4ij;

import com.intellij.openapi.editor.Document;
import com.intellij.openapi.editor.impl.DocumentImpl;
import com.intellij.testFramework.fixtures.BasePlatformTestCase;
import org.eclipse.lsp4j.Position;

/**
* Tests for {@link LSPIJUtils#toOffset(Position, Document)}.
*/
public class LSPIJUtils_toOffsetTest extends BasePlatformTestCase {

public void testValidPositionWithOneLine() {
assertOffset("foo", 0, 0, 0, 'f');
assertOffset("foo", 0, 1, 1, 'o');
assertOffset("foo", 0, 2, 2, 'o');
assertOffset("foo", 0, 3, 3, null);
}

public void testValidPositionWithTwoLines() {
assertOffset("foo\nbar", 0, 0, 0, 'f');
assertOffset("foo\nbar", 0, 1, 1, 'o');
assertOffset("foo\nbar", 0, 2, 2, 'o');
assertOffset("foo\nbar", 0, 3, 3, null);

assertOffset("foo\nbar", 1, 0, 4, 'b');
assertOffset("foo\nbar", 1, 1, 5, 'a');
assertOffset("foo\nbar", 1, 2, 6, 'r');
assertOffset("foo\nbar", 1, 3, 7, null);
}

public void testInvalidLineWithOneLine() {
assertOffset("foo", 999999, 0, 0, 'f');
assertOffset("foo", 999999, 1, 1, 'o');
assertOffset("foo", 999999, 2, 2, 'o');
assertOffset("foo", 999999, 3, 3, null);
}

public void testInvalidLineWithTwoLines() {
assertOffset("foo\nbar", 999999, 0, 4, 'b');
assertOffset("foo\nbar", 999999, 1, 5, 'a');
assertOffset("foo\nbar", 999999, 2, 6, 'r');
assertOffset("foo\nbar", 999999, 3, 7, null);
}

public void testInvalidCharacterWithOneLine() {
assertOffset("foo", 0, 999999, 3, null);
}

public void testInvalidCharacterWithTwoLines() {
assertOffset("foo\nbar", 0, 999999, 3, null);
assertOffset("foo\nbar", 1, 999999, 7, null);
}

public void testInvalidPositionWithOneLine() {
assertOffset("foo", 999999, 999999, 3, null);
}

public void testInvalidPositionWithTwoLine() {
assertOffset("foo\nbar", 999999, 999999, 7, null);
}

public void assertOffset(String content, int line, int character, int expectedOffset, Character expectedChar) {
assertOffset(content, new Position(line, character), expectedOffset, expectedChar);
}

public void assertOffset(String content, Position position, int expectedOffset, Character expectedChar) {
Document document = new DocumentImpl(content);
int actualOffset = LSPIJUtils.toOffset(position, document);
assertEquals(expectedOffset, actualOffset);
if (expectedChar == null) {
assertTrue(expectedOffset == content.length() /* offset is at the end of the file */
|| content.charAt(expectedOffset) == '\n' /* or offset is at the end of line */);
} else {
// get the character from the given offset
Character actualChar = document.getCharsSequence().charAt(expectedOffset);
assertEquals(expectedChar, actualChar);
}
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
*/
public class TypeScriptCompletionTest extends LSPCompletionFixtureTestCase {

public void testCompletionWithNewText() {
public void testCompletionWithTextEdit() {
// 1. Test completion items result
assertCompletion("test.ts",
"''.<caret>", """
Expand Down

0 comments on commit a05f0c8

Please sign in to comment.