RFR(m): 8145464: jdeprscan - java deprecation scanner (command-line tool)
Stuart Marks
stuart.marks at oracle.com
Wed Aug 24 01:20:03 UTC 2016
Hi Paul,
Thanks for the review and comments. All good stuff. I've pretty much updated
everything as you proposed, with just a few comments and exceptions as noted below.
> 224 if (! Files.isDirectory(Paths.get(dirname))) {
>
> Rogue space, or is that the style you prefer?
Prevailing style seems to prefer no space, so I've adjusted my code accordingly.
> 591 } catch (IOException | ConstantPoolException ex) {
>
> Sometimes you use spaces between the “|” and other times not in other files.
The majority of cases I saw had spaces around the "|" but the code base is
somewhat inconsistent. As it happens I prefer the spaces, so I went along with
the majority and made this code consistently uses spaces.
> 100 final Set<String> validReleases = Set.of("6", "7", "8", "9");
> 101 final Set<String> noForRemovalReleases = Set.of("6", "7", "8”);
>
> Can we produce the sequence using upper bound that is the current runtime version. Generally it would be nice to avoid updating this and other related code on every major release, if indeed that is possible.
This is fairly subtle, I think. The idea is to support the current version plus
three old releases, as defined in JEP 182. [1] The obvious thing to do is to
take the current runtime version and compute the three old releases from that.
The release flag is basically passed straight to the compiler, but I don't know
how the compiler determines its supported release. Simply computing the
supported releases based on the runtime version might create a mismatch with the
compiler (albeit a temporary one).
Joe Darcy has a checklist of things that need to be done when the major version
number is bumped. Either a manual step should be added to update jdeprscan, or
jdeprscan should be modified to get the right information from somewhere and
compute its supported releases from that. I'll have to check with Mr. Darcy.
Meanwhile, I've filed JDK-8164700 [2] to track this.
I've posted an updated webrev. [3]
Thanks,
s'marks
[1] http://openjdk.java.net/jeps/182
[2] https://bugs.openjdk.java.net/browse/JDK-8164700
[3] http://cr.openjdk.java.net/~smarks/reviews/8145464/webrev.1/
On 8/19/16 11:15 AM, Paul Sandoz wrote:
> Hi,
>
> Here is a quick review.
>
> The code in general looks well structured and documented.
>
> Paul.
>
> CSV.java
> —
>
> 74 static enum State {
>
> static is redundant
>
>
> CSVParseException.java
> —
>
> 33 final String reason;
> ...
> 37 public CSVParseException(String reason, String input, int index) {
> 38 super(reason);
> 39 this.reason = reason;
> …
> 53 public String getMessage() {
> 54 return reason + " at index " + index + ": " + input;
> 55 }
>
> Perhaps remove the reason field and use super.getMessage() ?
>
>
> LoadProc.java
> —
>
> 65 List<DeprData> deprList = new ArrayList<>();
>
> Make final?
>
>
> Main.java
> —
>
> 100 final Set<String> validReleases = Set.of("6", "7", "8", "9");
> 101 final Set<String> noForRemovalReleases = Set.of("6", "7", "8”);
>
> Can we produce the sequence using upper bound that is the current runtime version. Generally it would be nice to avoid updating this and other related code on every major release, if indeed that is possible.
>
>
> 224 if (! Files.isDirectory(Paths.get(dirname))) {
>
> Rogue space, or is that the style you prefer?
>
>
> 372 .flatMap(list -> list.stream())
>
> list::stream
>
>
> Pretty
> —
>
> 204 static final Pattern descPat = Pattern.compile("(?<name>.*)\\((?<args>.*)\\)(?<return>.*)");
>
> DESC_PAT
>
>
> 275 // public static void main(String[] args) {
> 276 // System.out.println(depr("", false));
> 277 // System.out.println(depr("9", false));
> 278 // System.out.println(depr("", true));
> 279 // System.out.println(depr("9", true));
> 280 //
> 281 // int[] pos = new int[] { 0 };
> 282 // String desc = "I[F[[Ljava/util/Map$Entry;Z";
> 283 // String t;
> 284 //
> 285 // while ((t = desc(desc, pos)) != null) {
> 286 // System.out.println(t);
> 287 // }
> 288 // }
>
> Wanna keep that?
>
>
> TraverseProc.java
> —
>
> 95 // printPublicTypes(publicTypes);
>
> Keep?
>
>
> CPEntries.java
> —
>
> 48 List<ConstantPool.CONSTANT_Class_info> classes;
> 49 List<ConstantPool.CONSTANT_Fieldref_info> fieldRefs;
> 50 List<ConstantPool.CONSTANT_Methodref_info> methodRefs;
> 51 List<ConstantPool.CONSTANT_InterfaceMethodref_info> interfaceMethodRefs;
>
> final?
>
>
> MethodSig
> —
>
> 168 // public static void main(String[] args) {
> 169 // System.out.println(fromDesc("([Ljava/rmi/RMISecurityManager;)Ljava/lang/Object;"));
> 170 // System.out.println(fromDesc("([[IZLjava/lang/String;B[J)V"));
> 171 // System.out.println(fromDesc("()Ljava/lang/Object;"));
> 172 // System.out.println(fromDesc("(ISJZ)"));
> 173 // }
>
> In tests?
>
>
> Scan.java
> —
>
> 339 // if (! startClassName.equals(foundClassName)) {
> 340 // System.err.printf(" method ref %s:%s%s resolved to %s%n",
> 341 // startClassName, findName, findDesc, curClass.getName());
> 342 // }
>
> Remove?
>
>
> 591 } catch (IOException | ConstantPoolException ex) {
>
> Sometimes you use spaces between the “|” and other times not in other files.
>
>
> TestScan
> —
>
> 66 public void test1() throws IOException {
>
> Better name for test?
>
>
>
>
>
>> On 21 Jul 2016, at 17:16, Stuart Marks <stuart.marks at oracle.com> wrote:
>>
>> Hi all,
>>
>> Please review the code for "jdeprscan", the new Java deprecation scanner command-line tool. This is part of JEP 277, Enhanced Deprecation.
>>
>> The only deprecation warnings provided up until now are those from by javac, emitted when compiling Java source code. If you don't have the source code, or if it's inconvenient to recompile, it's hard to find out what deprecated APIs a class might be using. The jdeprscan tool will scan class files for uses of APIs that are deprecated in the JDK. (A future enhancement will allow scanning for deprecated APIs from other class libraries.)
>>
>> For example, consider the following source file, which compiles without warnings on JDK 8:
>>
>> -----
>> package depr.example;
>>
>> import java.math.BigDecimal;
>>
>> public class Foo {
>> BigDecimal x(BigDecimal num) {
>> Runtime.getRuntime().traceInstructions(true);
>> return num.divide(BigDecimal.TEN, BigDecimal.ROUND_HALF_DOWN);
>> }
>> }
>> -----
>>
>> It's possible to run jdeprscan over the compiled class file in order to detect uses of APIs newly deprecated in Java 9:
>>
>> -----
>> $ jdeprscan -cp build/classes depr.example.Foo
>> class depr/example/Foo uses method java/lang/Runtime traceInstructions (Z)V deprecated FOR REMOVAL
>> class depr/example/Foo uses method java/math/BigDecimal divide (Ljava/math/BigDecimal;I)Ljava/math/BigDecimal; deprecated
>> -----
>>
>> The output is somewhat cryptic and could stand to be cleaned up, but the information is all there.
>>
>> Here's the webrev:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/8145464/webrev.0/
>>
>> User-level documentation is currently a markdown file in the source code:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/8145464/webrev.0/src/jdk.jdeps/share/classes/com/sun/tools/jdeprscan/readme.md.html
>>
>> (Eventually, of course, this will be migrated to the right location.)
>>
>> Most of the changes are in the langtools repository, shown by the webrev above. There is a small change to the jdk repository to build a launcher for jdeprscan. The patch for that is appended below.
>>
>> Thanks,
>>
>> s'marks
>>
>>
>>
>> diff -r b211a52a7439 make/launcher/Launcher-jdk.jdeps.gmk
>> --- a/make/launcher/Launcher-jdk.jdeps.gmk Wed Jul 20 08:32:07 2016 -0700
>> +++ b/make/launcher/Launcher-jdk.jdeps.gmk Thu Jul 21 14:09:38 2016 -0700
>> @@ -36,3 +36,9 @@
>> CFLAGS := -DEXPAND_CLASSPATH_WILDCARDS \
>> -DNEVER_ACT_AS_SERVER_CLASS_MACHINE, \
>> ))
>> +
>> +$(eval $(call SetupBuildLauncher, jdeprscan, \
>> + MAIN_CLASS := com.sun.tools.jdeprscan.Main, \
>> + CFLAGS := -DEXPAND_CLASSPATH_WILDCARDS \
>> + -DNEVER_ACT_AS_SERVER_CLASS_MACHINE, \
>> +))
>
More information about the jdk9-dev
mailing list