feat: predictible json/json5 handling (#24612)

Co-authored-by: Rhys Arkins <rhys@arkins.net>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>

lib/config/presets/gitea/index.ts

@@ -29,7 +29,7 @@ export async function fetchJSONFile(
   }
 
   // TODO: null check #22198
-  return parsePreset(fromBase64(res.content!));
+  return parsePreset(fromBase64(res.content!), fileName);
 }
 
 export function getPresetFromEndpoint(

lib/config/presets/github/index.ts

@@ -34,7 +34,7 @@ export async function fetchJSONFile(
     throw new Error(PRESET_DEP_NOT_FOUND);
   }
 
-  return parsePreset(fromBase64(res.body.content));
+  return parsePreset(fromBase64(res.body.content), fileName);
 }
 
 export function getPresetFromEndpoint(

lib/config/presets/gitlab/index.ts

@@ -52,7 +52,7 @@ export async function fetchJSONFile(
     throw new Error(PRESET_DEP_NOT_FOUND);
   }
 
-  return parsePreset(res.body);
+  return parsePreset(res.body, fileName);
 }
 
 export function getPresetFromEndpoint(

lib/config/presets/local/common.ts

@@ -29,7 +29,7 @@ export async function fetchJSONFile(
     throw new Error(PRESET_DEP_NOT_FOUND);
   }
 
-  return parsePreset(raw);
+  return parsePreset(raw, fileName);
 }
 
 export function getPresetFromEndpoint(

lib/config/presets/util.ts

@@ -1,5 +1,5 @@
-import JSON5 from 'json5';
 import { logger } from '../../logger';
+import { parseJson } from '../../util/common';
 import { regEx } from '../../util/regex';
 import { ensureTrailingSlash } from '../../util/url';
 import type { FetchPresetConfig, Preset } from './types';
@@ -87,9 +87,9 @@ export async function fetchPreset({
   return jsonContent;
 }
 
-export function parsePreset(content: string): Preset {
+export function parsePreset(content: string, fileName: string): Preset {
   try {
-    return JSON5.parse(content);
+    return parseJson(content, fileName) as Preset;
   } catch (err) {
     throw new Error(PRESET_INVALID_JSON);
   }

lib/modules/platform/azure/index.ts

@@ -9,7 +9,6 @@ import {
   GitVersionDescriptor,
   PullRequestStatus,
 } from 'azure-devops-node-api/interfaces/GitInterfaces.js';
-import JSON5 from 'json5';
 import {
   REPOSITORY_ARCHIVED,
   REPOSITORY_EMPTY,
@@ -18,6 +17,7 @@ import {
 import { logger } from '../../../logger';
 import type { BranchStatus } from '../../../types';
 import { ExternalHostError } from '../../../types/errors/external-host-error';
+import { parseJson } from '../../../util/common';
 import * as git from '../../../util/git';
 import * as hostRules from '../../../util/host-rules';
 import { regEx } from '../../../util/regex';
@@ -182,7 +182,7 @@ export async function getJsonFile(
   branchOrTag?: string
 ): Promise<any> {
   const raw = await getRawFile(fileName, repoName, branchOrTag);
-  return raw ? JSON5.parse(raw) : null;
+  return parseJson(raw, fileName);
 }
 
 export async function initRepo({

lib/modules/platform/bitbucket-server/index.ts

@@ -1,5 +1,4 @@
 import { setTimeout } from 'timers/promises';
-import JSON5 from 'json5';
 import type { PartialDeep } from 'type-fest';
 import {
   REPOSITORY_CHANGED,
@@ -9,6 +8,7 @@ import {
 import { logger } from '../../../logger';
 import type { BranchStatus } from '../../../types';
 import type { FileData } from '../../../types/platform/bitbucket-server';
+import { parseJson } from '../../../util/common';
 import * as git from '../../../util/git';
 import { deleteBranch } from '../../../util/git';
 import * as hostRules from '../../../util/host-rules';
@@ -146,8 +146,8 @@ export async function getJsonFile(
   branchOrTag?: string
 ): Promise<any> {
   // TODO #22198
-  const raw = (await getRawFile(fileName, repoName, branchOrTag)) as string;
-  return JSON5.parse(raw);
+  const raw = await getRawFile(fileName, repoName, branchOrTag);
+  return parseJson(raw, fileName);
 }
 
 // Initialize Bitbucket Server by getting base branch

lib/modules/platform/bitbucket/index.ts

@@ -1,9 +1,9 @@
 import URL from 'node:url';
 import is from '@sindresorhus/is';
-import JSON5 from 'json5';
 import { REPOSITORY_NOT_FOUND } from '../../../constants/error-messages';
 import { logger } from '../../../logger';
 import type { BranchStatus } from '../../../types';
+import { parseJson } from '../../../util/common';
 import * as git from '../../../util/git';
 import * as hostRules from '../../../util/host-rules';
 import { BitbucketHttp, setBaseUrl } from '../../../util/http/bitbucket';
@@ -158,8 +158,8 @@ export async function getJsonFile(
   branchOrTag?: string
 ): Promise<any> {
   // TODO #22198
-  const raw = (await getRawFile(fileName, repoName, branchOrTag)) as string;
-  return JSON5.parse(raw);
+  const raw = await getRawFile(fileName, repoName, branchOrTag);
+  return parseJson(raw, fileName);
 }
 
 // Initialize bitbucket by getting base branch and SHA

lib/modules/platform/codecommit/index.ts

@@ -4,8 +4,6 @@ import {
   ListRepositoriesOutput,
   PullRequestStatusEnum,
 } from '@aws-sdk/client-codecommit';
-import JSON5 from 'json5';
-
 import {
   PLATFORM_BAD_CREDENTIALS,
   REPOSITORY_EMPTY,
@@ -14,6 +12,7 @@ import {
 import { logger } from '../../../logger';
 import type { BranchStatus, PrState } from '../../../types';
 import { coerceArray } from '../../../util/array';
+import { parseJson } from '../../../util/common';
 import * as git from '../../../util/git';
 import { regEx } from '../../../util/regex';
 import { sanitize } from '../../../util/sanitize';
@@ -329,7 +328,7 @@ export async function getJsonFile(
   branchOrTag?: string
 ): Promise<any> {
   const raw = await getRawFile(fileName, repoName, branchOrTag);
-  return raw ? JSON5.parse(raw) : null;
+  return parseJson(raw, fileName);
 }
 
 export async function getRawFile(

lib/modules/platform/gitea/index.ts

@@ -1,5 +1,4 @@
 import is from '@sindresorhus/is';
-import JSON5 from 'json5';
 import semver from 'semver';
 import {
   REPOSITORY_ACCESS_FORBIDDEN,
@@ -11,6 +10,7 @@ import {
 } from '../../../constants/error-messages';
 import { logger } from '../../../logger';
 import type { BranchStatus } from '../../../types';
+import { parseJson } from '../../../util/common';
 import * as git from '../../../util/git';
 import { setBaseUrl } from '../../../util/http/gitea';
 import { sanitize } from '../../../util/sanitize';
@@ -256,8 +256,8 @@ const platform: Platform = {
     branchOrTag?: string
   ): Promise<any> {
     // TODO #22198
-    const raw = (await platform.getRawFile(fileName, repoName, branchOrTag))!;
-    return JSON5.parse(raw);
+    const raw = await platform.getRawFile(fileName, repoName, branchOrTag);
+    return parseJson(raw, fileName);
   },
 
   async initRepo({

lib/modules/platform/github/index.spec.ts

@@ -3430,6 +3430,17 @@ describe('modules/platform/github/index', () => {
   });
 
   describe('getJsonFile()', () => {
+    it('returns null', async () => {
+      const scope = httpMock.scope(githubApiHost);
+      initRepoMock(scope, 'some/repo');
+      await github.initRepo({ repository: 'some/repo' });
+      scope.get('/repos/some/repo/contents/file.json').reply(200, {
+        content: '',
+      });
+      const res = await github.getJsonFile('file.json');
+      expect(res).toBeNull();
+    });
+
     it('returns file content', async () => {
       const data = { foo: 'bar' };
       const scope = httpMock.scope(githubApiHost);

lib/modules/platform/github/index.ts

@@ -1,7 +1,6 @@
 import URL from 'node:url';
 import { setTimeout } from 'timers/promises';
 import is from '@sindresorhus/is';
-import JSON5 from 'json5';
 import { DateTime } from 'luxon';
 import semver from 'semver';
 import { GlobalConfig } from '../../../config/global';
@@ -25,6 +24,7 @@ import type { BranchStatus, VulnerabilityAlert } from '../../../types';
 import { ExternalHostError } from '../../../types/errors/external-host-error';
 import { isGithubFineGrainedPersonalAccessToken } from '../../../util/check-token';
 import { coerceToNull } from '../../../util/coerce';
+import { parseJson } from '../../../util/common';
 import * as git from '../../../util/git';
 import { listCommitTree, pushCommitToRenovateRef } from '../../../util/git';
 import type {
@@ -331,9 +331,8 @@ export async function getJsonFile(
   repoName?: string,
   branchOrTag?: string
 ): Promise<any> {
-  // TODO #22198
-  const raw = (await getRawFile(fileName, repoName, branchOrTag)) as string;
-  return JSON5.parse(raw);
+  const raw = await getRawFile(fileName, repoName, branchOrTag);
+  return parseJson(raw, fileName);
 }
 
 export async function listForks(

lib/modules/platform/gitlab/index.spec.ts

@@ -2641,6 +2641,19 @@ These updates have all been created already. Click a checkbox below to force a r
   });
 
   describe('getJsonFile()', () => {
+    it('returns null', async () => {
+      const scope = await initRepo();
+      scope
+        .get(
+          '/api/v4/projects/some%2Frepo/repository/files/dir%2Ffile.json?ref=HEAD'
+        )
+        .reply(200, {
+          content: '',
+        });
+      const res = await gitlab.getJsonFile('dir/file.json');
+      expect(res).toBeNull();
+    });
+
     it('returns file content', async () => {
       const data = { foo: 'bar' };
       const scope = await initRepo();

lib/modules/platform/gitlab/index.ts

@@ -1,7 +1,6 @@
 import URL from 'node:url';
 import { setTimeout } from 'timers/promises';
 import is from '@sindresorhus/is';
-import JSON5 from 'json5';
 import pMap from 'p-map';
 import semver from 'semver';
 import {
@@ -19,6 +18,7 @@ import {
 import { logger } from '../../../logger';
 import type { BranchStatus } from '../../../types';
 import { coerceArray } from '../../../util/array';
+import { parseJson } from '../../../util/common';
 import * as git from '../../../util/git';
 import * as hostRules from '../../../util/host-rules';
 import { setBaseUrl } from '../../../util/http/gitlab';
@@ -231,9 +231,8 @@ export async function getJsonFile(
   repoName?: string,
   branchOrTag?: string
 ): Promise<any> {
-  // TODO #22198
-  const raw = (await getRawFile(fileName, repoName, branchOrTag)) as string;
-  return JSON5.parse(raw);
+  const raw = await getRawFile(fileName, repoName, branchOrTag);
+  return parseJson(raw, fileName);
 }
 
 function getRepoUrl(

lib/util/common.spec.ts

@@ -1,6 +1,33 @@
-import { detectPlatform } from './common';
+import { logger } from '../../test/util';
+import { detectPlatform, parseJson } from './common';
 import * as hostRules from './host-rules';
 
+const validJsonString = `
+{
+  "name": "John Doe",
+  "age": 30,
+  "city": "New York"
+}
+`;
+const invalidJsonString = `
+{
+  "name": "Alice",
+  "age": 25,
+  "city": "Los Angeles",
+  "hobbies": ["Reading", "Running", "Cooking"]
+  "isStudent": true
+}
+`;
+const onlyJson5parsableString = `
+{
+  name: "Bob",
+  age: 35,
+  city: 'San Francisco',
+  // This is a comment
+  "isMarried": false,
+}
+`;
+
 describe('util/common', () => {
   beforeEach(() => hostRules.clear());
 
@@ -60,4 +87,35 @@ describe('util/common', () => {
       expect(detectPlatform('https://f.example.com/chalk/chalk')).toBeNull();
     });
   });
+
+  describe('parseJson', () => {
+    it('returns null', () => {
+      expect(parseJson(null, 'renovate.json')).toBeNull();
+    });
+
+    it('returns parsed json', () => {
+      expect(parseJson(validJsonString, 'renovate.json')).toEqual({
+        name: 'John Doe',
+        age: 30,
+        city: 'New York',
+      });
+    });
+
+    it('throws error for invalid json', () => {
+      expect(() => parseJson(invalidJsonString, 'renovate.json')).toThrow();
+    });
+
+    it('catches and warns if content parsing faield with JSON.parse but not with JSON5.parse', () => {
+      expect(parseJson(onlyJson5parsableString, 'renovate.json')).toEqual({
+        name: 'Bob',
+        age: 35,
+        city: 'San Francisco',
+        isMarried: false,
+      });
+      expect(logger.logger.warn).toHaveBeenCalledWith(
+        { context: 'renovate.json' },
+        'File contents are invalid JSON but parse using JSON5. Support for this will be removed in a future release so please change to a support .json5 file name or ensure correct JSON syntax.'
+      );
+    });
+  });
 });

lib/util/common.ts

@@ -1,9 +1,11 @@
+import JSON5 from 'json5';
 import {
   BITBUCKET_API_USING_HOST_TYPES,
   GITEA_API_USING_HOST_TYPES,
   GITHUB_API_USING_HOST_TYPES,
   GITLAB_API_USING_HOST_TYPES,
 } from '../constants';
+import { logger } from '../logger';
 import * as hostRules from './host-rules';
 import { parseUrl } from './url';
 
@@ -59,3 +61,32 @@ export function detectPlatform(
 
   return null;
 }
+
+export function parseJson(content: string | null, filename: string): unknown {
+  if (!content) {
+    return null;
+  }
+
+  return filename.endsWith('.json5')
+    ? JSON5.parse(content)
+    : parseJsonWithFallback(content, filename);
+}
+
+export function parseJsonWithFallback(
+  content: string,
+  context: string
+): unknown {
+  let parsedJson: unknown;
+
+  try {
+    parsedJson = JSON.parse(content);
+  } catch (err) {
+    parsedJson = JSON5.parse(content);
+    logger.warn(
+      { context },
+      'File contents are invalid JSON but parse using JSON5. Support for this will be removed in a future release so please change to a support .json5 file name or ensure correct JSON syntax.'
+    );
+  }
+
+  return parsedJson;
+}