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

Language server; error location in SyntaxErrors #750

Merged
merged 7 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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 packages/squiggle-lang/bsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"basic": false
}
},
"external-stdlib": "@rescript/std",
"refmt": 3,
"warnings": {
"number": "+A-42-48-9-30-4"
Expand Down
5 changes: 3 additions & 2 deletions packages/squiggle-lang/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@
],
"author": "Quantified Uncertainty Research Institute",
"dependencies": {
"@rescript/std": "^9.1.4",
"@stdlib/stats": "^0.0.13",
"jstat": "^1.9.5",
"lodash": "^4.17.21",
"mathjs": "^10.6.0",
"pdfast": "^0.2.0",
"rescript": "^9.1.4"
"pdfast": "^0.2.0"
},
"devDependencies": {
"@glennsl/rescript-jest": "^0.9.0",
Expand All @@ -58,6 +58,7 @@
"nyc": "^15.1.0",
"peggy": "^2.0.1",
"reanalyze": "^2.23.0",
"rescript": "^9.1.4",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs:

Make sure the version number of bs-platform and @rescript/std match in your package.json to avoid running into runtime issues due to mismatching stdlib assumptions.

Not sure if I should be more paranoid and pin both rescript and @rescript/std to a specific version. dependabot will always update them both simultaneously and we have them pinned in yarn.lock, so it should be ok, but still, it might be bad if they ever get out of sync.

"rescript-fast-check": "^1.1.1",
"ts-jest": "^27.1.4",
"ts-loader": "^9.3.0",
Expand Down
20 changes: 19 additions & 1 deletion packages/squiggle-lang/src/js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
evaluatePartialUsingExternalBindings,
evaluateUsingOptions,
foreignFunctionInterface,
parse as parseRescript,
} from "../rescript/TypescriptInterface.gen";
export {
makeSampleSetDist,
Expand All @@ -31,7 +32,7 @@ import {
convertRawToTypescript,
lambdaValue,
} from "./rescript_interop";
import { result, resultMap, tag, tagged } from "./types";
import { Ok, result, resultMap, tag, tagged } from "./types";
import { Distribution, shape } from "./distribution";

export { Distribution, resultMap, defaultEnvironment };
Expand All @@ -58,6 +59,23 @@ export function run(
return resultMap(res, (x) => createTsExport(x, e));
}

export function parse(
squiggleString: string
): result<null, Extract<errorValue, { tag: "RESyntaxError" }>> {
const maybeExpression = parseRescript(squiggleString);
if (maybeExpression.tag === "Ok") {
return Ok(null); // TODO - return AST
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't seem too important since I was just interested in errors.

} else {
if (
typeof maybeExpression.value !== "object" ||
maybeExpression.value.tag !== "RESyntaxError"
) {
throw new Error("Expected syntax error");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit hacky but I didn't want to force the consumer to check for all error types. Should we rethink error hierarchy so that parse could return only syntax errors?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of options re polymorphic variants to work with subsets of errors.

}
return { tag: "Error", value: maybeExpression.value };
}
}

// Run Partial. A partial is a block of code that doesn't return a value
export function runPartial(
squiggleString: string,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
@gentype.import("peggy") @genType.as("LocationRange")
type location

@genType
type errorValue =
| REArityError(option<string>, int, int) //TODO: Binding a lambda to a variable should record the variable name in lambda for error reporting
Expand All @@ -14,7 +17,7 @@ type errorValue =
| REOperationError(Operation.operationError)
| RERecordPropertyNotFound(string, string)
| RESymbolNotFound(string)
| RESyntaxError(string)
| RESyntaxError(string, option<location>)
| RETodo(string) // To do
| REUnitNotFound(string)

Expand Down Expand Up @@ -50,7 +53,7 @@ let errorToString = err =>
| RENotAFunction(valueString) => `${valueString} is not a function`
| RERecordPropertyNotFound(msg, index) => `${msg}: ${index}`
| RESymbolNotFound(symbolName) => `${symbolName} is not defined`
| RESyntaxError(desc) => `Syntax Error: ${desc}`
| RESyntaxError(desc, _) => `Syntax Error: ${desc}`
Copy link
Collaborator Author

@berekuk berekuk Jun 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should've appended a location here too but I wasn't even sure where errorToString is used and was too busy to check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth it, if you can get around to it.

| RETodo(msg) => `TODO: ${msg}`
| REExpectedType(typeName) => `Expected type: ${typeName}`
| REUnitNotFound(unitName) => `Unit not found: ${unitName}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ let evaluatePartialUsingExternalBindings = (
switch rAnswer {
| Ok(EvRecord(externalBindings)) => Ok(externalBindings)
| Ok(_) =>
Error(Reducer_ErrorValue.RESyntaxError(`Partials must end with an assignment or record`))
Error(Reducer_ErrorValue.RESyntaxError(`Partials must end with an assignment or record`, None))
| Error(err) => err->Error
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ type node = {"type": string}

@module("./Reducer_Peggy_GeneratedParser.js") external parse__: string => node = "parse"

let syntaxErrorToLocation: Js.Exn.t => Reducer_ErrorValue.location = error => %raw(`error.location`)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's probably a more elegant way to do this and I just don't know ReScript well enough. But Js.Ext is not documented well and I gave up after ~1h of googling and trying to cast Js.Ext.t to peggy's SyntaxError with genType...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, the Rescript <> JS interop is pretty awkward and poorly documented. It's generally the worst part of working with Rescript.


@genType
let parse = (expr: string): result<node, errorValue> =>
try {
Ok(parse__(expr))
} catch {
| Js.Exn.Error(obj) => REJavaScriptExn(Js.Exn.message(obj), Js.Exn.name(obj))->Error
| Js.Exn.Error(obj) =>
RESyntaxError(Belt.Option.getExn(Js.Exn.message(obj)), syntaxErrorToLocation(obj)->Some)->Error
}

type nodeBlock = {...node, "statements": array<node>}
Expand Down
3 changes: 3 additions & 0 deletions packages/squiggle-lang/src/rescript/TypescriptInterface.res
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ let evaluate = Reducer.evaluate
@genType
let evaluateUsingOptions = Reducer.evaluateUsingOptions

@genType
let parse = Reducer_Peggy_Parse.parse

@genType
let evaluatePartialUsingExternalBindings = Reducer.evaluatePartialUsingExternalBindings

Expand Down
3 changes: 2 additions & 1 deletion packages/vscode-ext/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/media/vendor
/out
/client/out
/server/out
/*.vsix
/syntaxes/*.json
60 changes: 60 additions & 0 deletions packages/vscode-ext/client/src/client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import * as path from "path";

import * as vscode from "vscode";
import {
LanguageClient,
LanguageClientOptions,
ServerOptions,
TransportKind,
} from "vscode-languageclient/node";

let client: LanguageClient;

export const startClient = (context: vscode.ExtensionContext) => {
// The server is implemented in node
let serverModule = context.asAbsolutePath(
path.join("server", "out", "server.js")
);
// The debug options for the server
// --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging
let debugOptions = { execArgv: ["--nolazy", "--inspect=6009"] };

// If the extension is launched in debug mode then the debug server options are used
// Otherwise the run options are used
let serverOptions: ServerOptions = {
run: { module: serverModule, transport: TransportKind.ipc },
debug: {
module: serverModule,
transport: TransportKind.ipc,
options: debugOptions,
},
};

// Options to control the language client
let clientOptions: LanguageClientOptions = {
// Register the server for plain text documents
documentSelector: [{ scheme: "file", language: "squiggle" }],
synchronize: {
// Notify the server about file changes to '.clientrc files contained in the workspace
fileEvents: vscode.workspace.createFileSystemWatcher("**/.clientrc"),
},
};

// Create the language client and start the client.
client = new LanguageClient(
"squiggleServer",
"Squiggle Server",
serverOptions,
clientOptions
);

// Start the client. This will also launch the server
client.start();
};

export const stopClient = () => {
if (!client) {
return undefined;
}
return client.stop();
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// The module 'vscode' contains the VS Code extensibility API
// Import the module and reference it with the alias vscode in your code below
import * as vscode from "vscode";
import { startClient, stopClient } from "./client";

import { SquiggleEditorProvider } from "./editor";
import { registerPreviewCommand } from "./preview";
Expand All @@ -11,7 +12,11 @@ export function activate(context: vscode.ExtensionContext) {
context.subscriptions.push(SquiggleEditorProvider.register(context));

registerPreviewCommand(context);

startClient(context);
}

// this method is called when your extension is deactivated
export function deactivate() {}
export function deactivate() {
stopClient();
}
12 changes: 12 additions & 0 deletions packages/vscode-ext/client/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es2020",
"lib": ["ES2020", "dom"],
"outDir": "out",
"rootDir": "src",
"sourceMap": true
},
"include": ["src"],
"exclude": ["node_modules", ".vscode-test"]
}
19 changes: 14 additions & 5 deletions packages/vscode-ext/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"displayName": "Squiggle",
"description": "Squiggle language support",
"license": "MIT",
"version": "0.1.2",
"version": "0.2.0",
"publisher": "QURI",
"repository": {
"type": "git",
Expand All @@ -18,10 +18,11 @@
"Visualization"
],
"activationEvents": [
"onLanguage:squiggle",
"onCustomEditor:squiggle.wysiwyg",
"onCommand:squiggle.preview"
],
"main": "./out/extension.js",
"main": "./client/out/extension.js",
"contributes": {
"languages": [
{
Expand Down Expand Up @@ -130,8 +131,9 @@
"compile": "yarn run compile:tsc && yarn run compile:grammar && yarn run compile:vendor",
"watch": "tsc -watch -p ./",
"pretest": "yarn run compile && yarn run lint",
"lint": "eslint src --ext ts",
"format": "eslint src --ext ts --fix"
"lint": "eslint client/src server/src --ext ts",
"format": "eslint client/src server/src --ext ts --fix",
"package": "npx vsce package --yarn"
},
"devDependencies": {
"@types/glob": "^7.2.0",
Expand All @@ -142,6 +144,13 @@
"eslint": "^8.18.0",
"glob": "^8.0.3",
"js-yaml": "^4.1.0",
"typescript": "^4.7.4"
"typescript": "^4.7.4",
"vsce-yarn-patch": "^1.66.2"
Copy link
Collaborator Author

@berekuk berekuk Jun 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for our monorepo compatibility (vsce is a tool for packaging vscode extensions), see microsoft/vscode-vsce#300 (comment).

This is what caused yesterday's issues with broken 0.1.0 and 0.1.1 versions.

Yes, it's a fork. It hasn't been updated in 2 years. Seems like everyone with a monorepo still uses it. Sigh.

},
"dependencies": {
"vscode-languageclient": "^8.0.1",
"vscode-languageserver": "^7.0.0",
"vscode-languageserver-textdocument": "^1.0.5",
"@quri/squiggle-lang": "^0.2.11"
}
}
84 changes: 84 additions & 0 deletions packages/vscode-ext/server/src/server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import {
createConnection,
TextDocuments,
Diagnostic,
DiagnosticSeverity,
ProposedFeatures,
InitializeParams,
TextDocumentSyncKind,
InitializeResult,
} from "vscode-languageserver/node";

import { parse } from "@quri/squiggle-lang";

import { TextDocument } from "vscode-languageserver-textdocument";

// Create a connection for the server, using Node's IPC as a transport.
// Also include all preview / proposed LSP features.
let connection = createConnection(ProposedFeatures.all);

const documents: TextDocuments<TextDocument> = new TextDocuments(TextDocument);

documents.onDidChangeContent((change) => {
validateSquiggleDocument(change.document);
});

let hasDiagnosticRelatedInformationCapability = false;

connection.onInitialize((params: InitializeParams) => {
const capabilities = params.capabilities;

hasDiagnosticRelatedInformationCapability = !!(
capabilities.textDocument &&
capabilities.textDocument.publishDiagnostics &&
capabilities.textDocument.publishDiagnostics.relatedInformation
);

const result: InitializeResult = {
capabilities: {
textDocumentSync: TextDocumentSyncKind.Incremental,
},
};
return result;
});

async function validateSquiggleDocument(
textDocument: TextDocument
): Promise<void> {
// The validator creates diagnostics for all uppercase words length 2 and more
const text = textDocument.getText();
const pattern = /\b[A-Z]{2,}\b/g;
let m: RegExpExecArray | null;

let problems = 0;
const diagnostics: Diagnostic[] = [];

const parseResult = parse(text);
if (parseResult.tag === "Error") {
const location = parseResult.value.value[1];
diagnostics.push({
severity: DiagnosticSeverity.Error,
range: {
start: {
line: location.start.line - 1,
character: location.start.column - 1,
},
end: {
line: location.end.line - 1,
character: location.end.column - 1,
},
},
message: parseResult.value.value[0],
});
}

// Send the computed diagnostics to VSCode.
connection.sendDiagnostics({ uri: textDocument.uri, diagnostics });
}

// Make the text document manager listen on the connection
// for open, change and close text document events
documents.listen(connection);

// Listen on the connection
connection.listen();
12 changes: 12 additions & 0 deletions packages/vscode-ext/server/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es2020",
"lib": ["ES2020", "dom"],
"outDir": "out",
"rootDir": "src",
"sourceMap": true
},
"include": ["src"],
"exclude": ["node_modules"]
}
Loading