Skip to content
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.

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

[#247] Add .clangd configuration file syntax checker #249

Merged
merged 5 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
11 changes: 11 additions & 0 deletions bundles/org.eclipse.cdt.lsp.clangd/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -110,5 +110,16 @@
</command>
</menuContribution>
</extension>
<extension
id="org.eclipse.cdt.lsp.clangd.config.marker"
name="Clangd Marker"
point="org.eclipse.core.resources.markers">
<super
type="org.eclipse.core.resources.problemmarker">
</super>
<persistent
value="true">
</persistent>
</extension>

</plugin>
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
/*******************************************************************************
* 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 java.util.Optional;
import java.util.regex.Pattern;

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.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.IDocument;
import org.yaml.snakeyaml.Yaml;

/**
* Checks the <code>.clangd</code> 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$
private final Pattern pattern = Pattern.compile(".*line (\\d+), column (\\d+).*"); //$NON-NLS-1$
private boolean temporaryLoadedFile = false;

/**
* Checks if the .clangd file contains valid yaml syntax. Adds error marker to the file if not.
* @param configFile
*/
public void checkConfigFile(IFile configFile) {
Yaml yaml = new Yaml();
try (var inputStream = configFile.getContents()) {
jonahgraham marked this conversation as resolved.
Show resolved Hide resolved
try {
removeMarkerFromClangdConfig(configFile);
//throws ScannerException and ParserException:
yaml.load(inputStream);
} catch (Exception yamlException) {
addMarkerToClangdConfig(configFile, yamlException);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many Yaml exceptions - like the one raised in #245 (comment) - are of type MarkedYAMLException which have some utility methods like getProblemMark and getProblem which will allow you to deconstruct the exception without having to parse the string.

I haven't tried to use them in this context, but it looks like it should do the job

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for hint! It's much easier!

}
} catch (IOException | CoreException e) {
Platform.getLog(getClass()).error(e.getMessage(), e);
}
}

private void addMarkerToClangdConfig(IFile configFile, Exception e) {
try {
var configMarker = parseYamlException(e, configFile);
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 class ClangdConfigMarker {
public String message;
public int line = 1;
public int charStart = -1;
public int charEnd = -1;
jonahgraham marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Fetch line and char position information from exception to create a marker for the .clangd file.
* @param e
* @param file
* @return
*/
private ClangdConfigMarker parseYamlException(Exception e, IFile file) {
var marker = new ClangdConfigMarker();
marker.message = getErrorMessage(e);
var doc = getDocument(file);
if (doc == null) {
return marker;
}
int startLine = -1;
int endLine = -1;
for (var line : toLines(e.getMessage())) {
var matcher = pattern.matcher(line);
if (matcher.matches()) {
var lineInt = Integer.parseInt(matcher.replaceAll("$1")); //$NON-NLS-1$
var column = Integer.parseInt(matcher.replaceAll("$2")); //$NON-NLS-1$
if (startLine == -1) {
startLine = lineInt;
} else if (endLine == -1) {
endLine = lineInt;
}
try {
if (marker.charStart == -1 && startLine > -1) {
var lineOffset = doc.getLineOffset(startLine - 1);
marker.charStart = lineOffset + column - 1;
} else if (marker.charEnd == -1 && endLine > -1) {
var lineOffset = doc.getLineOffset(endLine - 1);
marker.charEnd = lineOffset + column - 1;
}
} catch (BadLocationException bl) {
Platform.getLog(getClass()).error(bl.getMessage(), bl);
}
if (startLine > -1 && endLine > -1)
break;
}
}
//check if endChar has been found:
if (marker.charEnd == -1) {
if (marker.charStart < doc.getLength() - 1) {
marker.charEnd = marker.charStart + 1;
} else if (marker.charStart == doc.getLength() - 1 && marker.charStart > 0) {
marker.charEnd = marker.charStart;
marker.charStart--;
} else {
marker.charStart = 0;
marker.charEnd = 1;
}
}
cleanUp(file);
if (startLine > -1) {
marker.line = startLine;
}
return marker;
}

private String[] toLines(String message) {
return Optional.ofNullable(message).map(m -> m.lines().toArray(String[]::new)).orElse(new String[] {});
}

private String getErrorMessage(Exception e) {
return Optional.ofNullable(e.getLocalizedMessage())
.map(m -> m.replaceAll("[" + System.lineSeparator() + "]", " ")) //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
.orElse("Unknown yaml error"); //$NON-NLS-1$
}

private void removeMarkerFromClangdConfig(IFile configFile) {
try {
configFile.deleteMarkers(CLANGD_MARKER, false, IResource.DEPTH_INFINITE);
} catch (CoreException e) {
Platform.getLog(getClass()).log(e.getStatus());
}
}

private IDocument getDocument(IFile file) {
IDocument document = FileUtils.getDocumentFromBuffer(file);
if (document != null)
return document;
document = FileUtils.loadFileTemporary(file);
if (document != null)
temporaryLoadedFile = true;
return document;
}

private void cleanUp(IFile file) {
if (temporaryLoadedFile) {
FileUtils.disconnectTemporaryLoadedFile(file);
temporaryLoadedFile = false;
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*******************************************************************************
* 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.cdt.lsp.internal.clangd.editor.ClangdPlugin;
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.Status;
import org.eclipse.ui.statushandlers.StatusManager;

/**
* Monitor changes in <code>.clangd</code> files in the workspace and triggers a yaml checker
* to add error markers to the <code>.clangd</code> 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<IFile> 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.getKind() == IResourceDelta.REMOVED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the .clangd file is removed we probably don't want to run the checker on it. My reading of this code seems to suggest that a CoreException will be thrown as a result of configFile.getContents()

|| (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) {
StatusManager.getManager().handle(e, ClangdPlugin.PLUGIN_ID);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a StatusManager handle here, but not in the other places that add exceptions to the log?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, copy and paste error ;-)

}
}
}
};

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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*******************************************************************************
* 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.Optional;

import org.eclipse.core.filebuffers.FileBuffers;
import org.eclipse.core.filebuffers.ITextFileBuffer;
import org.eclipse.core.filebuffers.ITextFileBufferManager;
import org.eclipse.core.filebuffers.LocationKind;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.Platform;
import org.eclipse.jface.text.IDocument;

public class FileUtils {

private FileUtils() {
// do not instantiate
}

/**
* Loads a files document for temporary usage. {@link FileUtils#disconnectTemporaryLoadedFile(IFile)} has to be called on the file after usage!
* @param file
* @return temporary loaded document for the given file or null.
*/
public static IDocument loadFileTemporary(IFile file) {
if (file == null) {
return null;
}
IDocument document = null;

if (file.getType() == IResource.FILE) {
var bufferManager = getBufferManager();
if (bufferManager == null)
return document;
try {
bufferManager.connect(file.getFullPath(), LocationKind.IFILE, new NullProgressMonitor());
} catch (CoreException e) {
Platform.getLog(FileUtils.class).error(e.getMessage(), e);
return document;
}

ITextFileBuffer buffer = bufferManager.getTextFileBuffer(file.getFullPath(), LocationKind.IFILE);
if (buffer != null) {
document = buffer.getDocument();
}
}

return document;
}

/**
* When a files document has been obtained via {@link FileUtils#loadFileTemporary(IFile)}, then the file has to be disconnected from it's buffer manager.
* @param file
*/
public static void disconnectTemporaryLoadedFile(IFile file) {
Optional.ofNullable(getBufferManager()).ifPresent(bm -> {
try {
bm.disconnect(file.getFullPath(), LocationKind.IFILE, new NullProgressMonitor());
} catch (CoreException e) {
Platform.getLog(FileUtils.class).error(e.getMessage(), e);
}
});
}

/**
* Tries to fetch the document for the given file. Returns the document when the file is already in the text file buffer or <code>null</code> if not.
* @param file
* @return document for the given file or <code>null</code>
*/
public static IDocument getDocumentFromBuffer(IFile file) {
if (file == null) {
return null;
}
return Optional.ofNullable(getBufferManager())
.map(bm -> bm.getTextFileBuffer(file.getFullPath(), LocationKind.IFILE)).map(b -> b.getDocument())
.orElse(null);
}

private static ITextFileBufferManager getBufferManager() {
return FileBuffers.getTextFileBufferManager();
}

}
Loading
Loading