Skip to content
/ omegat Public

Commit 0f86132

Browse files
miurahrmiurahr
authored
refactor(core): adopt null-safety improvements and optimize code (#1930)
- Add `@Nullable` annotation in `MsOfficeFileFilter` for null-safety. - Mark `OpenXmlFilter` as `public` to match intended usage. - Replace legacy code patterns with modern, concise alternatives (e.g., `Integer.compare`, `tagsCount.replaceAll`). - Simplify string manipulation and conditional logic for improved readability. Signed-off-by: Hiroshi Miura <miurahr@linux.com>
1 parent 1f88715 commit 0f86132

File tree

2 files changed

+40
-54
lines changed

2 files changed

+40
-54
lines changed

‎src/org/omegat/filters4/xml/openxml/MsOfficeFileFilter.java‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.regex.Pattern;
3737
import java.util.zip.ZipEntry;
3838

39+
import org.jspecify.annotations.Nullable;
3940
import org.omegat.core.Core;
4041
import org.omegat.filters2.FilterContext;
4142
import org.omegat.filters2.Instance;
@@ -200,7 +201,7 @@ private String removePath(String fileName) {
200201

201202
@Override
202203
protected java.util.Comparator<ZipEntry> getEntryComparator() {
203-
/** Same order as in Filter3 **/
204+
/* Same order as in Filter3 */
204205
return (ZipEntry z1, ZipEntry z2) -> {
205206
String s1 = z1.getName();
206207
String s2 = z2.getName();
@@ -219,25 +220,15 @@ protected java.util.Comparator<ZipEntry> getEntryComparator() {
219220
if (getDigits.find()) {
220221
number2 = Integer.parseInt(getDigits.group(1));
221222
}
222-
if (number1 > number2) {
223-
return 1;
224-
} else if (number1 < number2) {
225-
return -1;
226-
} else {
227-
return 0;
228-
}
223+
return Integer.compare(number1, number2);
229224
} else {
230225
String shortname1 = removePath(words1[0]);
231226
String shortname2 = removePath(words2[0]);
232227

233228
// Specific case for Excel
234229
// because "comments" is present twice in DOCUMENTS
235-
if (shortname1.indexOf("sharedStrings") >= 0 || shortname2.indexOf("sharedStrings") >= 0) {
236-
if (shortname2.indexOf("sharedStrings") >= 0) {
237-
return 1; // sharedStrings must be first
238-
} else {
239-
return -1;
240-
}
230+
if (shortname1.contains("sharedStrings") || shortname2.contains("sharedStrings")) {
231+
return shortname2.contains("sharedStrings") ? 1 : -1; // sharedStrings must be first
241232
}
242233

243234
if (shortname1.endsWith(".xml")) {
@@ -281,7 +272,7 @@ public boolean hasOptions() {
281272
* options otherwise.
282273
*/
283274
@Override
284-
public Map<String, String> changeOptions(Window parent, Map<String, String> currentOptions) {
275+
public @Nullable Map<String, String> changeOptions(Window parent, Map<String, String> currentOptions) {
285276
try {
286277
EditOpenXMLOptionsDialog dialog = new EditOpenXMLOptionsDialog(parent, currentOptions);
287278
dialog.setVisible(true);

‎src/org/omegat/filters4/xml/openxml/OpenXmlFilter.java‎

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
*
5858
* @author Thomas Cordonnier
5959
*/
60-
class OpenXmlFilter extends AbstractXmlFilter {
60+
public class OpenXmlFilter extends AbstractXmlFilter {
6161
private final boolean removeComments;
6262

6363
OpenXmlFilter(boolean withComments) {
@@ -311,9 +311,7 @@ protected String buildTags() {
311311
textElement = new QName(ooxmlMainParaElement.getNamespaceURI(), "t");
312312
}
313313
tagsMap.clear();
314-
for (Character c : tagsCount.keySet()) {
315-
tagsCount.put(c, 0);
316-
}
314+
tagsCount.replaceAll((c, v) -> 0);
317315
StringBuffer res = new StringBuffer();
318316
defaultsForParagraph = null;
319317
for (int i = 0; i < currentPara.size(); i++) {
@@ -322,7 +320,7 @@ protected String buildTags() {
322320
continue;
323321
}
324322
if (run.get(0).isStartElement()
325-
&& ((StartElement) run.get(0)).asStartElement().getName().getLocalPart().equals("r")) {
323+
&& run.get(0).asStartElement().getName().getLocalPart().equals("r")) {
326324
ListIterator<XMLEvent> runIter = run.listIterator();
327325
char prefix = findPrefix(runIter);
328326
Integer tc = tagsCount.get(prefix);
@@ -331,17 +329,17 @@ protected String buildTags() {
331329
}
332330
if ((prefix == 'n') || (prefix == 'd') || (prefix == 'e')) { // empty
333331
// tags
334-
res.append("<" + prefix + tc + "/>");
332+
res.append("<").append(prefix).append(tc).append("/>");
335333
tagsMap.put("" + prefix + tc, run);
336334
tagsCount.put(prefix, tc + 1);
337335
} else { // contains text
338336
if (prefix != '\u0000') { // add begin tag
339-
res.append("<" + prefix + tc + ">");
337+
res.append("<").append(prefix).append(tc).append(">");
340338
tagsMap.put("" + prefix + tc, run.subList(0, runIter.nextIndex()));
341339
}
342340
browseRunContents(run, runIter, res);
343341
if (prefix != '\u0000') { // add end tag
344-
res.append("</" + prefix + tc + ">");
342+
res.append("</").append(prefix).append(tc).append(">");
345343
tagsMap.put("/" + prefix + tc, run.subList(runIter.previousIndex(), run.size()));
346344
tagsCount.put(prefix, tc + 1);
347345
}
@@ -429,15 +427,15 @@ protected String buildTags() {
429427
break;
430428
}
431429
// Something between two <w:r>
432-
if ((run.size() == 1) && run.get(0).isCharacters()
433-
&& (0 == run.get(0).asCharacters().getData().trim().length())) {
430+
if (run.size() == 1 && run.get(0).isCharacters()
431+
&& run.get(0).asCharacters().getData().trim().isEmpty()) {
434432
continue;
435433
}
436434
Integer tc = tagsCount.get('x');
437435
if (tc == null) {
438436
tc = 0;
439437
}
440-
res.append("<x" + tc + "/>");
438+
res.append("<x").append(tc).append("/>");
441439
tagsMap.put("x" + tc, run);
442440
tagsCount.put('x', tc + 1);
443441
}
@@ -484,7 +482,7 @@ private void browseRunContents(List<XMLEvent> run, ListIterator<XMLEvent> runIte
484482
break;
485483
}
486484
} else if (next.isStartElement()) {
487-
char prefixInt = '\u0000';
485+
char prefixInt;
488486
final int idx = runIter.previousIndex();
489487
String name = next.asStartElement().getName().getLocalPart();
490488
switch (name) {
@@ -510,13 +508,12 @@ private void browseRunContents(List<XMLEvent> run, ListIterator<XMLEvent> runIte
510508
if (tcInt == null) {
511509
tcInt = 0;
512510
}
513-
LinkedList<XMLEvent> nList = new LinkedList<>();
514-
nList.addAll(run.subList(idx, runIter.nextIndex()));
511+
LinkedList<XMLEvent> nList = new LinkedList<>(run.subList(idx, runIter.nextIndex()));
515512
QName qR = new QName(ooxmlMainParaElement.getNamespaceURI(), "r",
516513
ooxmlMainParaElement.getPrefix());
517514
nList.add(0, eFactory.createStartElement(qR, null, null));
518515
nList.add(eFactory.createEndElement(qR, null));
519-
res.append("<" + prefixInt + tcInt + "/>");
516+
res.append("<").append(prefixInt).append(tcInt).append("/>");
520517
tagsMap.put("" + prefixInt + tcInt, nList);
521518
} else if (next.isCharacters()) {
522519
res.append(next.asCharacters().getData());
@@ -533,8 +530,7 @@ private void compactBuiltTags(StringBuffer res, Pattern pattern) {
533530
List<XMLEvent> lGlobal = tagsMap.get(mFull.group(2));
534531
if (!(lGlobal instanceof LinkedList)) { // subList: copy because it
535532
// will be modified
536-
lGlobal = new LinkedList<>();
537-
lGlobal.addAll(tagsMap.get(mFull.group(2)));
533+
lGlobal = new LinkedList<>(tagsMap.get(mFull.group(2)));
538534
tagsMap.put(mFull.group(2), lGlobal);
539535
}
540536
mUniq = OMEGAT_TAG.matcher(mFull.group(3));
@@ -561,7 +557,7 @@ protected char findPrefix(ListIterator<XMLEvent> wr) {
561557
return '\u0000';
562558
}
563559
if (next.isStartElement() && next.asStartElement().getName().getLocalPart().equals("rPr")) {
564-
List<String> attrs = new LinkedList<String>();
560+
List<String> attrs = new LinkedList<>();
565561
RPR_LOOP: while (wr.hasNext()) { // read w:rPr
566562
next = wr.next();
567563
if (next.isEndElement() && next.asEndElement().getName().getLocalPart().equals("rPr")) {
@@ -571,8 +567,9 @@ protected char findPrefix(ListIterator<XMLEvent> wr) {
571567
continue;
572568
}
573569
if ((defaultsForParagraph != null) && (isInDefaults(next.asStartElement()) > 1)) {
574-
continue RPR_LOOP; // Remove all elements which are
575-
// identical in defaultsForParagraph
570+
// Remove all elements which are
571+
// identical in defaultsForParagraph
572+
continue;
576573
}
577574
String name = next.asStartElement().getName().getLocalPart();
578575
if ("lang".equals(name)) {
@@ -591,24 +588,23 @@ protected char findPrefix(ListIterator<XMLEvent> wr) {
591588
if (name.getLocalPart().startsWith("footnoteRef")) {
592589
return 'n';
593590
}
594-
if (name.getLocalPart().equals("tab")) {
591+
switch (name.getLocalPart()) {
592+
case "tab":
595593
return 'd';
596-
}
597-
if (name.getLocalPart().equals("br")) {
594+
case "br":
598595
return 'd';
599-
}
600-
if (name.getLocalPart().equals("fldChar")) {
596+
case "fldChar":
597+
return 'e';
598+
case "instrText":
599+
// instrText should NOT be translated!!!
601600
return 'e';
602-
}
603-
if (name.getLocalPart().equals("instrText")) {
604-
return 'e'; // instrText should NOT be translated!!!
605601
}
606602
}
607603
}
608604
if (next.isEndElement() && next.asEndElement().getName().getLocalPart().equals("r")) {
609605
return 'e';
610606
}
611-
if (attrs.size() < 1) {
607+
if (attrs.isEmpty()) {
612608
return '\u0000'; // none
613609
}
614610
if (attrs.size() > 1) {
@@ -654,17 +650,16 @@ protected char findPrefix(ListIterator<XMLEvent> wr) {
654650
if (name.getLocalPart().startsWith("footnoteRef")) {
655651
return 'n';
656652
}
657-
if (name.getLocalPart().equals("tab")) {
653+
switch (name.getLocalPart()) {
654+
case "tab":
658655
return 'd';
659-
}
660-
if (name.getLocalPart().equals("br")) {
656+
case "br":
661657
return 'd';
662-
}
663-
if (name.getLocalPart().equals("fldChar")) {
658+
case "fldChar":
659+
return 'e';
660+
case "instrText":
661+
// instrText should NOT be translated!!!
664662
return 'e';
665-
}
666-
if (name.getLocalPart().equals("instrText")) {
667-
return 'e'; // instrText should NOT be translated!!!
668663
}
669664
} else if (next.isCharacters()) {
670665
wr.previous();
@@ -700,8 +695,8 @@ private int isInDefaults(StartElement stEl) {
700695
* after buildTags(src, true) to have the necessary variables filled!
701696
**/
702697
protected List<XMLEvent> restoreTags(String tra) {
703-
LinkedList<XMLEvent> res = new LinkedList<XMLEvent>();
704-
while (tra.length() > 0) {
698+
LinkedList<XMLEvent> res = new LinkedList<>();
699+
while (!tra.isEmpty()) {
705700
Matcher m = OMEGAT_TAG.matcher(tra);
706701
if (m.find()) {
707702
if (m.start() > 0) {

0 commit comments

Comments
 (0)