Capture parse and source errors and make them redirect comments

src/analyzers/IsolatedAnalyzerImpl.ts

@@ -1,6 +1,9 @@
-import { getProcessLogger as getLogger, Logger } from '~src/utils/logger'
-
-import { IsolatedAnalyzerOutput, EarlyFinalization } from '~src/output/IsolatedAnalyzerOutput';
+import { NoSourceError } from '~src/errors/NoSourceError';
+import { ParserError } from '~src/errors/ParserError';
+import { EarlyFinalization, IsolatedAnalyzerOutput } from '~src/output/IsolatedAnalyzerOutput';
+import { getProcessLogger as getLogger, Logger } from '~src/utils/logger';
+import { makeNoSourceOutput } from '~src/output/makeNoSourceOutput';
+import { makeParseErrorOutput } from '~src/output/makeParseErrorOutput';
 
 export abstract class IsolatedAnalyzerImpl implements Analyzer {
   protected readonly logger: Logger
@@ -23,9 +26,31 @@ export abstract class IsolatedAnalyzerImpl implements Analyzer {
    * @memberof BaseAnalyzer
    */
   public async run(input: Input): Promise<Output> {
+    return this.run_(input)
+      .catch((err: Error) => {
+
+        // Here we handle errors that blew up the analyzer but we don't want to
+        // report as blown up. This converts these errors to the commentary.
+        if (err instanceof NoSourceError) {
+          return makeNoSourceOutput(err)
+        } else if (err instanceof ParserError) {
+          return makeParseErrorOutput(err)
+        }
+
+        // Unhandled issue
+        return Promise.reject(err)
+      })
+  }
+
+  private async run_(input: Input): Promise<Output> {
     const output = new IsolatedAnalyzerOutput()
+
+    // Block and execute
     await this.execute(input, output)
       .catch((err): void | never => {
+
+        // The isolated analyzer output can use exceptions as control flow.
+        // This block here explicitely accepts this.
         if (err instanceof EarlyFinalization) {
           this.logger.log(`=> early finialization (${output.status})`)
         } else {

src/analyzers/SourceImpl.ts

@@ -13,21 +13,33 @@ class SourceImpl implements Source {
   private readonly lines: string[]
 
   constructor(source: string) {
-    this.lines = source.split("\n")
+    this.lines = source.split(/\r?\n/)
   }
 
-  public get(node: NodeWithLocation): string {
-    const start = this.lines[node.loc.start.line - 1]
-    const end = this.lines[node.loc.end.line - 1]
-    if (start === end) {
-      return start.substring(node.loc.start.column, node.loc.end.column)
+  public getLines(node: NodeWithLocation): string[] {
+    const startLineLoc = Math.max(0, node.loc.start.line - 1)
+    const start = this.lines[startLineLoc]
+    const endLineLoc = Math.min(this.lines.length - 1, node.loc.end.line - 1)
+
+    if (startLineLoc === endLineLoc) {
+      return [start.substring(node.loc.start.column, node.loc.end.column)]
     }
 
+    const end = this.lines[endLineLoc]
+
     return [
       start.substring(node.loc.start.column),
-      ...this.lines.slice(node.loc.start.line, node.loc.end.line - 2),
-      end.substring(0, node.loc.end.column)
-    ].join("\n")
+      ...(
+        startLineLoc + 1 <= endLineLoc - 1
+          ? this.lines.slice(startLineLoc + 1, endLineLoc - 1)
+          : []
+      ),
+      end.substring(0, node.loc.end.column < 0 ? undefined : node.loc.end.column)
+    ]
+  }
+
+  public get(node: NodeWithLocation): string {
+    return this.getLines(node).join("\n")
   }
 
   public getOuter(node: NodeWithLocation): string {

src/analyzers/two-fer/index.ts

@@ -20,6 +20,8 @@ import { NO_METHOD, NO_NAMED_EXPORT, NO_PARAMETER, PREFER_STRICT_EQUALITY, PREFE
 import { AstParser, ParsedSource } from "~src/parsers/AstParser";
 import { NoSourceError } from "~src/errors/NoSourceError";
 import { ParserError } from "~src/errors/ParserError";
+import { makeParseErrorOutput } from "~src/output/makeParseErrorOutput";
+import { makeNoSourceOutput } from "~src/output/makeNoSourceOutput";
 
 /**
  * The factories here SHOULD be kept in sync with exercism/website-copy. Under
@@ -132,15 +134,17 @@ export class TwoFerAnalyzer extends AnalyzerImpl {
     try {
       return await Parser.parse(input)
     } catch (err) {
+
+      // Here we handle errors that blew up the analyzer but we don't want to
+      // report as blown up. This converts these errors to the commentary.
       if (err instanceof NoSourceError) {
-        this.logger.error(`=> [NoSourceError] ${err.message}`)
+        const output = makeNoSourceOutput(err)
+        output.comments.forEach((comment) => this.comment(comment))
+        this.redirect()
+      } else if (err instanceof ParserError) {
+        const output = makeParseErrorOutput(err)
+        output.comments.forEach((comment) => this.comment(comment))
         this.redirect()
-      }
-
-      if (err instanceof ParserError) {
-        this.logger.error(`=> [ParserError] ${err.message}`)
-        const { message, ...details } = err.original
-        this.disapprove(PARSE_ERROR({ error: message, details: JSON.stringify(details) }))
       }
 
       throw err

src/comments/shared.ts

@@ -108,3 +108,7 @@ this to the student directly. Use it to determine what you want to say.
 - If there is no icon, the commentary has not been updated to the latest
 standard. Proceed with caution.
 `('javascript.generic.beta_disapprove_commentary_prefix')
+
+export const ERROR_CAPTURED_NO_SOURCE = factory<'expected' | 'available'>`
+Expected source file "${'expected'}", found: ${'available'}.
+`('javascript.generic.error_captured_no_source')

src/errors/NoSourceError.ts

@@ -2,9 +2,19 @@ import { SOURCE_MISSING_ERROR } from "./codes";
 
 export class NoSourceError extends Error {
   public readonly code: typeof SOURCE_MISSING_ERROR;
+  public readonly expected: string;
+  public readonly available: string[];
+
+  constructor(expected?: string, available?: string[]) {
+    super(
+      expected
+        ? `Expected source file "${expected}", found: ${JSON.stringify(available)}`
+        : 'No source file(s) found'
+    )
+
+    this.expected = expected || '<unknown>'
+    this.available = available || []
 
-  constructor() {
-    super('No source file(s) found')
     Error.captureStackTrace(this, this.constructor)
 
     this.code = SOURCE_MISSING_ERROR

src/errors/ParserError.ts

@@ -3,7 +3,7 @@ import { SOURCE_PARSE_ERROR } from "./codes";
 export class ParserError extends Error {
   public readonly code: typeof SOURCE_PARSE_ERROR;
 
-  constructor(public readonly original: Error) {
+  constructor(public readonly original: Error & { lineNumber: number; column: number; index: number }, public readonly source?: string) {
     super(`
 Could not parse the source; most likely due to a syntax error.
 
@@ -15,3 +15,4 @@ ${original.message}
     this.code = SOURCE_PARSE_ERROR
   }
 }
+

src/input/DirectoryInput.ts

@@ -2,6 +2,7 @@ import { readDir } from "~src/utils/fs";
 import { FileInput } from "./FileInput";
 
 import nodePath from 'path'
+import { NoSourceError } from "~src/errors/NoSourceError";
 
 const EXTENSIONS = /\.(jsx?|tsx?|mjs)$/
 const TEST_FILES = /\.spec|test\./
@@ -12,8 +13,8 @@ export class DirectoryInput implements Input {
 
   public async read(n = 1): Promise<string[]> {
     const files = await readDir(this.path)
-
     const candidates = findCandidates(files, n, `${this.exerciseSlug}.js`)
+
     const fileSources = await Promise.all(
       candidates.map((candidate): Promise<string> => {
         return new FileInput(nodePath.join(this.path, candidate))
@@ -24,6 +25,13 @@ export class DirectoryInput implements Input {
 
     return fileSources
   }
+
+  public async informativeBail(): Promise<never> {
+    const expected = `${this.exerciseSlug}.js`
+    const available = await readDir(this.path)
+
+    return Promise.reject(new NoSourceError(expected, available))
+  }
 }
 
 /**

src/input/FileInput.ts

@@ -7,4 +7,8 @@ export class FileInput implements Input {
     const buffer = await readFile(this.path)
     return [buffer.toString("utf8")]
   }
+
+  public async informativeBail(): Promise<never> {
+    return Promise.reject(new Error(`Could not read file "${this.path}"`))
+  }
 }

src/interface.d.ts

@@ -33,6 +33,8 @@ interface Input {
    * @returns at most `n` strings
    */
   read(n?: number): Promise<string[]>;
+
+  informativeBail(): Promise<never>;
 }
 
 

src/output/makeNoSourceOutput.ts

@@ -0,0 +1,24 @@
+import { NoSourceError } from "~src/errors/NoSourceError"
+import { AnalyzerOutput } from "./AnalyzerOutput"
+import { ERROR_CAPTURED_NO_SOURCE } from "~src/comments/shared"
+
+/**
+ * Makes a generic output based on a NoSourceError
+ *
+ * @export
+ * @param {NoSourceError} err
+ * @returns {Output}
+ */
+export function makeNoSourceOutput(err: NoSourceError): Output {
+  const output = new AnalyzerOutput()
+
+  output.add(ERROR_CAPTURED_NO_SOURCE({
+    expected: err.expected,
+    available: err.available.join(', ')
+  }))
+
+  output.redirect()
+  process.exitCode = err.code
+
+  return output
+}

src/output/makeParseErrorOutput.ts

@@ -0,0 +1,61 @@
+import { AnalyzerOutput } from "./AnalyzerOutput"
+import { ParserError } from "~src/errors/ParserError"
+import { Source } from "~src/analyzers/SourceImpl"
+import { AST_NODE_TYPES } from "@typescript-eslint/typescript-estree"
+import { PARSE_ERROR } from "~src/comments/shared"
+
+/**
+ * Makes a generic output, based on a ParserError
+ *
+ * @export
+ * @param {ParserError} err
+ * @returns {Output}
+ */
+export function makeParseErrorOutput(err: ParserError): Output {
+  const output = new AnalyzerOutput()
+
+  const { message, ...details } = err.original
+  const source = new Source(err.source || '')
+
+  const startLine = details.lineNumber - 2
+  const endLine = details.lineNumber + 3 /* last line might be empty */
+
+  // Select all the source code until a few lines after the error. The end line
+  // is denoted by endline. The rest of the options fakes a "parsed source".
+  //
+  const surroundingSource = source.getLines({
+    loc: {
+      start: { line: 0, column: 0 },
+      end: { line: endLine, column: -1 }
+    },
+    sourceType: 'module',
+    body: [],
+    range: [0, Infinity],
+    type: AST_NODE_TYPES.Program
+  })
+
+  // Insert the marker BELOW, where the parse error occurred
+  //           -------^
+  //
+  surroundingSource.splice(
+    details.lineNumber,
+    0,
+    '^'.padStart(details.column + 1, '-')
+  )
+
+  // Create the error message, but only show few lines before ... after the
+  // parse error location. These are denoted by startLine (and the source
+  // array was already limited to endLine).
+  //
+  output.add(PARSE_ERROR({
+    error: message,
+    details: surroundingSource
+      .slice(Math.max(0, startLine - 1))
+      .join('\n')
+  }))
+
+  output.redirect()
+  process.exitCode = err.code
+
+  return output
+}

src/parsers/AstParser.ts

@@ -1,5 +1,4 @@
 import { parse as parseToTree, TSESTree, TSESTreeOptions } from "@typescript-eslint/typescript-estree";
-import { NoSourceError } from '~src/errors/NoSourceError';
 import { ParserError } from "~src/errors/ParserError";
 import { getProcessLogger } from "~src/utils/logger";
 
@@ -28,13 +27,18 @@ export class AstParser {
     sources.forEach((source): void => logger.log(`\n${source}\n`))
 
     if (sources.length === 0) {
-      throw new NoSourceError()
+      await input.informativeBail()
     }
 
-    try {
-      return sources.map((source): ParsedSource => new ParsedSource(parseToTree(source, this.options), source))
-    } catch(error) {
-      throw new ParserError(error)
-    }
+    return sources.map((source) => parseSource(source, this.options))
+  }
+}
+
+function parseSource(source: string, options?: TSESTreeOptions): ParsedSource {
+  try {
+    const program = parseToTree(source, options)
+    return new ParsedSource(program, source)
+  } catch (error) {
+    throw new ParserError(error, source)
   }
 }