Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/code-analyzer-pmd-engine/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@salesforce/code-analyzer-pmd-engine",
"description": "Plugin package that adds 'pmd' and 'cpd' as engines into Salesforce Code Analyzer",
"version": "0.15.0",
"version": "0.15.1-SNAPSHOT",
"author": "The Salesforce Code Analyzer Team",
"license": "BSD-3-Clause",
"homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
Expand Down
24 changes: 12 additions & 12 deletions packages/code-analyzer-pmd-engine/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type PmdEngineConfig = {
// If not defined, or equal to null, then an attempt will be made to automatically discover a 'java' command from your environment.
java_command: string

// !! NOTE !! - HIDDEN UNTIL A USER REQUESTS THIS - ALL LANGUAGES ARE ENABLED BY DEFAULT SO THERE MAY NOT BE A USE CASE FOR THIS YET
// List of languages associated with the PMD rules to be made available for 'pmd' engine rule selection.
// The languages that you may choose from are: 'apex', 'html', 'javascript' (or 'ecmascript'), 'visualforce', 'xml'
// See https://pmd.github.io/pmd/tag_rule_references.html to learn about the PMD rules available for each language.
Expand All @@ -42,7 +43,7 @@ export type PmdEngineConfig = {

export const DEFAULT_PMD_ENGINE_CONFIG: PmdEngineConfig = {
java_command: DEFAULT_JAVA_COMMAND,
rule_languages: ['apex', 'visualforce'],
rule_languages: PMD_AVAILABLE_LANGUAGES, // hidden
java_classpath_entries: [],
custom_rulesets: []
}
Expand All @@ -55,11 +56,10 @@ export const PMD_ENGINE_CONFIG_DESCRIPTION: ConfigDescription = {
valueType: "string",
defaultValue: null // Using null for doc and since it indicates that the value is calculated based on the environment
},
rule_languages: {
descriptionText: getMessage('PmdConfigFieldDescription_rule_languages', toAvailableLanguagesText(PMD_AVAILABLE_LANGUAGES)),
valueType: "array",
defaultValue: DEFAULT_PMD_ENGINE_CONFIG.rule_languages,
},

// rule_languages - is excluded here so that it can remain hidden
// rule_languages WILL REMAIN HIDDEN UNTIL A USER REQUESTS THIS - ALL LANGUAGES ARE ENABLED BY DEFAULT SO THERE MAY NOT BE A USE CASE FOR THIS YET

java_classpath_entries: {
descriptionText: getMessage('PmdConfigFieldDescription_java_classpath_entries'),
valueType: "array",
Expand All @@ -81,6 +81,7 @@ export type CpdEngineConfig = {
// If not defined, or equal to null, then an attempt will be made to automatically discover a 'java' command from your environment.
java_command: string

// !! NOTE !! - HIDDEN UNTIL A USER REQUESTS THIS - ALL LANGUAGES ARE ENABLED BY DEFAULT SO THERE MAY NOT BE A USE CASE FOR THIS YET
// List of languages associated with CPD to be made available for 'cpd' engine rule selection.
// The languages that you may choose from are: 'apex', 'html', 'javascript' (or 'ecmascript'), 'typescript', 'visualforce', 'xml'
rule_languages: string[]
Expand All @@ -97,7 +98,7 @@ export type CpdEngineConfig = {

export const DEFAULT_CPD_ENGINE_CONFIG: CpdEngineConfig = {
java_command: DEFAULT_JAVA_COMMAND,
rule_languages: ['apex', 'html', 'javascript', 'typescript', 'visualforce', 'xml'],
rule_languages: CPD_AVAILABLE_LANGUAGES, // hidden
minimum_tokens: 100,
skip_duplicate_files: false
}
Expand All @@ -110,11 +111,10 @@ export const CPD_ENGINE_CONFIG_DESCRIPTION: ConfigDescription = {
valueType: "string",
defaultValue: null // Using null for doc and since it indicates that the value is calculated based on the environment
},
rule_languages: {
descriptionText: getMessage('CpdConfigFieldDescription_rule_languages', toAvailableLanguagesText(CPD_AVAILABLE_LANGUAGES)),
valueType: "array",
defaultValue: DEFAULT_CPD_ENGINE_CONFIG.rule_languages
},

// rule_languages - is excluded here so that it can remain hidden
// rule_languages WILL REMAIN HIDDEN UNTIL A USER REQUESTS THIS - ALL LANGUAGES ARE ENABLED BY DEFAULT SO THERE MAY NOT BE A USE CASE FOR THIS YET

minimum_tokens: {
descriptionText: getMessage('CpdConfigFieldDescription_minimum_tokens'),
valueType: "number",
Expand Down
10 changes: 9 additions & 1 deletion packages/code-analyzer-pmd-engine/src/cpd-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,18 @@ export class CpdEngine extends Engine {

function createRuleForLanguage(languageId: LanguageId): RuleDescription {
const languageTag: string = languageId.charAt(0).toUpperCase() + languageId.slice(1);

// We agreed that html and xml can be noisy and are less important for users to be made aware of duplicate code
// so we will be just adding Recommended tag to programming languages: apex, javascript, typescript, and visualforce
const recommendedLanguages: Set<LanguageId> = new Set([LanguageId.APEX, LanguageId.JAVASCRIPT, LanguageId.TYPESCRIPT, LanguageId.VISUALFORCE]);

return {
name: getRuleNameFromLanguage(languageId),
severityLevel: SeverityLevel.Info,
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.DESIGN, languageTag],
tags: [
... (recommendedLanguages.has(languageId) ? [COMMON_TAGS.RECOMMENDED] : []),
COMMON_TAGS.CATEGORIES.DESIGN,
languageTag],
description: getMessage('DetectCopyPasteForLanguageRuleDescription', languageId),
resourceUrls: ['https://docs.pmd-code.org/latest/pmd_userdocs_cpd.html#refactoring-duplicates']
}
Expand Down
9 changes: 0 additions & 9 deletions packages/code-analyzer-pmd-engine/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
`May be provided as the name of a command that exists on the path, or an absolute file path location.\n` +
`If unspecified, or specified as null, then an attempt will be made to automatically discover a 'java' command from your environment.`,

PmdConfigFieldDescription_rule_languages:
`List of languages associated with the PMD rules to be made available for 'pmd' engine rule selection.\n` +
`The languages that you may choose from are: %s.\n` +
`See https://pmd.github.io/pmd/tag_rule_references.html to learn about the PMD rules available for each language.`,

PmdConfigFieldDescription_java_classpath_entries:
`List of jar files and/or folders to add the Java classpath when running PMD.\n` +
`Each entry may be given as an absolute path or a relative path to 'config_root'.\n` +
Expand All @@ -34,10 +29,6 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
`CPD ENGINE CONFIGURATION\n` +
`To learn more about this configuration, visit: __LINK_COMING_SOON__`,

CpdConfigFieldDescription_rule_languages:
`List of languages associated with CPD to be made available for 'cpd' engine rule selection.\n` +
`The languages that you may choose from are: %s.`,

CpdConfigFieldDescription_minimum_tokens:
`The minimum number of tokens required to be in a duplicate block of code in order to be reported as a violation.\n` +
`The concept of a token may be defined differently per language, but in general it a distinct basic element of source code.\n` +
Expand Down
5 changes: 5 additions & 0 deletions packages/code-analyzer-pmd-engine/src/pmd-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ export class PmdEngine extends Engine {
.map(ruleName => fetchRuleInfoByRuleName(ruleInfoList, ruleName))
.filter(ruleInfo => ruleInfo !== null);

if (selectedRuleInfoList.length === 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Small optimization that I noticed while I was in here... doing a quick return when there are zero rules to run.

this.emitRunRulesProgressEvent(100);
return {violations: []};
}

const pmdResults: PmdResults = await this.pmdWrapperInvoker.invokeRunCommand(selectedRuleInfoList, filesToScan,
(innerPerc: number) => this.emitRunRulesProgressEvent(10 + 88*(innerPerc/100))); // 10 to 98%

Expand Down
7 changes: 1 addition & 6 deletions packages/code-analyzer-pmd-engine/test/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ describe('Tests for the PmdCpdEnginesPlugin', () => {

it(`When describeEngineConfig is passed 'pmd' then the correct config description is returned`, async () => {
expect(plugin.describeEngineConfig('pmd')).toEqual(PMD_ENGINE_CONFIG_DESCRIPTION);

// Sanity check that we list the correct available languages:
expect(PMD_ENGINE_CONFIG_DESCRIPTION.fieldDescriptions!['rule_languages'].descriptionText).toEqual(
getMessage('PmdConfigFieldDescription_rule_languages',
`'apex', 'html', 'javascript' (or 'ecmascript'), 'visualforce', 'xml'`));
});

it('When describeEngineConfig is passed an unsupported engine name, then an error is thrown', async () => {
Expand Down Expand Up @@ -71,7 +66,7 @@ describe('Tests for the PmdCpdEnginesPlugin', () => {
expect(resolvedConfig.java_command.endsWith('java')).toEqual(true);
expect(resolvedConfig).toEqual({
java_command: resolvedConfig.java_command, // Already checked that it ends with 'java'
rule_languages: ['apex', 'visualforce'],
rule_languages: ['apex', 'html', 'javascript', 'visualforce', 'xml'],
java_classpath_entries: [],
custom_rulesets: []
});
Expand Down
20 changes: 15 additions & 5 deletions packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ describe('Tests for the getName method of PmdEngine', () => {


describe('Tests for the describeRules method of PmdEngine', () => {
it('When using defaults without workspace, then apex and visualforce rules are returned', async () => {
it('When using defaults without workspace, then all language rules are returned', async () => {
const engine: PmdEngine = new PmdEngine(DEFAULT_PMD_ENGINE_CONFIG);
const logEvents: LogEvent[] = [];
engine.onEvent(EventType.LogEvent, (e: LogEvent) => logEvents.push(e));
const progressEvents: DescribeRulesProgressEvent[] = [];
engine.onEvent(EventType.DescribeRulesProgressEvent, (e: DescribeRulesProgressEvent) => progressEvents.push(e));

const ruleDescriptions: RuleDescription[] = await engine.describeRules({});
await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_apexAndVisualforce.goldfile.json');
await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_allLanguages.goldfile.json');

// Also check that we have fine logs with the argument list and the duration in milliseconds
const fineLogEvents: LogEvent[] = logEvents.filter(e => e.logLevel === LogLevel.Fine);
Expand All @@ -66,11 +66,21 @@ describe('Tests for the describeRules method of PmdEngine', () => {
await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_apexOnly.goldfile.json');
});

it('When using defaults with workspace containing only apex and xml code, then only apex rules are returned', async () => {
it('When using defaults with workspace containing only apex and visualforce code, then only apex and visualforce rules are returned', async () => {
const engine: PmdEngine = new PmdEngine(DEFAULT_PMD_ENGINE_CONFIG);
const workspace: Workspace = new Workspace([
path.join(TEST_DATA_FOLDER, 'samplePmdWorkspace', 'dummy.trigger'),
path.join(TEST_DATA_FOLDER, 'samplePmdWorkspace', 'dummy.xml')
path.join(TEST_DATA_FOLDER, 'samplePmdWorkspace', 'dummy.page')
]);
const ruleDescriptions: RuleDescription[] = await engine.describeRules({workspace: workspace});
await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_apexAndVisualforce.goldfile.json');
});

it('When using defaults with workspace containing only apex and text files, then only apex rules are returned', async () => {
const engine: PmdEngine = new PmdEngine(DEFAULT_PMD_ENGINE_CONFIG);
const workspace: Workspace = new Workspace([
path.join(TEST_DATA_FOLDER, 'samplePmdWorkspace', 'dummy.trigger'),
path.join(TEST_DATA_FOLDER, 'samplePmdWorkspace', 'dummy.txt')
]);
const ruleDescriptions: RuleDescription[] = await engine.describeRules({workspace: workspace});
await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_apexOnly.goldfile.json');
Expand Down Expand Up @@ -393,7 +403,7 @@ describe('Tests for the runRules method of PmdEngine', () => {
const progressEvents: RunRulesProgressEvent[] = [];
engine.onEvent(EventType.RunRulesProgressEvent, (e: RunRulesProgressEvent) => progressEvents.push(e));

const workspace: Workspace = new Workspace([path.join(TEST_DATA_FOLDER, 'samplePmdWorkspace', 'dummy.xml')]);
const workspace: Workspace = new Workspace([path.join(TEST_DATA_FOLDER, 'samplePmdWorkspace', 'dummy.txt')]);
const ruleNames: string[] = ['OperationWithLimitsInLoop', 'VfUnescapeEl'];
const results: EngineRunResults = await engine.runRules(ruleNames, {workspace: workspace});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
"name": "DetectCopyPasteForHtml",
"severityLevel": 5,
"tags": [
"Recommended",
"Design",
"Html"
],
Expand Down Expand Up @@ -68,7 +67,6 @@
"name": "DetectCopyPasteForXml",
"severityLevel": 5,
"tags": [
"Recommended",
"Design",
"Xml"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
"name": "DetectCopyPasteForHtml",
"severityLevel": 5,
"tags": [
"Recommended",
"Design",
"Html"
],
Expand Down
Loading