Skip to content

Commit

Permalink
Add new check for nesting level
Browse files Browse the repository at this point in the history
Using a method inspired by Sonar-Java, check code for the depth of
nested blocks and raise issues accordingly.
  • Loading branch information
Zetten committed May 22, 2015
1 parent f4cdd45 commit 0c2d392
Show file tree
Hide file tree
Showing 4 changed files with 244 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Sonar C++ Plugin (Community)
* Copyright (C) 2011 Waleri Enns and CONTACT Software GmbH
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package org.sonar.cxx.checks;


import com.google.common.collect.Sets;
import com.sonar.sslr.api.AstNode;
import com.sonar.sslr.api.AstNodeType;
import com.sonar.sslr.api.Grammar;
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.cxx.parser.CxxGrammarImpl;
import org.sonar.squidbridge.annotations.ActivatedByDefault;
import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
import org.sonar.squidbridge.annotations.SqaleSubCharacteristic;
import org.sonar.squidbridge.annotations.Tags;
import org.sonar.squidbridge.checks.SquidCheck;

import java.util.List;
import java.util.Set;

@Rule(
key = "NestedStatements",
name = "Control flow statements \"if\", \"switch\", \"try\" and iterators should not be nested too deeply",
tags = { Tags.BRAIN_OVERLOAD },
priority = Priority.MAJOR
)
@ActivatedByDefault
@SqaleSubCharacteristic(RulesDefinition.SubCharacteristics.LOGIC_CHANGEABILITY)
@SqaleConstantRemediation("10min")
public class NestedStatementsCheck extends SquidCheck<Grammar> {

private static final AstNodeType[] CHECKED_TYPES = new AstNodeType[]{
CxxGrammarImpl.ifStatement,
CxxGrammarImpl.switchStatement,
CxxGrammarImpl.tryBlock,
CxxGrammarImpl.iterationStatement
};

private static final int DEFAULT_MAX = 3;

@RuleProperty(defaultValue = "" + DEFAULT_MAX,
description = "Maximum allowed control flow statement nesting depth.")
public int max = DEFAULT_MAX;

private Set<AstNode> checkedNodes = Sets.newHashSet();
private int nestingLevel = 0;

@Override
public void init() {
subscribeTo(CHECKED_TYPES);
}

@Override
public void visitNode(AstNode node) {
if (checkedNodes.contains(node)) {
return;
}

nestingLevel++;

List<AstNode> watchedDescendants = node.getDescendants(CHECKED_TYPES);

if (nestingLevel == max + 1) {
getContext().createLineViolation(
this,
"Refactor this code to not nest more than " + max + " if/switch/try/for/while/do statements.",
node);
checkedNodes.addAll(watchedDescendants);
} else {
for (AstNode descendant : watchedDescendants) {
visitNode(descendant);
}
}

checkedNodes.add(node);
nestingLevel--;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<p>
Nested <code>if</code>, <code>switch</code>, <code>try</code> and iterator (<code>for</code>, <code>while</code>,
<code>do</code>) statements are a key ingredient for making what's known as "Spaghetti code". Such code is hard to read,
refactor and therefore maintain.
</p>

<h2>Noncompliant Code Example</h2>

<p>The following code snippet illustrates this rule with the default threshold of 3.</p>

<pre>
class Abc {
public:
void noncompliant( ) {
if (true) {
try {
if (true) {
for (int i = 0; i < 1; i++) {
printf("The enclosing block is non-compliant.");
}
}
} catch (...) {
do {
if (true) {
printf("The enclosing block is non-compliant.");

if (true) {
printf("The enclosing block exceeds the limit, but issues are only reported on depth = 4.");
}
}
} while (true);
}
}
}
};
</pre>
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Sonar C++ Plugin (Community)
* Copyright (C) 2011 Waleri Enns and CONTACT Software GmbH
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package org.sonar.cxx.checks;

import org.junit.Rule;
import org.junit.Test;
import org.sonar.cxx.CxxAstScanner;
import org.sonar.squidbridge.api.SourceFile;
import org.sonar.squidbridge.checks.CheckMessagesVerifier;
import org.sonar.squidbridge.checks.CheckMessagesVerifierRule;

import java.io.File;

public class NestedStatementsCheckTest {

@Rule
public CheckMessagesVerifierRule checkMessagesVerifier = new CheckMessagesVerifierRule();

@Test
public void detected() {
NestedStatementsCheck check = new NestedStatementsCheck();
check.max = 5;
SourceFile file = CxxAstScanner.scanSingleFile(new File("src/test/resources/checks/NestedStatementsCheck.cc"), check);
checkMessagesVerifier.verify(file.getCheckMessages())
.next().atLine(35)
.next().atLine(45)
.next().atLine(49)
.noMore();
}

}
61 changes: 61 additions & 0 deletions cxx-checks/src/test/resources/checks/NestedStatementsCheck.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
class Abc {
public:

void compliant() {
if (true) {
if (true) {
if (true) {
if (true) {
printf("Four levels deep; compliant.");
}
}
}
}
for (int i = 0; i < 1; i++) {
while (true) {
if (true) {
try {
if (true) {
printf("Five levels deep; compliant.");
}
} catch (...) {
printf("Four levels deep; compliant.");
}
}
}
}
}

void noncompliant( ) {
if (true) {
try {
if (true) {
for (int i = 0; i < 1; i++) {
if (true) {
if (true) {
printf("Six levels deep; non-compliant.");
}
}
}
}
} catch (...) {
do {
if (true) {
try {
for (int i = 0; i < 1; i++) {
printf("Six levels deep; non-compliant.");
}
} catch (...) {
if (true) {
printf("Six levels deep; non-compliant.");
if (true) {
printf("Seven levels deep; non-compliant but issue is reported on outside block.");
}
}
}
}
} while (true);
}
}
}
};

0 comments on commit 0c2d392

Please sign in to comment.