Skip to content

Commit

Permalink
Fix some quality flaws
Browse files Browse the repository at this point in the history
  • Loading branch information
vilchik-elena committed Aug 24, 2015
1 parent 9259e2b commit f526a67
Show file tree
Hide file tree
Showing 18 changed files with 64 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,18 @@ private static AssignmentExpressionTree getInnerAssignmentExpression(ExpressionT
}

private static boolean isRelationalExpression(Tree tree) {
return tree.is(Kind.EQUAL_TO) ||
tree.is(Kind.STRICT_EQUAL_TO) ||
tree.is(Kind.NOT_EQUAL_TO) ||
tree.is(Kind.STRICT_NOT_EQUAL_TO) ||
tree.is(Kind.LESS_THAN) ||
tree.is(Kind.LESS_THAN_OR_EQUAL_TO) ||
tree.is(Kind.GREATER_THAN) ||
tree.is(Kind.GREATER_THAN_OR_EQUAL_TO) ||
tree.is(Kind.RELATIONAL_IN);
return tree.is(
Kind.EQUAL_TO,
Kind.STRICT_EQUAL_TO,
Kind.NOT_EQUAL_TO,
Kind.STRICT_NOT_EQUAL_TO,
Kind.LESS_THAN,
Kind.LESS_THAN_OR_EQUAL_TO,
Kind.GREATER_THAN,
Kind.GREATER_THAN_OR_EQUAL_TO,
Kind.RELATIONAL_IN
// TODO (Lena): Is Kind.INSTANCE_OF required here?
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private void visitPossibleException(@Nullable Tree tree){
}
}

private List<ExpressionTree> getAllSubExpressions(BinaryExpressionTree tree) {
private static List<ExpressionTree> getAllSubExpressions(BinaryExpressionTree tree) {
List<ExpressionTree> result = new LinkedList<>();
result.add(tree.rightOperand());
ExpressionTree currentExpression = tree.leftOperand();
Expand All @@ -99,7 +99,7 @@ private List<ExpressionTree> getAllSubExpressions(BinaryExpressionTree tree) {
return result;
}

private SyntaxToken getFirstComma(BinaryExpressionTree tree) {
private static SyntaxToken getFirstComma(BinaryExpressionTree tree) {
SyntaxToken result = tree.operator();
ExpressionTree currentExpression = tree.leftOperand();
while (currentExpression.is(Kind.COMMA_OPERATOR)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.plugins.javascript.api.tree.Tree.Kind;
import org.sonar.plugins.javascript.api.visitors.BaseTreeVisitor;
import org.sonar.plugins.javascript.api.tree.Tree;
import org.sonar.plugins.javascript.api.tree.declaration.BindingElementTree;
Expand All @@ -36,6 +37,8 @@
import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
import org.sonar.squidbridge.annotations.SqaleSubCharacteristic;

import javax.annotation.Nullable;

@Rule(
key = "S888",
name = "Relational operators should be used in \"for\" loop termination conditions",
Expand All @@ -46,6 +49,15 @@
@SqaleConstantRemediation("2min")
public class EqualInForLoopTerminationCheck extends BaseTreeVisitor {

private static final Kind[] INCREMENT_KINDS = {
Tree.Kind.POSTFIX_INCREMENT,
Tree.Kind.PREFIX_INCREMENT,
Tree.Kind.PLUS_ASSIGNMENT,
Tree.Kind.POSTFIX_DECREMENT,
Tree.Kind.PREFIX_DECREMENT,
Tree.Kind.MINUS_ASSIGNMENT
};

@Override
public void visitForStatement(ForStatementTree tree) {
ExpressionTree condition = tree.condition();
Expand All @@ -70,11 +82,11 @@ private static boolean isEquality(ExpressionTree condition) {
return condition.is(Tree.Kind.EQUAL_TO, Tree.Kind.NOT_EQUAL_TO);
}

private boolean isException(ForStatementTree forStatement) {
private static boolean isException(ForStatementTree forStatement) {
return isNullConditionException(forStatement) || isTrivialIteratorException(forStatement);
}

private boolean isTrivialIteratorException(ForStatementTree forStatement) {
private static boolean isTrivialIteratorException(ForStatementTree forStatement) {
// todo(Lena): SONARJS-383 consider usage of counter inside the loop. Do it with symbol table.
ExpressionTree condition = forStatement.condition();
if (condition != null && condition.is(Tree.Kind.NOT_EQUAL_TO)) {
Expand All @@ -87,7 +99,7 @@ private boolean isTrivialIteratorException(ForStatementTree forStatement) {
return false;
}

private boolean checkForTrivialIteratorException(Tree init, ExpressionTree condition, ExpressionTree update) {
private static boolean checkForTrivialIteratorException(Tree init, ExpressionTree condition, ExpressionTree update) {
int updateByOne = checkForUpdateByOne(update);
if (updateByOne != 0) {
Integer endValue = getValue(condition);
Expand All @@ -104,7 +116,8 @@ private static boolean isNullConditionException(ForStatementTree forStatement) {
return condition != null && condition.is(Tree.Kind.NOT_EQUAL_TO) && ((BinaryExpressionTree) condition).rightOperand().is(Tree.Kind.NULL_LITERAL);
}

private Integer getValue(Tree tree) {
@Nullable
private static Integer getValue(Tree tree) {
Integer result = null;
if (tree.is(Tree.Kind.NOT_EQUAL_TO)) {
result = getInteger(((BinaryExpressionTree) tree).rightOperand());
Expand Down Expand Up @@ -137,7 +150,7 @@ private static Integer getInteger(ExpressionTree expression) {
return null;
}

private int checkForUpdateByOne(ExpressionTree update) {
private static int checkForUpdateByOne(ExpressionTree update) {
if (update.is(Tree.Kind.POSTFIX_INCREMENT, Tree.Kind.PREFIX_INCREMENT) || (update.is(Tree.Kind.PLUS_ASSIGNMENT) && isUpdateOnOneWithAssign(update))) {
return +1;
}
Expand All @@ -147,16 +160,16 @@ private int checkForUpdateByOne(ExpressionTree update) {
return 0;
}

private boolean isUpdateIncDec(ExpressionTree update) {
private static boolean isUpdateIncDec(ExpressionTree update) {
boolean result = false;
if (update.is(Tree.Kind.COMMA_OPERATOR)) {
BinaryExpressionTree commaExpressions = (BinaryExpressionTree) update;
result = isUpdateIncDec(commaExpressions.leftOperand()) && isUpdateIncDec(commaExpressions.rightOperand());
} else if (update.is(Tree.Kind.POSTFIX_INCREMENT, Tree.Kind.PREFIX_INCREMENT) || update.is(Tree.Kind.PLUS_ASSIGNMENT)) {
result = true;
} else if (update.is(Tree.Kind.POSTFIX_DECREMENT, Tree.Kind.PREFIX_DECREMENT) || update.is(Tree.Kind.MINUS_ASSIGNMENT)) {

} else if (update.is(INCREMENT_KINDS)) {
result = true;
}

return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void visitBinaryExpression(BinaryExpressionTree tree) {
super.visitBinaryExpression(tree);
}

private boolean isExcluded(BinaryExpressionTree tree) {
private static boolean isExcluded(BinaryExpressionTree tree) {
return !isOneOntoOneShifting(tree) && !isPotentialNanComparison(tree);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public void visitCallExpression(CallExpressionTree tree) {
super.visitCallExpression(tree);
}

private LiteralTree getSelectorParameter(CallExpressionTree tree) {
private static LiteralTree getSelectorParameter(CallExpressionTree tree) {
SeparatedList<Tree> parameters = tree.arguments().parameters();
if (parameters.size() == 1 && parameters.get(0).is(Tree.Kind.STRING_LITERAL) && !isElementCreation((LiteralTree) parameters.get(0))) {
return (LiteralTree) parameters.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,31 +59,31 @@ public void visitBinaryExpression(BinaryExpressionTree tree) {
super.visitBinaryExpression(tree);
}

private ExpressionTree removeParenthesis(ExpressionTree expressionTree) {
private static ExpressionTree removeParenthesis(ExpressionTree expressionTree) {
if (expressionTree.is(Tree.Kind.PARENTHESISED_EXPRESSION)) {
return removeParenthesis(((ParenthesisedExpressionTree) expressionTree).expression());
}
return expressionTree;
}

private ExpressionTree getNonNullLiteralOperand(BinaryExpressionTree binaryExpressionTree) {
private static ExpressionTree getNonNullLiteralOperand(BinaryExpressionTree binaryExpressionTree) {
if (isNullOrUndefined(binaryExpressionTree.leftOperand())) {
return binaryExpressionTree.rightOperand();
}
return binaryExpressionTree.leftOperand();
}

private boolean isAndWithEqualToNull(BinaryExpressionTree tree) {
private static boolean isAndWithEqualToNull(BinaryExpressionTree tree) {
return tree.is(Tree.Kind.CONDITIONAL_AND)
&& isNullComparison(tree.leftOperand(), Tree.Kind.EQUAL_TO, Tree.Kind.STRICT_EQUAL_TO);
}

private boolean isOrWithNonEqualToNull(BinaryExpressionTree tree) {
private static boolean isOrWithNonEqualToNull(BinaryExpressionTree tree) {
return tree.is(Tree.Kind.CONDITIONAL_OR)
&& isNullComparison(tree.leftOperand(), Tree.Kind.NOT_EQUAL_TO, Tree.Kind.STRICT_NOT_EQUAL_TO);
}

private boolean isNullComparison(ExpressionTree expression, Tree.Kind kind1, Tree.Kind kind2) {
private static boolean isNullComparison(ExpressionTree expression, Tree.Kind kind1, Tree.Kind kind2) {
ExpressionTree tree = removeParenthesis(expression);
if (tree.is(kind1, kind2)) {
BinaryExpressionTree binaryExp = (BinaryExpressionTree) tree;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private void visitDefaults(CallExpressionTree tree) {
}
}

private boolean isBackboneSetMethod(DotMemberExpressionTree dotExpr) {
private static boolean isBackboneSetMethod(DotMemberExpressionTree dotExpr) {
return CheckUtils.asString(dotExpr.property()).equals(SET) && dotExpr.object().types().contains(Type.Kind.BACKBONE_MODEL_OBJECT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void visitCallExpression(CallExpressionTree tree) {
super.visitCallExpression(tree);
}

private String getMessage(CallExpressionTree tree, int parametersNumber, int argumentsNumber) {
private static String getMessage(CallExpressionTree tree, int parametersNumber, int argumentsNumber) {
String callee;
if (isParenthesisedFunctionExpr(tree.callee())){
callee = "This function";
Expand All @@ -82,7 +82,7 @@ private String getMessage(CallExpressionTree tree, int parametersNumber, int arg
return String.format(MESSAGE, callee, parametersNumber, argumentsNumber);
}

private boolean isParenthesisedFunctionExpr(ExpressionTree tree) {
private static boolean isParenthesisedFunctionExpr(ExpressionTree tree) {
return tree.is(Tree.Kind.PARENTHESISED_EXPRESSION) && ((ParenthesisedExpressionTree) tree).expression().is(Tree.Kind.FUNCTION_EXPRESSION);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void visitNode(Tree tree) {
}
}

private boolean isDateException(Tree tree, Type type) {
private static boolean isDateException(Tree tree, Type type) {
if (tree.is(Kind.UNARY_PLUS)) {
String exprString = CheckUtils.asString(((UnaryExpressionTree) tree).expression());
boolean isDateName = exprString.contains("Date") || exprString.contains("date");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private void raiseIssuesOnDeclarations(Symbol symbol, String message){
}
}

private boolean noUsages(Collection<Usage> usages) {
private static boolean noUsages(Collection<Usage> usages) {
return usages.isEmpty() || usagesAreInitializations(usages);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void visitCallExpression(CallExpressionTree tree) {
super.visitCallExpression(tree);
}

private boolean isOpenDatabase(ExpressionTree callee) {
private static boolean isOpenDatabase(ExpressionTree callee) {
return callee instanceof IdentifierTree && ((IdentifierTree) callee).name().equals(OPEN_DATABASE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void visitNode(Tree tree) {
* @return <b>true</b> if last statement of <b>tree</b> body is return statement.
*/
private static boolean isStatementWithLastReturn(Tree tree) {
Kind kind = (Kind) ((JavaScriptTree) tree).getKind();
Kind kind = ((JavaScriptTree) tree).getKind();
boolean result = false;

switch (kind) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.sonar.api.config.Settings;
import org.sonar.api.source.Symbolizable;
import org.sonar.javascript.tree.symbols.type.TypeVisitor;
import org.sonar.javascript.highlighter.SourceFileOffsets;
import org.sonar.plugins.javascript.api.symbols.Symbol;
import org.sonar.plugins.javascript.api.symbols.SymbolModel;
import org.sonar.plugins.javascript.api.tree.ScriptTree;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public void visitUnaryExpression(UnaryExpressionTree tree) {
}
}

private boolean isIncDec(UnaryExpressionTree tree) {
private static boolean isIncDec(UnaryExpressionTree tree) {
return tree.is(
Tree.Kind.PREFIX_INCREMENT,
Tree.Kind.PREFIX_DECREMENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public final void tokenize(SourceCode source, Tokens cpdTokens) {
cpdTokens.add(TokenEntry.getEOF());
}

private String getTokenImage(Token token) {
private static String getTokenImage(Token token) {
if (token.getType() == GenericTokenType.LITERAL) {
return GenericTokenType.LITERAL.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private Measure convertMeasure(Measure measure) {
return itMeasure;
}

private void checkDataIsNotNull(@Nullable String data) {
private static void checkDataIsNotNull(@Nullable String data) {
if (data == null) {
throw new IllegalStateException("Measure data is null but it shouldn't be");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
*/
package org.sonar.plugins.javascript.unittest.jstestdriver;

import java.io.File;
import java.util.Iterator;

import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -33,10 +30,13 @@
import org.sonar.api.config.Settings;
import org.sonar.api.resources.Project;
import org.sonar.api.resources.Resource;
import org.sonar.plugins.javascript.JavaScriptPlugin;
import org.sonar.plugins.javascript.JavaScriptLanguage;
import org.sonar.plugins.javascript.JavaScriptPlugin;
import org.sonar.plugins.javascript.unittest.surefireparser.AbstractSurefireParser;

import java.io.File;
import java.util.Iterator;

public class JsTestDriverSensor implements Sensor {

protected FileSystem fileSystem;
Expand Down Expand Up @@ -99,7 +99,7 @@ protected String getUnitTestFileName(String className) {
return fileName;
}

private String getUnitTestClassName(String classNameFromReport) {
private static String getUnitTestClassName(String classNameFromReport) {
return classNameFromReport.substring(classNameFromReport.indexOf('.') + 1);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@

public class UnitTestClassReport {

private long errors = 0L, failures = 0L, skipped = 0L, tests = 0L, durationMilliseconds = 0L;
private long errors = 0L;
private long failures = 0L;
private long skipped = 0L;
private long tests = 0L;
private long durationMilliseconds = 0L;

private List<UnitTestResult> results = null;

public UnitTestClassReport add(UnitTestClassReport other) {
Expand Down

0 comments on commit f526a67

Please sign in to comment.