From bed420f921432a156f1ede5931e7b688aff50a87 Mon Sep 17 00:00:00 2001 From: Tomasz Kania <56966694+komasztania@users.noreply.github.com> Date: Mon, 10 Jul 2023 14:11:40 +0200 Subject: [PATCH] tech-debt: Refactor of the rules using @RuleProperties (#99) --- .../metrics/CognitiveComplexityRule.java | 15 +++++----- .../metrics/CyclomaticComplexityRule.java | 17 ++++++----- .../plugin/rules/metrics/LinesOfCodeRule.java | 13 ++++----- .../rules/smells/LoggerLibraryRule.java | 24 +++++++++------ .../gosu/plugin/rules/smells/LoggerRule.java | 25 ++++++++-------- .../plugin/rules/smells/MagicNumbersRule.java | 29 +++++++++++-------- .../rules/smells/LoggerLibraryRuleTest.java | 11 ++++++- .../rules/smells/MagicNumbersRuleTest.java | 14 +++++++-- 8 files changed, 90 insertions(+), 58 deletions(-) diff --git a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CognitiveComplexityRule.java b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CognitiveComplexityRule.java index 89fc7e01..0e9bbdb3 100644 --- a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CognitiveComplexityRule.java +++ b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CognitiveComplexityRule.java @@ -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"; @@ -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 secondaryIssuesList = new ArrayList<>(); @@ -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()); } @@ -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) { diff --git a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CyclomaticComplexityRule.java b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CyclomaticComplexityRule.java index 0b97a77c..6dc14d5a 100644 --- a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CyclomaticComplexityRule.java +++ b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/CyclomaticComplexityRule.java @@ -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"; @@ -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 secondaryIssuesList = new ArrayList<>(); private GosuFileProperties gosuFileProperties; private CyclomaticComplexityMetric metric; @@ -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(); @@ -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) { diff --git a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/LinesOfCodeRule.java b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/LinesOfCodeRule.java index e5ef4e49..1e6e5517 100644 --- a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/LinesOfCodeRule.java +++ b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/metrics/LinesOfCodeRule.java @@ -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) { @@ -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()); @@ -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 diff --git a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRule.java b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRule.java index 99065b32..31ae56da 100644 --- a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRule.java +++ b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRule.java @@ -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"; @@ -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 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) @@ -54,13 +56,17 @@ private boolean isWrongLogger(String usesStatement) { && !isLibraryInPermittedLoggers(usesStatement); } - private boolean isLibraryInPermittedLoggers(String usesStatement){ - for(String logger : loggers){ + private List 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; } } diff --git a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerRule.java b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerRule.java index 7b70c1cc..4f268bf2 100644 --- a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerRule.java +++ b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerRule.java @@ -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"; @@ -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; } } @@ -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(); } diff --git a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRule.java b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRule.java index c0e32f59..53556bbf 100644 --- a/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRule.java +++ b/src/main/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRule.java @@ -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"; @@ -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 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; @@ -113,8 +108,13 @@ public void exitNumberLiteral(GosuParser.NumberLiteralContext ctx) { tryAddIssue(ctx.NumberLiteral().getSymbol()); } + private List authorizedNumbers() { + return Arrays.asList(approvedNumbers.split(",")); + } + private void tryAddIssue(Token token) { String literal = removeSuffix(token.getText()); + List numbers = authorizedNumbers(); if (!numbers.contains(literal) && !numbers.contains(removeFloatingPoint(literal))) { addIssue(new GosuIssue.GosuIssueBuilder(this) @@ -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; @@ -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; @@ -147,4 +147,9 @@ private static String removeFloatingPoint(String literal) { protected String getKey() { return KEY; } + + private enum IN_HASHCODE { + TRUE, + FALSE + } } diff --git a/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRuleTest.java b/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRuleTest.java index e92895d1..a5c520a7 100644 --- a/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRuleTest.java +++ b/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/LoggerLibraryRuleTest.java @@ -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 { @@ -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(); + } } diff --git a/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRuleTest.java b/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRuleTest.java index 52cbcc48..1e633010 100644 --- a/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRuleTest.java +++ b/src/test/java/de/friday/sonarqube/gosu/plugin/rules/smells/MagicNumbersRuleTest.java @@ -17,8 +17,10 @@ package de.friday.sonarqube.gosu.plugin.rules.smells; import de.friday.test.support.rules.dsl.gosu.GosuIssueLocations; -import java.util.Arrays; import org.junit.jupiter.api.Test; + +import java.util.Arrays; + import static de.friday.test.support.rules.dsl.gosu.GosuRuleTestDsl.given; class MagicNumbersRuleTest { @@ -42,10 +44,18 @@ void findsIssuesWhenMagicNumbersAreFound() { Arrays.asList(7, 8, 7, 9), Arrays.asList(7, 12, 7, 13), Arrays.asList(11, 20, 11, 22), - Arrays.asList(11,15, 11, 17), + Arrays.asList(11, 15, 11, 17), Arrays.asList(12, 30, 12, 34), Arrays.asList(12, 24, 12, 27) ) ); } + + @Test + void findsIssuesWhenMagicNumbersAreFoundButPropertiesSet() { + given("MagicNumbersRule/nok.gs") + .whenCheckedAgainst(MagicNumbersRule.class) + .withRuleProperty("Authorized numbers", "2,3,12") + .then().issuesFound().hasSizeEqualTo(4); + } }