Skip to content
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

Move linting code to under Stanc, tests #117

Merged
merged 5 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
97 changes: 2 additions & 95 deletions gui/src/app/FileEditor/StanFileEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
} from "react";
import StanCompileResultWindow from "./StanCompileResultWindow";
import useStanc from "../Stanc/useStanc";
import TextEditor, { CodeMarker, ToolbarItem } from "./TextEditor";
import TextEditor, { ToolbarItem } from "./TextEditor";
import compileStanProgram from "../compileStanProgram/compileStanProgram";
import { StancErrors } from "../Stanc/Types";
import { stancErrorsToCodeMarkers } from "../Stanc/Linting";

type Props = {
fileName: string;
Expand Down Expand Up @@ -269,97 +269,4 @@ const stringChecksum = (str: string) => {
return hash;
};

const stancErrorsToCodeMarkers = (stancErrors: StancErrors) => {
const codeMarkers: CodeMarker[] = [];

for (const x of stancErrors.errors || []) {
const marker = stancErrorStringToMarker(x, "error");
if (marker) codeMarkers.push(marker);
}
for (const x of stancErrors.warnings || []) {
const marker = stancErrorStringToMarker(x, "warning");
if (marker) codeMarkers.push(marker);
}

return codeMarkers;
};

const stancErrorStringToMarker = (
x: string,
severity: "error" | "warning",
): CodeMarker | undefined => {
if (!x) return undefined;

// Example: Syntax error in 'main.stan', line 1, column 0 to column 1, parsing error:

let lineNumber: number | undefined = undefined;
let startColumn: number | undefined = undefined;
let endColumn: number | undefined = undefined;

const sections = x.split(",").map((x) => x.trim());
for (const section of sections) {
if (section.startsWith("line ") && lineNumber === undefined) {
lineNumber = parseInt(section.slice("line ".length));
} else if (section.startsWith("column ") && startColumn === undefined) {
const cols = section.slice("column ".length).split(" to ");
startColumn = parseInt(cols[0]);
endColumn =
cols.length > 1
? parseInt(cols[1].slice("column ".length))
: startColumn + 1;
}
}

if (
lineNumber !== undefined &&
startColumn !== undefined &&
endColumn !== undefined
) {
return {
startLineNumber: lineNumber,
startColumn: startColumn + 1,
endLineNumber: lineNumber,
endColumn: endColumn + 1,
message:
severity === "warning" ? getWarningMessage(x) : getErrorMessage(x),
severity,
};
} else {
return undefined;
}
};

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Adapted from https://github.com/WardBrian/vscode-stan-extension
function getWarningMessage(message: string) {
let warning = message.replace(/Warning.*column \d+: /s, "");
warning = warning.replace(/\s+/gs, " ");
warning = warning.trim();
warning = message.includes("included from")
? "Warning in included file:\n" + warning
: warning;
return warning;
}

function getErrorMessage(message: string) {
let error = message;
// cut off code snippet for display
if (message.includes("------\n")) {
error = error.split("------\n")[2];
}
error = error.trim();
error = message.includes("included from")
? "Error in included file:\n" + error
: error;

// only relevant to vscode-stan-extension:
// error = error.includes("given information about")
// ? error +
// "\nConsider updating the includePaths setting of vscode-stan-extension"
// : error;

return error;
}
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

export default StanFileEditor;
11 changes: 1 addition & 10 deletions gui/src/app/FileEditor/TextEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,10 @@ import {
} from "react";
import { highlightJsData } from "./stanLang";
import { Hyperlink, SmallIconButton } from "@fi-sci/misc";
import { CodeMarker } from "../Stanc/Linting";

type Monaco = typeof monaco;

// An interface for passing markers (squiggles) to the editor without depending on monaco types
export type CodeMarker = {
startLineNumber: number;
startColumn: number;
endLineNumber: number;
endColumn: number;
message: string;
severity: "error" | "warning" | "hint" | "info";
};

type Props = {
defaultText?: string;
text: string | undefined;
Expand Down
117 changes: 117 additions & 0 deletions gui/src/app/Stanc/Linting.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { StancErrors } from "./Types";

type Position = {
startLineNumber: number;
startColumn: number;
endLineNumber: number;
endColumn: number;
};

// An interface for passing markers (squiggles) to the editor without depending on monaco types
export type CodeMarker = Position & {
message: string;
severity: "error" | "warning" | "hint" | "info";
};

export const stancErrorsToCodeMarkers = (stancErrors: StancErrors) => {
const codeMarkers: CodeMarker[] = [];

for (const x of stancErrors.errors || []) {
const marker = stancMessageToMarker(x, "error");
if (marker) codeMarkers.push(marker);
}
for (const x of stancErrors.warnings || []) {
const marker = stancMessageToMarker(x, "warning");
if (marker) codeMarkers.push(marker);
}

return codeMarkers;
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
};

const stancMessageToMarker = (
message: string,
severity: "error" | "warning",
): CodeMarker | undefined => {
const position = locationFromMessage(message);
if (position === undefined) return undefined;
const { startLineNumber, startColumn, endLineNumber, endColumn } = position;

return {
startLineNumber,
startColumn,
endLineNumber,
endColumn,
message:
severity === "warning"
? getWarningMessage(message)
: getErrorMessage(message),
severity,
};
};

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Adapted from https://github.com/WardBrian/vscode-stan-extension

const locationFromMessage = (message: string): Position | undefined => {
if (!message) return;
// format is "in 'filename', line (#)), column (#) to (line #,)? column (#)"
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
const start = message.matchAll(/'.*', line (\d+), column (\d+)( to)?/g);
if (!start) {
return undefined;
}
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
// there will be multiple in the case of #included files
const lastMatch = Array.from(start).pop();
if (!lastMatch) {
return undefined;
}

const startLineNumber = parseInt(lastMatch[1]);
const startColumn = parseInt(lastMatch[2]) + 1;

let endLineNumber = startLineNumber;
let endColumn = startColumn;

if (lastMatch[3]) {
// " to" was matched
const end = message.match(/to (line (\d+), )?column (\d+)/);
if (end) {
if (end[1]) {
endLineNumber = parseInt(end[2]);
}
endColumn = parseInt(end[3]) + 1;
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
}
}

return { startLineNumber, startColumn, endLineNumber, endColumn };
};

function getWarningMessage(message: string) {
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
let warning = message.replace(/Warning.*column \d+:/s, "");
warning = warning.replace(/\s+/gs, " ");
warning = warning.trim();
warning = message.includes("included from")
? "Warning in included file:\n" + warning
: warning;
return warning;
}

function getErrorMessage(message: string) {
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
let error = message;
// cut off code snippet for display
if (message.includes("------\n")) {
error = error.split("------\n")[2];
}
error = error.trim();
error = message.includes("included from")
? "Error in included file:\n" + error
: error;

return error;
}
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

export const exportedForTesting = {
locationFromMessage,
getErrorMessage,
getWarningMessage,
};
112 changes: 112 additions & 0 deletions gui/test/app/Stanc/Linting.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import { assert, describe, expect, test } from "vitest";
import { exportedForTesting } from "../../../src/app/Stanc/Linting";

const { locationFromMessage, getWarningMessage, getErrorMessage } =
exportedForTesting;

// all messages actually generated by stanc
const jacobianWarning = `Warning in 'jacobian.stan', line 1, column 12: Variable name 'jacobian' will
be a reserved word starting in Stan 2.38.0. Please rename it!
`;

const multiColumnError = `Semantic error in 'multicol.stan', line 3, column 4 to column 10:
-------------------------------------------------
1: parameters {
2: real y;
3: int x;
^
4: }
5: model {
-------------------------------------------------

(Transformed) Parameters cannot be integers.
`;

const multiLineError = `Semantic error in 'multiline.stan', line 2, column 2 to line 4, column 16:
-------------------------------------------------
1: generated quantities {
2: array[3] int x
^
3: =
4: 10;
-------------------------------------------------

Ill-typed arguments supplied to assignment operator =:
The left hand side has type
array[] int
and the right hand side has type
int
`;

describe("Linting", () => {
describe("position detection", () => {
test("random message returns undefined", () => {
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
const message = "random message";
const position = locationFromMessage(message);
expect(position).toBeUndefined();
});

test("should detect the position of a single line, single column message", () => {
const position = locationFromMessage(jacobianWarning);
expect(position).toBeDefined();
assert(position);
expect(position.startLineNumber).toEqual(position.endLineNumber);
expect(position.startColumn).toEqual(position.endColumn);

expect(position.startLineNumber).toEqual(1);
expect(position.startColumn).toEqual(13); // 0 vs 1 based indexing
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
});

test("should detect the position of a single line, multi-column error", () => {
const position = locationFromMessage(multiColumnError);
expect(position).toBeDefined();
assert(position);
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
expect(position.startLineNumber).toEqual(position.endLineNumber);
expect(position.startColumn).not.toEqual(position.endColumn);

expect(position.startLineNumber).toEqual(3);
expect(position.startColumn).toEqual(5);
expect(position.endColumn).toEqual(11);
});

test("should detect the position of a multi-line, multi-column error", () => {
const position = locationFromMessage(multiLineError);
expect(position).toBeDefined();
assert(position);
WardBrian marked this conversation as resolved.
Show resolved Hide resolved
expect(position.startLineNumber).not.toEqual(position.endLineNumber);
expect(position.startColumn).not.toEqual(position.endColumn);

expect(position.startLineNumber).toEqual(2);
expect(position.startColumn).toEqual(3);
expect(position.endLineNumber).toEqual(4);
expect(position.endColumn).toEqual(17);
});
});

describe("message extraction", () => {
test("should extract warning message without position", () => {
const warningMessage = getWarningMessage(jacobianWarning);
expect(warningMessage).toEqual(
"Variable name 'jacobian' will be a reserved word starting in Stan 2.38.0. Please rename it!",
);
});

test("should extract error message without position or snippet", () => {
const errorMessage = getErrorMessage(multiColumnError);
expect(errorMessage).toEqual(
"(Transformed) Parameters cannot be integers.",
);
});

test("should extract longer error message without position and snippet", () => {
const errorMessage = getErrorMessage(multiLineError);
expect(errorMessage).toEqual(
`Ill-typed arguments supplied to assignment operator =:
The left hand side has type
array[] int
and the right hand side has type
int`,
);
});
});
});