Skip to content

fix: jsonc parsing in the IDE2 backend #1948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions arduino-ide-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
38 changes: 9 additions & 29 deletions arduino-ide-extension/src/node/arduino-daemon-impl.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand All @@ -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>();
Expand Down Expand Up @@ -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<{
Expand Down
3 changes: 3 additions & 0 deletions arduino-ide-extension/src/node/arduino-ide-backend-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down
53 changes: 53 additions & 0 deletions arduino-ide-extension/src/node/settings-reader.ts
Original file line number Diff line number Diff line change
@@ -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;
}
33 changes: 5 additions & 28 deletions arduino-ide-extension/src/node/sketches-service-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
firstToUpperCase,
startsWithUpperCase,
} from '../common/utils';
import { SettingsReader } from './settings-reader';

const RecentSketches = 'recent-sketches.json';
const DefaultIno = `void setup() {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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') {
Expand Down
45 changes: 45 additions & 0 deletions arduino-ide-extension/src/test/node/settings.reader.test.ts
Original file line number Diff line number Diff line change
@@ -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;
});
});
});
2 changes: 2 additions & 0 deletions arduino-ide-extension/src/test/node/test-bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
}
Expand Down