[jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation

[Kevin Rushforth]

On Mon, 26 Jul 2021 13:55:45 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

> This PR corrects/adds missing documentation for classes in javafx.css package.

I applied the patch and confirm that there are no more "missing comments" warnings for the `javafx.css` package.

The docs look reasonable in general. I did note a few examples of usage patterns that should be fixed globally. Most of these fell into the following general categories.

1. The body of the javadoc comments for a class, method, etc., should be one or more complete sentences starting with a capital letter and ending with a period.
2. The text following `@return` and `@param` typically starts with a lower-case letter and does not end with a period.
3. When referring to a class name, method name, or formal parameter name, we usually use `{@code ... }` style or else separate words that are all lower-case. For example, either `{@code ParsedValue}` or `parsed value` is fine (but not `ParsedValue`).

In addition to noting one or two examples of each of the above, I left a few random inline comments.

A second pair of eyes would be good for these changes.

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4724:

> 4722: 
> 4723:     /**
> 4724:      * List of errors that may have occurred during css processing.

Maybe `Returns a list of errors...`? Also, CSS should probably be all-caps for consistency.

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4725:

> 4723:     /**
> 4724:      * List of errors that may have occurred during css processing.
> 4725:      * @return An {@code ObservableList} of {@code ParseError}

We typically start the text following `@return` or `@param` with a lower-case letter. So: `an ...`

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4739:

> 4737:     public static class ParseError {
> 4738: 
> 4739:         /**

Can you also add a one-sentence description, something like: `Returns the error message.`?

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4740:

> 4738: 
> 4739:         /**
> 4740:          * @return The error message from the CSS code.

We typically start with a lower can letter and omit the period at the end of `@return` or `@param`. Also, I'm not sure that "from the CSS code" is needed. So maybe:


    * @return the error message

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4748:

> 4746:         /**
> 4747:          * Constructs a {@code ParseError} object with given message.
> 4748:          * @param message message

Maybe "the message"?

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4835:

> 4833:             /**
> 4834:              * Constructs a {@code PropertySetError} object.
> 4835:              * @param styleableProperty css meta data

css -> CSS

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 36:

> 34: 
> 35: /**
> 36:  * This class serves as a container of CSS-property and it's value.

`CSS-property` should not be hyphenated, and it should be _a_ CSS property. Also: `it's` --> `its`.

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 67:

> 65:     /**
> 66:      * Get the parsed value
> 67:      * @return ParsedValue

Either `@return the {@code ParsedValue}` or `@return the parsed value`

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 74:

> 72: 
> 73:     /**
> 74:      * Get the CSS property name

End with a period. And we usually would say "Gets" rather than "Get" here.

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 75:

> 73:     /**
> 74:      * Get the CSS property name
> 75:      * @return css-property

For consistency, I'd say: `@return the CSS property name`

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 108:

> 106:     }
> 107:     /**
> 108:      * One declaration is equal to another regardless of the {@code Rule} to which

This is a good second sentence. The first sentence should probably be copied from the `Object::equals` method:


Indicates whether some other object is "equal to" this one.


Since this is pre-existing, it would be fine to not do this as part of this bug.

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 140:

> 138:     /**
> 139:      * Returns the hash code of this {@code Declaration}
> 140:      */

Since there is nothing extra to say beyond what the superclass says, you can instead replace this with:


/** {@inheritDoc} */


If you do want to add an explicit javadoc comment here, you need an `@return` tag.

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 151:

> 149:     /**
> 150:      * Returns a String version of this {@code Declaration}
> 151:      */

Same comment as for `hashCode`. Probably best to use use:


/** {@inheritDoc} */

modules/javafx.graphics/src/main/java/javafx/css/PseudoClass.java line 85:

> 83:     /**
> 84:      * Gets the {@code PseudoClass} instance for a given pseudo class name.
> 85:      * Note : There is only one PseudoClass instance for a given pseudo class name.

Maybe put this in a new paragraph? Also, can you remove the space before the `:`?

modules/javafx.graphics/src/main/java/javafx/css/SizeUnits.java line 38:

> 36: 
> 37:     /**
> 38:      * Percentage

Maybe make this a complete sentence by saying something like `Represents a size as a percentage.` for this one? For the concrete units, something like `Represents a size in inches.` (for example).

modules/javafx.graphics/src/main/java/javafx/css/SizeUnits.java line 138:

> 136: 
> 137:     /**
> 138:      * EX

What does "EX" specify? Since it isn't an obvious unit type, it would be good to define it.

modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 28:

> 26: 
> 27: /**
> 28:  * A class that contains StyleClass information

Best to use `{@code StyleClass}`. Also needs to end with a period.

modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 45:

> 43:     /**
> 44:      * Returns the name of StyleClass.
> 45:      * @return the style-class

`StyleClass` --> `the {@code StyleClass)` or `the style-class`.

modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 60:

> 58: 
> 59:     /**
> 60:      * Returns the index of this StyleClass in styleClasses list.

Where is the `styleClasses` list defined? This may be beyond the scope of this fix, so a follow-on issue could be filed. At the least, you should add "the" before "styleClasses".

modules/javafx.graphics/src/main/java/javafx/css/converter/EffectConverter.java line 83:

> 81:     /**
> 82:      * Converter to convert DropShadow {@code Effect}
> 83:      * @since 9

Good catch on the `@since 9` tag. I'm a little surprised it doesn't inherit the `@since` from the enclosing class if not present in the nested class, but I can see that it doesn't. So we'll to check the docs of all nested classes to make sure that there aren't any missing.

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

PR: https://git.openjdk.java.net/jfx/pull/589