From 4466c60164bfb99131c7c252e8f2f27a4dea6dec Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Tue, 6 Feb 2024 16:02:26 -0500 Subject: [PATCH 1/6] @W-14980216@: [PMD7-Preview](Part 1) Refactor pmd and cpd engines to make it easy to swap PMD versions --- src/Constants.ts | 4 +- src/Controller.ts | 17 +++++++++ src/lib/cpd/CpdWrapper.ts | 12 ++---- src/lib/output/SarifOutputFormatter.ts | 6 +-- src/lib/pmd/PmdCommandInfo.ts | 45 +++++++++++++++++++++++ src/lib/pmd/PmdEngine.ts | 16 ++++++-- src/lib/pmd/PmdWrapper.ts | 19 +++------- test/lib/output/ResultsFormatting.test.ts | 4 +- test/lib/pmd/PmdEngine.test.ts | 4 +- 9 files changed, 94 insertions(+), 33 deletions(-) create mode 100644 src/lib/pmd/PmdCommandInfo.ts diff --git a/src/Constants.ts b/src/Constants.ts index 0e2c2eca3..f239b9e8e 100644 --- a/src/Constants.ts +++ b/src/Constants.ts @@ -1,7 +1,7 @@ import os = require('os'); import path = require('path'); -export const PMD_VERSION = '6.55.0'; +export const PMD6_VERSION = '6.55.0'; export const PMD_APPEXCHANGE_RULES_VERSION = '0.12'; export const SFGE_VERSION = '1.0.1-pilot'; export const DEFAULT_SCANNER_PATH = path.join(os.homedir(), '.sfdx-scanner'); @@ -132,7 +132,7 @@ export enum Severity { } // Here, current dir __dirname = /sfdx-scanner/src -export const PMD_LIB = path.join(__dirname, '..', 'dist', 'pmd', 'lib'); +export const PMD6_LIB = path.join(__dirname, '..', 'dist', 'pmd', 'lib'); // Here, current dir __dirname = /sfdx-scanner/src export const APPEXCHANGE_PMD_LIB = path.join(__dirname, '..', 'pmd-appexchange', 'lib'); diff --git a/src/Controller.ts b/src/Controller.ts index 3e69aef1d..de0637a0f 100644 --- a/src/Controller.ts +++ b/src/Controller.ts @@ -9,6 +9,7 @@ import {RuleEngine} from './lib/services/RuleEngine'; import {RulePathManager} from './lib/RulePathManager'; import {RuleCatalog} from './lib/services/RuleCatalog'; import {BundleName, getMessage} from "./MessageCatalog"; +import {Pmd6CommandInfo, PmdCommandInfo} from "./lib/pmd/PmdCommandInfo"; /** * Converts an array of RuleEngines to a sorted, comma delimited * string of their names. @@ -23,6 +24,14 @@ function enginesToString(engines: RuleEngine[]): string { // See https://stackoverflow.com/questions/137975/what-are-drawbacks-or-disadvantages-of-singleton-pattern +// We all should hate global state. But our team agreed that we have a bit of refactoring to do before we can more +// easily swap out the pmd version. So this is meant to be a temporary solution until we do that refactoring. +declare global { + // eslint-disable-next-line no-var + var _activePmdCommandInfo: PmdCommandInfo; +} +globalThis._activePmdCommandInfo = new Pmd6CommandInfo(); + // This is probably more appropriately called a ProviderFactory (Salesforce Core folks know this code smell all too well) export const Controller = { container, @@ -92,5 +101,13 @@ export const Controller = { } return engines; + }, + + setActivePmdCommandInfo(pmdCommandInfo: PmdCommandInfo): void { + globalThis._activePmdCommandInfo = pmdCommandInfo; + }, + + getActivePmdCommandInfo: (): PmdCommandInfo => { + return globalThis._activePmdCommandInfo; } }; diff --git a/src/lib/cpd/CpdWrapper.ts b/src/lib/cpd/CpdWrapper.ts index 0b40b5473..dba960a30 100644 --- a/src/lib/cpd/CpdWrapper.ts +++ b/src/lib/cpd/CpdWrapper.ts @@ -2,11 +2,9 @@ import {Logger} from '@salesforce/core'; import {FileHandler} from '../util/FileHandler'; import * as JreSetupManager from './../JreSetupManager'; import path = require('path'); -import { PMD_LIB } from '../../Constants'; import { CommandLineSupport} from '../services/CommandLineSupport'; - -const MAIN_CLASS = 'net.sourceforge.pmd.cpd.CPD'; -const HEAP_SIZE = '-Xmx1024m'; +import {Controller} from "../../Controller"; +import {PmdCommandInfo} from "../pmd/PmdCommandInfo"; interface CpdWrapperOptions { path: string; @@ -36,14 +34,12 @@ export default class CpdWrapper extends CommandLineSupport { const javaHome = await JreSetupManager.verifyJreSetup(); const command = path.join(javaHome, 'bin', 'java'); - const classpath = [`${PMD_LIB}/*`]; - const fileHandler = new FileHandler(); const tmpPath = await fileHandler.tmpFileWithCleanup(); await fileHandler.writeFile(tmpPath, this.path); - const args = ['-cp', classpath.join(path.delimiter), HEAP_SIZE, MAIN_CLASS, '--filelist', tmpPath, - '--format', 'xml', '--minimum-tokens', String(this.minimumTokens), '--language', this.language]; + const pmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo(); + const args: string[] = pmdCommandInfo.constructJavaCommandArgsForCpd(tmpPath, this.minimumTokens, this.language) this.logger.trace(`Preparing to execute CPD with command: "${command}", args: "${JSON.stringify(args)}"`); return [command, args]; diff --git a/src/lib/output/SarifOutputFormatter.ts b/src/lib/output/SarifOutputFormatter.ts index 7c8ba0908..ebbadb087 100644 --- a/src/lib/output/SarifOutputFormatter.ts +++ b/src/lib/output/SarifOutputFormatter.ts @@ -5,7 +5,7 @@ import {deepCopy, isPathlessViolation} from "../util/Utils"; import * as url from "url"; import {RuleCatalog} from "../services/RuleCatalog"; import {ESLint} from "eslint"; -import {ENGINE, PMD_VERSION, SFGE_VERSION} from "../../Constants"; +import {ENGINE, SFGE_VERSION} from "../../Constants"; import {Controller} from "../../Controller"; import * as retire from 'retire'; @@ -256,7 +256,7 @@ class PMDSarifFormatter extends SarifFormatter { tool: { driver: { name: ENGINE.PMD, - version: PMD_VERSION, + version: Controller.getActivePmdCommandInfo().getVersion(), informationUri: 'https://pmd.github.io/pmd', rules: [] } @@ -278,7 +278,7 @@ class CPDSarifFormatter extends SarifFormatter { tool: { driver: { name: ENGINE.CPD, - version: PMD_VERSION, /*CPD would use the same PMD version*/ + version: Controller.getActivePmdCommandInfo().getVersion(), /*CPD would use the same PMD version*/ informationUri: 'https://pmd.github.io/latest/pmd_userdocs_cpd.html', rules: [] } diff --git a/src/lib/pmd/PmdCommandInfo.ts b/src/lib/pmd/PmdCommandInfo.ts new file mode 100644 index 000000000..5a3017343 --- /dev/null +++ b/src/lib/pmd/PmdCommandInfo.ts @@ -0,0 +1,45 @@ +import {PMD6_LIB, PMD6_VERSION} from "../../Constants"; +import * as path from 'path'; + +const PMD6_MAIN_CLASS = 'net.sourceforge.pmd.PMD'; +const CPD6_MAIN_CLASS = 'net.sourceforge.pmd.cpd.CPD'; +const HEAP_SIZE = '-Xmx1024m'; + +export interface PmdCommandInfo { + getVersion(): string + + getJarPathForLanguage(lang: string): string + + constructJavaCommandArgsForPmd(fileList: string, classPathsForExternalRules: string[], rulesets: string): string[] + + constructJavaCommandArgsForCpd(fileList: string, minimumTokens: number, language: string): string[] +} + +export class Pmd6CommandInfo implements PmdCommandInfo { + getVersion(): string { + return PMD6_VERSION; + } + + getJarPathForLanguage(language: string): string { + return path.join(PMD6_LIB, `pmd-${language}-${this.getVersion()}.jar`); + } + + constructJavaCommandArgsForPmd(fileList: string, classPathsForExternalRules: string[], rulesets: string): string[] { + // The classpath needs PMD's lib folder. There may be redundancy with the shared classpath, but having the + // same JAR in the classpath twice is fine. Also note that the classpath is not wrapped in quotes like how it + // would be if we invoked directly through the CLI, because child_process.spawn() hates that. + const classpath = classPathsForExternalRules.concat([`${PMD6_LIB}/*`]).join(path.delimiter); + const args = ['-cp', classpath, HEAP_SIZE, PMD6_MAIN_CLASS, '-filelist', fileList, + '-format', 'xml']; + if (rulesets.length > 0) { + args.push('-rulesets', rulesets); + } + return args; + } + + constructJavaCommandArgsForCpd(fileList: string, minimumTokens: number, language: string): string[] { + const classpath = `${PMD6_LIB}/*`; + return ['-cp', classpath, HEAP_SIZE, CPD6_MAIN_CLASS, '--filelist', fileList, + '--format', 'xml', '--minimum-tokens', String(minimumTokens), '--language', language]; + } +} diff --git a/src/lib/pmd/PmdEngine.ts b/src/lib/pmd/PmdEngine.ts index 7f46fc588..2622a84ab 100644 --- a/src/lib/pmd/PmdEngine.ts +++ b/src/lib/pmd/PmdEngine.ts @@ -1,10 +1,18 @@ import {Logger, SfError} from '@salesforce/core'; import {Element, xml2js} from 'xml-js'; import {Controller} from '../../Controller'; -import {Catalog, Rule, RuleGroup, RuleResult, RuleTarget, RuleViolation, TargetPattern} from '../../types'; +import { + Catalog, + Rule, + RuleGroup, + RuleResult, + RuleTarget, + RuleViolation, + TargetPattern +} from '../../types'; import {AbstractRuleEngine} from '../services/RuleEngine'; import {Config} from '../util/Config'; -import {APPEXCHANGE_PMD_LIB, PMD_APPEXCHANGE_RULES_VERSION, CUSTOM_CONFIG, ENGINE, EngineBase, HARDCODED_RULES, PMD_LIB, PMD_VERSION, Severity} from '../../Constants'; +import {APPEXCHANGE_PMD_LIB, PMD_APPEXCHANGE_RULES_VERSION, CUSTOM_CONFIG, ENGINE, EngineBase, HARDCODED_RULES, Severity} from '../../Constants'; import {PmdCatalogWrapper} from './PmdCatalogWrapper'; import PmdWrapper from './PmdWrapper'; import {EVENTS, uxEvents} from "../ScannerEvents"; @@ -628,10 +636,12 @@ export class _PmdRuleMapper extends AsyncCreatable { const rulePathsByLanguage = new Map>(); // Add the default PMD jar for each activated language. (await PmdLanguageManager.getSupportedLanguages()).forEach(language => { - const pmdJarPath = path.join(PMD_LIB, `pmd-${language}-${PMD_VERSION}.jar`); const rulePaths = rulePathsByLanguage.get(language) || new Set(); + + const pmdJarPath = Controller.getActivePmdCommandInfo().getJarPathForLanguage(language); rulePaths.add(pmdJarPath); this.logger.trace(`Adding JAR ${pmdJarPath}, the default PMD JAR for language ${language}`); + rulePathsByLanguage.set(language, rulePaths); }); diff --git a/src/lib/pmd/PmdWrapper.ts b/src/lib/pmd/PmdWrapper.ts index dc59c528e..d0dba5625 100644 --- a/src/lib/pmd/PmdWrapper.ts +++ b/src/lib/pmd/PmdWrapper.ts @@ -1,12 +1,10 @@ import {Logger} from '@salesforce/core'; -import {PMD_LIB} from '../../Constants'; import {PmdSupport, PmdSupportOptions} from './PmdSupport'; import * as JreSetupManager from './../JreSetupManager'; import path = require('path'); import {FileHandler} from '../util/FileHandler'; - -const MAIN_CLASS = 'net.sourceforge.pmd.PMD'; -const HEAP_SIZE = '-Xmx1024m'; +import {Controller} from "../../Controller"; +import {PmdCommandInfo} from "./PmdCommandInfo"; type PmdWrapperOptions = PmdSupportOptions & { targets: string[]; @@ -49,21 +47,16 @@ export default class PmdWrapper extends PmdSupport { const javaHome = await JreSetupManager.verifyJreSetup(); const command = path.join(javaHome, 'bin', 'java'); - // The classpath needs PMD's lib folder. There may be redundancy with the shared classpath, but having the - // same JAR in the classpath twice is fine. Also note that the classpath is not wrapped in quotes like how it - // would be if we invoked directly through the CLI, because child_process.spawn() hates that. - const classpath = [...this.supplementalClasspath, `${PMD_LIB}/*`, ...this.buildSharedClasspath()].join(path.delimiter); // Operating systems impose limits on the maximum length of a command line invocation. This can be problematic // when scanning a large number of files. Store the list of files to scan in a temp file. Pass the location // of the temp file to PMD. The temp file is cleaned up when the process exits. const fileHandler = new FileHandler(); const tmpPath = await fileHandler.tmpFileWithCleanup(); await fileHandler.writeFile(tmpPath, this.targets.join(',')); - const args = ['-cp', classpath, HEAP_SIZE, MAIN_CLASS, '-filelist', tmpPath, - '-format', 'xml']; - if (this.rules.length > 0) { - args.push('-rulesets', this.rules); - } + + const pmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo(); + const classPathsForExternalRules: string[] = this.buildSharedClasspath().concat(this.supplementalClasspath); + const args: string[] = pmdCommandInfo.constructJavaCommandArgsForPmd(tmpPath, classPathsForExternalRules, this.rules); this.logger.trace(`Preparing to execute PMD with command: "${command}", args: "${JSON.stringify(args)}"`); return [command, args]; diff --git a/test/lib/output/ResultsFormatting.test.ts b/test/lib/output/ResultsFormatting.test.ts index 9c3265a6e..0a5763b71 100644 --- a/test/lib/output/ResultsFormatting.test.ts +++ b/test/lib/output/ResultsFormatting.test.ts @@ -6,7 +6,7 @@ import path = require('path'); import * as csvParse from 'csv-parse'; import {parseString} from 'xml2js'; import * as TestOverrides from '../../test-related-lib/TestOverrides'; -import { PathlessEngineFilters, ENGINE, PMD_VERSION, SFGE_VERSION } from '../../../src/Constants'; +import { PathlessEngineFilters, ENGINE, PMD6_VERSION, SFGE_VERSION } from '../../../src/Constants'; import { fail } from 'assert'; import {Results, RunResults} from "../../../src/lib/output/Results"; import { OutputFormat } from '../../../src/lib/output/OutputFormat'; @@ -517,7 +517,7 @@ describe('Results Formatting', () => { function validatePMDSarif(run: unknown, normalizeSeverity: boolean): void { const driver = run['tool']['driver']; expect(driver.name).to.equal('pmd'); - expect(driver.version).to.equal(PMD_VERSION); + expect(driver.version).to.equal(PMD6_VERSION); expect(driver.informationUri).to.equal('https://pmd.github.io/pmd'); // tool.driver.rules diff --git a/test/lib/pmd/PmdEngine.test.ts b/test/lib/pmd/PmdEngine.test.ts index c7a0398fb..8ce58784f 100644 --- a/test/lib/pmd/PmdEngine.test.ts +++ b/test/lib/pmd/PmdEngine.test.ts @@ -7,7 +7,7 @@ import Sinon = require('sinon'); import {PmdEngine, _PmdRuleMapper} from '../../../src/lib/pmd/PmdEngine' import {uxEvents, EVENTS} from '../../../src/lib/ScannerEvents'; import * as TestOverrides from '../../test-related-lib/TestOverrides'; -import {CUSTOM_CONFIG, ENGINE, LANGUAGE, PMD_VERSION} from '../../../src/Constants'; +import {CUSTOM_CONFIG, ENGINE, LANGUAGE, PMD6_VERSION} from '../../../src/Constants'; import * as DataGenerator from '../eslint/EslintTestDataGenerator'; import {BundleName, getMessage} from "../../../src/MessageCatalog"; import {Config} from "../../../src/lib/util/Config"; @@ -412,7 +412,7 @@ describe('_PmdRuleMapper', () => { const validJar = 'jar-that-exists.jar'; const missingJar = 'jar-that-is-missing.jar'; // This jar is automatically included by the PmdCatalogWrapper - const pmdJar = path.resolve(path.join('dist', 'pmd', 'lib', `pmd-java-${PMD_VERSION}.jar`)); + const pmdJar = path.resolve(path.join('dist', 'pmd', 'lib', `pmd-java-${PMD6_VERSION}.jar`)); let uxSpy = null; before(() => { From 4d7991c79f8eaeb3f15f560678bfb596aa57bae6 Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Wed, 7 Feb 2024 10:26:33 -0500 Subject: [PATCH 2/6] @W-14980258@: Add pmd7 download, install, and cleanup tasks to our gradle build --- pmd-cataloger/build.gradle.kts | 147 ++++++++++++++++++++++----------- 1 file changed, 101 insertions(+), 46 deletions(-) diff --git a/pmd-cataloger/build.gradle.kts b/pmd-cataloger/build.gradle.kts index 66f2d6bd7..1495a4a96 100644 --- a/pmd-cataloger/build.gradle.kts +++ b/pmd-cataloger/build.gradle.kts @@ -10,51 +10,11 @@ plugins { group = "sfdx" version = "1.0" -val distDir = "$buildDir/../../dist" -val pmdVersion = "6.55.0" -val pmdFile = "pmd-bin-$pmdVersion.zip" -val pmdUrl = "https://github.com/pmd/pmd/releases/download/pmd_releases%2F${pmdVersion}/${pmdFile}" -val skippableJarRegexes = setOf("""^common_[\d\.-]*\.jar""".toRegex(), - """^fastparse.*\.jar""".toRegex(), - """^groovy.*\.jar""".toRegex(), - """^lenses.*\.jar""".toRegex(), - """^parsers.*\.jar""".toRegex(), - """^pmd-(cpp|cs|dart|fortran|go|groovy|jsp|kotlin|lua|matlab|modelica|objectivec|perl|php|plsql|python|ruby|scala|swift|ui)[-_\d\.]*\.jar""".toRegex(), - """^protobuf-java-[\d\.]*\.jar""".toRegex(), - """^scala.*\.jar""".toRegex(), - """^sourcecode_[\d\.-]*\.jar""".toRegex(), - """^trees_[\d\.-]*\.jar""".toRegex() -) - repositories { mavenCentral() google() } -jacoco { - toolVersion = "0.8.7" -} - -tasks.register("downloadPmd") { - src(pmdUrl) - dest(buildDir) - overwrite(false) -} - -tasks.register("installPmd") { - dependsOn("downloadPmd") - from(zipTree("$buildDir/$pmdFile")) - exclude { details: FileTreeElement -> - skippableJarRegexes.any {it.containsMatchIn(details.file.name)} - } - into("$distDir/pmd") - // TODO include("just the *.jars etc. we care about") - includeEmptyDirs = false - eachFile { - relativePath = RelativePath(true, *relativePath.segments.drop(1).toTypedArray()) - } -} - dependencies { implementation(project(":cli-messaging")) implementation ("com.googlecode.json-simple:json-simple:1.1.1") { @@ -73,25 +33,120 @@ dependencies { testImplementation(files("$buildDir/../../test/test-jars/apex/testjar-categories-and-rulesets-1.jar")) } + +// ======== MODIFY PLUGIN PROPERTIES =================================================================================== java.sourceCompatibility = JavaVersion.VERSION_1_8 application { mainClass.set("sfdc.sfdx.scanner.pmd.Main"); } -// Running the cli locally needs the dist exploded, so just do that -// automatically with build for ease of use. +jacoco { + toolVersion = "0.8.7" +} + +val distDir = "$buildDir/../../dist" + + +// ======== DEFINE/UPDATE PMD-CATALOGER DIST RELATED TASKS ============================================================= +val pmdCatalogerDistDir = "$distDir/pmd-cataloger" + tasks.named("installDist") { - into("$distDir/pmd-cataloger") + // The installDist task comes with the distribution plugin which comes with the applciation plugin. We modify it here: + into(pmdCatalogerDistDir) +} + +tasks.register("deletePmdCatalogerDist") { + delete(pmdCatalogerDistDir) +} + +// ======== DEFINE/UPDATE PMD6 DIST RELATED TASKS ===================================================================== +val pmd6DistDir = "$distDir/pmd" +val pmd6Version = "6.55.0" +val pmd6File = "pmd-bin-$pmd6Version.zip" + +tasks.register("downloadPmd6") { + src("https://github.com/pmd/pmd/releases/download/pmd_releases%2F${pmd6Version}/${pmd6File}") + dest(buildDir) + overwrite(false) +} + +tasks.register("installPmd6") { + dependsOn("downloadPmd6") + from(zipTree("$buildDir/$pmd6File")) + + val skippablePmd6JarRegexes = setOf("""^common_[\d\.-]*\.jar""".toRegex(), + """^fastparse.*\.jar""".toRegex(), + """^groovy.*\.jar""".toRegex(), + """^lenses.*\.jar""".toRegex(), + """^parsers.*\.jar""".toRegex(), + """^pmd-(cpp|cs|dart|fortran|go|groovy|jsp|kotlin|lua|matlab|modelica|objectivec|perl|php|plsql|python|ruby|scala|swift|ui)[-_\d\.]*\.jar""".toRegex(), + """^protobuf-java-[\d\.]*\.jar""".toRegex(), + """^scala.*\.jar""".toRegex(), + """^sourcecode_[\d\.-]*\.jar""".toRegex(), + """^trees_[\d\.-]*\.jar""".toRegex() + ) + exclude { details: FileTreeElement -> + skippablePmd6JarRegexes.any {it.containsMatchIn(details.file.name)} + } + + into(pmd6DistDir) + // TODO include("just the *.jars etc. we care about") + includeEmptyDirs = false + eachFile { + // We drop the parent "pmd-bin-6.55.0" folder and put files directly into our "pmd" folder + relativePath = RelativePath(true, *relativePath.segments.drop(1).toTypedArray()) + } +} + +tasks.register("deletePmd6Dist") { + delete(pmd6DistDir) +} + + +// ======== DEFINE/UPDATE PMD7 DIST RELATED TASKS ===================================================================== +val pmd7DistDir = "$distDir/pmd7" +val pmd7Version = "7.0.0-rc4" +val pmd7File = "pmd-dist-$pmd7Version-bin.zip" + +tasks.register("downloadPmd7") { + src("https://github.com/pmd/pmd/releases/download/pmd_releases%2F${pmd7Version}/${pmd7File}") + dest(buildDir) + overwrite(false) +} + +tasks.register("installPmd7") { + dependsOn("downloadPmd7") + from(zipTree("$buildDir/$pmd7File")) + // TODO: We will soon optimize this with W-14980337 by reducing the dist down to only the jars we care about. + into(pmd7DistDir) + includeEmptyDirs = false + eachFile { + // We drop the parent "pmd-bin-7.0.0-rc4" folder and put files directly into our "pmd7" folder + relativePath = RelativePath(true, *relativePath.segments.drop(1).toTypedArray()) + } +} + +tasks.register("deletePmd7Dist") { + delete(pmd7DistDir) } -tasks.named("assemble") { - // TODO: These currently do not get cleaned with ./gradlew clean which can cause a lot of confusion. +// ======== ATTACH TASKS TO ASSEMBLE AND CLEAN ======================================================================== +tasks.assemble { dependsOn("installDist") - dependsOn("installPmd") + dependsOn("installPmd6") + dependsOn("installPmd7") +} + +tasks.clean { + dependsOn("deletePmdCatalogerDist") + dependsOn("deletePmd6Dist") + dependsOn("deletePmd7Dist") } + +// ======== TEST RELATED TASKS ========================================================================================= tasks.test { // Use JUnit 5 useJUnitPlatform() From 3fa79c1d18bd2cb95e0f8df732ca87436b5d98fc Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Thu, 8 Feb 2024 17:19:44 -0500 Subject: [PATCH 3/6] @W-14980290@: Implement Pmd7CommandInfo to allow us to call pmd7 jars --- src/Constants.ts | 2 + src/lib/Display.ts | 14 ++- src/lib/actions/RuleDescribeAction.ts | 24 ++-- src/lib/cpd/CpdEngine.ts | 8 +- src/lib/pmd/PmdCommandInfo.ts | 32 ++++- src/types.d.ts | 1 + test/commands/scanner/rule/describe.test.ts | 2 +- test/lib/FakeDisplay.ts | 11 ++ test/lib/actions/RuleDescribeAction.test.ts | 69 +++++++++++ test/lib/actions/RuleListAction.test.ts | 72 ++++++++++++ test/lib/actions/RunAction.test.ts | 124 ++++++++++++++++++++ test/lib/actions/fakes.ts | 41 +++++++ test/lib/output/ResultsFormatting.test.ts | 51 +++++--- test/lib/pmd/PmdEngine.test.ts | 24 +++- 14 files changed, 442 insertions(+), 33 deletions(-) create mode 100644 test/lib/actions/RuleDescribeAction.test.ts create mode 100644 test/lib/actions/RuleListAction.test.ts create mode 100644 test/lib/actions/RunAction.test.ts create mode 100644 test/lib/actions/fakes.ts diff --git a/src/Constants.ts b/src/Constants.ts index f239b9e8e..d0f86b934 100644 --- a/src/Constants.ts +++ b/src/Constants.ts @@ -2,6 +2,7 @@ import os = require('os'); import path = require('path'); export const PMD6_VERSION = '6.55.0'; +export const PMD7_VERSION = '7.0.0-rc4'; export const PMD_APPEXCHANGE_RULES_VERSION = '0.12'; export const SFGE_VERSION = '1.0.1-pilot'; export const DEFAULT_SCANNER_PATH = path.join(os.homedir(), '.sfdx-scanner'); @@ -133,6 +134,7 @@ export enum Severity { // Here, current dir __dirname = /sfdx-scanner/src export const PMD6_LIB = path.join(__dirname, '..', 'dist', 'pmd', 'lib'); +export const PMD7_LIB = path.join(__dirname, '..', 'dist', 'pmd7', 'lib'); // Here, current dir __dirname = /sfdx-scanner/src export const APPEXCHANGE_PMD_LIB = path.join(__dirname, '..', 'pmd-appexchange', 'lib'); diff --git a/src/lib/Display.ts b/src/lib/Display.ts index b42ed9ed2..e12fe449f 100644 --- a/src/lib/Display.ts +++ b/src/lib/Display.ts @@ -1,5 +1,6 @@ import {Spinner} from "@salesforce/sf-plugins-core"; import {Ux} from "@salesforce/sf-plugins-core/lib/ux"; +import {AnyJson} from "@salesforce/ts-types"; export interface Display { /** @@ -27,6 +28,11 @@ export interface Display { */ displayTable(data: R[], columns: Ux.Table.Columns): void; + /** + * Output object to stdout only if the "--json" flag is not present. + */ + displayStyledObject(obj: AnyJson): void; + /** * Display a message as a warning. */ @@ -91,6 +97,10 @@ export class UxDisplay implements Display { this.displayable.table(data, columns); } + public displayStyledObject(obj: AnyJson): void { + this.displayable.styledObject(obj); + } + public displayWarning(msg: string): void { this.displayable.warn(msg); } @@ -126,13 +136,15 @@ export interface Displayable { // Display an error or message as a warning. [Implemented by Command] warn(input: string): void; - // Simplified prompt for single-question confirmation. Times out and throws after 10s. [Implemented by SfCommand] confirm(message: string): Promise; // Output stylized header to stdout only when "--json" flag is not present. [Implemented by SfCommand] styledHeader(headerText: string): void; + // Output stylized object to stdout only when "--json" flag is not present. [Implemented by SfCommand] + styledObject(obj: AnyJson): void; + // Output table to stdout only when "--json" flag is not present. [Implemented by SfCommand] table(data: R[], columns: Ux.Table.Columns, options?: Ux.Table.Options): void; } diff --git a/src/lib/actions/RuleDescribeAction.ts b/src/lib/actions/RuleDescribeAction.ts index e8f87cff0..6221e9727 100644 --- a/src/lib/actions/RuleDescribeAction.ts +++ b/src/lib/actions/RuleDescribeAction.ts @@ -7,7 +7,6 @@ import {BundleName, getMessage} from "../../MessageCatalog"; import {deepCopy} from "../util/Utils"; import Dfa from "../../commands/scanner/run/dfa"; import Run from "../../commands/scanner/run"; -import {Ux} from "@salesforce/sf-plugins-core"; import {Display} from "../Display"; import {RuleFilterFactory} from "../RuleFilterFactory"; @@ -34,8 +33,6 @@ export class RuleDescribeAction implements Action { } public async run(inputs: Inputs): Promise { - const jsonEnabled: boolean = inputs.json as boolean; - const ruleFilters: RuleFilter[] = this.ruleFilterFactory.createRuleFilters(inputs); // TODO: Inject RuleManager as a dependency to improve testability by removing coupling to runtime implementation @@ -52,11 +49,11 @@ export class RuleDescribeAction implements Action { this.display.displayWarning(msg); rules.forEach((rule, idx) => { this.display.displayStyledHeader(`Rule #${idx + 1}`); - this.displayStyledRule(rule, jsonEnabled); + this.displayStyledRule(rule); }); } else { // If there's exactly one rule, we don't need to do anything special, and can just log the rule. - this.displayStyledRule(rules[0], jsonEnabled); + this.displayStyledRule(rules[0]); } // We need to return something for when the --json flag is used, so we'll just return the list of rules. return deepCopy(rules); @@ -87,9 +84,18 @@ export class RuleDescribeAction implements Action { }); } - private displayStyledRule(rule: DescribeStyledRule, jsonEnabled: boolean): void { - // TODO: We should remove this instantiation of new Ux in favor of possibly a new method on Display - new Ux({jsonEnabled: jsonEnabled}) - .styledObject(rule, ['name', 'engine', 'runWith', 'isPilot', 'enabled', 'categories', 'rulesets', 'languages', 'description', 'message']); + private displayStyledRule(rule: DescribeStyledRule): void { + this.display.displayStyledObject({ + name: rule.name, + engine: rule.engine, + runWith: rule.runWith, + isPilot: rule.isPilot, + enabled: rule.enabled, + categories: rule.categories, + rulesets: rule.rulesets, + languages: rule.languages, + description: rule.description, + message: rule.message + }) } } diff --git a/src/lib/cpd/CpdEngine.ts b/src/lib/cpd/CpdEngine.ts index 327c5019d..8688e35aa 100644 --- a/src/lib/cpd/CpdEngine.ts +++ b/src/lib/cpd/CpdEngine.ts @@ -216,10 +216,10 @@ export class CpdEngine extends AbstractRuleEngine { for (const occ of occurences) { // create a violation for each occurence of the code fragment const violation: RuleViolation = { - line: occ.attributes.line as number, - column: occ.attributes.column as number, - endLine: occ.attributes.endline as number, - endColumn: occ.attributes.endcolumn as number, + line: Number(occ.attributes.line), + column: Number(occ.attributes.column), + endLine: Number(occ.attributes.endline), + endColumn: Number(occ.attributes.endcolumn), ruleName: CpdRuleName, severity: CpdViolationSeverity, message: getMessage(BundleName.CpdEngine, "CpdViolationMessage", [codeFragmentID, occCount, occurences.length, duplication.attributes.lines, duplication.attributes.tokens]), diff --git a/src/lib/pmd/PmdCommandInfo.ts b/src/lib/pmd/PmdCommandInfo.ts index 5a3017343..a5aaa0610 100644 --- a/src/lib/pmd/PmdCommandInfo.ts +++ b/src/lib/pmd/PmdCommandInfo.ts @@ -1,8 +1,9 @@ -import {PMD6_LIB, PMD6_VERSION} from "../../Constants"; +import {PMD6_LIB, PMD6_VERSION, PMD7_LIB, PMD7_VERSION} from "../../Constants"; import * as path from 'path'; const PMD6_MAIN_CLASS = 'net.sourceforge.pmd.PMD'; const CPD6_MAIN_CLASS = 'net.sourceforge.pmd.cpd.CPD'; +const PMD7_CLI_CLASS = 'net.sourceforge.pmd.cli.PmdCli'; const HEAP_SIZE = '-Xmx1024m'; export interface PmdCommandInfo { @@ -40,6 +41,33 @@ export class Pmd6CommandInfo implements PmdCommandInfo { constructJavaCommandArgsForCpd(fileList: string, minimumTokens: number, language: string): string[] { const classpath = `${PMD6_LIB}/*`; return ['-cp', classpath, HEAP_SIZE, CPD6_MAIN_CLASS, '--filelist', fileList, - '--format', 'xml', '--minimum-tokens', String(minimumTokens), '--language', language]; + '--format', 'xml', '--minimum-tokens', minimumTokens.toString(), '--language', language]; + } +} + +export class Pmd7CommandInfo implements PmdCommandInfo { + getVersion(): string { + return PMD7_VERSION; + } + + getJarPathForLanguage(language: string): string { + return path.join(PMD7_LIB, `pmd-${language}-${this.getVersion()}.jar`); + } + + constructJavaCommandArgsForPmd(fileList: string, classPathsForExternalRules: string[], rulesets: string): string[] { + const classpath = classPathsForExternalRules.concat([`${PMD7_LIB}/*`]).join(path.delimiter); + const args = ['-cp', classpath, HEAP_SIZE, PMD7_CLI_CLASS, 'check', '--file-list', fileList, + '--format', 'xml']; + if (rulesets.length > 0) { + args.push('--rulesets', rulesets); + } + return args; + } + + constructJavaCommandArgsForCpd(fileList: string, minimumTokens: number, language: string): string[] { + const classpath = `${PMD7_LIB}/*`; + const resolvedLanguage = language === 'visualforce' ? 'vf' : language; + return ['-cp', classpath, HEAP_SIZE, PMD7_CLI_CLASS, 'cpd', '--file-list', fileList, '--format', 'xml', + '--minimum-tokens', minimumTokens.toString(), '--language', resolvedLanguage, '--skip-lexical-errors']; } } diff --git a/src/types.d.ts b/src/types.d.ts index fa9a51219..21d9da770 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -18,6 +18,7 @@ export type Rule = { // be OR'd together in this property. defaultConfig?: ESRuleConfigValue; url?: string; + message?: string; } export type TelemetryData = { diff --git a/test/commands/scanner/rule/describe.test.ts b/test/commands/scanner/rule/describe.test.ts index 15cd92d0d..cdb914abe 100644 --- a/test/commands/scanner/rule/describe.test.ts +++ b/test/commands/scanner/rule/describe.test.ts @@ -60,7 +60,7 @@ describe('scanner rule describe', () => { expect(ctx.stderr.toLowerCase()).to.contain(`WARNING: ${formattedWarning}`.toLowerCase(), 'Warning message should be formatted correctly'); // Next, verify that there are rule descriptions that are distinctly identified. - const regex = /=== Rule #1\n\nname:\s+constructor-super(.*\n)*=== Rule #2\n\nname:\s+constructor-super(.*\n)*=== Rule #3\n\nname:\s+constructor-super/g; + const regex = /=== Rule #1\n(.*\n)*name:\s+constructor-super(.*\n)*=== Rule #2\n(.*\n)*name:\s+constructor-super(.*\n)*=== Rule #3\n(.*\n)*name:\s+constructor-super/g; expect(ctx.stdout).to.match(regex, 'Output should contain three rules named constructor-super for each eslint based engine'); }); diff --git a/test/lib/FakeDisplay.ts b/test/lib/FakeDisplay.ts index 3de33c141..b9ab12444 100644 --- a/test/lib/FakeDisplay.ts +++ b/test/lib/FakeDisplay.ts @@ -1,11 +1,13 @@ import {Display} from "../../src/lib/Display"; import {Ux} from "@salesforce/sf-plugins-core"; +import {AnyJson} from "@salesforce/ts-types"; export class FakeDisplay implements Display { private outputs: string[] = []; private confirmationPromptResponse: boolean = true; private lastTableColumns: Ux.Table.Columns; private lastTableData: Ux.Table.Data[]; + private lastStyledObject: AnyJson; public getOutputArray(): string[] { return this.outputs; @@ -27,6 +29,10 @@ export class FakeDisplay implements Display { return this.lastTableData; } + public getLastStyledObject(): AnyJson { + return this.lastStyledObject; + } + displayConfirmationPrompt(msg: string): Promise { this.outputs.push(msg); @@ -59,6 +65,11 @@ export class FakeDisplay implements Display { this.outputs.push("[Table][" + JSON.stringify(columns) + "]: " + JSON.stringify(data)); } + displayStyledObject(obj: AnyJson): void { + this.lastStyledObject = obj; + this.outputs.push(JSON.stringify(obj)) + } + spinnerStart(msg: string, status?: string): void { const statusText = status ? "[" + status + "]" : ""; this.outputs.push("[SpinnerStart]" + statusText + ": " + msg) diff --git a/test/lib/actions/RuleDescribeAction.test.ts b/test/lib/actions/RuleDescribeAction.test.ts new file mode 100644 index 000000000..23f5b508d --- /dev/null +++ b/test/lib/actions/RuleDescribeAction.test.ts @@ -0,0 +1,69 @@ +import {FakeDisplay} from "../FakeDisplay"; +import {initContainer} from "../../../src/ioc.config"; +import {RuleFilterFactoryImpl} from "../../../src/lib/RuleFilterFactory"; +import {Pmd7CommandInfo, PmdCommandInfo} from "../../../src/lib/pmd/PmdCommandInfo"; +import {Controller} from "../../../src/Controller"; +import {after} from "mocha"; +import {Inputs} from "../../../src/types"; +import {expect} from "chai"; +import {RuleDescribeAction} from "../../../src/lib/actions/RuleDescribeAction"; +import {AnyJson} from "@salesforce/ts-types"; + +describe("Tests for RuleDescribeAction", () => { + let display: FakeDisplay; + let ruleDescribeAction: RuleDescribeAction; + before(() => { + initContainer(); + }); + beforeEach(() => { + display = new FakeDisplay(); + ruleDescribeAction = new RuleDescribeAction(display, new RuleFilterFactoryImpl()); + }); + + describe("Tests to confirm that PMD7 binary files are invoked when choosing PMD7 with pmd engine", () => { + + // TODO: Soon we will have an input flag to control this. Once we do, we can update this to use that instead + const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() + before(() => { + Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); + }); + after(() => { + Controller.setActivePmdCommandInfo(originalPmdCommandInfo); + }); + + it("When using PMD7, the rule description for a pmd rule should give correct info from PMD 7", async () => { + const inputs: Inputs = { + rulename: 'ApexCRUDViolation' + } + await ruleDescribeAction.run(inputs); + + const rule: AnyJson = display.getLastStyledObject(); + expect(rule['name']).to.equal('ApexCRUDViolation'); + expect(rule['engine']).to.equal('pmd'); + expect(rule['isPilot']).to.equal(false); + expect(rule['enabled']).to.equal(true); + expect(rule['categories']).to.deep.equal(['Security']); + expect(rule['rulesets']).to.contain('quickstart'); + expect(rule['languages']).to.deep.equal(['apex']); + expect(rule['description']).to.have.length.greaterThan(0); + expect(rule['message']).to.have.length.greaterThan(0); + }) + + it("When using PMD7, the rule description for a cpd rule should give back correct info from PMD 7", async () => { + const inputs: Inputs = { + rulename: 'copy-paste-detected' + } + await ruleDescribeAction.run(inputs); + + const rule: AnyJson = display.getLastStyledObject(); + expect(rule['name']).to.equal('copy-paste-detected'); + expect(rule['engine']).to.equal('cpd'); + expect(rule['isPilot']).to.equal(false); + expect(rule['enabled']).to.equal(false); + expect(rule['categories']).to.deep.equal(['Copy/Paste Detected']); + expect(rule['rulesets']).to.deep.equal([]); + expect(rule['languages']).to.deep.equal(['apex', 'java', 'visualforce', 'xml']); + expect(rule['description']).to.have.length.greaterThan(0); + }); + }); +}); diff --git a/test/lib/actions/RuleListAction.test.ts b/test/lib/actions/RuleListAction.test.ts new file mode 100644 index 000000000..c25849761 --- /dev/null +++ b/test/lib/actions/RuleListAction.test.ts @@ -0,0 +1,72 @@ +import {FakeDisplay} from "../FakeDisplay"; +import {initContainer} from "../../../src/ioc.config"; +import {RuleFilterFactoryImpl} from "../../../src/lib/RuleFilterFactory"; +import {RuleListAction} from "../../../src/lib/actions/RuleListAction"; +import {Pmd7CommandInfo, PmdCommandInfo} from "../../../src/lib/pmd/PmdCommandInfo"; +import {Controller} from "../../../src/Controller"; +import {after} from "mocha"; +import {Inputs} from "../../../src/types"; +import {expect} from "chai"; +import {Ux} from "@salesforce/sf-plugins-core"; +import {PMD7_LIB} from "../../../src/Constants"; + +describe("Tests for RuleListAction", () => { + let display: FakeDisplay; + let ruleListAction: RuleListAction; + before(() => { + initContainer(); + }); + beforeEach(() => { + display = new FakeDisplay(); + ruleListAction = new RuleListAction(display, new RuleFilterFactoryImpl()); + }); + + describe("Tests to confirm that PMD7 binary files are invoked when choosing PMD7", () => { + + // TODO: Soon we will have an input flag to control this. Once we do, we can update this to use that instead + const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() + before(() => { + Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); + }); + after(() => { + Controller.setActivePmdCommandInfo(originalPmdCommandInfo); + }); + + it("When using PMD7, the rule list for the pmd engine should give back rules for PMD 7", async () => { + const inputs: Inputs = { + engine: ['pmd'] + } + await ruleListAction.run(inputs); + + let tableData: Ux.Table.Data[] = display.getLastTableData(); + expect(tableData).to.have.length(67); + for (const rowData of tableData) { + expect(rowData.engine).to.equal("pmd"); + expect(rowData.sourcepackage).to.contain(PMD7_LIB); + expect(rowData.name).to.have.length.greaterThan(0); + expect(rowData.categories).to.have.length.greaterThan(0); + expect(rowData.isDfa).to.equal(false); + expect(rowData.isPilot).to.equal(false); + expect(rowData.languages).to.have.length.greaterThan(0); + } + }) + + it("When using PMD7, the rule list for the cpd engine should give back the copy-paste-detected rule", async () => { + const inputs: Inputs = { + engine: ['cpd'] + } + await ruleListAction.run(inputs); + + let tableData: Ux.Table.Data[] = display.getLastTableData(); + expect(tableData).to.have.length(1); + expect(tableData[0].engine).to.equal("cpd"); + expect(tableData[0].sourcepackage).to.equal("cpd"); + expect(tableData[0].name).to.equal("copy-paste-detected"); + expect(tableData[0].categories).to.deep.equal(["Copy/Paste Detected"]); + expect(tableData[0].rulesets).to.deep.equal([]); + expect(tableData[0].isDfa).to.equal(false); + expect(tableData[0].isPilot).to.equal(false); + expect(tableData[0].languages).to.deep.equal(['apex', 'java', 'visualforce', 'xml']); + }); + }); +}); diff --git a/test/lib/actions/RunAction.test.ts b/test/lib/actions/RunAction.test.ts new file mode 100644 index 000000000..4fca40f42 --- /dev/null +++ b/test/lib/actions/RunAction.test.ts @@ -0,0 +1,124 @@ +import {InputProcessor, InputProcessorImpl} from "../../../src/lib/InputProcessor"; +import {RuleFilterFactoryImpl} from "../../../src/lib/RuleFilterFactory"; +import {RunEngineOptionsFactory} from "../../../src/lib/EngineOptionsFactory"; +import {RunAction} from "../../../src/lib/actions/RunAction"; +import {FakeDisplay} from "../FakeDisplay"; +import {Logger} from "@salesforce/core"; +import {Inputs, PathlessRuleViolation, RuleResult} from "../../../src/types"; +import * as path from "path"; +import {initContainer} from '../../../src/ioc.config'; +import {expect} from "chai"; +import {Pmd7CommandInfo, PmdCommandInfo} from "../../../src/lib/pmd/PmdCommandInfo"; +import {Controller} from "../../../src/Controller"; +import {after} from "mocha"; +import {Results} from "../../../src/lib/output/Results"; +import {PMD6_VERSION, PMD7_VERSION} from "../../../src/Constants"; +import {FakeResultsProcessorFactory, RawResultsProcessor} from "./fakes"; + +const codeFixturesDir = path.join(__dirname, '..', '..', 'code-fixtures'); +const pathToSomeTestClass = path.join(codeFixturesDir, 'apex', 'SomeTestClass.cls'); +const pathToCodeForCpd = path.join(codeFixturesDir, 'cpd'); + + +describe("Tests for RunAction", () => { + let display: FakeDisplay; + let resultsProcessor: RawResultsProcessor; + let runAction: RunAction; + before(() => { + initContainer(); + }); + beforeEach(() => { + display = new FakeDisplay(); + resultsProcessor = new RawResultsProcessor(); + + const inputProcessor: InputProcessor = new InputProcessorImpl("2.11.8", display); + runAction = new RunAction( + Logger.childFromRoot("forTesting"), + display, + inputProcessor, + new RuleFilterFactoryImpl(), + new RunEngineOptionsFactory(inputProcessor), + new FakeResultsProcessorFactory(resultsProcessor)); + }); + + describe("Tests to confirm that PMD7 binary files are invoked when choosing PMD7", () => { + + // TODO: Soon we will have an input flag to control this. Once we do, we can update this to use that instead + const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() + before(() => { + Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); + }); + after(() => { + Controller.setActivePmdCommandInfo(originalPmdCommandInfo); + }); + + it("When using PMD7, the pmd engine actually uses PMD7 instead of PMD6", async () => { + const inputs: Inputs = { + target: [pathToSomeTestClass], + engine: ['pmd'], + 'normalize-severity': true + } + await runAction.run(inputs); + + const results: Results = resultsProcessor.getResults(); + expect(results.getExecutedEngines().size).to.equal(1); + expect(results.getExecutedEngines()).to.contain('pmd'); + const ruleResults: RuleResult[] = results.getRuleResults(); + expect(ruleResults).to.have.length(1); + expect(ruleResults[0].violations).to.have.length(8); + for (let violation of ruleResults[0].violations) { + violation = violation as PathlessRuleViolation; + + // Unfortunately, there isn't an easy way to detect that we are using PMD 7 binaries other than checking + // that the violation urls contain version 7 information instead of version 6. + expect(violation.url).to.contain(PMD7_VERSION); + expect(violation.url).not.to.contain(PMD6_VERSION); + + // Other sanity checks to make the fields are filled in + expect(violation.ruleName).to.have.length.greaterThan(0); + expect(violation.category).to.have.length.greaterThan(0); + expect(violation.line).to.be.greaterThan(0); + expect(violation.column).to.be.greaterThan(0); + expect(violation.message).to.have.length.greaterThan(0); + expect(violation.severity).to.be.greaterThanOrEqual(3); + expect(violation.normalizedSeverity).to.equal(3); + } + }); + + it("When using PMD7, the cpd engine actually uses PMD7 instead of PMD6", async () => { + const inputs: Inputs = { + target: [pathToCodeForCpd], + engine: ['cpd'], + 'normalize-severity': true + } + await runAction.run(inputs); + + const results: Results = resultsProcessor.getResults(); + expect(results.getExecutedEngines().size).to.equal(1); + expect(results.getExecutedEngines()).to.contain('cpd'); + const ruleResults: RuleResult[] = results.getRuleResults(); + expect(ruleResults).to.have.length(2); + expect(ruleResults[0].violations).to.have.length(1); + expect(ruleResults[1].violations).to.have.length(1); + const violation1: PathlessRuleViolation = ruleResults[0].violations[0] as PathlessRuleViolation; + const violation2: PathlessRuleViolation = ruleResults[1].violations[0] as PathlessRuleViolation; + + for (let violation of [violation1, violation2]) { + + // Unfortunately, there isn't an easy way to detect that we are using PMD 7 binaries. + // The best we can do is check for 'latest' in the url. + expect(violation.url).to.contain('latest'); + expect(violation.url).not.to.contain(PMD6_VERSION); + + // Other sanity checks to make the fields are filled in + expect(violation.ruleName).to.have.length.greaterThan(0); + expect(violation.category).to.have.length.greaterThan(0); + expect(violation.line).to.be.greaterThan(0); + expect(violation.column).to.be.greaterThan(0); + expect(violation.message).to.have.length.greaterThan(0); + expect(violation.severity).to.be.greaterThanOrEqual(3); + expect(violation.normalizedSeverity).to.equal(3); + } + }); + }); +}); diff --git a/test/lib/actions/fakes.ts b/test/lib/actions/fakes.ts new file mode 100644 index 000000000..e198733b5 --- /dev/null +++ b/test/lib/actions/fakes.ts @@ -0,0 +1,41 @@ +import {ResultsProcessorFactory} from "../../../src/lib/output/ResultsProcessorFactory"; +import {ResultsProcessor} from "../../../src/lib/output/ResultsProcessor"; +import {Display} from "../../../src/lib/Display"; +import {RunOutputOptions} from "../../../src/lib/output/RunResultsProcessor"; +import {JsonReturnValueHolder} from "../../../src/lib/output/JsonReturnValueHolder"; +import {Results} from "../../../src/lib/output/Results"; + + + +/** + * This fake does zero processing, but instead gives you the ability to access the raw results that were to be processed + */ +export class RawResultsProcessor implements ResultsProcessor { + private results: Results; + + processResults(results: Results): Promise { + this.results = results; + return Promise.resolve(); + } + + getResults(): Results { + return this.results; + } +} + + + +/** + * This fake just passes back whatever results processor you pass in. + */ +export class FakeResultsProcessorFactory implements ResultsProcessorFactory { + private readonly resultsProcessor: ResultsProcessor; + + constructor(resultsProcessor: ResultsProcessor) { + this.resultsProcessor = resultsProcessor + } + + createResultsProcessor(_d: Display, _r: RunOutputOptions, _j: JsonReturnValueHolder): ResultsProcessor { + return this.resultsProcessor + } +} diff --git a/test/lib/output/ResultsFormatting.test.ts b/test/lib/output/ResultsFormatting.test.ts index 0a5763b71..e828093b7 100644 --- a/test/lib/output/ResultsFormatting.test.ts +++ b/test/lib/output/ResultsFormatting.test.ts @@ -10,6 +10,9 @@ import { PathlessEngineFilters, ENGINE, PMD6_VERSION, SFGE_VERSION } from '../.. import { fail } from 'assert'; import {Results, RunResults} from "../../../src/lib/output/Results"; import { OutputFormat } from '../../../src/lib/output/OutputFormat'; +import {Controller} from "../../../lib/Controller"; +import {Pmd7CommandInfo, PmdCommandInfo} from "../../../lib/lib/pmd/PmdCommandInfo"; +import {PMD7_VERSION} from "../../../lib/Constants"; const sampleFile1 = path.join('Users', 'SomeUser', 'samples', 'sample-file1.js'); const sampleFile2 = path.join('Users', 'SomeUser', 'samples', 'sample-file2.js'); @@ -303,6 +306,22 @@ const retireJsVerboseViolations: RuleResult[] = [ } ]; +function createSampleRuleResultsForEngine(engine: string): RuleResult[] { + return [{ + engine: engine, + fileName: sampleFile1, + violations: [{ + "line": 2, + "column": 11, + "severity": 2, + "message": "A generic message", + "ruleName": "rule-name", + "category": "category-name", + "url": "https://some/url.org" + }] + }]; +} + function isString(x: string | {columns; rows}): x is string { return typeof x === 'string'; } @@ -610,7 +629,6 @@ describe('Results Formatting', () => { } it ('Happy Path - pathless rules', async () => { - const results: Results = new RunResults(allFakePathlessRuleResults, new Set(['eslint', 'pmd'])); const formattedOutput: FormattedOutput = await results.toFormattedOutput(OutputFormat.SARIF, false); const minSev: number = results.getMinSev(); @@ -744,20 +762,7 @@ describe('Results Formatting', () => { it('Handles all pathless engines', async () => { const allEngines = PathlessEngineFilters.map(engine => engine.valueOf()); for (const engine of allEngines) { - const ruleResults: RuleResult[] = [{ - engine: engine, - fileName: sampleFile1, - violations: [{ - "line": 2, - "column": 11, - "severity": 2, - "message": "A generic message", - "ruleName": "rule-name", - "category": "category-name", - "url": "https://some/url.org" - }] - }]; - const results: Results = new RunResults(ruleResults, new Set([engine])); + const results: Results = new RunResults(createSampleRuleResultsForEngine(engine), new Set([engine])); await results.toFormattedOutput(OutputFormat.SARIF, false); // should throw an error if the engine was not handled } @@ -785,6 +790,22 @@ describe('Results Formatting', () => { } } }); + + it ('Switching to PMD7 is reflected in sarif output for pmd and cpd engines', async () => { + const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() + try { + Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); + for (const engine of ['pmd', 'cpd']) { + const results: Results = new RunResults(createSampleRuleResultsForEngine(engine), new Set([engine])); + const formattedOutput: FormattedOutput = await results.toFormattedOutput(OutputFormat.SARIF, false); + const sarifResults: unknown[] = JSON.parse(formattedOutput as string); + expect(sarifResults['runs']).to.have.lengthOf(1); + expect(sarifResults['runs'][0]['tool']['driver']['version']).to.equal(PMD7_VERSION); + } + } finally { + Controller.setActivePmdCommandInfo(originalPmdCommandInfo); + } + }) }); describe('Output Format: JSON', () => { diff --git a/test/lib/pmd/PmdEngine.test.ts b/test/lib/pmd/PmdEngine.test.ts index 8ce58784f..f9e03a81b 100644 --- a/test/lib/pmd/PmdEngine.test.ts +++ b/test/lib/pmd/PmdEngine.test.ts @@ -7,12 +7,14 @@ import Sinon = require('sinon'); import {PmdEngine, _PmdRuleMapper} from '../../../src/lib/pmd/PmdEngine' import {uxEvents, EVENTS} from '../../../src/lib/ScannerEvents'; import * as TestOverrides from '../../test-related-lib/TestOverrides'; -import {CUSTOM_CONFIG, ENGINE, LANGUAGE, PMD6_VERSION} from '../../../src/Constants'; +import {CUSTOM_CONFIG, ENGINE, LANGUAGE, PMD6_VERSION, PMD7_LIB, PMD7_VERSION} from '../../../src/Constants'; import * as DataGenerator from '../eslint/EslintTestDataGenerator'; import {BundleName, getMessage} from "../../../src/MessageCatalog"; import {Config} from "../../../src/lib/util/Config"; import {CustomRulePathManager} from "../../../src/lib/CustomRulePathManager"; import {after} from "mocha"; +import {Pmd7CommandInfo, PmdCommandInfo} from "../../../src/lib/pmd/PmdCommandInfo"; +import {Controller} from "../../../src/Controller"; TestOverrides.initializeTestSetup(); @@ -450,4 +452,24 @@ describe('_PmdRuleMapper', () => { Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, `Custom rule file path [${missingJar}] for language [${LANGUAGE.JAVA}] was not found.`); }); }); + + describe('Using PMD7', async () => { + const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() + before(() => { + Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); + }); + after(() => { + Controller.setActivePmdCommandInfo(originalPmdCommandInfo); + }) + + it('PMD7 lib jar files are found correctly', async () => { + const mapper = await _PmdRuleMapper.create({}); + const ruleMap = await mapper.createStandardRuleMap(); + expect(ruleMap.size).to.greaterThan(0); + ruleMap.forEach((jars: Set, language: string) => { + expect(jars.size).to.equal(1); + expect(jars).to.contain(path.join(PMD7_LIB, `pmd-${language}-${PMD7_VERSION}.jar`)); + }) + }) + }); }); From b47bd2695e358d1be9119a1dace967f6f821ded6 Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Mon, 12 Feb 2024 17:17:00 -0500 Subject: [PATCH 4/6] @W-14980337@: Optimize the pmd6 and pmd7 binary distributions --- pmd-cataloger/build.gradle.kts | 66 +++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/pmd-cataloger/build.gradle.kts b/pmd-cataloger/build.gradle.kts index 1495a4a96..28a07e36c 100644 --- a/pmd-cataloger/build.gradle.kts +++ b/pmd-cataloger/build.gradle.kts @@ -57,7 +57,7 @@ tasks.named("installDist") { } tasks.register("deletePmdCatalogerDist") { - delete(pmdCatalogerDistDir) + delete(pmdCatalogerDistDir) } // ======== DEFINE/UPDATE PMD6 DIST RELATED TASKS ===================================================================== @@ -75,23 +75,29 @@ tasks.register("installPmd6") { dependsOn("downloadPmd6") from(zipTree("$buildDir/$pmd6File")) - val skippablePmd6JarRegexes = setOf("""^common_[\d\.-]*\.jar""".toRegex(), - """^fastparse.*\.jar""".toRegex(), - """^groovy.*\.jar""".toRegex(), - """^lenses.*\.jar""".toRegex(), - """^parsers.*\.jar""".toRegex(), - """^pmd-(cpp|cs|dart|fortran|go|groovy|jsp|kotlin|lua|matlab|modelica|objectivec|perl|php|plsql|python|ruby|scala|swift|ui)[-_\d\.]*\.jar""".toRegex(), - """^protobuf-java-[\d\.]*\.jar""".toRegex(), - """^scala.*\.jar""".toRegex(), - """^sourcecode_[\d\.-]*\.jar""".toRegex(), - """^trees_[\d\.-]*\.jar""".toRegex() + // I went to https://github.com/pmd/pmd/tree/pmd_releases/6.55.0 and for each of the languages that we support + // (apex, java, visualforce, xml), I took a look at its pom file (like pmd-apex/src/pom.xml for example). + // That pom said what other modules it depends on and I listed these dependencies (besides test-scoped and optional + // modules). I did this recursively for any dependent pmd-* modules and put it all here. + // For completeness, I listed the modules and all their dependencies. Duplicates don't matter since we use setOf. + val pmd6ModulesToInclude = setOf( + // LANGUAGE MODULE DEPENDENCIES + "pmd-apex", "pmd-core", "antlr-runtime", "pmd-apex-jorje", "commons-lang3", + "pmd-java", "pmd-core", "saxon", "asm", "commons-lang3", + "pmd-visualforce", "pmd-core", "pmd-apex", + "pmd-xml", "pmd-core", "antlr4-runtime", "saxon", + // DEPENDENT MODULE DEPENDENCIES + "pmd-core", "antlr4-runtime", "jcommander", "saxon", "commons-lang3", "asm", "gson", + "pmd-apex-jorje", "cglib", "logback-classic", "logback-core", "jsr305", "gson", "error_prone_annotations", "guava", "j2objc-annotations", "antlr-runtime", "stringtemplate", "common-lang3", "animal-sniffer-annotations", "jol-core", "slf4j-api", "snakeyaml", "aopalliance", "javax.inject", "asm" ) - exclude { details: FileTreeElement -> - skippablePmd6JarRegexes.any {it.containsMatchIn(details.file.name)} + + val pmd6JarsToIncludeRegexes = mutableSetOf("""^LICENSE""".toRegex()) + pmd6ModulesToInclude.forEach { + pmd6JarsToIncludeRegexes.add("""^$it-.*\.jar""".toRegex()) } + include { details: FileTreeElement -> pmd6JarsToIncludeRegexes.any { it.containsMatchIn(details.file.name) } } into(pmd6DistDir) - // TODO include("just the *.jars etc. we care about") includeEmptyDirs = false eachFile { // We drop the parent "pmd-bin-6.55.0" folder and put files directly into our "pmd" folder @@ -118,7 +124,35 @@ tasks.register("downloadPmd7") { tasks.register("installPmd7") { dependsOn("downloadPmd7") from(zipTree("$buildDir/$pmd7File")) - // TODO: We will soon optimize this with W-14980337 by reducing the dist down to only the jars we care about. + + // I went to https://github.com/pmd/pmd/tree/pmd_releases/7.0.0-rc4 and for each of the languages that we support + // (apex, java, visualforce, xml), I took a look at its pom file (like pmd-apex/src/pom.xml for example). + // That pom said what other modules it depends on and I took these dependencies (besides test-scoped and optional + // modules). I did this recursively for any dependent pmd-* modules and put it all here. + // For completeness, I listed the modules and all their dependencies except for pmd-ui (since it isn't needed) and + // the dependencies of pmd-languages-deps (since it contains all language modules). I also had to add in pmd-cli. + // Note that "pkgforce_2.13" was missing from pmd-apex, so I added it in and that unfortunately required me to pull + // in all of its dependencies listed at https://central.sonatype.com/artifact/com.github.nawforce/pkgforce_2.13/dependencies. + // Duplicates don't matter since we use setOf. + val pmd7ModulesToInclude = setOf( + // LANGUAGE MODULE DEPENDENCIES + "pmd-apex", "pmd-core", "antlr-runtime", "pmd-apex-jorje", "apex-link", "commons-lang3", "guava", "pkgforce_2.13", + "pmd-java", "pmd-core", "asm", "commons-lang3", "checker-qual", "Saxon-HE", "pcollections", + "pmd-visualforce", "pmd-core", "pmd-apex", + "pmd-xml", "pmd-core", "antlr4-runtime", + // MAIN CLI MODULE DEPENDENCIES + "pmd-cli", "pmd-languages-deps", "slf4j-api", "slf4j-simple", "picocli", "progressbar", "checker-qual", + // DEPENDENT MODULE DEPENDENCIES + "pmd-core", "slf4j-api", "jul-to-slf4j", "antlr4-runtime", "Saxon-HE", "commons-lang3", "asm", "gson", "checker-qual", "pcollections", "nice-xml-messages", + "pmd-apex-jorje", "cglib", "jsr305", "gson", "error_prone_annotations", "guava", "j2objc-annotations", "antlr-runtime", "stringtemplate", "commons-lang3", "animal-sniffer-annotations", "slf4j-api", "aopalliance", "javax.inject", "asm", + "pkgforce_2.13", "scala-json-rpc-upickle-json-serializer_2.13", "scala-json-rpc_2.13", "geny_2.13", "ujson_2.13", "upack_2.13", "upickle-core_2.13", "upickle-implicits_2.13", "upickle_2.13", "apex-parser", "antlr4-runtime", "scala-collection-compat_2.13", "scala-xml_2.13", "scala-library", "scala-reflect" + ) + val pmd7JarsToIncludeRegexes = mutableSetOf("""^LICENSE""".toRegex()) + pmd7ModulesToInclude.forEach { + pmd7JarsToIncludeRegexes.add("""^$it-.*\.jar""".toRegex()) + } + + include { details: FileTreeElement -> pmd7JarsToIncludeRegexes.any { it.containsMatchIn(details.file.name) } } into(pmd7DistDir) includeEmptyDirs = false eachFile { @@ -140,7 +174,7 @@ tasks.assemble { } tasks.clean { - dependsOn("deletePmdCatalogerDist") + dependsOn("deletePmdCatalogerDist") dependsOn("deletePmd6Dist") dependsOn("deletePmd7Dist") } From e40c03dc9f3332b918bae50bd014f95d7281eb59 Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Fri, 9 Feb 2024 12:21:42 -0500 Subject: [PATCH 5/6] @W-14980315@: Add --preview-pmd7 to run, rule describe, and rule list --- messages/describe.md | 8 +++++++ messages/list.md | 8 +++++++ messages/run-pathless.md | 8 +++++++ src/commands/scanner/rule/describe.ts | 7 +++++- src/commands/scanner/rule/list.ts | 8 +++++-- src/commands/scanner/run.ts | 7 +++++- src/lib/actions/AbstractRunAction.ts | 2 ++ src/lib/actions/RuleDescribeAction.ts | 2 ++ src/lib/actions/RuleListAction.ts | 2 ++ test/lib/actions/RuleDescribeAction.test.ts | 21 +++++++---------- test/lib/actions/RuleListAction.test.ts | 24 +++++++++----------- test/lib/actions/RunAction.test.ts | 25 +++++++++------------ 12 files changed, 76 insertions(+), 46 deletions(-) diff --git a/messages/describe.md b/messages/describe.md index 5226678da..40b57f11e 100644 --- a/messages/describe.md +++ b/messages/describe.md @@ -14,6 +14,14 @@ the name of the rule The name of the rule. +# flags.previewPmd7Summary + +use PMD version %s to describe PMD and CPD rules + +# flags.previewPmd7Description + +Uses PMD version %s instead of %s to describe PMD and CPD rules. + # output.noMatchingRules No rules were found with the name '%s'. diff --git a/messages/list.md b/messages/list.md index 7fa8f2d50..723146886 100644 --- a/messages/list.md +++ b/messages/list.md @@ -38,6 +38,14 @@ select rules by engine Selects rules by engine. Enter multiple engines as a comma-separated list. +# flags.previewPmd7Summary + +use PMD version %s to list PMD and CPD rules + +# flags.previewPmd7Description + +Uses PMD version %s instead of %s to list PMD and CPD rules. + # rulesetDeprecation The 'ruleset' command parameter is deprecated. Use 'category' instead diff --git a/messages/run-pathless.md b/messages/run-pathless.md index 8f313161d..7d001585c 100644 --- a/messages/run-pathless.md +++ b/messages/run-pathless.md @@ -66,6 +66,14 @@ specify location of PMD rule reference XML file to customize rule selection Specifies the location of PMD rule reference XML file to customize rule selection. +# flags.previewPmd7Summary + +use PMD version %s when running PMD and CPD + +# flags.previewPmd7Description + +Uses PMD version %s instead of %s when running PMD and CPD engines. + # flags.verboseViolationsSummary return retire-js violation message details diff --git a/src/commands/scanner/rule/describe.ts b/src/commands/scanner/rule/describe.ts index bcbde2744..e04553576 100644 --- a/src/commands/scanner/rule/describe.ts +++ b/src/commands/scanner/rule/describe.ts @@ -5,6 +5,7 @@ import {BundleName, getMessage} from "../../../MessageCatalog"; import {Logger} from "@salesforce/core"; import {Display} from "../../../lib/Display"; import {RuleDescribeAction} from "../../../lib/actions/RuleDescribeAction"; +import {PMD6_VERSION, PMD7_VERSION} from "../../../Constants"; /** * Defines the "rule describe" command for the "scanner" cli. @@ -28,7 +29,11 @@ export default class Describe extends ScannerCommand { }), verbose: Flags.boolean({ summary: getMessage(BundleName.Common, 'flags.verboseSummary') - }) + }), + "preview-pmd7": Flags.boolean({ + summary: getMessage(BundleName.Describe, 'flags.previewPmd7Summary', [PMD7_VERSION]), + description: getMessage(BundleName.Describe, 'flags.previewPmd7Description', [PMD7_VERSION, PMD6_VERSION]) + }), }; protected createAction(_logger: Logger, display: Display): Action { diff --git a/src/commands/scanner/rule/list.ts b/src/commands/scanner/rule/list.ts index ca3774949..c94908458 100644 --- a/src/commands/scanner/rule/list.ts +++ b/src/commands/scanner/rule/list.ts @@ -1,6 +1,6 @@ import {Flags} from '@salesforce/sf-plugins-core'; import {Action, ScannerCommand} from '../../../lib/ScannerCommand'; -import {AllowedEngineFilters} from '../../../Constants'; +import {AllowedEngineFilters, PMD6_VERSION, PMD7_VERSION} from '../../../Constants'; import {BundleName, getMessage} from "../../../MessageCatalog"; import {Logger} from "@salesforce/core"; import {Display} from "../../../lib/Display"; @@ -56,7 +56,11 @@ export default class List extends ScannerCommand { options: [...AllowedEngineFilters], delimiter: ',', multiple: true - })() + })(), + "preview-pmd7": Flags.boolean({ + summary: getMessage(BundleName.List, 'flags.previewPmd7Summary', [PMD7_VERSION]), + description: getMessage(BundleName.List, 'flags.previewPmd7Description', [PMD7_VERSION, PMD6_VERSION]) + }), }; protected createAction(_logger: Logger, display: Display): Action { diff --git a/src/commands/scanner/run.ts b/src/commands/scanner/run.ts index ce67dad98..78cbcd19d 100644 --- a/src/commands/scanner/run.ts +++ b/src/commands/scanner/run.ts @@ -1,5 +1,5 @@ import {Flags} from '@salesforce/sf-plugins-core'; -import {PathlessEngineFilters} from '../../Constants'; +import {PathlessEngineFilters, PMD6_VERSION, PMD7_VERSION} from '../../Constants'; import {ScannerRunCommand} from '../../lib/ScannerRunCommand'; import {EngineOptionsFactory, RunEngineOptionsFactory} from "../../lib/EngineOptionsFactory"; import {InputProcessor, InputProcessorImpl} from "../../lib/InputProcessor"; @@ -69,6 +69,11 @@ export default class Run extends ScannerRunCommand { summary: getMessage(BundleName.Run, 'flags.pmdConfigSummary'), description: getMessage(BundleName.Run, 'flags.pmdConfigDescription') }), + "preview-pmd7": Flags.boolean({ + summary: getMessage(BundleName.Run, 'flags.previewPmd7Summary', [PMD7_VERSION]), + description: getMessage(BundleName.Run, 'flags.previewPmd7Description', [PMD7_VERSION, PMD6_VERSION]) + }), + // TODO: This flag was implemented for W-7791882, and it's suboptimal. It leaks the abstraction and pollutes the command. // It should be replaced during the 3.0 release cycle. env: Flags.string({ diff --git a/src/lib/actions/AbstractRunAction.ts b/src/lib/actions/AbstractRunAction.ts index a840368ce..d015a3a68 100644 --- a/src/lib/actions/AbstractRunAction.ts +++ b/src/lib/actions/AbstractRunAction.ts @@ -21,6 +21,7 @@ import {ResultsProcessorFactory} from "../output/ResultsProcessorFactory"; import {JsonReturnValueHolder} from "../output/JsonReturnValueHolder"; import untildify = require('untildify'); import normalize = require('normalize-path'); +import {Pmd6CommandInfo, Pmd7CommandInfo} from "../pmd/PmdCommandInfo"; /** * Abstract Action to share a common implementation behind the "run" and "run dfa" commands @@ -80,6 +81,7 @@ export abstract class AbstractRunAction implements Action { } async run(inputs: Inputs): Promise { + Controller.setActivePmdCommandInfo(inputs['preview-pmd7'] ? new Pmd7CommandInfo() : new Pmd6CommandInfo()); const filters: RuleFilter[] = this.ruleFilterFactory.createRuleFilters(inputs); const targetPaths: string[] = this.inputProcessor.resolveTargetPaths(inputs); const runOptions: RunOptions = this.inputProcessor.createRunOptions(inputs, this.isDfa()); diff --git a/src/lib/actions/RuleDescribeAction.ts b/src/lib/actions/RuleDescribeAction.ts index 6221e9727..4df308eb1 100644 --- a/src/lib/actions/RuleDescribeAction.ts +++ b/src/lib/actions/RuleDescribeAction.ts @@ -9,6 +9,7 @@ import Dfa from "../../commands/scanner/run/dfa"; import Run from "../../commands/scanner/run"; import {Display} from "../Display"; import {RuleFilterFactory} from "../RuleFilterFactory"; +import {Pmd6CommandInfo, Pmd7CommandInfo} from "../pmd/PmdCommandInfo"; type DescribeStyledRule = Rule & { runWith: string; @@ -33,6 +34,7 @@ export class RuleDescribeAction implements Action { } public async run(inputs: Inputs): Promise { + Controller.setActivePmdCommandInfo(inputs['preview-pmd7'] ? new Pmd7CommandInfo() : new Pmd6CommandInfo()); const ruleFilters: RuleFilter[] = this.ruleFilterFactory.createRuleFilters(inputs); // TODO: Inject RuleManager as a dependency to improve testability by removing coupling to runtime implementation diff --git a/src/lib/actions/RuleListAction.ts b/src/lib/actions/RuleListAction.ts index 22663d54d..f0c5f2521 100644 --- a/src/lib/actions/RuleListAction.ts +++ b/src/lib/actions/RuleListAction.ts @@ -7,6 +7,7 @@ import {RuleFilter} from "../RuleFilter"; import {Controller} from "../../Controller"; import {Display} from "../Display"; import {RuleFilterFactory} from "../RuleFilterFactory"; +import {Pmd6CommandInfo, Pmd7CommandInfo} from "../pmd/PmdCommandInfo"; const MSG_YES: string = getMessage(BundleName.List, 'yes'); const MSG_NO: string = getMessage(BundleName.List, 'no'); @@ -59,6 +60,7 @@ export class RuleListAction implements Action { } public async run(inputs: Inputs): Promise { + Controller.setActivePmdCommandInfo(inputs['preview-pmd7'] ? new Pmd7CommandInfo() : new Pmd6CommandInfo()); const ruleFilters: RuleFilter[] = this.ruleFilterFactory.createRuleFilters(inputs); // TODO: Inject RuleManager as a dependency to improve testability by removing coupling to runtime implementation diff --git a/test/lib/actions/RuleDescribeAction.test.ts b/test/lib/actions/RuleDescribeAction.test.ts index 23f5b508d..0f376edac 100644 --- a/test/lib/actions/RuleDescribeAction.test.ts +++ b/test/lib/actions/RuleDescribeAction.test.ts @@ -1,13 +1,12 @@ import {FakeDisplay} from "../FakeDisplay"; import {initContainer} from "../../../src/ioc.config"; import {RuleFilterFactoryImpl} from "../../../src/lib/RuleFilterFactory"; -import {Pmd7CommandInfo, PmdCommandInfo} from "../../../src/lib/pmd/PmdCommandInfo"; -import {Controller} from "../../../src/Controller"; -import {after} from "mocha"; import {Inputs} from "../../../src/types"; import {expect} from "chai"; import {RuleDescribeAction} from "../../../src/lib/actions/RuleDescribeAction"; import {AnyJson} from "@salesforce/ts-types"; +import {Pmd6CommandInfo} from "../../../lib/lib/pmd/PmdCommandInfo"; +import {Controller} from "../../../lib/Controller"; describe("Tests for RuleDescribeAction", () => { let display: FakeDisplay; @@ -21,19 +20,15 @@ describe("Tests for RuleDescribeAction", () => { }); describe("Tests to confirm that PMD7 binary files are invoked when choosing PMD7 with pmd engine", () => { - - // TODO: Soon we will have an input flag to control this. Once we do, we can update this to use that instead - const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() - before(() => { - Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); - }); - after(() => { - Controller.setActivePmdCommandInfo(originalPmdCommandInfo); - }); + afterEach(() => { + // Until we remove global state, we should cleanup after ourselves to prevent other tests from being impacted + Controller.setActivePmdCommandInfo(new Pmd6CommandInfo()) + }) it("When using PMD7, the rule description for a pmd rule should give correct info from PMD 7", async () => { const inputs: Inputs = { - rulename: 'ApexCRUDViolation' + rulename: 'ApexCRUDViolation', + "preview-pmd7": true } await ruleDescribeAction.run(inputs); diff --git a/test/lib/actions/RuleListAction.test.ts b/test/lib/actions/RuleListAction.test.ts index c25849761..9be671a71 100644 --- a/test/lib/actions/RuleListAction.test.ts +++ b/test/lib/actions/RuleListAction.test.ts @@ -2,13 +2,12 @@ import {FakeDisplay} from "../FakeDisplay"; import {initContainer} from "../../../src/ioc.config"; import {RuleFilterFactoryImpl} from "../../../src/lib/RuleFilterFactory"; import {RuleListAction} from "../../../src/lib/actions/RuleListAction"; -import {Pmd7CommandInfo, PmdCommandInfo} from "../../../src/lib/pmd/PmdCommandInfo"; -import {Controller} from "../../../src/Controller"; -import {after} from "mocha"; import {Inputs} from "../../../src/types"; import {expect} from "chai"; import {Ux} from "@salesforce/sf-plugins-core"; import {PMD7_LIB} from "../../../src/Constants"; +import {Controller} from "../../../lib/Controller"; +import {Pmd6CommandInfo} from "../../../lib/lib/pmd/PmdCommandInfo"; describe("Tests for RuleListAction", () => { let display: FakeDisplay; @@ -22,19 +21,15 @@ describe("Tests for RuleListAction", () => { }); describe("Tests to confirm that PMD7 binary files are invoked when choosing PMD7", () => { - - // TODO: Soon we will have an input flag to control this. Once we do, we can update this to use that instead - const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() - before(() => { - Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); - }); - after(() => { - Controller.setActivePmdCommandInfo(originalPmdCommandInfo); - }); + afterEach(() => { + // Until we remove global state, we should cleanup after ourselves to prevent other tests from being impacted + Controller.setActivePmdCommandInfo(new Pmd6CommandInfo()) + }) it("When using PMD7, the rule list for the pmd engine should give back rules for PMD 7", async () => { const inputs: Inputs = { - engine: ['pmd'] + engine: ['pmd'], + "preview-pmd7": true } await ruleListAction.run(inputs); @@ -53,7 +48,8 @@ describe("Tests for RuleListAction", () => { it("When using PMD7, the rule list for the cpd engine should give back the copy-paste-detected rule", async () => { const inputs: Inputs = { - engine: ['cpd'] + engine: ['cpd'], + "preview-pmd7": true } await ruleListAction.run(inputs); diff --git a/test/lib/actions/RunAction.test.ts b/test/lib/actions/RunAction.test.ts index 4fca40f42..1af2e9102 100644 --- a/test/lib/actions/RunAction.test.ts +++ b/test/lib/actions/RunAction.test.ts @@ -8,18 +8,16 @@ import {Inputs, PathlessRuleViolation, RuleResult} from "../../../src/types"; import * as path from "path"; import {initContainer} from '../../../src/ioc.config'; import {expect} from "chai"; -import {Pmd7CommandInfo, PmdCommandInfo} from "../../../src/lib/pmd/PmdCommandInfo"; -import {Controller} from "../../../src/Controller"; -import {after} from "mocha"; import {Results} from "../../../src/lib/output/Results"; import {PMD6_VERSION, PMD7_VERSION} from "../../../src/Constants"; import {FakeResultsProcessorFactory, RawResultsProcessor} from "./fakes"; +import {Controller} from "../../../lib/Controller"; +import {Pmd6CommandInfo} from "../../../lib/lib/pmd/PmdCommandInfo"; const codeFixturesDir = path.join(__dirname, '..', '..', 'code-fixtures'); const pathToSomeTestClass = path.join(codeFixturesDir, 'apex', 'SomeTestClass.cls'); const pathToCodeForCpd = path.join(codeFixturesDir, 'cpd'); - describe("Tests for RunAction", () => { let display: FakeDisplay; let resultsProcessor: RawResultsProcessor; @@ -42,21 +40,17 @@ describe("Tests for RunAction", () => { }); describe("Tests to confirm that PMD7 binary files are invoked when choosing PMD7", () => { - - // TODO: Soon we will have an input flag to control this. Once we do, we can update this to use that instead - const originalPmdCommandInfo: PmdCommandInfo = Controller.getActivePmdCommandInfo() - before(() => { - Controller.setActivePmdCommandInfo(new Pmd7CommandInfo()); - }); - after(() => { - Controller.setActivePmdCommandInfo(originalPmdCommandInfo); - }); + afterEach(() => { + // Until we remove global state, we should cleanup after ourselves to prevent other tests from being impacted + Controller.setActivePmdCommandInfo(new Pmd6CommandInfo()) + }) it("When using PMD7, the pmd engine actually uses PMD7 instead of PMD6", async () => { const inputs: Inputs = { target: [pathToSomeTestClass], engine: ['pmd'], - 'normalize-severity': true + 'normalize-severity': true, + "preview-pmd7": true } await runAction.run(inputs); @@ -89,7 +83,8 @@ describe("Tests for RunAction", () => { const inputs: Inputs = { target: [pathToCodeForCpd], engine: ['cpd'], - 'normalize-severity': true + 'normalize-severity': true, + "preview-pmd7": true } await runAction.run(inputs); From 96acf31b17f60b00f0aae25983e44debb2e47139 Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Wed, 14 Feb 2024 13:19:19 -0500 Subject: [PATCH 6/6] @W-15041815@: Force the global --json flag to show up in our help text --- pmd-cataloger/build.gradle.kts | 51 ++++++++++++++-------------------- src/lib/ScannerCommand.ts | 4 +++ 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/pmd-cataloger/build.gradle.kts b/pmd-cataloger/build.gradle.kts index 28a07e36c..a2361247d 100644 --- a/pmd-cataloger/build.gradle.kts +++ b/pmd-cataloger/build.gradle.kts @@ -76,19 +76,17 @@ tasks.register("installPmd6") { from(zipTree("$buildDir/$pmd6File")) // I went to https://github.com/pmd/pmd/tree/pmd_releases/6.55.0 and for each of the languages that we support - // (apex, java, visualforce, xml), I took a look at its pom file (like pmd-apex/src/pom.xml for example). - // That pom said what other modules it depends on and I listed these dependencies (besides test-scoped and optional - // modules). I did this recursively for any dependent pmd-* modules and put it all here. - // For completeness, I listed the modules and all their dependencies. Duplicates don't matter since we use setOf. + // (apex, java, visualforce, xml), I took a look at its direct and indirect dependencies at + // https://central.sonatype.com/artifact/net.sourceforge.pmd/pmd-apex/dependencies + // by selecting the 6.55.0 dropdown and clicking on "Dependencies" and selecting "All Dependencies". + // For completeness, I listed the modules and all their compile time dependencies (direct and indirect). + // Duplicates don't matter since we use setOf. val pmd6ModulesToInclude = setOf( - // LANGUAGE MODULE DEPENDENCIES - "pmd-apex", "pmd-core", "antlr-runtime", "pmd-apex-jorje", "commons-lang3", - "pmd-java", "pmd-core", "saxon", "asm", "commons-lang3", - "pmd-visualforce", "pmd-core", "pmd-apex", - "pmd-xml", "pmd-core", "antlr4-runtime", "saxon", - // DEPENDENT MODULE DEPENDENCIES - "pmd-core", "antlr4-runtime", "jcommander", "saxon", "commons-lang3", "asm", "gson", - "pmd-apex-jorje", "cglib", "logback-classic", "logback-core", "jsr305", "gson", "error_prone_annotations", "guava", "j2objc-annotations", "antlr-runtime", "stringtemplate", "common-lang3", "animal-sniffer-annotations", "jol-core", "slf4j-api", "snakeyaml", "aopalliance", "javax.inject", "asm" + // LANGUAGE MODULE DEPENDENCIES (direct and indirect) + "pmd-apex", "animal-sniffer-annotations", "antlr", "antlr-runtime", "antlr4-runtime", "aopalliance", "asm", "cglib", "commons-lang3", "error_prone_annotations", "gson", "j2objc-annotations", "javax.inject", "jcommander", "jol-core", "jsr305", "logback-classic", "logback-core", "pmd-apex-jorje", "pmd-core", "saxon", "slf4j-api", "stringtemplate", + "pmd-java", "antlr4-runtime", "asm", "commons-lang3", "gson", "jcommander", "pmd-core", "saxon", + "pmd-visualforce", "animal-sniffer-annotations", "antlr", "antlr-runtime", "antlr4-runtime", "aopalliance", "asm", "cglib", "commons-lang3", "error_prone_annotations", "gson", "j2objc-annotations", "javax.inject", "jcommander", "jol-core", "jsr305", "logback-classic", "logback-core", "pmd-apex", "pmd-apex-jorje", "pmd-core", "saxon", "slf4j-api", "stringtemplate", + "pmd-xml", "antr4-runtime", "asm", "commons-lang3", "gson", "jcommander", "pmd-core", "saxon" ) val pmd6JarsToIncludeRegexes = mutableSetOf("""^LICENSE""".toRegex()) @@ -126,26 +124,19 @@ tasks.register("installPmd7") { from(zipTree("$buildDir/$pmd7File")) // I went to https://github.com/pmd/pmd/tree/pmd_releases/7.0.0-rc4 and for each of the languages that we support - // (apex, java, visualforce, xml), I took a look at its pom file (like pmd-apex/src/pom.xml for example). - // That pom said what other modules it depends on and I took these dependencies (besides test-scoped and optional - // modules). I did this recursively for any dependent pmd-* modules and put it all here. - // For completeness, I listed the modules and all their dependencies except for pmd-ui (since it isn't needed) and - // the dependencies of pmd-languages-deps (since it contains all language modules). I also had to add in pmd-cli. - // Note that "pkgforce_2.13" was missing from pmd-apex, so I added it in and that unfortunately required me to pull - // in all of its dependencies listed at https://central.sonatype.com/artifact/com.github.nawforce/pkgforce_2.13/dependencies. + // (apex, java, visualforce, xml), I took a look at its direct and indirect dependencies at + // https://central.sonatype.com/artifact/net.sourceforge.pmd/pmd-apex/dependencies + // by selecting the 7.0.0-rc4 dropdown and clicking on "Dependencies" and selecting "All Dependencies". + // For completeness, I listed the modules and all their compile time dependencies (direct and indirect). // Duplicates don't matter since we use setOf. val pmd7ModulesToInclude = setOf( - // LANGUAGE MODULE DEPENDENCIES - "pmd-apex", "pmd-core", "antlr-runtime", "pmd-apex-jorje", "apex-link", "commons-lang3", "guava", "pkgforce_2.13", - "pmd-java", "pmd-core", "asm", "commons-lang3", "checker-qual", "Saxon-HE", "pcollections", - "pmd-visualforce", "pmd-core", "pmd-apex", - "pmd-xml", "pmd-core", "antlr4-runtime", - // MAIN CLI MODULE DEPENDENCIES - "pmd-cli", "pmd-languages-deps", "slf4j-api", "slf4j-simple", "picocli", "progressbar", "checker-qual", - // DEPENDENT MODULE DEPENDENCIES - "pmd-core", "slf4j-api", "jul-to-slf4j", "antlr4-runtime", "Saxon-HE", "commons-lang3", "asm", "gson", "checker-qual", "pcollections", "nice-xml-messages", - "pmd-apex-jorje", "cglib", "jsr305", "gson", "error_prone_annotations", "guava", "j2objc-annotations", "antlr-runtime", "stringtemplate", "commons-lang3", "animal-sniffer-annotations", "slf4j-api", "aopalliance", "javax.inject", "asm", - "pkgforce_2.13", "scala-json-rpc-upickle-json-serializer_2.13", "scala-json-rpc_2.13", "geny_2.13", "ujson_2.13", "upack_2.13", "upickle-core_2.13", "upickle-implicits_2.13", "upickle_2.13", "apex-parser", "antlr4-runtime", "scala-collection-compat_2.13", "scala-xml_2.13", "scala-library", "scala-reflect" + // LANGUAGE MODULE DEPENDENCIES (direct and indirect) + "pmd-apex", "Saxon-HE", "animal-sniffer-annotations", "antlr", "antlr-runtime", "antlr4-runtime", "aopalliance", "apex-parser", "apexlink", "asm", "cglib", "checker-qual", "commons-lang3", "error_prone_annotations", "failureaccess", "geny_2.13", "gson", "guava", "j2objc-annotations", "javax.inject", "jsr305", "jul-to-slf4j", "listenablefuture", "nice-xml-messages", "pcollections", "pkgforce_2.13", "pmd-apex-jorje", "pmd-core", "runforce", "scala-collection-compat_2.13", "scala-json-rpc-upickle-json-serializer_2.13", "scala-json-rpc_2.13", "scala-library", "scala-parallel-collections_2.13", "scala-reflect", "scala-xml_2.13", "slf4j-api", "stringtemplate", "ujson_2.13", "upack_2.13", "upickle-core_2.13", "upickle-implicits_2.13", "upickle_2.13", + "pmd-java", "Saxon-HE", "antlr4-runtime", "asm", "checker-qual", "commons-lang3", "gson", "jul-to-slf4j", "nice-xml-messages", "pcollections", "pmd-core", "slf4j-api", + "pmd-visualforce", "Saxon-HE", "animal-sniffer-annotations", "antlr", "antlr-runtime", "antlr4-runtime", "aopalliance", "apex-parser", "apexlink", "asm", "cglib", "checker-qual", "commons-lang3", "error_prone_annotations", "failureaccess", "geny_2.13", "gson", "guava", "j2objc-annotations", "javax.inject", "jsr305", "jul-to-slf4j", "listenablefuture", "nice-xml-messages", "pcollections", "pkgforce_2.13", "pmd-apex", "pmd-apex-jorje", "pmd-core", "runforce", "scala-collection-compat_2.13", "scala-json-rpc-upickle-json-serializer_2.13", "scala-json-rpc_2.13", "scala-library", "scala-parallel-collections_2.13", "scala-reflect", "scala-xml_2.13", "slf4j-api", "stringtemplate", "ujson_2.13", "upack_2.13", "upickle-core_2.13", "upickle-implicits_2.13", "upickle_2.13", + "pmd-xml", "Saxon-HE", "antlr4-runtime", "asm", "checker-qual", "commons-lang3", "gson", "jul-to-slf4j", "nice-xml-messages", "pcollections", "pmd-core", "slf4j-api", + // MAIN CLI MODULE DEPENDENCIES (direct and indirect) + "pmd-cli", "Saxon-HE", "antlr4-runtime", "asm", "checker-qual", "commons-lang3", "gson", "jline", "jul-to-slf4j", "nice-xml-messages", "pcollections", "picocli", "pmd-core", "pmd-ui", "progressbar", "slf4j-api", "slf4j-simple", ) val pmd7JarsToIncludeRegexes = mutableSetOf("""^LICENSE""".toRegex()) pmd7ModulesToInclude.forEach { diff --git a/src/lib/ScannerCommand.ts b/src/lib/ScannerCommand.ts index 5b7a4e39e..e37acca0e 100644 --- a/src/lib/ScannerCommand.ts +++ b/src/lib/ScannerCommand.ts @@ -24,6 +24,10 @@ import {Logger} from "@salesforce/core"; * runtime dependency injection points to their corresponding Action classes. */ export abstract class ScannerCommand extends SfCommand implements Displayable { + + // It appears we need to explicitly set this in order for the global --json flag to show up in the generated help text + public static readonly enableJsonFlag = true; + protected async init(): Promise { await super.init(); initContainer();