On Fri, 21 May 2021 14:00:39 GMT, Weijun Wang <wei...@openjdk.org> wrote:

>> The code change refactors classes that have a `SuppressWarnings("removal")` 
>> annotation that covers more than 50KB of code. The big annotation is often 
>> quite faraway from the actual deprecated API usage it is suppressing, and 
>> with the annotation covering too big a portion it's easy to call other 
>> deprecated methods without being noticed.
>> 
>> The code change shows some common solutions to avoid such too wide 
>> annotations:
>> 
>> 1. Extract calls into a method and add annotation on that method
>> 2. Assign the return value of a deprecated method call to a new local 
>> variable and add annotation on the declaration, and then assign that value 
>> to the original l-value.
>> 3. Put declaration and assignment into a single statement if possible.
>> 4. Rewrite code to achieve #3 above.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   typo on windows

src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 120:

> 118:                         vals[1] = 
> Integer.getInteger("sun.net.client.defaultConnectTimeout", 
> 300_000).intValue();
> 119:                         return System.getProperty("file.encoding", 
> "ISO8859_1");
> 120:                     }

It is a bit strange that "file.encoding" seem to get a special treatment - but 
I guess that's OK.

src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 550:

> 548:      * @throws IOException if the connection was unsuccessful.
> 549:      */
> 550:     @SuppressWarnings("removal")

Could the scope of the annotation be further reduced by applying it to the two 
places where `doPrivileged` is called in this method?

src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 921:

> 919:     }
> 920: 
> 921:     @SuppressWarnings("removal")

Could the scope be further reduced by applying it at line 926 in this method - 
at the cost of creating a temporary variable? The code could probably be 
further simplified by using a lambda...


            PrivilegedAction<Socket> pa = () -> new Socket(proxy);
            @SuppressWarnings("removal")
            var ps = AccessController.doPrivileged(pa);
            s = ps;

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

PR: https://git.openjdk.java.net/jdk/pull/4138

Reply via email to