Skip to content

Commit 61be8ca

Browse files
ghentschkeghentschke
authored
[#245] catch all exceptions during yaml parsing (#246)
* [#245] catch all exceptions during yaml parsing - do not set database path in .clangd file with invalid yaml syntax. The user has to fix it first.
1 parent 2fe1c1d commit 61be8ca

File tree

5 files changed

+51
-67
lines changed

5 files changed

+51
-67
lines changed

‎bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/clangd/ClangdConfigurationFileManager.java‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,12 @@
2727
import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
2828
import org.eclipse.cdt.core.settings.model.ICProjectDescription;
2929
import org.eclipse.cdt.lsp.LspPlugin;
30-
import org.eclipse.cdt.lsp.LspUtils;
31-
import org.eclipse.cdt.lsp.internal.clangd.editor.ClangdPlugin;
32-
import org.eclipse.cdt.lsp.internal.clangd.editor.LspEditorUiMessages;
3330
import org.eclipse.core.resources.IFile;
3431
import org.eclipse.core.resources.IProject;
3532
import org.eclipse.core.resources.IResource;
3633
import org.eclipse.core.runtime.CoreException;
37-
import org.eclipse.core.runtime.IStatus;
3834
import org.eclipse.core.runtime.NullProgressMonitor;
3935
import org.eclipse.core.runtime.Platform;
40-
import org.eclipse.core.runtime.Status;
4136
import org.osgi.service.component.annotations.Component;
4237
import org.osgi.service.component.annotations.Reference;
4338
import org.yaml.snakeyaml.Yaml;
@@ -70,7 +65,8 @@ public void handleEvent(CProjectDescriptionEvent event, MacroResolver macroResol
7065
}
7166

7267
/**
73-
* Set the <code>CompilationDatabase</code> entry in the <code>.clangd</code> file which is located in the <code>project</code> root.
68+
* Set the <code>CompilationDatabase</code> entry in the <code>.clangd</code> file which is located in the <code>project</code> root,
69+
* if the yaml file syntax can be parsed.
7470
* The <code>.clangd</code> file will be created, if it's not existing.
7571
* The <code>CompilationDatabase</code> points to the build folder of the active build configuration
7672
* (in case <code>project</code> is a managed C/C++ project).
@@ -88,19 +84,9 @@ protected void setCompilationDatabasePath(IProject project, ICProjectDescription
8884
if (enableSetCompilationDatabasePath(project)) {
8985
var relativeDatabasePath = getRelativeDatabasePath(project, newCProjectDescription, macroResolver);
9086
if (!relativeDatabasePath.isEmpty()) {
91-
try {
92-
setCompilationDatabase(project, relativeDatabasePath);
93-
} catch (ScannerException e) {
94-
var status = new Status(IStatus.ERROR, ClangdPlugin.PLUGIN_ID, e.getMessage());
95-
var projectLocation = project.getLocation().addTrailingSeparator().toPortableString();
96-
LspUtils.showErrorMessage(LspEditorUiMessages.CProjectChangeMonitor_yaml_scanner_error,
97-
LspEditorUiMessages.CProjectChangeMonitor_yaml_scanner_error_message + projectLocation
98-
+ CLANGD_CONFIG_FILE_NAME,
99-
status);
100-
}
87+
setCompilationDatabase(project, relativeDatabasePath);
10188
} else {
102-
Platform.getLog(getClass()).log(new Status(Status.ERROR, ClangdPlugin.PLUGIN_ID,
103-
"Cannot determine path to compile_commands.json")); //$NON-NLS-1$
89+
Platform.getLog(getClass()).error("Cannot determine path to compile_commands.json"); //$NON-NLS-1$
10490
}
10591
}
10692
}
@@ -189,8 +175,14 @@ public void setCompilationDatabase(IProject project, String databasePath) {
189175
Map<String, Object> data = null;
190176
Yaml yaml = new Yaml();
191177
try (var inputStream = configFile.getContents()) {
192-
//throws ScannerException
193-
data = yaml.load(inputStream);
178+
//throws ScannerException and ParserException:
179+
try {
180+
data = yaml.load(inputStream);
181+
} catch (Exception e) {
182+
Platform.getLog(getClass()).error(e.getMessage(), e);
183+
// return, since the file syntax is corrupted. The user has to fix it first:
184+
return;
185+
}
194186
}
195187
if (data == null) {
196188
//empty file: (re)create .clangd file:
@@ -237,5 +229,4 @@ private boolean createClangdConfigFile(IFile configFile, String charset, String
237229
}
238230
return false;
239231
}
240-
241232
}

‎bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/LspEditorUiMessages.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,4 @@ public class LspEditorUiMessages extends NLS {
4646
public static String LspEditorPreferencePage_completion_default;
4747
public static String LspEditorPreferencePage_select_clangd_executable;
4848

49-
public static String CProjectChangeMonitor_yaml_scanner_error;
50-
public static String CProjectChangeMonitor_yaml_scanner_error_message;
51-
5249
}

‎bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/LspEditorUiMessages.properties‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,3 @@ LspEditorPreferencePage_completion_detailed=Detailed
3333
LspEditorPreferencePage_completion_bundled=Bundled
3434
LspEditorPreferencePage_completion_default=Default
3535
LspEditorPreferencePage_select_clangd_executable=Select clangd executable
36-
37-
CProjectChangeMonitor_yaml_scanner_error=Yaml Scanner Error
38-
CProjectChangeMonitor_yaml_scanner_error_message=Error while reading:

‎bundles/org.eclipse.cdt.lsp/src/org/eclipse/cdt/lsp/LspUtils.java‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,16 @@
2121
import org.eclipse.core.resources.IFile;
2222
import org.eclipse.core.resources.IProject;
2323
import org.eclipse.core.resources.IWorkspace;
24-
import org.eclipse.core.runtime.IProgressMonitor;
25-
import org.eclipse.core.runtime.IStatus;
2624
import org.eclipse.core.runtime.Platform;
2725
import org.eclipse.core.runtime.ServiceCaller;
28-
import org.eclipse.core.runtime.Status;
29-
import org.eclipse.jface.dialogs.ErrorDialog;
3026
import org.eclipse.lsp4e.LanguageServerWrapper;
3127
import org.eclipse.lsp4e.LanguageServiceAccessor;
32-
import org.eclipse.swt.widgets.Shell;
3328
import org.eclipse.ui.IEditorInput;
3429
import org.eclipse.ui.IEditorReference;
3530
import org.eclipse.ui.IURIEditorInput;
3631
import org.eclipse.ui.PartInitException;
3732
import org.eclipse.ui.PlatformUI;
3833
import org.eclipse.ui.part.FileEditorInput;
39-
import org.eclipse.ui.progress.UIJob;
4034

4135
public class LspUtils {
4236

@@ -50,31 +44,6 @@ public static boolean isCContentType(String id) {
5044
return (id.startsWith("org.eclipse.cdt.core.c") && (id.endsWith("Source") || id.endsWith("Header"))); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
5145
}
5246

53-
/**
54-
* Show error dialog to user
55-
* @param title
56-
* @param errorText
57-
* @param status
58-
*/
59-
public static void showErrorMessage(final String title, final String errorText, final Status status) {
60-
UIJob job = new UIJob("LSP Utils") //$NON-NLS-1$
61-
{
62-
@Override
63-
public IStatus runInUIThread(IProgressMonitor monitor) {
64-
ErrorDialog.openError(getActiveShell(), title, errorText, status);
65-
return Status.OK_STATUS;
66-
}
67-
};
68-
job.setSystem(true);
69-
job.schedule();
70-
}
71-
72-
private static Shell getActiveShell() {
73-
if (PlatformUI.getWorkbench() != null && PlatformUI.getWorkbench().getActiveWorkbenchWindow() != null)
74-
return PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell();
75-
return null;
76-
}
77-
7847
public static boolean isFileOpenedInLspEditor(URI uri) {
7948
if (uri == null) {
8049
return false;

‎tests/org.eclipse.cdt.lsp.clangd.tests/src/org/eclipse/cdt/lsp/clangd/tests/ClangdConfigurationFileManagerTest.java‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
package org.eclipse.cdt.lsp.clangd.tests;
1616

17-
import static org.junit.jupiter.api.Assertions.assertThrows;
17+
import static org.junit.jupiter.api.Assertions.assertEquals;
1818
import static org.junit.jupiter.api.Assertions.assertTrue;
1919
import static org.mockito.Mockito.mock;
2020
import static org.mockito.Mockito.when;
@@ -48,7 +48,6 @@
4848
import org.junit.jupiter.api.Test;
4949
import org.junit.jupiter.api.TestInfo;
5050
import org.junit.jupiter.api.io.TempDir;
51-
import org.yaml.snakeyaml.scanner.ScannerException;
5251

5352
final class ClangdConfigurationFileManagerTest {
5453

@@ -58,6 +57,7 @@ final class ClangdConfigurationFileManagerTest {
5857
private static final String DEFAULT_CDB_SETTING = "CompileFlags: {CompilationDatabase: %s}";
5958
private static final String MODIFIED_DEFAULT_CDB_SETTING = DEFAULT_CDB_SETTING + "\n";
6059
private static final String INVALID_YAML_SYNTAX_CONTAINS_TAB = "CompileFlags:\n\tCompilationDatabase: %s";
60+
private static final String INVALID_YAML_SYNTAX_MISSING_BRACE = "CompileFlags: {CompilationDatabase: Release\r\n";
6161
private final ClangdCProjectDescriptionListener clangdConfigurationManager = PlatformUI.getWorkbench()
6262
.getService(ClangdCProjectDescriptionListener.class);
6363
private final MacroResolver macroResolver = new MockMacroResolver();
@@ -248,21 +248,51 @@ void testUpdateExpandedClangdConfigFileInProject() throws IOException, CoreExcep
248248
}
249249

250250
/**
251-
* Test whether a ScannerExcpetion will be thrown if the file contains invalid yaml syntax (here: tab)
251+
* Test whether a ScannerException will be thrown if the file contains invalid yaml syntax (here: tab)
252252
*
253253
* @throws IOException
254254
* @throws CoreException
255255
*/
256256
@Test
257257
void testInvalidYamlSyntax() throws IOException, CoreException {
258258
// GIVEN an existing .clangd configuration file with invalid yaml syntax (contains tab):
259-
createConfigFile(INVALID_YAML_SYNTAX_CONTAINS_TAB, RELATIVE_DIR_PATH_BUILD_DEFAULT);
259+
var configFile = createConfigFile(INVALID_YAML_SYNTAX_CONTAINS_TAB, RELATIVE_DIR_PATH_BUILD_DEFAULT);
260+
String beforeSet;
261+
try (var inputStream = configFile.getContents()) {
262+
beforeSet = new String(inputStream.readAllBytes());
263+
}
260264
// WHEN the ClangdConfigurationManager.setCompilationDatabasePath method gets called with a new cdb path "build/debug":
261-
// THEN a ScannerExcpetion is expected:
262-
assertThrows(ScannerException.class, () -> {
263-
((ClangdConfigurationFileManager) clangdConfigurationManager).setCompilationDatabase(project,
264-
RELATIVE_DIR_PATH_BUILD_DEBUG);
265-
});
265+
((ClangdConfigurationFileManager) clangdConfigurationManager).setCompilationDatabase(project,
266+
RELATIVE_DIR_PATH_BUILD_DEBUG);
267+
// THEN the file has not been changed, because the user shall fix the errors first:
268+
try (var inputStream = configFile.getContents()) {
269+
var afterSet = new String(inputStream.readAllBytes());
270+
assertEquals(beforeSet, afterSet);
271+
}
272+
}
273+
274+
/**
275+
* Test whether a ParserException will be thrown if the file contains invalid yaml syntax (here: missing })
276+
*
277+
* @throws IOException
278+
* @throws CoreException
279+
*/
280+
@Test
281+
void testInvalidYamlSyntax2() throws IOException, CoreException {
282+
// GIVEN an existing .clangd configuration file with invalid yaml syntax (missing }):
283+
var configFile = createConfigFile(INVALID_YAML_SYNTAX_MISSING_BRACE, RELATIVE_DIR_PATH_BUILD_DEFAULT);
284+
String beforeSet;
285+
try (var inputStream = configFile.getContents()) {
286+
beforeSet = new String(inputStream.readAllBytes());
287+
}
288+
// WHEN the ClangdConfigurationManager.setCompilationDatabasePath method gets called with a new cdb path "build/debug":
289+
((ClangdConfigurationFileManager) clangdConfigurationManager).setCompilationDatabase(project,
290+
RELATIVE_DIR_PATH_BUILD_DEBUG);
291+
// THEN the file has not been changed, because the user shall fix the errors first:
292+
try (var inputStream = configFile.getContents()) {
293+
var afterSet = new String(inputStream.readAllBytes());
294+
assertEquals(beforeSet, afterSet);
295+
}
266296
}
267297

268298
}

0 commit comments

Comments
 (0)