Skip to content

Commit

Permalink
tech-debt: Refactor of the rules using @RuleProperties (#99)
Browse files Browse the repository at this point in the history
  • Loading branch information
komasztania authored Jul 10, 2023
1 parent 949b2e6 commit bed420f
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,19 @@
import de.friday.sonarqube.gosu.antlr.GosuParser;
import de.friday.sonarqube.gosu.language.utils.GosuUtil;
import de.friday.sonarqube.gosu.plugin.GosuFileProperties;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import de.friday.sonarqube.gosu.plugin.issues.GosuIssue;
import de.friday.sonarqube.gosu.plugin.issues.SecondaryIssue;
import de.friday.sonarqube.gosu.plugin.measures.metrics.BaseMetric;
import de.friday.sonarqube.gosu.plugin.measures.metrics.CognitiveComplexityMetric;
import java.util.ArrayList;
import java.util.List;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;

import java.util.ArrayList;
import java.util.List;

@Rule(key = CognitiveComplexityRule.KEY)
public class CognitiveComplexityRule extends BaseGosuRule {
static final String KEY = "CognitiveComplexityRule";
Expand All @@ -40,7 +41,7 @@ public class CognitiveComplexityRule extends BaseGosuRule {
key = "Threshold",
description = "The maximum authorized complexity.",
defaultValue = "" + DEFAULT_METHOD_THRESHOLD)
private int max = DEFAULT_METHOD_THRESHOLD;
private int methodThreshold = DEFAULT_METHOD_THRESHOLD;

private List<SecondaryIssue> secondaryIssuesList = new ArrayList<>();

Expand Down Expand Up @@ -144,11 +145,11 @@ public void exitProperty(GosuParser.PropertyContext ctx) {
}

private void addComplexityIssue(Token token, String message) {
if (metric.getMethodComplexity() > max) {
if (metric.getMethodComplexity() > methodThreshold) {
addIssue(new GosuIssue.GosuIssueBuilder(this)
.withSecondaryIssues(secondaryIssuesList)
.onToken(token)
.withGap(metric.getMethodComplexity() - max)
.withGap(metric.getMethodComplexity() - methodThreshold)
.withMessage(message)
.build());
}
Expand All @@ -157,7 +158,7 @@ private void addComplexityIssue(Token token, String message) {

private String buildMessage(String message) {
return "The Cognitive Complexity of this " + message + " is " +
metric.getMethodComplexity() + " which is greater than " + max + " authorized.";
metric.getMethodComplexity() + " which is greater than " + methodThreshold + " authorized.";
}

private void addSecondariesInsideExpression(GosuParser.ExpressionContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,20 @@
import de.friday.sonarqube.gosu.antlr.GosuParser;
import de.friday.sonarqube.gosu.language.utils.GosuUtil;
import de.friday.sonarqube.gosu.plugin.GosuFileProperties;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import de.friday.sonarqube.gosu.plugin.issues.GosuIssue;
import de.friday.sonarqube.gosu.plugin.issues.SecondaryIssue;
import de.friday.sonarqube.gosu.plugin.measures.metrics.BaseMetric;
import de.friday.sonarqube.gosu.plugin.measures.metrics.CyclomaticComplexityMetric;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

@Rule(key = CyclomaticComplexityRule.KEY)
public class CyclomaticComplexityRule extends BaseGosuRule {
static final String KEY = "CyclomaticComplexityRule";
Expand All @@ -42,7 +43,7 @@ public class CyclomaticComplexityRule extends BaseGosuRule {
key = "Threshold",
description = "The maximum authorized complexity.",
defaultValue = "" + DEFAULT_METHOD_THRESHOLD)
private int max = DEFAULT_METHOD_THRESHOLD;
private int methodThreshold = DEFAULT_METHOD_THRESHOLD;
private List<SecondaryIssue> secondaryIssuesList = new ArrayList<>();
private GosuFileProperties gosuFileProperties;
private CyclomaticComplexityMetric metric;
Expand Down Expand Up @@ -128,12 +129,12 @@ private void addComplexityIssue(Token token, String message) {
secondaryIssuesList.clear();
return;
}
if (metric.getMethodComplexity() > max) {
if (metric.getMethodComplexity() > methodThreshold) {
secondaryIssuesList.add(new SecondaryIssue(token, "+1"));
addIssue(new GosuIssue.GosuIssueBuilder(this)
.withSecondaryIssues(secondaryIssuesList)
.onToken(token)
.withGap(metric.getMethodComplexity() - max)
.withGap(metric.getMethodComplexity() - methodThreshold)
.withMessage(message)
.build());
secondaryIssuesList.clear();
Expand All @@ -142,7 +143,7 @@ private void addComplexityIssue(Token token, String message) {

private String buildMessage(String message) {
return "The Cyclomatic Complexity of this " + message + " is " +
metric.getMethodComplexity() + " which is greater than " + max + " authorized.";
metric.getMethodComplexity() + " which is greater than " + methodThreshold + " authorized.";
}

private void addSecondariesInsideExpression(GosuParser.ExpressionContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,14 @@
@Rule(key = LinesOfCodeRule.KEY)
public class LinesOfCodeRule extends BaseGosuRule {
static final String KEY = "LinesOfCodeRule";
private static final int DEFAULT_METHOD_THRESHOLD = 500;
private static final int DEFAULT_MAX_NUMBER_OF_LINES = 500;
private final GosuFileProperties gosuFileProperties;
@RuleProperty(
key = "Max",
description = "Maximum authorized lines in a file.",
defaultValue = "" + DEFAULT_METHOD_THRESHOLD
defaultValue = "" + DEFAULT_MAX_NUMBER_OF_LINES
)
private int max = DEFAULT_METHOD_THRESHOLD;

private final GosuFileProperties gosuFileProperties;
private int maxNumberOfLines = DEFAULT_MAX_NUMBER_OF_LINES;

@Inject
LinesOfCodeRule(GosuFileProperties gosuFileProperties) {
Expand All @@ -46,7 +45,7 @@ public class LinesOfCodeRule extends BaseGosuRule {
@Override
public void exitStart(GosuParser.StartContext ctx) {
final GosuFileLineData fileLineData = gosuFileProperties.getFileLineData();
if (fileLineData.isNumberOfLinesOfCodeGreaterThan(max)) {
if (fileLineData.isNumberOfLinesOfCodeGreaterThan(maxNumberOfLines)) {
addIssue(new GosuIssue.GosuIssueBuilder(this)
.withMessage(messageBuilder(fileLineData.getNumberOfLinesOfCode()))
.build());
Expand All @@ -55,7 +54,7 @@ public void exitStart(GosuParser.StartContext ctx) {

private String messageBuilder(int lines) {
return "This file has " + lines + " lines, which is greater than "
+ max + " authorized. Split it into smaller files.";
+ maxNumberOfLines + " authorized. Split it into smaller files.";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@
package de.friday.sonarqube.gosu.plugin.rules.smells;

import de.friday.sonarqube.gosu.antlr.GosuParser;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import de.friday.sonarqube.gosu.plugin.issues.GosuIssue;
import java.util.Arrays;
import java.util.List;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import org.apache.commons.lang3.StringUtils;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;

import java.util.Arrays;
import java.util.List;

@Rule(key = LoggerLibraryRule.KEY)
public class LoggerLibraryRule extends BaseGosuRule {
static final String KEY = "LoggerLibraryRule";
Expand All @@ -35,11 +36,12 @@ public class LoggerLibraryRule extends BaseGosuRule {
description = "Comma separated logger libraries approved in project.",
defaultValue = "" + DEFAULT_LOGGER)
private String approvedLoggers = DEFAULT_LOGGER;
private List<String> loggers = Arrays.asList(approvedLoggers.split(",", -1));

@Override
public void exitUsesStatement(GosuParser.UsesStatementContext ctx) {
if(ctx == null || ctx.namespace() == null) { return; }
if (ctx == null || ctx.namespace() == null) {
return;
}
String usesStatement = ctx.namespace().getText();
if (isWrongLogger(usesStatement)) {
addIssue(new GosuIssue.GosuIssueBuilder(this)
Expand All @@ -54,13 +56,17 @@ private boolean isWrongLogger(String usesStatement) {
&& !isLibraryInPermittedLoggers(usesStatement);
}

private boolean isLibraryInPermittedLoggers(String usesStatement){
for(String logger : loggers){
private List<String> loggerLibs() {
return Arrays.asList(approvedLoggers.split(","));
}

private boolean isLibraryInPermittedLoggers(String usesStatement) {
for (String logger : loggerLibs()) {
logger = logger.trim();
if(logger.endsWith("*")){
if (logger.endsWith("*")) {
logger = logger.substring(0, logger.length() - 1);
}
if(usesStatement.startsWith(logger)){
if (usesStatement.startsWith(logger)) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@
package de.friday.sonarqube.gosu.plugin.rules.smells;

import de.friday.sonarqube.gosu.antlr.GosuParser;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import de.friday.sonarqube.gosu.plugin.issues.GosuIssue;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;

import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;

@Rule(key = LoggerRule.KEY)
public class LoggerRule extends BaseGosuRule {
static final String KEY = "LoggerRule";
Expand All @@ -39,16 +40,16 @@ public class LoggerRule extends BaseGosuRule {
key = "format",
description = "Regular expression used to check the logger names against.",
defaultValue = "" + DEFAULT_LOGGER_REGEX)
private String regex = DEFAULT_LOGGER_REGEX;
private String loggerRegex = DEFAULT_LOGGER_REGEX;
private Pattern loggerPattern;
private String className;

@Override
public void enterStart(GosuParser.StartContext ctx) {
try {
loggerPattern = Pattern.compile(regex);
loggerPattern = Pattern.compile(loggerRegex);
} catch (PatternSyntaxException e) {
LOG.error("LoggerRule - wrong syntax of \"format\" property - " + regex, e);
LOG.error("LoggerRule - wrong syntax of \"format\" property - " + loggerRegex, e);
loggerPattern = MATCH_ALL_LOGGERS_PATTERN;
}
}
Expand Down Expand Up @@ -76,26 +77,26 @@ public void exitField(GosuParser.FieldContext ctx) {
verifyLogger(ctx, methodCallContext.arguments().argExpression(0));
}

private static boolean isMethodCall(GosuParser.MemberAccessContext ctx) {
private boolean isMethodCall(GosuParser.MemberAccessContext ctx) {
return ctx.expression(1) instanceof GosuParser.MethodCallContext;
}

private static boolean isNotLogger(GosuParser.MemberAccessContext memberAccessContext,
GosuParser.MethodCallContext methodCallContext) {
private boolean isNotLogger(GosuParser.MemberAccessContext memberAccessContext,
GosuParser.MethodCallContext methodCallContext) {
return !isLoggerFactory(memberAccessContext.expression(0))
|| !hasGetLoggerMethod(methodCallContext.expression())
|| hasNoArguments(methodCallContext);
}

private static boolean isLoggerFactory(GosuParser.ExpressionContext context) {
private boolean isLoggerFactory(GosuParser.ExpressionContext context) {
return context.getText().contains(LOGGER);
}

private static boolean hasGetLoggerMethod(GosuParser.ExpressionContext context) {
private boolean hasGetLoggerMethod(GosuParser.ExpressionContext context) {
return GET_LOGGER.equals(context.getText());
}

private static boolean hasNoArguments(GosuParser.MethodCallContext methodCallContext) {
private boolean hasNoArguments(GosuParser.MethodCallContext methodCallContext) {
return methodCallContext.arguments().argExpression().isEmpty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@
import de.friday.sonarqube.gosu.antlr.GosuLexer;
import de.friday.sonarqube.gosu.antlr.GosuParser;
import de.friday.sonarqube.gosu.plugin.GosuFileProperties;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import de.friday.sonarqube.gosu.plugin.issues.GosuIssue;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Pattern;
import de.friday.sonarqube.gosu.plugin.rules.BaseGosuRule;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.Token;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;

import java.util.Arrays;
import java.util.List;
import java.util.regex.Pattern;

@Rule(key = MagicNumbersRule.KEY)
public class MagicNumbersRule extends BaseGosuRule {
static final String KEY = "MagicNumbersRule";
Expand All @@ -42,15 +43,9 @@ public class MagicNumbersRule extends BaseGosuRule {
description = "Comma separated list of authorized numbers. Example: -1,0,1,2",
defaultValue = "" + DEFAULT_NUMBERS)
private String approvedNumbers = DEFAULT_NUMBERS;
private final List<String> numbers = Arrays.asList(approvedNumbers.split(",", -1));
private IN_HASHCODE hashCodeFlag = IN_HASHCODE.FALSE;
private GosuFileProperties gosuFileProperties;

private enum IN_HASHCODE {
TRUE,
FALSE
}

@Inject
MagicNumbersRule(GosuFileProperties gosuFileProperties) {
this.gosuFileProperties = gosuFileProperties;
Expand Down Expand Up @@ -113,8 +108,13 @@ public void exitNumberLiteral(GosuParser.NumberLiteralContext ctx) {
tryAddIssue(ctx.NumberLiteral().getSymbol());
}

private List<String> authorizedNumbers() {
return Arrays.asList(approvedNumbers.split(","));
}

private void tryAddIssue(Token token) {
String literal = removeSuffix(token.getText());
List<String> numbers = authorizedNumbers();

if (!numbers.contains(literal) && !numbers.contains(removeFloatingPoint(literal))) {
addIssue(new GosuIssue.GosuIssueBuilder(this)
Expand All @@ -124,7 +124,7 @@ private void tryAddIssue(Token token) {
}
}

private static String removeSuffix(String literal) {
private String removeSuffix(String literal) {

if (Character.isDigit(literal.charAt(literal.length() - 1))) {
return literal;
Expand All @@ -135,7 +135,7 @@ private static String removeSuffix(String literal) {
}
}

private static String removeFloatingPoint(String literal) {
private String removeFloatingPoint(String literal) {
if (FLOATING_POINT_PATTERN.matcher(literal).matches()) {
literal = literal.substring(0, literal.indexOf('.'));
return literal.length() == 0 ? "0" : literal;
Expand All @@ -147,4 +147,9 @@ private static String removeFloatingPoint(String literal) {
protected String getKey() {
return KEY;
}

private enum IN_HASHCODE {
TRUE,
FALSE
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package de.friday.sonarqube.gosu.plugin.rules.smells;

import org.junit.jupiter.api.Test;

import static de.friday.test.support.rules.dsl.gosu.GosuRuleTestDsl.given;

class LoggerLibraryRuleTest {
Expand All @@ -29,9 +30,17 @@ void findsNoIssueWhenStandardLoggerLibraryIsUsed() {
}

@Test
void findsNoIssueWhenLoggerLibraryImplementationIsUsedDirectly() {
void findsIssueWhenLoggerLibraryImplementationIsUsedDirectly() {
given("LoggerLibraryRule/nok.gs")
.whenCheckedAgainst(LoggerLibraryRule.class)
.then().issuesFound().hasSizeEqualTo(1);
}

@Test
void findsNoIssueWhenLoggerLibraryImplementationIsUsedDirectlyButPropertyIsSet() {
given("LoggerLibraryRule/nok.gs")
.whenCheckedAgainst(LoggerLibraryRule.class)
.withRuleProperty("libs", "org.apache")
.then().issuesFound().areEmpty();
}
}
Loading

0 comments on commit bed420f

Please sign in to comment.