NEW(pmd): @W-17386449@: Add file_extensions config field for both cpd and pmd engines

Author: stephen-carter-at-sf

packages/code-analyzer-core/test/config.test.ts

@@ -171,7 +171,7 @@ describe("Tests for creating and accessing configuration values", () => {
     it("When top level config has an unknown key, then we error", () => {
         expect(() => CodeAnalyzerConfig.fromObject({doesNotExist: 3})).toThrow(
             getMessageFromCatalog(SHARED_MESSAGE_CATALOG,'ConfigObjectContainsInvalidKey','<TopLevel>', 'doesNotExist',
-                '["config_root","log_folder","rules","engines"]'));
+                '["config_root","engines","log_folder","rules"]'));
     });
 
     it("When engines value is not an object then we throw an error", () => {

packages/code-analyzer-engine-api/src/config.ts

@@ -74,7 +74,7 @@ export class ConfigValueExtractor {
         for (const key of actualKeys) {
             if (!lowercasePublicKeys.includes(key.toLowerCase()) && !this.keysThatBypassValidation.includes(key)) {
                 throw new Error(getMessage('ConfigObjectContainsInvalidKey',
-                    this.fieldPathRoot || '<TopLevel>', key, JSON.stringify(keys)))
+                    this.fieldPathRoot || '<TopLevel>', key, JSON.stringify(keys.sort())))
             }
         }
     }

packages/code-analyzer-engine-api/test/config.test.ts

@@ -237,7 +237,7 @@ describe("Tests for ConfigValueExtractor", () => {
     it("When validateContainsOnlySpecifiedKeys is called on an object that has extra keys, then error", () => {
         const topLevelExtractor: ConfigValueExtractor = new ConfigValueExtractor({a: 3, b: 2, c: 3}, '', __dirname);
         expect(() => topLevelExtractor.validateContainsOnlySpecifiedKeys(['a','C'])).toThrow(
-            Error(getMessage('ConfigObjectContainsInvalidKey','<TopLevel>', 'b', '["a","C"]')));
+            Error(getMessage('ConfigObjectContainsInvalidKey','<TopLevel>', 'b', '["C","a"]')));
         const subExtractor: ConfigValueExtractor = new ConfigValueExtractor({d: 5, E: 6}, 'some.other[3]', __dirname);
         subExtractor.addKeysThatBypassValidation(['hidden_keys']); // Sanity check that these don't show up in the error messages
         expect(() => subExtractor.validateContainsOnlySpecifiedKeys(['d'])).toThrow(

packages/code-analyzer-eslint-engine/src/config.ts

@@ -158,54 +158,35 @@ class ESLintEngineConfigValueExtractor {
     }
 
     extractFileExtensionsValue(): FileExtensionsObject {
-        if (!this.delegateExtractor.hasValueDefinedFor('file_extensions')) {
-            return DEFAULT_CONFIG.file_extensions;
-        }
-
-        const fileExtsObjExtractor: ConfigValueExtractor = this.delegateExtractor.extractObjectAsExtractor('file_extensions');
-
-        // Validate languages
-        const validLanguages: string[] = Object.keys(DEFAULT_CONFIG.file_extensions);
-        for (const key of fileExtsObjExtractor.getKeys()) {
-            // Note: In the future we may want to make the languages case-insensitive. Right now it is a little tricky
-            //       because the extract* methods (like extractArray) look for the exact key name.
-            if (!(validLanguages.includes(key))) {
-                throw new Error(getMessage('InvalidFieldKeyForObject', fileExtsObjExtractor.getFieldPath(), key, validLanguages.join(', ')))
+        const fileExtensionsExtractor: ConfigValueExtractor = this.delegateExtractor.extractObjectAsExtractor(
+            'file_extensions', DEFAULT_CONFIG.file_extensions);
+        fileExtensionsExtractor.validateContainsOnlySpecifiedKeys(Object.keys(DEFAULT_CONFIG.file_extensions));
+
+        const extToLangMap: Map<string, string> = new Map(); // To keep track if file extension shows up with more than one language
+        const fileExtensionsMap: FileExtensionsObject = {... DEFAULT_CONFIG.file_extensions}; // Start with copy
+        for (const language of Object.keys(fileExtensionsMap)) {
+            const fileExts: string[] = makeUnique(fileExtensionsExtractor.extractArray(language,
+                (element, elementFieldPath) => ValueValidator.validateString(element,
+                    elementFieldPath, ESLintEngineConfigValueExtractor.FILE_EXT_PATTERN),
+                DEFAULT_CONFIG.file_extensions[language as keyof FileExtensionsObject]
+            )!).map(fileExt => fileExt.toLowerCase());
+
+            // Validate that none of the file extensions already exist in another language
+            for (const fileExt of fileExts) {
+                if (extToLangMap.has(fileExt) && extToLangMap.get(fileExt) !== language) {
+                    throw new Error(getMessage('InvalidFileExtensionDueToItBeingListedTwice',
+                        fileExtensionsExtractor.getFieldPath(), fileExt,
+                        JSON.stringify([extToLangMap.get(fileExt), language])));
+                }
+                extToLangMap.set(fileExt, language);
             }
-        }
 
-        // Validate file extension patterns
-        const extractExtensionsValue = function (fieldName: string, defaultValue: string[]): string[] {
-            const fileExts: string[] = fileExtsObjExtractor.extractArray(fieldName, ValueValidator.validateString, defaultValue)!;
-            fileExts.map((fileExt, i) => validateStringMatches(
-                ESLintEngineConfigValueExtractor.FILE_EXT_PATTERN, fileExt, `${fileExtsObjExtractor.getFieldPath(fieldName)}[${i}]`));
-            return makeUnique(fileExts);
-        }
-        const fileExtsObj: FileExtensionsObject = {
-            javascript: extractExtensionsValue('javascript', DEFAULT_CONFIG.file_extensions.javascript),
-            typescript: extractExtensionsValue('typescript', DEFAULT_CONFIG.file_extensions.typescript)
+            fileExtensionsMap[language as keyof FileExtensionsObject] = fileExts;
         }
-
-        // Validate that there is no file extension listed with multiple languages
-        const allExts: string[] = fileExtsObj.javascript.concat(fileExtsObj.typescript);
-        if (allExts.length != (new Set(allExts)).size) {
-            const currentValuesString: string =
-                `  ${fileExtsObjExtractor.getFieldPath('javascript')}: ${JSON.stringify(fileExtsObj.javascript)}\n` +
-                `  ${fileExtsObjExtractor.getFieldPath('typescript')}: ${JSON.stringify(fileExtsObj.typescript)}`;
-            throw new Error(getMessage('ConfigStringArrayValuesMustNotShareElements', currentValuesString));
-        }
-
-        return fileExtsObj;
+        return fileExtensionsMap;
     }
 
     extractBooleanValue(field_name: string): boolean {
         return this.delegateExtractor.extractBoolean(field_name, DEFAULT_CONFIG[field_name as keyof ESLintEngineConfig] as boolean)!;
     }
-}
-
-function validateStringMatches(pattern: RegExp, value: string, fieldName: string): string {
-    if (!pattern.test(value)) {
-        throw new Error(getMessage('ConfigStringValueMustMatchPattern', fieldName, value, pattern.source));
-    }
-    return value;
 }

packages/code-analyzer-eslint-engine/src/messages.ts

@@ -44,14 +44,8 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
     InvalidLegacyIgnoreFileName:
         `The '%s' configuration value is invalid. Expected the file name '%s' to be equal to '%s'.`,
 
-    InvalidFieldKeyForObject:
-        `The '%s' configuration value is invalid. The value contained an invalid key '%s'. Valid keys for this object are: %s`,
-
-    ConfigStringValueMustMatchPattern:
-        `The '%s' configuration value '%s' must match the pattern: /%s/`,
-
-    ConfigStringArrayValuesMustNotShareElements:
-        `The following configuration values must not share any common array elements between them:\n%s`,
+    InvalidFileExtensionDueToItBeingListedTwice:
+        `The '%s' configuration object is invalid. The file extension '%s' is currently listed under more than one language: %s`,
 
     ESLintErroredWhenScanningFile:
         `When scanning file '%s' with the eslint engine, ESLint gave the following error:\n%s`,

packages/code-analyzer-eslint-engine/test/plugin.test.ts

@@ -47,9 +47,8 @@ describe('Tests for the ESLintEnginePlugin', () => {
         const userProvidedOverrides: ConfigObject = {dummy: 3};
         await expect(callCreateEngineConfig(plugin, userProvidedOverrides)).rejects.toThrow(
             getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigObjectContainsInvalidKey', 'engines.eslint', 'dummy',
-                '["eslint_config_file","eslint_ignore_file","auto_discover_eslint_config",' +
-                '"disable_javascript_base_config","disable_lwc_base_config","disable_typescript_base_config",' +
-                '"file_extensions"]'));
+                '["auto_discover_eslint_config","disable_javascript_base_config","disable_lwc_base_config",' +
+                '"disable_typescript_base_config","eslint_config_file","eslint_ignore_file","file_extensions"]'));
     });
 
     it('When a valid eslint_config_file is passed to createEngineConfig, then it is set on the config', async () => {
@@ -176,14 +175,15 @@ describe('Tests for the ESLintEnginePlugin', () => {
                 'engines.eslint.disable_typescript_base_config', 'boolean', 'string'));
     });
 
-    it('When an file_extensions value contains an invalid language, then createEngineConfig errors', async () => {
+    it('When a file_extensions value contains an invalid language, then createEngineConfig errors', async () => {
         const userProvidedOverrides: ConfigObject = {
             file_extensions: {
-                oops: ['.js', '.jsx', '.js']
+                oops: ['.js', '.jsx']
             }
         };
         await expect(callCreateEngineConfig(plugin, userProvidedOverrides)).rejects.toThrow(
-            getMessage('InvalidFieldKeyForObject', 'engines.eslint.file_extensions', 'oops', 'javascript, typescript'));
+            getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigObjectContainsInvalidKey', 'engines.eslint.file_extensions',
+                'oops', '["javascript","typescript"]'));
     });
 
     it('When a valid file_extensions.javascript value is passed to createEngineConfig, then it is set on the config', async () => {
@@ -212,7 +212,7 @@ describe('Tests for the ESLintEnginePlugin', () => {
     it('When a valid file_extensions.typescript value is passed to createEngineConfig, then it is set on the config', async () => {
         const userProvidedOverrides: ConfigObject = {
             file_extensions: {
-                typescript: ['.ts', '.tsx']
+                typescript: ['.tS', '.tsX'] // Also confirm we normalize the extensions to lowercase
             }
         };
         const resolvedConfig: ConfigObject = await callCreateEngineConfig(plugin, userProvidedOverrides);
@@ -229,20 +229,19 @@ describe('Tests for the ESLintEnginePlugin', () => {
             }
         };
         await expect(callCreateEngineConfig(plugin, userProvidedOverrides)).rejects.toThrow(
-            getMessage('ConfigStringValueMustMatchPattern',
-                'engines.eslint.file_extensions.typescript[1]', 'missingDot', '^[.][a-zA-Z0-9]+$'));
+            getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigValueMustMatchRegExp',
+                'engines.eslint.file_extensions.typescript[1]', '/^[.][a-zA-Z0-9]+$/'));
     });
 
-    it('When an extension is listed in more than one *_file_extensions field, then createEngineConfig errors', async () => {
+    it('When an extension is listed under more than one language within the file_extensions object, then createEngineConfig errors', async () => {
         const userProvidedOverrides: ConfigObject = {
             file_extensions: {
                 typescript: ['.ts', '.js']
             }
         };
         await expect(callCreateEngineConfig(plugin, userProvidedOverrides)).rejects.toThrow(
-            getMessage('ConfigStringArrayValuesMustNotShareElements',
-                `  engines.eslint.file_extensions.javascript: [".js",".cjs",".mjs"]\n` +
-                `  engines.eslint.file_extensions.typescript: [".ts",".js"]`));
+            getMessage('InvalidFileExtensionDueToItBeingListedTwice',
+                'engines.eslint.file_extensions', '.js', '["javascript","typescript"]'));
     });
 
     it('When createEngine is passed an invalid engine name, then an error is thrown', async () => {

packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunner.java

@@ -9,6 +9,7 @@
 import net.sourceforge.pmd.cpd.Mark;
 import net.sourceforge.pmd.cpd.Match;
 import net.sourceforge.pmd.lang.Language;
+import net.sourceforge.pmd.lang.LanguageVersion;
 import net.sourceforge.pmd.reporting.Report;
 import net.sourceforge.pmd.util.log.PmdReporter;
 import org.slf4j.event.Level;
@@ -53,16 +54,23 @@ private CpdLanguageRunResults runLanguage(String language, List<Path> pathsToSca
         System.out.println("Running CPD for language '" + language + "' with " + pathsToScan.size() +
                 " file(s) to scan using minimumTokens=" + minimumTokens + " and skipDuplicateFiles=" + skipDuplicateFiles + ".");
 
-        // Note that the name "minimumTokens" comes from the public facing documentation and the cli but
-        // behind the scenes, it maps to MinimumTileSize. To learn more about the mappings to the config, see:
-        // https://github.com/pmd/pmd/blob/main/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/CpdCommand.java
         CPDConfiguration config = new CPDConfiguration();
+
+        // Force the language so that pmd doesn't look at file extensions. Note: we already associated the files based
+        // on their file extensions to the correct languages the typescript side.
         Language cpdLanguageId = config.getLanguageRegistry().getLanguageById(language);
         if (cpdLanguageId == null) {
             throw new RuntimeException("The language \"" + language + "\" is not recognized by CPD.");
         }
-        config.setOnlyRecognizeLanguage(cpdLanguageId);
+        LanguageVersion forcedLangVer = config.getLanguageVersionDiscoverer()
+                .getDefaultLanguageVersion(cpdLanguageId);
+        config.setForceLanguageVersion(forcedLangVer);
+
+        // Note that the name "minimumTokens" comes from the public facing documentation and the cli but
+        // behind the scenes, it maps to MinimumTileSize. To learn more about the mappings to the config, see:
+        // https://github.com/pmd/pmd/blob/main/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/CpdCommand.java
         config.setMinimumTileSize(minimumTokens);
+
         config.setInputPathList(pathsToScan);
         config.setSkipDuplicates(skipDuplicateFiles);
         CpdErrorListener errorListener = new CpdErrorListener();