RFR [15] 8236435: Fix typos in javac area

Pavel Rappo pavel.rappo at oracle.com
Sat Dec 21 00:13:13 UTC 2019


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