Skip to content
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

Issue #374: Fix SingleBreakOrContinue violations #423

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public class CustomDeclarationOrderCheck extends Check
*/
private static final Comparator<DetailAST> AST_LINE_COMPARATOR = new Comparator<DetailAST>()
{

public int compare(DetailAST aObj1, DetailAST aObj2)
{
return aObj1.getLineNo() - aObj2.getLineNo();
Expand Down Expand Up @@ -591,44 +591,28 @@ private int getPositionInOrderDeclaration(final DetailAST ast)
{
int result = -1;
final String modifiers = getCombinedModifiersList(ast);
for (int index = 0; index < customOrderDeclaration.size(); index++) {
for (int index = 0; index < customOrderDeclaration.size() && result != 1; index++) {
final FormatMatcher currentRule = customOrderDeclaration.get(index);
if (currentRule.getClassMember() == ast.getType()
&& currentRule.getRegexp().matcher(modifiers).find())
{
if (currentRule.hasRule(ANNON_CLASS_FIELD_MACRO)) {
if (isAnonymousClassField(ast)) {
result = index;
break;
}
}
else if (currentRule.hasRule(GETTER_SETTER_MACRO)) {
if (currentRule.hasRule(ANNON_CLASS_FIELD_MACRO)
|| currentRule.hasRule(GETTER_SETTER_MACRO)
|| currentRule.hasRule(MAIN_METHOD_MACRO)) {

final String methodName = getIdentifier(ast);
final ClassDetail classDetail = classDetails.peek();
if (classDetail.containsGetter(methodName)
|| classDetail.containsSetter(methodName))
{
result = index;
break;
}
}
else if (currentRule.hasRule(MAIN_METHOD_MACRO)) {
if (isMainMethod(ast)) {

if (isAnonymousClassField(ast)
|| classDetail.containsGetter(methodName)
|| classDetail.containsSetter(methodName)
|| isMainMethod(ast)) {
result = index;
break;
}
}
else {
// if more than one rule matches current AST node, then keep first one
result = (result == -1) ? index : result;
if (ast.getType() == TokenTypes.METHOD_DEF
|| ast.getType() == TokenTypes.VARIABLE_DEF)
{
// continue to find more specific rule
continue;
} else {
break;
}
result = result == -1 ? index : result;
}
}
}
Expand Down Expand Up @@ -902,22 +886,22 @@ private static boolean isFieldUpdate(DetailAST statementsAst, String fieldName)
DetailAST currentStatement = statementsAst.getFirstChild();

while (currentStatement != null && currentStatement != statementsAst) {

String nameOfSetterField = null;
if (currentStatement.getType() == TokenTypes.ASSIGN) {
nameOfSetterField = getNameOfAssignedField(currentStatement);
} else if (currentStatement.getType() == TokenTypes.METHOD_CALL) {
nameOfSetterField = getNameOfSuperClassUpdatedField(currentStatement);
}

if (fieldName.equalsIgnoreCase(nameOfSetterField)) {
result = true;
break;
}

DetailAST nextStatement = currentStatement.getFirstChild();

while ((currentStatement != null) && (nextStatement == null)) {
while (currentStatement != null && nextStatement == null) {
nextStatement = currentStatement.getNextSibling();
if (nextStatement == null) {
currentStatement = currentStatement.getParent();
Expand All @@ -941,20 +925,20 @@ private static String getNameOfAssignedField(DetailAST assignAst)
{
String nameOfSettingField = null;

if (assignAst.getChildCount() > 0
if (assignAst.getChildCount() > 0
&& (assignAst.getLastChild().getType() == TokenTypes.IDENT
|| assignAst.getLastChild().getType() == TokenTypes.METHOD_CALL)) {

final DetailAST methodCallDot = assignAst.getFirstChild();
if (methodCallDot.getChildCount() == 2
&& "this".equals(methodCallDot.getFirstChild().getText())) {
if (methodCallDot.getChildCount() == 2
&& "this".equals(methodCallDot.getFirstChild().getText())) {
nameOfSettingField = methodCallDot.getLastChild().getText();
}
}

return nameOfSettingField;
}

/**
* <p>
* Return name of the field of a super class, that was assigned in setter.
Expand All @@ -965,13 +949,13 @@ private static String getNameOfAssignedField(DetailAST assignAst)
*/
private static String getNameOfSuperClassUpdatedField(DetailAST methodCallAst) {
String nameOfSettingField = null;

final DetailAST methodCallDot = methodCallAst.getFirstChild();
if (methodCallDot.getChildCount() == 2
if (methodCallDot.getChildCount() == 2
&& "super".equals(methodCallDot.getFirstChild().getText())) {
nameOfSettingField = getFieldName(methodCallDot);
}

return nameOfSettingField;
}

Expand Down Expand Up @@ -1374,7 +1358,7 @@ else if (isBooleanGetterName(getterName)) {
}
// if fields are same and setter is sibling with getter
if (getterField.equals(setterField)
&& j != (i + 1))
&& j != i + 1)
{
result.put(setterAst, getterAst);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void visitToken(DetailAST variableDefNode) {
DetailAST typeArgs =
getFirstChildTokenOfType(newNode, TokenTypes.TYPE_ARGUMENTS);

if (typeArgs != null && varDefArguments.equalsTree(typeArgs)) {
if (varDefArguments.equalsTree(typeArgs)) {
log(typeArgs, MSG_KEY);
}
}
Expand All @@ -89,23 +89,17 @@ public void visitToken(DetailAST variableDefNode) {
* AST subtree to process.
*/
private static DetailAST getFirstChildTokenOfType(DetailAST rootToken, int tokenType) {

DetailAST resultNode = null;
DetailAST currentNode = rootToken.getFirstChild();
while (currentNode != null) {
if (currentNode.getType() == tokenType) {
resultNode = currentNode;
break;
}
DetailAST childNode = getFirstChildTokenOfType(currentNode, tokenType);
DetailAST resultNode = rootToken.getFirstChild();

if (resultNode != null
&& resultNode.getType() != tokenType) {
DetailAST childNode = getFirstChildTokenOfType(resultNode, tokenType);

if (childNode == null) {
currentNode = currentNode.getNextSibling();
}
else {
resultNode = childNode;
break;
resultNode = resultNode.getNextSibling();
}
}

return resultNode;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,22 +555,17 @@ private static boolean containsExceptionParameter(
{
boolean result = false;
DetailAST parameterAst = parametersAst.getFirstChild();

while (parameterAst != null) {
if (exceptionVariableName.equals(getIdentifier(parameterAst)))
{
if (exceptionVariableName.equals(getIdentifier(parameterAst))
|| isInstanceMethodCall(exceptionVariableName,
parameterAst.getFirstChild())) {
result = true;
break;
parameterAst = null;
}
else {
final DetailAST methodCallAst = parameterAst.getFirstChild();
if (isInstanceMethodCall(exceptionVariableName,
methodCallAst))
{
result = true;
break;
}
parameterAst = parameterAst.getNextSibling();
}
parameterAst = parameterAst.getNextSibling();
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,57 @@ private boolean addedUsingForbiddenImport(final String className,
boolean result = false;

for (String importText : importsList) {
if (importText.endsWith("*")) {
final String importTextWithoutAsterisk =
importText.substring(0, importText.length() - 1);
if (forbiddenClassNameAndPath.equals(
importTextWithoutAsterisk + className))
{
result = true;
break;
}
}
else if (importText.equals(forbiddenClassNameAndPath)
&& importText.endsWith(className))
{
if (isWildcardForbiddenImported(importText, forbiddenClassNameAndPath, className)
|| isForbiddenImported(importText, forbiddenClassNameAndPath, className)) {
result = true;
break;
}
}

return result;
}

/**
* Tests if the class with given className is imported with the forbidden
* import and false otherwise.
*
* @param importText
* - String representation of imports from the processed class.
* @param className
* - the name of the class to check.
* @param forbiddenClassNameAndPath
* - full name&path of the given forbidden class.
* @return true if the class with given className is imported with the
* forbidden import and false otherwise.
*/
private boolean isWildcardForbiddenImported(String importText, String forbiddenClassNameAndPath,
String className) {
final String importTextWithoutAsterisk =
importText.substring(0, importText.length() - 1);

return importText.endsWith("*")
&& forbiddenClassNameAndPath.equals(importTextWithoutAsterisk + className);
}

/**
* Tests if the class with given className is imported with the forbidden
* import and false otherwise.
*
* @param importText
* - String representation of imports from the processed class.
* @param className
* - the name of the class to check.
* @param forbiddenClassNameAndPath
* - full name&path of the given forbidden class.
* @return true if the class with given className is imported with the
* forbidden import and false otherwise.
*/
private boolean isForbiddenImported(String importText, String forbiddenClassNameAndPath,
String className) {
return importText.equals(forbiddenClassNameAndPath)
&& importText.endsWith(className);
}

/**
* Gets the class name from full (dotted) classPath.
* @param classNameAndPath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,39 +378,31 @@ private boolean isOverridableMethodCall(final DetailAST methodCallAST)
visitedMethodCalls.add(methodCallAST);

final String methodName = getMethodName(methodCallAST);
final DetailAST methodDef = getMethodDef(methodCallAST);

if (methodName != null) {
final DetailAST methodDef = getMethodDef(methodCallAST);
if (methodDef != null) {

if (hasModifier(methodDef, TokenTypes.LITERAL_STATIC)) {
// do nothing
}
else if (hasModifier(methodDef, TokenTypes.LITERAL_PRIVATE)
|| hasModifier(methodDef, TokenTypes.FINAL))
{
final List<DetailAST> methodCallsList = getMethodCallsList(
methodDef);
for (DetailAST curNode : methodCallsList) {
if (visitedMethodCalls.contains(curNode)) {
result = false;
break;
}
else if (isOverridableMethodCall(curNode)) {
result = true;
break;
}
if (methodName != null
&& methodDef != null)
{
if (hasModifier(methodDef, TokenTypes.LITERAL_STATIC)) {
// do nothing
}
else if (hasModifier(methodDef, TokenTypes.LITERAL_PRIVATE)
|| hasModifier(methodDef, TokenTypes.FINAL))
{
final List<DetailAST> methodCallsList = getMethodCallsList(
methodDef);
for (DetailAST curNode : methodCallsList) {
if (!visitedMethodCalls.contains(curNode)
&& isOverridableMethodCall(curNode)) {
result = true;
break;
}
}
else {
curOverridableMetName = methodName;
result = true;
}
}
}
else
{
result = false;
else {
curOverridableMetName = methodName;
result = true;
}
}
return result;
}
Expand Down Expand Up @@ -890,7 +882,7 @@ private static DetailAST getClassDef(DetailAST rootNode, String className)

while (curNode != null) {
DetailAST toVisit = curNode.getFirstChild();
while ((curNode != null) && (toVisit == null)) {
while (curNode != null && toVisit == null) {
toVisit = curNode.getNextSibling();
if (toVisit == null) {
curNode = curNode.getParent();
Expand All @@ -899,11 +891,8 @@ private static DetailAST getClassDef(DetailAST rootNode, String className)

curNode = toVisit;

if (curNode == null) {
break;
}

if (curNode.getType() == TokenTypes.CLASS_DEF
if (curNode != null
&& curNode.getType() == TokenTypes.CLASS_DEF
&& curNode.findFirstToken(TokenTypes.IDENT).getText()
.equals(className))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public void doSmth() {
}
}
RuleViolation ruleViolation = new ParametricRuleViolation<Object>();
ParametricRuleViolation<? extends Object> parametricRule = new ParametricRuleViolation<String>(); // OK: no violation
}