RFR [15] 8236435: Fix typos in javac area
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Sun Dec 22 21:12:58 UTC 2019
Incremental patch looks good
Thanks
Maurizio
On 21/12/2019 00:13, Pavel Rappo wrote:
> Maurizio,
>
>> On 20 Dec 2019, at 21:35, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>
>> * ClientCodeWrapper: should "propagate upwards" be replaced with "propagated upwards" ? Or perhaps we should use consistent wording with the phrase below "... are left alone to continue propagating upwards."
> Would the following be more appropriate?
>
> * <li>Checked exceptions are left alone to propagate upwards in the
>
>> * ArgumentAttr: "behavior is influences by the currently selected cache policy." --> s/influences/influenced
> Fixed. (Jon also caught that one.)
>
>> * Check.java: "(except this in typesSkip)" -> s/this/those
> Fixed.
>
>> * LambdaToMethod: " (rather than i.e. accessing to captured members" - I think we can drop the "to"
> Fixed. Separately, I was wondering if "i.e." should've been "e.g." in the first
> place. It's a dangerous territory, where the semantics could be easily changed.
>
>> * Modules: "not the entire provides tree" - should "provides" be in a code block?
> Hm... I'm not sure I'm following, this is a simple code comment,
> not the javadoc comment.
>
>> * RichDiagnosticFormatter: "after these information is collected" --> s/these/this
> Thanks. I found that one too right after I have posted the RFR.
>
>
> Below is the incremental (inline) patch, addressing comments made by yourself
> and Jon, with a couple of extra fixes.
>
> Please note, that update also removes a part in DocumentationTool I was talking
> about, that unfortunately slipped through into the first RFR:
>
> diff --git a/src/java.compiler/share/classes/javax/tools/DocumentationTool.java b/src/java.compiler/share/classes/javax/tools/DocumentationTool.java
> --- a/src/java.compiler/share/classes/javax/tools/DocumentationTool.java
> +++ b/src/java.compiler/share/classes/javax/tools/DocumentationTool.java
> @@ -113,8 +113,9 @@
> Charset charset);
>
> /**
> - * Interface representing a future for a documentation task.
> - * To start the task, call the {@linkplain #call call} method.
> + * Interface representing a future for a documentation task. The
> + * task has not yet started. To start the task, call
> + * the {@linkplain #call call} method.
> *
> * <p>Before calling the {@code call} method, additional aspects of the
> * task can be configured, for example, by calling the
> diff --git a/src/java.compiler/share/classes/javax/tools/JavaCompiler.java b/src/java.compiler/share/classes/javax/tools/JavaCompiler.java
> --- a/src/java.compiler/share/classes/javax/tools/JavaCompiler.java
> +++ b/src/java.compiler/share/classes/javax/tools/JavaCompiler.java
> @@ -290,7 +290,7 @@
> * compilation task has not yet started. To start the task, call
> * the {@linkplain #call call} method.
> *
> - * <p>Before calling the call method, additional aspects of the
> + * <p>Before calling the {@code call} method, additional aspects of the
> * task can be configured, for example, by calling the
> * {@linkplain #setProcessors setProcessors} method.
> */
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/api/ClientCodeWrapper.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/api/ClientCodeWrapper.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/api/ClientCodeWrapper.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/api/ClientCodeWrapper.java
> @@ -73,7 +73,7 @@
> *
> * For each method, exceptions are handled as follows:
> * <ul>
> - * <li>Checked exceptions are left alone and propagate upwards in the
> + * <li>Checked exceptions are left alone to propagate upwards in the
> * obvious way, since they are an expected aspect of the method's
> * specification.
> * <li>Unchecked exceptions which have already been caught and wrapped in
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java
> @@ -112,7 +112,7 @@
> /** Result of method attribution. */
> Type result;
>
> - /** Cache for argument types; behavior is influences by the currently selected cache policy. */
> + /** Cache for argument types; behavior is influenced by the currently selected cache policy. */
> Map<UniquePos, ArgumentType<?>> argumentTypeCache = new LinkedHashMap<>();
>
> public static ArgumentAttr instance(Context context) {
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
> @@ -1952,7 +1952,7 @@
> * @param t1 The first type.
> * @param t2 The second type.
> * @param site The most derived type.
> - * @returns symbol from t2 that conflicts with one in t1.
> + * @return symbol from t2 that conflicts with one in t1.
> */
> private Symbol firstIncompatibility(DiagnosticPosition pos, Type t1, Type t2, Type site) {
> Map<TypeSymbol,Type> interfaces1 = new HashMap<>();
> @@ -1982,7 +1982,7 @@
> }
> }
>
> - /** Compute all the supertypes of t, indexed by type symbol (except this in typesSkip). */
> + /** Compute all the supertypes of t, indexed by type symbol (except those in typesSkip). */
> private void closure(Type t, Map<TypeSymbol,Type> typesSkip, Map<TypeSymbol,Type> typeMap) {
> if (!t.hasTag(CLASS)) return;
> if (typesSkip.get(t.tsym) != null) return;
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
> @@ -381,7 +381,7 @@
> //translate lambda body
> //As the lambda body is translated, all references to lambda locals,
> //captured variables, enclosing members are adjusted accordingly
> - //to refer to the static method parameters (rather than i.e. accessing to
> + //to refer to the static method parameters (rather than i.e. accessing
> //captured members directly).
> lambdaDecl.body = translate(makeLambdaBody(tree, lambdaDecl));
>
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/RichDiagnosticFormatter.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/RichDiagnosticFormatter.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/RichDiagnosticFormatter.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/RichDiagnosticFormatter.java
> @@ -52,7 +52,7 @@
> /**
> * A rich diagnostic formatter is a formatter that provides better integration
> * with javac's type system. A diagnostic is first preprocessed in order to keep
> - * track of each types/symbols in it; after these information is collected,
> + * track of each types/symbols in it; after this information is collected,
> * the diagnostic is rendered using a standard formatter, whose type/symbol printer
> * has been replaced by a more refined version provided by this rich formatter.
> * The rich formatter currently enables three different features: (i) simple class
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/sjavac/Source.java b/src/jdk.compiler/share/classes/com/sun/tools/sjavac/Source.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/sjavac/Source.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/sjavac/Source.java
> @@ -227,7 +227,7 @@
> existing = currentModule.lookupSource(file.toString());
> if (existing != null) {
>
> - // Oops, the source is already added, could be ok, could be not, lets check.
> + // Oops, the source is already added, could be ok, could be not, let's check.
> if (inLinksrc) {
> // So we are collecting sources for linking only.
> if (existing.isLinkedOnly()) {
>
>> On 20/12/2019 15:44, Pavel Rappo wrote:
>>> Hello,
>>>
>>> Please review the following change for https://bugs.openjdk.java.net/browse/JDK-8236435:
>>>
>>> http://cr.openjdk.java.net/~prappo/8236435/webrev.00/
>>>
>>> I've been reading up on the javadoc code and comments. At one point, jumping
>>> through the links, I reached the javax.tools.DocumentationTool and
>>> com.sun.tools.javac.util.ClientCodeException, which reside in java.compiler and
>>> jdk.compiler respectively. I found a couple of typos there and fixed them.
>>>
>>> One thing led to another and half an hour later I found myself compulsively
>>> checking the full javac code base for typos and spelling. Sigh.
>>>
>>> Long story short, I just went ahead and fixed as many issues as I could in one
>>> sitting. There was a lot.
>>>
>>> On the bright side, this patch doesn't involve any code changes (except for
>>> spelling in internal variables and methods' parameters), addresses aesthetic
>>> issues, readability, helps with searches and assists IDEs to pick up usages
>>> more reliably when refactoring (e.g. renaming).
>>>
>>> The initial concern with DocumentationTool turned out to be more subtle and I
>>> decided not to include it in this patch. It is definitely not just a typo and
>>> might require a CSR.
>>>
>>> Please review at your convenience. Hopefully, if there are false positives,
>>> you'll catch them.
>>>
>>> I suggest reviewing this change using a diff tool with a character-level
>>> resolution. That is, a tool capable of highlighting mismatching characters in
>>> lines that differ.
>>>
>>> All test/langtools tests pass. Copyright years will be fixed before the push.
>>>
>>> Thanks,
>>> -Pavel
>>>
More information about the compiler-dev
mailing list