diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c266844ca4..598e6ffffba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv ### Added +- We added the integrity check to the jabkit cli application. [#13848](https://github.com/JabRef/jabref/issues/13848) - We added support for Cygwin-file paths on a Windows Operating System. [#13274](https://github.com/JabRef/jabref/issues/13274) - We fixed an issue where "Print preview" would throw a `NullPointerException` if no printers were available. [#13708](https://github.com/JabRef/jabref/issues/13708) - We added the option to enable the language server in the preferences. [#13697](https://github.com/JabRef/jabref/pull/13697) diff --git a/docs/requirements/cli.md b/docs/requirements/cli.md new file mode 100644 index 00000000000..7694adac9dd --- /dev/null +++ b/docs/requirements/cli.md @@ -0,0 +1,14 @@ +--- +parent: Requirements +--- +# CLI + +## Unified `--input` option across all commands +`req~jabkit.cli.input-flag~1` + +All `jabkit` commands that need a file input must have the `--input` option to specify the input file. +See [ADR 45](../decisions/0045-use-input-flag-always-for-input-files.md) for more details. + +Needs: impl + + \ No newline at end of file diff --git a/jabkit/src/main/java/org/jabref/cli/ArgumentProcessor.java b/jabkit/src/main/java/org/jabref/cli/ArgumentProcessor.java index 35e77c12e7d..e48b5e4c197 100644 --- a/jabkit/src/main/java/org/jabref/cli/ArgumentProcessor.java +++ b/jabkit/src/main/java/org/jabref/cli/ArgumentProcessor.java @@ -41,7 +41,7 @@ // sorted alphabetically subcommands = { CheckConsistency.class, -// CheckIntegrity.class, + CheckIntegrity.class, Convert.class, Fetch.class, GenerateBibFromAux.class, diff --git a/jabkit/src/main/java/org/jabref/cli/CheckConsistency.java b/jabkit/src/main/java/org/jabref/cli/CheckConsistency.java index 59e369f1c5c..9af8ecce73f 100644 --- a/jabkit/src/main/java/org/jabref/cli/CheckConsistency.java +++ b/jabkit/src/main/java/org/jabref/cli/CheckConsistency.java @@ -33,6 +33,7 @@ class CheckConsistency implements Callable { @Mixin private ArgumentProcessor.SharedOptions sharedOptions = new ArgumentProcessor.SharedOptions(); + // [impl->req~jabkit.cli.input-flag~1] @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input BibTeX file", required = true) private Path inputFile; diff --git a/jabkit/src/main/java/org/jabref/cli/CheckIntegrity.java b/jabkit/src/main/java/org/jabref/cli/CheckIntegrity.java index 7186de34f56..b74ed7e42b0 100644 --- a/jabkit/src/main/java/org/jabref/cli/CheckIntegrity.java +++ b/jabkit/src/main/java/org/jabref/cli/CheckIntegrity.java @@ -1,37 +1,117 @@ package org.jabref.cli; +import java.io.IOException; +import java.io.OutputStreamWriter; +import java.io.Writer; import java.nio.file.Path; +import java.util.List; +import java.util.Locale; +import java.util.Optional; +import java.util.concurrent.Callable; +import java.util.stream.Collectors; import org.jabref.cli.converter.CygWinPathConverter; +import org.jabref.logic.importer.ParserResult; +import org.jabref.logic.integrity.IntegrityCheck; +import org.jabref.logic.integrity.IntegrityCheckResultCsvWriter; +import org.jabref.logic.integrity.IntegrityCheckResultErrorFormatWriter; +import org.jabref.logic.integrity.IntegrityCheckResultTxtWriter; +import org.jabref.logic.integrity.IntegrityCheckResultWriter; +import org.jabref.logic.integrity.IntegrityMessage; +import org.jabref.logic.journals.JournalAbbreviationLoader; import org.jabref.logic.l10n.Localization; +import org.jabref.model.database.BibDatabaseContext; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import picocli.CommandLine; import static picocli.CommandLine.Command; import static picocli.CommandLine.Mixin; import static picocli.CommandLine.Option; -import static picocli.CommandLine.Parameters; @Command(name = "check-integrity", description = "Check integrity of the database.") -class CheckIntegrity implements Runnable { +class CheckIntegrity implements Callable { + + private static final Logger LOGGER = LoggerFactory.getLogger(CheckIntegrity.class); + + @CommandLine.ParentCommand + private ArgumentProcessor argumentProcessor; @Mixin private ArgumentProcessor.SharedOptions sharedOptions = new ArgumentProcessor.SharedOptions(); - @Parameters(index = "0", converter = CygWinPathConverter.class, description = "BibTeX file to check", arity = "0..1") + // [impl->req~jabkit.cli.input-flag~1] + @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input BibTeX file", required = true) private Path inputFile; - @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input BibTeX file") - private Path inputOption; + @Option(names = {"--output-format"}, description = "Output format: errorformat, txt or csv", defaultValue = "errorformat") + private String outputFormat; - @Option(names = {"--output-format"}, description = "Output format: txt or csv") - private String outputFormat = "txt"; // FixMe: Default value? + // in BibTeX it could be preferences.getEntryEditorPreferences().shouldAllowIntegerEditionBibtex() + @Option(names = {"--allow-integer-edition"}, description = "Allows Integer edition", negatable = true, defaultValue = "true", fallbackValue = "true") + private boolean allowIntegerEdition; @Override - public void run() { + public Integer call() { + Optional parserResult = ArgumentProcessor.importFile( + inputFile, + "bibtex", + argumentProcessor.cliPreferences, + sharedOptions.porcelain); + if (parserResult.isEmpty()) { + System.out.println(Localization.lang("Unable to open file '%0'.", inputFile)); + return 2; + } + + if (parserResult.get().isInvalid()) { + System.out.println(Localization.lang("Input file '%0' is invalid and could not be parsed.", inputFile)); + return 2; + } + if (!sharedOptions.porcelain) { System.out.println(Localization.lang("Checking integrity of '%0'.", inputFile)); System.out.flush(); } - // TODO: Implement integrity checking + BibDatabaseContext databaseContext = parserResult.get().getDatabaseContext(); + + IntegrityCheck integrityCheck = new IntegrityCheck( + databaseContext, + argumentProcessor.cliPreferences.getFilePreferences(), + argumentProcessor.cliPreferences.getCitationKeyPatternPreferences(), + JournalAbbreviationLoader.loadRepository(argumentProcessor.cliPreferences.getJournalAbbreviationPreferences()), + allowIntegerEdition + ); + + List messages = databaseContext.getEntries().stream() + .flatMap(entry -> integrityCheck.checkEntry(entry).stream()) + .collect(Collectors.toList()); + + messages.addAll(integrityCheck.checkDatabase(databaseContext.getDatabase())); + + Writer writer = new OutputStreamWriter(System.out); + IntegrityCheckResultWriter checkResultWriter; + switch (outputFormat.toLowerCase(Locale.ROOT)) { + case "errorformat" -> + checkResultWriter = new IntegrityCheckResultErrorFormatWriter(writer, messages, parserResult.get(), inputFile); + case "txt" -> + checkResultWriter = new IntegrityCheckResultTxtWriter(writer, messages); + case "csv" -> + checkResultWriter = new IntegrityCheckResultCsvWriter(writer, messages); + default -> { + System.out.println(Localization.lang("Unknown output format '%0'.", outputFormat)); + return 3; + } + } + + try { + checkResultWriter.writeFindings(); + writer.flush(); + } catch (IOException e) { + LOGGER.error("Error writing results", e); + return 2; + } + return 0; } } diff --git a/jabkit/src/main/java/org/jabref/cli/Convert.java b/jabkit/src/main/java/org/jabref/cli/Convert.java index f2f2700462b..59617349f28 100644 --- a/jabkit/src/main/java/org/jabref/cli/Convert.java +++ b/jabkit/src/main/java/org/jabref/cli/Convert.java @@ -37,6 +37,7 @@ public class Convert implements Runnable { @Mixin private ArgumentProcessor.SharedOptions sharedOptions = new ArgumentProcessor.SharedOptions(); + // [impl->req~jabkit.cli.input-flag~1] @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input file", required = true) private Path inputFile; diff --git a/jabkit/src/main/java/org/jabref/cli/GenerateBibFromAux.java b/jabkit/src/main/java/org/jabref/cli/GenerateBibFromAux.java index 99d6c461a1a..ef88e995fa7 100644 --- a/jabkit/src/main/java/org/jabref/cli/GenerateBibFromAux.java +++ b/jabkit/src/main/java/org/jabref/cli/GenerateBibFromAux.java @@ -5,6 +5,7 @@ import java.util.Optional; import java.util.stream.Collectors; +import org.jabref.cli.converter.CygWinPathConverter; import org.jabref.logic.auxparser.AuxParser; import org.jabref.logic.auxparser.AuxParserResult; import org.jabref.logic.auxparser.AuxParserStatisticsProvider; @@ -35,8 +36,9 @@ class GenerateBibFromAux implements Runnable { @Option(names = "--aux", required = true) private Path auxFile; - @Option(names = "--input", required = true) - private String inputFile; + // [impl->req~jabkit.cli.input-flag~1] + @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input BibTeX file", required = true) + private Path inputFile; @Option(names = "--output") private Path outputFile; diff --git a/jabkit/src/main/java/org/jabref/cli/GenerateCitationKeys.java b/jabkit/src/main/java/org/jabref/cli/GenerateCitationKeys.java index 7aa8614a297..db4d2a820d3 100644 --- a/jabkit/src/main/java/org/jabref/cli/GenerateCitationKeys.java +++ b/jabkit/src/main/java/org/jabref/cli/GenerateCitationKeys.java @@ -4,6 +4,7 @@ import java.util.Optional; import java.util.stream.Collectors; +import org.jabref.cli.converter.CygWinPathConverter; import org.jabref.logic.citationkeypattern.CitationKeyGenerator; import org.jabref.logic.importer.ParserResult; import org.jabref.logic.l10n.Localization; @@ -28,8 +29,9 @@ public class GenerateCitationKeys implements Runnable { @Mixin private ArgumentProcessor.SharedOptions sharedOptions = new ArgumentProcessor.SharedOptions(); - @Option(names = "--input", description = "The input .bib file.", required = true) - private String inputFile; + // [impl->req~jabkit.cli.input-flag~1] + @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input BibTeX file", required = true) + private Path inputFile; @Option(names = "--output", description = "The output .bib file.") private Path outputFile; diff --git a/jabkit/src/main/java/org/jabref/cli/PdfUpdate.java b/jabkit/src/main/java/org/jabref/cli/PdfUpdate.java index 60d15ab1a5a..08ebb73d135 100644 --- a/jabkit/src/main/java/org/jabref/cli/PdfUpdate.java +++ b/jabkit/src/main/java/org/jabref/cli/PdfUpdate.java @@ -9,6 +9,7 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.TransformerException; +import org.jabref.cli.converter.CygWinPathConverter; import org.jabref.logic.FilePreferences; import org.jabref.logic.bibtex.FieldPreferences; import org.jabref.logic.exporter.EmbeddedBibFilePdfExporter; @@ -49,8 +50,9 @@ class PdfUpdate implements Runnable { @Option(names = {"-k", "--citation-key"}, description = "Citation keys", required = true) private List citationKeys = List.of(); // ToDo: check dedault value - @Option(names = "--input", description = "Input file", required = true) - private Path inputFile; // Local files only + // [impl->req~jabkit.cli.input-flag~1] + @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input BibTeX file", required = true) + private Path inputFile; @Option(names = "--input-format", description = "Input format of the file", required = true) private String inputFormat = "*"; diff --git a/jabkit/src/main/java/org/jabref/cli/Search.java b/jabkit/src/main/java/org/jabref/cli/Search.java index 1b77e4f7592..a2fb4526f29 100644 --- a/jabkit/src/main/java/org/jabref/cli/Search.java +++ b/jabkit/src/main/java/org/jabref/cli/Search.java @@ -47,6 +47,7 @@ class Search implements Runnable { @Option(names = {"--query"}, description = "Search query", required = true) private String query; + // [impl->req~jabkit.cli.input-flag~1] @Option(names = {"--input"}, converter = CygWinPathConverter.class, description = "Input BibTeX file", required = true) private Path inputFile; diff --git a/jablib/src/main/java/org/jabref/logic/importer/ParserResult.java b/jablib/src/main/java/org/jabref/logic/importer/ParserResult.java index c0a75bc4903..da436eb4af7 100644 --- a/jablib/src/main/java/org/jabref/logic/importer/ParserResult.java +++ b/jablib/src/main/java/org/jabref/logic/importer/ParserResult.java @@ -4,7 +4,9 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -14,6 +16,7 @@ import org.jabref.model.database.BibDatabases; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.BibEntryType; +import org.jabref.model.entry.field.Field; import org.jabref.model.metadata.MetaData; public class ParserResult { @@ -25,6 +28,9 @@ public class ParserResult { private boolean invalid; private boolean changedOnMigration = false; + private final Map articleRanges = new IdentityHashMap<>(); + private final Map> fieldRanges = new IdentityHashMap<>(); + public ParserResult() { this(List.of()); } @@ -147,4 +153,16 @@ public boolean getChangedOnMigration() { public void setChangedOnMigration(boolean wasChangedOnMigration) { this.changedOnMigration = wasChangedOnMigration; } + + public Map> getFieldRanges() { + return fieldRanges; + } + + public Map getArticleRanges() { + return articleRanges; + } + + public record Range(int startLine, int startColumn, int endLine, int endColumn) { + public static final Range NULL_RANGE = new Range(0, 0, 0, 0); + } } diff --git a/jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java b/jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java index 260312b337d..ea6c1423f89 100644 --- a/jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java +++ b/jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java @@ -7,6 +7,7 @@ import java.io.Reader; import java.io.StringWriter; import java.nio.file.Path; +import java.util.ArrayDeque; import java.util.Base64; import java.util.Collection; import java.util.Deque; @@ -46,6 +47,7 @@ import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; import org.jabref.model.entry.field.FieldProperty; +import org.jabref.model.entry.field.InternalField; import org.jabref.model.entry.field.StandardField; import org.jabref.model.entry.types.EntryTypeFactory; import org.jabref.model.groups.ExplicitGroup; @@ -90,7 +92,7 @@ */ public class BibtexParser implements Parser { private static final Logger LOGGER = LoggerFactory.getLogger(BibtexParser.class); - private static final Integer LOOKAHEAD = 1024; + private static final int LOOKAHEAD = 1024; 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; @@ -100,7 +102,14 @@ public class BibtexParser implements Parser { private BibDatabase database; private Set entryTypes; private boolean eof; + private int line = 1; + private int column = 1; + // Stores the last read column of the highest column number encountered on any line so far. + // The intended data structure is Stack, but it is not used because Java code style checkers complain. + // In basic JDK data structures, there is no size-limited stack. We did not want to include Apache Commons Collections only for "CircularFifoBuffer" + private final Deque highestColumns = new ArrayDeque<>(); + private ParserResult parserResult; private final MetaDataParser metaDataParser; private final Map parsedBibDeskGroups; @@ -634,6 +643,10 @@ private int read() throws IOException { } if (character == '\n') { line++; + highestColumns.push(column); + column = 1; + } else { + column++; } return character; } @@ -641,6 +654,9 @@ private int read() throws IOException { private void unread(int character) throws IOException { if (character == '\n') { line--; + column = highestColumns.pop(); + } else { + column--; } pushbackReader.unread(character); if (pureTextFromFile.getLast() == character) { @@ -679,13 +695,22 @@ private String parsePreamble() throws IOException { private BibEntry parseEntry(String entryType) throws IOException { BibEntry result = new BibEntry(EntryTypeFactory.parse(entryType)); + int articleStartLine = line; + int articleStartColumn = column; + skipWhitespace(); consume('{', '('); int character = peek(); if ((character != '\n') && (character != '\r')) { skipWhitespace(); } + int keyStartLine = line; + int keyStartColumn = column; String key = parseKey(); + + ParserResult.Range keyRange = new ParserResult.Range(keyStartLine, keyStartColumn, line, column); + parserResult.getFieldRanges().computeIfAbsent(result, _ -> new HashMap<>()).put(InternalField.KEY_FIELD, keyRange); + result.setCitationKey(key); skipWhitespace(); @@ -713,10 +738,13 @@ private BibEntry parseEntry(String entryType) throws IOException { // Consume new line which signals end of entry skipOneNewline(); + parserResult.getArticleRanges().put(result, new ParserResult.Range(articleStartLine, articleStartColumn, line, column)); return result; } private void parseField(BibEntry entry) throws IOException { + int startLine = line; + int startColumn = column; Field field = FieldFactory.parseField(parseTextToken().toLowerCase(Locale.ROOT)); skipWhitespace(); @@ -770,6 +798,8 @@ private void parseField(BibEntry entry) throws IOException { } } } + ParserResult.Range keyRange = new ParserResult.Range(startLine, startColumn, line, column); + parserResult.getFieldRanges().computeIfAbsent(entry, _ -> new HashMap<>()).put(field, keyRange); } private String parseFieldContent(Field field) throws IOException { diff --git a/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultCsvWriter.java b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultCsvWriter.java new file mode 100644 index 00000000000..c782474200d --- /dev/null +++ b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultCsvWriter.java @@ -0,0 +1,29 @@ +package org.jabref.logic.integrity; + +import java.io.IOException; +import java.io.Writer; +import java.util.List; + +import org.apache.commons.csv.CSVFormat; +import org.apache.commons.csv.CSVPrinter; + +public class IntegrityCheckResultCsvWriter extends IntegrityCheckResultWriter { + + private CSVPrinter csvPrinter; + + public IntegrityCheckResultCsvWriter(Writer writer, List messages) { + super(writer, messages); + } + + @Override + public void writeFindings() throws IOException { + csvPrinter = new CSVPrinter(writer, CSVFormat.DEFAULT); + csvPrinter.printRecord("Citation Key", "Field", "Message"); + csvPrinter.printRecords(messages.stream().map(message -> List.of(message.entry().getCitationKey().orElse(""), message.field().getDisplayName(), message.message()))); + } + + @Override + public void close() throws IOException { + csvPrinter.close(); + } +} diff --git a/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultErrorFormatWriter.java b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultErrorFormatWriter.java new file mode 100644 index 00000000000..15cb7ee53ae --- /dev/null +++ b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultErrorFormatWriter.java @@ -0,0 +1,37 @@ +package org.jabref.logic.integrity; + +import java.io.IOException; +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 { + + private final ParserResult parserResult; + private final Path inputFile; + + public IntegrityCheckResultErrorFormatWriter(Writer writer, List messages, ParserResult parserResult, Path inputFile) { + super(writer, messages); + this.parserResult = parserResult; + this.inputFile = inputFile; + } + + @Override + public void writeFindings() throws IOException { + for (IntegrityMessage message : messages) { + Map 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))); + + writer.append("%s:%d:%d: %s\n".formatted( + inputFile, + fieldRange.startLine(), + fieldRange.startColumn(), + message.message())); + } + } +} diff --git a/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultTxtWriter.java b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultTxtWriter.java new file mode 100644 index 00000000000..7bfb6e3c876 --- /dev/null +++ b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultTxtWriter.java @@ -0,0 +1,20 @@ +package org.jabref.logic.integrity; + +import java.io.IOException; +import java.io.Writer; +import java.util.List; + +public class IntegrityCheckResultTxtWriter extends IntegrityCheckResultWriter { + + public IntegrityCheckResultTxtWriter(Writer writer, List messages) { + super(writer, messages); + } + + @Override + public void writeFindings() throws IOException { + for (IntegrityMessage message : messages) { + writer.write(message.toString()); + writer.write(System.lineSeparator()); + } + } +} diff --git a/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultWriter.java b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultWriter.java new file mode 100644 index 00000000000..9edf3da71ff --- /dev/null +++ b/jablib/src/main/java/org/jabref/logic/integrity/IntegrityCheckResultWriter.java @@ -0,0 +1,23 @@ +package org.jabref.logic.integrity; + +import java.io.Closeable; +import java.io.IOException; +import java.io.Writer; +import java.util.List; + +public abstract class IntegrityCheckResultWriter implements Closeable { + + protected final List messages; + protected final Writer writer; + + /// Writer lifecycle: The caller is responsible for closing the writer at the appropriate time. + public IntegrityCheckResultWriter(Writer writer, List messages) { + this.writer = writer; + this.messages = messages; + } + + public abstract void writeFindings() throws IOException; + + @Override + public void close() throws IOException { } +} diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties index 73ea6b1055f..0c50f677e0b 100644 --- a/jablib/src/main/resources/l10n/JabRef_en.properties +++ b/jablib/src/main/resources/l10n/JabRef_en.properties @@ -387,6 +387,7 @@ Unknown\ export\ format\ %0=Unknown export format %0 Importing\ %0=Importing %0 Importing\ file\ %0\ as\ unknown\ format=Importing file %0 as unknown format Format\ used\:\ %0=Format used: %0 +Unknown\ output\ format\ '%0'.=Unknown output format '%0'. Extension=Extension