feat: validate packageRules selectors (#1810)

packageRules selectors should only ever be inside a packageRule object, or at the top level of a preset. This feature enforces this rule with a validation check.

Closes #1773 

bin/config-validator.js

@@ -11,8 +11,8 @@ initLogger();
 
 let returnVal = 0;
 
-async function validate(desc, config) {
-  const res = await validateConfig(massageConfig(config));
+async function validate(desc, config, isPreset = false) {
+  const res = await validateConfig(massageConfig(config), isPreset);
   if (res.errors.length) {
     console.log(
       `${desc} contains errors:\n\n${JSON.stringify(res.errors, null, 2)}`
@@ -57,7 +57,7 @@ async function validate(desc, config) {
     if (pkgJson['renovate-config']) {
       console.log(`Validating package.json > renovate-config`);
       for (const presetConfig of Object.values(pkgJson['renovate-config'])) {
-        await validate('package.json > renovate-config', presetConfig);
+        await validate('package.json > renovate-config', presetConfig, true);
       }
     }
   } catch (err) {

lib/config/validation.js

@@ -12,7 +12,7 @@ module.exports = {
   validateConfig,
 };
 
-async function validateConfig(config, parentPath) {
+async function validateConfig(config, isPreset, parentPath) {
   if (!optionTypes) {
     optionTypes = {};
     options.forEach(option => {
@@ -103,6 +103,7 @@ async function validateConfig(config, parentPath) {
               if (isObject(subval)) {
                 const subValidation = await module.exports.validateConfig(
                   subval,
+                  isPreset,
                   `${currentPath}[${subIndex}]`
                 );
                 warnings = warnings.concat(subValidation.warnings);
@@ -125,14 +126,15 @@ async function validateConfig(config, parentPath) {
                 }
               }
             }
+
+            const selectors = [
+              'depTypeList',
+              'packageNames',
+              'packagePatterns',
+              'excludePackageNames',
+              'excludePackagePatterns',
+            ];
             if (key === 'packageRules') {
-              const selectors = [
-                'depTypeList',
-                'packageNames',
-                'packagePatterns',
-                'excludePackageNames',
-                'excludePackagePatterns',
-              ];
               for (const packageRule of val) {
                 let hasSelector = false;
                 if (isObject(packageRule)) {
@@ -172,6 +174,16 @@ async function validateConfig(config, parentPath) {
                 });
               }
             }
+            if (
+              selectors.includes(key) &&
+              !(parentPath && parentPath.match(/packageRules\[\d+\]$/)) && // Inside a packageRule
+              (parentPath || !isPreset) // top level in a preset
+            ) {
+              errors.push({
+                depName: 'Configuration Error',
+                message: `${currentPath}: ${key} should be inside a \`packageRule\` only`,
+              });
+            }
           }
         } else if (type === 'string') {
           if (!isString(val)) {
@@ -184,6 +196,7 @@ async function validateConfig(config, parentPath) {
           if (isObject(val)) {
             const subValidation = await module.exports.validateConfig(
               val,
+              isPreset,
               currentPath
             );
             warnings = warnings.concat(subValidation.warnings);

test/config/__snapshots__/validation.spec.js.snap

@@ -57,72 +57,30 @@ Array [
 ]
 `;
 
-exports[`config/validation validateConfig(config) errors for all types 2`] = `
+exports[`config/validation validateConfig(config) ignore packageRule nesting validation for presets 1`] = `Array []`;
+
+exports[`config/validation validateConfig(config) returns nested errors 1`] = `
 Array [
   Object {
     "depName": "Configuration Error",
-    "message": "Invalid semver range for allowedVersions: \`foo\`",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Configuration option \`enabled\` should be boolean. Found: 1 (number)",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Invalid schedule: \`Schedule \\"every 15 mins every weekday\\" should not specify minutes\`",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "timezone: Invalid timezone: Asia",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Configuration option \`labels\` should be a list (Array)",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Configuration option \`semanticCommitType\` should be a string",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Configuration option \`lockFileMaintenance\` should be a json object",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "extends: Invalid timezone: Europe/Brussel",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Invalid configuration option: \`packageRules[1].foo\`",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Configuration option \`packageRules[3].packagePatterns\` should be a list (Array)",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "Invalid regExp for packageRules[3].excludePackagePatterns: \`abc ([a-z]+) ([a-z]+))\`",
-  },
-  Object {
-    "depName": "Configuration Error",
-    "message": "packageRules: Each packageRule must contain at least one selector (depTypeList, packageNames, packagePatterns, excludePackageNames, excludePackagePatterns). If you wish for configuration to apply to all packages, it is not necessary to place it inside a packageRule at all.",
+    "message": "Invalid configuration option: \`foo\`",
   },
   Object {
     "depName": "Configuration Error",
-    "message": "packageRules must contain JSON objects",
+    "message": "Invalid configuration option: \`lockFileMaintenance.bar\`",
   },
 ]
 `;
 
-exports[`config/validation validateConfig(config) returns nested errors 1`] = `
+exports[`config/validation validateConfig(config) selectors outside packageRules array trigger errors 1`] = `
 Array [
   Object {
     "depName": "Configuration Error",
-    "message": "Invalid configuration option: \`foo\`",
+    "message": "packageNames: packageNames should be inside a \`packageRule\` only",
   },
   Object {
     "depName": "Configuration Error",
-    "message": "Invalid configuration option: \`lockFileMaintenance.bar\`",
+    "message": "docker.minor.packageNames: packageNames should be inside a \`packageRule\` only",
   },
 ]
 `;

test/config/validation.spec.js

@@ -57,7 +57,47 @@ describe('config/validation', () => {
       expect(warnings).toHaveLength(0);
       expect(errors).toMatchSnapshot();
       expect(errors).toHaveLength(13);
+    });
+    it('selectors outside packageRules array trigger errors', async () => {
+      const config = {
+        packageNames: ['angular'],
+        meteor: {
+          packageRules: [
+            {
+              packageNames: ['meteor'],
+            },
+          ],
+        },
+        docker: {
+          minor: {
+            packageNames: ['testPackage'],
+          },
+        },
+      };
+      const { warnings, errors } = await configValidation.validateConfig(
+        config
+      );
+      expect(warnings).toHaveLength(0);
+      expect(errors).toMatchSnapshot();
+      expect(errors).toHaveLength(2);
+    });
+    it('ignore packageRule nesting validation for presets', async () => {
+      const config = {
+        description: ['All angular.js packages'],
+        packageNames: [
+          'angular',
+          'angular-animate',
+          'angular-scroll',
+          'angular-sanitize',
+        ],
+      };
+      const { warnings, errors } = await configValidation.validateConfig(
+        config,
+        true
+      );
+      expect(warnings).toHaveLength(0);
       expect(errors).toMatchSnapshot();
+      expect(errors).toHaveLength(0);
     });
   });
 });