Skip to content

@W-7742718@ Filter PMD results before processing violations #130

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.

Sign up for GitHub

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

Merged
jbartolotta-sfdc merged 1 commit into from
Jul 7, 2020
Merged

@W-7742718@ Filter PMD results before processing violations #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/lib/pmd/PmdEngine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,23 @@ export class PmdEngine implements RuleEngine {
}
}

private xmlToRuleResults(pmdXml: string): RuleResult[] {
protected xmlToRuleResults(pmdXml: string): RuleResult[] {
// If the results were just an empty string, we can return it.
if (pmdXml === '') {
this.logger.trace('No PMD results to convert');
return [];
}

const pmdJson = xml2js(pmdXml, {compact: false, ignoreDeclaration: true});
return pmdJson.elements[0].elements.map(

// Only provide results for nodes that are files. Other nodes are filtered out and logged.
const fileNodes = pmdJson.elements[0].elements.filter(e => 'file' === e.name);
const otherNodes = pmdJson.elements[0].elements.filter(e => 'file' !== e.name);
for (const otherNode of otherNodes) {
this.logger.trace(`Skipping non-file node ${JSON.stringify(otherNode)}`);
Copy link
Contributor Author

@jbartolotta-sfdc jbartolotta-sfdc Jul 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a line item to the 3.0 planning to decide if we want to somehow process these and show them to the user.

}

return fileNodes.map(
(f): RuleResult => {
return {
engine: PmdEngine.NAME,
Expand Down
48 changes: 48 additions & 0 deletions test/code-fixtures/pmd-results/result-with-configerror.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?xml version="1.0" encoding="UTF-8"?>
<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0 http://pmd.sourceforge.net/report_2_0_0.xsd"
version="6.22.0" timestamp="2020-07-07T09:18:15.741">
<file name="sfdx-scanner/pmd-cataloger/src/main/java/sfdc/sfdx/scanner/messaging/EventKey.java">
<violation beginline="2" endline="2" begincolumn="1" endcolumn="57" rule="UnusedImports" ruleset="Best Practices" package="sfdc.sfdx.scanner.messaging" class="EventKey" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_bestpractices.html#unusedimports" priority="4">
Avoid unused imports such as 'sfdc.sfdx.scanner.messaging.SfdxMessager'
</violation>
<violation beginline="4" endline="56" begincolumn="8" endcolumn="1" rule="CommentRequired" ruleset="Documentation" package="sfdc.sfdx.scanner.messaging" class="EventKey" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_documentation.html#commentrequired" priority="3">
Enum comments are required
</violation>
<violation beginline="5" endline="5" begincolumn="1" endcolumn="2" rule="CommentSize" ruleset="Documentation" package="sfdc.sfdx.scanner.messaging" class="EventKey" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_documentation.html#commentsize" priority="3">
Comment is too large: Line too long
</violation>
<violation beginline="23" endline="23" begincolumn="9" endcolumn="26" rule="CommentRequired" ruleset="Documentation" package="sfdc.sfdx.scanner.messaging" class="EventKey" variable="VariableDeclaratorId" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_documentation.html#commentrequired" priority="3">
Field comments are required
</violation>
<violation beginline="24" endline="24" begincolumn="9" endcolumn="21" rule="CommentRequired" ruleset="Documentation" package="sfdc.sfdx.scanner.messaging" class="EventKey" variable="VariableDeclaratorId" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_documentation.html#commentrequired" priority="3">
Field comments are required
</violation>
<violation beginline="25" endline="25" begincolumn="9" endcolumn="32" rule="CommentRequired" ruleset="Documentation" package="sfdc.sfdx.scanner.messaging" class="EventKey" variable="VariableDeclaratorId" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_documentation.html#commentrequired" priority="3">
Field comments are required
</violation>
<violation beginline="26" endline="26" begincolumn="9" endcolumn="38" rule="CommentRequired" ruleset="Documentation" package="sfdc.sfdx.scanner.messaging" class="EventKey" variable="VariableDeclaratorId" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_documentation.html#commentrequired" priority="3">
Field comments are required
</violation>
<violation beginline="27" endline="27" begincolumn="9" endcolumn="24" rule="CommentRequired" ruleset="Documentation" package="sfdc.sfdx.scanner.messaging" class="EventKey" variable="VariableDeclaratorId" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_documentation.html#commentrequired" priority="3">
Field comments are required
</violation>
<violation beginline="29" endline="29" begincolumn="37" endcolumn="48" rule="MethodArgumentCouldBeFinal" ruleset="Code Style" package="sfdc.sfdx.scanner.messaging" class="EventKey" method="EventKey" variable="argCount" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_codestyle.html#methodargumentcouldbefinal" priority="3">
Parameter 'argCount' is not assigned and could be declared final
</violation>
<violation beginline="29" endline="29" begincolumn="76" endcolumn="104" rule="MethodArgumentCouldBeFinal" ruleset="Code Style" package="sfdc.sfdx.scanner.messaging" class="EventKey" method="EventKey" variable="messageHandler" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_codestyle.html#methodargumentcouldbefinal" priority="3">
Parameter 'messageHandler' is not assigned and could be declared final
</violation>
<violation beginline="29" endline="29" begincolumn="18" endcolumn="34" rule="MethodArgumentCouldBeFinal" ruleset="Code Style" package="sfdc.sfdx.scanner.messaging" class="EventKey" method="EventKey" variable="messageKey" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_codestyle.html#methodargumentcouldbefinal" priority="3">
Parameter 'messageKey' is not assigned and could be declared final
</violation>
<violation beginline="29" endline="29" begincolumn="51" endcolumn="73" rule="MethodArgumentCouldBeFinal" ruleset="Code Style" package="sfdc.sfdx.scanner.messaging" class="EventKey" method="EventKey" variable="messageType" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_codestyle.html#methodargumentcouldbefinal" priority="3">
Parameter 'messageType' is not assigned and could be declared final
</violation>
<violation beginline="29" endline="29" begincolumn="107" endcolumn="121" rule="MethodArgumentCouldBeFinal" ruleset="Code Style" package="sfdc.sfdx.scanner.messaging" class="EventKey" method="EventKey" variable="verbose" externalInfoUrl="https://pmd.github.io/pmd-6.22.0/pmd_rules_java_codestyle.html#methodargumentcouldbefinal" priority="3">
Parameter 'verbose' is not assigned and could be declared final
</violation>
</file>
<configerror rule="LoosePackageCoupling" msg="No packages or classes specified"/>
</pmd>
45 changes: 45 additions & 0 deletions test/lib/pmd/PmdEngine.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import "reflect-metadata";
import {FileHandler} from '../../../src/lib/util/FileHandler';
import {RuleResult} from '../../../src/types';
import path = require('path');
import {expect} from 'chai';
import Sinon = require('sinon');
import {container} from 'tsyringe'
import {Services} from '../../../src/ioc.config';
import {RuleEngine} from '../../../src/lib/services/RuleEngine'
import { PmdEngine } from '../../../src/lib/pmd/PmdEngine'

class TestPmdEngine extends PmdEngine {
public xmlToRuleResults(pmdXml: string): RuleResult[] {
return super.xmlToRuleResults(pmdXml);
}
}

describe('PmdEngine', () => {
before(() => {
Sinon.createSandbox();
// Bootstrap the container.
// Avoids 'Cannot register a type name as a singleton without a "to" token' exception
container.resolveAll<RuleEngine>(Services.RuleEngine);
});
after(() => {
Sinon.restore();
});
describe('xmlToRuleResults()', () => {
it('Unknown XML nodes are ignored', async () => {
// This xml file contains a 'configerror' node that is a direct child of the pmd node.
// The configerror node should be filtered out of the results without causing any errors.
const xmlPath = path.join('test', 'code-fixtures', 'pmd-results', 'result-with-configerror.xml');
const fileHandler: FileHandler = new FileHandler();
const xml: string = await fileHandler.readFile(xmlPath);
expect(xml).to.not.be.null;

const testPmdEngine = new TestPmdEngine();
await testPmdEngine.init();

const results = testPmdEngine.xmlToRuleResults(xml);
expect(results).to.be.length(1, 'Results should be for be a single file');
expect(results[0].violations).to.be.length(13, 'The file should have 13 violations');
});
});
});