From be4614eb5e4efcea3f3ef4d18f94cfb36fd557f4 Mon Sep 17 00:00:00 2001 From: Jonathan Gibbons Date: Fri, 5 Jan 2024 22:16:52 +0000 Subject: [PATCH] 8323016: Improve reporting for bad options Reviewed-by: prappo --- .../jdk/javadoc/internal/tool/Start.java | 39 +++++++++- .../tool/resources/javadoc.properties | 11 ++- .../jdk/javadoc/tool/BadOptionsTest.java | 73 ++++++++++++++++++- 3 files changed, 118 insertions(+), 5 deletions(-) diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java index 3f06cf1ae20..6407196a2ab 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -40,6 +40,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.tools.JavaFileManager; import javax.tools.JavaFileObject; @@ -457,6 +458,7 @@ public class Start { if (haveErrors && result.isOK()) { result = ERROR; } + log.flush(); log.printErrorWarningCounts(); log.flush(); } @@ -657,11 +659,44 @@ public class Start { // check if arg is accepted by the tool before emitting error if (!isToolOption) { text = log.getText("main.invalid_flag", arg); - throw new OptionException(ERROR, this::showUsage, text); + throw new OptionException(ERROR, () -> reportBadOption(arg), text); } return m * idx; } + private void reportBadOption(String name) { + var allOptionNames = Stream.concat( + getToolOptions().getSupportedOptions().stream() + .flatMap(o -> o.getNames().stream()), + docletOptions.stream() + .flatMap(o -> o.getNames().stream())); + record Pair(String word, double similarity) { } + final double MIN_SIMILARITY = 0.7; + var suggestions = allOptionNames + .map(t -> new Pair(t, similarity(t, name))) + .sorted(Comparator.comparingDouble(Pair::similarity).reversed() /* more similar first */) + // .peek(p -> System.out.printf("%.3f, (%s ~ %s)%n", p.similarity, p.word, name)) // debug + .takeWhile(p -> Double.compare(p.similarity, MIN_SIMILARITY) >= 0) + .map(Pair::word) + .toList(); + switch (suggestions.size()) { + case 0 -> { } + case 1 -> showLinesUsingKey("main.did-you-mean", suggestions.getFirst()); + default -> showLinesUsingKey("main.did-you-mean-one-of", String.join(" ", suggestions)); + } + showLinesUsingKey("main.for-more-details-see-usage"); + } + + // a value in [0, 1] range: the closer the value is to 1, the more similar + // the strings are + private static double similarity(String a, String b) { + // Normalize the distance so that similarity between "x" and "y" is + // less than that of "ax" and "ay". Use the greater of two lengths + // as normalizer, as it's an upper bound for the distance. + return 1.0 - ((double) StringUtils.DamerauLevenshteinDistance.of(a, b)) + / Math.max(a.length(), b.length()); + } + private static Set getSupportedOptionsOf(Doclet doclet) { Set options = doclet.getSupportedOptions(); return options == null ? Set.of() : options; diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties index e4aacca212d..ef9425a02be 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -34,6 +34,15 @@ main.usage=Usage:\n\ \ javadoc [options] [packagenames] [sourcefiles] [@files]\n\ where options include: +main.did-you-mean=\ + Did you mean: {0} + +main.did-you-mean-one-of=\ + Did you mean one of: {0} + +main.for-more-details-see-usage=\ + For more details on available options, use --help or --help-extra + main.opt.at.arg=\ main.opt.at.desc=\ diff --git a/test/langtools/jdk/javadoc/tool/BadOptionsTest.java b/test/langtools/jdk/javadoc/tool/BadOptionsTest.java index e0887ed5646..568b6de9055 100644 --- a/test/langtools/jdk/javadoc/tool/BadOptionsTest.java +++ b/test/langtools/jdk/javadoc/tool/BadOptionsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,7 +23,7 @@ /* * @test - * @bug 8169676 8175055 + * @bug 8169676 8175055 8323016 * @summary boolean result of Option.process is often ignored * @modules jdk.compiler/com.sun.tools.javac.api * @modules jdk.compiler/com.sun.tools.javac.main @@ -151,6 +151,75 @@ public class BadOptionsTest extends TestRunner { "1 error"); } + @Test + public void testOptionNotFound_NoSuggestions() { + var result = new JavadocTask(tb, Task.Mode.CMDLINE) + .options("--not-a-path") + .run(Task.Expect.FAIL) + .writeAll(); + checkFound(String.join("\n", result.getOutputLines(Task.OutputKind.DIRECT)), + """ + error: invalid flag: --not-a-path + For more details on available options, use --help or --help-extra""" + ); + } + + @Test + public void testOptionNotFound_OneSuggestion() { + var result = new JavadocTask(tb, Task.Mode.CMDLINE) + .options("--middle-path") + .run(Task.Expect.FAIL) + .writeAll(); + checkFound(String.join("\n", result.getOutputLines(Task.OutputKind.DIRECT)), + """ + error: invalid flag: --middle-path + Did you mean: --module-path + For more details on available options, use --help or --help-extra""" + ); + } + + @Test + public void testOptionNotFound_TwoSuggestions() { + var result = new JavadocTask(tb, Task.Mode.CMDLINE) + .options("--sourcepath") + .run(Task.Expect.FAIL) + .writeAll(); + checkFound(String.join("\n", result.getOutputLines(Task.OutputKind.DIRECT)), + """ + error: invalid flag: --sourcepath + Did you mean one of: --source-path -sourcepath + For more details on available options, use --help or --help-extra""" + ); + } + + @Test + public void testOptionNotFound_ThreeSuggestions() { + var result = new JavadocTask(tb, Task.Mode.CMDLINE) + .options("--classpath") + .run(Task.Expect.FAIL) + .writeAll(); + checkFound(String.join("\n", result.getOutputLines(Task.OutputKind.DIRECT)), + """ + error: invalid flag: --classpath + Did you mean one of: --class-path -classpath -bootclasspath + For more details on available options, use --help or --help-extra""" + ); + } + + @Test + public void testOptionNotFound_DocletOption() { + var result = new JavadocTask(tb, Task.Mode.CMDLINE) + .options("-tiglet") + .run(Task.Expect.FAIL) + .writeAll(); + checkFound(String.join("\n", result.getOutputLines(Task.OutputKind.DIRECT)), + """ + error: invalid flag: -tiglet + Did you mean: -taglet + For more details on available options, use --help or --help-extra""" + ); + } + private void checkFound(String log, String... expect) { for (String e : expect) { if (!log.contains(e)) {