fix: additional logging for compiler error parsing

Ref: https://forum.arduino.cc/t/how-to-enable-compile-error-highlighting/1172773/ Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
arduino · Oct 12, 2023 · a9d1277 · a9d1277
1 parent a8e63c8
commit a9d1277
Show file tree

Hide file tree

Showing 7 changed files with 253 additions and 56 deletions.
diff --git a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts
@@ -462,6 +462,13 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
     })
     .inSingletonScope()
     .whenTargetNamed('store');
+  bind(ILogger)
+    .toDynamicValue((ctx) => {
+      const parentLogger = ctx.container.get<ILogger>(ILogger);
+      return parentLogger.child('compiler-errors');
+    })
+    .inSingletonScope()
+    .whenTargetNamed('compiler-errors');
 
   // Boards auto-installer
   bind(BoardsAutoInstaller).toSelf().inSingletonScope();

diff --git a/arduino-ide-extension/src/browser/contributions/compiler-errors.ts b/arduino-ide-extension/src/browser/contributions/compiler-errors.ts
@@ -4,16 +4,19 @@ import {
   Disposable,
   DisposableCollection,
   Emitter,
+  ILogger,
   MaybeArray,
   MaybePromise,
   nls,
   notEmpty,
 } from '@theia/core';
-import { ApplicationShell, FrontendApplication } from '@theia/core/lib/browser';
-import { ITextModel } from '@theia/monaco-editor-core/esm/vs/editor/common/model';
+import type {
+  ApplicationShell,
+  FrontendApplication,
+} from '@theia/core/lib/browser';
 import URI from '@theia/core/lib/common/uri';
-import { inject, injectable } from '@theia/core/shared/inversify';
-import {
+import { inject, injectable, named } from '@theia/core/shared/inversify';
+import type {
   Location,
   Range,
 } from '@theia/core/shared/vscode-languageserver-protocol';
@@ -27,7 +30,9 @@ import {
 } from '@theia/editor/lib/browser/decorations/editor-decoration';
 import { EditorManager } from '@theia/editor/lib/browser/editor-manager';
 import * as monaco from '@theia/monaco-editor-core';
+import type { ITextModel } from '@theia/monaco-editor-core/esm/vs/editor/common/model';
 import { MonacoEditor } from '@theia/monaco/lib/browser/monaco-editor';
+import { MonacoEditorModel } from '@theia/monaco/lib/browser/monaco-editor-model';
 import { MonacoToProtocolConverter } from '@theia/monaco/lib/browser/monaco-to-protocol-converter';
 import { ProtocolToMonacoConverter } from '@theia/monaco/lib/browser/protocol-to-monaco-converter';
 import { OutputUri } from '@theia/output/lib/common/output-uri';
@@ -36,7 +41,6 @@ import { ErrorRevealStrategy } from '../arduino-preferences';
 import { ArduinoOutputSelector, InoSelector } from '../selectors';
 import { Contribution } from './contribution';
 import { CoreErrorHandler } from './core-error-handler';
-import { MonacoEditorModel } from '@theia/monaco/lib/browser/monaco-editor-model';
 
 interface ErrorDecorationRef {
   /**
@@ -132,15 +136,15 @@ export class CompilerErrors
   extends Contribution
   implements monaco.languages.CodeLensProvider, monaco.languages.LinkProvider
 {
+  @inject(ILogger)
+  @named('compiler-errors')
+  private readonly errorLogger: ILogger;
   @inject(EditorManager)
   private readonly editorManager: EditorManager;
-
   @inject(ProtocolToMonacoConverter)
   private readonly p2m: ProtocolToMonacoConverter;
-
   @inject(MonacoToProtocolConverter)
   private readonly m2p: MonacoToProtocolConverter;
-
   @inject(CoreErrorHandler)
   private readonly coreErrorHandler: CoreErrorHandler;
 
@@ -408,11 +412,16 @@ export class CompilerErrors
   private async handleCompilerErrorsDidChange(
     errors: CoreError.ErrorLocation[]
   ): Promise<void> {
-    this.toDisposeOnCompilerErrorDidChange.dispose();
+    this.errorLogger.info(
+      `Handling new compiler error locations: ${JSON.stringify(errors)}`
+    );
     const groupedErrors = this.groupBy(
       errors,
       (error: CoreError.ErrorLocation) => error.location.uri
     );
+    this.errorLogger.info(
+      `Grouped error locations: ${JSON.stringify([...groupedErrors.entries()])}`
+    );
     const decorations = await this.decorateEditors(groupedErrors);
     this.errors.push(...decorations.errors);
     this.toDisposeOnCompilerErrorDidChange.pushAll([

diff --git a/arduino-ide-extension/src/browser/contributions/core-error-handler.ts b/arduino-ide-extension/src/browser/contributions/core-error-handler.ts
@@ -1,25 +1,39 @@
-import { Emitter, Event } from '@theia/core';
-import { injectable } from '@theia/core/shared/inversify';
+import { Emitter, Event, ILogger } from '@theia/core';
+import { inject, injectable, named } from '@theia/core/shared/inversify';
 import { CoreError } from '../../common/protocol/core-service';
 
 @injectable()
 export class CoreErrorHandler {
+  @inject(ILogger)
+  @named('compiler-errors')
+  private readonly errorsLogger: ILogger;
+
   private readonly errors: CoreError.ErrorLocation[] = [];
   private readonly compilerErrorsDidChangeEmitter = new Emitter<
     CoreError.ErrorLocation[]
   >();
 
   tryHandle(error: unknown): void {
-    if (CoreError.is(error)) {
-      this.errors.length = 0;
-      this.errors.push(...error.data);
-      this.fireCompilerErrorsDidChange();
+    this.errorsLogger.info('Handling compiler errors...');
+    if (!CoreError.is(error)) {
+      this.errorsLogger.info(
+        `Handling compiler errors. Skipped. Unknown errors: ${JSON.stringify(
+          error
+        )}`
+      );
+      return;
     }
+    this.errors.length = 0;
+    this.errors.push(...error.data);
+    this.fireCompilerErrorsDidChange();
+    this.errorsLogger.info('Handling compiler errors. Done.');
   }
 
   reset(): void {
+    this.errorsLogger.info('Invalidating errors...');
     this.errors.length = 0;
     this.fireCompilerErrorsDidChange();
+    this.errorsLogger.info('Invalidating errors. Done.');
   }
 
   get onCompilerErrorsDidChange(): Event<CoreError.ErrorLocation[]> {

diff --git a/arduino-ide-extension/src/node/arduino-ide-backend-module.ts b/arduino-ide-extension/src/node/arduino-ide-backend-module.ts
@@ -345,6 +345,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
     'discovery-log', // Boards discovery
     'config', // Logger for the CLI config reading and manipulation
     'sketches-service', // For creating, loading, and cloning sketches
+    'compiler-errors', // For parsing compilation errors and highlighting the error locations in the editors
     MonitorManagerName, // Logger for the monitor manager and its services
     MonitorServiceName,
   ].forEach((name) => bindChildLogger(bind, name));

diff --git a/arduino-ide-extension/src/node/cli-error-parser.ts b/arduino-ide-extension/src/node/cli-error-parser.ts
@@ -1,16 +1,18 @@
-import { notEmpty } from '@theia/core/lib/common/objects';
+import { ILogger } from '@theia/core/lib/common/logger';
 import { nls } from '@theia/core/lib/common/nls';
+import { notEmpty } from '@theia/core/lib/common/objects';
 import { FileUri } from '@theia/core/lib/node/file-uri';
 import {
-  Range,
   Position,
+  Range,
 } from '@theia/core/shared/vscode-languageserver-protocol';
 import { CoreError } from '../common/protocol';
 import { Sketch } from '../common/protocol/sketches-service';
 
 export interface OutputSource {
   readonly content: string | ReadonlyArray<Uint8Array>;
   readonly sketch?: Sketch;
+  readonly logger?: ILogger;
 }
 export namespace OutputSource {
   export function content(source: OutputSource): string {
@@ -22,26 +24,33 @@ export namespace OutputSource {
 }
 
 export function tryParseError(source: OutputSource): CoreError.ErrorLocation[] {
-  const { sketch } = source;
-  const content = OutputSource.content(source);
-  if (sketch) {
-    return tryParse(content)
-      .map(remapErrorMessages)
-      .filter(isLocationInSketch(sketch))
-      .map(toErrorInfo)
-      .reduce((acc, curr) => {
-        const existingRef = acc.find((candidate) =>
-          CoreError.ErrorLocationRef.equals(candidate, curr)
-        );
-        if (existingRef) {
-          existingRef.rangesInOutput.push(...curr.rangesInOutput);
-        } else {
-          acc.push(curr);
-        }
-        return acc;
-      }, [] as CoreError.ErrorLocation[]);
+  const { sketch, logger } = source;
+  if (!sketch) {
+    logger?.info('Could not parse the compiler errors. Sketch not found.');
+    return [];
   }
-  return [];
+  const content = OutputSource.content(source);
+  logger?.info(`Parsing compiler errors. Sketch: ${sketch.uri.toString()}`);
+  logger?.info('----START----');
+  logger?.info(content);
+  logger?.info('----END----');
+  const locations = tryParse({ content, logger })
+    .map(remapErrorMessages)
+    .filter(isLocationInSketch(sketch))
+    .map(toErrorInfo)
+    .reduce((acc, curr) => {
+      const existingRef = acc.find((candidate) =>
+        CoreError.ErrorLocationRef.equals(candidate, curr)
+      );
+      if (existingRef) {
+        existingRef.rangesInOutput.push(...curr.rangesInOutput);
+      } else {
+        acc.push(curr);
+      }
+      return acc;
+    }, [] as CoreError.ErrorLocation[]);
+  logger?.info(`Parsed error locations: ${JSON.stringify(locations)}`);
+  return locations;
 }
 
 interface ParseResult {
@@ -105,21 +114,27 @@ function range(line: number, column?: number): Range {
   };
 }
 
-function tryParse(content: string): ParseResult[] {
+function tryParse({
+  content,
+  logger,
+}: {
+  content: string;
+  logger?: ILogger;
+}): ParseResult[] {
   // Shamelessly stolen from the Java IDE: https://github.com/arduino/Arduino/blob/43b0818f7fa8073301db1b80ac832b7b7596b828/arduino-core/src/cc/arduino/Compiler.java#L137
   const re = new RegExp(
     '(.+\\.\\w+):(\\d+)(:\\d+)*:\\s*((fatal)?\\s*error:\\s*)(.*)\\s*',
     'gm'
   );
   return Array.from(content.matchAll(re) ?? [])
     .map((match) => {
-      const { index: start } = match;
+      const { index: startIndex } = match;
       const [, path, rawLine, rawColumn, errorPrefix, , error] = match.map(
         (match) => (match ? match.trim() : match)
       );
       const line = Number.parseInt(rawLine, 10);
       if (!Number.isInteger(line)) {
-        console.warn(
+        logger?.warn(
           `Could not parse line number. Raw input: <${rawLine}>, parsed integer: <${line}>.`
         );
         return undefined;
@@ -129,16 +144,17 @@ function tryParse(content: string): ParseResult[] {
         const normalizedRawColumn = rawColumn.slice(-1); // trims the leading colon => `:3` will be `3`
         column = Number.parseInt(normalizedRawColumn, 10);
         if (!Number.isInteger(column)) {
-          console.warn(
+          logger?.warn(
             `Could not parse column number. Raw input: <${normalizedRawColumn}>, parsed integer: <${column}>.`
           );
         }
       }
-      const rangeInOutput = findRangeInOutput(
-        start,
-        { path, rawLine, rawColumn },
-        content
-      );
+      const rangeInOutput = findRangeInOutput({
+        startIndex,
+        groups: { path, rawLine, rawColumn },
+        content,
+        logger,
+      });
       return {
         path,
         line,
@@ -182,13 +198,16 @@ const KnownErrors: Record<string, { error: string; message?: string }> = {
     ),
   },
 };
-
-function findRangeInOutput(
-  startIndex: number | undefined,
-  groups: { path: string; rawLine: string; rawColumn: string | null },
-  content: string // TODO? lines: string[]? can this code break line on `\n`? const lines = content.split(/\r?\n/) ?? [];
-): Range | undefined {
+interface FindRangeInOutputParams {
+  readonly startIndex: number | undefined;
+  readonly groups: { path: string; rawLine: string; rawColumn: string | null };
+  readonly content: string; // TODO? lines: string[]? can this code break line on `\n`? const lines = content.split(/\r?\n/) ?? [];
+  readonly logger?: ILogger;
+}
+function findRangeInOutput(params: FindRangeInOutputParams): Range | undefined {
+  const { startIndex, groups, content, logger } = params;
   if (startIndex === undefined) {
+    logger?.warn("No 'startIndex'. Skipping");
     return undefined;
   }
   // /path/to/location/Sketch/Sketch.ino:36:42
@@ -199,10 +218,18 @@ function findRangeInOutput(
     (groups.rawColumn ? groups.rawColumn.length : 0);
   const start = toPosition(startIndex, content);
   if (!start) {
+    logger?.warn(
+      `Could not resolve 'start'. Skipping. 'startIndex': ${startIndex}, 'content': ${content}`
+    );
     return undefined;
   }
   const end = toPosition(startIndex + offset, content);
   if (!end) {
+    logger?.warn(
+      `Could not resolve 'end'. Skipping. 'startIndex': ${startIndex}, 'offset': ${
+        startIndex + offset
+      }, 'content': ${content}`
+    );
     return undefined;
   }
   return { start, end };

diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts
@@ -1,5 +1,5 @@
 import { FileUri } from '@theia/core/lib/node/file-uri';
-import { inject, injectable } from '@theia/core/shared/inversify';
+import { inject, injectable, named } from '@theia/core/shared/inversify';
 import { relative } from 'node:path';
 import * as jspb from 'google-protobuf';
 import { BoolValue } from 'google-protobuf/google/protobuf/wrappers_pb';
@@ -35,7 +35,13 @@ import {
 } from '../common/protocol';
 import { ArduinoCoreServiceClient } from './cli-protocol/cc/arduino/cli/commands/v1/commands_grpc_pb';
 import { Port as RpcPort } from './cli-protocol/cc/arduino/cli/commands/v1/port_pb';
-import { ApplicationError, CommandService, Disposable, nls } from '@theia/core';
+import {
+  ApplicationError,
+  CommandService,
+  Disposable,
+  ILogger,
+  nls,
+} from '@theia/core';
 import { MonitorManager } from './monitor-manager';
 import { AutoFlushingBuffer } from './utils/buffers';
 import { tryParseError } from './cli-error-parser';
@@ -63,6 +69,9 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
   private readonly commandService: CommandService;
   @inject(BoardDiscovery)
   private readonly boardDiscovery: BoardDiscovery;
+  @inject(ILogger)
+  @named('compiler-errors')
+  private readonly errorsLogger: ILogger;
 
   async compile(options: CoreService.Options.Compile): Promise<void> {
     const coreClient = await this.coreClient;
@@ -91,6 +100,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
             const compilerErrors = tryParseError({
               content: handler.content,
               sketch: options.sketch,
+              logger: this.errorsLogger,
             });
             const message = nls.localize(
               'arduino/compile/error',