diff --git a/bundles/org.eclipse.cdt.lsp.clangd/META-INF/MANIFEST.MF b/bundles/org.eclipse.cdt.lsp.clangd/META-INF/MANIFEST.MF index a1827a6f..9714a669 100644 --- a/bundles/org.eclipse.cdt.lsp.clangd/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.cdt.lsp.clangd/META-INF/MANIFEST.MF @@ -30,7 +30,8 @@ Require-Bundle: org.eclipse.cdt.lsp;bundle-version="0.0.0", org.eclipse.ui.ide;bundle-version="0.0.0", org.eclipse.ui.workbench;bundle-version="0.0.0", org.eclipse.ui.workbench.texteditor;bundle-version="0.0.0", - org.eclipse.core.variables;bundle-version="0.0.0" + org.eclipse.core.variables;bundle-version="0.0.0", + org.yaml.snakeyaml;bundle-version="0.0.0" Service-Component: OSGI-INF/org.eclipse.cdt.lsp.clangd.BuiltinClangdOptionsDefaults.xml, OSGI-INF/org.eclipse.cdt.lsp.clangd.ClangdConfigurationFileManager.xml, OSGI-INF/org.eclipse.cdt.lsp.internal.clangd.ClangdConfigurationAccess.xml, diff --git a/bundles/org.eclipse.cdt.lsp.clangd/plugin.xml b/bundles/org.eclipse.cdt.lsp.clangd/plugin.xml index 85e8603c..8b9d4516 100644 --- a/bundles/org.eclipse.cdt.lsp.clangd/plugin.xml +++ b/bundles/org.eclipse.cdt.lsp.clangd/plugin.xml @@ -110,5 +110,16 @@ + + + + + + diff --git a/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigFileChecker.java b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigFileChecker.java new file mode 100644 index 00000000..a66a1d69 --- /dev/null +++ b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigFileChecker.java @@ -0,0 +1,126 @@ +/******************************************************************************* + * Copyright (c) 2024 Bachmann electronic GmbH and others. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Gesa Hentschke (Bachmann electronic GmbH) - initial implementation + *******************************************************************************/ + +package org.eclipse.cdt.lsp.internal.clangd; + +import java.io.IOException; + +import org.eclipse.cdt.lsp.internal.clangd.editor.ClangdPlugin; +import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IMarker; +import org.eclipse.core.resources.IResource; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.Platform; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.error.MarkedYAMLException; + +/** + * Checks the .clangd file for syntax errors and notifies the user via error markers in the file and Problems view. + */ +public class ClangdConfigFileChecker { + public static final String CLANGD_MARKER = ClangdPlugin.PLUGIN_ID + ".config.marker"; //$NON-NLS-1$ + + /** + * Checks if the .clangd file contains valid yaml syntax. Adds error marker to the file if not. + * @param configFile + */ + public void checkConfigFile(IFile configFile) { + if (!configFile.exists()) { + return; + } + Yaml yaml = new Yaml(); + try (var inputStream = configFile.getContents()) { + try { + removeMarkerFromClangdConfig(configFile); + //throws ScannerException and ParserException: + yaml.load(inputStream); + } catch (MarkedYAMLException yamlException) { + // re-read the file, because the buffer which comes along with MarkedYAMLException is limited to ~800 bytes. + try (var reReadStream = configFile.getContents()) { + addMarkerToClangdConfig(configFile, yamlException, reReadStream.readAllBytes()); + } + } catch (Exception exception) { + //log unexpected exception: + Platform.getLog(getClass()).error("Expected MarkedYAMLException, but was: " + exception.getMessage(), //$NON-NLS-1$ + exception); + } + } catch (IOException | CoreException e) { + Platform.getLog(getClass()).error(e.getMessage(), e); + } + } + + private void addMarkerToClangdConfig(IFile configFile, MarkedYAMLException yamlException, byte[] buffer) { + try { + var configMarker = parseYamlException(yamlException, buffer); + var marker = configFile.createMarker(CLANGD_MARKER); + marker.setAttribute(IMarker.MESSAGE, configMarker.message); + marker.setAttribute(IMarker.SEVERITY, IMarker.SEVERITY_ERROR); + marker.setAttribute(IMarker.LINE_NUMBER, configMarker.line); + marker.setAttribute(IMarker.CHAR_START, configMarker.charStart); + marker.setAttribute(IMarker.CHAR_END, configMarker.charEnd); + } catch (CoreException core) { + Platform.getLog(getClass()).log(core.getStatus()); + } + } + + private void removeMarkerFromClangdConfig(IFile configFile) { + try { + configFile.deleteMarkers(CLANGD_MARKER, false, IResource.DEPTH_ZERO); + } catch (CoreException e) { + Platform.getLog(getClass()).log(e.getStatus()); + } + } + + private class ClangdConfigMarker { + public String message; + public int line = 1; + public int charStart = -1; + public int charEnd = -1; + } + + /** + * Fetch line and char position information from exception to create a marker for the .clangd file. + * @param exception + * @param file + * @return + */ + private ClangdConfigMarker parseYamlException(MarkedYAMLException exception, byte[] buffer) { + var marker = new ClangdConfigMarker(); + marker.message = exception.getProblem(); + var problemMark = exception.getProblemMark(); + if (problemMark == null) { + return marker; + } + marker.line = problemMark.getLine() + 1; //getLine() is zero based, IMarker wants 1-based + int index = problemMark.getIndex(); + if (index == buffer.length) { + // When index == buffer.length() the marker index points to the non visible + // \r or \n character and the marker is not displayed in the editor. + // Or, even worse, there is no next line and index + 1 would be > buffer.length + // Therefore we have to find the last visible char: + index = getIndexOfLastVisibleChar(buffer); + } + marker.charStart = index; + marker.charEnd = index + 1; + return marker; + } + + private int getIndexOfLastVisibleChar(byte[] buffer) { + for (int i = buffer.length - 1; i >= 0; i--) { + if ('\r' != ((char) buffer[i]) && '\n' != ((char) buffer[i])) { + return i; + } + } + return Math.max(0, buffer.length - 2); + } +} diff --git a/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigFileMonitor.java b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigFileMonitor.java new file mode 100644 index 00000000..d85b9f22 --- /dev/null +++ b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/ClangdConfigFileMonitor.java @@ -0,0 +1,87 @@ +/******************************************************************************* + * Copyright (c) 2024 Bachmann electronic GmbH and others. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Gesa Hentschke (Bachmann electronic GmbH) - initial implementation + *******************************************************************************/ + +package org.eclipse.cdt.lsp.internal.clangd; + +import java.util.concurrent.ConcurrentLinkedQueue; + +import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IResourceChangeEvent; +import org.eclipse.core.resources.IResourceChangeListener; +import org.eclipse.core.resources.IResourceDelta; +import org.eclipse.core.resources.IWorkspace; +import org.eclipse.core.resources.WorkspaceJob; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Platform; +import org.eclipse.core.runtime.Status; + +/** + * Monitor changes in .clangd files in the workspace and triggers a yaml checker + * to add error markers to the .clangd file when the edits causes yaml loader failures. + */ +public class ClangdConfigFileMonitor { + private static final String CLANGD_CONFIG_FILE = ".clangd"; //$NON-NLS-1$ + private final ConcurrentLinkedQueue pendingFiles = new ConcurrentLinkedQueue<>(); + private final IWorkspace workspace; + private final ClangdConfigFileChecker checker = new ClangdConfigFileChecker(); + + private final IResourceChangeListener listener = new IResourceChangeListener() { + @Override + public void resourceChanged(IResourceChangeEvent event) { + if (event.getDelta() != null && event.getType() == IResourceChangeEvent.POST_CHANGE) { + try { + event.getDelta().accept(delta -> { + if ((delta.getKind() == IResourceDelta.ADDED + || (delta.getFlags() & IResourceDelta.CONTENT) != 0) + && CLANGD_CONFIG_FILE.equals(delta.getResource().getName())) { + if (delta.getResource() instanceof IFile file) { + pendingFiles.add(file); + checkJob.schedule(100); + } + } + return true; + }); + } catch (CoreException e) { + Platform.getLog(getClass()).log(e.getStatus()); + } + } + } + }; + + public ClangdConfigFileMonitor(IWorkspace workspace) { + this.workspace = workspace; + } + + private final WorkspaceJob checkJob = new WorkspaceJob("Check .clangd file") { //$NON-NLS-1$ + + @Override + public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException { + while (pendingFiles.peek() != null) { + checker.checkConfigFile(pendingFiles.poll()); + } + return Status.OK_STATUS; + } + + }; + + public ClangdConfigFileMonitor start() { + workspace.addResourceChangeListener(listener); + return this; + } + + public void stop() { + workspace.removeResourceChangeListener(listener); + } +} diff --git a/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdPlugin.java b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdPlugin.java index 2957097c..8bc693c6 100644 --- a/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdPlugin.java +++ b/bundles/org.eclipse.cdt.lsp.clangd/src/org/eclipse/cdt/lsp/internal/clangd/editor/ClangdPlugin.java @@ -15,6 +15,7 @@ package org.eclipse.cdt.lsp.internal.clangd.editor; import org.eclipse.cdt.lsp.internal.clangd.CProjectChangeMonitor; +import org.eclipse.cdt.lsp.internal.clangd.ClangdConfigFileMonitor; import org.eclipse.core.resources.IWorkspace; import org.eclipse.ui.plugin.AbstractUIPlugin; import org.osgi.framework.BundleContext; @@ -27,6 +28,7 @@ public class ClangdPlugin extends AbstractUIPlugin { private IWorkspace workspace; private CompileCommandsMonitor compileCommandsMonitor; private CProjectChangeMonitor cProjectChangeMonitor; + private ClangdConfigFileMonitor configFileMonitor; // The plug-in ID public static final String PLUGIN_ID = "org.eclipse.cdt.lsp.clangd"; //$NON-NLS-1$ @@ -49,6 +51,7 @@ public void start(BundleContext context) throws Exception { workspace = workspaceTracker.getService(); compileCommandsMonitor = new CompileCommandsMonitor(workspace).start(); cProjectChangeMonitor = new CProjectChangeMonitor().start(); + configFileMonitor = new ClangdConfigFileMonitor(workspace).start(); } @Override @@ -56,6 +59,7 @@ public void stop(BundleContext context) throws Exception { plugin = null; compileCommandsMonitor.stop(); cProjectChangeMonitor.stop(); + configFileMonitor.stop(); super.stop(context); } diff --git a/tests/org.eclipse.cdt.lsp.clangd.tests/src/org/eclipse/cdt/lsp/internal/clangd/tests/ClangdConfigFileCheckerTest.java b/tests/org.eclipse.cdt.lsp.clangd.tests/src/org/eclipse/cdt/lsp/internal/clangd/tests/ClangdConfigFileCheckerTest.java new file mode 100644 index 00000000..f4edb956 --- /dev/null +++ b/tests/org.eclipse.cdt.lsp.clangd.tests/src/org/eclipse/cdt/lsp/internal/clangd/tests/ClangdConfigFileCheckerTest.java @@ -0,0 +1,149 @@ +/******************************************************************************* + * Copyright (c) 2024 Bachmann electronic GmbH and others. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Gesa Hentschke (Bachmann electronic GmbH) - initial implementation + *******************************************************************************/ + +package org.eclipse.cdt.lsp.internal.clangd.tests; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.UnsupportedEncodingException; + +import org.eclipse.cdt.lsp.clangd.ClangdConfigurationFileManager; +import org.eclipse.cdt.lsp.clangd.tests.TestUtils; +import org.eclipse.cdt.lsp.internal.clangd.ClangdConfigFileChecker; +import org.eclipse.cdt.lsp.internal.clangd.ClangdConfigFileMonitor; +import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IMarker; +import org.eclipse.core.resources.IProject; +import org.eclipse.core.resources.IResource; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.NullProgressMonitor; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; + +class ClangdConfigFileCheckerTest { + private static final String ERROR_MSG = ".clangd problem marker has not been added to the config file!"; + private static final String VALID_YAML_SYNTAX = "\r\nCompileFlags: {CompilationDatabase: Release}\r\n"; + private static final String INVALID_YAML_SYNTAX_TAB_ONLY = "\t"; + private static final String INVALID_YAML_SYNTAX_CONTAINS_TAB = "\r\nCompileFlags:\n\tCompilationDatabase: Release"; + private static final String INVALID_YAML_SYNTAX_MISSING_BRACE = "CompileFlags: {CompilationDatabase: Release\r\n"; + + private IProject project; + + @BeforeEach + public void setUp(TestInfo testInfo) throws CoreException { + var projectName = TestUtils.getName(testInfo); + project = TestUtils.createCProject(projectName); + } + + @AfterEach + public void cleanUp() throws CoreException { + TestUtils.deleteProject(project); + } + + private IFile createConfigFile(String content) throws UnsupportedEncodingException, IOException, CoreException { + var file = project.getFile(ClangdConfigurationFileManager.CLANGD_CONFIG_FILE_NAME); + try (final var data = new ByteArrayInputStream(content.getBytes(project.getDefaultCharset()))) { + if (!file.exists()) { + file.create(data, false, new NullProgressMonitor()); + } else { + file.setContents(data, IResource.KEEP_HISTORY, new NullProgressMonitor()); + } + } + return file; + } + + /** + * Test valid yaml syntax. No marker should be added. + * + * @throws IOException + * @throws CoreException + */ + @Test + void testValidYamlSyntax() throws IOException, CoreException { + // GIVEN an existing .clangd configuration file with valid yaml syntax: + var configFile = createConfigFile(VALID_YAML_SYNTAX); + // WHEN the ClangdConfigFileChecker().checkConfigFile get called on the configFile: + new ClangdConfigFileChecker().checkConfigFile(configFile); + // THEN we expect that NO ClangdConfigFileChecker.CLANGD_MARKER has been added: + var marker = configFile.findMarkers(ClangdConfigFileChecker.CLANGD_MARKER, false, IResource.DEPTH_ZERO); + assertNotNull(marker); + assertEquals(0, marker.length, "Expected no marker for valid yaml syntax"); + } + + /** + * Test whether a .clangd yaml Problem marker will be added to the .clangd file if the file contains invalid yaml syntax (here: tab only) + * + * @throws IOException + * @throws CoreException + */ + @Test + void testInvalidYamlSyntaxTabOnly() throws IOException, CoreException { + // GIVEN an existing .clangd configuration file with invalid yaml syntax (contains tab only): + var configFile = createConfigFile(INVALID_YAML_SYNTAX_TAB_ONLY); + // WHEN the ClangdConfigFileChecker().checkConfigFile get called on the configFile: + new ClangdConfigFileChecker().checkConfigFile(configFile); + // THEN we expect that an ClangdConfigFileChecker.CLANGD_MARKER has been added: + var marker = configFile.findMarkers(ClangdConfigFileChecker.CLANGD_MARKER, false, IResource.DEPTH_ZERO); + assertNotNull(marker); + assertEquals(1, marker.length, ERROR_MSG); + } + + /** + * Test whether a .clangd yaml Problem marker will be added to the .clangd file if the file contains invalid yaml syntax (here: tab) + * + * @throws IOException + * @throws CoreException + */ + @Test + void testInvalidYamlSyntaxContainsTab() throws IOException, CoreException { + // GIVEN an existing .clangd configuration file with invalid yaml syntax (contains tab): + var configFile = createConfigFile(INVALID_YAML_SYNTAX_CONTAINS_TAB); + // WHEN the ClangdConfigFileChecker().checkConfigFile get called on the configFile: + new ClangdConfigFileChecker().checkConfigFile(configFile); + // THEN we expect that an ClangdConfigFileChecker.CLANGD_MARKER has been added: + var marker = configFile.findMarkers(ClangdConfigFileChecker.CLANGD_MARKER, false, IResource.DEPTH_ZERO); + assertNotNull(marker); + assertEquals(1, marker.length, ERROR_MSG); + } + + /** + * Test whether a .clangd yaml Problem marker will be added to the .clangd file if the file contains invalid yaml syntax (here: missing closing brace) + * because the {@link ClangdConfigFileMonitor#checkJob} should have been run after a delay of 100ms. + * + * @throws IOException + * @throws CoreException + * @throws InterruptedException + */ + @Test + void testInvalidYamlSyntaxMissingBrace() throws IOException, CoreException, InterruptedException { + // GIVEN an existing .clangd configuration file with invalid yaml syntax (missing closing brace): + // WHEN a new .clang file gets added to the project (should trigger ClangdConfigFileMonitor.checkJob). + var configFile = createConfigFile(INVALID_YAML_SYNTAX_MISSING_BRACE); + int timeoutCnt = 0; + var marker = new IMarker[] {}; + do { + Thread.sleep(50); + marker = configFile.findMarkers(ClangdConfigFileChecker.CLANGD_MARKER, false, IResource.DEPTH_ZERO); + timeoutCnt++; + } while (marker.length == 0 && timeoutCnt < 20); + // THEN we expect that an ClangdConfigFileChecker.CLANGD_MARKER has been added in the meantime, + // because the ClangdConfigFileMonitor.checkJob, which calls the ClangdConfigFileChecker().checkConfigFile, should have been run after a delay of 100ms: + assertEquals(1, marker.length, ERROR_MSG); + } + +}