CheckStyle-Rule-Extension: 6, 7, 8

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

@@ -0,0 +1,121 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.tools.checkstyle.checks;
+
+import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+
+import java.util.Stack;
+
+/**
+ * The @ServiceClientBuilder class should have the following rules:
+ *  1) All service client builder should be named <ServiceName>ClientBuilder and annotated with @ServiceClientBuilder.
+ *  2) No other method have prefix 'build' other than 'buildClient' or 'buildAsyncClient'.
+ */
+public class ServiceClientBuilderCheck extends AbstractCheck {
+    private static final String SERVICE_CLIENT_BUILDER = "ServiceClientBuilder";
+    private static final String BUILD_CLIENT = "buildClient";
+    private static final String BUILD_ASYNC_CLIENT = "buildAsyncClient";
+
+    private Stack<Boolean> hasServiceClientBuilderAnnotationStack = new Stack();
+    private boolean hasServiceClientBuilderAnnotation;
+
+    @Override
+    public int[] getDefaultTokens() {
+        return getRequiredTokens();
+    }
+
+    @Override
+    public int[] getAcceptableTokens() {
+        return getRequiredTokens();
+    }
+
+    @Override
+    public int[] getRequiredTokens() {
+        return new int[] {
+            TokenTypes.CLASS_DEF,
+            TokenTypes.METHOD_DEF
+        };
+    }
+
+    @Override
+    public void leaveToken(DetailAST token) {
+        switch (token.getType()) {
+            case TokenTypes.CLASS_DEF:
+                // end of CLASS_DEF node, reset the value back to previous state
+                hasServiceClientBuilderAnnotation = hasServiceClientBuilderAnnotationStack.pop();
+                break;
+            default:
+                // Checkstyle complains if there's no default block in switch
+                break;
+        }
+    }
+
+    @Override
+    public void visitToken(DetailAST token) {
+        switch (token.getType()) {
+            case TokenTypes.CLASS_DEF:
+                // Save the state of variable 'hasServiceClientBuilderAnnotation' to limit the scope of accessibility
+                hasServiceClientBuilderAnnotationStack.push(hasServiceClientBuilderAnnotation);
+
+                final DetailAST serviceClientAnnotationBuilderToken = getServiceClientBuilderAnnotation(token);
+                final String className = token.findFirstToken(TokenTypes.IDENT).getText();
+
+                hasServiceClientBuilderAnnotation = serviceClientAnnotationBuilderToken != null;
+                if (hasServiceClientBuilderAnnotation) {
+                    // Don't need to check if the 'serviceClients' exist. It is required when using @ServiceClientBuilder
+
+                    // HAS @ServiceClientBuilder annotation but NOT named the class <ServiceName>ClientBuilder
+                    if (!className.endsWith("ClientBuilder")) {
+                        log(token, String.format("Class annotated with @ServiceClientBuilder ''%s'' should be named <ServiceName>ClientBuilder.", className));
+                    }
+                } else {
+                    // No @ServiceClientBuilder annotation but HAS named the class <ServiceName>ClientBuilder
+                    if (className.endsWith("ClientBuilder")) {
+                        log(token, String.format("Class ''%s'' should be annotated with @ServiceClientBuilder.", className));
+                    }
+                }
+                break;
+            case TokenTypes.METHOD_DEF:
+                if (!hasServiceClientBuilderAnnotation) {
+                    return;
+                }
+
+                final String methodName = token.findFirstToken(TokenTypes.IDENT).getText();
+                // method name has prefix 'build' but not 'buildClient' or 'buildAsyncClient'
+                if (methodName.startsWith("build") && !BUILD_ASYNC_CLIENT.equals(methodName) && !BUILD_CLIENT.equals(methodName)) {
+                    log(token, String.format(
+                        "@ServiceClientBuilder class should not have a method name, '''' starting with ''build'' " +
+                            "other than ''buildClient'' or ''buildAsyncClient''." , methodName));
+                }
+
+                break;
+            default:
+                // Checkstyle complains if there's no default block in switch
+                break;
+        }
+    }
+
+    /**
+     * Checks if the class is annotated with @ServiceClientBuilder.
+     *
+     * @param classDefToken the CLASS_DEF AST node
+     * @return the annotation node if the class is annotated with @ServiceClientBuilder, null otherwise.
+     */
+    private DetailAST getServiceClientBuilderAnnotation(DetailAST classDefToken) {
+        final DetailAST modifiersToken = classDefToken.findFirstToken(TokenTypes.MODIFIERS);
+
+        if (!modifiersToken.branchContains(TokenTypes.ANNOTATION)) {
+            return null;
+        }
+
+        DetailAST annotationToken = modifiersToken.findFirstToken(TokenTypes.ANNOTATION);
+        if (!SERVICE_CLIENT_BUILDER.equals(annotationToken.findFirstToken(TokenTypes.IDENT).getText())) {
+            return null;
+        }
+
+        return annotationToken;
+    }
+}

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

@@ -0,0 +1,119 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.tools.checkstyle.checks;
+
+import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+
+import java.util.regex.Pattern;
+
+/**
+ * The @ServiceInterface class should have the following rules:
+ *   1) The annotation property 'name' should be non-empty
+ *   2) The length of value of property 'name' should be less than 10 characters and without space
+ */
+public class ServiceInterfaceCheck extends AbstractCheck {
+    @Override
+    public int[] getDefaultTokens() {
+        return getRequiredTokens();
+    }
+
+    @Override
+    public int[] getAcceptableTokens() {
+        return getRequiredTokens();
+    }
+
+    @Override
+    public int[] getRequiredTokens() {
+        return new int[] {
+            TokenTypes.INTERFACE_DEF
+        };
+    }
+
+    @Override
+    public void visitToken(DetailAST token) {
+        switch (token.getType()) {
+            case TokenTypes.INTERFACE_DEF:
+                checkServiceInterface(token);
+                break;
+            default:
+                // Checkstyle complains if there's no default block in switch
+                break;
+        }
+    }
+
+    /**
+     *  The @ServiceInterface class should have the following rules:
+     *   1) The annotation property 'name' should be non-empty
+     *   2) The length of value of property 'name' should be less than 10 characters and without space
+     *
+     * @param interfaceDefToken INTERFACE_DEF AST node
+     */
+    private void checkServiceInterface(DetailAST interfaceDefToken) {
+        DetailAST serviceInterfaceAnnotationNode = null;
+        String nameValue = null;
+
+        DetailAST modifiersToken = interfaceDefToken.findFirstToken(TokenTypes.MODIFIERS);
+        // Find the @ServiceInterface and the property 'name' and the corresponding value
+        for (DetailAST ast = modifiersToken.getFirstChild(); ast != null; ast = ast.getNextSibling()) {
+            // We care about only the ANNOTATION type
+            if (ast.getType() != TokenTypes.ANNOTATION) {
+                continue;
+            }
+            // Skip if not @ServiceInterface annotation
+            if (!"ServiceInterface".equals(ast.findFirstToken(TokenTypes.IDENT).getText())) {
+                continue;
+            }
+
+            // Get the @ServiceInterface annotation node
+            serviceInterfaceAnnotationNode = ast;
+
+            // Get the 'name' property value of @ServiceInterface
+            // @ServiceInterface requires 'name' property
+            DetailAST annotationMemberValuePairToken = ast.findFirstToken(TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR);
+            if ("name".equals(annotationMemberValuePairToken.findFirstToken(TokenTypes.IDENT).getText())) {
+                nameValue = getNamePropertyValue(annotationMemberValuePairToken.findFirstToken(TokenTypes.EXPR));
+                break;
+            }
+        }
+
+        // Checks the rules:
+        // Skip the check if no @ServiceInterface annotation found
+        if (serviceInterfaceAnnotationNode == null) {
+            return;
+        }
+
+        // 'name' is required at @ServiceInterface
+        // 'name' should not be empty, no Space allowed and the length should less than or equal to 10 characters
+        Pattern serviceNamePattern = Pattern.compile("^[a-zA-Z0-9]{1,10}$");
+        if (!serviceNamePattern.matcher(nameValue).find()) {
+            log(serviceInterfaceAnnotationNode, String.format(
+                "The ''name'' property of @ServiceInterface, ''%s'' should be non-empty, alphanumeric and not more than 10 characters",
+                nameValue));
+        }
+    }
+
+    /**
+     * Get the name property value from the EXPR node
+     *
+     * @param exprToken EXPR
+     * @return null if EXPR node doesn't exist or no STRING_LITERAL. Otherwise, returns the value of the property.
+     */
+    private String getNamePropertyValue(DetailAST exprToken) {
+        if (exprToken == null) {
+            return null;
+        }
+
+        final DetailAST nameValueToken = exprToken.findFirstToken(TokenTypes.STRING_LITERAL);
+        if (nameValueToken == null) {
+            return null;
+        }
+
+        String nameValue = nameValueToken.getText();
+
+        // remove the beginning and ending double quote
+        return nameValue.replaceAll("^\"|\"$", "");
+    }
+}

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

@@ -89,8 +89,23 @@
   <!-- Custom checkstyle rules only check track 2 libraries -->
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files=".*[/\\]com[/\\]microsoft[/\\].*"/>
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientInstantiationCheck" files=".*[/\\]com[/\\]microsoft[/\\].*"/>
+  <suppress checks="com.azure.tools.checkstyle.checks.ThrownClientLoggerCheck" files=".*[/\\]com[/\\]microsoft[/\\].*"/>
 
   <!-- Custom checkstyle rules that don't apply to files under test or implementation package -->
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
   <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientInstantiationCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
+  <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
+  <suppress checks="com.azure.tools.checkstyle.checks.ServiceInterfaceCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
+
+  <!-- OpenCensus should only be depended on from within the tracing-opencensus module -->
+  <suppress checks="IllegalImport" files=".*[/\\]com[/\\]azure[/\\]tracing[/\\]opentelemetry[/\\]*"/>
+
+  <!-- Any code in any package, it should never be a 'throw' keyword in the client library codebase except for in the client logger -->
+<!--  <suppress checks="com.azure.tools.checkstyle.checks.ThrownClientLoggerCheck" files=".*[/\\]com[/\\]azure[/\\]core[/\\]util[/\\]logging[/\\]*"/>-->
+
+  <!-- Suppression for throws IOException Class, Will remove these classes after be able to define the error is IOException -->
+<!--  <suppress checks="com.azure.tools.checkstyle.checks.ThrownClientLoggerCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\]com[/\\]azure[/\\]core[/\\]implementation[/\\](.*RestProxyStressTests[/\\].*)|(.*util[/\\]FluxUtilTests[/\\].*)\.java"/>-->
+
+<!--  Storage still depends on 1.0.0-preview.2 azure core. suppress this rule for now until storage has upgraded to preview-3 azure core-->
+  <suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" files=".*[/\\]storage[/\\].*\.java"/>
 </suppressions>

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

@@ -49,7 +49,7 @@ page at http://checkstyle.sourceforge.net/config.html -->
     <module name="AvoidStarImport"/>
     <module name="IllegalImport">
       <property name="regexp" value="true"/>
-      <property name="illegalPkgs" value="(com\.)?sun\.\w*"/>
+      <property name="illegalPkgs" value="^(com\.)?sun\.\w*, ^io\.opencensus"/>
     </module>
     <module name="RedundantImport"/>
     <module name="UnusedImports"/>
@@ -290,5 +290,22 @@ page at http://checkstyle.sourceforge.net/config.html -->
     We should only return types, and accept argument types, that are from the com.azure namespace.
     All other types should have suppression added if required. -->
     <module name="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck"/>
+
+    <!--CUSTOM CHECKS-->
+    <!-- The @ServiceClientBuilder class should have the following rules:
+    1) All service client builder should be named <ServiceName>ClientBuilder and annotated with @ServiceClientBuilder.
+    2) Has a method 'buildClient()' to build a synchronous client,
+    3) Has a method 'buildAsyncClient()' to build an asynchronous client -->
+    <module name="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck"/>
+
+    <!--CUSTOM CHECKS-->
+    <!-- The @ServiceInterface class should have the following rules:
+    1) The annotation property 'name' should be non-empty
+    2) The length of value of property 'name' should be less than 10 characters and without space -->
+    <module name="com.azure.tools.checkstyle.checks.ServiceInterfaceCheck"/>
+
+    <!--CUSTOM CHECKS-->
+    <!-- Must use 'logger.logAndThrow' but not directly calling 'throw exception' -->
+<!--    <module name="com.azure.tools.checkstyle.checks.ThrownClientLoggerCheck"/>-->
   </module>
 </module>

sdk/keyvault/azure-keyvault-secrets/src/main/java/com/azure/security/keyvault/secrets/SecretService.java

@@ -38,7 +38,7 @@
  * This is package-private so that these REST calls are transparent to the user.
  */
 @Host("{url}")
-@ServiceInterface(name = "KeyVaultSecrets")
+@ServiceInterface(name = "KVSecrets")
 interface SecretService {
 
     @Put("secrets/{secret-name}")