Issues/5530 use consistent naming #5698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

chrischild wants to merge 21 commits into Azure:master from chrischild:issues/5530-use-consistent-naming

...uality-reports/src/main/java/com/azure/tools/checkstyle/checks/BlacklistedWordsCheck.java

-Original file line number
+Diff line change
@@ -0,0 +1,108 @@
+    // 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.Arrays;
+    import java.util.Collections;
+    import java.util.HashSet;
+    import java.util.Set;
+    import java.util.stream.Collectors;
+    /**
+     * Ensure that code is not using words or abbreviations that are blacklisted by this Checkstyle.
+     * blacklistedWords: the words that have been blacklisted in the checkstyle.xml config file
+     *
+     * Prints out a message stating the location and the class, method or variable as well as the list
+     * blacklisted words.
+     */
+    public class BlacklistedWordsCheck extends AbstractCheck {
+        private final Set<String> blacklistedWords = new HashSet<>(Arrays.asList());
+        private final String ERROR_MESSAGE = "%s, All Public API Classes, Fields and Methods should follow " +
+            "Camelcase standards for the following words: %s.";
+        /**
+         * Adds words that Classes, Methods and Variables that should follow Camelcasing standards
+         * @param blacklistedWords words that should follow normal Camelcasing standards
+         */
+        public final void setBlacklistedWords(String... blacklistedWords) {
+            if (blacklistedWords != null) {
+                Collections.addAll(this.blacklistedWords, blacklistedWords);
+            }
+        }
+        @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,
+                TokenTypes.VARIABLE_DEF};
+        }
+        @Override
+        public void visitToken(DetailAST token) {
+            switch (token.getType()) {
+                case TokenTypes.CLASS_DEF:
+                case TokenTypes.METHOD_DEF:
+                case TokenTypes.VARIABLE_DEF:
+                    if (isPublicApi(token)) {
+                        String tokenName = token.findFirstToken(TokenTypes.IDENT).getText();
+                        if (hasBlacklistedWords(tokenName)) {
+                            log(token, String.format(ERROR_MESSAGE, tokenName, this.blacklistedWords.stream().collect(Collectors.joining(", ", "", ""))));
+                        }
+                    }
+                    break;
+                default:
+                    // Checkstyle complains if there's no default block in switch
+                    break;
+            }
+        }
+        /**
+         * Should we check member with given modifiers.
+         *
+         * @param token
+         *                modifiers of member to check.
+         * @return true if we should check such member.
+         */
+        private boolean isPublicApi(DetailAST token) {
+            final DetailAST modifiersAST =
+                token.findFirstToken(TokenTypes.MODIFIERS);
+            final boolean isStatic = modifiersAST.findFirstToken(TokenTypes.LITERAL_STATIC) != null;
+            final boolean isPublic = modifiersAST
+                .findFirstToken(TokenTypes.LITERAL_PUBLIC) != null;
+            final boolean isProtected = modifiersAST.findFirstToken(TokenTypes.LITERAL_PROTECTED) != null;
+            return (isPublic || isProtected) && !isStatic;
+        }
+        /**
+         * Gets the disallowed abbreviation contained in given String.
+         * @param tokenName
+         *        the given String.
+         * @return the disallowed abbreviation contained in given String as a
+         *         separate String.
+         */
+        private boolean hasBlacklistedWords(String tokenName) {
+            boolean result = false;
+            for (String blacklistedWord : blacklistedWords) {
+                if (tokenName.contains(blacklistedWord)) {
+                    result = true;
+                    break;
+                }
+            }
+            return result;
+        }
+    }

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

-Original file line number
+Diff line change
@@ Expand Up / @@ -206,4 +206,7 @@ @@
       <!-- Allows the SasTokenCredentialPolicy in Implementation -->
       <suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.storage.common.implementation.policy.SasTokenCredentialPolicy.java"/>
+      <!-- Only checks Public Api classes -->
+      <suppress checks="com.azure.tools.checkstyle.checks.BlacklistedWordsCheck" files="[/\\]test[/\\]" />
+      <suppress checks="com.azure.tools.checkstyle.checks.BlacklistedWordsCheck" files="[/\\]implementation[/\\]" />
     </suppressions>

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

-Original file line number
+Diff line change
@@ Expand Up / @@ -378,6 +378,16 @@ page at http://checkstyle.sourceforge.net/config.html --> @@
 ) All classes should use ClientLogger as logger only but except ClientLogger itself -->
         <module name="com.azure.tools.checkstyle.checks.GoodLoggingCheck"/>
+        <!-- CUSTOM CHECKS -->
+        <!-- Public API classes should follow Camelcase rules: -->
+        <!-- 1. ClassName -->
+        <!-- 2. methodName -->
+        <!-- 3. variableName -->
+        <!-- 4. longerVariableOrMethodName -->
+        <module name="com.azure.tools.checkstyle.checks.BlacklistedWordsCheck">
+          <property name="blacklistedWords" value="URL, HTTP, XML, JSON, SAS, CPK, API" />
+        </module>
         <!-- CUSTOM CHECKS -->
         <!-- Fluent method checks:
         (1) A method returns an instance of the class, and that have one parameter,
@@ Expand Down @@

...y-reports/src/test/java/com/azure/tools/checkstyle/checks/BlacklistedWordsCheckTests.java

-Original file line number
+Diff line change
@@ -0,0 +1,65 @@
+    // Copyright (c) Microsoft Corporation. All rights reserved.
+    // Licensed under the MIT License.
+    package com.azure.tools.checkstyle.checks;
+    import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
+    import com.puppycrawl.tools.checkstyle.Checker;
+    import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
+    import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
+    import org.junit.After;
+    import org.junit.Before;
+    import org.junit.Test;
+    public class BlacklistedWordsCheckTests extends AbstractModuleTestSupport {
+        private static final String BLACKLISTED_WORD_ERROR_MESSAGE = "%s, All Public API Classes, Fields and Methods should follow" +
+            " Camelcase standards for the following words: XML, HTTP, URL.";
+        private Checker checker;
+        @Before
+        public void prepare() throws Exception {
+            checker = prepareCheckStyleChecker();
+            checker.addListener(this.getBriefUtLogger());
+        }
+        @After
+        public void cleanup() {
+            checker.destroy();
+        }
+        @Override
+        protected String getPackageLocation() {
+            return "com/azure/tools/checkstyle/checks/BlacklistedWordsChecks";
+        }
+        @Test
+        public void blacklistedWordsTestData() throws Exception {
+            String[] expected = {
+                expectedErrorMessage(3, 5, String.format(BLACKLISTED_WORD_ERROR_MESSAGE, "errorHTTPMethod")),
+                expectedErrorMessage(9, 5, String.format(BLACKLISTED_WORD_ERROR_MESSAGE, "invalidXMLMethod"))
+            };
+            verify(checker, getPath("BlacklistedWordsTestData.java"), expected);
+        }
+        private String expectedErrorMessage(int line, int column, String errorMessage) {
+            return String.format("%d:%d: %s", line, column, errorMessage);
+        }
+        private Checker prepareCheckStyleChecker() throws CheckstyleException {
+            Checker checker = new Checker();
+            checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
+            checker.configure(prepareConfiguration());
+            return checker;
+        }
+        private DefaultConfiguration prepareConfiguration() {
+            DefaultConfiguration checks = new DefaultConfiguration("Checks");
+            DefaultConfiguration treeWalker = new DefaultConfiguration("TreeWalker");
+            DefaultConfiguration blacklistedWordsCheck = new DefaultConfiguration(BlacklistedWordsCheck.class.getCanonicalName());
+            blacklistedWordsCheck.addAttribute("blacklistedWords", "URL, HTTP, XML");
+            checks.addChild(treeWalker);
+            treeWalker.addChild(blacklistedWordsCheck);
+            return checks;
+        }
+    }

...es/com/azure/tools/checkstyle/checks/BlacklistedWordsChecks/BlacklistedWordsTestData.java

-Original file line number
+Diff line change
@@ -0,0 +1,12 @@
+    @JacksonXmlRootElement(localName = "File-SetHTTPHeaders-Headers")
+    public class CamelCaseTestData {
+        public void errorHTTPMethod() { throw new RuntimeException("Error Messages."); }
+        public void validHttpMethod() { throw new RuntimeException("Error Messages."); }
+        public static void itIsAURLError() { throw new RuntimeException("Error Messages."); }
+        protected void invalidXMLMethod() { throw new RuntimeException("Error Messages."); }
+        private void shouldNotSearch() { throw new RuntimeException("Error Messages."); }
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issues/5530 use consistent naming #5698

Uh oh!

Diff view

Diff view

There are no files selected for viewing