Skip to content

Commit

Permalink
#160 Added check to identify commented out code inline with other Son…
Browse files Browse the repository at this point in the history
…arQube plugins (S125).
  • Loading branch information
iwarapter committed Aug 13, 2015
1 parent ec4ccdf commit 23851e8
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public static List<Class> getChecks() {
CaseWithoutDefaultCheck.class,
ClassAndDefineNamingConventionCheck.class,
CommentConventionCheck.class,
CommentedOutCodeCheck.class,
CommentRegularExpressionCheck.class,
DocumentClassesAndDefinesCheck.class,
DeprecatedNodeInheritanceCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* SonarQube Puppet Plugin
* The MIT License (MIT)
*
* Copyright (c) 2015 Iain Adams and David RACODON
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
package com.iadams.sonarqube.puppet.checks;

import com.google.common.collect.ImmutableSet;
import com.iadams.sonarqube.puppet.PuppetCommentAnalyser;
import com.iadams.sonarqube.puppet.api.PuppetKeyword;
import com.sonar.sslr.api.AstAndTokenVisitor;
import com.sonar.sslr.api.Token;
import com.sonar.sslr.api.Trivia;
import java.util.Set;
import java.util.regex.Pattern;
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.squidbridge.annotations.ActivatedByDefault;
import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
import org.sonar.squidbridge.annotations.SqaleSubCharacteristic;
import org.sonar.squidbridge.checks.SquidCheck;
import org.sonar.squidbridge.recognizer.CodeRecognizer;
import org.sonar.squidbridge.recognizer.Detector;
import org.sonar.squidbridge.recognizer.EndWithDetector;
import org.sonar.squidbridge.recognizer.KeywordsDetector;
import org.sonar.squidbridge.recognizer.LanguageFootprint;
import org.sonar.sslr.parser.LexerlessGrammar;

@Rule(
key = "S125",
priority = Priority.MAJOR,
name = "Sections of code should not be \"commented out\"",
tags = Tags.UNUSED)
@SqaleSubCharacteristic(RulesDefinition.SubCharacteristics.UNDERSTANDABILITY)
@SqaleConstantRemediation("5min")
@ActivatedByDefault
public class CommentedOutCodeCheck extends SquidCheck<LexerlessGrammar> implements AstAndTokenVisitor {

private static final double THRESHOLD = 0.9;

private final CodeRecognizer codeRecognizer = new CodeRecognizer(THRESHOLD, new PuppetRecognizer());

private static class PuppetRecognizer implements LanguageFootprint {

@Override
public Set<Detector> getDetectors() {
return ImmutableSet.of(
new EndWithDetector(0.95, '}', ';', '{'),
new KeywordsDetector(0.3, PuppetKeyword.keywordValues()));
}
}

@Override
public void visitToken(Token token) {
Trivia previousTrivia = null;

for (Trivia trivia : token.getTrivia()) {
checkTrivia(previousTrivia, trivia);
previousTrivia = trivia;
}
}

private void checkTrivia(Trivia previousTrivia, Trivia trivia) {
if (isCommentedCode(getContext().getCommentAnalyser().getContents(trivia.getToken().getValue()))
&& !previousLineIsCommentedCode(trivia, previousTrivia)) {
reportIssue(trivia.getToken().getLine());
}
}

private void reportIssue(int line) {
getContext().createLineViolation(this, "Remove this commented out code.", line);
}

private boolean previousLineIsCommentedCode(Trivia trivia, Trivia previousTrivia) {
return previousTrivia != null && (trivia.getToken().getLine() == previousTrivia.getToken().getLine() + 1)
&& isCommentedCode(previousTrivia.getToken().getValue());
}

private boolean isCommentedCode(String line) {
return codeRecognizer.isLineOfCode(line);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class Tags {
public static final String SECURITY = "security";
public static final String PITFALL = "pitfall";
public static final String CONFUSING = "confusing";
public static final String UNUSED = "unused";

private Tags() {
// This class only defines constants
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<p>
Programmers should not comment out code as it bloats programs and reduces readability.
Unused code should be deleted and can be retrieved from source control history if required.
</p>
<h2>See</h2>
<ul>
<li>MISRA C:2004, 2.4 - Sections of code should not be "commented out".</li>
<li>MISRA C++:2008, 2-7-2 - Sections of code shall not be "commented out" using C-style comments.</li>
<li>MISRA C+:2008, 2-7-3 - Sections of code should not be "commented out" using C+ comments.</li>
<li>MISRA C:2012, Dir. 4.4 - Sections of code should not be "commented out"</li>
</ul>
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* SonarQube Puppet Plugin
* The MIT License (MIT)
*
* Copyright (c) 2015 Iain Adams and David RACODON
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
package com.iadams.sonarqube.puppet.checks

import com.iadams.sonarqube.puppet.PuppetAstScanner
import org.sonar.squidbridge.api.SourceFile
import org.sonar.squidbridge.checks.CheckMessagesVerifier
import spock.lang.Specification

class CommentedOutCodeCheckSpec extends Specification {

private static final String MESSAGE = "Remove this commented out code.";

def "validate rule"() {
given:
CommentedOutCodeCheck check = new CommentedOutCodeCheck();

SourceFile file = PuppetAstScanner.scanSingleFile(new File("src/test/resources/checks/commented_out_code.pp"), check);

expect:
CheckMessagesVerifier.verify(file.getCheckMessages())
.next().atLine(5).withMessage(MESSAGE)
.next().atLine(8).withMessage(MESSAGE)
.next().atLine(11).withMessage(MESSAGE)
.next().atLine(27).withMessage(MESSAGE)
.noMore();
}
}
27 changes: 27 additions & 0 deletions puppet-checks/src/test/resources/checks/commented_out_code.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# comment
if (1 == 1) {}

# Noncompliant
# if (1 == 1) {}

# Noncompliant
/* if (1 == 1) {} */

# Noncompliant
/*
file {'/tmp/junk':
owner => root,
}
*/

/* this is an example comment */

/*
Another comment across multilines
*/

# Noncompliant
# file { '/tmp/readme.txt':
# ensure => file,
# mode => '0644',
# }
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class PuppetCommentAnalyserSpec extends Specification {
where:
input | output | testFor
' ' | true | 'spaces'
' ' | true | 'tabs'
'\t' | true | 'tabs'
'words' | false | 'letters'
'12345' | false | 'numbers'
'12wor' | false | 'letters and numbers'
Expand Down

0 comments on commit 23851e8

Please sign in to comment.