From 947f065f8fe90a589d4ac9d4729acf3c39328258 Mon Sep 17 00:00:00 2001 From: Oscar Gustafsson Date: Tue, 29 Dec 2015 18:46:27 +0100 Subject: [PATCH] Removed Coverity warning in bst and cleaned up all files there --- .../net/sf/jabref/bst/BibtexCaseChanger.java | 18 +- .../sf/jabref/bst/BibtexNameFormatter.java | 60 ++++--- .../net/sf/jabref/bst/ChangeCaseFunction.java | 10 +- .../net/sf/jabref/bst/FormatNameFunction.java | 12 +- .../net/sf/jabref/bst/TextPrefixFunction.java | 5 +- src/main/java/net/sf/jabref/bst/VM.java | 157 ++++++++---------- src/test/java/net/sf/jabref/bst/TestVM.java | 6 +- 7 files changed, 131 insertions(+), 137 deletions(-) diff --git a/src/main/java/net/sf/jabref/bst/BibtexCaseChanger.java b/src/main/java/net/sf/jabref/bst/BibtexCaseChanger.java index 353ad68de51..8b500a8b4a0 100644 --- a/src/main/java/net/sf/jabref/bst/BibtexCaseChanger.java +++ b/src/main/java/net/sf/jabref/bst/BibtexCaseChanger.java @@ -20,7 +20,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -public class BibtexCaseChanger { +public final class BibtexCaseChanger { private static final Log LOGGER = LogFactory.getLog(BibtexCaseChanger.class); @@ -52,12 +52,12 @@ public enum FORMAT_MODE { // DIFFERENCE to old CaseChangers.TITLE: last word is NOT capitalized in all cases //TITLE_UPPERS('T'); + private final char asChar; + public char asChar() { return asChar; } - private final char asChar; - FORMAT_MODE(char asChar) { this.asChar = asChar; } @@ -222,6 +222,9 @@ private int convertAccented(char[] c, int pos, String s, StringBuffer sb, FORMAT sb.append(s); } break; + default: + LOGGER.info("convertAccented - Unknown format: " + format); + break; } return pos; } @@ -237,6 +240,9 @@ private int convertNonControl(char[] c, int pos, StringBuffer sb, FORMAT_MODE fo sb.append(Character.toUpperCase(c[pos])); pos++; break; + default: + LOGGER.info("convertNonControl - Unknown format: " + format); + break; } return pos; } @@ -262,6 +268,10 @@ private int convertCharIfBraceLevelIsZero(char[] c, int i, StringBuffer sb, FORM break; case ALL_UPPERS: sb.append(Character.toUpperCase(c[i])); + break; + default: + LOGGER.info("convertCharIfBraceLevelIsZero - Unknown format: " + format); + break; } i++; return i; @@ -277,7 +287,7 @@ private int convertCharIfBraceLevelIsZero(char[] c, int i, StringBuffer sb, FORM * @param pos the position * @return the special LaTeX character or null */ - static Optional findSpecialChar(char[] c, int pos) { + public static Optional findSpecialChar(char[] c, int pos) { if ((pos + 1) < c.length) { if ((c[pos] == 'o') && (c[pos + 1] == 'e')) { return Optional.of("oe"); diff --git a/src/main/java/net/sf/jabref/bst/BibtexNameFormatter.java b/src/main/java/net/sf/jabref/bst/BibtexNameFormatter.java index 42defd48ba4..06a859c0d2c 100644 --- a/src/main/java/net/sf/jabref/bst/BibtexNameFormatter.java +++ b/src/main/java/net/sf/jabref/bst/BibtexNameFormatter.java @@ -20,7 +20,7 @@ /** * From Bibtex: - * + * * "The |built_in| function {\.{format.name\$}} pops the * top three literals (they are a string, an integer, and a string * literal, in that order). The last string literal represents a @@ -30,9 +30,9 @@ * described in the \BibTeX\ documentation. Finally, this function * pushes the formatted name. If any of the types is incorrect, it * complains and pushes the null string." - * + * * Sounds easy - is a nightmare... X-( - * + * */ public class BibtexNameFormatter { @@ -47,7 +47,7 @@ public static String formatName(String authorsNameList, int whichName, String fo } /** - * + * * @param author * @param format * @param warn may-be-null @@ -66,7 +66,6 @@ public static String formatName(Author author, String format, Warn warn) { while (i < n) { if (c[i] == '{') { group++; - int groupStart = sb.length(); i++; braceLevel++; StringBuffer level1Chars = new StringBuffer(); @@ -83,15 +82,15 @@ public static String formatName(Author author, String format, Warn warn) { i++; continue; } - if (braceLevel == 1) { - if (Character.isLetter(c[i])) { - if ("fvlj".indexOf(c[i]) == -1) { - if (warn != null) { - warn.warn("Format String in format.name$ may only contain fvlj on brace level 1 in group " + group + ": " + format); - } - } else { - level1Chars.append(c[i]); + if ((braceLevel == 1) && Character.isLetter(c[i])) { + if ("fvlj".indexOf(c[i]) == -1) { + if (warn != null) { + warn.warn( + "Format String in format.name$ may only contain fvlj on brace level 1 in group " + + group + ": " + format); } + } else { + level1Chars.append(c[i]); } } i++; @@ -156,6 +155,7 @@ public static String formatName(Author author, String format, Warn warn) { int bLevel = 1; String interToken = null; + int groupStart = sb.length(); for (int j = 0; j < d.length; j++) { @@ -164,12 +164,10 @@ public static String formatName(Author author, String format, Warn warn) { if (!abbreviateThatIsSingleLetter) { j++; } - if ((j + 1) < d.length) { - if (d[j + 1] == '{') { - StringBuffer interTokenSb = new StringBuffer(); - j = BibtexNameFormatter.consumeToMatchingBrace(interTokenSb, d, j + 1); - interToken = interTokenSb.substring(1, interTokenSb.length() - 1); - } + if (((j + 1) < d.length) && (d[j + 1] == '{')) { + StringBuffer interTokenSb = new StringBuffer(); + j = BibtexNameFormatter.consumeToMatchingBrace(interTokenSb, d, j + 1); + interToken = interTokenSb.substring(1, interTokenSb.length() - 1); } for (int k = 0; k < tokens.length; k++) { @@ -195,15 +193,15 @@ public static String formatName(Author author, String format, Warn warn) { // Output Intertoken String if (interToken == null) { if (abbreviateThatIsSingleLetter) { - sb.append("."); + sb.append('.'); } // No clue what this means (What the hell are tokens anyway??? // if (lex_class[name_sep_char[cur_token]] = sep_char) then // append_ex_buf_char_and_check (name_sep_char[cur_token]) if ((k == (tokens.length - 2)) || (BibtexNameFormatter.numberOfChars(sb.substring(groupStart, sb.length()), 3) < 3)) { - sb.append("~"); + sb.append('~'); } else { - sb.append(" "); + sb.append(' '); } } else { sb.append(interToken); @@ -242,7 +240,7 @@ public static String formatName(Author author, String format, Warn warn) { } i++; } - if (braceLevel != 0) { + if ((braceLevel != 0) && (warn != null)) { warn.warn("Unbalanced brace in format string for nameFormat: " + format); } @@ -251,12 +249,12 @@ public static String formatName(Author author, String format, Warn warn) { /** * Including the matching brace. - * + * * @param sb * @param c * @param pos * @return - * + * * assert c[pos] == '{' */ public static int consumeToMatchingBrace(StringBuffer sb, char[] c, int pos) { @@ -282,7 +280,7 @@ public static int consumeToMatchingBrace(StringBuffer sb, char[] c, int pos) { /** * Takes care of special characters too - * + * * @param s * @return */ @@ -292,12 +290,10 @@ public static String getFirstCharOfString(String s) { if (Character.isLetter(c[i])) { return String.valueOf(c[i]); } - if (c[i] == '{') { - if (((i + 1) < c.length) && (c[i + 1] == '\\')) { - StringBuffer sb = new StringBuffer(); - BibtexNameFormatter.consumeToMatchingBrace(sb, c, i); - return sb.toString(); - } + if ((c[i] == '{') && ((i + 1) < c.length) && (c[i + 1] == '\\')) { + StringBuffer sb = new StringBuffer(); + BibtexNameFormatter.consumeToMatchingBrace(sb, c, i); + return sb.toString(); } } return ""; diff --git a/src/main/java/net/sf/jabref/bst/ChangeCaseFunction.java b/src/main/java/net/sf/jabref/bst/ChangeCaseFunction.java index 304747c353a..bf432b7b42d 100644 --- a/src/main/java/net/sf/jabref/bst/ChangeCaseFunction.java +++ b/src/main/java/net/sf/jabref/bst/ChangeCaseFunction.java @@ -23,7 +23,7 @@ /** * From the Bibtex manual: - * + * * Pops the top two (string) literals; it changes the case of the second * according to the specifications of the first, as follows. (Note: The word * `letters' in the next sentence refers only to those at brace-level 0, the @@ -41,9 +41,9 @@ * note: It ignores case differences in the specification string; for example, * the strings t and T are equivalent for the purposes of this built-in * function.) - * + * * Christopher: I think this should be another grammar! This parser is horrible. - * + * */ public class ChangeCaseFunction implements BstFunction { @@ -61,13 +61,13 @@ public void execute(BstEntry context) { if (stack.size() < 2) { throw new VMException("Not enough operands on stack for operation change.case$"); } - Object o1 = stack.pop(); - Object o2 = stack.pop(); + Object o1 = stack.pop(); if (!((o1 instanceof String) && (((String) o1).length() == 1))) { throw new VMException("A format string of length 1 is needed for change.case$"); } + Object o2 = stack.pop(); if (!(o2 instanceof String)) { throw new VMException("A string is needed as second parameter for change.case$"); } diff --git a/src/main/java/net/sf/jabref/bst/FormatNameFunction.java b/src/main/java/net/sf/jabref/bst/FormatNameFunction.java index e8cf21a50ec..a0b52ad0d4c 100644 --- a/src/main/java/net/sf/jabref/bst/FormatNameFunction.java +++ b/src/main/java/net/sf/jabref/bst/FormatNameFunction.java @@ -24,7 +24,7 @@ /** * From Bibtex: - * + * * "The |built_in| function {\.{format.name\$}} pops the * top three literals (they are a string, an integer, and a string * literal, in that order). The last string literal represents a @@ -34,9 +34,9 @@ * described in the \BibTeX\ documentation. Finally, this function * pushes the formatted name. If any of the types is incorrect, it * complains and pushes the null string." - * + * * All the pain is encapsulated in BibtexNameFormatter. :-) - * + * */ public class FormatNameFunction implements BstFunction { @@ -68,7 +68,9 @@ public void execute(BstEntry context) { Integer name = (Integer) o2; String names = (String) o3; - if (names != null) { + if (names == null) { + stack.push(""); + } else { AuthorList a = AuthorList.getAuthorList(names); if (name > a.size()) { throw new VMException("Author Out of Bounds. Number " + name + " invalid for " + names); @@ -76,8 +78,6 @@ public void execute(BstEntry context) { Author author = a.getAuthor(name - 1); stack.push(BibtexNameFormatter.formatName(author, format, vm)); - } else { - stack.push(""); } } } diff --git a/src/main/java/net/sf/jabref/bst/TextPrefixFunction.java b/src/main/java/net/sf/jabref/bst/TextPrefixFunction.java index 16940d75192..9b10d34ed95 100644 --- a/src/main/java/net/sf/jabref/bst/TextPrefixFunction.java +++ b/src/main/java/net/sf/jabref/bst/TextPrefixFunction.java @@ -51,14 +51,15 @@ public void execute(BstEntry context) { if (stack.size() < 2) { throw new VMException("Not enough operands on stack for operation text.prefix$"); } - Object o1 = stack.pop(); - Object o2 = stack.pop(); + Object o1 = stack.pop(); if (!(o1 instanceof Integer)) { vm.warn("An integer is needed as first parameter to text.prefix$"); stack.push(""); return; } + + Object o2 = stack.pop(); if (!(o2 instanceof String)) { vm.warn("A string is needed as second parameter to text.prefix$"); stack.push(""); diff --git a/src/main/java/net/sf/jabref/bst/VM.java b/src/main/java/net/sf/jabref/bst/VM.java index b2d24fcb3ef..69e0a5387a9 100644 --- a/src/main/java/net/sf/jabref/bst/VM.java +++ b/src/main/java/net/sf/jabref/bst/VM.java @@ -17,14 +17,15 @@ import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.List; import java.util.ListIterator; import java.util.Map; import java.util.Stack; -import java.util.Vector; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -56,6 +57,30 @@ public class VM implements Warn { private static final Log LOGGER = LogFactory.getLog(VM.class); + private List entries; + + private Map strings = new HashMap<>(); + + private Map integers = new HashMap<>(); + + private Map functions = new HashMap<>(); + + private Stack stack = new Stack<>(); + + public static final Integer FALSE = 0; + + public static final Integer TRUE = 1; + + private final Map buildInFunctions; + + private File file; + + private final CommonTree tree; + + private StringBuffer bbl; + + private String preamble; + public static class Identifier { @@ -90,15 +115,6 @@ public interface BstFunction { } - public static final Integer FALSE = 0; - - public static final Integer TRUE = 1; - - private final HashMap buildInFunctions; - - private File file; - - public VM(File f) throws RecognitionException, IOException { this(new ANTLRFileStream(f.getPath())); this.file = f; @@ -144,11 +160,6 @@ public void execute(BstEntry context) { throw new VMException("Can only compare two integers with >"); } - if (o1 == o2) { - stack.push(VM.FALSE); - return; - } - stack.push(((Integer) o1).compareTo((Integer) o2) > 0 ? VM.TRUE : VM.FALSE); } }); @@ -168,11 +179,6 @@ public void execute(BstEntry context) { throw new VMException("Can only compare two integers with <"); } - if (o1 == o2) { - stack.push(VM.FALSE); - return; - } - stack.push(((Integer) o1).compareTo((Integer) o2) < 0 ? VM.TRUE : VM.FALSE); } @@ -197,7 +203,7 @@ public void execute(BstEntry context) { return; } - if (o1 == o2) { + if ((o1 == null) && (o2 == null)) { stack.push(VM.TRUE); return; } @@ -346,7 +352,7 @@ public void execute(BstEntry context) { throw new VMException( "Call.type$ can only be called from within a context (ITERATE or REVERSE)."); } - VM.this.execute(context.entry.getType().getName().toLowerCase(), context); + VM.this.execute(context.getBibtexEntry().getType().getName().toLowerCase(), context); } }); @@ -384,7 +390,7 @@ public void execute(BstEntry context) { */ @Override public void execute(BstEntry context) { - stack.push(context.entry.getCiteKey()); + stack.push(context.getBibtexEntry().getCiteKey()); } }); @@ -604,10 +610,10 @@ public void execute(BstEntry context) { */ @Override public void execute(BstEntry context) { - if (preamble != null) { - stack.push(preamble); - } else { + if (preamble == null) { stack.push(""); + } else { + stack.push(preamble); } } @@ -864,7 +870,7 @@ public void execute(BstEntry context) { */ @Override public void execute(BstEntry context) { - stack.push(context.entry.getType().getName()); + stack.push(context.getBibtexEntry().getType().getName()); } }); @@ -974,13 +980,6 @@ private boolean assign(BstEntry context, Object o1, Object o2) { } - private final CommonTree tree; - - private StringBuffer bbl; - - private String preamble; - - public String run(BibDatabase db) { preamble = db.getPreamble(); return run(db.getEntries()); @@ -988,10 +987,22 @@ public String run(BibDatabase db) { public String run(Collection bibtex) { - reset(); + // Reset + bbl = new StringBuffer(); + + strings = new HashMap<>(); + + integers = new HashMap<>(); + integers.put("entry.max$", Integer.MAX_VALUE); + integers.put("global.max$", Integer.MAX_VALUE); + + functions = new HashMap<>(); + functions.putAll(buildInFunctions); + + stack = new Stack<>(); { // Create entries - entries = new Vector<>(bibtex.size()); + entries = new ArrayList<>(bibtex.size()); ListIterator i = entries.listIterator(); for (BibEntry entry : bibtex) { i.add(new BstEntry(entry)); @@ -1034,29 +1045,15 @@ public String run(Collection bibtex) { case BstParser.MACRO: macro(child); break; + default: + LOGGER.info("Unknown type: " + child.getType()); + break; } } return bbl.toString(); } - private void reset() { - bbl = new StringBuffer(); - - entries = null; - - strings = new HashMap<>(); - - integers = new HashMap<>(); - integers.put("entry.max$", Integer.MAX_VALUE); - integers.put("global.max$", Integer.MAX_VALUE); - - functions = new HashMap<>(); - functions.putAll(buildInFunctions); - - stack = new Stack<>(); - } - /** * Dredges up from the database file the field values for each entry in the * list. It has no arguments. If a database entry doesn't have a value for a @@ -1069,16 +1066,16 @@ private void read() { for (BstEntry e : entries) { - for (Map.Entry mEntry : e.fields.entrySet()) { - Object fieldValue = e.entry.getField(mEntry.getKey()); + for (Map.Entry mEntry : e.getFields().entrySet()) { + Object fieldValue = e.getBibtexEntry().getField(mEntry.getKey()); mEntry.setValue((fieldValue == null ? null : fieldValue.toString())); } } for (BstEntry e : entries) { - if (!e.fields.containsKey("crossref")) { - e.fields.put("crossref", null); + if (!e.getFields().containsKey("crossref")) { + e.getFields().put("crossref", null); } } } @@ -1104,7 +1101,7 @@ private void macro(Tree child) { public class MacroFunction implements BstFunction { - final String replacement; + private final String replacement; public MacroFunction(String replacement) { @@ -1123,7 +1120,7 @@ public void execute(BstEntry context) { * (possibly empty) list of variable names. The three lists are of: fields, * integer entry variables, and string entry variables. There is an * additional field that BibTEX automatically declares, crossref, used for - * cross ref- erencing. And there is an additional string entry variable + * cross referencing. And there is an additional string entry variable * automatically declared, sort.key$, used by the SORT command. Each of * these variables has a value for each entry on the list. */ @@ -1136,7 +1133,7 @@ private void entry(Tree child) { String name = t.getChild(i).getText(); for (BstEntry entry : entries) { - entry.fields.put(name, null); + entry.getFields().put(name, null); } } @@ -1214,7 +1211,7 @@ private void execute(Tree child) { public class StackFunction implements BstFunction { - final Tree localTree; + private final Tree localTree; public Tree getTree() { @@ -1252,11 +1249,11 @@ public void execute(BstEntry context) { VM.this.execute(c.getText(), context); } } catch (VMException e) { - if (file != null) { + if (file == null) { + LOGGER.error("ERROR " + e.getMessage() + " (" + c.getLine() + ")"); + } else { LOGGER.error("ERROR " + e.getMessage() + " (" + file.getPath() + ":" + c.getLine() + ")"); - } else { - LOGGER.error("ERROR " + e.getMessage() + " (" + c.getLine() + ")"); } throw e; } @@ -1274,8 +1271,8 @@ private void execute(String name, BstEntry context) { if (context != null) { - if (context.fields.containsKey(name)) { - stack.push(context.fields.get(name)); + if (context.getFields().containsKey(name)) { + stack.push(context.getFields().get(name)); return; } if (context.localStrings.containsKey(name)) { @@ -1350,19 +1347,18 @@ private void strings(Tree child) { public static class BstEntry { - public BstEntry(BibEntry e) { - this.entry = e; - } - + private final BibEntry entry; - final BibEntry entry; + private final Map localStrings = new HashMap<>(); - final Map localStrings = new HashMap<>(); + private final Map fields = new HashMap<>(); - final Map fields = new HashMap<>(); + private final Map localIntegers = new HashMap<>(); - final Map localIntegers = new HashMap<>(); + public BstEntry(BibEntry e) { + this.entry = e; + } public Map getFields() { return fields; @@ -1374,17 +1370,6 @@ public BibEntry getBibtexEntry() { } - private Vector entries; - - private Map strings = new HashMap<>(); - - private Map integers = new HashMap<>(); - - private Map functions = new HashMap<>(); - - private Stack stack = new Stack<>(); - - private void push(Integer integer) { stack.push(integer); } @@ -1405,7 +1390,7 @@ public Map getIntegers() { return integers; } - public Vector getEntries() { + public List getEntries() { return entries; } diff --git a/src/test/java/net/sf/jabref/bst/TestVM.java b/src/test/java/net/sf/jabref/bst/TestVM.java index 43656e12f91..604481a2821 100644 --- a/src/test/java/net/sf/jabref/bst/TestVM.java +++ b/src/test/java/net/sf/jabref/bst/TestVM.java @@ -16,7 +16,9 @@ import java.io.File; import java.io.IOException; import java.io.StringReader; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Vector; public class TestVM { @@ -376,14 +378,14 @@ public void testSort() throws RecognitionException, IOException { VM vm = new VM("" + "ENTRY { title } { } { label }" + "FUNCTION {presort} { cite$ 'sort.key$ := } ITERATE { presort } SORT"); - Vector v = new Vector<>(); + List v = new ArrayList<>(); v.add(TestVM.bibtexString2BibtexEntry("@article{a, author=\"AAA\"}")); v.add(TestVM.bibtexString2BibtexEntry("@article{b, author=\"BBB\"}")); v.add(TestVM.bibtexString2BibtexEntry("@article{d, author=\"DDD\"}")); v.add(TestVM.bibtexString2BibtexEntry("@article{c, author=\"CCC\"}")); vm.run(v); - Vector v2 = vm.getEntries(); + List v2 = vm.getEntries(); Assert.assertEquals("a", v2.get(0).getBibtexEntry().getCiteKey()); Assert.assertEquals("b", v2.get(1).getBibtexEntry().getCiteKey()); Assert.assertEquals("c", v2.get(2).getBibtexEntry().getCiteKey());