Checkstyle - Javadoc - enforce formating for param, return and throw …

…literals usage (Azure#4994)

* Adding javaDoc formating rule

* Adding fixes for javadoc formatting

eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/JavaDocFormatting.java

@@ -0,0 +1,85 @@
+// Licensed under the MIT License.
+// Copyright (c) Microsoft Corporation. All rights reserved.
+
+package com.azure.tools.checkstyle.checks;
+
+import com.puppycrawl.tools.checkstyle.api.DetailNode;
+import com.puppycrawl.tools.checkstyle.api.JavadocTokenTypes;
+import com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheck;
+import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;
+
+/**
+ *  Description text should only have one space character after the parameter name or {@code @return} statement.
+ *  Text should not start on a new line or have any additional spacing or indentation.
+ */
+public class JavaDocFormatting extends AbstractJavadocCheck {
+
+    private static final String JAVA_DOC_RETURN = "javadoc return";
+    private static final String JAVA_DOC_PARAMETER = "javadoc parameter";
+    private static final String JAVA_DOC_THROW = "javadoc throw";
+
+    private static final String ERROR_DESCRIPTION_ON_NEW_LINE = "Description for %s must be on same the same line.";
+    private static final String ERROR_NO_DESCRIPTION = "Description is missing for %s. Consider adding a description.";
+    private static final String ERROR_NO_WS_AFTER_IDENT = "No white space after %s. Consider fixing format.";
+    private static final String ERROR_EXTRA_SPACE = "Only one white space is expected after %s. Consider removing extra spaces.";
+
+    @Override
+    public int[] getAcceptableJavadocTokens() {
+        return getRequiredJavadocTokens();
+    }
+
+    @Override
+    public int[] getRequiredJavadocTokens() {
+        return new int[] {
+            JavadocTokenTypes.PARAMETER_NAME,
+            JavadocTokenTypes.RETURN_LITERAL,
+            JavadocTokenTypes.THROWS_LITERAL,
+        };
+    }
+
+    @Override
+    public int[] getDefaultJavadocTokens() {
+        return getRequiredJavadocTokens();
+    }
+
+    @Override
+    public void visitJavadocToken(DetailNode javaDocTag) {
+        switch (javaDocTag.getType()) {
+            case JavadocTokenTypes.RETURN_LITERAL:
+                evaluateValidFormat(javaDocTag, JAVA_DOC_RETURN);
+            case JavadocTokenTypes.PARAMETER_NAME:
+                evaluateValidFormat(javaDocTag, JAVA_DOC_PARAMETER);
+                break;
+            case JavadocTokenTypes.THROWS_LITERAL:
+                // Evaluate what is the format after the CLASS_NAME of a @throw
+                DetailNode throwFormat = JavadocUtil.getNextSibling(javaDocTag, JavadocTokenTypes.CLASS_NAME);
+                evaluateValidFormat(throwFormat, JAVA_DOC_THROW);
+            default:
+                break;
+        }
+    }
+
+    /*
+     * Function receives a DetailNode as the start token and then validates what comes after that node.
+     * valid format is:
+     * - A single white space is expected after the node. (no NEW_LINE) or other token
+     *
+     */
+    private void evaluateValidFormat(DetailNode javaDocTag, String identifier) {
+        DetailNode nextNodeAfterParameterName = JavadocUtil.getNextSibling(javaDocTag);
+
+        if (nextNodeAfterParameterName.getType() == JavadocTokenTypes.NEWLINE) {
+            if (JavadocUtil.getNextSibling(nextNodeAfterParameterName, JavadocTokenTypes.DESCRIPTION) != null) {
+                // Description on next line or after some other lines/spaces/staff
+                log(javaDocTag.getLineNumber(), String.format(ERROR_DESCRIPTION_ON_NEW_LINE, identifier));
+            } else {
+                // No description for parameter name
+                log(javaDocTag.getLineNumber(), String.format(ERROR_NO_DESCRIPTION, identifier));
+            }
+        } else if (nextNodeAfterParameterName.getType() != JavadocTokenTypes.WS) {
+            log(javaDocTag.getLineNumber(), String.format(ERROR_NO_WS_AFTER_IDENT, identifier));
+        } else if (!nextNodeAfterParameterName.getText().equals(" ")) {
+            log(javaDocTag.getLineNumber(), String.format(ERROR_EXTRA_SPACE, identifier));
+        }
+    }
+}

eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml

@@ -101,12 +101,14 @@
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceInterfaceCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>
   <suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicyCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>
+  <suppress checks="com.azure.tools.checkstyle.checks.JavaDocFormatting" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>
 
   <!-- Custom checkstyle rules that don't apply to files under implementation package -->
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files=".*[/\\]implementation[/\\].*\.java"/>
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientInstantiationCheck" files=".*[/\\]implementation[/\\].*\.java"/>
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" files=".*[/\\]implementation[/\\].*\.java"/>
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceInterfaceCheck" files=".*[/\\]implementation[/\\].*\.java"/>
+  <suppress checks="com.azure.tools.checkstyle.checks.JavaDocFormatting" files=".*[/\\]implementation[/\\].*\.java"/>
 
   <!-- Custom checkstyle rules that don't apply to files under samples package -->
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files=".*[/\\]samples[/\\].*\.java"/>

eng/code-quality-reports/src/main/resources/checkstyle/checkstyle.xml

@@ -317,6 +317,11 @@ page at http://checkstyle.sourceforge.net/config.html -->
     1) All fields must be final -->
     <module name="com.azure.tools.checkstyle.checks.OnlyFinalFieldsForImmutableClassCheck"/>
 
+    <!-- CUSTOM CHECKS -->
+    <!-- Javadoc format: 'param / return / throws' descriptions text should only have one space character after the
+     parameter name or return -->
+    <module name="com.azure.tools.checkstyle.checks.JavaDocFormatting"/>
+
     <!--CUSTOM CHECKS-->
     <!-- Must use 'logger.logAndThrow' but not directly calling 'throw exception' -->
 <!--    <module name="com.azure.tools.checkstyle.checks.ThrownClientLoggerCheck"/>-->

sdk/core/azure-core-management/src/main/java/com/azure/core/management/implementation/AzureAsyncOperationPollStrategy.java

@@ -59,11 +59,11 @@ private static class AzureAsyncOperationPollStrategyData extends PollStrategyDat
          * Create a new AzureAsyncOperationPollStrategyData object that will poll the provided operation
          * resource URL.
          * @param operationResourceUrl The URL of the operation resource this pollStrategy will poll.
-         * @param originalResourceUrl  The URL of the resource that the long running operation is
+         * @param originalResourceUrl The URL of the resource that the long running operation is
          *                             operating on.
-         * @param locationUrl          The location uri received from service along with operationResourceUrl.
-         * @param initialHttpMethod    The http method used to initiate the long running operation
-         * @param delayInMilliseconds  The delay (in milliseconds) that the pollStrategy will use when
+         * @param locationUrl The location uri received from service along with operationResourceUrl.
+         * @param initialHttpMethod The http method used to initiate the long running operation
+         * @param delayInMilliseconds The delay (in milliseconds) that the pollStrategy will use when
          *                             polling.
          */
         AzureAsyncOperationPollStrategyData(RestProxy restProxy, SwaggerMethodParser methodParser, URL operationResourceUrl, URL originalResourceUrl, URL locationUrl, HttpMethod initialHttpMethod, long delayInMilliseconds) {

sdk/core/azure-core-management/src/test/java/com/azure/core/management/implementation/Value.java

@@ -6,7 +6,7 @@
 /**
  * A container for a generic type. Serves a similar purpose as pointers in C/C++. It's a workaround
  * for the fact that Java doesn't allow mutation of local variables in closure.
- * @param <T>
+ * @param <T> The type of the object stored within the {@link Value} instance
  */
 public class Value<T> {
     private T value;
@@ -19,7 +19,7 @@ public class Value<T> {
 
     /**
      * Create a new Value with the provided inner value.
-     * @param value
+     * @param value The type of the object stored within the {@link Value} instance
      */
     Value(T value) {
         set(value);

sdk/core/azure-core/src/main/java/com/azure/core/implementation/serializer/HttpResponseHeaderDecoder.java

@@ -75,7 +75,7 @@ static Mono<Object> decode(HttpResponse httpResponse, SerializerAdapter serializ
      * @param headers the REST API returned headers
      * @return instance of header entity type created based on provided {@headers}, if header entity model does
      * not exists then return null
-     * @throws IOException
+     * @throws IOException If an I/O error occurs
      */
     private static Object deserializeHeaders(HttpHeaders headers, SerializerAdapter serializer, HttpResponseDecodeData decodeData) throws IOException {
         final Type deserializedHeadersType = decodeData.headersType();

sdk/core/azure-core/src/main/java/com/azure/core/implementation/serializer/ItemPage.java

@@ -20,7 +20,7 @@
  *      "items": [{ serialized(T) }, ... ]
  * }
  * or any other cases where the property names of that type are swapped
- * @param <T>
+ * @param <T> The type of the object stored within the {@link ItemPage} instance
  */
 class ItemPage<T> implements Page<T> {
     @JsonAlias({"items", "value"})

sdk/core/azure-core/src/samples/java/com/azure/core/util/polling/PollResponseJavaDocCodeSnippets.java

@@ -11,7 +11,7 @@
 /**
  * Code snipper for PollResponse
  *
- * @param <T>
+ * @param <T> The type of the object stored within the {@link PollResponseJavaDocCodeSnippets} instance
  */
 
 

sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubAsyncProducer.java

@@ -252,7 +252,7 @@ public Mono<Void> send(Flux<EventData> events) {
      * Sends a set of events to the associated Event Hub using a batched approach. If the size of events exceed the
      * maximum size of a single batch, an exception will be triggered and the send will fail. By default, the message
      * size is the max amount allowed on the link.
-     * @param events  Events to send to the service.
+     * @param events Events to send to the service.
      * @param options The set of options to consider when sending this batch.
      *
      * @return A {@link Mono} that completes when all events are pushed to the service.

sdk/identity/azure-identity/src/main/java/com/azure/identity/credential/AadCredentialBuilderBase.java

@@ -13,7 +13,7 @@ public abstract class AadCredentialBuilderBase<T extends AadCredentialBuilderBas
     /**
      * Specifies the Azure Active Directory endpoint to acquire tokens.
      * @param authorityHost the Azure Active Directory endpoint
-     * @return {@link <T>} itself
+     * @return itself
      */
     @SuppressWarnings("unchecked")
     public T authorityHost(String authorityHost) {
@@ -24,7 +24,7 @@ public T authorityHost(String authorityHost) {
     /**
      * Sets the client ID of the application.
      * @param clientId the client ID of the application.
-     * @return {@link <T>} itself
+     * @return itself
      */
     @SuppressWarnings("unchecked")
     public T clientId(String clientId) {

sdk/identity/azure-identity/src/main/java/com/azure/identity/credential/CredentialBuilderBase.java

@@ -22,7 +22,7 @@ public abstract class CredentialBuilderBase<T extends CredentialBuilderBase<T>>
     /**
      * Specifies the max number of retries when an authentication request fails.
      * @param maxRetry the number of retries
-     * @return {@link <T>} itself
+     * @return itself
      */
     @SuppressWarnings("unchecked")
     public T maxRetry(int maxRetry) {
@@ -33,7 +33,7 @@ public T maxRetry(int maxRetry) {
     /**
      * Specifies a Function to calculate seconds of timeout on every retried request.
      * @param retryTimeout the Function that returns a timeout in seconds given the number of retry
-     * @return {@link <T>} itself
+     * @return itself
      */
     @SuppressWarnings("unchecked")
     public T retryTimeout(Function<Integer, Integer> retryTimeout) {
@@ -44,7 +44,7 @@ public T retryTimeout(Function<Integer, Integer> retryTimeout) {
     /**
      * Specifies he options for proxy configuration.
      * @param proxyOptions the options for proxy configuration
-     * @return {@link <T>} itself
+     * @return itself
      */
     @SuppressWarnings("unchecked")
     public T proxyOptions(ProxyOptions proxyOptions) {

sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/MSIToken.java

@@ -32,7 +32,7 @@ public final class MSIToken extends AccessToken {
     /**
      * Creates an access token instance.
      *
-     * @param token     the token string.
+     * @param token the token string.
      * @param expiresOn the expiration time.
      */
     public MSIToken(String token, OffsetDateTime expiresOn) {

sdk/keyvault/azure-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyClient.java

@@ -503,7 +503,6 @@ public Response<DeletedKey> deleteKeyWithResponse(String name, Context context)
      * returned in the response.</p>
      * //Assuming key is deleted on a soft-delete enabled key vault.
      * {@codesnippet com.azure.keyvault.keys.keyclient.getDeletedKey#string}
-     * </pre>
      *
      * @param name The name of the deleted key.
      * @throws ResourceNotFoundException when a key with {@code name} doesn't exist in the key vault.

sdk/keyvault/azure-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/cryptography/AsymmetricEncryptionAlgorithm.java

@@ -17,7 +17,7 @@ abstract class AsymmetricEncryptionAlgorithm extends LocalEncryptionAlgorithm {
 
     /**
      * Constructor.
-     * 
+     *
      * @param name The name of the algorithm.
      */
     protected AsymmetricEncryptionAlgorithm(String name) {
@@ -29,10 +29,10 @@ protected AsymmetricEncryptionAlgorithm(String name) {
      * uses the specified {@link KeyPair} and the default {@link Provider} provider.
      *
      * @param keyPair The key pair to use.
-     * @return
-     * @throws InvalidKeyException
-     * @throws NoSuchAlgorithmException
-     * @throws NoSuchPaddingException
+     * @return abstract {@link ICryptoTransform}
+     * @throws InvalidKeyException when key is not valid
+     * @throws NoSuchAlgorithmException if algorithm is not found
+     * @throws NoSuchPaddingException if padding is set wrong
      */
     public abstract ICryptoTransform createEncryptor(KeyPair keyPair) throws InvalidKeyException, NoSuchAlgorithmException, NoSuchPaddingException;
 
@@ -42,10 +42,10 @@ protected AsymmetricEncryptionAlgorithm(String name) {
      *
      * @param keyPair The key pair to use.
      * @param provider The provider to use.
-     * @return
-     * @throws InvalidKeyException
-     * @throws NoSuchAlgorithmException
-     * @throws NoSuchPaddingException
+     * @return abstract {@link ICryptoTransform}
+     * @throws InvalidKeyException when key is not valid
+     * @throws NoSuchAlgorithmException if algorithm is not found
+     * @throws NoSuchPaddingException if padding is set wrong
      */
     public abstract ICryptoTransform createEncryptor(KeyPair keyPair, Provider provider) throws InvalidKeyException, NoSuchAlgorithmException, NoSuchPaddingException;
 
@@ -54,10 +54,10 @@ protected AsymmetricEncryptionAlgorithm(String name) {
      * uses the specified {@link KeyPair} and the default {@link Provider} provider.
      *
      * @param keyPair The key pair to use.
-     * @return
-     * @throws InvalidKeyException
-     * @throws NoSuchAlgorithmException
-     * @throws NoSuchPaddingException
+     * @return abstract {@link ICryptoTransform}
+     * @throws InvalidKeyException when key is not valid
+     * @throws NoSuchAlgorithmException if algorithm is not found
+     * @throws NoSuchPaddingException if padding is set wrong
      */
     public abstract ICryptoTransform createDecryptor(KeyPair keyPair) throws InvalidKeyException, NoSuchAlgorithmException, NoSuchPaddingException;
 
@@ -67,10 +67,10 @@ protected AsymmetricEncryptionAlgorithm(String name) {
      *
      * @param keyPair The key pair to use.
      * @param provider The provider to use.
-     * @return
-     * @throws InvalidKeyException
-     * @throws NoSuchAlgorithmException
-     * @throws NoSuchPaddingException
+     * @return abstract {@link ICryptoTransform}
+     * @throws InvalidKeyException when key is not valid
+     * @throws NoSuchAlgorithmException if algorithm is not found
+     * @throws NoSuchPaddingException if padding is set wrong
      */
     public abstract ICryptoTransform createDecryptor(KeyPair keyPair, Provider provider) throws InvalidKeyException, NoSuchAlgorithmException, NoSuchPaddingException;
 }

sdk/keyvault/azure-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/cryptography/models/SignatureAlgorithm.java

@@ -30,8 +30,7 @@ public enum SignatureAlgorithm {
     /**
      * Creates a custom value for SignatureAlgorithm.
      *
-     * @param value
-     *            the custom value
+     * @param value the custom value
      */
     SignatureAlgorithm(String value) {
         this.value = value;

sdk/keyvault/azure-keyvault-keys/src/samples/java/com/azure/security/keyvault/keys/cryptography/CryptographyAsyncClientJavaDocCodeSnippets.java

@@ -228,7 +228,7 @@ public void signDataVerifyData() throws NoSuchAlgorithmException {
 
     /**
      * Generates a code sample for using {@link CryptographyAsyncClient#wrapKey(KeyWrapAlgorithm, byte[])} and
-     * {@link CryptographyAsyncClient#unwrapKey(KeyWrapAlgorithm, byte[])
+     * {@link CryptographyAsyncClient#unwrapKey(KeyWrapAlgorithm, byte[])}
      */
     public void wrapKeyUnwrapKey() {
         CryptographyAsyncClient cryptographyAsyncClient = createAsyncClient();

sdk/keyvault/azure-keyvault-keys/src/samples/java/com/azure/security/keyvault/keys/cryptography/CryptographyClientJavaDocCodeSnippets.java

@@ -220,7 +220,7 @@ public void signDataVerifyData() throws NoSuchAlgorithmException {
 
     /**
      * Generates a code sample for using {@link CryptographyClient#wrapKey(KeyWrapAlgorithm, byte[])} and
-     * {@link CryptographyClient#unwrapKey(KeyWrapAlgorithm, byte[])
+     * {@link CryptographyClient#unwrapKey(KeyWrapAlgorithm, byte[])}
      */
     public void wrapKeyUnwrapKey() {
         CryptographyClient cryptographyClient = createClient();