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

[fix] error diagnostics with correct position #146

Merged
merged 4 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
- Improved heuristics for guessing source roots. It looks directory structures such as `src/java` or `main/java`. | [#126](https://github.com/JetBrains/bazel-bsp/pull/126)

### Fixes 🛠️
- Now the project is built using bazel version `3.7.2`, as the rules currently used are no longer supported by bazel. | [141](https://github.com/JetBrains/bazel-bsp/pull/141)
- Error diagnostics are now also sent for source files, including targets. | [#146](https://github.com/JetBrains/bazel-bsp/pull/146)
- Now the project is built using bazel version `3.7.2`, as the rules currently used are no longer supported by bazel. | [#141](https://github.com/JetBrains/bazel-bsp/pull/141)


## [1.0.1] - 24.09.2021
Expand Down
4 changes: 4 additions & 0 deletions install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ echo -e "Building done."

echo -e "\nInstalling server at '$project_path' ..."
cd "$project_path" || { echo "cd $project_path failed! EXITING"; exit 155; }

rm -r .bsp/
rm -r .bazelbsp/

$bsp_path

echo -e "\nDone! Enjoy Bazel BSP!"
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.jetbrains.bsp.bazel.bazelrunner.data.BazelData;
import org.jetbrains.bsp.bazel.commons.Constants;
import org.jetbrains.bsp.bazel.commons.ExitCodeMapper;
import org.jetbrains.bsp.bazel.commons.Uri;
import org.jetbrains.bsp.bazel.server.bep.parsers.ClasspathParser;
import org.jetbrains.bsp.bazel.server.bep.parsers.error.FileDiagnostic;
import org.jetbrains.bsp.bazel.server.bep.parsers.error.StderrDiagnosticsParser;
import org.jetbrains.bsp.bazel.server.loggers.BuildClientLogger;

Expand Down Expand Up @@ -258,26 +260,34 @@ private void processProgressEvent(BuildEventStreamProtos.BuildEvent event) {
}

private void consumeProgressEvent(BuildEventStreamProtos.Progress progress) {
Map<String, List<Diagnostic>> fileDiagnostics =
StderrDiagnosticsParser.parse(progress.getStderr());

fileDiagnostics.entrySet().stream()
StderrDiagnosticsParser.parse(progress.getStderr()).entrySet().stream()
.map(this::createParamsFromEntry)
.forEach(bspClient::onBuildPublishDiagnostics);

logProgress(progress);
}

private PublishDiagnosticsParams createParamsFromEntry(
Map.Entry<String, List<Diagnostic>> entry) {
String fileLocation = entry.getKey();
List<Diagnostic> diagnostics = entry.getValue();

return new PublishDiagnosticsParams(
new TextDocumentIdentifier(Uri.fromAbsolutePath(fileLocation).toString()),
new BuildTargetIdentifier(""),
diagnostics,
false);
Map.Entry<String, List<FileDiagnostic>> entry) {
var rawFileLocation = entry.getKey();
var fileLocation = new TextDocumentIdentifier(Uri.fromAbsolutePath(rawFileLocation).toString());

var fileDiagnostics = entry.getValue();
var targetId = getTargetIdFromDiagnostics(fileDiagnostics);
var diagnostics = getDiagnosticsFromFileDiagnostics(fileDiagnostics);

return new PublishDiagnosticsParams(fileLocation, targetId, diagnostics, false);
}

private BuildTargetIdentifier getTargetIdFromDiagnostics(List<FileDiagnostic> diagnostics) {
return diagnostics.stream()
.findFirst()
.map(FileDiagnostic::getTarget)
.orElse(new BuildTargetIdentifier(""));
}

private List<Diagnostic> getDiagnosticsFromFileDiagnostics(List<FileDiagnostic> diagnostics) {
return diagnostics.stream().map(FileDiagnostic::getDiagnostic).collect(Collectors.toList());
}

private void logProgress(BuildEventStreamProtos.Progress progress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ load("@rules_java//java:defs.bzl", "java_library")
java_library(
name = "error",
srcs = glob(["*.java"]),
visibility = ["//server/src/main/java/org/jetbrains/bsp/bazel/server/bep:__subpackages__"],
visibility = ["//server:__subpackages__"],
deps = [
"//commons",
"@maven//:ch_epfl_scala_bsp4j",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,54 +1,146 @@
package org.jetbrains.bsp.bazel.server.bep.parsers.error;

import ch.epfl.scala.bsp4j.BuildTargetIdentifier;
import ch.epfl.scala.bsp4j.Diagnostic;
import ch.epfl.scala.bsp4j.DiagnosticSeverity;
import ch.epfl.scala.bsp4j.Position;
import ch.epfl.scala.bsp4j.Range;
import java.util.ArrayList;
import java.util.Objects;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

class FileDiagnostic {
public class FileDiagnostic {

private static final Logger LOGGER = LogManager.getLogger(FileDiagnostic.class);

private final Diagnostic diagnostic;
private final String fileLocation;
private final BuildTargetIdentifier target;

private FileDiagnostic(Diagnostic diagnostic, String fileLocation) {
FileDiagnostic(Diagnostic diagnostic, String fileLocation, BuildTargetIdentifier target) {
this.diagnostic = diagnostic;
this.fileLocation = fileLocation;
this.target = target;
}

public static FileDiagnostic fromError(String error) {
public static Stream<FileDiagnostic> fromError(String error) {
LOGGER.info("Error: {}", error);

String erroredFile = ErrorFileParser.getFileWithError(error);
String fileInfo = ErrorFileParser.extractFileInfo(error);
String fileLocation = getFileLocation(erroredFile, fileInfo);
var targetId = findTargetId(error);

return createFileDiagnostic(error, fileLocation);
return Stream.concat(
Stream.of(createBUILDFileDiagnostic(error, targetId)),
createSourceFilesDiagnostics(error, targetId));
}

private static FileDiagnostic createFileDiagnostic(String error, String fileLocation) {
Position position = ErrorPositionParser.getErrorPosition(error);
Range diagnosticRange = new Range(position, position);
Diagnostic diagnostic = new Diagnostic(diagnosticRange, error);
private static BuildTargetIdentifier findTargetId(String error) {
var matcher = Pattern.compile("//[^ :]*:?[^ :]+").matcher(error);

if (matcher.find()) {
var rawTargetId = matcher.group();
return new BuildTargetIdentifier(rawTargetId);
}

return new BuildTargetIdentifier("");
}

private static FileDiagnostic createBUILDFileDiagnostic(
String error, BuildTargetIdentifier targetId) {
var erroredFile = ErrorFileParser.getFileWithError(error);
var fileInfo = ErrorFileParser.extractFileInfo(error);
var fileLocation = getFileLocation(erroredFile, fileInfo);
var message = error.split("\n", 2)[0];

return createFileDiagnostic(message, fileLocation, targetId);
}

private static FileDiagnostic createFileDiagnostic(
String error, String fileLocation, BuildTargetIdentifier targetId) {
var position = ErrorPositionParser.getErrorPosition(error);
var diagnosticRange = new Range(position, position);
var diagnostic = new Diagnostic(diagnosticRange, error);
diagnostic.setSeverity(DiagnosticSeverity.ERROR);

return new FileDiagnostic(diagnostic, fileLocation);
return new FileDiagnostic(diagnostic, fileLocation, targetId);
}

private static String getFileLocation(String erroredFile, String fileInfo) {
int fileLocationUrlEndIndex = fileInfo.indexOf(erroredFile) + erroredFile.length();
var fileLocationUrlEndIndex = fileInfo.indexOf(erroredFile) + erroredFile.length();

return fileInfo.substring(0, fileLocationUrlEndIndex);
}

private static Stream<FileDiagnostic> createSourceFilesDiagnostics(
String error, BuildTargetIdentifier targetId) {
var pattern = Pattern.compile("\n((([^\\s\\/:]+\\/)*[^\\/:]+):(\\d+):[^\\^]*\\^)");
var matcher = pattern.matcher(error);

var result = new ArrayList<FileDiagnostic>();

while (matcher.find()) {
var message = matcher.group(1);
var filePath = matcher.group(2);
var errorLine = matcher.group(4);

result.add(createSourceFileDiagnostic(message, filePath, errorLine, targetId));
}

return result.stream();
}

private static FileDiagnostic createSourceFileDiagnostic(
String message, String filePath, String errorLine, BuildTargetIdentifier targetId) {
var position = new Position(Integer.parseInt(errorLine), 0);
var diagnosticRange = new Range(position, position);
var diagnostic = new Diagnostic(diagnosticRange, message);
diagnostic.setSeverity(DiagnosticSeverity.ERROR);

return new FileDiagnostic(diagnostic, filePath, targetId);
}

public Diagnostic getDiagnostic() {
return diagnostic;
}

public String getFileLocation() {
return fileLocation;
}

public BuildTargetIdentifier getTarget() {
return target;
}

@Override
public String toString() {
return "FileDiagnostic{"
+ "diagnostic="
+ diagnostic
+ ", fileLocation='"
+ fileLocation
+ '\''
+ ", target="
+ target
+ '}';
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
FileDiagnostic that = (FileDiagnostic) o;
// TODO normal .equals() fails don't know why...
return Objects.equals(diagnostic.getMessage(), that.diagnostic.getMessage())
&& Objects.equals(diagnostic.getRange(), that.diagnostic.getRange())
&& Objects.equals(diagnostic.getSeverity(), that.diagnostic.getSeverity())
&& Objects.equals(fileLocation, that.fileLocation)
&& Objects.equals(target, that.target);
}

@Override
public int hashCode() {
return Objects.hash(diagnostic, fileLocation, target);
}
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
package org.jetbrains.bsp.bazel.server.bep.parsers.error;

import ch.epfl.scala.bsp4j.Diagnostic;
import com.google.common.base.Splitter;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public final class StderrDiagnosticsParser {

private static final String ERROR = "ERROR";

private static final String STDERR_DELIMITER = "\n";

public static Map<String, List<Diagnostic>> parse(String stderr) {
return splitStderr(stderr).stream()
public static Map<String, List<FileDiagnostic>> parse(String stderr) {
return splitStderr(stderr)
.filter(StderrDiagnosticsParser::isError)
.filter(StderrDiagnosticsParser::isBazelError)
.map(FileDiagnostic::fromError)
.collect(
Collectors.groupingBy(
FileDiagnostic::getFileLocation,
Collectors.mapping(FileDiagnostic::getDiagnostic, Collectors.toList())));
.flatMap(FileDiagnostic::fromError)
.collect(Collectors.groupingBy(FileDiagnostic::getFileLocation, Collectors.toList()));
}

private static List<String> splitStderr(String stderr) {
return Splitter.on(STDERR_DELIMITER).splitToList(stderr);
private static Stream<String> splitStderr(String stderr) {
return Splitter.onPattern("(?=(ERROR:))").splitToList(stderr).stream();
}

private static boolean isError(String stderrPart) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
load("@rules_java//java:defs.bzl", "java_test")

java_test(
name = "FileDiagnosticTest",
size = "small",
srcs = ["FileDiagnosticTest.java"],
runtime_deps = [
"@maven//:junit_junit",
],
deps = [
"//server/src/main/java/org/jetbrains/bsp/bazel/server/bep/parsers/error",
"@maven//:ch_epfl_scala_bsp4j",
"@maven//:com_google_guava_guava",
],
)

java_test(
name = "StderrDiagnosticsParserTest",
size = "small",
srcs = ["StderrDiagnosticsParserTest.java"],
runtime_deps = [
"@maven//:junit_junit",
],
deps = [
"//server/src/main/java/org/jetbrains/bsp/bazel/server/bep/parsers/error",
"@maven//:ch_epfl_scala_bsp4j",
"@maven//:com_google_guava_guava",
],
)
Loading