RFR: 8352858: Make java.net.JarURLConnection fields final [v2]

[Eirik Bjørsnøs]

On Tue, 25 Mar 2025 09:39:07 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please help review this cleanup PR which makes the `java.net.JarURLConnection` fields `jarFileURL` and `entryName` final.
>> 
>> The current `parseSpec` method is somewhat crufty, with some code quality  issues which this PR aims to improve:
>> 
>> * The method-level comment seems stale and misplaced
>> * The local variable  `separator` is confusingly incremented during parsing (using pre AND post increment operators) 
>> * Unused local variables introduced for the sole purpose of attaching `@SuppressWarnings` annotations
>> * The `jarFileURL` and `entryName` fields are both assigned more than once
>> * Block comments are used where line comments would suffice
>> 
>> The PR addresses the above issues by inlining `parseSpec` into the constructor, then extracting static helper methods for parsing the file URL and the entry name. This allows the fields to be made final.
>> 
>> Since this is purely a refactoring PR, no tests are updated and the `noreg-cleanup` label is added in JBS.
>> 
>> Reviewing individual commits in this PR may aid verification of separate refactorings.
>
> Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rename 'separator' to 'separatorIndex', 'nameIdx' to 'nameIndex'

src/java.base/share/classes/java/net/JarURLConnection.java line 165:

> 163:         int separatorIndex = spec.indexOf("!/");
> 164: 
> 165:         // REMIND: we don't handle nested JAR URLs

Just noticed that this comment seems misplaced and is confusing.

The check just asserts that the URL contains the separator. I don't see how this is related to nesting at all. If the URL was nested, we would have more than one separator, right?

If we agree this comment is misplaced, perhaps we should just remove it?

Or perhaps the REMIND here is a type of TODO?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24218#discussion_r2012849349