Skip to content

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.

Sign up for GitHub

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 from
Nov 15, 2019
Merged

Issues/5530 use consistent naming #6054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf8b091
Refactored new Checkstyle
chrischild Oct 25, 2019
6d5e0cd
Merge branch 'master' into issues/5530-use-consistent-naming
chrischild Oct 25, 2019
b61db89
New Checkstyle
chrischild Nov 6, 2019
fd3042c
Merge branch 'master' into issues/5530-use-consistent-naming
chrischild Nov 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions ...uality-reports/src/main/java/com/azure/tools/checkstyle/checks/BlacklistedWordsCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// 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 com.puppycrawl.tools.checkstyle.checks.naming.AccessModifier;
import com.puppycrawl.tools.checkstyle.utils.CheckUtil;

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 AccessModifier accessModifier = CheckUtil.getAccessModifierFromModifiersToken(modifiersAST);
final boolean isStatic = modifiersAST.findFirstToken(TokenTypes.LITERAL_STATIC) != null;
return (accessModifier.equals(AccessModifier.PUBLIC) || accessModifier.equals(AccessModifier.PROTECTED)) && !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) {
for (String blacklistedWord : blacklistedWords) {
if (tokenName.contains(blacklistedWord)) {
return true;
}
}
return false;
}
}
5 changes: 5 additions & 0 deletions eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,9 @@
This suppression need to be deleted after AzConfig using AAD credential.
Issue Link: https://github.com/Azure/azure-sdk-for-java/issues/5131 -->
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicyCheck" files="com.azure.data.appconfiguration.implementation.ConfigurationCredentialsPolicy.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=".*[/\\]samples[/\\].*" />
<suppress checks="com.azure.tools.checkstyle.checks.BlacklistedWordsCheck" files=".*[/\\]implementation[/\\].*" />
</suppressions>
10 changes: 10 additions & 0 deletions eng/code-quality-reports/src/main/resources/checkstyle/checkstyle.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,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>

<!-- CUSTOM CHECKS -->
<!-- Fluent method checks:
(1) A method returns an instance of the class, and that have one parameter,
Expand Down
65 changes: 65 additions & 0 deletions ...y-reports/src/test/java/com/azure/tools/checkstyle/checks/BlacklistedWordsCheckTests.java
View file
Open in desktop
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 ...es/com/azure/tools/checkstyle/checks/BlacklistedWordsChecks/BlacklistedWordsTestData.java
View file
Open in desktop
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."); }
}