fix: jsonc parsing in the IDE2 backend

Occurred when `settings.json` contained comments or a trailing comma.

Closes #1945

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Author: Akos Kitta

arduino-ide-extension/package.json

@@ -78,6 +78,7 @@
     "google-protobuf": "^3.20.1",
     "hash.js": "^1.1.7",
     "js-yaml": "^3.13.1",
+    "jsonc-parser": "^2.2.0",
     "just-diff": "^5.1.1",
     "jwt-decode": "^3.1.2",
     "keytar": "7.2.0",

arduino-ide-extension/src/node/arduino-daemon-impl.ts

@@ -1,5 +1,4 @@
 import { join } from 'path';
-import { promises as fs } from 'fs';
 import { inject, injectable, named } from '@theia/core/shared/inversify';
 import { spawn, ChildProcess } from 'child_process';
 import { FileUri } from '@theia/core/lib/node/file-uri';
@@ -16,7 +15,7 @@ import { BackendApplicationContribution } from '@theia/core/lib/node/backend-app
 import { ArduinoDaemon, NotificationServiceServer } from '../common/protocol';
 import { CLI_CONFIG } from './cli-config';
 import { getExecPath } from './exec-util';
-import { ErrnoException } from './utils/errors';
+import { SettingsReader } from './settings-reader';
 
 @injectable()
 export class ArduinoDaemonImpl
@@ -32,6 +31,9 @@ export class ArduinoDaemonImpl
   @inject(NotificationServiceServer)
   private readonly notificationService: NotificationServiceServer;
 
+  @inject(SettingsReader)
+  private readonly settingsReader: SettingsReader;
+
   private readonly toDispose = new DisposableCollection();
   private readonly onDaemonStartedEmitter = new Emitter<string>();
   private readonly onDaemonStoppedEmitter = new Emitter<void>();
@@ -149,34 +151,12 @@ export class ArduinoDaemonImpl
   }
 
   private async debugDaemon(): Promise<boolean> {
-    // Poor man's preferences on the backend. (https://github.com/arduino/arduino-ide/issues/1056#issuecomment-1153975064)
-    const configDirUri = await this.envVariablesServer.getConfigDirUri();
-    const configDirPath = FileUri.fsPath(configDirUri);
-    try {
-      const raw = await fs.readFile(join(configDirPath, 'settings.json'), {
-        encoding: 'utf8',
-      });
-      const json = this.tryParse(raw);
-      if (json) {
-        const value = json['arduino.cli.daemon.debug'];
-        return typeof value === 'boolean' && !!value;
-      }
-      return false;
-    } catch (error) {
-      if (ErrnoException.isENOENT(error)) {
-        return false;
-      }
-      throw error;
-    }
-  }
-
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  private tryParse(raw: string): any | undefined {
-    try {
-      return JSON.parse(raw);
-    } catch {
-      return undefined;
+    const settings = await this.settingsReader.read();
+    if (settings) {
+      const value = settings['arduino.cli.daemon.debug'];
+      return value === true;
     }
+    return false;
   }
 
   protected async spawnDaemonProcess(): Promise<{

arduino-ide-extension/src/node/arduino-ide-backend-module.ts

@@ -118,6 +118,7 @@ import {
   LocalDirectoryPluginDeployerResolverWithFallback,
   PluginDeployer_GH_12064,
 } from './theia/plugin-ext/plugin-deployer';
+import { SettingsReader } from './settings-reader';
 
 export default new ContainerModule((bind, unbind, isBound, rebind) => {
   bind(BackendApplication).toSelf().inSingletonScope();
@@ -403,6 +404,8 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
     .toSelf()
     .inSingletonScope();
   rebind(PluginDeployer).to(PluginDeployer_GH_12064).inSingletonScope();
+
+  bind(SettingsReader).toSelf().inSingletonScope();
 });
 
 function bindChildLogger(bind: interfaces.Bind, name: string): void {

arduino-ide-extension/src/node/settings-reader.ts

@@ -0,0 +1,53 @@
+import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
+import { FileUri } from '@theia/core/lib/node/file-uri';
+import { inject, injectable } from '@theia/core/shared/inversify';
+import { promises as fs } from 'fs';
+import {
+  parse as parseJsonc,
+  ParseError,
+  printParseErrorCode,
+} from 'jsonc-parser';
+import { join } from 'path';
+import { ErrnoException } from './utils/errors';
+
+// Poor man's preferences on the backend. (https://github.com/arduino/arduino-ide/issues/1056#issuecomment-1153975064)
+@injectable()
+export class SettingsReader {
+  @inject(EnvVariablesServer)
+  private readonly envVariableServer: EnvVariablesServer;
+
+  async read(): Promise<Record<string, unknown> | undefined> {
+    const configDirUri = await this.envVariableServer.getConfigDirUri();
+    const configDirPath = FileUri.fsPath(configDirUri);
+    const settingsPath = join(configDirPath, 'settings.json');
+    try {
+      const raw = await fs.readFile(settingsPath, { encoding: 'utf8' });
+      return parse(raw);
+    } catch (err) {
+      if (ErrnoException.isENOENT(err)) {
+        return undefined;
+      }
+    }
+  }
+}
+
+export function parse(raw: string): Record<string, unknown> | undefined {
+  const errors: ParseError[] = [];
+  const settings =
+    parseJsonc(raw, errors, {
+      allowEmptyContent: true,
+      allowTrailingComma: true,
+      disallowComments: false,
+    }) ?? {};
+  if (errors.length) {
+    console.error('Detected JSONC parser errors:');
+    console.error('----- CONTENT START -----');
+    console.error(raw);
+    console.error('----- CONTENT END -----');
+    errors.forEach(({ error, offset }) =>
+      console.error(` - ${printParseErrorCode(error)} at ${offset}`)
+    );
+    return undefined;
+  }
+  return typeof settings === 'object' ? settings : undefined;
+}

arduino-ide-extension/src/node/sketches-service-impl.ts

@@ -43,6 +43,7 @@ import {
   firstToUpperCase,
   startsWithUpperCase,
 } from '../common/utils';
+import { SettingsReader } from './settings-reader';
 
 const RecentSketches = 'recent-sketches.json';
 const DefaultIno = `void setup() {
@@ -86,6 +87,9 @@ export class SketchesServiceImpl
   @inject(IsTempSketch)
   private readonly isTempSketch: IsTempSketch;
 
+  @inject(SettingsReader)
+  private readonly settingsReader: SettingsReader;
+
   async getSketches({ uri }: { uri?: string }): Promise<SketchContainer> {
     const root = await this.root(uri);
     if (!root) {
@@ -631,38 +635,11 @@ export class SketchesServiceImpl
     return crypto.createHash('md5').update(path).digest('hex').toUpperCase();
   }
 
-  // Returns the default.ino from the settings or from default folder.
-  private async readSettings(): Promise<Record<string, unknown> | undefined> {
-    const configDirUri = await this.envVariableServer.getConfigDirUri();
-    const configDirPath = FileUri.fsPath(configDirUri);
-
-    try {
-      const raw = await fs.readFile(join(configDirPath, 'settings.json'), {
-        encoding: 'utf8',
-      });
-
-      return this.tryParse(raw);
-    } catch (err) {
-      if (ErrnoException.isENOENT(err)) {
-        return undefined;
-      }
-      throw err;
-    }
-  }
-
-  private tryParse(raw: string): Record<string, unknown> | undefined {
-    try {
-      return JSON.parse(raw);
-    } catch {
-      return undefined;
-    }
-  }
-
   // Returns the default.ino from the settings or from default folder.
   private async loadInoContent(): Promise<string> {
     if (!this.inoContent) {
       this.inoContent = new Deferred<string>();
-      const settings = await this.readSettings();
+      const settings = await this.settingsReader.read();
       if (settings) {
         const inoBlueprintPath = settings['arduino.sketch.inoBlueprint'];
         if (inoBlueprintPath && typeof inoBlueprintPath === 'string') {

arduino-ide-extension/src/test/node/settings.reader.test.ts

@@ -0,0 +1,45 @@
+import { expect } from 'chai';
+import { parse } from '../../node/settings-reader';
+
+describe('settings-reader', () => {
+  describe('parse', () => {
+    it('should handle comments', () => {
+      const actual = parse(`
+{
+    "alma": "korte",
+    // comment
+    "szilva": false
+}`);
+      expect(actual).to.be.deep.equal({
+        alma: 'korte',
+        szilva: false,
+      });
+    });
+
+    it('should handle trailing comma', () => {
+      const actual = parse(`
+{
+    "alma": "korte",
+    "szilva": 123,
+}`);
+      expect(actual).to.be.deep.equal({
+        alma: 'korte',
+        szilva: 123,
+      });
+    });
+
+    it('should parse empty', () => {
+      const actual = parse('');
+      expect(actual).to.be.deep.equal({});
+    });
+
+    it('should parse to undefined when parse has failed', () => {
+      const actual = parse(`
+{
+    alma:: 'korte'
+    trash
+}`);
+      expect(actual).to.be.undefined;
+    });
+  });
+});

arduino-ide-extension/src/test/node/test-bindings.ts

@@ -51,6 +51,7 @@ import {
   MonitorServiceFactory,
   MonitorServiceFactoryOptions,
 } from '../../node/monitor-service-factory';
+import { SettingsReader } from '../../node/settings-reader';
 import { SketchesServiceImpl } from '../../node/sketches-service-impl';
 import { EnvVariablesServer } from '../../node/theia/env-variables/env-variables-server';
 
@@ -277,6 +278,7 @@ export function createBaseContainer(
     bind(IsTempSketch).toSelf().inSingletonScope();
     bind(SketchesServiceImpl).toSelf().inSingletonScope();
     bind(SketchesService).toService(SketchesServiceImpl);
+    bind(SettingsReader).toSelf().inSingletonScope();
     if (containerCustomizations) {
       containerCustomizations(bind, rebind);
     }