Fix: Refactor BibtexParser to use ranges of articles and work with them in the LSP instead of string matching

jablib/src/main/java/org/jabref/logic/importer/ParserResult.java

@@ -3,6 +3,7 @@
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.IdentityHashMap;
 import java.util.List;
@@ -17,11 +18,15 @@
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.BibEntryType;
 import org.jabref.model.entry.field.Field;
+import org.jabref.model.entry.field.InternalField;
 import org.jabref.model.metadata.MetaData;
 
+import com.google.common.collect.Multimap;
+import com.google.common.collect.MultimapBuilder;
+
 public class ParserResult {
     private final Set<BibEntryType> entryTypes;
-    private final List<String> warnings = new ArrayList<>();
+    private final Multimap<Range, String> warnings;
     private BibDatabase database;
     private MetaData metaData;
     private Path file;
@@ -47,11 +52,12 @@ public ParserResult(BibDatabase database, MetaData metaData, Set<BibEntryType> e
         this.database = Objects.requireNonNull(database);
         this.metaData = Objects.requireNonNull(metaData);
         this.entryTypes = Objects.requireNonNull(entryTypes);
+        this.warnings = MultimapBuilder.hashKeys().hashSetValues().build();
     }
 
     public static ParserResult fromErrorMessage(String message) {
         ParserResult parserResult = new ParserResult();
-        parserResult.addWarning(message);
+        parserResult.addWarning(Range.NULL_RANGE, message);
         parserResult.setInvalid(true);
         return parserResult;
     }
@@ -95,25 +101,31 @@ public void setPath(Path path) {
     /**
      * Add a parser warning.
      *
-     * @param s String Warning text. Must be pretranslated. Only added if there isn't already a dupe.
+     * @param s String Warning text. Must be pre-translated. Only added if there isn't already a dupe.
      */
     public void addWarning(String s) {
-        if (!warnings.contains(s)) {
-            warnings.add(s);
-        }
+        addWarning(Range.NULL_RANGE, s);
     }
 
-    public void addException(Exception exception) {
+    public void addWarning(Range range, String s) {
+        warnings.put(range, s);
+    }
+
+    public void addException(Range range, Exception exception) {
         String errorMessage = getErrorMessage(exception);
-        addWarning(errorMessage);
+        addWarning(range, errorMessage);
     }
 
     public boolean hasWarnings() {
         return !warnings.isEmpty();
     }
 
     public List<String> warnings() {
-        return new ArrayList<>(warnings);
+        return new ArrayList<>(warnings.values());
+    }
+
+    public Multimap<Range, String> getWarningsMap() {
+        return warnings;
     }
 
     public boolean isInvalid() {
@@ -162,7 +174,45 @@ public Map<BibEntry, Range> getArticleRanges() {
         return articleRanges;
     }
 
-    public record Range(int startLine, int startColumn, int endLine, int endColumn) {
+    public record Range(
+            int startLine,
+            int startColumn,
+            int endLine,
+            int endColumn) {
         public static final Range NULL_RANGE = new Range(0, 0, 0, 0);
+
+        public Range(int startLine, int startColumn) {
+            this(startLine, startColumn, startLine, startColumn);
+        }
+    }
+
+    /// Returns a `Range` indicating that a complete entry is hit. We use the line of the key. No key is found, the complete entry range is used.
+    public Range getFieldRange(BibEntry entry, Field field) {
+        Map<Field, Range> rangeMap = fieldRanges.getOrDefault(entry, Collections.emptyMap());
+
+        if (rangeMap.isEmpty()) {
+            return Range.NULL_RANGE;
+        }
+
+        Range range = rangeMap.get(field);
+        if (range != null) {
+            return range;
+        }
+
+        return field.getAlias()
+                    .map(rangeMap::get)
+                    .orElseGet(() -> getCompleteEntryIndicator(entry));
+    }
+
+    /// Returns a `Range` indicating that a complete entry is hit. We use the line of the key. No key is found, the complete entry range is used.
+    public Range getCompleteEntryIndicator(BibEntry entry) {
+        Map<Field, Range> rangeMap = fieldRanges.getOrDefault(entry, Collections.emptyMap());
+        Range range = rangeMap.get(InternalField.KEY_FIELD);
+        if (range != null) {
+            // this ensures that the line is highlighted from the beginning of the entry so it highlights "@Article{key," (but only if on the same line) and not just the citation key
+            return new Range(range.startLine(), 0, range.endLine(), range.endColumn());
+        }
+
+        return articleRanges.getOrDefault(entry, Range.NULL_RANGE);
     }
 }

jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java

@@ -96,6 +96,7 @@ public class BibtexParser implements Parser {
     private static final String BIB_DESK_ROOT_GROUP_NAME = "BibDeskGroups";
     private static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY = DocumentBuilderFactory.newInstance();
     private static final int INDEX_RELATIVE_PATH_IN_PLIST = 4;
+    private final Pattern epilogPattern;
     private final Deque<Character> pureTextFromFile = new LinkedList<>();
     private final ImportFormatPreferences importFormatPreferences;
     private PushbackReader pushbackReader;
@@ -120,6 +121,7 @@ public BibtexParser(ImportFormatPreferences importFormatPreferences, FileUpdateM
         this.importFormatPreferences = Objects.requireNonNull(importFormatPreferences);
         this.metaDataParser = new MetaDataParser(fileMonitor);
         this.parsedBibDeskGroups = new HashMap<>();
+        this.epilogPattern = Pattern.compile("\\w+\\s*=.*,");
     }
 
     public BibtexParser(ImportFormatPreferences importFormatPreferences) {
@@ -270,6 +272,8 @@ private ParserResult parseFileContent() throws IOException {
 
         addBibDeskGroupEntriesToJabRefGroups();
 
+        int startLine = line;
+        int startColumn = column;
         try {
             MetaData metaData = metaDataParser.parse(
                     meta,
@@ -296,7 +300,7 @@ private ParserResult parseFileContent() throws IOException {
             }
             parserResult.setMetaData(metaData);
         } catch (ParseException exception) {
-            parserResult.addException(exception);
+            parserResult.addException(new ParserResult.Range(startLine, startColumn, line, column), exception);
         }
 
         parseRemainingContent();
@@ -309,8 +313,8 @@ private ParserResult parseFileContent() throws IOException {
     private void checkEpilog() {
         // This is an incomplete and inaccurate try to verify if something went wrong with previous parsing activity even though there were no warnings so far
         // regex looks for something like 'identifier = blabla ,'
-        if (!parserResult.hasWarnings() && Pattern.compile("\\w+\\s*=.*,").matcher(database.getEpilog()).find()) {
-            parserResult.addWarning("following BibTex fragment has not been parsed:\n" + database.getEpilog());
+        if (!parserResult.hasWarnings() && epilogPattern.matcher(database.getEpilog()).find()) {
+            parserResult.addWarning(new ParserResult.Range(line, column, line, column), "following BibTeX fragment has not been parsed:\n" + database.getEpilog());
         }
     }
 
@@ -319,6 +323,8 @@ private void parseRemainingContent() {
     }
 
     private void parseAndAddEntry(String type) {
+        int startLine = line;
+        int startColumn = column;
         try {
             // collect all comments and the entry type definition in front of the actual entry
             // this is at least `@Type`
@@ -347,13 +353,16 @@ private void parseAndAddEntry(String type) {
             // This makes the parser more robust:
             // If an exception is thrown when parsing an entry, drop the entry and try to resume parsing.
             LOGGER.warn("Could not parse entry", ex);
-            parserResult.addWarning(Localization.lang("Error occurred when parsing entry") + ": '" + ex.getMessage()
-                    + "'. " + "\n\n" + Localization.lang("JabRef skipped the entry."));
+            String errorMessage = Localization.lang("Error occurred when parsing entry") + ": '" + ex.getMessage()
+                    + "'. " + "\n\n" + Localization.lang("JabRef skipped the entry.");
+            parserResult.addWarning(new ParserResult.Range(startLine, startColumn, line, column), errorMessage);
         }
     }
 
     private void parseJabRefComment(Map<String, String> meta) {
         StringBuilder buffer;
+        int startLine = line;
+        int startColumn = column;
         try {
             buffer = parseBracketedFieldContent();
         } catch (IOException e) {
@@ -385,7 +394,7 @@ private void parseJabRefComment(Map<String, String> meta) {
             if (typ.isPresent()) {
                 entryTypes.add(typ.get());
             } else {
-                parserResult.addWarning(Localization.lang("Ill-formed entrytype comment in BIB file") + ": " + comment);
+                parserResult.addWarning(new ParserResult.Range(startLine, startColumn, line, column), Localization.lang("Ill-formed entrytype comment in BIB file") + ": " + comment);
             }
 
             // custom entry types are always re-written by JabRef and not stored in the file
@@ -394,7 +403,7 @@ private void parseJabRefComment(Map<String, String> meta) {
             try {
                 parseBibDeskComment(comment, meta);
             } catch (ParseException ex) {
-                parserResult.addException(ex);
+                parserResult.addException(new ParserResult.Range(startLine, startColumn, line, column), ex);
             }
         }
     }
@@ -461,11 +470,13 @@ private void parseBibDeskComment(String comment, Map<String, String> meta) throw
     }
 
     private void parseBibtexString() throws IOException {
+        int startLine = line;
+        int startColumn = column;
         BibtexString bibtexString = parseString();
         try {
             database.addString(bibtexString);
         } catch (KeyCollisionException ex) {
-            parserResult.addWarning(Localization.lang("Duplicate string name: '%0'", bibtexString.getName()));
+            parserResult.addWarning(new ParserResult.Range(startLine, startColumn, line, column), Localization.lang("Duplicate string name: '%0'", bibtexString.getName()));
         }
     }
 
@@ -920,19 +931,19 @@ private String fixKey() throws IOException {
 
                         // Finished, now reverse newKey and remove whitespaces:
                         key = newKey.reverse();
-                        parserResult.addWarning(
+                        parserResult.addWarning(new ParserResult.Range(line, column),
                                 Localization.lang("Line %0: Found corrupted citation key %1.", String.valueOf(line), key.toString()));
                     }
                 }
                 break;
 
             case ',':
-                parserResult.addWarning(
+                parserResult.addWarning(new ParserResult.Range(line, column),
                         Localization.lang("Line %0: Found corrupted citation key %1 (contains whitespaces).", String.valueOf(line), key.toString()));
                 break;
 
             case '\n':
-                parserResult.addWarning(
+                parserResult.addWarning(new ParserResult.Range(line, column),
                         Localization.lang("Line %0: Found corrupted citation key %1 (comma missing).", String.valueOf(line), key.toString()));
                 break;
 

jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultErrorFormatWriter.java

@@ -4,11 +4,8 @@
 import java.io.Writer;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.Map;
 
 import org.jabref.logic.importer.ParserResult;
-import org.jabref.model.entry.field.Field;
-import org.jabref.model.entry.field.InternalField;
 
 public class IntegrityCheckResultErrorFormatWriter extends IntegrityCheckResultWriter {
 
@@ -24,9 +21,7 @@ public IntegrityCheckResultErrorFormatWriter(Writer writer, List<IntegrityMessag
     @Override
     public void writeFindings() throws IOException {
         for (IntegrityMessage message : messages) {
-            Map<Field, ParserResult.Range> fieldRangeMap = parserResult.getFieldRanges().getOrDefault(message.entry(), Map.of());
-            ParserResult.Range fieldRange = fieldRangeMap.getOrDefault(message.field(), fieldRangeMap.getOrDefault(InternalField.KEY_FIELD, parserResult.getArticleRanges().getOrDefault(message.entry(), ParserResult.Range.NULL_RANGE)));
-
+            ParserResult.Range fieldRange = parserResult.getFieldRange(message.entry(), message.field());
             writer.append("%s:%d:%d: %s\n".formatted(
                     inputFile,
                     fieldRange.startLine(),

jablib/src/main/java/org/jabref/model/entry/field/Field.java

@@ -3,6 +3,7 @@
 import java.util.EnumSet;
 import java.util.Optional;
 
+import org.jabref.model.entry.EntryConverter;
 import org.jabref.model.strings.StringUtil;
 
 public interface Field {
@@ -36,7 +37,7 @@ default boolean isDeprecated() {
     }
 
     default Optional<Field> getAlias() {
-        return Optional.empty();
+        return Optional.ofNullable(EntryConverter.FIELD_ALIASES.get(this));
     }
 
     default boolean isNumeric() {

jabls/build.gradle.kts

@@ -14,6 +14,8 @@ dependencies {
     implementation("org.eclipse.lsp4j:org.eclipse.lsp4j")
     implementation("org.eclipse.lsp4j:org.eclipse.lsp4j.websocket")
 
+    implementation("com.google.guava:guava")
+
     // route all requests to java.util.logging to SLF4J (which in turn routes to tinylog)
     testImplementation("org.slf4j:jul-to-slf4j")
 }

jabls/src/main/java/module-info.java

@@ -6,10 +6,12 @@
 
     requires org.jabref.jablib;
 
+    requires com.google.common;
+    requires com.google.gson;
+
     requires org.slf4j;
 
     requires org.eclipse.lsp4j;
     requires org.eclipse.lsp4j.jsonrpc;
     requires org.eclipse.lsp4j.websocket;
-    requires com.google.gson;
 }

jabls/src/main/java/org/jabref/languageserver/util/LspConsistencyCheck.java

@@ -8,9 +8,9 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.quality.consistency.BibliographyConsistencyCheck;
-import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntryType;
 import org.jabref.model.entry.BibEntryTypesManager;
 import org.jabref.model.entry.field.BibField;
@@ -21,10 +21,10 @@
 
 public class LspConsistencyCheck {
 
-    public List<Diagnostic> check(BibDatabaseContext bibDatabaseContext, String content) {
+    public List<Diagnostic> check(ParserResult parserResult) {
         List<Diagnostic> diagnostics = new ArrayList<>();
         BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck();
-        BibliographyConsistencyCheck.Result result = consistencyCheck.check(bibDatabaseContext, (_, _) -> {
+        BibliographyConsistencyCheck.Result result = consistencyCheck.check(parserResult.getDatabaseContext(), (_, _) -> {
         });
 
         List<Field> allReportedFields = result.entryTypeToResultMap().values().stream()
@@ -34,7 +34,7 @@ public List<Diagnostic> check(BibDatabaseContext bibDatabaseContext, String cont
                                               .toList();
 
         result.entryTypeToResultMap().forEach((entryType, entryTypeResult) -> {
-            Optional<BibEntryType> bibEntryType = new BibEntryTypesManager().enrich(entryType, bibDatabaseContext.getMode());
+            Optional<BibEntryType> bibEntryType = new BibEntryTypesManager().enrich(entryType, parserResult.getDatabaseContext().getMode());
             Set<Field> requiredFields = bibEntryType
                     .map(BibEntryType::getRequiredFields)
                     .stream()
@@ -44,9 +44,8 @@ public List<Diagnostic> check(BibDatabaseContext bibDatabaseContext, String cont
 
             entryTypeResult.sortedEntries().forEach(entry -> requiredFields.forEach(requiredField -> {
                 if (entry.getFieldOrAlias(requiredField).isEmpty()) {
-                    LspDiagnosticBuilder diagnosticBuilder = LspDiagnosticBuilder.create(Localization.lang("Required field \"%0\" is empty.", requiredField.getName()));
+                    LspDiagnosticBuilder diagnosticBuilder = LspDiagnosticBuilder.create(parserResult, Localization.lang("Required field \"%0\" is empty.", requiredField.getName()));
                     diagnosticBuilder.setSeverity(DiagnosticSeverity.Error);
-                    diagnosticBuilder.setContent(content);
                     diagnosticBuilder.setEntry(entry);
                     diagnostics.add(diagnosticBuilder.build());
                 }
@@ -62,8 +61,7 @@ public List<Diagnostic> check(BibDatabaseContext bibDatabaseContext, String cont
 
             optionalFields.forEach(optionalField -> entryTypeResult.sortedEntries().forEach(entry -> {
                 if (entry.getFieldOrAlias(optionalField).isEmpty()) {
-                    LspDiagnosticBuilder diagnosticBuilder = LspDiagnosticBuilder.create(Localization.lang("Optional field \"%0\" is empty.", optionalField.getName()));
-                    diagnosticBuilder.setContent(content);
+                    LspDiagnosticBuilder diagnosticBuilder = LspDiagnosticBuilder.create(parserResult, Localization.lang("Optional field \"%0\" is empty.", optionalField.getName()));
                     diagnosticBuilder.setEntry(entry);
                     diagnostics.add(diagnosticBuilder.build());
                 }