Language server; error location in SyntaxErrors #750

berekuk · 2022-06-21T22:17:41Z

This is not yet released on VS Code marketplace because the extension bundle is huge.

I'll have to wrap the server code in webpack or rollup first. Or repackage squiggle-lang with @rescript/std.

netlify · 2022-06-21T22:17:44Z

✅ Deploy Preview for squiggle-documentation ready!

Name	Link
🔨 Latest commit	`9da6c12`
🔍 Latest deploy log	https://app.netlify.com/sites/squiggle-documentation/deploys/62b42ba62dc0e500088ac492
😎 Deploy Preview	https://deploy-preview-750--squiggle-documentation.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-06-21T22:17:45Z

✅ Deploy Preview for squiggle-components ready!

Name	Link
🔨 Latest commit	`9da6c12`
🔍 Latest deploy log	https://app.netlify.com/sites/squiggle-components/deploys/62b42ba6d3774700083a1b05
😎 Deploy Preview	https://deploy-preview-750--squiggle-components.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

berekuk · 2022-06-21T22:18:07Z

packages/squiggle-lang/src/js/index.ts

+): result<null, Extract<errorValue, { tag: "RESyntaxError" }>> {
+  const maybeExpression = parseRescript(squiggleString);
+  if (maybeExpression.tag === "Ok") {
+    return Ok(null); // TODO - return AST


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

berekuk · 2022-06-21T22:19:52Z

packages/squiggle-lang/src/js/index.ts

+      typeof maybeExpression.value !== "object" ||
+      maybeExpression.value.tag !== "RESyntaxError"
+    ) {
+      throw new Error("Expected syntax error");


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?

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

berekuk · 2022-06-21T22:20:40Z

packages/squiggle-lang/src/rescript/Reducer/Reducer_ErrorValue.res

@@ -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}`


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

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

codecov-commenter · 2022-06-21T22:22:36Z

Codecov Report

Merging #750 (9da6c12) into develop (06d7804) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #750      +/-   ##
===========================================
+ Coverage    52.32%   52.38%   +0.05%     
===========================================
  Files           59       59              
  Lines         3606     3608       +2     
===========================================
+ Hits          1887     1890       +3     
+ Misses        1719     1718       -1

Impacted Files	Coverage Δ
.../Reducer/Reducer_Expression/Reducer_Expression.res	`88.37% <ø> (ø)`
...e-lang/src/rescript/Reducer/Reducer_ErrorValue.res	`65.21% <100.00%> (+4.34%)`	⬆️
...ript/Reducer/Reducer_Peggy/Reducer_Peggy_Parse.res	`95.74% <100.00%> (+0.18%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06d7804...9da6c12. Read the comment docs.

berekuk · 2022-06-21T22:22:43Z

packages/squiggle-lang/src/rescript/Reducer/Reducer_Peggy/Reducer_Peggy_Parse.res

@@ -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'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...

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

berekuk · 2022-06-21T22:25:58Z

packages/vscode-ext/package.json

@@ -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"


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.

berekuk · 2022-06-21T22:50:12Z

packages/squiggle-lang/package.json

@@ -58,6 +58,7 @@
    "nyc": "^15.1.0",
    "peggy": "^2.0.1",
    "reanalyze": "^2.23.0",
+    "rescript": "^9.1.4",


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.

quinn-dougherty

Yep! great stuff. I only looked at the squiggle-lang changes really, and didn't see anything to take issue with.

quinn-dougherty · 2022-06-22T20:22:57Z

packages/squiggle-lang/src/js/index.ts

+      typeof maybeExpression.value !== "object" ||
+      maybeExpression.value.tag !== "RESyntaxError"
+    ) {
+      throw new Error("Expected syntax error");


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

quinn-dougherty · 2022-06-22T20:23:46Z

packages/squiggle-lang/src/rescript/Reducer/Reducer_ErrorValue.res

@@ -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}`


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

OAGr

From what I understand, I'm fine with it. I'm not that worried about the other stuff. I recommend considering @quinn-dougherty 's comments though, of course.

berekuk · 2022-06-23T09:42:48Z

Hmm, actually, should I return an expression (Reducer_Expression.parse) instead of Reducer_Peggy_Parse.parse?

It's a bit more work since I'd have to convert it to typescript values, but exposing raw peggy nodes seems wrong.

Upd: I forgot I don't return any AST :) But in the future it definitely should be Reducer_Expression.parse.

OAGr · 2022-06-23T17:11:08Z

Hmm, actually, should I return an expression (Reducer_Expression.parse) instead of Reducer_Peggy_Parse.parse?

It's a bit more work since I'd have to convert it to typescript values, but exposing raw peggy nodes seems wrong.

Upd: I forgot I don't return any AST :) But in the future it definitely should be Reducer_Expression.parse.

This PR and some other work is changing some of the typescript stuff a bit. Maybe wait a few days to make the change?
#752

OAGr · 2022-06-24T00:08:25Z

Going to merge, feel free to add modifications/adjustments later.

berekuk added 2 commits June 22, 2022 00:34

store location on RESyntaxError; export parse() function

84a0d83

language server for vscode; patched vsce

250d427

berekuk requested review from quinn-dougherty, Hazelfire and OAGr as code owners June 21, 2022 22:17

berekuk commented Jun 21, 2022

View reviewed changes

external stdlib for rescript

840944f

berekuk commented Jun 21, 2022

View reviewed changes

update yarn.lock

0cf5c47

quinn-dougherty reviewed Jun 22, 2022

View reviewed changes

OAGr approved these changes Jun 23, 2022

View reviewed changes

berekuk added 3 commits June 23, 2022 10:35

cleanup unused code; fix tsc builds

8e4fb99

Merge branch 'develop' into language-server

490ea7e

fix compile script

9da6c12

OAGr merged commit 934608d into develop Jun 24, 2022

OAGr deleted the language-server branch June 24, 2022 00:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Language server; error location in SyntaxErrors #750

Language server; error location in SyntaxErrors #750

berekuk commented Jun 21, 2022

netlify bot commented Jun 21, 2022 •

edited

Loading

netlify bot commented Jun 21, 2022 •

edited

Loading

berekuk Jun 21, 2022

berekuk Jun 21, 2022

quinn-dougherty Jun 22, 2022

berekuk Jun 21, 2022 •

edited

Loading

quinn-dougherty Jun 22, 2022

codecov-commenter commented Jun 21, 2022 •

edited

Loading

berekuk Jun 21, 2022

OAGr Jun 21, 2022

berekuk Jun 21, 2022 •

edited

Loading

berekuk Jun 21, 2022

quinn-dougherty left a comment

quinn-dougherty Jun 22, 2022

quinn-dougherty Jun 22, 2022

OAGr left a comment •

edited

Loading

berekuk commented Jun 23, 2022 •

edited

Loading

OAGr commented Jun 23, 2022

OAGr commented Jun 24, 2022

		@@ -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`)

Language server; error location in SyntaxErrors #750

Language server; error location in SyntaxErrors #750

Conversation

berekuk commented Jun 21, 2022

netlify bot commented Jun 21, 2022 • edited Loading

✅ Deploy Preview for squiggle-documentation ready!

netlify bot commented Jun 21, 2022 • edited Loading

✅ Deploy Preview for squiggle-components ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

berekuk Jun 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 21, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

berekuk Jun 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quinn-dougherty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OAGr left a comment • edited Loading

Choose a reason for hiding this comment

berekuk commented Jun 23, 2022 • edited Loading

OAGr commented Jun 23, 2022

OAGr commented Jun 24, 2022

netlify bot commented Jun 21, 2022 •

edited

Loading

netlify bot commented Jun 21, 2022 •

edited

Loading

berekuk Jun 21, 2022 •

edited

Loading

codecov-commenter commented Jun 21, 2022 •

edited

Loading

berekuk Jun 21, 2022 •

edited

Loading

OAGr left a comment •

edited

Loading

berekuk commented Jun 23, 2022 •

edited

Loading