Skip to content

Commit 6cf8b0d

Browse files
claude[bot]vmassol
claude[bot]
and
vmassol
authored
[Misc] Explain why these code blocks are empty and fix comment syntax (SonarQube) (#6221)
* [Misc] Explain why these code blocks are empty and fix comment syntax (SonarQube) * java:S108: document the 80 empty blocks reported in 19 files with a comment saying why they are empty, and remove two blocks that are empty and useless (an empty finally and an empty else in XWiki). * java:S9355: turn the block comments that carry Javadoc tags into real Javadoc comments (MailStatus fields and XWiki#getUserTimeZone). Co-Authored-By: Vincent Massol <vincent@massol.net> * [Misc] Turn the empty-block comments into TODOs (SonarQube) * Following review: a catch that neither rethrows nor logs is a bug to fix later, so each comment now starts with a "// TODO:" asking either to log a warning, or (where an exception is the expected outcome) to change the logic so the case isn't signalled by an exception. * The two blocks that are not exception handling (dropping XML control characters, an empty switch default) keep their plain comment. Co-Authored-By: Vincent Massol <vincent@massol.net> * [Misc] Replace the try/catch of MonitorTimer#toString() with a null check * Following review: the catch was only there to swallow the NPE thrown when the duration of a timer that has not ended yet is printed, so a null check on the two dates says it directly and removes the empty block. * Behaviour is unchanged: the "Duration: " label was already appended before the throwing expression, so a running timer printed the label with no value before and still does. Co-Authored-By: Vincent Massol <vincent@massol.net> * [Misc] Drop the TODOs where catching the exception is the valid design * Following review: XWikiRightNotFoundException is a domain signal meaning "no right defined at this level", so catching it to continue with the next check is valid and the 12 TODOs asking to change that logic are removed; the comment saying what the catch means stays. * FeedPlugin's TODO is reworded: falling back when the Map constructor is missing is fine, but catching Throwable there also hides a failure of the constructor that was found, so the ask is to log a warning. Co-Authored-By: Vincent Massol <vincent@massol.net> --------- Co-authored-by: Vincent Massol <vincent@massol.net>
1 parent 6f166ae commit 6cf8b0d

20 files changed

Lines changed: 161 additions & 17 deletions

File tree

‎xwiki-platform-core/xwiki-platform-feed/xwiki-platform-feed-api/src/main/java/com/xpn/xwiki/plugin/feed/FeedPlugin.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,8 @@ public void initCache(XWikiContext context) throws XWikiException
189189
iCapacity = Integer.parseInt(capacity);
190190
}
191191
} catch (Exception e) {
192+
// TODO: log a warning instead of ignoring this exception.
193+
// The default capacity is then used.
192194
}
193195

194196
initCache(iCapacity, context);
@@ -219,6 +221,8 @@ protected void prepareCache(XWikiContext context)
219221
initCache(context);
220222
}
221223
} catch (XWikiException e) {
224+
// TODO: log a warning instead of ignoring this exception.
225+
// The cache initialization is retried the next time the cache is needed.
222226
}
223227
}
224228

@@ -855,6 +859,9 @@ public SyndEntrySource getSyndEntrySource(String className, Map<String, Object>
855859
ctor = sesc.getConstructor(Map.class);
856860
return ctor.newInstance(params);
857861
} catch (Throwable t) {
862+
// TODO: log a warning instead of ignoring this exception: catching Throwable here also
863+
// hides a failure of the constructor that was found.
864+
// The default constructor is used below.
858865
}
859866
}
860867
ctor = sesc.getConstructor();

‎xwiki-platform-core/xwiki-platform-mail/xwiki-platform-mail-send/xwiki-platform-mail-send-api/src/main/java/org/xwiki/mail/MailStatus.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,42 +41,42 @@ public class MailStatus
4141
*/
4242
private String uniqueMessageId;
4343

44-
/*
44+
/**
4545
* @see #getState()
4646
*/
4747
private String state;
4848

49-
/*
49+
/**
5050
* @see #getBatchID()
5151
*/
5252
private String batchId;
5353

54-
/*
54+
/**
5555
* @see #getDate()
5656
*/
5757
private Date date;
5858

59-
/*
59+
/**
6060
* @see #getRecipients()
6161
*/
6262
private String recipients;
6363

64-
/*
64+
/**
6565
* @see #getType()
6666
*/
6767
private String type;
6868

69-
/*
69+
/**
7070
* @see #getErrorSummary()
7171
*/
7272
private String errorSummary;
7373

74-
/*
74+
/**
7575
* @see #getErrorDescription()
7676
*/
7777
private String errorDescription;
7878

79-
/*
79+
/**
8080
* @see #getWiki()
8181
*/
8282
private String wiki;

‎xwiki-platform-core/xwiki-platform-notifications/xwiki-platform-notifications-filters/xwiki-platform-notifications-filters-default/src/main/java/org/xwiki/notifications/filters/internal/livedata/custom/NotificationCustomFiltersQueryHelper.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ private Optional<FiltersHQLQuery> handleFilter(List<LiveDataQuery.Filter> queryF
9696
this.handleEventTypeFilter(queryFilter, queryWhereClauses, result);
9797

9898
default -> {
99+
// The other properties don't contribute to the where clause.
99100
}
100101
}
101102
}

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,7 +1228,6 @@ public static Object getPrivateField(Object obj, String fieldName)
12281228
LOGGER.error("Failed to get private field with name [{}]", fieldName, e);
12291229

12301230
return null;
1231-
} finally {
12321231
}
12331232
}
12341233

@@ -1745,6 +1744,8 @@ public byte[] getResourceContentAsBytes(String name) throws IOException
17451744
return IOUtils.toByteArray(is);
17461745
}
17471746
} catch (Exception e) {
1747+
// TODO: log a warning instead of ignoring this exception.
1748+
// The resource is then read from the file system below.
17481749
}
17491750
}
17501751
return FileUtils.readFileToByteArray(new File(name));
@@ -1758,6 +1759,8 @@ public boolean resourceExists(String name)
17581759
return true;
17591760
}
17601761
} catch (IOException e) {
1762+
// TODO: log a warning instead of ignoring this exception.
1763+
// The resource is then looked for on the file system below.
17611764
}
17621765
}
17631766
try {
@@ -1815,7 +1818,6 @@ public String ParamAsRealPathVerified(String param)
18151818
fpath = new File(path);
18161819
if (fpath.exists()) {
18171820
return path;
1818-
} else {
18191821
}
18201822
return null;
18211823
}
@@ -3193,6 +3195,8 @@ public Locale getLocalePreference(XWikiContext context)
31933195
}
31943196
}
31953197
} catch (Exception e) {
3198+
// TODO: log a warning instead of ignoring this exception.
3199+
// The next way of getting the locale is used below.
31963200
}
31973201

31983202
// As no language parameter was passed in the request, try to get the language to use from a cookie.
@@ -3206,6 +3210,8 @@ public Locale getLocalePreference(XWikiContext context)
32063210
}
32073211
}
32083212
} catch (Exception e) {
3213+
// TODO: log a warning instead of ignoring this exception.
3214+
// The next way of getting the locale is used below.
32093215
}
32103216

32113217
// If the default language is preferred, and since the user didn't explicitly ask for a
@@ -3398,6 +3404,8 @@ public String getDocLanguagePreferenceNew(XWikiContext context)
33983404
try {
33993405
requestLanguage = Util.normalizeLanguage(context.getRequest().getParameter(LANGUAGE));
34003406
} catch (Exception ex) {
3407+
// TODO: log a warning instead of ignoring this exception.
3408+
// This language source is then simply not taken into account.
34013409
}
34023410

34033411
// Get user preference
@@ -3409,6 +3417,8 @@ public String getDocLanguagePreferenceNew(XWikiContext context)
34093417
userdoc.getStringValue(XWikiUsersDocumentInitializer.CLASS_REFERENCE_STRING, DEFAULT_LANGUAGE);
34103418
}
34113419
} catch (XWikiException e) {
3420+
// TODO: log a warning instead of ignoring this exception.
3421+
// This language source is then simply not taken into account.
34123422
}
34133423

34143424
// Get navigator language setting
@@ -3427,6 +3437,8 @@ public String getDocLanguagePreferenceNew(XWikiContext context)
34273437
try {
34283438
cookieLanguage = Util.normalizeLanguage(getUserPreferenceFromCookie(LANGUAGE, context));
34293439
} catch (Exception e) {
3440+
// TODO: log a warning instead of ignoring this exception.
3441+
// This language source is then simply not taken into account.
34303442
}
34313443

34323444
// Determine which language to use
@@ -3495,6 +3507,8 @@ public String getInterfaceLanguagePreference(XWikiContext context)
34953507
try {
34963508
requestLanguage = Util.normalizeLanguage(context.getRequest().getParameter(INTERFACE_LANGUAGE));
34973509
} catch (Exception ex) {
3510+
// TODO: log a warning instead of ignoring this exception.
3511+
// This language source is then simply not taken into account.
34983512
}
34993513

35003514
// Get context language
@@ -3510,6 +3524,8 @@ public String getInterfaceLanguagePreference(XWikiContext context)
35103524
"default_interface_language");
35113525
}
35123526
} catch (XWikiException e) {
3527+
// TODO: log a warning instead of ignoring this exception.
3528+
// This language source is then simply not taken into account.
35133529
}
35143530

35153531
// Get navigator language setting
@@ -3528,6 +3544,8 @@ public String getInterfaceLanguagePreference(XWikiContext context)
35283544
try {
35293545
cookieLanguage = Util.normalizeLanguage(getUserPreferenceFromCookie(INTERFACE_LANGUAGE, context));
35303546
} catch (Exception e) {
3547+
// TODO: log a warning instead of ignoring this exception.
3548+
// This language source is then simply not taken into account.
35313549
}
35323550

35333551
// Determine which language to use
@@ -3656,9 +3674,13 @@ public void flushCache(XWikiContext context)
36563674
try {
36573675
getClass(className, context).flushCache();
36583676
} catch (Exception e) {
3677+
// TODO: log a warning instead of ignoring this exception.
3678+
// A class that cannot be flushed must not prevent the other classes from being flushed.
36593679
}
36603680
}
36613681
} catch (Exception e) {
3682+
// TODO: log a warning instead of ignoring this exception.
3683+
// Failing to get the list of classes only means that no class cache is flushed.
36623684
}
36633685

36643686
}
@@ -4501,6 +4523,8 @@ public String include(String topic, boolean isForm, XWikiContext context) throws
45014523
}
45024524
includedDocs.add(prefixedTopic);
45034525
} catch (Exception e) {
4526+
// TODO: log a warning instead of ignoring this exception.
4527+
// Failing to update the list of included documents only disables the recursion check.
45044528
}
45054529

45064530
// Get document to include
@@ -4550,6 +4574,8 @@ public String include(String topic, boolean isForm, XWikiContext context) throws
45504574
includedDocs.remove(prefixedTopic);
45514575
}
45524576
} catch (Exception e) {
4577+
// TODO: log a warning instead of ignoring this exception.
4578+
// Failing to update the list of included documents only disables the recursion check.
45534579
}
45544580
return result;
45554581
} finally {
@@ -6263,6 +6289,8 @@ public String getRefererText(String referer, XWikiContext context)
62636289
}
62646290
}
62656291
} catch (Exception e) {
6292+
// TODO: log a warning instead of ignoring this exception.
6293+
// The referer text is then computed from the URL below.
62666294
}
62676295

62686296
String result = referer.substring(referer.indexOf("://") + 3);
@@ -6542,6 +6570,8 @@ public String formatDate(Date date, String format, XWikiContext context)
65426570
try {
65436571
sdf.setTimeZone(TimeZone.getTimeZone(getUserTimeZone(context)));
65446572
} catch (Exception e) {
6573+
// TODO: log a warning instead of ignoring this exception.
6574+
// The date is then formatted with the default time zone.
65456575
}
65466576

65476577
return sdf.format(date);
@@ -7160,6 +7190,8 @@ public String getMacroList(XWikiContext context)
71607190
try {
71617191
macrosmapping = getResourceContent(MACROS_FILE);
71627192
} catch (IOException e) {
7193+
// TODO: log a warning instead of ignoring this exception.
7194+
// The mapping is then only made of the macros defined in the wiki preferences.
71637195
}
71647196

71657197
macrosmapping += "\r\n" + xwiki.getXWikiPreference("macros_mapping", "", context);

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/api/Document.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,8 @@ public Vector<Object> getObjects(String classname, String key, String value)
13401340
}
13411341
}
13421342
} catch (Exception e) {
1343+
// TODO: log a warning instead of ignoring this exception.
1344+
// The objects that could be retrieved so far are returned.
13431345
}
13441346
return result;
13451347
}

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/api/XWiki.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,6 +1003,8 @@ public List<Document> wrapDocs(List<?> docs)
10031003
}
10041004
}
10051005
} catch (XWikiException ex) {
1006+
// TODO: log a warning instead of ignoring this exception.
1007+
// A document that cannot be wrapped is simply not added to the result.
10061008
}
10071009
}
10081010
}
@@ -2329,9 +2331,10 @@ public String formatDate(Date date, String format)
23292331
return this.xwiki.formatDate(date, format, getXWikiContext());
23302332
}
23312333

2332-
/*
2333-
* Allow to read user setting providing the user timezone All dates will be expressed with this timezone @return the
2334-
* timezone
2334+
/**
2335+
* Allow to read the user setting providing the user timezone. All dates are expressed with this timezone.
2336+
*
2337+
* @return the timezone
23352338
*/
23362339
public String getUserTimeZone()
23372340
{

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3706,6 +3706,8 @@ public String display(String fieldname, BaseObject obj, XWikiContext context)
37063706
try {
37073707
type = (String) context.get("display");
37083708
} catch (Exception e) {
3709+
// TODO: log a warning instead of ignoring this exception.
3710+
// The "view" display type is then used below.
37093711
}
37103712

37113713
if (type == null) {
@@ -5426,6 +5428,7 @@ protected String encodedXMLStringAsUTF8(String xmlString)
54265428
break;
54275429
default:
54285430
if (character < 0x20) {
5431+
// Control characters are not allowed in XML and are simply dropped.
54295432
} else if (character > 0x7F) {
54305433
result.append("&#x");
54315434
result.append(Integer.toHexString(character).toUpperCase());
@@ -7404,7 +7407,8 @@ private XWikiAttachment retrieveDeletedAttachment(XWikiDocument doc, XWikiAttach
74047407
try {
74057408
is.close();
74067409
} catch (IOException ex) {
7407-
7410+
// TODO: log a warning instead of ignoring this exception.
7411+
// A failure to close the stream must not hide the result of the operation.
74087412
}
74097413
}
74107414
}

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/groovy/ParseGroovyFromString.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ private void initCache(XWikiContext xcontext) throws XWikiException
6969
classCacheSize = Integer.parseInt(capacity);
7070
}
7171
} catch (Exception e) {
72+
// TODO: log a warning instead of ignoring this exception.
73+
// The default cache capacity is then used.
7274
}
7375

7476
initCache(classCacheSize);
@@ -93,6 +95,8 @@ private void prepareCache(XWikiContext context)
9395
initCache(context);
9496
}
9597
} catch (Exception e) {
98+
// TODO: log a warning instead of ignoring this exception.
99+
// The cache initialization is retried the next time the cache is needed.
96100
}
97101
}
98102

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/monitor/api/MonitorPlugin.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ public void setWikiPage(String page)
155155
mdata.setWikiPage(page);
156156
}
157157
} catch (Throwable e) {
158+
// TODO: log a warning instead of ignoring this exception.
159+
// Monitoring must never break the request being monitored.
158160
}
159161
}
160162

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/monitor/api/MonitorTimer.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,10 @@ public String toString()
8484
str.append(" End Date: ");
8585
str.append(this.endDate);
8686
str.append(" Duration: ");
87-
try {
88-
str.append(this.endDate.getTime() - this.startDate.getTime());
89-
} catch (Exception e) {
87+
// The duration is only known once the timer has ended: a timer that is still running has no end
88+
// date, and printing it must not fail.
89+
if (this.startDate != null && this.endDate != null) {
90+
str.append(getDuration());
9091
}
9192
return str.toString();
9293
}

0 commit comments

Comments
 (0)