Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[apex] added fix to not tag single block for two error statements #3641

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -4,8 +4,143 @@

package net.sourceforge.pmd.lang.apex.rule.errorprone;

import static org.junit.Assert.assertEquals;

import org.junit.Before;
import org.junit.Test;

import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Report;
import net.sourceforge.pmd.RuleSet;
import net.sourceforge.pmd.RuleSetFactory;
import net.sourceforge.pmd.RuleSetLoader;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.lang.apex.ApexLanguageModule;
import net.sourceforge.pmd.testframework.PmdRuleTst;

/**
* Test class to test if empty if statements are captured
* and no empty block statement error is thrown
*
*/
public class EmptyIfStmtTest extends PmdRuleTst {
// no additional unit tests
/**
* Assert error string, in case of failures
*/
private static final String MATCH_ERROR = "Expected Violation count do not match";
/**
* Rule set factory initialization
*/
private static RuleSetFactory factory = new RuleSetLoader().enableCompatibility(false).toFactory();
/**
* language variable to hold apex language configuration
*/
private static final LanguageVersion LANGUAGE_VERSION = LanguageRegistry
.getLanguage(ApexLanguageModule.NAME)
.getDefaultVersion();
/**
* boolean to control unit test run class path
*/
private static final boolean USE_CLASS_PATH = true;
/**
* variable to hold ruleset
*/
private static RuleSet ruleSet;
/**
* variable to hold report of violations
*/
private static Report rpt;
/**
* variable to hold error logs
*/
private static String errorLog;
/**empty if block created
* for testing if block error scenario
*/
private static final String TESTIF = "public class BlockWithEmptyIf {" + PMD.EOL + "public void methodEmpty() {"
+ PMD.EOL + "int i = 0;" + PMD.EOL
+ PMD.EOL + "if (i<5) {" + PMD.EOL + "}" + PMD.EOL + "}" + PMD.EOL + "}";
/**non-empty if block created
* for testing if block no error scenario
*/
private static final String TESTIFWITHDATA = "public class BlockWithNotEmptyIf {" + PMD.EOL + "public void methodNotEmpty() {"
+ PMD.EOL + "int i = 0;" + PMD.EOL
+ PMD.EOL + "if (i<5) {" + PMD.EOL
+ PMD.EOL + "i++;" + PMD.EOL
+ "}" + PMD.EOL + "}" + PMD.EOL + "}";
/**empty method block created
* for testing Block error scenario
*/
private static final String TESTBLOCK = "public class BlockEmpty {" + PMD.EOL + "public void methodEmpty() {"
+ PMD.EOL + "}" + PMD.EOL + "}";
/**non-empty method block created
* for testing no block error scenario
*/
private static final String TESTBLOCKWITHDATA = "public class BlockNotEmpty {" + PMD.EOL + "public void methodNotEmpty() {"
+ PMD.EOL + "int i = 0;" + PMD.EOL
+ PMD.EOL + "}" + PMD.EOL + "}";

/**
* method to initialize ruleset
* and report before every new test method execution
**/
@Before
public void initialize() {
initializeRuleSet();
rpt = new Report();
}

private static void initializeRuleSet() {
try {
ruleSet = factory.createRuleSet("rulesets/apex/empty.xml");
} catch (Exception exception) {
errorLog = errorLog + exception.getMessage();
}
}

/**
* Test method to test if error is reported if the
* incoming "if block" has no statements
**/
@Test
public void testEmptyRuleSetIfEmpty() {
runTestFromString(TESTIF, ruleSet, rpt,
LANGUAGE_VERSION, USE_CLASS_PATH);
assertEquals(MATCH_ERROR, 1, rpt.size());
}

/**
* Test method to test if no error is reported
* if the incoming "if block" has statements
**/
@Test
public void testEmptyRuleSetIfWithData() {
runTestFromString(TESTIFWITHDATA, ruleSet, rpt,
LANGUAGE_VERSION, USE_CLASS_PATH);
assertEquals(MATCH_ERROR, 0, rpt.size());
}

/**
* Test method to test if error is reported
* if the incoming block has no statements
* and no if or while or catch block
**/
@Test
public void testEmptyRuleSetBlockEmpty() {
runTestFromString(TESTBLOCK, ruleSet, rpt,
LANGUAGE_VERSION, USE_CLASS_PATH);
assertEquals(MATCH_ERROR, 1, rpt.size());
}

/**
* Test method to test if no errors are reported
* if the incoming block has statements
**/
@Test
public void testEmptyRuleSetBlockNotEmpty() {
runTestFromString(TESTBLOCKWITHDATA, ruleSet, rpt,
LANGUAGE_VERSION, USE_CLASS_PATH);
assertEquals(MATCH_ERROR, 0, rpt.size());
}
}
87 changes: 82 additions & 5 deletions pmd-core/src/main/java/net/sourceforge/pmd/Report.java
Expand Up @@ -52,6 +52,22 @@ public class Report implements Iterable<RuleViolation> {
private long start;
private long end;
private final List<SuppressedViolation> suppressedRuleViolations = new ArrayList<>();
/**
* Variable for empty If rule
**/
private static final String EMPTY_IF_RULE = "EmptyIfStmt";
/**
* Variable for empty If rule
**/
private static final String EMPTY_WHILE_RULE = "EmptyWhileStmt";
/**
* Variable for empty If rule
**/
private static final String EMPTY_CATCH_RULE = "EmptyCatchBlock";
/**
* Variable for empty If rule
**/
private static final String EMPTY_BLOCK_RULE = "EmptyStatementBlock";

/**
* Creates a new, initialized, empty report for the given file name.
Expand Down Expand Up @@ -346,13 +362,74 @@ public void addRuleViolation(RuleViolation violation) {
suppressedRuleViolations.add(new SuppressedViolation(violation, false, null));
return;
}
if (!isDuplicateViolation(violation)) {
int index = Collections.binarySearch(violations, violation, RuleViolation.DEFAULT_COMPARATOR);
violations.add(index < 0 ? -index - 1 : index, violation);
violationTree.addRuleViolation(violation);
for (ThreadSafeReportListener listener : listeners) {
listener.ruleViolationAdded(violation);
}
}
}

int index = Collections.binarySearch(violations, violation, RuleViolation.DEFAULT_COMPARATOR);
violations.add(index < 0 ? -index - 1 : index, violation);
violationTree.addRuleViolation(violation);
for (ThreadSafeReportListener listener : listeners) {
listener.ruleViolationAdded(violation);
private boolean isDuplicateViolation(final RuleViolation violation) {
boolean isDuplicateBlockType = false;
for (final RuleViolation existingViolation : violations) {
if (isCheckDuplicate(existingViolation, violation)) {
isDuplicateBlockType = true;
break;
}
}
return isDuplicateBlockType;
}

private boolean isCheckDuplicate(final RuleViolation existingViolation, final RuleViolation violation) {
boolean isDuplicateViolation = false;
if (isSameCodeBlockViolation(existingViolation, violation)) {
if (isEmptyBock(existingViolation)
&& isEmptyIfWhileCatch(violation)) {
violations.remove(existingViolation);
isDuplicateViolation = false;
} else if (isEmptyIfWhileCatch(existingViolation)
&& isEmptyBock(violation)) {
isDuplicateViolation = true;
}
}
return isDuplicateViolation;
}

private boolean isSameCodeBlockViolation(final RuleViolation existingViolation, final RuleViolation violation) {
return existingViolation != null
&& violation != null
&& existingViolation.getBeginLine() == violation.getBeginLine()
&& existingViolation.getClassName() != null && existingViolation.getClassName().equals(violation.getClassName())
&& existingViolation.getMethodName() != null && existingViolation.getMethodName().equals(violation.getMethodName())
&& existingViolation.getFilename() != null && existingViolation.getFilename().equals(violation.getFilename())
&& existingViolation.getPackageName() != null && existingViolation.getPackageName().equals(violation.getPackageName());
}

private boolean isEmptyIfWhileCatch(final RuleViolation violation) {
boolean isMatch = false;
if (isCheckValidViolation(violation)) {
String currentRuleName = violation.getRule().getName();
if (EMPTY_IF_RULE.equals(currentRuleName)
|| EMPTY_WHILE_RULE.equals(currentRuleName)
|| EMPTY_CATCH_RULE.equals(currentRuleName)) {
isMatch = true;
}
}
return isMatch;
}

private boolean isCheckValidViolation(final RuleViolation violation) {
return violation != null
&& violation.getRule() != null
&& violation.getRule().getName() != null;
}

private boolean isEmptyBock(final RuleViolation violation) {
return isCheckValidViolation(violation)
&& EMPTY_BLOCK_RULE.equals(violation.getRule().getName());
}

/**
Expand Down
Expand Up @@ -267,6 +267,12 @@ public void runTestFromString(String code, Rule rule, Report report, LanguageVer

public void runTestFromString(String code, Rule rule, Report report, LanguageVersion languageVersion,
boolean isUseAuxClasspath) {
RuleSet rules = RuleSet.forSingleRule(rule);
runTestFromString(code, rules, report, languageVersion, isUseAuxClasspath);
}

public void runTestFromString(String code, RuleSet rules, Report report, LanguageVersion languageVersion,
boolean isUseAuxClasspath) {
try {
PMD p = new PMD();
p.getConfiguration().setDefaultLanguageVersion(languageVersion);
Expand Down Expand Up @@ -294,7 +300,7 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
ctx.setSourceCodeFile(new File("n/a"));
ctx.setLanguageVersion(languageVersion);
ctx.setIgnoreExceptions(false);
RuleSet rules = RuleSet.forSingleRule(rule);
//RuleSet rules = RuleSet.forSingleRule(rule);
p.getSourceCodeProcessor().processSourceCode(new StringReader(code), new RuleSets(rules), ctx);
} catch (Exception e) {
throw new RuntimeException(e);
Expand Down