-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Issues/5530 use consistent naming #6054
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
Merged
conniey
merged 4 commits into
Azure:master
from
chrischild:issues/5530-use-consistent-naming
Nov 15, 2019
Merged
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
108 changes: 108 additions & 0 deletions
108
...uality-reports/src/main/java/com/azure/tools/checkstyle/checks/BlacklistedWordsCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff 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 | ||
|
chrischild marked this conversation as resolved.
Outdated
|
||
| .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; | ||
|
chrischild marked this conversation as resolved.
Outdated
|
||
| break; | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -378,6 +378,16 @@ page at http://checkstyle.sourceforge.net/config.html --> | |
| 4) 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--> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will uncomment this once the naming issues have been fixed. Otherwise it'll break the build.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. :) |
||
|
|
||
| <!-- CUSTOM CHECKS --> | ||
| <!-- Fluent method checks: | ||
| (1) A method returns an instance of the class, and that have one parameter, | ||
|
|
||
65 changes: 65 additions & 0 deletions
65
...y-reports/src/test/java/com/azure/tools/checkstyle/checks/BlacklistedWordsCheckTests.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff 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; | ||
| } | ||
| } |
12 changes: 12 additions & 0 deletions
12
...es/com/azure/tools/checkstyle/checks/BlacklistedWordsChecks/BlacklistedWordsTestData.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff 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."); } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is it format better if keeping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I also saw the other note about the same issue on another method.