Skip to content

Commit fbe45c6

Browse files
committed
Capture parse and source errors and make them redirect comments
1 parent 5cabefa commit fbe45c6

File tree

12 files changed

+189
-30
lines changed

12 files changed

+189
-30
lines changed

src/analyzers/IsolatedAnalyzerImpl.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
import { getProcessLogger as getLogger, Logger } from '~src/utils/logger'
2-
3-
import { IsolatedAnalyzerOutput, EarlyFinalization } from '~src/output/IsolatedAnalyzerOutput';
1+
import { NoSourceError } from '~src/errors/NoSourceError';
2+
import { ParserError } from '~src/errors/ParserError';
3+
import { EarlyFinalization, IsolatedAnalyzerOutput } from '~src/output/IsolatedAnalyzerOutput';
4+
import { getProcessLogger as getLogger, Logger } from '~src/utils/logger';
5+
import { makeNoSourceOutput } from '~src/output/makeNoSourceOutput';
6+
import { makeParseErrorOutput } from '~src/output/makeParseErrorOutput';
47

58
export abstract class IsolatedAnalyzerImpl implements Analyzer {
69
protected readonly logger: Logger
@@ -23,9 +26,31 @@ export abstract class IsolatedAnalyzerImpl implements Analyzer {
2326
* @memberof BaseAnalyzer
2427
*/
2528
public async run(input: Input): Promise<Output> {
29+
return this.run_(input)
30+
.catch((err: Error) => {
31+
32+
// Here we handle errors that blew up the analyzer but we don't want to
33+
// report as blown up. This converts these errors to the commentary.
34+
if (err instanceof NoSourceError) {
35+
return makeNoSourceOutput(err)
36+
} else if (err instanceof ParserError) {
37+
return makeParseErrorOutput(err)
38+
}
39+
40+
// Unhandled issue
41+
return Promise.reject(err)
42+
})
43+
}
44+
45+
private async run_(input: Input): Promise<Output> {
2646
const output = new IsolatedAnalyzerOutput()
47+
48+
// Block and execute
2749
await this.execute(input, output)
2850
.catch((err): void | never => {
51+
52+
// The isolated analyzer output can use exceptions as control flow.
53+
// This block here explicitely accepts this.
2954
if (err instanceof EarlyFinalization) {
3055
this.logger.log(`=> early finialization (${output.status})`)
3156
} else {

src/analyzers/SourceImpl.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,33 @@ class SourceImpl implements Source {
1313
private readonly lines: string[]
1414

1515
constructor(source: string) {
16-
this.lines = source.split("\n")
16+
this.lines = source.split(/\r?\n/)
1717
}
1818

19-
public get(node: NodeWithLocation): string {
20-
const start = this.lines[node.loc.start.line - 1]
21-
const end = this.lines[node.loc.end.line - 1]
22-
if (start === end) {
23-
return start.substring(node.loc.start.column, node.loc.end.column)
19+
public getLines(node: NodeWithLocation): string[] {
20+
const startLineLoc = Math.max(0, node.loc.start.line - 1)
21+
const start = this.lines[startLineLoc]
22+
const endLineLoc = Math.min(this.lines.length - 1, node.loc.end.line - 1)
23+
24+
if (startLineLoc === endLineLoc) {
25+
return [start.substring(node.loc.start.column, node.loc.end.column)]
2426
}
2527

28+
const end = this.lines[endLineLoc]
29+
2630
return [
2731
start.substring(node.loc.start.column),
28-
...this.lines.slice(node.loc.start.line, node.loc.end.line - 2),
29-
end.substring(0, node.loc.end.column)
30-
].join("\n")
32+
...(
33+
startLineLoc + 1 <= endLineLoc - 1
34+
? this.lines.slice(startLineLoc + 1, endLineLoc - 1)
35+
: []
36+
),
37+
end.substring(0, node.loc.end.column < 0 ? undefined : node.loc.end.column)
38+
]
39+
}
40+
41+
public get(node: NodeWithLocation): string {
42+
return this.getLines(node).join("\n")
3143
}
3244

3345
public getOuter(node: NodeWithLocation): string {

src/analyzers/two-fer/index.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import { NO_METHOD, NO_NAMED_EXPORT, NO_PARAMETER, PREFER_STRICT_EQUALITY, PREFE
2020
import { AstParser, ParsedSource } from "~src/parsers/AstParser";
2121
import { NoSourceError } from "~src/errors/NoSourceError";
2222
import { ParserError } from "~src/errors/ParserError";
23+
import { makeParseErrorOutput } from "~src/output/makeParseErrorOutput";
24+
import { makeNoSourceOutput } from "~src/output/makeNoSourceOutput";
2325

2426
/**
2527
* The factories here SHOULD be kept in sync with exercism/website-copy. Under
@@ -132,15 +134,17 @@ export class TwoFerAnalyzer extends AnalyzerImpl {
132134
try {
133135
return await Parser.parse(input)
134136
} catch (err) {
137+
138+
// Here we handle errors that blew up the analyzer but we don't want to
139+
// report as blown up. This converts these errors to the commentary.
135140
if (err instanceof NoSourceError) {
136-
this.logger.error(`=> [NoSourceError] ${err.message}`)
141+
const output = makeNoSourceOutput(err)
142+
output.comments.forEach((comment) => this.comment(comment))
143+
this.redirect()
144+
} else if (err instanceof ParserError) {
145+
const output = makeParseErrorOutput(err)
146+
output.comments.forEach((comment) => this.comment(comment))
137147
this.redirect()
138-
}
139-
140-
if (err instanceof ParserError) {
141-
this.logger.error(`=> [ParserError] ${err.message}`)
142-
const { message, ...details } = err.original
143-
this.disapprove(PARSE_ERROR({ error: message, details: JSON.stringify(details) }))
144148
}
145149

146150
throw err

src/comments/shared.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,7 @@ this to the student directly. Use it to determine what you want to say.
108108
- If there is no icon, the commentary has not been updated to the latest
109109
standard. Proceed with caution.
110110
`('javascript.generic.beta_disapprove_commentary_prefix')
111+
112+
export const ERROR_CAPTURED_NO_SOURCE = factory<'expected' | 'available'>`
113+
Expected source file "${'expected'}", found: ${'available'}.
114+
`('javascript.generic.error_captured_no_source')

src/errors/NoSourceError.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,19 @@ import { SOURCE_MISSING_ERROR } from "./codes";
22

33
export class NoSourceError extends Error {
44
public readonly code: typeof SOURCE_MISSING_ERROR;
5+
public readonly expected: string;
6+
public readonly available: string[];
7+
8+
constructor(expected?: string, available?: string[]) {
9+
super(
10+
expected
11+
? `Expected source file "${expected}", found: ${JSON.stringify(available)}`
12+
: 'No source file(s) found'
13+
)
14+
15+
this.expected = expected || '<unknown>'
16+
this.available = available || []
517

6-
constructor() {
7-
super('No source file(s) found')
818
Error.captureStackTrace(this, this.constructor)
919

1020
this.code = SOURCE_MISSING_ERROR

src/errors/ParserError.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { SOURCE_PARSE_ERROR } from "./codes";
33
export class ParserError extends Error {
44
public readonly code: typeof SOURCE_PARSE_ERROR;
55

6-
constructor(public readonly original: Error) {
6+
constructor(public readonly original: Error & { lineNumber: number; column: number; index: number }, public readonly source?: string) {
77
super(`
88
Could not parse the source; most likely due to a syntax error.
99
@@ -15,3 +15,4 @@ ${original.message}
1515
this.code = SOURCE_PARSE_ERROR
1616
}
1717
}
18+

src/input/DirectoryInput.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { readDir } from "~src/utils/fs";
22
import { FileInput } from "./FileInput";
33

44
import nodePath from 'path'
5+
import { NoSourceError } from "~src/errors/NoSourceError";
56

67
const EXTENSIONS = /\.(jsx?|tsx?|mjs)$/
78
const TEST_FILES = /\.spec|test\./
@@ -12,8 +13,8 @@ export class DirectoryInput implements Input {
1213

1314
public async read(n = 1): Promise<string[]> {
1415
const files = await readDir(this.path)
15-
1616
const candidates = findCandidates(files, n, `${this.exerciseSlug}.js`)
17+
1718
const fileSources = await Promise.all(
1819
candidates.map((candidate): Promise<string> => {
1920
return new FileInput(nodePath.join(this.path, candidate))
@@ -24,6 +25,13 @@ export class DirectoryInput implements Input {
2425

2526
return fileSources
2627
}
28+
29+
public async informativeBail(): Promise<never> {
30+
const expected = `${this.exerciseSlug}.js`
31+
const available = await readDir(this.path)
32+
33+
return Promise.reject(new NoSourceError(expected, available))
34+
}
2735
}
2836

2937
/**

src/input/FileInput.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,8 @@ export class FileInput implements Input {
77
const buffer = await readFile(this.path)
88
return [buffer.toString("utf8")]
99
}
10+
11+
public async informativeBail(): Promise<never> {
12+
return Promise.reject(new Error(`Could not read file "${this.path}"`))
13+
}
1014
}

src/interface.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ interface Input {
3333
* @returns at most `n` strings
3434
*/
3535
read(n?: number): Promise<string[]>;
36+
37+
informativeBail(): Promise<never>;
3638
}
3739

3840

src/output/makeNoSourceOutput.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { NoSourceError } from "~src/errors/NoSourceError"
2+
import { AnalyzerOutput } from "./AnalyzerOutput"
3+
import { ERROR_CAPTURED_NO_SOURCE } from "~src/comments/shared"
4+
5+
/**
6+
* Makes a generic output based on a NoSourceError
7+
*
8+
* @export
9+
* @param {NoSourceError} err
10+
* @returns {Output}
11+
*/
12+
export function makeNoSourceOutput(err: NoSourceError): Output {
13+
const output = new AnalyzerOutput()
14+
15+
output.add(ERROR_CAPTURED_NO_SOURCE({
16+
expected: err.expected,
17+
available: err.available.join(', ')
18+
}))
19+
20+
output.redirect()
21+
process.exitCode = err.code
22+
23+
return output
24+
}

0 commit comments

Comments
 (0)