Skip to content

fix(codeql): Address some easy to fix CodeQL warnings #1569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FlorianKroiss wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(codeql): Address some easy to fix CodeQL warnings #1569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e.test/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: Tests for language server bundle (Incubation)
Bundle-SymbolicName: org.eclipse.lsp4e.test;singleton:=true
Bundle-Version: 0.16.12.qualifier
Bundle-Version: 0.16.13.qualifier
Fragment-Host: org.eclipse.lsp4e
Bundle-Vendor: Eclipse LSP4E
Bundle-RequiredExecutionEnvironment: JavaSE-21
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e.test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
</parent>
<artifactId>org.eclipse.lsp4e.test</artifactId>
<packaging>eclipse-test-plugin</packaging>
<version>0.16.12-SNAPSHOT</version>
<version>0.16.13-SNAPSHOT</version>

<properties>
<os-jvm-flags /> <!-- for the default case -->
Expand Down
88 changes: 64 additions & 24 deletions org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/edit/LSPEclipseUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Random;
import java.util.UUID;
import java.util.stream.Stream;

Expand Down Expand Up @@ -113,7 +112,7 @@ public void testWorkspaceEdit_WithExaggeratedRange() throws Exception {
}

private AbstractTextEditor applyWorkspaceTextEdit(TextEdit textEdit) throws CoreException {
IFile f = TestUtils.createFile(project, "dummy" + new Random().nextInt(), "Here");
IFile f = TestUtils.createUniqueTestFile(project, "Here");
final var editor = (AbstractTextEditor)TestUtils.openEditor(f);
final var workspaceEdit = new WorkspaceEdit(Collections.singletonMap(
LSPEclipseUtils.toUri(f).toString(),
Expand All @@ -124,7 +123,9 @@ private AbstractTextEditor applyWorkspaceTextEdit(TextEdit textEdit) throws Core

@Test
public void testWorkspaceEditMultipleChanges() throws Exception {
IFile f = TestUtils.createFile(project, "dummy", "Here\nHere2");
IFile f = TestUtils.createUniqueTestFile(project, """
Here
Here2""");
final var editor = (AbstractTextEditor)TestUtils.openEditor(f);
final var edits = new LinkedList<TextEdit>();
// order the TextEdits from the top of the document to the bottom
Expand All @@ -134,8 +135,12 @@ public void testWorkspaceEditMultipleChanges() throws Exception {
LSPEclipseUtils.toUri(f).toString(), edits));
// they should be applied from bottom to top
LSPEclipseUtils.applyWorkspaceEdit(workspaceEdit);
assertEquals("abcHere\nabcHere2", ((StyledText) editor.getAdapter(Control.class)).getText());
assertEquals("abcHere\nabcHere2",
assertEquals("""
abcHere
abcHere2""", ((StyledText) editor.getAdapter(Control.class)).getText());
assertEquals("""
abcHere
abcHere2""",
editor.getDocumentProvider().getDocument(editor.getEditorInput()).get());
}

Expand All @@ -149,12 +154,16 @@ public void testWorkspaceEdit_CreateAndPopulateFile() throws Exception {
edits.add(Either.forLeft(
new TextDocumentEdit(new VersionedTextDocumentIdentifier(uri, null),
List.of(Either.forLeft(
new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), "abcHere\nabcHere2"))))));
new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), """
abcHere
abcHere2"""))))));
final var workspaceEdit = new WorkspaceEdit(edits);
// they should be applied from bottom to top
LSPEclipseUtils.applyWorkspaceEdit(workspaceEdit);
assertTrue(file.exists());
assertEquals("abcHere\nabcHere2", Files.readString(file.getLocation().toPath()));
assertEquals("""
abcHere
abcHere2""", Files.readString(file.getLocation().toPath()));
}

@Test
Expand Down Expand Up @@ -260,22 +269,30 @@ public void testCustomResourceToURIMapping() throws CoreException { // bug 57642

@Test
public void testApplyTextEditLongerThanOrigin() throws Exception {
IFile file = TestUtils.createUniqueTestFile(project, "line1\nlineInsertHere");
IFile file = TestUtils.createUniqueTestFile(project, """
line1
lineInsertHere""");
ITextViewer viewer = TestUtils.openTextViewer(file);
final var textEdit = new TextEdit(new Range(new Position(1, 4), new Position(1, 4 + "InsertHere".length())), "Inserted");
IDocument document = viewer.getDocument();
LSPEclipseUtils.applyEdit(textEdit, document);
assertEquals("line1\nlineInserted", document.get());
assertEquals("""
line1
lineInserted""", document.get());
}

@Test
public void testApplyTextEditShorterThanOrigin() throws Exception {
IFile file = TestUtils.createUniqueTestFile(project, "line1\nlineHERE");
IFile file = TestUtils.createUniqueTestFile(project, """
line1
lineHERE""");
ITextViewer viewer = TestUtils.openTextViewer(file);
final var textEdit = new TextEdit(new Range(new Position(1, 4), new Position(1, 4 + "HERE".length())), "Inserted");
IDocument document = viewer.getDocument();
LSPEclipseUtils.applyEdit(textEdit, document);
assertEquals("line1\nlineInserted", document.get());
assertEquals("""
line1
lineInserted""", document.get());
}

@Test
Expand All @@ -292,16 +309,27 @@ public void testTextEditInsertSameOffset() throws Exception {

@Test
public void testTextEditSplittedLineEndings() throws Exception {
IFile file = TestUtils.createUniqueTestFile(project, "line1\r\nline2\r\nline3\r\n");
IFile file = TestUtils.createUniqueTestFile(project, """
line1\r
line2\r
line3\r
""");
ITextViewer viewer = TestUtils.openTextViewer(file);
// GIVEN a TextEdit which splits the '\r\n' line ending in the third line:
final var edits = new TextEdit[] { new TextEdit(new Range(new Position(0, 0), new Position(2, 6)), "line3\r\nline2\r\nline1\r") };
final var edits = new TextEdit[] { new TextEdit(new Range(new Position(0, 0), new Position(2, 6)), """
line3\r
line2\r
line1\r""") };
IDocument document = viewer.getDocument();
int linesBeforeApplyEdits = document.getNumberOfLines();
// WHEN the TextEdit gets applied to the document:
LSPEclipseUtils.applyEdits(document, List.of(edits));
// THEN line1 has been swapped with line 3:
assertEquals("line3\r\nline2\r\nline1\r\n", document.get());
assertEquals("""
line3\r
line2\r
line1\r
""", document.get());
// AND the number of lines is still the same, because we have not appended a line:
assertEquals(linesBeforeApplyEdits, document.getNumberOfLines());
}
Expand Down Expand Up @@ -400,14 +428,18 @@ public void editExternalFile(@TempDir Path tempDir) throws Exception {
Path file = Files.createFile(tempDir.resolve("editExternalFile.whatever"));
final var te = new TextEdit();
te.setRange(new Range(new Position(0, 0), new Position(0, 0)));
te.setNewText("abc\ndef");
te.setNewText("""
abc
def""");
final var docEdit = new TextDocumentEdit(
new VersionedTextDocumentIdentifier(file.toUri().toString(), null),
List.of(Either.forLeft(te)));
final var we = new WorkspaceEdit(List.of(Either.forLeft(docEdit)));
LSPEclipseUtils.applyWorkspaceEdit(we);
assertTrue(Files.isRegularFile(file));
assertEquals("abc\ndef", Files.readString(file));
assertEquals("""
abc
def""", Files.readString(file));
}

@Test
Expand Down Expand Up @@ -439,13 +471,17 @@ public void testTextEditDoesntAutomaticallySaveOpenResourceFiles() throws Except
"org.eclipse.ui.genericeditor.GenericEditor");
final var te = new TextEdit();
te.setRange(new Range(new Position(0, 0), new Position(0, 0)));
te.setNewText("abc\ndef");
te.setNewText("""
abc
def""");
final var docEdit = new TextDocumentEdit(
new VersionedTextDocumentIdentifier(LSPEclipseUtils.toUri(targetFile).toString(), null),
List.of(Either.forLeft(te)));
final var we = new WorkspaceEdit(List.of(Either.forLeft(docEdit)));
LSPEclipseUtils.applyWorkspaceEdit(we);
assertEquals("abc\ndef", ((StyledText) ((AbstractTextEditor) editor).getAdapter(Control.class)).getText());
assertEquals("""
abc
def""", ((StyledText) ((AbstractTextEditor) editor).getAdapter(Control.class)).getText());
assertTrue(editor.isDirty());
}

Expand All @@ -455,13 +491,17 @@ public void testTextEditDoesntAutomaticallySaveOpenExternalFiles(@TempDir Path t
IEditorPart editor = IDE.openInternalEditorOnFileStore(UI.getActivePage(), EFS.getStore(file.toUri()));
final var te = new TextEdit();
te.setRange(new Range(new Position(0, 0), new Position(0, 0)));
te.setNewText("abc\ndef");
te.setNewText("""
abc
def""");
final var docEdit = new TextDocumentEdit(
new VersionedTextDocumentIdentifier(file.toUri().toString(), null),
List.of(Either.forLeft(te)));
final var we = new WorkspaceEdit(List.of(Either.forLeft(docEdit)));
LSPEclipseUtils.applyWorkspaceEdit(we);
assertEquals("abc\ndef", ((StyledText) ((AbstractTextEditor) editor).getAdapter(Control.class)).getText());
assertEquals("""
abc
def""", ((StyledText) ((AbstractTextEditor) editor).getAdapter(Control.class)).getText());
assertTrue(editor.isDirty());
}

Expand Down Expand Up @@ -524,7 +564,7 @@ public void testGetOpenEditorExternalFile(@TempDir Path tempDir) throws Exceptio
@Test
public void testToCompletionParams_EmptyDocument() throws Exception {
// Given an empty file/document
var file = TestUtils.createFile(project, "dummy" + new Random().nextInt(), "");
var file = TestUtils.createUniqueTestFile(project, "");
var triggerChars = new char[] {':', '>'};
// When toCompletionParams get called with offset == 0 and document.getLength() == 0:
var param = LSPEclipseUtils.toCompletionParams(file.getLocationURI(), 0, LSPEclipseUtils.getDocument(file), triggerChars);
Expand All @@ -535,7 +575,7 @@ public void testToCompletionParams_EmptyDocument() throws Exception {
@Test
public void testToCompletionParams_ZeroOffset() throws Exception {
// Given a non empty file/document containing a non trigger character at position 3:
var file = TestUtils.createFile(project, "dummy" + new Random().nextInt(), "std");
var file = TestUtils.createUniqueTestFile(project, "std");
var triggerChars = new char[] {':', '>'};
// When toCompletionParams get called with offset == 0 and document.getLength() > 0:
var param = LSPEclipseUtils.toCompletionParams(file.getLocationURI(), 0, LSPEclipseUtils.getDocument(file), triggerChars);
Expand All @@ -546,7 +586,7 @@ public void testToCompletionParams_ZeroOffset() throws Exception {
@Test
public void testToCompletionParams_MatchingTriggerCharacter() throws Exception {
// Given a non empty file/document containing a trigger character at position 4:
var file = TestUtils.createFile(project, "dummy" + new Random().nextInt(), "std:");
var file = TestUtils.createUniqueTestFile(project, "std:");
var triggerChars = new char[] {':', '>'};
// When toCompletionParams get called with offset > 0 and document.getLength() > 0:
var param = LSPEclipseUtils.toCompletionParams(file.getLocationURI(), 4, LSPEclipseUtils.getDocument(file), triggerChars);
Expand All @@ -559,7 +599,7 @@ public void testToCompletionParams_MatchingTriggerCharacter() throws Exception {
@Test
public void testToCompletionParams_NonMatchingTriggerCharacter() throws Exception {
// Given a non empty file/document containing a non trigger character at position 3:
var file = TestUtils.createFile(project, "dummy" + new Random().nextInt(), "std");
var file = TestUtils.createUniqueTestFile(project, "std");
var triggerChars = new char[] {':', '>'};
// When toCompletionParams get called with offset > 0 and document.getLength() > 0:
var param = LSPEclipseUtils.toCompletionParams(file.getLocationURI(), 3, LSPEclipseUtils.getDocument(file), triggerChars);
Expand Down
42 changes: 9 additions & 33 deletions org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/utils/TestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@
import java.util.List;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;

import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.filesystem.IFileStore;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IFolder;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IProjectDescription;
import org.eclipse.core.resources.IWorkspace;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
Expand Down Expand Up @@ -62,6 +61,11 @@

public class TestUtils {

/**
* Used generate unique names, e.g., for projects or files.
*/
private static final AtomicInteger UNIQUE_COUNTER = new AtomicInteger();

@FunctionalInterface
public interface Condition {
boolean isMet() throws Exception;
Expand Down Expand Up @@ -165,30 +169,6 @@ public static IProject createProject(String projectName) throws CoreException {
return project;
}

public static IProject createNestedProject(IProject parent, String projectName) throws CoreException {
IFolder nestedFolder = parent.getFolder(projectName);
nestedFolder.create(true, true, null);

IWorkspace ws = ResourcesPlugin.getWorkspace();
IProject project = ws.getRoot().getProject(projectName);
if (project.exists() && project.isOpen()) {
return project;
}

// avoids java.lang.IllegalArgumentException: Attempted to beginRule:
// P/WorkspaceFoldersTest_testPojectCreate_1726575959224, does not match outer scope rule: P/
ws.run(monitor -> {
if (!project.exists()) {
IProjectDescription desc = ws.newProjectDescription(projectName);
desc.setLocation(nestedFolder.getLocation());
project.create(desc, null);
}
project.open(null);
}, ws.getRoot(), IWorkspace.AVOID_UPDATE, null); // Ensure proper scheduling

return project;
}

public static IFile createUniqueTestFile(IProject p, String content) throws CoreException {
return createUniqueTestFile(p, "lspt", content);
}
Expand All @@ -201,18 +181,14 @@ public static IFile createUniqueTestFileOfUnknownType(IProject p, String content
return createUniqueTestFile(p, "lsptunknown", content);
}

public static synchronized IFile createUniqueTestFile(IProject p, String extension, String content)
public static IFile createUniqueTestFile(IProject p, String extension, String content)
throws CoreException {
long fileNameSalt = System.currentTimeMillis();
if (p == null) {
p = ResourcesPlugin.getWorkspace().getRoot().getProject(Long.toString(fileNameSalt));
p = ResourcesPlugin.getWorkspace().getRoot().getProject(Long.toString(UNIQUE_COUNTER.getAndIncrement()));
p.create(null);
p.open(null);
}
while (p.getFile("test" + fileNameSalt + '.' + extension).exists()) {
fileNameSalt++;
}
return createFile(p, "test" + fileNameSalt + '.' + extension, content);
return createFile(p, "test" + UNIQUE_COUNTER.getAndIncrement() + '.' + extension, content);
}

public static IFile createFile(IProject p, String name, String content) throws CoreException {
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: Language Server Protocol client for Eclipse IDE (Incubation)
Bundle-SymbolicName: org.eclipse.lsp4e;singleton:=true
Bundle-Version: 0.19.13.qualifier
Bundle-Version: 0.19.14.qualifier
Bundle-RequiredExecutionEnvironment: JavaSE-21
Require-Bundle: org.eclipse.core.runtime;bundle-version="3.12.0",
org.eclipse.equinox.common;bundle-version="3.8.0",
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</parent>
<artifactId>org.eclipse.lsp4e</artifactId>
<packaging>eclipse-plugin</packaging>
<version>0.19.13-SNAPSHOT</version>
<version>0.19.14-SNAPSHOT</version>

<build>
<plugins>
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/color/ColorInformationMining.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private static int getSquareSize(FontMetrics fontMetrics) {
/**
* Compute width of square
*
* @param styledText
* @param fontMetrics
* @return the width of square
*/
private static int getSquareWidth(FontMetrics fontMetrics) {
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/CompletionProposalTools.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public static boolean isSubstringFoundOrderedInString(String subString, String s
int lastIndex = 0;
subString = subString.toLowerCase();
string = string.toLowerCase();
for (Character c : subString.toCharArray()) {
for (char c : subString.toCharArray()) {
int index = string.indexOf(c, lastIndex);
if (index < 0) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSCompletionProposal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public StyledString getStyledDisplayString(IDocument document, int offset, BoldS
String subString = getDocumentFilter(offset).toLowerCase();
int lastIndex = 0;
String lowerRawString = rawString.toLowerCase();
for (Character c : subString.toCharArray()) {
for (char c : subString.toCharArray()) {
int index = lowerRawString.indexOf(c, lastIndex);
if (index < 0) {
return res;
Expand Down
4 changes: 2 additions & 2 deletions ....lsp4e/src/org/eclipse/lsp4e/operations/declaration/OpenDeclarationHyperlinkDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ private static record LabeledLocations(String label,
/**
* Returns a list of {@link LSBasedHyperlink} using the given LSP locations
*
* @param document
* @param doc
* the document
* @param linkRegion
* @param region
* the region
* @param locations
* the LSP locations
Expand Down
3 changes: 1 addition & 2 deletions org.eclipse.lsp4e/src/org/eclipse/lsp4e/outline/LSSymbolsContentProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.eclipse.jface.text.reconciler.IReconcilingStrategy;
import org.eclipse.jface.viewers.ITreeContentProvider;
import org.eclipse.jface.viewers.ITreeSelection;
import org.eclipse.jface.viewers.StructuredViewer;
import org.eclipse.jface.viewers.TreePath;
import org.eclipse.jface.viewers.TreeSelection;
import org.eclipse.jface.viewers.TreeViewer;
Expand Down Expand Up @@ -242,7 +241,7 @@ public void resourceChanged(IResourceChangeEvent event) {
event.getDelta().accept(delta -> {
if (delta.getResource().equals(this.resource)) {
viewer.getControl().getDisplay().asyncExec(() -> {
if (!viewer.getControl().isDisposed() && viewer instanceof StructuredViewer) {
if (!viewer.getControl().isDisposed()) {
viewer.refresh(true);
}
});
Expand Down