[2025-06-27T05:03:47.949Z] code review started
[2025-06-27T05:03:47.949Z] owner: JabRef
[2025-06-27T05:03:47.950Z] repo: jabref
[2025-06-27T05:03:47.950Z] repoUrl: https://github.com/JabRef/jabref
[2025-06-27T05:03:47.950Z] author: tushar-2320
[2025-06-27T05:03:47.950Z] listing pull request files
[2025-06-27T05:03:59.050Z] total file count: 5
[2025-06-27T05:03:59.050Z] eligible file count: 5
[2025-06-27T05:04:02.949Z] pro user privilege applied
[2025-06-27T05:04:02.949Z] getting project rules
[2025-06-27T05:04:05.683Z] model: claude-3-5-sonnet-20240620
[2025-06-27T05:04:05.683Z] on rule review mode: true
[2025-06-27T05:04:05.683Z] glob ignore:
[2025-06-27T05:04:05.683Z] pull number: 13387
[2025-06-27T05:04:05.683Z] projectId: 885262de-6ea4-406f-ab33-0a18e077aaf9
[2025-06-27T05:04:11.453Z] Found 3 existing review comments
[2025-06-27T05:04:11.453Z] file: jabgui/src/main/java/module-info.java
[2025-06-27T05:04:11.453Z] file is outside of new changes on pr, skipping
[2025-06-27T05:04:11.453Z] file: jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java
[2025-06-27T05:04:11.453Z] file is outside of new changes on pr, skipping
[2025-06-27T05:04:11.453Z] file: jabgui/src/main/java/org/jabref/gui/maintable/RightClickMenu.java
[2025-06-27T05:04:11.453Z] file is outside of new changes on pr, skipping
[2025-06-27T05:04:11.453Z] file: jabgui/src/main/java/org/jabref/gui/preview/ClipboardContentGenerator.java
[2025-06-27T05:04:11.453Z] reading file blob
[2025-06-27T05:04:19.748Z] filteredRules: 1. If code in org.jabref.model or org.jabref.logic has been changed, tests need to be adapted or updated accordingly.

Notes:

• This rule does not apply for import statements.

2. The pull request title should contain a short title of the issue fixed (or what the PR adresses) and not just "Fix issue xyz"
3. The "Mandatory checks" are Markdown TODOs. They should be formatted as that. Wrong: - [ x]. Either - [ ] or - [x].
4. New methods (and new classes) should follow the Single-responsibility principle (SRP).
5. Avoid code duplication
6. Use modern Java best practices, such as Arguments.of() instead of new Object[] especially in JUnit tests or Path.of() instead of Paths.get(), to improve readability and maintainability.
7. Exceptions should be used for exceptional states - not for normal control flow
8. Follow the principles of "Effective Java"
9. JabRef is a multilingual program, When you write any user-facing text, it should be localized.

To do this in Java code, call Localization.lang method, like this:

Localization.lang("Ok")

More information at: https://devdocs.jabref.org/code-howtos/localization.html.

Notes:

• This rule is not applied for logging. Logging strings should stay in English. I.e., LOGGER.error("...") should contain English text.

10. No use of Java SWING, only JavaFX is allowed as UI technology
11. In JabRef, we don't use @DisplayName, we typically just write method name as is. The method name itself should be comprehensive enough.
12. Comments should add new information (e.g. reasoning of the code). It should not be plainly derived from the code itself.

Example for trivial comments:

// Commit the staged changes
RevCommit commit = git.commit()
fieldName = fieldName.trim().toLowerCase(); // Trim and convert to lower case

Write without comments:

RevCommit commit = git.commit()
fieldName = fieldName.trim().toLowerCase();

Notes:

• This rule does not apply to fixes of spelling mistakes

13. Instead of Files.createTempDirectory @TempDir JUnit5 annotation should be used.
14. Do not catch the general java java.lang.Exception. Catch specific exeptions only
15. Avoid exclamation marks at the end of a sentence. They are more for screaming. Use a dot to end the sentence.
16. All labels and texts in the UI should be sentence case (and not title case)
17. New public methods should not return null. They should make use of java.util.Optional. In case null really needs to be used, the JSpecify annotations must be used.
18. Use "BibTeX" as spelling for bibtex in Java strings. In variable names "Bibtex" should be used.
19. New strings should be consistent to other strings. They should also be grouped semantically together.
20. Existings strings should be reused instead of introducing slightly different strings
21. The CHANGELOG.md entry should be for end users (and not programmers).
22. User dialogs should have proper button labels: NOT yes/no/cancel, but indicating the action which happens when pressing the button
23. GUI code should only be a gateway to code in org.jabref.logic. More complex code regarding non-GUI operations should go into org.jabref.logic. Think of layerd archicture.
24. null should never be passed to a method (except it has the same name).
25. Do not add extra blank lines in CHANGELOG.md
26. Remove commented code. (To keep a history of changes git was made for.)
27. Do not throw unchecked exceptions (e.g., do not throw new RuntimeException, do not throw new IllegalStateException)

Reason: This tears down the whole application. One does not want to loose data only because "a corner" of the application broke.
28. When adding JavaDoc, the text should be non-trivial.

Example for trivial JavaDoc:

 * @param backupDir the backup directory
 * @param dbfile the database file
 * @throws IOException     if an I/O error occurs
 * @throws GitAPIException if a Git API error occurs

29. Code should not be reformatted only because of syntax. There need to be new statements added if reformatting.
30. If @TempDir is used, there is no need to clean it up

Example for wrong code:

    @AfterEach
    void tearDown() throws IOException {
        FileUtils.cleanDirectory(tempDir.toFile());
    }

31. Assert the contents of objects (assertEquals), not checking for some Boolean conditions (assertTrue/assertFalse)

Example for wrong code:

        assertTrue(
                entry.getFiles().stream()
                     .anyMatch(file -> file.getLink().equals(newFile.getFileName().toString()) ||
                             file.getLink().endsWith("/" + newFile.getFileName().toString()))
        );

32. No "new Thread()", use "org.jabref.logic.util.BackgroundTask" and its "executeWith"
33. try blocks shoud cover as less statements as possible (and not whole methods)
34. use "throws Exception" in the method declaration instead of try-catch and fail/log/...
35. When creating a new BibEntry object "withers" should be used: Instead of setField, withField methods should be used.
36. In case Java comments are added, they should match the code following. They should be a high-level summary or guidance of the following code (and not some reandom text or just re-stating the obvious)
37. Use the methods of java.util.Optional. ifPresent.

NOT

Optional<String> resolved = bibEntry.getResolvedFieldOrAlias(...);
String value = resolved.orElse("");
doSomething(value)

Following is fine:

Optional<String> resolved = bibEntry.getResolvedFieldOrAlias(...);
resolved.ifPresent(value -> doSomething(value));

38. If the java.util.Optional is really present, use use get() (and not orElse(""))
39. Use Java Text blocks (""") for multiline string constants
40. Use compiled patterns (Pattern.compile)

Examples:

NOT: x.matches(".\s{2,}.")

BUT:
private final static PATTERN = ...
and then PATTERN.matcher(x)
41. Use placeholders if variance is in localizaiton:

BAD: Localization.lang("Current JabRef version") + ": " + buildInfo.version);

GOOD: Localization.lang("Current JabRef version: %0", buildInfo.version);
42. BAD:

try {
    // do some actions
} catch (Exception e) {
    LOGGER.info("Failed to push: ".concat(e.toString()));
}

This code converts an error to string and then concatenates it with a message. This is not how it's done in JabRef.

GOOD:

try {
    // do some actions
} catch (Exception e) {
    LOGGER.info("Failed to push", e);
}

In JabRef, we use logging capabilities. The last arugment of the logger call should be an exception.

Notes:

• Logging may include other arguments. But the exception should be the last in arguments. Example: LOGGER.info("Error. Var1: {}, Var2: {}", var1, var2, e).

43. Boolean method parameters (for public methods) should be avoided. Better create two distinct methods (which maybe call some private methods)
44. Use modern Java data structures

BAD: new HashSet<>(Arrays.asList(...))

GOOD: Sef.of(...)
45. Java 21 introduced SequencedCollection and SequencedSet interfaces. Use it instead of LinkedHashSet (where applicable)
46. Use StringJoiner instead of StringBuilder (if possible)
47. In case defaults (defaults.put) are modified, there needs to be a preference migration.

This rule doesn't apply for new preferences, and, thus, new defaults, as they do not require migration.
48. Whenever you include a text in FXML (text labels, buttons, prompts in text fields, window titles, etc.), it should be localized.

To localize a string in FXML, prefix it with %.

Bad example:

<Label text="Want to help?"/>

In this code text property is the field that is used to show text to the user. This must be localized.

Fix:

<Label text="%Want to help?"/>

49. Labels should not end with ":"

BAD:

GOOD:
50. Plain JUnit assert should be used instead of org.assertj (if possible)

BAD: assertThat(gitPreferences.getAutoPushEnabled()).isFalse();

GOOD: assertFalse(gitPreferences.getAutoPushEnabled());
51. Do not catch exceptions in Test - let JUnit handle

BAD: try {...code...} catch (IOException e) {
throw new AssertionError("Failed to set up test directory", e);
}

GOOD: ...code...
52. Minimal quality for variable names: Not extraEntry2, extraEntry3; but include meaning/intention into the variable names
53. One should use jabref's dialogService (instead of Java native FileChooser)

dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(path -> ...)

and with FileDialogConfiguration offers the Builder pattern.
(see e.g NewLibraryFromPdfAction)

54. In JabRef, to create an empty list we use List.of() instead of Collections.emptyList()
55. Correctly spelled variable names (meaning: no typos in variable names)
    [2025-06-27T05:04:46.448Z] filtering out non-relevant issues
    [2025-06-27T05:04:46.449Z] broad list of issues found
    ⚠️ [2025-06-27T05:04:46.449Z] rule violated
    [2025-06-27T05:04:46.548Z] score: 4
    ⚠️ [2025-06-27T05:04:46.548Z] this log will only exist in github checkrun logs
    [2025-06-27T05:04:46.548Z] Reason: The comment is trivial and simply restates what the code does without providing additional information about the reasoning or important details about the implementation.
    ⚠️ [2025-06-27T05:04:46.548Z] rule violated
    [2025-06-27T05:04:46.548Z] score: 5
    ⚠️ [2025-06-27T05:04:46.548Z] this log will only exist in github checkrun logs
    [2025-06-27T05:04:46.548Z] Reason: The HTML template string should use Java Text blocks (triple quotes) for better readability and maintainability instead of concatenating multiple strings with newlines.
    [2025-06-27T05:04:46.548Z] score: 7
    ⚠️ [2025-06-27T05:04:46.548Z] this log will only exist in github checkrun logs
    [2025-06-27T05:04:46.548Z] Reason: The method duplicates HTML template code from processHtml method. This violates DRY principle and makes maintenance harder. The HTML template should be extracted to a shared method.
    [2025-06-27T05:04:46.548Z] filtering out low importance issues
    [2025-06-27T05:04:46.549Z] writing comments to pr...
    [2025-06-27T05:04:55.449Z] rule: Follow the principles of "Effective Java"
    ⚠️ [2025-06-27T05:04:55.449Z] this log will only exist in github checkrun logs
    [2025-06-27T05:04:55.450Z] comment: The comment is trivial and simply restates what the code does without providing additional informati...
    [2025-06-27T05:04:57.451Z] file: jablib/src/main/java/org/jabref/logic/citationstyle/CitationStyleOutputFormat.java
    [2025-06-27T05:04:57.451Z] file is outside of new changes on pr, skipping
    [2025-06-27T05:04:57.451Z] files scanned: 1
    [2025-06-27T05:04:57.451Z] lines scanned: 49
    [2025-06-27T05:04:57.451Z] rules for project: 55
    [2025-06-27T05:04:57.451Z] issues caught by your rules: 1