Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions analysis/src/Cli.ml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ API examples:
./rescript-editor-analysis.exe hover src/MyFile.res 10 2
./rescript-editor-analysis.exe references src/MyFile.res 10 2
./rescript-editor-analysis.exe rename src/MyFile.res 10 2 foo
./rescript-editor-analysis.exe diagnosticSyntax src/MyFile.res

Dev-time examples:
./rescript-editor-analysis.exe dump src/MyFile.res src/MyFile2.res
Expand Down Expand Up @@ -60,6 +61,10 @@ Options:

./rescript-editor-analysis.exe format src/MyFile.res

diagnosticSyntax: print to stdout diagnostic for syntax

./rescript-editor-analysis.exe diagnosticSyntax src/MyFile.res

test: run tests specified by special comments in file src/MyFile.res

./rescript-editor-analysis.exe test src/src/MyFile.res
Expand Down Expand Up @@ -88,6 +93,8 @@ let main () =
Commands.codeAction ~path
~pos:(int_of_string line, int_of_string col)
~currentFile ~debug:false
| [_; "diagnosticSyntax"; path;] ->
Commands.diagnosticSyntax ~path
| _ :: "reanalyze" :: _ ->
let len = Array.length Sys.argv in
for i = 1 to len - 2 do
Expand Down
7 changes: 7 additions & 0 deletions analysis/src/Commands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,12 @@ let format ~path =
signature
else ""

let diagnosticSyntax ~path =
print_endline
(match Diagnostics.document_syntax ~path with
| [] -> Protocol.null
| d -> Protocol.array d)

let test ~path =
Uri.stripPath := true;
match Files.readFile path with
Expand Down Expand Up @@ -378,6 +384,7 @@ let test ~path =
Printf.printf "%s\nnewText:\n%s<--here\n%s%s\n"
(Protocol.stringifyRange range)
indent indent newText)))
| "dia" -> diagnosticSyntax ~path
| _ -> ());
print_newline ())
in
Expand Down
26 changes: 26 additions & 0 deletions analysis/src/Diagnostics.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
let document_syntax ~path =
let parse =
Res_driver.parsingEngine.parseImplementation ~forPrinter:false
~filename:path
in
match parse.diagnostics with
| [] -> []
| diagnostics ->
diagnostics
|> List.rev_map (fun diagnostic ->
let _, startline, startcol =
Location.get_pos_info (Res_diagnostics.getStartPos diagnostic)
in
let _, endline, endcol =
Location.get_pos_info (Res_diagnostics.getEndPos diagnostic)
in
Protocol.stringifyDiagnostic
{
range =
{
start = {line = startline; character = startcol};
end_ = {line = endline; character = endcol};
};
message = Res_diagnostics.explain diagnostic;
severity = Error;
})
23 changes: 21 additions & 2 deletions analysis/src/Protocol.ml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@ type completionItem = {
documentation : markupContent option;
}

type hover = string
type location = {uri : string; range : range}
type documentSymbolItem = {name : string; kind : int; location : location}
type renameFile = {oldUri : string; newUri : string}
type textEdit = {range : range; newText : string}

type diagnosticSeverity = Error | Warning | Information | Hint
type diagnostic = {
range : range;
message : string;
severity : diagnosticSeverity;
}

type optionalVersionedTextDocumentIdentifier = {
version : int option;
uri : string;
Expand Down Expand Up @@ -89,7 +95,7 @@ let stringifyRenameFile {oldUri; newUri} =
}|}
(Json.escape oldUri) (Json.escape newUri)

let stringifyTextEdit te =
let stringifyTextEdit (te : textEdit) =
Printf.sprintf {|{
"range": %s,
"newText": "%s"
Expand Down Expand Up @@ -123,3 +129,16 @@ let stringifyCodeAction ca =
Printf.sprintf {|{"title": "%s", "kind": "%s", "edit": %s}|} ca.title
(codeActionKindToString ca.codeActionKind)
(ca.edit |> stringifyCodeActionEdit)

let stringifyDiagnostic d =
Printf.sprintf {|{
"range: %s,
"message": "%s",
"severity": %d,
}|}
(stringifyRange d.range) (Json.escape d.message)
(match d.severity with
| Error -> 1
| Warning -> 2
| Information -> 3
| Hint -> 4)
1 change: 1 addition & 0 deletions analysis/tests/bsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"subdirs": true
}
],
"ignored-dirs": ["src/not_compiled"],
"bsc-flags": ["-w -33-44"],
"bs-dependencies": ["@rescript/react"],
"reason": { "react-jsx": 3 }
Expand Down
5 changes: 5 additions & 0 deletions analysis/tests/src/not_compiled/Diagnostics.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
let = 1 + 1.0
let add = =2
lett a = 2

//^dia
13 changes: 13 additions & 0 deletions analysis/tests/src/not_compiled/expected/Diagnostics.res.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[{
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's the question of what to do with parser recovery, see if this is too noisy. Always possible to report only the first thing.
Something to evaluate after hooking this command to the editor in server.ts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate a little more? It is related to this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So the parser runs in recovery mode, and keeps on parsing after the first syntax error.
You can see this in your test where 3 things are reported for the same issue.
Now, one could just stop after the first report. Which would take care of the noise in this example.
However, it would be nice to report more than one (syntax) error at once, if possible, when these errors happen in different regions.
Since this PR exposes the syntax errors to users, it's part of this PR to evaluate how those errors look like in practice.
Then based on that, decide how to proceed.

Copy link
Collaborator

@cristianoc cristianoc Jun 16, 2022

Choose a reason for hiding this comment

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

It might just be OK the way it is now. But one would need to experiment once this command is hooked up to the extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll test when #453 is finished because @zth will update the vscode client.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another related aspect, that also needs to wait, is about making sure there is no double reporting when the compiler is already reporting the syntax error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

E.g.:
1 introduce syntax error
2 compile
3 edit unrelated part of the file

Then you get a file that is unsaved, with syntax errors, but where the syntax error was reported already.

"range: {"start": {"line": 1, "character": 4}, "end": {"line": 1, "character": 5}},
"message": "I was expecting a name for this let-binding. Example: `let message = \"hello\"`",
"severity": 1,
}, {
"range: {"start": {"line": 2, "character": 9}, "end": {"line": 2, "character": 11}},
"message": "This let-binding misses an expression",
"severity": 1,
}, {
"range: {"start": {"line": 3, "character": 4}, "end": {"line": 3, "character": 6}},
"message": "consecutive statements on a line must be separated by ';' or a newline",
"severity": 1,
}]
9 changes: 9 additions & 0 deletions analysis/tests/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ for file in src/*.{res,resi}; do
fi
done

for file in src/not_compiled/*.res; do
output="$(dirname $file)/expected/$(basename $file).txt"
../rescript-editor-analysis.exe test $file &> $output
# CI. We use LF, and the CI OCaml fork prints CRLF. Convert.
if [ "$RUNNER_OS" == "Windows" ]; then
perl -pi -e 's/\r\n/\n/g' -- $output
fi
done

warningYellow='\033[0;33m'
successGreen='\033[0;32m'
reset='\033[0m'
Expand Down