feat: validate packageRule selectors (#1728)

Validates that each packageRule must contain at least one selector. Resolves the rule first to allow for presets.

Closes #1345, Closes #1693

lib/config/migrate-validate.js

@@ -6,7 +6,7 @@ module.exports = {
   migrateAndValidate,
 };
 
-function migrateAndValidate(config, input) {
+async function migrateAndValidate(config, input) {
   logger.debug('migrateAndValidate()');
   try {
     const { isMigrated, migratedConfig } = configMigration.migrateConfig(input);
@@ -20,7 +20,7 @@ function migrateAndValidate(config, input) {
     }
     const massagedConfig = configMassage.massageConfig(migratedConfig);
     logger.debug({ config: massagedConfig }, 'massaged config');
-    const { warnings, errors } = configValidation.validateConfig(
+    const { warnings, errors } = await configValidation.validateConfig(
       massagedConfig
     );
     // istanbul ignore if

lib/config/validation.js

 const semver = require('semver');
 const options = require('./definitions').getOptions();
+const { resolveConfigPresets } = require('./presets');
 const {
   hasValidSchedule,
   hasValidTimezone,
@@ -11,7 +12,7 @@ module.exports = {
   validateConfig,
 };
 
-function validateConfig(config) {
+async function validateConfig(config) {
   if (!optionTypes) {
     optionTypes = {};
     options.forEach(option => {
@@ -99,7 +100,9 @@ function validateConfig(config) {
           } else {
             for (const subval of val) {
               if (isObject(subval)) {
-                const subValidation = module.exports.validateConfig(subval);
+                const subValidation = await module.exports.validateConfig(
+                  subval
+                );
                 warnings = warnings.concat(subValidation.warnings);
                 errors = errors.concat(subValidation.errors);
               }
@@ -120,6 +123,41 @@ function validateConfig(config) {
                 }
               }
             }
+            if (key === 'packageRules') {
+              const selectors = [
+                'packageNames',
+                'packagePatterns',
+                'excludePackageNames',
+                'excludePackagePatterns',
+              ];
+              for (const packageRule of val) {
+                let hasSelector = false;
+                logger.trace({ packageRule }, 'packageRule');
+                if (isObject(packageRule)) {
+                  const resolvedRule = await resolveConfigPresets(packageRule);
+                  for (const pKey of Object.keys(resolvedRule)) {
+                    if (selectors.includes(pKey)) {
+                      hasSelector = true;
+                    }
+                  }
+                  if (!hasSelector) {
+                    const message =
+                      'Each packageRule must contain at least one of ' +
+                      JSON.stringify(selectors);
+                    logger.info({ packageRule }, `packageRule warning`);
+                    warnings.push({
+                      depName: 'Configuration Warning',
+                      message,
+                    });
+                  }
+                } else {
+                  errors.push({
+                    depName: 'Configuration Error',
+                    message: 'packageRules must contain JSON objects',
+                  });
+                }
+              }
+            }
           }
         } else if (type === 'string') {
           if (!isString(val)) {
@@ -130,7 +168,7 @@ function validateConfig(config) {
           }
         } else if (type === 'json') {
           if (isObject(val)) {
-            const subValidation = module.exports.validateConfig(val);
+            const subValidation = await module.exports.validateConfig(val);
             warnings = warnings.concat(subValidation.warnings);
             errors = errors.concat(subValidation.errors);
           } else {

lib/workers/global/index.js

@@ -12,7 +12,7 @@ async function start() {
   initLogger();
   try {
     const config = await configParser.parseConfigs(process.env, process.argv);
-    const { warnings, errors } = configValidation.validateConfig(config);
+    const { warnings, errors } = await configValidation.validateConfig(config);
     // istanbul ignore if
     if (warnings.length) {
       logger.warn({ warnings }, 'Found config warnings');

lib/workers/repository/init/config.js

@@ -18,7 +18,7 @@ async function mergeRenovateConfig(config) {
       )
     );
     logger.debug({ renovateJson }, 'mirrorMode config');
-    const migratedConfig = migrateAndValidate(config, renovateJson);
+    const migratedConfig = await migrateAndValidate(config, renovateJson);
     const resolvedConfig = await presets.resolveConfigPresets(migratedConfig);
     if (resolvedConfig.npmrc && resolvedConfig.ignoreNpmrc !== false) {
       resolvedConfig.ignoreNpmrc = true;
@@ -85,7 +85,7 @@ async function mergeRenovateConfig(config) {
     renovateJson = JSON.parse(renovateConfig);
     logger.info({ config: renovateJson }, 'renovate.json config');
   }
-  const migratedConfig = migrateAndValidate(config, renovateJson);
+  const migratedConfig = await migrateAndValidate(config, renovateJson);
   if (migratedConfig.errors.length) {
     const error = new Error('config-validation');
     error.configFile = configFile;

lib/workers/repository/validate.js

@@ -53,7 +53,7 @@ async function validatePrs(config) {
               : parsed;
           if (toValidate) {
             logger.debug({ config: toValidate }, 'Validating config');
-            const { errors } = migrateAndValidate(config, toValidate);
+            const { errors } = await migrateAndValidate(config, toValidate);
             if (errors && errors.length) {
               validations = validations.concat(
                 errors.map(error => ({

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

 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`config/validation validateConfig(config) errors for all types 1`] = `
+Array [
+  Object {
+    "depName": "Configuration Warning",
+    "message": "Each packageRule must contain at least one of [\\"packageNames\\",\\"packagePatterns\\",\\"excludePackageNames\\",\\"excludePackagePatterns\\"]",
+  },
+]
+`;
+
+exports[`config/validation validateConfig(config) errors for all types 2`] = `
 Array [
   Object {
     "depName": "Configuration Error",
@@ -38,6 +47,10 @@ Array [
     "depName": "Configuration Error",
     "message": "Invalid configuration option: \`foo\`",
   },
+  Object {
+    "depName": "Configuration Error",
+    "message": "packageRules must contain JSON objects",
+  },
 ]
 `;
 

test/config/migrate-validate.spec.js

@@ -8,18 +8,18 @@ beforeEach(() => {
 
 describe('config/migrate-validate', () => {
   describe('migrateAndValidate()', () => {
-    it('handles empty', () => {
-      const res = migrateAndValidate(config, {});
+    it('handles empty', async () => {
+      const res = await migrateAndValidate(config, {});
       expect(res).toMatchSnapshot();
     });
-    it('handles migration', () => {
+    it('handles migration', async () => {
       const input = { automerge: 'none' };
-      const res = migrateAndValidate(config, input);
+      const res = await migrateAndValidate(config, input);
       expect(res).toMatchSnapshot();
     });
-    it('handles invalid', () => {
+    it('handles invalid', async () => {
       const input = { foo: 'none' };
-      const res = migrateAndValidate(config, input);
+      const res = await migrateAndValidate(config, input);
       expect(res).toMatchSnapshot();
       expect(res.errors).toHaveLength(1);
     });

test/config/validation.spec.js

@@ -2,7 +2,7 @@ const configValidation = require('../../lib/config/validation.js');
 
 describe('config/validation', () => {
   describe('validateConfig(config)', () => {
-    it('returns nested errors', () => {
+    it('returns nested errors', async () => {
       const config = {
         foo: 1,
         schedule: ['after 5pm'],
@@ -12,12 +12,14 @@ describe('config/validation', () => {
           bar: 2,
         },
       };
-      const { warnings, errors } = configValidation.validateConfig(config);
+      const { warnings, errors } = await configValidation.validateConfig(
+        config
+      );
       expect(warnings).toHaveLength(0);
       expect(errors).toHaveLength(2);
       expect(errors).toMatchSnapshot();
     });
-    it('errors for all types', () => {
+    it('errors for all types', async () => {
       const config = {
         allowedVersions: 'foo',
         enabled: 1,
@@ -28,15 +30,23 @@ describe('config/validation', () => {
         lockFileMaintenance: false,
         extends: [':timezone(Europe/Brussel)'],
         packageRules: [
+          {
+            excludePackageNames: ['foo'],
+            enabled: true,
+          },
           {
             foo: 1,
           },
+          'what?',
         ],
       };
-      const { warnings, errors } = configValidation.validateConfig(config);
-      expect(warnings).toHaveLength(0);
+      const { warnings, errors } = await configValidation.validateConfig(
+        config
+      );
+      expect(warnings).toMatchSnapshot();
+      expect(warnings).toHaveLength(1);
       expect(errors).toMatchSnapshot();
-      expect(errors).toHaveLength(9);
+      expect(errors).toHaveLength(10);
     });
   });
 });