-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Implemented BibtexkeyChecker #2022
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package net.sf.jabref.logic.integrity; | ||
|
||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
import net.sf.jabref.logic.integrity.IntegrityCheck.Checker; | ||
import net.sf.jabref.logic.l10n.Localization; | ||
import net.sf.jabref.model.entry.BibEntry; | ||
import net.sf.jabref.model.entry.FieldName; | ||
|
||
public class BibtexkeyChecker implements Checker { | ||
|
||
@Override | ||
public List<IntegrityMessage> check(BibEntry entry) { | ||
Optional<String> valuekey = entry.getCiteKeyOptional(); | ||
Optional<String> valueauthor = entry.getField(FieldName.AUTHOR); | ||
Optional<String> valuetitle = entry.getField(FieldName.TITLE); | ||
Optional<String> valueyear = entry.getField(FieldName.YEAR); | ||
String authortitleyear = entry.getAuthorTitleYear(100); | ||
|
||
if (!valueauthor.isPresent() || !valuetitle.isPresent() || !valueyear.isPresent()) { | ||
return Collections.emptyList(); | ||
} | ||
|
||
if (!valuekey.isPresent() || valuekey.get().isEmpty()) { | ||
return Collections.singletonList(new IntegrityMessage( | ||
Localization.lang("empty bibtexkey") + ": " + authortitleyear, entry, BibEntry.KEY_FIELD)); | ||
} | ||
|
||
return Collections.emptyList(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2307,3 +2307,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2309,3 +2309,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2307,3 +2307,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2310,3 +2310,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2307,3 +2307,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2307,3 +2307,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2305,3 +2305,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2309,3 +2309,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2309,3 +2309,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2307,3 +2307,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2310,3 +2310,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2306,3 +2306,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2309,3 +2309,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2307,3 +2307,5 @@ Invalid_ISBN\:_'%0'.= | |
|
||
should_be_an_integer_or_normalized= | ||
should_be_normalized= | ||
|
||
empty_bibtexkey= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,6 +111,11 @@ public void testMonthChecks() { | |
assertWrong(withMode(createContext("month", "Lorem"), BibDatabaseMode.BIBLATEX)); | ||
} | ||
|
||
@Test | ||
public void testBibtexkeyChecks() { | ||
assertCorrect(createContext("bibtexkey", "Knuth2014")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you do one assertCorrect per Test please? 😇 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we now have a class for each checker, we should also create a test class for each checker separately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed my thumb up. A separate test class for every checker is too much (for evt. one or two entries). I think, it is better to organize the checker tests by fields (in methods) and not by checker classes. |
||
} | ||
|
||
@Test | ||
public void testBracketChecks() { | ||
assertCorrect(createContext("title", "x")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this if statement. So if an entry is missing an author (or title/year) and also has no bibtexkey, then no warning will be printed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because getAuthorTitleYear(i) requires, that all three field exist. Otherwise some check tests are failing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Judging based on the code, I think getAuthorTitleYear would return
N/A: N/A (N/A)
in the case where the fields are missing. I would suggest that you change the order of the if statements.If the key is empty, then we definitely want to report a message. But if then all the author, title and year are empty, we use BibEntry.toString (it is a bit ugly) or otherwise we use getAuthorTitleYear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the order of the if statements causes the fail of all checker tests:
java.lang.AssertionError: expected:<[]> but was:<[[] in bibtexkey: empty bibtexkey: N/A: "N/A" (N/A)]