Skip to content

Commit

Permalink
Detect complex conditions - #99
Browse files Browse the repository at this point in the history
  • Loading branch information
arturbosch committed Apr 11, 2017
1 parent 22b4fa8 commit 9d8d64e
Show file tree
Hide file tree
Showing 15 changed files with 238 additions and 6 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Current found smells are:
- StateChecking
- ShotgunSurgery
- NestedBlockDepth
- ComplexCondition

### Installation

Expand Down
8 changes: 6 additions & 2 deletions default-config.groovy
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//noinspection GroovyAssignabilityCheck
config {

input '/home/artur/Repos/quide'
input '/home/artur/Repos/quide-master/Implementierung/QuideService/src/main/java'
output '/home/artur/test/stuff.xml'

filters {
Expand All @@ -12,6 +12,10 @@ config {
detector('comment') { // loose comments or comments over private members -> naming should be meaningful -> no doc needed
let('active', 'false')
}
detector('complexcondition') { // complexity based on number of '&&' and '||' expressions
let('active', 'true')
let('threshold', '3')
}
detector('complexmethod') { // complexity based on McCabe
let('active', 'true')
let('threshold', '10')
Expand All @@ -25,7 +29,7 @@ config {
detector('deadcode') { // private unused members
let('active', 'true')
}
detector('featureenvy') { // uses the feature envy factor method by Kwankamol Nongpong (Link: 10.1109/KST.2015.7051460)
detector('featureenvy') { // uses the feature envy factor method by Kwankamol Nongpong -> http://ieeexplore.ieee.org/document/7051460/
let('active', 'true')
let('ignoreStatic', 'true') // static methods are often just plain utility methods which make extended use in parameters
let('threshold', '0.52') // the feature envy factor threshold, 0.52 is based on experience
Expand Down
3 changes: 3 additions & 0 deletions default-config.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
comment:
active: 'false'
complexcondition:
active: 'true'
threshold: '3'
complexmethod:
active: 'true'
threshold: '10'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import io.gitlab.arturbosch.smartsmells.metrics.ClassInfoDetector
import io.gitlab.arturbosch.smartsmells.smells.DetectionResult
import io.gitlab.arturbosch.smartsmells.smells.comment.CommentDetector
import io.gitlab.arturbosch.smartsmells.smells.comment.JavadocDetector
import io.gitlab.arturbosch.smartsmells.smells.complexcondition.ComplexConditionDetector
import io.gitlab.arturbosch.smartsmells.smells.complexmethod.ComplexMethodDetector
import io.gitlab.arturbosch.smartsmells.smells.cycle.CycleDetector
import io.gitlab.arturbosch.smartsmells.smells.dataclass.DataClassDetector
Expand Down Expand Up @@ -139,7 +140,7 @@ class DetectorFacade {
new LargeClassDetector(), new DataClassDetector(),
new CycleDetector(), new FeatureEnvyDetector(), new MiddleManDetector(),
new ShotgunSurgeryDetector(), new MessageChainDetector(), new GodClassDetector(),
new StateCheckingDetector(), new NestedBlockDepthDetector()]
new StateCheckingDetector(), new ComplexConditionDetector(), new NestedBlockDepthDetector()]
build()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Constants {

static final String BASE = "base"
static final String COMPLEX_METHOD = "complexmethod"
static final String COMPLEX_CONDITION = "complexcondition"

static final String CLASS_INFO = "metrics"
static final String SKIP_CC_CM = "skipCC_CM"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Defaults {
static int LONG_METHOD = 20
static int LONG_PARAMETER_LIST = 5
static int COMPLEX_METHOD = 10
static int WEIGHTED_METHOD_COUNT = 20
static int WEIGHTED_METHOD_COUNT = 20
static int ACCESS_TO_FOREIGN_DATA = 4
static double TIED_CLASS_COHESION = 0.33
static boolean ONLY_PRIVATE_DEAD_CODE = true
Expand All @@ -26,4 +26,5 @@ class Defaults {
static boolean ONLY_INTERFACES = false

static MiddleManVisitor.MMT MIDDLE_MAN_THRESHOLD = MiddleManVisitor.MMT.half
static int COMPLEX_CONDITION = 3
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.gitlab.arturbosch.smartsmells.common.Detector
import io.gitlab.arturbosch.smartsmells.metrics.ClassInfoDetector
import io.gitlab.arturbosch.smartsmells.smells.comment.CommentDetector
import io.gitlab.arturbosch.smartsmells.smells.comment.JavadocDetector
import io.gitlab.arturbosch.smartsmells.smells.complexcondition.ComplexConditionDetector
import io.gitlab.arturbosch.smartsmells.smells.complexmethod.ComplexMethodDetector
import io.gitlab.arturbosch.smartsmells.smells.cycle.CycleDetector
import io.gitlab.arturbosch.smartsmells.smells.dataclass.DataClassDetector
Expand Down Expand Up @@ -46,6 +47,13 @@ enum Smell {
Optional<Detector> initialize(DetectorConfig detectorConfig) {
return initDefault(detectorConfig, Constants.COMMENT, { new CommentDetector() })
}
}, COMPLEX_CONDITION{
@Override
Optional<Detector> initialize(DetectorConfig detectorConfig) {
return initDefault(detectorConfig, Constants.COMPLEX_CONDITION, {
new ComplexConditionDetector(toInt(it.get(Constants.THRESHOLD), Defaults.COMPLEX_CONDITION))
})
}
}, COMPLEX_METHOD{
@Override
Optional<Detector> initialize(DetectorConfig detectorConfig) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package io.gitlab.arturbosch.smartsmells.smells.complexcondition

import com.github.javaparser.ast.expr.Expression
import com.github.javaparser.ast.stmt.Statement
import groovy.transform.Immutable
import groovy.transform.ToString
import io.gitlab.arturbosch.jpal.ast.source.SourcePath
import io.gitlab.arturbosch.jpal.ast.source.SourceRange
import io.gitlab.arturbosch.smartsmells.smells.DetectionResult
import io.gitlab.arturbosch.smartsmells.smells.ElementTarget
import io.gitlab.arturbosch.smartsmells.smells.LocalSpecific

/**
* @author Artur Bosch
*/
@Immutable
@ToString(includePackage = false)
class ComplexCondition implements DetectionResult, LocalSpecific {

String inScope //ClassSignature#MethodSignature
String signature

@Delegate
SourcePath sourcePath
@Delegate
SourceRange sourceRange

ElementTarget elementTarget = ElementTarget.LOCAL

@Override
String name() {
return name
}

@Override
String signature() {
return signature
}

@Override
LocalSpecific copy(Statement statement) {
return null
}

@Override
LocalSpecific copy(Expression expression) {
return null
}

@Override
String asCompactString() {
return null
}

@Override
String asComparableString() {
return null
}

@Override
ElementTarget elementTarget() {
return elementTarget
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package io.gitlab.arturbosch.smartsmells.smells.complexcondition

import groovy.transform.CompileStatic
import io.gitlab.arturbosch.smartsmells.common.Detector
import io.gitlab.arturbosch.smartsmells.common.Visitor
import io.gitlab.arturbosch.smartsmells.config.Defaults
import io.gitlab.arturbosch.smartsmells.config.Smell

/**
* @author Artur Bosch
*/
@CompileStatic
class ComplexConditionDetector extends Detector<ComplexCondition> {

private int threshold

ComplexConditionDetector(int threshold = Defaults.COMPLEX_CONDITION) {
this.threshold = threshold
}

@Override
protected Visitor getVisitor() {
return new ComplexConditionVisitor(threshold)
}

@Override
Smell getType() {
return Smell.COMPLEX_CONDITION
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package io.gitlab.arturbosch.smartsmells.smells.complexcondition

import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration
import com.github.javaparser.ast.body.EnumDeclaration
import com.github.javaparser.ast.expr.Expression
import com.github.javaparser.ast.stmt.DoStmt
import com.github.javaparser.ast.stmt.IfStmt
import com.github.javaparser.ast.stmt.WhileStmt
import groovy.transform.CompileStatic
import io.gitlab.arturbosch.jpal.ast.ClassHelper
import io.gitlab.arturbosch.jpal.ast.EnumHelper
import io.gitlab.arturbosch.jpal.ast.NodeHelper
import io.gitlab.arturbosch.jpal.ast.source.SourcePath
import io.gitlab.arturbosch.jpal.ast.source.SourceRange
import io.gitlab.arturbosch.jpal.internal.Printer
import io.gitlab.arturbosch.jpal.resolution.Resolver
import io.gitlab.arturbosch.smartsmells.common.Visitor
import io.gitlab.arturbosch.smartsmells.smells.ElementTarget
import io.gitlab.arturbosch.smartsmells.smells.statechecking.StateCheckingVisitor
import io.gitlab.arturbosch.smartsmells.util.Strings

/**
* @author Artur Bosch
*/
@CompileStatic
class ComplexConditionVisitor extends Visitor<ComplexCondition> {

private String currentClassName = ""
private int threshold

ComplexConditionVisitor(int threshold) {
this.threshold = threshold
}

@Override
void visit(ClassOrInterfaceDeclaration n, Resolver arg) {
currentClassName = ClassHelper.createFullSignature(n)
super.visit(n, arg)
}

@Override
void visit(EnumDeclaration n, Resolver arg) {
currentClassName = EnumHelper.createFullSignature(n)
super.visit(n, arg)
}

@Override
void visit(IfStmt n, Resolver arg) {
checkCondition(n.condition)
super.visit(n, arg)
}

private void checkCondition(Expression expression) {
String condition = expression.toString(Printer.NO_COMMENTS)
int cases = Strings.amountOf(condition, "&&") + Strings.amountOf(condition, "||") + 1
if (cases > threshold) {
def methodName = NodeHelper.findDeclaringMethod(expression)
.map { it.declarationAsString }
.orElse(StateCheckingVisitor.UNKNOWN_METHOD)
def scope = currentClassName + "#" + methodName
smells.add(new ComplexCondition(scope, condition, SourcePath.of(info), SourceRange.fromNode(expression), ElementTarget.LOCAL))
}
}

@Override
void visit(DoStmt n, Resolver arg) {
checkCondition(n.condition)
super.visit(n, arg)
}

@Override
void visit(WhileStmt n, Resolver arg) {
checkCondition(n.condition)
super.visit(n, arg)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class StateChecking implements DetectionResult, LocalSpecific {
@Delegate
SourceRange sourceRange

ElementTarget elementTarget = ElementTarget.CLASS
ElementTarget elementTarget = ElementTarget.LOCAL

@Override
ElementTarget elementTarget() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.github.javaparser.ast.stmt.Statement
import com.github.javaparser.ast.stmt.SwitchStmt
import groovy.transform.CompileStatic
import io.gitlab.arturbosch.jpal.ast.ClassHelper
import io.gitlab.arturbosch.jpal.ast.EnumHelper
import io.gitlab.arturbosch.jpal.ast.NodeHelper
import io.gitlab.arturbosch.jpal.ast.source.SourcePath
import io.gitlab.arturbosch.jpal.ast.source.SourceRange
Expand Down Expand Up @@ -41,7 +42,7 @@ class StateCheckingVisitor extends Visitor<StateChecking> {

@Override
void visit(EnumDeclaration n, Resolver arg) {
currentClassName = n.nameAsString
currentClassName = EnumHelper.createFullSignature(n)
super.visit(n, arg)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Test {
static Path DATA_CLASS_DUMMY_PATH = BASE_PATH.resolve("DataClassDummy.java")
static Path EMPTY_DATA_CLASS_DUMMY_PATH = BASE_PATH.resolve("EmptyDataClassDummy.java")
static Path COMPLEX_METHOD_DUMMY_PATH = BASE_PATH.resolve("ComplexMethodDummy.java")
static Path COMPLEX_CONDITION_DUMMY_PATH = BASE_PATH.resolve("ComplexConditionDummy.java")
static Path LONG_METHOD_DUMMY_PATH = BASE_PATH.resolve("LongMethodDummy.java")
static Path GOD_CLASS_DUMMY_PATH = BASE_PATH.resolve("GodClassDummy.java")
static Path DEAD_CODE_PATH = BASE_PATH.resolve("DeadCodeDummy.java")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.gitlab.arturbosch.smartsmells.java;

/**
* @author Artur Bosch
*/
@SuppressWarnings("ALL")
public class ComplexConditionDummy {

public void method() {
if (5 > 4 && 4 < 6 || (3 < 5 || 2 < 5)) {
System.out.println();
}
while (5 > 4 && 4 < 6 || (3 < 5 || 2 < 5)) {
break;
}
do {
System.out.println();
} while (5 > 4 && 4 < 6 || (3 < 5 || 2 < 5));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.gitlab.arturbosch.smartsmells.smells.complexcondition

import io.gitlab.arturbosch.smartsmells.common.Test
import spock.lang.Specification

/**
* @author Artur Bosch
*/
class ComplexConditionDetectorTest extends Specification {

def "find one complex condition"() {
expect:
smells.size() == 3
smells[0].signature == "5 > 4 && 4 < 6 || (3 < 5 || 2 < 5)"

where:
smells = new ComplexConditionDetector().run(Test.COMPLEX_CONDITION_DUMMY_PATH)
}

}

0 comments on commit 9d8d64e

Please sign in to comment.