Misc IntelliJ warnings and fixes #884

Juiceman · 2024-01-16T03:41:15Z

Assortment of fixes suggested by IntelliJ when analyzing the source code.

Code compiles and my node seems to be working.

ArneBab · 2024-01-26T21:00:28Z

src/freenet/client/filter/FilterUtils.java

-					Integer.valueOf(value.substring(3,4),16).intValue();
+                    Integer.valueOf(value.substring(1, 2), 16);
+                    Integer.valueOf(value.substring(2, 3), 16);
+                    Integer.valueOf(value.substring(3, 4), 16);


here the indentation seems to have broken. Please keep it consistent with the surrounding indentation, even if that may seem strange. Consistency wins against beauty here, because we cannot just re-indent all files without breaking all pull-requests.

Should be fixed in [b32d928]

Thank you

ArneBab · 2024-01-26T21:00:38Z

src/freenet/client/filter/FilterUtils.java

-					Integer.valueOf(value.substring(5,7),16).intValue();
+                    Integer.valueOf(value.substring(1, 3), 16);
+                    Integer.valueOf(value.substring(3, 5), 16);
+                    Integer.valueOf(value.substring(5, 7), 16);


indentation, too.

Should be fixed in [b32d928]

Thank you

ArneBab · 2024-01-26T21:01:16Z

src/freenet/client/filter/FlacFilter.java

-					buffer.add(Byte.valueOf((byte) ((frameHeader & 0xFF00) >>> 8)));
-					buffer.add(Byte.valueOf((byte) (frameHeader & 0x00FF)));
+					buffer.add((byte) ((frameHeader & 0xFF00) >>> 8));
+					buffer.add((byte) (frameHeader & 0x00FF));


is it guaranteed that casting to byte will work in all cases here?

The casting already happens inside the call to valueOf, removing the manual boxing here doesn’t change anything about that.

ArneBab · 2024-01-26T21:01:34Z

src/freenet/client/filter/FlacFilter.java

@@ -90,10 +90,10 @@ public void readFilter(
 								packet = new FlacFrame(payload);
 							} else {
 								firstHalfOfSyncHeaderFound = false;
-								buffer.add(Byte.valueOf((byte) 0xFF));
+								buffer.add((byte) 0xFF);


is the cast safe?

Same thing, the casting isn’t changed, only the boxing is removed.

ArneBab · 2024-01-26T21:01:39Z

src/freenet/client/filter/FlacFilter.java

 							}
 						}
-						buffer.add(Byte.valueOf((byte) (data & 0xFF)));
+						buffer.add((byte) (data & 0xFF));


is the cast safe?

Also safe. 🙂

ArneBab · 2024-01-26T21:02:28Z

src/freenet/client/filter/OggPage.java

 			}
 			if(concludingPartialSegment != 0) {
-				segmentSizes.add(Byte.valueOf(intToUnsignedByte(concludingPartialSegment)));
+				segmentSizes.add(intToUnsignedByte(concludingPartialSegment));


these two are also implicit casts. Are they safe?

There’s no implicit casting happening here, intToUnsignedByte already returns a byte, that would be an identity conversion, followed by a boxing conversion (JLS § 5.1.1, § 5.1.7).

ArneBab · 2024-01-26T21:03:05Z

src/freenet/clients/fcp/LoadPlugin.java

+                        Logger.error(this, "This should really not happen!", new Exception("FIXME"));
+                        handler.send(new ProtocolErrorMessage(ProtocolErrorMessage.INTERNAL_ERROR, false, "This should really not happen! See logs for details.", identifier, false));
+                        return;
+                }


looks much nicer! (but needs consistent indentation)

I personally hate switch statements and would much rather see this replaced with a different construct that actually hides all this ugliness (e.g. by a map or a strategy pattern or something) but that’s very much an issue for another day, this is fine. 🙂

ArneBab · 2024-01-26T21:04:00Z

src/freenet/clients/http/ConnectionsToadlet.java

@@ -499,7 +499,7 @@ public void handleMethodGET(URI uri, final HTTPRequest request, ToadletContext c
 								//i now points to the proper location (equal, insertion point, or end-of-list)
 								//maybe better called "reverseGroup"?
 								List<PeerNodeStatus> peerGroup;
-								if (i<max && locations.get(i).doubleValue()==location) {
+								if (i<max && locations.get(i) ==location) {


is == safe without the doubleValue()? This may cause type coercion to float.

No, doubles are not coerced to floats; in an equality comparison binary numeric promotion is performed, and as one of the operands is a double, the other one is promoted to double as well; as they are both already doubles, nothing ~~bad~~ at all will happen here. (JLS § 15.21.1, § 5.6.2.)

ArneBab · 2024-01-26T21:04:29Z

src/freenet/clients/http/DarknetConnectionsToadlet.java

+                    return ((DarknetPeerNodeStatus) firstNode).getTheirVisibility().compareTo(((DarknetPeerNodeStatus) secondNode).getTheirVisibility());
+                default:
+                    return super.customCompare(firstNode, secondNode, sortBy);
+            }


mind the indentation.

ArneBab · 2024-01-26T21:04:46Z

src/freenet/clients/http/FProxyToadlet.java

@@ -1103,8 +1103,7 @@ private static String getForceValue(FreenetURI key, long time) {
 			throw new Error(e);
 		}

-		String f = HexUtil.bytesToHex(SHA256.digest(bos.toByteArray()));
-		return f;
+        return HexUtil.bytesToHex(SHA256.digest(bos.toByteArray()));


indentation

ArneBab · 2024-01-26T21:05:26Z

src/freenet/clients/http/QueueToadlet.java

+                        default:
+                            isSet = false;
+                            break;
+                    }


indentation again (IntelliJ can have problems there)

ArneBab · 2024-01-26T21:06:21Z

src/freenet/clients/http/staticfiles/freenetjs/hosted.html

@@ -322,7 +322,7 @@
 window.__gwt_module_id = 0;
 </script></head>
 <body>
-<font face='arial' size='-1'>This html file is for Development Mode support.</font>
+<span style="font-family: arial; font-size: smaller; ">This html file is for Development Mode support.</span>


yikes, we still had those old styles in there! Good catch!

ArneBab · 2024-01-26T21:06:40Z

src/freenet/clients/http/wizardsteps/SECURITY_PHYSICAL.java

+                } catch (IllegalArgumentException e) {
+                    //Render the default page if unable to parse password prompt type.
+                    return false;
+                }


indentation

ArneBab · 2024-01-26T21:07:19Z

src/freenet/clients/http/wizardsteps/SECURITY_PHYSICAL.java

+            case "delete":
+                SecurityLevelsToadlet.sendCantDeleteMasterKeysFileInner(helper, core.node.getMasterPasswordFile().getPath(), newThreatLevel.name());
+                return true;
+        }


this needs consistent indentation again

ArneBab · 2024-01-26T21:07:31Z

src/freenet/io/Inet4AddressMatcher.java

@@ -87,8 +87,7 @@ public Inet4AddressMatcher(String cidrHostname) {
 	 */
 	public static int convertToBytes(String address) {
 		StringTokenizer addressTokens = new StringTokenizer(address, ".");
-		int bytes = Integer.parseInt(addressTokens.nextToken()) << 24 | Integer.parseInt(addressTokens.nextToken()) << 16 | Integer.parseInt(addressTokens.nextToken()) << 8 | Integer.parseInt(addressTokens.nextToken());
-		return bytes;
+        return Integer.parseInt(addressTokens.nextToken()) << 24 | Integer.parseInt(addressTokens.nextToken()) << 16 | Integer.parseInt(addressTokens.nextToken()) << 8 | Integer.parseInt(addressTokens.nextToken());


indentation

ArneBab · 2024-01-26T21:07:39Z

src/freenet/io/comm/DMT.java

@@ -1439,8 +1439,7 @@ public static Message createFNPVisibility(short visibility) {
 	}};

 	public static Message createFNPVoid() {
-		Message msg = new Message(FNPVoid);
-		return msg;
+        return new Message(FNPVoid);


indentation

ArneBab · 2024-01-26T21:09:03Z

src/freenet/keys/FreenetURI.java

+                throw new MalformedURLException("Cannot write USKs as binary keys");
+            default:
+                throw new MalformedURLException("Cannot write key of type " + keyType + " - do not know how");
+        }
 		if(!keyType.equals("KSK")) {


this is nice! (but indentation)

ArneBab · 2024-01-26T21:09:16Z

src/freenet/l10n/ISO639_3.java

+                        return Scope.Special;
+                    default:
+                        throw new IllegalArgumentException("Unknown scope abbreviation: " + abbreviation);
+                }


indentation

ArneBab · 2024-01-26T21:09:24Z

src/freenet/l10n/ISO639_3.java

+                        return Type.Special;
+                    default:
+                        throw new IllegalArgumentException("Unknwon type abbreviation: " + abbreviation);
+                }


indentation

ArneBab · 2024-01-26T21:09:30Z

src/freenet/node/Announcer.java

@@ -302,8 +302,7 @@ protected void stop() {

 	public int getAnnouncementThreshold() {
 		// First, do we actually need to announce?
-		int target = Math.min(MIN_OPENNET_CONNECTED_PEERS, om.getNumberOfConnectedPeersToAimIncludingDarknet() / 2);
-		return target;
+        return Math.min(MIN_OPENNET_CONNECTED_PEERS, om.getNumberOfConnectedPeersToAimIncludingDarknet() / 2);


indentation

ArneBab · 2024-01-26T21:11:06Z

src/freenet/support/Fields.java

+            1000L * 1000L * 1000L, 1L << 30,
+            1000L * 1000L * 1000L * 1000L, 1L << 40,
+            1000L * 1000L * 1000L * 1000L * 1000, 1L << 50,
+            1000L * 1000L * 1000L * 1000L * 1000L * 1000L, 1L << 60


indentation

I am unable to find an issue with the indentation of the source file.

ArneBab · 2024-01-26T21:11:39Z

src/freenet/support/Fields.java

+                    default:
+                        throw new NumberFormatException(
+                                "unknown time/date delta type: " + deltaTypeString);
+                }


is the string switch already available in Java 8?

Yes, with constant expressions for the case statements. Had to check myself because I also remember something about it being loudly introduced at a much later version but that’s probably for non-constant expressions in the case statements. 🙂

Bombe · 2024-01-28T09:19:44Z

src/freenet/client/FailureCodeTracker.java

@@ -219,7 +219,7 @@ public InsertExceptionMode getFirstCodeInsert() {
    }

 	public synchronized int getFirstCode() {
-		return ((Integer) map.keySet().toArray()[0]).intValue();
+		return (Integer) map.keySet().toArray()[0];


I think I would replace this with

return map.keySet().iterator().next();

and avoid the casting altogether.

Bombe · 2024-01-28T09:23:10Z

src/freenet/client/filter/FlacFilter.java

-					buffer.add(Byte.valueOf((byte) ((frameHeader & 0xFF00) >>> 8)));
-					buffer.add(Byte.valueOf((byte) (frameHeader & 0x00FF)));
+					buffer.add((byte) ((frameHeader & 0xFF00) >>> 8));
+					buffer.add((byte) (frameHeader & 0x00FF));


The casting already happens inside the call to valueOf, removing the manual boxing here doesn’t change anything about that.

Bombe · 2024-01-28T09:23:50Z

src/freenet/client/filter/FlacFilter.java

@@ -90,10 +90,10 @@ public void readFilter(
 								packet = new FlacFrame(payload);
 							} else {
 								firstHalfOfSyncHeaderFound = false;
-								buffer.add(Byte.valueOf((byte) 0xFF));
+								buffer.add((byte) 0xFF);


Same thing, the casting isn’t changed, only the boxing is removed.

Bombe · 2024-01-28T09:24:03Z

src/freenet/client/filter/FlacFilter.java

 							}
 						}
-						buffer.add(Byte.valueOf((byte) (data & 0xFF)));
+						buffer.add((byte) (data & 0xFF));


Also safe. 🙂

Bombe · 2024-01-28T09:33:28Z

src/freenet/client/filter/OggPage.java

 			}
 			if(concludingPartialSegment != 0) {
-				segmentSizes.add(Byte.valueOf(intToUnsignedByte(concludingPartialSegment)));
+				segmentSizes.add(intToUnsignedByte(concludingPartialSegment));


There’s no implicit casting happening here, intToUnsignedByte already returns a byte, that would be an identity conversion, followed by a boxing conversion (JLS § 5.1.1, § 5.1.7).

src/freenet/node/LoggingConfigHandler.java

src/freenet/node/PeerManager.java

Bombe · 2024-01-28T10:09:23Z

src/freenet/support/Fields.java

+                    default:
+                        throw new NumberFormatException(
+                                "unknown time/date delta type: " + deltaTypeString);
+                }


Yes, with constant expressions for the case statements. Had to check myself because I also remember something about it being loudly introduced at a much later version but that’s probably for non-constant expressions in the case statements. 🙂

spaces back into tabs to match existing style

…ables

Juiceman · 2024-02-09T15:36:52Z

Closing. Will redo sometime in the future as smaller patches that can be merged or changed without holding up the whole PR.

Juiceman added 7 commits January 15, 2024 20:08

Fix typo and remove extra '+'

0ced8cb

Replace deprecated tag

d1024c7

Replace indexOf with .contains()

2e0c884

Boxing and unboxing are unnecessary post Java 5

d9a34eb

Use switch statements

80e010d

Inline some redundant local variables

b913233

Use capital L for readability

f2c6731

ArneBab reviewed Jan 26, 2024

View reviewed changes

Bombe requested changes Jan 28, 2024

View reviewed changes

Fix indentation

b32d928

spaces back into tabs to match existing style

Juiceman added 12 commits January 28, 2024 16:33

Attempt to fix indentation

80de5de

Revert spaces to tabs to match existing

5ac96f0

Revert spaces to tabs to match existing

26bb183

Revert spaces to tabs to match existing

95c9bc3

Revert spaces to tabs to match existing

e99983e

Remove now unnecessary parenthesis

0150339

Spaces back to tabs to match existing indentation

38a23ec

Spaces back to tabs to match existing indentation

2717f80

Spaces back to tabs to match existing indentation

181e9c1

Spaces back to tabs to match existing indentation

36b651e

Remove unnecessary parenthesis

1806c75

Remove unnecessary variables which were just renames of existing vari…

2bff114

…ables

Juiceman closed this Feb 9, 2024

Misc IntelliJ warnings and fixes #884

Misc IntelliJ warnings and fixes #884

Conversation

Juiceman commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Juiceman commented Feb 9, 2024