-
Notifications
You must be signed in to change notification settings - Fork 25
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
Changes from all commits
84a0d83
250d427
840944f
0cf5c47
8e4fb99
490ea7e
9da6c12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import { | |
evaluatePartialUsingExternalBindings, | ||
evaluateUsingOptions, | ||
foreignFunctionInterface, | ||
parse as parseRescript, | ||
} from "../rescript/TypescriptInterface.gen"; | ||
export { | ||
makeSampleSetDist, | ||
|
@@ -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 }; | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
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 | ||
|
@@ -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) | ||
|
||
|
@@ -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}` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
/media/vendor | ||
/out | ||
/client/out | ||
/server/out | ||
/*.vsix | ||
/syntaxes/*.json |
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 |
---|---|---|
@@ -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"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
|
@@ -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": [ | ||
{ | ||
|
@@ -124,14 +125,15 @@ | |
}, | ||
"scripts": { | ||
"vscode:prepublish": "yarn run compile", | ||
"compile:tsc": "tsc -p ./", | ||
"compile:tsc": "tsc -b", | ||
"compile:grammar": "js-yaml syntaxes/squiggle.tmLanguage.yaml >syntaxes/squiggle.tmLanguage.json", | ||
"compile:vendor": "(cd ../squiggle-lang && yarn run build) && (cd ../components && yarn run bundle && yarn run build:css) && mkdir -p media/vendor && cp ../components/dist/bundle.js media/vendor/components.js && cp ../components/dist/main.css media/vendor/components.css && cp ../../node_modules/react/umd/react.production.min.js media/vendor/react.js && cp ../../node_modules/react-dom/umd/react-dom.production.min.js media/vendor/react-dom.js && cp ../website/static/img/quri-logo.png media/vendor/icon.png", | ||
"compile": "yarn run compile:tsc && yarn run compile:grammar && yarn run compile:vendor", | ||
"watch": "tsc -watch -p ./", | ||
"compile": "yarn run compile:vendor && yarn run compile:grammar && yarn run compile:tsc", | ||
"watch": "tsc -b -watch", | ||
"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", | ||
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
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> { | ||
const text = textDocument.getText(); | ||
|
||
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(); |
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"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs:
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.