From f75321c7fd912ed310ff18d531cd7b4d1f8eecdf Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 23 Mar 2022 11:40:39 +0100 Subject: [PATCH 01/51] crude, hacky version of code actions --- analysis/src/Cli.ml | 2 ++ analysis/src/Commands.ml | 39 ++++++++++++++++++++++++++++ analysis/src/Protocol.ml | 11 ++++++++ package-lock.json | 1 + server/src/server.ts | 56 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+) diff --git a/analysis/src/Cli.ml b/analysis/src/Cli.ml index dd228cb4b..b780760b2 100644 --- a/analysis/src/Cli.ml +++ b/analysis/src/Cli.ml @@ -75,6 +75,8 @@ let main () = | [_; "documentSymbol"; path] -> Commands.documentSymbol ~path | [_; "hover"; path; line; col] -> Commands.hover ~path ~line:(int_of_string line) ~col:(int_of_string col) + | [_; "codeAction"; path; line; col] -> + Commands.codeAction ~path ~line:(int_of_string line) ~col:(int_of_string col) | [_; "references"; path; line; col] -> Commands.references ~path ~line:(int_of_string line) ~col:(int_of_string col) diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 9e49e2c65..f58866d00 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -54,6 +54,45 @@ let hover ~path ~line ~col = in print_endline result +let codeAction ~path ~line ~col = + let result = + match Cmt.fromPath ~path with + | None -> Protocol.null + | Some ({file} as full) -> ( + match References.getLocItem ~full ~line ~col with + | None -> Protocol.null + | Some locItem -> ( + let isModule = + match locItem.locType with + | SharedTypes.LModule _ | TopLevelModule _ -> true + | TypeDefinition _ | Typed _ | Constant _ -> false + in + let uriLocOpt = References.definitionForLocItem ~full locItem in + let skipZero = + match uriLocOpt with + | None -> false + | Some (_, loc) -> + let isInterface = file.uri |> Uri2.isInterface in + let posIsZero {Lexing.pos_lnum; pos_bol; pos_cnum} = + (not isInterface) && pos_lnum = 1 && pos_cnum - pos_bol = 0 + in + (* Skip if range is all zero, unless it's a module *) + (not isModule) && posIsZero loc.loc_start && posIsZero loc.loc_end + in + if skipZero then Protocol.null + else + match locItem.SharedTypes.locType with + | Typed (n, t, _) -> ( + match Printtyp.tree_of_typexp false t with + | Otyp_constr (Oide_ident "option", _) -> Protocol.stringifyCodeAction {range = Utils.cmtLocToRange locItem.loc; newText = "switch " ^ n ^ " { | None => failWith(\"TODO\") | Some(" ^ n ^ ") => _" ^ n ^ " }"} + | _ -> Protocol.null + ) + | Constant _constant -> + Protocol.stringifyHover {contents = "constant"} + | _ -> Protocol.null)) + in + print_endline result + let definition ~path ~line ~col = let locationOpt = match Cmt.fromPath ~path with diff --git a/analysis/src/Protocol.ml b/analysis/src/Protocol.ml index 4927fd6dd..a32b64912 100644 --- a/analysis/src/Protocol.ml +++ b/analysis/src/Protocol.ml @@ -18,6 +18,8 @@ type location = {uri : string; range : range} type documentSymbolItem = {name : string; kind : int; location : location} +type codeAction = {newText : string; range : range} + type renameFile = {oldUri : string; newUri : string} type textEdit = {range : range; newText : string} @@ -66,6 +68,15 @@ let stringifyCompletionItem c = let stringifyHover h = Printf.sprintf {|{"contents": "%s"}|} (Json.escape h.contents) +let stringifyCodeAction c = + Printf.sprintf + {|{ + "content": "%s", + "range": %s +}|} + (Json.escape c.newText) + (stringifyRange c.range) + let stringifyLocation (h : location) = Printf.sprintf {|{"uri": "%s", "range": %s}|} (Json.escape h.uri) (stringifyRange h.range) diff --git a/package-lock.json b/package-lock.json index 17768abea..a9631684b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5,6 +5,7 @@ "requires": true, "packages": { "": { + "name": "rescript-vscode", "version": "1.2.1", "hasInstallScript": true, "license": "MIT", diff --git a/server/src/server.ts b/server/src/server.ts index 25e915d45..55dd0cbf5 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -425,6 +425,59 @@ function completion(msg: p.RequestMessage) { return response; } +function codeAction(msg: p.RequestMessage): p.ResponseMessage { + let params = msg.params as p.CodeActionParams; + let filePath = fileURLToPath(params.textDocument.uri); + let response = utils.runAnalysisCommand( + filePath, + [ + "codeAction", + filePath, + params.range.start.line, + params.range.start.character, + ], + msg + ); + let result: null | { + content: string; + range: { + start: { line: number; character: number }; + end: { line: number; character: number }; + }; + } = response.result as any; + + let res: v.ResponseMessage = { + jsonrpc: c.jsonrpcVersion, + id: msg.id, + }; + + if (result == null) { + res.result = null; + return res; + } + + let textEdit: v.TextEdit = { newText: result.content, range: result.range }; + + let codeAction: v.CodeAction = { + title: "Unwrap optional", + kind: v.CodeActionKind.RefactorRewrite, + edit: { + documentChanges: [ + { + textDocument: { + version: null, + uri: params.textDocument.uri, + }, + edits: [textEdit], + }, + ], + }, + }; + + res.result = [codeAction]; + return res; +} + function format(msg: p.RequestMessage): Array { // technically, a formatting failure should reply with the error. Sadly // the LSP alert box for these error replies sucks (e.g. doesn't actually @@ -737,6 +790,7 @@ function onMessage(msg: m.Message) { definitionProvider: true, typeDefinitionProvider: true, referencesProvider: true, + codeActionProvider: true, renameProvider: { prepareProvider: true }, // disabled right now until we use the parser to show non-stale symbols per keystroke // documentSymbolProvider: true, @@ -819,6 +873,8 @@ function onMessage(msg: m.Message) { send(completion(msg)); } else if (msg.method === p.SemanticTokensRequest.method) { send(semanticTokens(msg)); + } else if (msg.method === p.CodeActionRequest.method) { + send(codeAction(msg)); } else if (msg.method === p.DocumentFormattingRequest.method) { let responses = format(msg); responses.forEach((response) => send(response)); From 32d944acbcef136e60d1d5df0f900ea63f2e8f23 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 23 Mar 2022 15:58:41 +0100 Subject: [PATCH 02/51] start mapping out the response structure from analysis bin --- analysis/src/CodeActions.ml | 66 +++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 analysis/src/CodeActions.ml diff --git a/analysis/src/CodeActions.ml b/analysis/src/CodeActions.ml new file mode 100644 index 000000000..a393f988f --- /dev/null +++ b/analysis/src/CodeActions.ml @@ -0,0 +1,66 @@ +type kind = RefactorRewrite + +let kindToString kind = match kind with RefactorRewrite -> "refactor.rewrite" + +module TextDocument = struct + type t = {version : string option; uri : string} + + let toString t = + Printf.sprintf {|{"version": %s, "uri": "%s"}|} + (match t.version with + | Some version -> "\"" ^ Json.escape version ^ "\"" + | None -> "null") + t.uri +end + +module Position = struct + type t = {line : int; character : int} + + let toString t = + Printf.sprintf {|{"line": %s, "character": %s}|} (string_of_int t.line) + (string_of_int t.character) +end + +module TextEditRange = struct + type t = {start : Position.t; end_ : Position.t} + + let toString t = + Printf.sprintf {|{"start": %s, "end": %s}|} + (t.start |> Position.toString) + (t.end_ |> Position.toString) +end + +module TextEdit = struct + type t = {newText : string; range : TextEditRange.t} + + let toString t = + Printf.sprintf {|{"newText": %s, "range": %s}|} t.newText + (t.range |> TextEditRange.toString) +end + +module DocumentChange = struct + type t = {textDocument : TextDocument.t; edits : TextEdit.t list} + + let toString t = + Printf.sprintf {|{"textDocument": %s, "edits": [%s]}|} + (t.textDocument |> TextDocument.toString) + "" +end + +module CodeActionEdit = struct + type t = {documentChanges : DocumentChange.t list} + + let toString t = Printf.sprintf {|{"documentChanges": [%s]}|} "" +end + +module CodeAction = struct + type t = {title : string; kind : kind; edit : CodeActionEdit.t} + + let toString t = + Printf.sprintf {|{"title": %s, "kind": "%s", "edit": %s}|} t.title + (kindToString t.kind) + (t.edit |> CodeActionEdit.toString) +end + +(* This is the return that's expected when resolving code actions *) +type result = CodeAction.t list \ No newline at end of file From caa47c349abf703d573400002be7efede09b2f26 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 23 Mar 2022 21:01:50 +0100 Subject: [PATCH 03/51] prepare code actions in analysis bin --- analysis/src/CodeActions.ml | 37 ++++++++++++++++++++++++++++----- analysis/src/Commands.ml | 41 ++++++++++++++++++++++++++++++++----- server/src/server.ts | 34 +++--------------------------- 3 files changed, 71 insertions(+), 41 deletions(-) diff --git a/analysis/src/CodeActions.ml b/analysis/src/CodeActions.ml index a393f988f..4861b70e2 100644 --- a/analysis/src/CodeActions.ml +++ b/analysis/src/CodeActions.ml @@ -5,6 +5,8 @@ let kindToString kind = match kind with RefactorRewrite -> "refactor.rewrite" module TextDocument = struct type t = {version : string option; uri : string} + let make ~version ~uri = {version; uri} + let toString t = Printf.sprintf {|{"version": %s, "uri": "%s"}|} (match t.version with @@ -16,6 +18,8 @@ end module Position = struct type t = {line : int; character : int} + let make ~line ~character = {line; character} + let toString t = Printf.sprintf {|{"line": %s, "character": %s}|} (string_of_int t.line) (string_of_int t.character) @@ -24,6 +28,8 @@ end module TextEditRange = struct type t = {start : Position.t; end_ : Position.t} + let make ~start ~end_ = {start; end_} + let toString t = Printf.sprintf {|{"start": %s, "end": %s}|} (t.start |> Position.toString) @@ -33,34 +39,55 @@ end module TextEdit = struct type t = {newText : string; range : TextEditRange.t} + let make ~newText ~range = {newText; range} + let toString t = - Printf.sprintf {|{"newText": %s, "range": %s}|} t.newText + Printf.sprintf {|{"newText": "%s", "range": %s}|} (Json.escape t.newText) (t.range |> TextEditRange.toString) end module DocumentChange = struct type t = {textDocument : TextDocument.t; edits : TextEdit.t list} + let make ~textDocument ~edits = {textDocument; edits} + let toString t = Printf.sprintf {|{"textDocument": %s, "edits": [%s]}|} (t.textDocument |> TextDocument.toString) - "" + (t.edits + |> List.map (fun edit -> edit |> TextEdit.toString) + |> String.concat ",") end module CodeActionEdit = struct type t = {documentChanges : DocumentChange.t list} - let toString t = Printf.sprintf {|{"documentChanges": [%s]}|} "" + let make ~documentChanges = {documentChanges} + + let toString t = + Printf.sprintf {|{"documentChanges": [%s]}|} + (t.documentChanges + |> List.map (fun documentChange -> + documentChange |> DocumentChange.toString) + |> String.concat ",") end module CodeAction = struct type t = {title : string; kind : kind; edit : CodeActionEdit.t} + let make ~title ~kind ~edit = {title; kind; edit} + let toString t = - Printf.sprintf {|{"title": %s, "kind": "%s", "edit": %s}|} t.title + Printf.sprintf {|{"title": "%s", "kind": "%s", "edit": %s}|} t.title (kindToString t.kind) (t.edit |> CodeActionEdit.toString) end (* This is the return that's expected when resolving code actions *) -type result = CodeAction.t list \ No newline at end of file +type result = CodeAction.t list + +let stringifyCodeActions codeActions = + Printf.sprintf {|[%s]|} + (codeActions + |> List.map (fun codeAction -> codeAction |> CodeAction.toString) + |> String.concat ",") diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index f58866d00..69eadd6a8 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -84,11 +84,42 @@ let codeAction ~path ~line ~col = match locItem.SharedTypes.locType with | Typed (n, t, _) -> ( match Printtyp.tree_of_typexp false t with - | Otyp_constr (Oide_ident "option", _) -> Protocol.stringifyCodeAction {range = Utils.cmtLocToRange locItem.loc; newText = "switch " ^ n ^ " { | None => failWith(\"TODO\") | Some(" ^ n ^ ") => _" ^ n ^ " }"} - | _ -> Protocol.null - ) - | Constant _constant -> - Protocol.stringifyHover {contents = "constant"} + | Otyp_constr (Oide_ident "option", _) -> + let range = Utils.cmtLocToRange locItem.loc in + CodeActions.( + stringifyCodeActions + [ + CodeAction.make ~title:"Unwrap optional" + ~kind:RefactorRewrite + ~edit: + (CodeActionEdit.make + ~documentChanges: + [ + DocumentChange.make + ~textDocument: + (TextDocument.make ~version:None ~uri:path) + ~edits: + [ + TextEdit.make + ~newText: + ("switch " ^ n + ^ " { | None => failWith(\"TODO\") | \ + Some(" ^ n ^ ") => _" ^ n ^ " }") + ~range: + (TextEditRange.make + ~start: + (Position.make + ~line:range.start.line + ~character: + range.start.character) + ~end_: + (Position.make + ~line:range.end_.line + ~character:range.end_.character)); + ]; + ]); + ]) + | _ -> Protocol.null) | _ -> Protocol.null)) in print_endline result diff --git a/server/src/server.ts b/server/src/server.ts index 55dd0cbf5..134888948 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -438,43 +438,15 @@ function codeAction(msg: p.RequestMessage): p.ResponseMessage { ], msg ); - let result: null | { - content: string; - range: { - start: { line: number; character: number }; - end: { line: number; character: number }; - }; - } = response.result as any; + + let { result } = response; let res: v.ResponseMessage = { jsonrpc: c.jsonrpcVersion, id: msg.id, }; - if (result == null) { - res.result = null; - return res; - } - - let textEdit: v.TextEdit = { newText: result.content, range: result.range }; - - let codeAction: v.CodeAction = { - title: "Unwrap optional", - kind: v.CodeActionKind.RefactorRewrite, - edit: { - documentChanges: [ - { - textDocument: { - version: null, - uri: params.textDocument.uri, - }, - edits: [textEdit], - }, - ], - }, - }; - - res.result = [codeAction]; + res.result = result ?? null; return res; } From fbd710eccd2c809299646e1b27ddf59380d4c959 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 24 Mar 2022 12:20:59 +0100 Subject: [PATCH 04/51] move most things to Protocol --- analysis/src/CodeActions.ml | 109 +++++++++--------------------------- analysis/src/Commands.ml | 36 +++--------- analysis/src/Protocol.ml | 17 +++--- 3 files changed, 40 insertions(+), 122 deletions(-) diff --git a/analysis/src/CodeActions.ml b/analysis/src/CodeActions.ml index 4861b70e2..300c759b1 100644 --- a/analysis/src/CodeActions.ml +++ b/analysis/src/CodeActions.ml @@ -1,93 +1,36 @@ +open Protocol + type kind = RefactorRewrite let kindToString kind = match kind with RefactorRewrite -> "refactor.rewrite" -module TextDocument = struct - type t = {version : string option; uri : string} - - let make ~version ~uri = {version; uri} - - let toString t = - Printf.sprintf {|{"version": %s, "uri": "%s"}|} - (match t.version with - | Some version -> "\"" ^ Json.escape version ^ "\"" - | None -> "null") - t.uri -end - -module Position = struct - type t = {line : int; character : int} - - let make ~line ~character = {line; character} - - let toString t = - Printf.sprintf {|{"line": %s, "character": %s}|} (string_of_int t.line) - (string_of_int t.character) -end +type codeAction = {title : string; kind : kind; edit : codeActionEdit} -module TextEditRange = struct - type t = {start : Position.t; end_ : Position.t} - - let make ~start ~end_ = {start; end_} - - let toString t = - Printf.sprintf {|{"start": %s, "end": %s}|} - (t.start |> Position.toString) - (t.end_ |> Position.toString) -end - -module TextEdit = struct - type t = {newText : string; range : TextEditRange.t} - - let make ~newText ~range = {newText; range} - - let toString t = - Printf.sprintf {|{"newText": "%s", "range": %s}|} (Json.escape t.newText) - (t.range |> TextEditRange.toString) -end +let stringifyCodeAction ca = + Printf.sprintf {|{"title": "%s", "kind": "%s", "edit": %s}|} ca.title + (kindToString ca.kind) + (ca.edit |> stringifyCodeActionEdit) -module DocumentChange = struct - type t = {textDocument : TextDocument.t; edits : TextEdit.t list} - - let make ~textDocument ~edits = {textDocument; edits} - - let toString t = - Printf.sprintf {|{"textDocument": %s, "edits": [%s]}|} - (t.textDocument |> TextDocument.toString) - (t.edits - |> List.map (fun edit -> edit |> TextEdit.toString) - |> String.concat ",") -end - -module CodeActionEdit = struct - type t = {documentChanges : DocumentChange.t list} - - let make ~documentChanges = {documentChanges} +(* This is the return that's expected when resolving code actions *) +type result = codeAction list - let toString t = - Printf.sprintf {|{"documentChanges": [%s]}|} - (t.documentChanges - |> List.map (fun documentChange -> - documentChange |> DocumentChange.toString) - |> String.concat ",") -end +let stringifyCodeActions codeActions = + Printf.sprintf {|%s|} (codeActions |> List.map stringifyCodeAction |> array) module CodeAction = struct - type t = {title : string; kind : kind; edit : CodeActionEdit.t} - - let make ~title ~kind ~edit = {title; kind; edit} - - let toString t = - Printf.sprintf {|{"title": "%s", "kind": "%s", "edit": %s}|} t.title - (kindToString t.kind) - (t.edit |> CodeActionEdit.toString) + let makeRangeReplace ~title ~kind ~file ~newText ~range = + { + title; + kind; + edit = + { + documentChanges = + [ + { + textDocument = {version = None; uri = file}; + edits = [{newText; range}]; + }; + ]; + }; + } end - -(* This is the return that's expected when resolving code actions *) -type result = CodeAction.t list - -let stringifyCodeActions codeActions = - Printf.sprintf {|[%s]|} - (codeActions - |> List.map (fun codeAction -> codeAction |> CodeAction.toString) - |> String.concat ",") diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 69eadd6a8..2eced70bb 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -89,35 +89,13 @@ let codeAction ~path ~line ~col = CodeActions.( stringifyCodeActions [ - CodeAction.make ~title:"Unwrap optional" - ~kind:RefactorRewrite - ~edit: - (CodeActionEdit.make - ~documentChanges: - [ - DocumentChange.make - ~textDocument: - (TextDocument.make ~version:None ~uri:path) - ~edits: - [ - TextEdit.make - ~newText: - ("switch " ^ n - ^ " { | None => failWith(\"TODO\") | \ - Some(" ^ n ^ ") => _" ^ n ^ " }") - ~range: - (TextEditRange.make - ~start: - (Position.make - ~line:range.start.line - ~character: - range.start.character) - ~end_: - (Position.make - ~line:range.end_.line - ~character:range.end_.character)); - ]; - ]); + CodeAction.makeRangeReplace ~title:"Unwrap optional" + ~kind:RefactorRewrite ~file:path + ~newText: + ("switch " ^ n + ^ " { | None => failWith(\"TODO\") | Some(" ^ n + ^ ") => _" ^ n ^ " }") + ~range; ]) | _ -> Protocol.null) | _ -> Protocol.null)) diff --git a/analysis/src/Protocol.ml b/analysis/src/Protocol.ml index a32b64912..feef0eca6 100644 --- a/analysis/src/Protocol.ml +++ b/analysis/src/Protocol.ml @@ -1,7 +1,5 @@ type position = {line : int; character : int} - type range = {start : position; end_ : position} - type markupContent = {kind : string; value : string} type completionItem = { @@ -13,15 +11,10 @@ type completionItem = { } type hover = {contents : string} - type location = {uri : string; range : range} - type documentSymbolItem = {name : string; kind : int; location : location} - type codeAction = {newText : string; range : range} - type renameFile = {oldUri : string; newUri : string} - type textEdit = {range : range; newText : string} type optionalVersionedTextDocumentIdentifier = { @@ -34,8 +27,9 @@ type textDocumentEdit = { edits : textEdit list; } -let null = "null" +type codeActionEdit = {documentChanges : textDocumentEdit list} +let null = "null" let array l = "[" ^ String.concat ", " l ^ "]" let stringifyPosition p = @@ -74,8 +68,7 @@ let stringifyCodeAction c = "content": "%s", "range": %s }|} - (Json.escape c.newText) - (stringifyRange c.range) + (Json.escape c.newText) (stringifyRange c.range) let stringifyLocation (h : location) = Printf.sprintf {|{"uri": "%s", "range": %s}|} (Json.escape h.uri) @@ -121,3 +114,7 @@ let stringifyTextDocumentEdit tde = }|} (stringifyoptionalVersionedTextDocumentIdentifier tde.textDocument) (tde.edits |> List.map stringifyTextEdit |> array) + +let stringifyCodeActionEdit cae = + Printf.sprintf {|{"documentChanges": %s}|} + (cae.documentChanges |> List.map stringifyTextDocumentEdit |> array) From dcbd04edc1aef0428545719affd70fb3c4b00890 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 25 Mar 2022 20:10:27 +0100 Subject: [PATCH 05/51] extract potential code actions from diagnostics where appropriate, and add a first batch of code actions --- server/src/codeActions.ts | 346 ++++++++++++++++++++++++++++++++++++++ server/src/server.ts | 29 +++- server/src/utils.ts | 51 +++++- 3 files changed, 421 insertions(+), 5 deletions(-) create mode 100644 server/src/codeActions.ts diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts new file mode 100644 index 000000000..2db18d972 --- /dev/null +++ b/server/src/codeActions.ts @@ -0,0 +1,346 @@ +// This file holds code actions derived from diagnostics. There are more code +// actions available in the extension, but they are derived via the analysis +// OCaml binary. +import * as p from "vscode-languageserver-protocol"; + +export type filesCodeActions = { + [key: string]: { range: p.Range; codeAction: p.CodeAction }[]; +}; + +interface findCodeActionsConfig { + diagnostic: p.Diagnostic; + diagnosticMessage: string[]; + file: string; + range: p.Range; + addFoundActionsHere: filesCodeActions; +} + +export let findCodeActionsInDiagnosticsMessage = ({ + diagnostic, + diagnosticMessage, + file, + range, + addFoundActionsHere: codeActions, +}: findCodeActionsConfig) => { + diagnosticMessage.forEach((line, index, array) => { + // Because of how actions work, there can only be one per diagnostic. So, + // halt whenever a code action has been found. + let actions = [ + didYouMeanAction, + addUndefinedRecordFields, + simpleConversion, + topLevelUnitType, + ]; + + for (let action of actions) { + if ( + action({ + array, + codeActions, + diagnostic, + file, + index, + line, + range, + }) + ) { + break; + } + } + }); +}; + +interface codeActionExtractorConfig { + line: string; + index: number; + array: string[]; + file: string; + range: p.Range; + diagnostic: p.Diagnostic; + codeActions: filesCodeActions; +} + +type codeActionExtractor = (config: codeActionExtractorConfig) => boolean; + +let didYouMeanAction: codeActionExtractor = ({ + codeActions, + diagnostic, + file, + line, + range, +}) => { + if (line.startsWith("Hint: Did you mean")) { + let regex = /Did you mean ([A-Za-z0-9_]*)?/; + let match = line.match(regex); + + if (match === null) { + return false; + } + + let [_, suggestion] = match; + + if (suggestion != null) { + codeActions[file] = codeActions[file] || []; + let codeAction: p.CodeAction = { + title: `Replace with '${suggestion}'`, + edit: { + changes: { + [file]: [{ range, newText: suggestion }], + }, + }, + diagnostics: [diagnostic], + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + } + + return false; +}; + +let addUndefinedRecordFields: codeActionExtractor = ({ + array, + codeActions, + diagnostic, + file, + index, + line, + range, +}) => { + if (line.startsWith("Some record fields are undefined:")) { + let recordFieldNames = line + .trim() + .split("Some record fields are undefined: ")[1] + ?.split(" "); + + // This collects the rest of the fields if fields are printed on + // multiple lines. + array.slice(index + 1).forEach((line) => { + recordFieldNames.push(...line.trim().split(" ")); + }); + + if (recordFieldNames != null) { + codeActions[file] = codeActions[file] || []; + + // The formatter outputs trailing commas automatically if the record + // definition is on multiple lines, and no trailing comma if it's on a + // single line. We need to adapt to this so we don't accidentally + // insert an invalid comma. + let multilineRecordDefinitionBody = range.start.line !== range.end.line; + + // Let's build up the text we're going to insert. + let newText = ""; + + if (multilineRecordDefinitionBody) { + // If it's a multiline body, we know it looks like this: + // ``` + // let someRecord = { + // atLeastOneExistingField: string, + // } + // ``` + // We can figure out the formatting from the range the code action + // gives us. We'll insert to the direct left of the ending brace. + + // The end char is the closing brace, and it's always going to be 2 + // characters back from the record fields. + let paddingCharacters = multilineRecordDefinitionBody + ? range.end.character + 2 + : 0; + let paddingContentRecordField = Array.from({ + length: paddingCharacters, + }).join(" "); + let paddingContentEndBrace = Array.from({ + length: range.end.character, + }).join(" "); + + recordFieldNames.forEach((fieldName, index) => { + if (index === 0) { + // This adds spacing from the ending brace up to the equivalent + // of the last record field name, needed for the first inserted + // record field name. + newText += " "; + } else { + // The rest of the new record field names will start from a new + // line, so they need left padding all the way to the same level + // as the rest of the record fields. + newText += paddingContentRecordField; + } + + newText += `${fieldName}: assert false,\n`; + }); + + // Let's put the end brace back where it was (we still have it to the direct right of us). + newText += `${paddingContentEndBrace}`; + } else { + // A single line record definition body is a bit easier - we'll just add the new fields on the same line. + newText += ", "; + newText += recordFieldNames + .map((fieldName) => `${fieldName}: assert false`) + .join(", "); + } + + let codeAction: p.CodeAction = { + title: `Add missing record fields`, + edit: { + changes: { + [file]: [ + { + range: { + start: { + line: range.end.line, + character: range.end.character - 1, + }, + end: { + line: range.end.line, + character: range.end.character - 1, + }, + }, + newText, + }, + ], + }, + }, + diagnostics: [diagnostic], + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + } + + return false; +}; + +let simpleConversion: codeActionExtractor = ({ + line, + codeActions, + file, + range, + diagnostic, +}) => { + if (line.startsWith("You can convert ")) { + let regex = /You can convert (\w*) to (\w*) with ([\w.]*).$/; + let match = line.match(regex); + + if (match === null) { + return false; + } + + let [_, from, to, fn] = match; + + if (from != null && to != null && fn != null) { + codeActions[file] = codeActions[file] || []; + let codeAction: p.CodeAction = { + title: `Convert ${from} to ${to} with ${fn}`, + edit: { + changes: { + [file]: [ + { + range: { + start: { + line: range.start.line, + character: range.start.character, + }, + end: { + line: range.start.line, + character: range.start.character, + }, + }, + newText: `${fn}(`, + }, + { + range: { + start: { + line: range.end.line, + character: range.end.character, + }, + end: { + line: range.end.line, + character: range.end.character, + }, + }, + newText: `)`, + }, + ], + }, + }, + diagnostics: [diagnostic], + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + } + + return false; +}; + +let topLevelUnitType: codeActionExtractor = ({ + line, + codeActions, + file, + range, + diagnostic, +}) => { + if (line.startsWith("Toplevel expression is expected to have unit type.")) { + codeActions[file] = codeActions[file] || []; + let codeAction: p.CodeAction = { + title: `Wrap expression in ignore`, + edit: { + changes: { + [file]: [ + { + range: { + start: { + line: range.start.line, + character: range.start.character, + }, + end: { + line: range.start.line, + character: range.start.character, + }, + }, + newText: `ignore(`, + }, + { + range: { + start: { + line: range.end.line, + character: range.end.character, + }, + end: { + line: range.end.line, + character: range.end.character, + }, + }, + newText: `)`, + }, + ], + }, + }, + diagnostics: [diagnostic], + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + + return false; +}; diff --git a/server/src/server.ts b/server/src/server.ts index 134888948..3ee672803 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -12,6 +12,7 @@ import { DidCloseTextDocumentNotification, } from "vscode-languageserver-protocol"; import * as utils from "./utils"; +import * as codeActions from "./codeActions"; import * as c from "./constants"; import * as chokidar from "chokidar"; import { assert } from "console"; @@ -36,6 +37,9 @@ let projectsFiles: Map< > = new Map(); // ^ caching AND states AND distributed system. Why does LSP has to be stupid like this +// This keeps track of code actions extracted from diagnostics. +let codeActionsFromDiagnostics: codeActions.filesCodeActions = {}; + // will be properly defined later depending on the mode (stdio/node-rpc) let send: (msg: m.Message) => void = (_) => {}; @@ -65,8 +69,13 @@ let sendUpdatedDiagnostics = () => { path.join(projectRootPath, c.compilerLogPartialPath), { encoding: "utf-8" } ); - let { done, result: filesAndErrors } = - utils.parseCompilerLogOutput(content); + let { + done, + result: filesAndErrors, + codeActions, + } = utils.parseCompilerLogOutput(content); + + codeActionsFromDiagnostics = codeActions; // diff Object.keys(filesAndErrors).forEach((file) => { @@ -428,6 +437,17 @@ function completion(msg: p.RequestMessage) { function codeAction(msg: p.RequestMessage): p.ResponseMessage { let params = msg.params as p.CodeActionParams; let filePath = fileURLToPath(params.textDocument.uri); + + // Check local code actions coming from the diagnostics. + let localResults: v.CodeAction[] = []; + codeActionsFromDiagnostics[params.textDocument.uri]?.forEach( + ({ range, codeAction }) => { + if (utils.rangeContainsRange(range, params.range)) { + localResults.push(codeAction); + } + } + ); + let response = utils.runAnalysisCommand( filePath, [ @@ -446,7 +466,10 @@ function codeAction(msg: p.RequestMessage): p.ResponseMessage { id: msg.id, }; - res.result = result ?? null; + res.result = + result != null && Array.isArray(result) + ? [...localResults, result] + : localResults; return res; } diff --git a/server/src/utils.ts b/server/src/utils.ts index 1bd7b7659..b0fe500b2 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -1,4 +1,5 @@ import * as c from "./constants"; +import * as codeActions from "./codeActions"; import * as childProcess from "child_process"; import * as p from "vscode-languageserver-protocol"; import * as path from "path"; @@ -454,6 +455,7 @@ type filesDiagnostics = { type parsedCompilerLogResult = { done: boolean; result: filesDiagnostics; + codeActions: codeActions.filesCodeActions; }; export let parseCompilerLogOutput = ( content: string @@ -571,6 +573,8 @@ export let parseCompilerLogOutput = ( } let result: filesDiagnostics = {}; + let foundCodeActions: codeActions.filesCodeActions = {}; + parsedDiagnostics.forEach((parsedDiagnostic) => { let [fileAndRangeLine, ...diagnosticMessage] = parsedDiagnostic.content; let { file, range } = parseFileAndRange(fileAndRangeLine); @@ -578,7 +582,8 @@ export let parseCompilerLogOutput = ( if (result[file] == null) { result[file] = []; } - result[file].push({ + + let diagnostic: p.Diagnostic = { severity: parsedDiagnostic.severity, tags: parsedDiagnostic.tag === undefined ? [] : [parsedDiagnostic.tag], code: parsedDiagnostic.code, @@ -586,8 +591,50 @@ export let parseCompilerLogOutput = ( source: "ReScript", // remove start and end whitespaces/newlines message: diagnosticMessage.join("\n").trim() + "\n", + }; + + // Check for potential code actions + codeActions.findCodeActionsInDiagnosticsMessage({ + addFoundActionsHere: foundCodeActions, + diagnostic, + diagnosticMessage, + file, + range, }); + + result[file].push(diagnostic); }); - return { done, result }; + return { done, result, codeActions: foundCodeActions }; +}; + +export let rangeContainsRange = ( + range: p.Range, + otherRange: p.Range +): boolean => { + if ( + otherRange.start.line < range.start.line || + otherRange.end.line < range.start.line + ) { + return false; + } + if ( + otherRange.start.line > range.end.line || + otherRange.end.line > range.end.line + ) { + return false; + } + if ( + otherRange.start.line === range.start.line && + otherRange.start.character < range.start.character + ) { + return false; + } + if ( + otherRange.end.line === range.end.line && + otherRange.end.character > range.end.character + ) { + return false; + } + return true; }; From 8d6c9edc07f79ca35f32437110c80ef53e527645 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 24 Mar 2022 11:31:26 +0100 Subject: [PATCH 06/51] Test action if-then-else to switch. --- analysis/src/Actions.ml | 89 +++++++++++++++++++++ analysis/src/Commands.ml | 5 ++ analysis/tests/src/Actions.res | 13 +++ analysis/tests/src/expected/Actions.res.txt | 34 ++++++++ analysis/tests/src/expected/Debug.res.txt | 3 +- 5 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 analysis/src/Actions.ml create mode 100644 analysis/tests/src/Actions.res create mode 100644 analysis/tests/src/expected/Actions.res.txt diff --git a/analysis/src/Actions.ml b/analysis/src/Actions.ml new file mode 100644 index 000000000..6d8496c6d --- /dev/null +++ b/analysis/src/Actions.ml @@ -0,0 +1,89 @@ +let posInLoc ~pos ~loc = + Utils.tupleOfLexing loc.Location.loc_start <= pos + && pos < Utils.tupleOfLexing loc.loc_end + +let rec expToPat (exp : Parsetree.expression) = + let mkPat ppat_desc = + Ast_helper.Pat.mk ~loc:exp.pexp_loc ~attrs:exp.pexp_attributes ppat_desc + in + match exp.pexp_desc with + | Pexp_construct (lid, None) -> + Some (mkPat (Parsetree.Ppat_construct (lid, None))) + | Pexp_construct (lid, Some e1) -> ( + match expToPat e1 with + | None -> None + | Some p1 -> Some (mkPat (Parsetree.Ppat_construct (lid, Some p1)))) + | Pexp_constant c -> Some (mkPat (Parsetree.Ppat_constant c)) + | _ -> None + +let mkMapper ~pos ~changed = + let value_binding (mapper : Ast_mapper.mapper) (vb : Parsetree.value_binding) + = + let newExp = + match vb.pvb_pat.ppat_desc with + | Ppat_var {txt; loc} when posInLoc ~pos ~loc -> ( + match vb.pvb_expr.pexp_desc with + | Pexp_ifthenelse + ( { + pexp_desc = + Pexp_apply + ( { + pexp_desc = + Pexp_ident {txt = Lident (("=" | "<>") as op)}; + }, + [(Nolabel, arg1); (Nolabel, arg2)] ); + }, + e1, + Some e2 ) -> ( + let e1, e2 = if op = "=" then (e1, e2) else (e2, e1) in + let mkMatch ~arg ~pat = + let cases = + [ + Ast_helper.Exp.case pat e1; + Ast_helper.Exp.case (Ast_helper.Pat.any ()) e2; + ] + in + Ast_helper.Exp.match_ ~loc:vb.pvb_expr.pexp_loc + ~attrs:vb.pvb_expr.pexp_attributes arg cases + in + + match expToPat arg2 with + | None -> ( + match expToPat arg1 with + | None -> None + | Some pat1 -> + let newExp = mkMatch ~arg:arg2 ~pat:pat1 in + Printf.printf "Hit %s\n" txt; + Some newExp) + | Some pat2 -> + let newExp = mkMatch ~arg:arg1 ~pat:pat2 in + Printf.printf "Hit %s\n" txt; + Some newExp) + | _ -> None) + | _ -> None + in + match newExp with + | Some newExp -> + changed := true; + {vb with pvb_expr = newExp} + | None -> Ast_mapper.default_mapper.value_binding mapper vb + in + + {Ast_mapper.default_mapper with value_binding} + +let command ~path ~pos = + if Filename.check_suffix path ".res" then + let parser = + Res_driver.parsingEngine.parseImplementation ~forPrinter:false + in + let {Res_driver.parsetree = structure; comments; filename} = + parser ~filename:path + in + let printer = + Res_driver.printEngine.printImplementation + ~width:!Res_cli.ResClflags.width ~comments ~filename + in + let changed = ref false in + let mapper = mkMapper ~pos ~changed in + let newStructure = mapper.structure mapper structure in + if !changed then printer newStructure diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 2eced70bb..5dc74968b 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -378,6 +378,11 @@ let test ~path = dir ++ parent_dir_name ++ "lib" ++ "bs" ++ "src" ++ name in Printf.printf "%s" (CreateInterface.command ~path ~cmiFile) + | "act" -> + print_endline + ("Actions " ^ path ^ " " ^ string_of_int line ^ ":" + ^ string_of_int col); + Actions.command ~path ~pos:(line, col) | _ -> ()); print_newline ()) in diff --git a/analysis/tests/src/Actions.res b/analysis/tests/src/Actions.res new file mode 100644 index 000000000..4f1228867 --- /dev/null +++ b/analysis/tests/src/Actions.res @@ -0,0 +1,13 @@ +type kind = First | Second | Third + +let _ = (kind, kindStr) => { + let _ifThenElse = if kind == First { + // ^act + "First" + } else { + "Not First" + } + + let _ternary = "First" != kindStr ? "Not First" : "First" + // ^act +} diff --git a/analysis/tests/src/expected/Actions.res.txt b/analysis/tests/src/expected/Actions.res.txt new file mode 100644 index 000000000..a128d4f7a --- /dev/null +++ b/analysis/tests/src/expected/Actions.res.txt @@ -0,0 +1,34 @@ +Actions tests/src/Actions.res 3:10 +Hit _ifThenElse +type kind = First | Second | Third + +let _ = (kind, kindStr) => { + let _ifThenElse = switch kind { + | First => // ^act + "First" + | _ => "Not First" + } + + let _ternary = "First" != kindStr ? "Not First" : "First" + // ^act +} + +Actions tests/src/Actions.res 10:9 +Hit _ternary +type kind = First | Second | Third + +let _ = (kind, kindStr) => { + let _ifThenElse = if kind == First { + // ^act + "First" + } else { + "Not First" + } + + let _ternary = switch kindStr { + | "First" => "First" + | _ => "Not First" + } + // ^act +} + diff --git a/analysis/tests/src/expected/Debug.res.txt b/analysis/tests/src/expected/Debug.res.txt index fecfa8798..c10b692fb 100644 --- a/analysis/tests/src/expected/Debug.res.txt +++ b/analysis/tests/src/expected/Debug.res.txt @@ -4,7 +4,8 @@ Dependencies: @rescript/react Source directories: tests/node_modules/@rescript/react/src tests/node_modules/@rescript/react/src/legacy Source files: tests/node_modules/@rescript/react/src/React.res tests/node_modules/@rescript/react/src/ReactDOM.res tests/node_modules/@rescript/react/src/ReactDOMServer.res tests/node_modules/@rescript/react/src/ReactDOMStyle.res tests/node_modules/@rescript/react/src/ReactEvent.res tests/node_modules/@rescript/react/src/ReactEvent.resi tests/node_modules/@rescript/react/src/ReactTestUtils.res tests/node_modules/@rescript/react/src/ReactTestUtils.resi tests/node_modules/@rescript/react/src/RescriptReactErrorBoundary.res tests/node_modules/@rescript/react/src/RescriptReactErrorBoundary.resi tests/node_modules/@rescript/react/src/RescriptReactRouter.res tests/node_modules/@rescript/react/src/RescriptReactRouter.resi tests/node_modules/@rescript/react/src/legacy/ReactDOMRe.res tests/node_modules/@rescript/react/src/legacy/ReasonReact.res Source directories: tests/src tests/src/expected -Source files: tests/src/Auto.res tests/src/CompletePrioritize1.res tests/src/CompletePrioritize2.res tests/src/Completion.res tests/src/Component.res tests/src/Component.resi tests/src/CreateInterface.res tests/src/Cross.res tests/src/Debug.res tests/src/Definition.res tests/src/DefinitionWithInterface.res tests/src/DefinitionWithInterface.resi tests/src/Div.res tests/src/Fragment.res tests/src/Highlight.res tests/src/Hover.res tests/src/Jsx.res tests/src/Jsx.resi tests/src/LongIdentTest.res tests/src/Obj.res tests/src/Patterns.res tests/src/RecModules.res tests/src/RecordCompletion.res tests/src/References.res tests/src/ReferencesWithInterface.res tests/src/ReferencesWithInterface.resi tests/src/Rename.res tests/src/RenameWithInterface.res tests/src/RenameWithInterface.resi tests/src/TableclothMap.ml tests/src/TableclothMap.mli tests/src/TypeDefinition.res +Source files: tests/src/Actions.res tests/src/Auto.res tests/src/CompletePrioritize1.res tests/src/CompletePrioritize2.res tests/src/Completion.res tests/src/Component.res tests/src/Component.resi tests/src/CreateInterface.res tests/src/Cross.res tests/src/Debug.res tests/src/Definition.res tests/src/DefinitionWithInterface.res tests/src/DefinitionWithInterface.resi tests/src/Div.res tests/src/Fragment.res tests/src/Highlight.res tests/src/Hover.res tests/src/Jsx.res tests/src/Jsx.resi tests/src/LongIdentTest.res tests/src/Obj.res tests/src/Patterns.res tests/src/RecModules.res tests/src/RecordCompletion.res tests/src/References.res tests/src/ReferencesWithInterface.res tests/src/ReferencesWithInterface.resi tests/src/Rename.res tests/src/RenameWithInterface.res tests/src/RenameWithInterface.resi tests/src/TableclothMap.ml tests/src/TableclothMap.mli tests/src/TypeDefinition.res +Impl cmt:tests/lib/bs/src/Actions.cmt res:tests/src/Actions.res Impl cmt:tests/lib/bs/src/Auto.cmt res:tests/src/Auto.res Impl cmt:tests/lib/bs/src/CompletePrioritize1.cmt res:tests/src/CompletePrioritize1.res Impl cmt:tests/lib/bs/src/CompletePrioritize2.cmt res:tests/src/CompletePrioritize2.res From 91a33ae188912697a3a4ae8c00f14372cbdec564 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 25 Mar 2022 09:19:58 +0100 Subject: [PATCH 07/51] Print to a string, not standard output. --- analysis/src/Actions.ml | 11 +++++------ analysis/tests/src/expected/Actions.res.txt | 2 ++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/analysis/src/Actions.ml b/analysis/src/Actions.ml index 6d8496c6d..a5a147207 100644 --- a/analysis/src/Actions.ml +++ b/analysis/src/Actions.ml @@ -76,14 +76,13 @@ let command ~path ~pos = let parser = Res_driver.parsingEngine.parseImplementation ~forPrinter:false in - let {Res_driver.parsetree = structure; comments; filename} = - parser ~filename:path - in + let {Res_driver.parsetree = structure; comments} = parser ~filename:path in let printer = - Res_driver.printEngine.printImplementation - ~width:!Res_cli.ResClflags.width ~comments ~filename + Res_printer.printImplementation ~width:!Res_cli.ResClflags.width ~comments in let changed = ref false in let mapper = mkMapper ~pos ~changed in let newStructure = mapper.structure mapper structure in - if !changed then printer newStructure + if !changed then + let formatted = printer newStructure in + Printf.printf "Formatted:\n%s" formatted diff --git a/analysis/tests/src/expected/Actions.res.txt b/analysis/tests/src/expected/Actions.res.txt index a128d4f7a..288ad10d1 100644 --- a/analysis/tests/src/expected/Actions.res.txt +++ b/analysis/tests/src/expected/Actions.res.txt @@ -1,5 +1,6 @@ Actions tests/src/Actions.res 3:10 Hit _ifThenElse +Formatted: type kind = First | Second | Third let _ = (kind, kindStr) => { @@ -15,6 +16,7 @@ let _ = (kind, kindStr) => { Actions tests/src/Actions.res 10:9 Hit _ternary +Formatted: type kind = First | Second | Third let _ = (kind, kindStr) => { From 7b81173373cbf84d8c9fd11f35f1cd9ba56bbdc6 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 25 Mar 2022 09:42:12 +0100 Subject: [PATCH 08/51] Variants and records. --- analysis/src/Actions.ml | 31 ++++++++++++++++++--- analysis/tests/src/Actions.res | 3 +- analysis/tests/src/expected/Actions.res.txt | 10 ++++--- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/analysis/src/Actions.ml b/analysis/src/Actions.ml index a5a147207..9c75932a2 100644 --- a/analysis/src/Actions.ml +++ b/analysis/src/Actions.ml @@ -2,18 +2,41 @@ let posInLoc ~pos ~loc = Utils.tupleOfLexing loc.Location.loc_start <= pos && pos < Utils.tupleOfLexing loc.loc_end +let rec listToPat ~itemToPat = function + | [] -> Some [] + | x :: xList -> ( + match (itemToPat x, listToPat ~itemToPat xList) with + | Some p, Some pList -> Some (p :: pList) + | _ -> None) + let rec expToPat (exp : Parsetree.expression) = let mkPat ppat_desc = Ast_helper.Pat.mk ~loc:exp.pexp_loc ~attrs:exp.pexp_attributes ppat_desc in match exp.pexp_desc with - | Pexp_construct (lid, None) -> - Some (mkPat (Parsetree.Ppat_construct (lid, None))) + | Pexp_construct (lid, None) -> Some (mkPat (Ppat_construct (lid, None))) | Pexp_construct (lid, Some e1) -> ( match expToPat e1 with | None -> None - | Some p1 -> Some (mkPat (Parsetree.Ppat_construct (lid, Some p1)))) - | Pexp_constant c -> Some (mkPat (Parsetree.Ppat_constant c)) + | Some p1 -> Some (mkPat (Ppat_construct (lid, Some p1)))) + | Pexp_variant (label, None) -> Some (mkPat (Ppat_variant (label, None))) + | Pexp_variant (label, Some e1) -> ( + match expToPat e1 with + | None -> None + | Some p1 -> Some (mkPat (Ppat_variant (label, Some p1)))) + | Pexp_constant c -> Some (mkPat (Ppat_constant c)) + | Pexp_tuple eList -> ( + match listToPat ~itemToPat:expToPat eList with + | None -> None + | Some patList -> Some (mkPat (Ppat_tuple patList))) + | Pexp_record (items, None) -> ( + let itemToPat (x, e) = + match expToPat e with None -> None | Some p -> Some (x, p) + in + match listToPat ~itemToPat items with + | None -> None + | Some patItems -> Some (mkPat (Ppat_record (patItems, Closed)))) + | Pexp_record (_, Some _) -> None | _ -> None let mkMapper ~pos ~changed = diff --git a/analysis/tests/src/Actions.res b/analysis/tests/src/Actions.res index 4f1228867..58e29a8a1 100644 --- a/analysis/tests/src/Actions.res +++ b/analysis/tests/src/Actions.res @@ -1,4 +1,5 @@ type kind = First | Second | Third +type r = {name: string, age: int} let _ = (kind, kindStr) => { let _ifThenElse = if kind == First { @@ -8,6 +9,6 @@ let _ = (kind, kindStr) => { "Not First" } - let _ternary = "First" != kindStr ? "Not First" : "First" + let _ternary = #kind("First", {name: "abc", age: 3}) != kindStr ? "Not First" : "First" // ^act } diff --git a/analysis/tests/src/expected/Actions.res.txt b/analysis/tests/src/expected/Actions.res.txt index 288ad10d1..8b6763f3f 100644 --- a/analysis/tests/src/expected/Actions.res.txt +++ b/analysis/tests/src/expected/Actions.res.txt @@ -1,7 +1,8 @@ -Actions tests/src/Actions.res 3:10 +Actions tests/src/Actions.res 4:10 Hit _ifThenElse Formatted: type kind = First | Second | Third +type r = {name: string, age: int} let _ = (kind, kindStr) => { let _ifThenElse = switch kind { @@ -10,14 +11,15 @@ let _ = (kind, kindStr) => { | _ => "Not First" } - let _ternary = "First" != kindStr ? "Not First" : "First" + let _ternary = #kind("First", {name: "abc", age: 3}) != kindStr ? "Not First" : "First" // ^act } -Actions tests/src/Actions.res 10:9 +Actions tests/src/Actions.res 11:9 Hit _ternary Formatted: type kind = First | Second | Third +type r = {name: string, age: int} let _ = (kind, kindStr) => { let _ifThenElse = if kind == First { @@ -28,7 +30,7 @@ let _ = (kind, kindStr) => { } let _ternary = switch kindStr { - | "First" => "First" + | #kind("First", {name: "abc", age: 3}) => "First" | _ => "Not First" } // ^act From 621fd9d1122ee38940f48b0f904d6dc86c47f451 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 25 Mar 2022 11:22:49 +0100 Subject: [PATCH 09/51] Trigger on the body of if-then-else. --- analysis/src/Actions.ml | 76 +++++++++------------ analysis/tests/src/Actions.res | 4 +- analysis/tests/src/expected/Actions.res.txt | 14 ++-- 3 files changed, 42 insertions(+), 52 deletions(-) diff --git a/analysis/src/Actions.ml b/analysis/src/Actions.ml index 9c75932a2..1bb3f73de 100644 --- a/analysis/src/Actions.ml +++ b/analysis/src/Actions.ml @@ -40,59 +40,51 @@ let rec expToPat (exp : Parsetree.expression) = | _ -> None let mkMapper ~pos ~changed = - let value_binding (mapper : Ast_mapper.mapper) (vb : Parsetree.value_binding) - = + let expr (mapper : Ast_mapper.mapper) (e : Parsetree.expression) = let newExp = - match vb.pvb_pat.ppat_desc with - | Ppat_var {txt; loc} when posInLoc ~pos ~loc -> ( - match vb.pvb_expr.pexp_desc with - | Pexp_ifthenelse - ( { - pexp_desc = - Pexp_apply - ( { - pexp_desc = - Pexp_ident {txt = Lident (("=" | "<>") as op)}; - }, - [(Nolabel, arg1); (Nolabel, arg2)] ); - }, - e1, - Some e2 ) -> ( - let e1, e2 = if op = "=" then (e1, e2) else (e2, e1) in - let mkMatch ~arg ~pat = - let cases = - [ - Ast_helper.Exp.case pat e1; - Ast_helper.Exp.case (Ast_helper.Pat.any ()) e2; - ] - in - Ast_helper.Exp.match_ ~loc:vb.pvb_expr.pexp_loc - ~attrs:vb.pvb_expr.pexp_attributes arg cases + match e.pexp_desc with + | Pexp_ifthenelse + ( { + pexp_desc = + Pexp_apply + ( {pexp_desc = Pexp_ident {txt = Lident (("=" | "<>") as op)}}, + [(Nolabel, arg1); (Nolabel, arg2)] ); + }, + e1, + Some e2 ) + when posInLoc ~pos ~loc:e.pexp_loc -> ( + let e1, e2 = if op = "=" then (e1, e2) else (e2, e1) in + let mkMatch ~arg ~pat = + let cases = + [ + Ast_helper.Exp.case pat e1; + Ast_helper.Exp.case (Ast_helper.Pat.any ()) e2; + ] in + Ast_helper.Exp.match_ ~loc:e.pexp_loc ~attrs:e.pexp_attributes arg + cases + in - match expToPat arg2 with - | None -> ( - match expToPat arg1 with - | None -> None - | Some pat1 -> - let newExp = mkMatch ~arg:arg2 ~pat:pat1 in - Printf.printf "Hit %s\n" txt; - Some newExp) - | Some pat2 -> - let newExp = mkMatch ~arg:arg1 ~pat:pat2 in - Printf.printf "Hit %s\n" txt; + match expToPat arg2 with + | None -> ( + match expToPat arg1 with + | None -> None + | Some pat1 -> + let newExp = mkMatch ~arg:arg2 ~pat:pat1 in Some newExp) - | _ -> None) + | Some pat2 -> + let newExp = mkMatch ~arg:arg1 ~pat:pat2 in + Some newExp) | _ -> None in match newExp with | Some newExp -> changed := true; - {vb with pvb_expr = newExp} - | None -> Ast_mapper.default_mapper.value_binding mapper vb + newExp + | None -> Ast_mapper.default_mapper.expr mapper e in - {Ast_mapper.default_mapper with value_binding} + {Ast_mapper.default_mapper with expr} let command ~path ~pos = if Filename.check_suffix path ".res" then diff --git a/analysis/tests/src/Actions.res b/analysis/tests/src/Actions.res index 58e29a8a1..b1ac5cf2a 100644 --- a/analysis/tests/src/Actions.res +++ b/analysis/tests/src/Actions.res @@ -3,12 +3,12 @@ type r = {name: string, age: int} let _ = (kind, kindStr) => { let _ifThenElse = if kind == First { - // ^act + // ^act "First" } else { "Not First" } let _ternary = #kind("First", {name: "abc", age: 3}) != kindStr ? "Not First" : "First" - // ^act + // ^act } diff --git a/analysis/tests/src/expected/Actions.res.txt b/analysis/tests/src/expected/Actions.res.txt index 8b6763f3f..a3dcd3881 100644 --- a/analysis/tests/src/expected/Actions.res.txt +++ b/analysis/tests/src/expected/Actions.res.txt @@ -1,29 +1,27 @@ -Actions tests/src/Actions.res 4:10 -Hit _ifThenElse +Actions tests/src/Actions.res 4:20 Formatted: type kind = First | Second | Third type r = {name: string, age: int} let _ = (kind, kindStr) => { let _ifThenElse = switch kind { - | First => // ^act + | First => // ^act "First" | _ => "Not First" } let _ternary = #kind("First", {name: "abc", age: 3}) != kindStr ? "Not First" : "First" - // ^act + // ^act } -Actions tests/src/Actions.res 11:9 -Hit _ternary +Actions tests/src/Actions.res 11:17 Formatted: type kind = First | Second | Third type r = {name: string, age: int} let _ = (kind, kindStr) => { let _ifThenElse = if kind == First { - // ^act + // ^act "First" } else { "Not First" @@ -33,6 +31,6 @@ let _ = (kind, kindStr) => { | #kind("First", {name: "abc", age: 3}) => "First" | _ => "Not First" } - // ^act + // ^act } From 933aacf901d0a8f6a20cef065ca91d95b981418f Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 25 Mar 2022 11:28:18 +0100 Subject: [PATCH 10/51] tweak test --- analysis/tests/src/Actions.res | 19 ++++---- analysis/tests/src/expected/Actions.res.txt | 49 +++++++++++---------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/analysis/tests/src/Actions.res b/analysis/tests/src/Actions.res index b1ac5cf2a..4eddc4018 100644 --- a/analysis/tests/src/Actions.res +++ b/analysis/tests/src/Actions.res @@ -1,14 +1,15 @@ type kind = First | Second | Third type r = {name: string, age: int} -let _ = (kind, kindStr) => { - let _ifThenElse = if kind == First { - // ^act - "First" - } else { - "Not First" - } +let ret = _ => assert false +let kind = assert false - let _ternary = #kind("First", {name: "abc", age: 3}) != kindStr ? "Not First" : "First" - // ^act +if kind == First { + // ^act + ret("First") +} else { + ret("Not First") } + +#kind("First", {name: "abc", age: 3}) != kind ? ret("Not First") : ret("First") +// ^act diff --git a/analysis/tests/src/expected/Actions.res.txt b/analysis/tests/src/expected/Actions.res.txt index a3dcd3881..2800a7c68 100644 --- a/analysis/tests/src/expected/Actions.res.txt +++ b/analysis/tests/src/expected/Actions.res.txt @@ -1,36 +1,39 @@ -Actions tests/src/Actions.res 4:20 +Actions tests/src/Actions.res 6:5 Formatted: type kind = First | Second | Third type r = {name: string, age: int} -let _ = (kind, kindStr) => { - let _ifThenElse = switch kind { - | First => // ^act - "First" - | _ => "Not First" - } +let ret = _ => assert false +let kind = assert false - let _ternary = #kind("First", {name: "abc", age: 3}) != kindStr ? "Not First" : "First" - // ^act +switch kind { +| First => + // ^act + ret("First") +| _ => ret("Not First") } -Actions tests/src/Actions.res 11:17 +#kind("First", {name: "abc", age: 3}) != kind ? ret("Not First") : ret("First") +// ^act + +Actions tests/src/Actions.res 13:15 Formatted: type kind = First | Second | Third type r = {name: string, age: int} -let _ = (kind, kindStr) => { - let _ifThenElse = if kind == First { - // ^act - "First" - } else { - "Not First" - } - - let _ternary = switch kindStr { - | #kind("First", {name: "abc", age: 3}) => "First" - | _ => "Not First" - } - // ^act +let ret = _ => assert false +let kind = assert false + +if kind == First { + // ^act + ret("First") +} else { + ret("Not First") +} + +switch kind { +| #kind("First", {name: "abc", age: 3}) => ret("First") +// ^act +| _ => ret("Not First") } From f68c7db4ab1603a6f28a5e5b62e51e87b005d472 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 26 Mar 2022 10:45:52 +0100 Subject: [PATCH 11/51] Rename/cleanup. --- analysis/src/Actions.ml | 103 --------------- analysis/src/Commands.ml | 6 +- analysis/src/Xform.ml | 117 ++++++++++++++++++ analysis/tests/src/{Actions.res => Xform.res} | 4 +- analysis/tests/src/expected/Debug.res.txt | 4 +- .../{Actions.res.txt => Xform.res.txt} | 16 +-- 6 files changed, 132 insertions(+), 118 deletions(-) delete mode 100644 analysis/src/Actions.ml create mode 100644 analysis/src/Xform.ml rename analysis/tests/src/{Actions.res => Xform.res} (89%) rename analysis/tests/src/expected/{Actions.res.txt => Xform.res.txt} (75%) diff --git a/analysis/src/Actions.ml b/analysis/src/Actions.ml deleted file mode 100644 index 1bb3f73de..000000000 --- a/analysis/src/Actions.ml +++ /dev/null @@ -1,103 +0,0 @@ -let posInLoc ~pos ~loc = - Utils.tupleOfLexing loc.Location.loc_start <= pos - && pos < Utils.tupleOfLexing loc.loc_end - -let rec listToPat ~itemToPat = function - | [] -> Some [] - | x :: xList -> ( - match (itemToPat x, listToPat ~itemToPat xList) with - | Some p, Some pList -> Some (p :: pList) - | _ -> None) - -let rec expToPat (exp : Parsetree.expression) = - let mkPat ppat_desc = - Ast_helper.Pat.mk ~loc:exp.pexp_loc ~attrs:exp.pexp_attributes ppat_desc - in - match exp.pexp_desc with - | Pexp_construct (lid, None) -> Some (mkPat (Ppat_construct (lid, None))) - | Pexp_construct (lid, Some e1) -> ( - match expToPat e1 with - | None -> None - | Some p1 -> Some (mkPat (Ppat_construct (lid, Some p1)))) - | Pexp_variant (label, None) -> Some (mkPat (Ppat_variant (label, None))) - | Pexp_variant (label, Some e1) -> ( - match expToPat e1 with - | None -> None - | Some p1 -> Some (mkPat (Ppat_variant (label, Some p1)))) - | Pexp_constant c -> Some (mkPat (Ppat_constant c)) - | Pexp_tuple eList -> ( - match listToPat ~itemToPat:expToPat eList with - | None -> None - | Some patList -> Some (mkPat (Ppat_tuple patList))) - | Pexp_record (items, None) -> ( - let itemToPat (x, e) = - match expToPat e with None -> None | Some p -> Some (x, p) - in - match listToPat ~itemToPat items with - | None -> None - | Some patItems -> Some (mkPat (Ppat_record (patItems, Closed)))) - | Pexp_record (_, Some _) -> None - | _ -> None - -let mkMapper ~pos ~changed = - let expr (mapper : Ast_mapper.mapper) (e : Parsetree.expression) = - let newExp = - match e.pexp_desc with - | Pexp_ifthenelse - ( { - pexp_desc = - Pexp_apply - ( {pexp_desc = Pexp_ident {txt = Lident (("=" | "<>") as op)}}, - [(Nolabel, arg1); (Nolabel, arg2)] ); - }, - e1, - Some e2 ) - when posInLoc ~pos ~loc:e.pexp_loc -> ( - let e1, e2 = if op = "=" then (e1, e2) else (e2, e1) in - let mkMatch ~arg ~pat = - let cases = - [ - Ast_helper.Exp.case pat e1; - Ast_helper.Exp.case (Ast_helper.Pat.any ()) e2; - ] - in - Ast_helper.Exp.match_ ~loc:e.pexp_loc ~attrs:e.pexp_attributes arg - cases - in - - match expToPat arg2 with - | None -> ( - match expToPat arg1 with - | None -> None - | Some pat1 -> - let newExp = mkMatch ~arg:arg2 ~pat:pat1 in - Some newExp) - | Some pat2 -> - let newExp = mkMatch ~arg:arg1 ~pat:pat2 in - Some newExp) - | _ -> None - in - match newExp with - | Some newExp -> - changed := true; - newExp - | None -> Ast_mapper.default_mapper.expr mapper e - in - - {Ast_mapper.default_mapper with expr} - -let command ~path ~pos = - if Filename.check_suffix path ".res" then - let parser = - Res_driver.parsingEngine.parseImplementation ~forPrinter:false - in - let {Res_driver.parsetree = structure; comments} = parser ~filename:path in - let printer = - Res_printer.printImplementation ~width:!Res_cli.ResClflags.width ~comments - in - let changed = ref false in - let mapper = mkMapper ~pos ~changed in - let newStructure = mapper.structure mapper structure in - if !changed then - let formatted = printer newStructure in - Printf.printf "Formatted:\n%s" formatted diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 5dc74968b..06443313e 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -378,11 +378,11 @@ let test ~path = dir ++ parent_dir_name ++ "lib" ++ "bs" ++ "src" ++ name in Printf.printf "%s" (CreateInterface.command ~path ~cmiFile) - | "act" -> + | "xfm" -> print_endline - ("Actions " ^ path ^ " " ^ string_of_int line ^ ":" + ("Xform " ^ path ^ " " ^ string_of_int line ^ ":" ^ string_of_int col); - Actions.command ~path ~pos:(line, col) + Xform.command ~path ~pos:(line, col) | _ -> ()); print_newline ()) in diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml new file mode 100644 index 000000000..99a0d0420 --- /dev/null +++ b/analysis/src/Xform.ml @@ -0,0 +1,117 @@ +(** Code transformations using the parser/printer and ast operations *) + +let posInLoc ~pos ~loc = + Utils.tupleOfLexing loc.Location.loc_start <= pos + && pos < Utils.tupleOfLexing loc.loc_end + +module IfThenElse = struct + (* Convert if-then-else to switch *) + + let rec listToPat ~itemToPat = function + | [] -> Some [] + | x :: xList -> ( + match (itemToPat x, listToPat ~itemToPat xList) with + | Some p, Some pList -> Some (p :: pList) + | _ -> None) + + let rec expToPat (exp : Parsetree.expression) = + let mkPat ppat_desc = + Ast_helper.Pat.mk ~loc:exp.pexp_loc ~attrs:exp.pexp_attributes ppat_desc + in + match exp.pexp_desc with + | Pexp_construct (lid, None) -> Some (mkPat (Ppat_construct (lid, None))) + | Pexp_construct (lid, Some e1) -> ( + match expToPat e1 with + | None -> None + | Some p1 -> Some (mkPat (Ppat_construct (lid, Some p1)))) + | Pexp_variant (label, None) -> Some (mkPat (Ppat_variant (label, None))) + | Pexp_variant (label, Some e1) -> ( + match expToPat e1 with + | None -> None + | Some p1 -> Some (mkPat (Ppat_variant (label, Some p1)))) + | Pexp_constant c -> Some (mkPat (Ppat_constant c)) + | Pexp_tuple eList -> ( + match listToPat ~itemToPat:expToPat eList with + | None -> None + | Some patList -> Some (mkPat (Ppat_tuple patList))) + | Pexp_record (items, None) -> ( + let itemToPat (x, e) = + match expToPat e with None -> None | Some p -> Some (x, p) + in + match listToPat ~itemToPat items with + | None -> None + | Some patItems -> Some (mkPat (Ppat_record (patItems, Closed)))) + | Pexp_record (_, Some _) -> None + | _ -> None + + let mkMapper ~pos ~changed = + let expr (mapper : Ast_mapper.mapper) (e : Parsetree.expression) = + let newExp = + match e.pexp_desc with + | Pexp_ifthenelse + ( { + pexp_desc = + Pexp_apply + ( { + pexp_desc = + Pexp_ident {txt = Lident (("=" | "<>") as op)}; + }, + [(Nolabel, arg1); (Nolabel, arg2)] ); + }, + e1, + Some e2 ) + when posInLoc ~pos ~loc:e.pexp_loc -> ( + let e1, e2 = if op = "=" then (e1, e2) else (e2, e1) in + let mkMatch ~arg ~pat = + let cases = + [ + Ast_helper.Exp.case pat e1; + Ast_helper.Exp.case (Ast_helper.Pat.any ()) e2; + ] + in + Ast_helper.Exp.match_ ~loc:e.pexp_loc ~attrs:e.pexp_attributes arg + cases + in + + match expToPat arg2 with + | None -> ( + match expToPat arg1 with + | None -> None + | Some pat1 -> + let newExp = mkMatch ~arg:arg2 ~pat:pat1 in + Some newExp) + | Some pat2 -> + let newExp = mkMatch ~arg:arg1 ~pat:pat2 in + Some newExp) + | _ -> None + in + match newExp with + | Some newExp -> + changed := true; + newExp + | None -> Ast_mapper.default_mapper.expr mapper e + in + + {Ast_mapper.default_mapper with expr} + + let xform ~pos structure = + let changed = ref false in + let mapper = mkMapper ~pos ~changed in + let newStructure = mapper.structure mapper structure in + if !changed then Some newStructure else None +end + +let command ~path ~pos = + if Filename.check_suffix path ".res" then + let parser = + Res_driver.parsingEngine.parseImplementation ~forPrinter:false + in + let {Res_driver.parsetree = structure; comments} = parser ~filename:path in + let printer = + Res_printer.printImplementation ~width:!Res_cli.ResClflags.width ~comments + in + match IfThenElse.xform ~pos structure with + | None -> () + | Some newStructure -> + let formatted = printer newStructure in + Printf.printf "Hit IfThenElse. Formatted:\n%s" formatted diff --git a/analysis/tests/src/Actions.res b/analysis/tests/src/Xform.res similarity index 89% rename from analysis/tests/src/Actions.res rename to analysis/tests/src/Xform.res index 4eddc4018..4229ea8f4 100644 --- a/analysis/tests/src/Actions.res +++ b/analysis/tests/src/Xform.res @@ -5,11 +5,11 @@ let ret = _ => assert false let kind = assert false if kind == First { - // ^act + // ^xfm ret("First") } else { ret("Not First") } #kind("First", {name: "abc", age: 3}) != kind ? ret("Not First") : ret("First") -// ^act +// ^xfm diff --git a/analysis/tests/src/expected/Debug.res.txt b/analysis/tests/src/expected/Debug.res.txt index c10b692fb..d16c8b30a 100644 --- a/analysis/tests/src/expected/Debug.res.txt +++ b/analysis/tests/src/expected/Debug.res.txt @@ -4,8 +4,7 @@ Dependencies: @rescript/react Source directories: tests/node_modules/@rescript/react/src tests/node_modules/@rescript/react/src/legacy Source files: tests/node_modules/@rescript/react/src/React.res tests/node_modules/@rescript/react/src/ReactDOM.res tests/node_modules/@rescript/react/src/ReactDOMServer.res tests/node_modules/@rescript/react/src/ReactDOMStyle.res tests/node_modules/@rescript/react/src/ReactEvent.res tests/node_modules/@rescript/react/src/ReactEvent.resi tests/node_modules/@rescript/react/src/ReactTestUtils.res tests/node_modules/@rescript/react/src/ReactTestUtils.resi tests/node_modules/@rescript/react/src/RescriptReactErrorBoundary.res tests/node_modules/@rescript/react/src/RescriptReactErrorBoundary.resi tests/node_modules/@rescript/react/src/RescriptReactRouter.res tests/node_modules/@rescript/react/src/RescriptReactRouter.resi tests/node_modules/@rescript/react/src/legacy/ReactDOMRe.res tests/node_modules/@rescript/react/src/legacy/ReasonReact.res Source directories: tests/src tests/src/expected -Source files: tests/src/Actions.res tests/src/Auto.res tests/src/CompletePrioritize1.res tests/src/CompletePrioritize2.res tests/src/Completion.res tests/src/Component.res tests/src/Component.resi tests/src/CreateInterface.res tests/src/Cross.res tests/src/Debug.res tests/src/Definition.res tests/src/DefinitionWithInterface.res tests/src/DefinitionWithInterface.resi tests/src/Div.res tests/src/Fragment.res tests/src/Highlight.res tests/src/Hover.res tests/src/Jsx.res tests/src/Jsx.resi tests/src/LongIdentTest.res tests/src/Obj.res tests/src/Patterns.res tests/src/RecModules.res tests/src/RecordCompletion.res tests/src/References.res tests/src/ReferencesWithInterface.res tests/src/ReferencesWithInterface.resi tests/src/Rename.res tests/src/RenameWithInterface.res tests/src/RenameWithInterface.resi tests/src/TableclothMap.ml tests/src/TableclothMap.mli tests/src/TypeDefinition.res -Impl cmt:tests/lib/bs/src/Actions.cmt res:tests/src/Actions.res +Source files: tests/src/Auto.res tests/src/CompletePrioritize1.res tests/src/CompletePrioritize2.res tests/src/Completion.res tests/src/Component.res tests/src/Component.resi tests/src/CreateInterface.res tests/src/Cross.res tests/src/Debug.res tests/src/Definition.res tests/src/DefinitionWithInterface.res tests/src/DefinitionWithInterface.resi tests/src/Div.res tests/src/Fragment.res tests/src/Highlight.res tests/src/Hover.res tests/src/Jsx.res tests/src/Jsx.resi tests/src/LongIdentTest.res tests/src/Obj.res tests/src/Patterns.res tests/src/RecModules.res tests/src/RecordCompletion.res tests/src/References.res tests/src/ReferencesWithInterface.res tests/src/ReferencesWithInterface.resi tests/src/Rename.res tests/src/RenameWithInterface.res tests/src/RenameWithInterface.resi tests/src/TableclothMap.ml tests/src/TableclothMap.mli tests/src/TypeDefinition.res tests/src/Xform.res Impl cmt:tests/lib/bs/src/Auto.cmt res:tests/src/Auto.res Impl cmt:tests/lib/bs/src/CompletePrioritize1.cmt res:tests/src/CompletePrioritize1.res Impl cmt:tests/lib/bs/src/CompletePrioritize2.cmt res:tests/src/CompletePrioritize2.res @@ -32,6 +31,7 @@ Impl cmt:tests/lib/bs/src/Rename.cmt res:tests/src/Rename.res IntfAndImpl cmti:tests/lib/bs/src/RenameWithInterface.cmti resi:tests/src/RenameWithInterface.resi cmt:tests/lib/bs/src/RenameWithInterface.cmt res:tests/src/RenameWithInterface.res IntfAndImpl cmti:tests/lib/bs/src/TableclothMap.cmti resi:tests/src/TableclothMap.mli cmt:tests/lib/bs/src/TableclothMap.cmt res:tests/src/TableclothMap.ml Impl cmt:tests/lib/bs/src/TypeDefinition.cmt res:tests/src/TypeDefinition.res +Impl cmt:tests/lib/bs/src/Xform.cmt res:tests/src/Xform.res Dependency dirs: tests/node_modules/@rescript/react/lib/bs/src tests/node_modules/@rescript/react/lib/bs/src/legacy Opens from bsconfig: locItems: diff --git a/analysis/tests/src/expected/Actions.res.txt b/analysis/tests/src/expected/Xform.res.txt similarity index 75% rename from analysis/tests/src/expected/Actions.res.txt rename to analysis/tests/src/expected/Xform.res.txt index 2800a7c68..1dd67d1c6 100644 --- a/analysis/tests/src/expected/Actions.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -1,5 +1,5 @@ -Actions tests/src/Actions.res 6:5 -Formatted: +Xform tests/src/Xform.res 6:5 +Hit IfThenElse. Formatted: type kind = First | Second | Third type r = {name: string, age: int} @@ -8,16 +8,16 @@ let kind = assert false switch kind { | First => - // ^act + // ^xfm ret("First") | _ => ret("Not First") } #kind("First", {name: "abc", age: 3}) != kind ? ret("Not First") : ret("First") -// ^act +// ^xfm -Actions tests/src/Actions.res 13:15 -Formatted: +Xform tests/src/Xform.res 13:15 +Hit IfThenElse. Formatted: type kind = First | Second | Third type r = {name: string, age: int} @@ -25,7 +25,7 @@ let ret = _ => assert false let kind = assert false if kind == First { - // ^act + // ^xfm ret("First") } else { ret("Not First") @@ -33,7 +33,7 @@ if kind == First { switch kind { | #kind("First", {name: "abc", age: 3}) => ret("First") -// ^act +// ^xfm | _ => ret("Not First") } From 86a632b0eda5efad194c234da6d302d6b1b87478 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 26 Mar 2022 10:56:39 +0100 Subject: [PATCH 12/51] refactor --- analysis/src/Xform.ml | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 99a0d0420..1d0d58dca 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -101,17 +101,21 @@ module IfThenElse = struct if !changed then Some newStructure else None end +let parse ~filename = + let {Res_driver.parsetree = structure; comments} = + Res_driver.parsingEngine.parseImplementation ~forPrinter:false ~filename + in + let print ~structure = + Res_printer.printImplementation ~width:!Res_cli.ResClflags.width ~comments + structure + in + (structure, print) + let command ~path ~pos = if Filename.check_suffix path ".res" then - let parser = - Res_driver.parsingEngine.parseImplementation ~forPrinter:false - in - let {Res_driver.parsetree = structure; comments} = parser ~filename:path in - let printer = - Res_printer.printImplementation ~width:!Res_cli.ResClflags.width ~comments - in + let structure, print = parse ~filename:path in match IfThenElse.xform ~pos structure with | None -> () | Some newStructure -> - let formatted = printer newStructure in + let formatted = print newStructure in Printf.printf "Hit IfThenElse. Formatted:\n%s" formatted From 9a34ab40a7ff0e1489d2530e9250643ffe1cb5b7 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 26 Mar 2022 11:28:38 +0100 Subject: [PATCH 13/51] Use simple diff to output xform result. --- analysis/src/Xform.ml | 31 ++++++++++++++++++++++- analysis/tests/src/expected/Xform.res.txt | 27 ++------------------ 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 1d0d58dca..5a149e3b7 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -111,6 +111,29 @@ let parse ~filename = in (structure, print) +let diff ~filename ~newContents = + match Files.readFile ~filename with + | None -> assert false + | Some oldContents -> + let rec findFirstLineDifferent n old new_ = + match (old, new_) with + | old1 :: oldRest, new1 :: newRest -> + if old1 = new1 then findFirstLineDifferent (n + 1) oldRest newRest + else (n, old, new_) + | _ -> (n, old, new_) + in + let oldLines = String.split_on_char '\n' oldContents in + let newLines = String.split_on_char '\n' newContents in + let firstLineDifferent, old, new_ = + findFirstLineDifferent 0 oldLines newLines + in + let firstLineR, _oldR, newR = + findFirstLineDifferent 0 (List.rev old) (List.rev new_) + in + let lastLineEqual = firstLineDifferent + List.length old - firstLineR in + let newLines = List.rev newR in + (firstLineDifferent, lastLineEqual, newLines) + let command ~path ~pos = if Filename.check_suffix path ".res" then let structure, print = parse ~filename:path in @@ -118,4 +141,10 @@ let command ~path ~pos = | None -> () | Some newStructure -> let formatted = print newStructure in - Printf.printf "Hit IfThenElse. Formatted:\n%s" formatted + let firstLineDifferent, lastLineEqual, newLines = + diff ~filename:path ~newContents:formatted + in + Printf.printf + "Hit IfThenElse firstLineDifferent:%d lastLineEqual:%d newLines:\n%s\n" + firstLineDifferent lastLineEqual + (newLines |> String.concat "\n") diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 1dd67d1c6..4c3378e6b 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -1,36 +1,13 @@ Xform tests/src/Xform.res 6:5 -Hit IfThenElse. Formatted: -type kind = First | Second | Third -type r = {name: string, age: int} - -let ret = _ => assert false -let kind = assert false - +Hit IfThenElse firstLineDifferent:6 lastLineEqual:11 newLines: switch kind { | First => // ^xfm ret("First") | _ => ret("Not First") -} - -#kind("First", {name: "abc", age: 3}) != kind ? ret("Not First") : ret("First") -// ^xfm Xform tests/src/Xform.res 13:15 -Hit IfThenElse. Formatted: -type kind = First | Second | Third -type r = {name: string, age: int} - -let ret = _ => assert false -let kind = assert false - -if kind == First { - // ^xfm - ret("First") -} else { - ret("Not First") -} - +Hit IfThenElse firstLineDifferent:13 lastLineEqual:15 newLines: switch kind { | #kind("First", {name: "abc", age: 3}) => ret("First") // ^xfm From d3cf9cd0eb96d6d1ef948de5bed05c6ebcb08486 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 27 Mar 2022 10:40:25 +0200 Subject: [PATCH 14/51] wire up code actions with the analysis bin --- analysis/src/Commands.ml | 48 ++-------------------------------------- analysis/src/Xform.ml | 30 +++++++++++++++++++++++++ server/src/server.ts | 7 ++++-- 3 files changed, 37 insertions(+), 48 deletions(-) diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 06443313e..43b1b5eaa 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -55,52 +55,8 @@ let hover ~path ~line ~col = print_endline result let codeAction ~path ~line ~col = - let result = - match Cmt.fromPath ~path with - | None -> Protocol.null - | Some ({file} as full) -> ( - match References.getLocItem ~full ~line ~col with - | None -> Protocol.null - | Some locItem -> ( - let isModule = - match locItem.locType with - | SharedTypes.LModule _ | TopLevelModule _ -> true - | TypeDefinition _ | Typed _ | Constant _ -> false - in - let uriLocOpt = References.definitionForLocItem ~full locItem in - let skipZero = - match uriLocOpt with - | None -> false - | Some (_, loc) -> - let isInterface = file.uri |> Uri2.isInterface in - let posIsZero {Lexing.pos_lnum; pos_bol; pos_cnum} = - (not isInterface) && pos_lnum = 1 && pos_cnum - pos_bol = 0 - in - (* Skip if range is all zero, unless it's a module *) - (not isModule) && posIsZero loc.loc_start && posIsZero loc.loc_end - in - if skipZero then Protocol.null - else - match locItem.SharedTypes.locType with - | Typed (n, t, _) -> ( - match Printtyp.tree_of_typexp false t with - | Otyp_constr (Oide_ident "option", _) -> - let range = Utils.cmtLocToRange locItem.loc in - CodeActions.( - stringifyCodeActions - [ - CodeAction.makeRangeReplace ~title:"Unwrap optional" - ~kind:RefactorRewrite ~file:path - ~newText: - ("switch " ^ n - ^ " { | None => failWith(\"TODO\") | Some(" ^ n - ^ ") => _" ^ n ^ " }") - ~range; - ]) - | _ -> Protocol.null) - | _ -> Protocol.null)) - in - print_endline result + Xform.extractCodeActions ~path ~pos:(line, col) + |> CodeActions.stringifyCodeActions |> print_endline let definition ~path ~line ~col = let locationOpt = diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 5a149e3b7..3ecb4b540 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -148,3 +148,33 @@ let command ~path ~pos = "Hit IfThenElse firstLineDifferent:%d lastLineEqual:%d newLines:\n%s\n" firstLineDifferent lastLineEqual (newLines |> String.concat "\n") + +open CodeActions + +let extractCodeActions ~path ~pos = + if Filename.check_suffix path ".res" then ( + let structure, print = parse ~filename:path in + let actions = ref [] in + (match IfThenElse.xform ~pos structure with + | None -> () + | Some newStructure -> + let formatted = print newStructure in + let firstLineDifferent, lastLineEqual, newLines = + diff ~filename:path ~newContents:formatted + in + + actions := + CodeAction.makeRangeReplace ~title:"Replace with switch" + ~kind:RefactorRewrite + ~file:path + (* We add an explicit ending newline to the text to put the first + character after the replace back in the correct place. *) + ~newText:((newLines |> String.concat "\n") ^ "\n") + ~range: + { + start = {line = firstLineDifferent; character = 0}; + end_ = {line = lastLineEqual; character = 0}; + } + :: !actions); + !actions) + else [] diff --git a/server/src/server.ts b/server/src/server.ts index 3ee672803..8b5bcc990 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -466,10 +466,13 @@ function codeAction(msg: p.RequestMessage): p.ResponseMessage { id: msg.id, }; - res.result = + // We must send `null` when there are no results, empty array isn't enough. + let codeActions = result != null && Array.isArray(result) - ? [...localResults, result] + ? [...localResults, ...result] : localResults; + + res.result = codeActions.length > 0 ? codeActions : null; return res; } From 217ceee3a045c52153c27c89f6aff95f7a78e5e4 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Mon, 28 Mar 2022 20:15:59 +0200 Subject: [PATCH 15/51] Use local pretty-printing of single expression. --- analysis/src/Commands.ml | 9 +- analysis/src/Xform.ml | 101 ++++++++++++---------- analysis/tests/src/expected/Xform.res.txt | 6 +- 3 files changed, 66 insertions(+), 50 deletions(-) diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 43b1b5eaa..29c3b3a16 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -334,11 +334,16 @@ let test ~path = dir ++ parent_dir_name ++ "lib" ++ "bs" ++ "src" ++ name in Printf.printf "%s" (CreateInterface.command ~path ~cmiFile) - | "xfm" -> + | "xfm" -> ( print_endline ("Xform " ^ path ^ " " ^ string_of_int line ^ ":" ^ string_of_int col); - Xform.command ~path ~pos:(line, col) + match Xform.command ~path ~pos:(line, col) with + | Some (range, newText) -> + Printf.printf "Hit IfThenElse %s newText:\n%s\n" + (Protocol.stringifyRange range) + newText + | None -> ()) | _ -> ()); print_newline ()) in diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 3ecb4b540..71d1d3158 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -4,6 +4,15 @@ let posInLoc ~pos ~loc = Utils.tupleOfLexing loc.Location.loc_start <= pos && pos < Utils.tupleOfLexing loc.loc_end +let mkPosition (p : Lexing.position) = + let line, character = Utils.tupleOfLexing p in + {Protocol.line; character} + +let rangeOfLoc (loc : Location.t) = + let start = mkPosition loc.loc_start in + let end_ = mkPosition loc.loc_end in + {Protocol.start; end_} + module IfThenElse = struct (* Convert if-then-else to switch *) @@ -87,7 +96,7 @@ module IfThenElse = struct in match newExp with | Some newExp -> - changed := true; + changed := Some newExp; newExp | None -> Ast_mapper.default_mapper.expr mapper e in @@ -95,21 +104,46 @@ module IfThenElse = struct {Ast_mapper.default_mapper with expr} let xform ~pos structure = - let changed = ref false in + let changed = ref None in let mapper = mkMapper ~pos ~changed in - let newStructure = mapper.structure mapper structure in - if !changed then Some newStructure else None + let _ = mapper.structure mapper structure in + !changed end +let indent n text = + let spaces = String.make n ' ' in + let len = String.length text in + let text = + if len != 0 && text.[len - 1] = '\n' then String.sub text 0 (len - 1) + else text + in + let lines = String.split_on_char '\n' text in + match lines with + | [] -> "" + | [line] -> line + | line :: lines -> + line ^ "\n" + ^ (lines |> List.map (fun line -> spaces ^ line) |> String.concat "\n") + let parse ~filename = let {Res_driver.parsetree = structure; comments} = Res_driver.parsingEngine.parseImplementation ~forPrinter:false ~filename in - let print ~structure = - Res_printer.printImplementation ~width:!Res_cli.ResClflags.width ~comments - structure + let printExpr ~(range : Protocol.range) (expr : Parsetree.expression) = + let structure = [Ast_helper.Str.eval ~loc:expr.pexp_loc expr] in + let filterComments = function + (* Relevant comments in the range of the expression *) + | comment -> + posInLoc + ~pos:((Res_comment.loc comment).loc_start |> Utils.tupleOfLexing) + ~loc:expr.pexp_loc + in + structure + |> Res_printer.printImplementation ~width:!Res_cli.ResClflags.width + ~comments:(List.filter filterComments comments) + |> indent range.start.character in - (structure, print) + (structure, printExpr) let diff ~filename ~newContents = match Files.readFile ~filename with @@ -136,45 +170,22 @@ let diff ~filename ~newContents = let command ~path ~pos = if Filename.check_suffix path ".res" then - let structure, print = parse ~filename:path in + let structure, printExpr = parse ~filename:path in match IfThenElse.xform ~pos structure with - | None -> () - | Some newStructure -> - let formatted = print newStructure in - let firstLineDifferent, lastLineEqual, newLines = - diff ~filename:path ~newContents:formatted - in - Printf.printf - "Hit IfThenElse firstLineDifferent:%d lastLineEqual:%d newLines:\n%s\n" - firstLineDifferent lastLineEqual - (newLines |> String.concat "\n") + | None -> None + | Some newExpr -> + let range = rangeOfLoc newExpr.pexp_loc in + let newText = printExpr ~range newExpr in + Some (range, newText) + else None open CodeActions let extractCodeActions ~path ~pos = - if Filename.check_suffix path ".res" then ( - let structure, print = parse ~filename:path in - let actions = ref [] in - (match IfThenElse.xform ~pos structure with - | None -> () - | Some newStructure -> - let formatted = print newStructure in - let firstLineDifferent, lastLineEqual, newLines = - diff ~filename:path ~newContents:formatted - in - - actions := - CodeAction.makeRangeReplace ~title:"Replace with switch" - ~kind:RefactorRewrite - ~file:path - (* We add an explicit ending newline to the text to put the first - character after the replace back in the correct place. *) - ~newText:((newLines |> String.concat "\n") ^ "\n") - ~range: - { - start = {line = firstLineDifferent; character = 0}; - end_ = {line = lastLineEqual; character = 0}; - } - :: !actions); - !actions) - else [] + match command ~path ~pos with + | Some (range, newText) -> + [ + CodeAction.makeRangeReplace ~title:"Replace with switch" + ~kind:RefactorRewrite ~file:path ~newText ~range; + ] + | None -> [] diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 4c3378e6b..8461c7439 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -1,16 +1,16 @@ Xform tests/src/Xform.res 6:5 -Hit IfThenElse firstLineDifferent:6 lastLineEqual:11 newLines: +Hit IfThenElse {"start": {"line": 6, "character": 0}, "end": {"line": 11, "character": 1}} newText: switch kind { | First => // ^xfm ret("First") | _ => ret("Not First") +} Xform tests/src/Xform.res 13:15 -Hit IfThenElse firstLineDifferent:13 lastLineEqual:15 newLines: +Hit IfThenElse {"start": {"line": 13, "character": 0}, "end": {"line": 13, "character": 79}} newText: switch kind { | #kind("First", {name: "abc", age: 3}) => ret("First") -// ^xfm | _ => ret("Not First") } From 4f3eb50a8695f2e60d18b8da43d53243ecdaa108 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 29 Mar 2022 08:56:35 +0200 Subject: [PATCH 16/51] Make code actions work on unsaved files. --- analysis/src/Cli.ml | 5 +++-- analysis/src/CodeActions.ml | 9 ++------- analysis/src/Commands.ml | 6 +++--- analysis/src/Xform.ml | 12 ++++++------ server/src/server.ts | 7 +++++++ 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/analysis/src/Cli.ml b/analysis/src/Cli.ml index b780760b2..f2f6f6c54 100644 --- a/analysis/src/Cli.ml +++ b/analysis/src/Cli.ml @@ -75,8 +75,9 @@ let main () = | [_; "documentSymbol"; path] -> Commands.documentSymbol ~path | [_; "hover"; path; line; col] -> Commands.hover ~path ~line:(int_of_string line) ~col:(int_of_string col) - | [_; "codeAction"; path; line; col] -> - Commands.codeAction ~path ~line:(int_of_string line) ~col:(int_of_string col) + | [_; "codeAction"; path; line; col; currentFile] -> + Commands.codeAction ~path ~line:(int_of_string line) + ~col:(int_of_string col) ~currentFile | [_; "references"; path; line; col] -> Commands.references ~path ~line:(int_of_string line) ~col:(int_of_string col) diff --git a/analysis/src/CodeActions.ml b/analysis/src/CodeActions.ml index 300c759b1..70d148905 100644 --- a/analysis/src/CodeActions.ml +++ b/analysis/src/CodeActions.ml @@ -18,19 +18,14 @@ let stringifyCodeActions codeActions = Printf.sprintf {|%s|} (codeActions |> List.map stringifyCodeAction |> array) module CodeAction = struct - let makeRangeReplace ~title ~kind ~file ~newText ~range = + let makeRangeReplace ~title ~kind ~uri ~newText ~range = { title; kind; edit = { documentChanges = - [ - { - textDocument = {version = None; uri = file}; - edits = [{newText; range}]; - }; - ]; + [{textDocument = {version = None; uri}; edits = [{newText; range}]}]; }; } end diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 29c3b3a16..246bc9371 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -54,8 +54,8 @@ let hover ~path ~line ~col = in print_endline result -let codeAction ~path ~line ~col = - Xform.extractCodeActions ~path ~pos:(line, col) +let codeAction ~path ~line ~col ~currentFile = + Xform.extractCodeActions ~path ~pos:(line, col) ~currentFile |> CodeActions.stringifyCodeActions |> print_endline let definition ~path ~line ~col = @@ -338,7 +338,7 @@ let test ~path = print_endline ("Xform " ^ path ^ " " ^ string_of_int line ^ ":" ^ string_of_int col); - match Xform.command ~path ~pos:(line, col) with + match Xform.command ~currentFile:path ~pos:(line, col) with | Some (range, newText) -> Printf.printf "Hit IfThenElse %s newText:\n%s\n" (Protocol.stringifyRange range) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 71d1d3158..c7ad0c57b 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -168,9 +168,9 @@ let diff ~filename ~newContents = let newLines = List.rev newR in (firstLineDifferent, lastLineEqual, newLines) -let command ~path ~pos = - if Filename.check_suffix path ".res" then - let structure, printExpr = parse ~filename:path in +let command ~currentFile ~pos = + if Filename.check_suffix currentFile ".res" then + let structure, printExpr = parse ~filename:currentFile in match IfThenElse.xform ~pos structure with | None -> None | Some newExpr -> @@ -181,11 +181,11 @@ let command ~path ~pos = open CodeActions -let extractCodeActions ~path ~pos = - match command ~path ~pos with +let extractCodeActions ~path ~pos ~currentFile = + match command ~currentFile ~pos with | Some (range, newText) -> [ CodeAction.makeRangeReplace ~title:"Replace with switch" - ~kind:RefactorRewrite ~file:path ~newText ~range; + ~kind:RefactorRewrite ~uri:path ~newText ~range; ] | None -> [] diff --git a/server/src/server.ts b/server/src/server.ts index 8b5bcc990..be84ebd22 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -437,6 +437,9 @@ function completion(msg: p.RequestMessage) { function codeAction(msg: p.RequestMessage): p.ResponseMessage { let params = msg.params as p.CodeActionParams; let filePath = fileURLToPath(params.textDocument.uri); + let code = getOpenedFileContent(params.textDocument.uri); + let extension = path.extname(params.textDocument.uri); + let tmpname = utils.createFileInTempDir(extension); // Check local code actions coming from the diagnostics. let localResults: v.CodeAction[] = []; @@ -448,6 +451,7 @@ function codeAction(msg: p.RequestMessage): p.ResponseMessage { } ); + fs.writeFileSync(tmpname, code, { encoding: "utf-8" }); let response = utils.runAnalysisCommand( filePath, [ @@ -455,9 +459,12 @@ function codeAction(msg: p.RequestMessage): p.ResponseMessage { filePath, params.range.start.line, params.range.start.character, + tmpname ], msg ); + fs.unlink(tmpname, () => null); + let { result } = response; From f5f8125d06bb553964e8507a23e3f09694323664 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 29 Mar 2022 09:01:17 +0200 Subject: [PATCH 17/51] dead code --- analysis/src/Protocol.ml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/analysis/src/Protocol.ml b/analysis/src/Protocol.ml index feef0eca6..f67418d4f 100644 --- a/analysis/src/Protocol.ml +++ b/analysis/src/Protocol.ml @@ -13,7 +13,6 @@ type completionItem = { type hover = {contents : string} type location = {uri : string; range : range} type documentSymbolItem = {name : string; kind : int; location : location} -type codeAction = {newText : string; range : range} type renameFile = {oldUri : string; newUri : string} type textEdit = {range : range; newText : string} @@ -62,14 +61,6 @@ let stringifyCompletionItem c = let stringifyHover h = Printf.sprintf {|{"contents": "%s"}|} (Json.escape h.contents) -let stringifyCodeAction c = - Printf.sprintf - {|{ - "content": "%s", - "range": %s -}|} - (Json.escape c.newText) (stringifyRange c.range) - let stringifyLocation (h : location) = Printf.sprintf {|{"uri": "%s", "range": %s}|} (Json.escape h.uri) (stringifyRange h.range) From 119d4036cf492d0bdf79b7312697ac915a75e7a8 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 29 Mar 2022 09:07:33 +0200 Subject: [PATCH 18/51] Move types to protocol. --- analysis/src/CodeActions.ml | 40 ++++++++++++------------------------- analysis/src/Protocol.ml | 15 ++++++++++++++ analysis/src/Xform.ml | 6 ++---- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/analysis/src/CodeActions.ml b/analysis/src/CodeActions.ml index 70d148905..844e25bce 100644 --- a/analysis/src/CodeActions.ml +++ b/analysis/src/CodeActions.ml @@ -1,31 +1,17 @@ -open Protocol - -type kind = RefactorRewrite - -let kindToString kind = match kind with RefactorRewrite -> "refactor.rewrite" - -type codeAction = {title : string; kind : kind; edit : codeActionEdit} - -let stringifyCodeAction ca = - Printf.sprintf {|{"title": "%s", "kind": "%s", "edit": %s}|} ca.title - (kindToString ca.kind) - (ca.edit |> stringifyCodeActionEdit) - (* This is the return that's expected when resolving code actions *) -type result = codeAction list +type result = Protocol.codeAction list let stringifyCodeActions codeActions = - Printf.sprintf {|%s|} (codeActions |> List.map stringifyCodeAction |> array) + Printf.sprintf {|%s|} + (codeActions |> List.map Protocol.stringifyCodeAction |> Protocol.array) -module CodeAction = struct - let makeRangeReplace ~title ~kind ~uri ~newText ~range = - { - title; - kind; - edit = - { - documentChanges = - [{textDocument = {version = None; uri}; edits = [{newText; range}]}]; - }; - } -end +let make ~title ~kind ~uri ~newText ~range = + { + Protocol.title; + codeActionKind = kind; + edit = + { + documentChanges = + [{textDocument = {version = None; uri}; edits = [{newText; range}]}]; + }; + } diff --git a/analysis/src/Protocol.ml b/analysis/src/Protocol.ml index f67418d4f..218d77775 100644 --- a/analysis/src/Protocol.ml +++ b/analysis/src/Protocol.ml @@ -27,6 +27,13 @@ type textDocumentEdit = { } type codeActionEdit = {documentChanges : textDocumentEdit list} +type codeActionKind = RefactorRewrite + +type codeAction = { + title : string; + codeActionKind : codeActionKind; + edit : codeActionEdit; +} let null = "null" let array l = "[" ^ String.concat ", " l ^ "]" @@ -106,6 +113,14 @@ let stringifyTextDocumentEdit tde = (stringifyoptionalVersionedTextDocumentIdentifier tde.textDocument) (tde.edits |> List.map stringifyTextEdit |> array) +let codeActionKindToString kind = + match kind with RefactorRewrite -> "refactor.rewrite" + let stringifyCodeActionEdit cae = Printf.sprintf {|{"documentChanges": %s}|} (cae.documentChanges |> List.map stringifyTextDocumentEdit |> array) + +let stringifyCodeAction ca = + Printf.sprintf {|{"title": "%s", "kind": "%s", "edit": %s}|} ca.title + (codeActionKindToString ca.codeActionKind) + (ca.edit |> stringifyCodeActionEdit) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index c7ad0c57b..9f2d25ee4 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -179,13 +179,11 @@ let command ~currentFile ~pos = Some (range, newText) else None -open CodeActions - let extractCodeActions ~path ~pos ~currentFile = match command ~currentFile ~pos with | Some (range, newText) -> [ - CodeAction.makeRangeReplace ~title:"Replace with switch" - ~kind:RefactorRewrite ~uri:path ~newText ~range; + CodeActions.make ~title:"Replace with switch" ~kind:RefactorRewrite + ~uri:path ~newText ~range; ] | None -> [] From fedb5f0da85007e5dea7dffa7da13230697007d7 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 29 Mar 2022 09:24:03 +0200 Subject: [PATCH 19/51] Refactor to allow more code actions later. --- analysis/src/Commands.ml | 23 +++++++++++----- analysis/src/Xform.ml | 32 ++++++++++------------- analysis/tests/src/expected/Xform.res.txt | 6 +++-- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 246bc9371..b77176d45 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -334,16 +334,25 @@ let test ~path = dir ++ parent_dir_name ++ "lib" ++ "bs" ++ "src" ++ name in Printf.printf "%s" (CreateInterface.command ~path ~cmiFile) - | "xfm" -> ( + | "xfm" -> print_endline ("Xform " ^ path ^ " " ^ string_of_int line ^ ":" ^ string_of_int col); - match Xform.command ~currentFile:path ~pos:(line, col) with - | Some (range, newText) -> - Printf.printf "Hit IfThenElse %s newText:\n%s\n" - (Protocol.stringifyRange range) - newText - | None -> ()) + let codeActions = + Xform.extractCodeActions ~path ~pos:(line, col) ~currentFile:path + in + codeActions + |> List.iter (fun codeAction -> + match codeAction with + | { + Protocol.title; + edit = {documentChanges = [{edits = [{range; newText}]}]}; + } -> + Printf.printf "Hit %s %s\nnewText:\n%s\n" title + (Protocol.stringifyRange range) + newText + | _ -> assert false + (* TODO *)) | _ -> ()); print_newline ()) in diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 9f2d25ee4..062bbbb82 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -168,22 +168,18 @@ let diff ~filename ~newContents = let newLines = List.rev newR in (firstLineDifferent, lastLineEqual, newLines) -let command ~currentFile ~pos = - if Filename.check_suffix currentFile ".res" then - let structure, printExpr = parse ~filename:currentFile in - match IfThenElse.xform ~pos structure with - | None -> None - | Some newExpr -> - let range = rangeOfLoc newExpr.pexp_loc in - let newText = printExpr ~range newExpr in - Some (range, newText) - else None - let extractCodeActions ~path ~pos ~currentFile = - match command ~currentFile ~pos with - | Some (range, newText) -> - [ - CodeActions.make ~title:"Replace with switch" ~kind:RefactorRewrite - ~uri:path ~newText ~range; - ] - | None -> [] + let codeActions = ref [] in + (if Filename.check_suffix currentFile ".res" then + let structure, printExpr = parse ~filename:currentFile in + match IfThenElse.xform ~pos structure with + | None -> () + | Some newExpr -> + let range = rangeOfLoc newExpr.pexp_loc in + let newText = printExpr ~range newExpr in + let codeAction = + CodeActions.make ~title:"Replace with switch" ~kind:RefactorRewrite + ~uri:path ~newText ~range + in + codeActions := codeAction :: !codeActions); + !codeActions diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 8461c7439..83724bf40 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -1,5 +1,6 @@ Xform tests/src/Xform.res 6:5 -Hit IfThenElse {"start": {"line": 6, "character": 0}, "end": {"line": 11, "character": 1}} newText: +Hit Replace with switch {"start": {"line": 6, "character": 0}, "end": {"line": 11, "character": 1}} +newText: switch kind { | First => // ^xfm @@ -8,7 +9,8 @@ switch kind { } Xform tests/src/Xform.res 13:15 -Hit IfThenElse {"start": {"line": 13, "character": 0}, "end": {"line": 13, "character": 79}} newText: +Hit Replace with switch {"start": {"line": 13, "character": 0}, "end": {"line": 13, "character": 79}} +newText: switch kind { | #kind("First", {name: "abc", age: 3}) => ret("First") | _ => ret("Not First") From f4edc44e1dc648f2d10fa7aa46e7cb8df7430b74 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 29 Mar 2022 09:38:11 +0200 Subject: [PATCH 20/51] More generic test printing of code actions. --- analysis/src/Commands.ml | 20 +++++++++----------- analysis/tests/src/expected/Xform.res.txt | 6 ++++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index b77176d45..192a41435 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -342,17 +342,15 @@ let test ~path = Xform.extractCodeActions ~path ~pos:(line, col) ~currentFile:path in codeActions - |> List.iter (fun codeAction -> - match codeAction with - | { - Protocol.title; - edit = {documentChanges = [{edits = [{range; newText}]}]}; - } -> - Printf.printf "Hit %s %s\nnewText:\n%s\n" title - (Protocol.stringifyRange range) - newText - | _ -> assert false - (* TODO *)) + |> List.iter (fun {Protocol.title; edit = {documentChanges}} -> + Printf.printf "Hit: %s\n" title; + documentChanges + |> List.iter (fun {Protocol.edits} -> + edits + |> List.iter (fun {Protocol.range; newText} -> + Printf.printf "%s\nnewText:\n%s\n" + (Protocol.stringifyRange range) + newText))) | _ -> ()); print_newline ()) in diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 83724bf40..f7de4241c 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -1,5 +1,6 @@ Xform tests/src/Xform.res 6:5 -Hit Replace with switch {"start": {"line": 6, "character": 0}, "end": {"line": 11, "character": 1}} +Hit: Replace with switch +{"start": {"line": 6, "character": 0}, "end": {"line": 11, "character": 1}} newText: switch kind { | First => @@ -9,7 +10,8 @@ switch kind { } Xform tests/src/Xform.res 13:15 -Hit Replace with switch {"start": {"line": 13, "character": 0}, "end": {"line": 13, "character": 79}} +Hit: Replace with switch +{"start": {"line": 13, "character": 0}, "end": {"line": 13, "character": 79}} newText: switch kind { | #kind("First", {name: "abc", age: 3}) => ret("First") From 04d9639093caa723090c8e1d24c2c61048472ac8 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 29 Mar 2022 11:05:42 +0200 Subject: [PATCH 21/51] Use iterator instead of mapper. --- analysis/src/Xform.ml | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 062bbbb82..b49610159 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -53,8 +53,8 @@ module IfThenElse = struct | Pexp_record (_, Some _) -> None | _ -> None - let mkMapper ~pos ~changed = - let expr (mapper : Ast_mapper.mapper) (e : Parsetree.expression) = + let mkIterator ~pos ~changed = + let expr (iterator : Ast_iterator.iterator) (e : Parsetree.expression) = let newExp = match e.pexp_desc with | Pexp_ifthenelse @@ -95,18 +95,16 @@ module IfThenElse = struct | _ -> None in match newExp with - | Some newExp -> - changed := Some newExp; - newExp - | None -> Ast_mapper.default_mapper.expr mapper e + | Some newExp -> changed := Some newExp + | None -> Ast_iterator.default_iterator.expr iterator e in - {Ast_mapper.default_mapper with expr} + {Ast_iterator.default_iterator with expr} let xform ~pos structure = let changed = ref None in - let mapper = mkMapper ~pos ~changed in - let _ = mapper.structure mapper structure in + let iterator = mkIterator ~pos ~changed in + iterator.structure iterator structure; !changed end From 8edb55151f77171eb6a3d2cc1a5f2f8a641ae9ef Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Wed, 30 Mar 2022 10:34:24 +0200 Subject: [PATCH 22/51] Keep information on whether a value definition is type annotated. --- analysis/src/Hover.ml | 6 ++-- analysis/src/ProcessAttributes.ml | 5 ++-- analysis/src/ProcessCmt.ml | 50 ++++++++++++++++++++----------- analysis/src/References.ml | 12 ++++---- analysis/src/SharedTypes.ml | 6 ++-- 5 files changed, 49 insertions(+), 30 deletions(-) diff --git a/analysis/src/Hover.ml b/analysis/src/Hover.ml index a8498c4ab..a55591502 100644 --- a/analysis/src/Hover.ml +++ b/analysis/src/Hover.ml @@ -41,8 +41,8 @@ let newHover ~full:{file; package} locItem = | TypeDefinition (name, decl, _stamp) -> let typeDef = Shared.declToString name decl in Some (codeBlock typeDef) - | LModule (Definition (stamp, _tip)) | LModule (LocalReference (stamp, _tip)) - -> ( + | LModule (Definition (stamp, _tip, _)) + | LModule (LocalReference (stamp, _tip)) -> ( match Stamps.findModule file.stamps stamp with | None -> None | Some md -> ( @@ -85,7 +85,7 @@ let newHover ~full:{file; package} locItem = | Some file -> showModule ~docstring:file.structure.docstring ~name:file.moduleName ~file None) - | Typed (_, _, Definition (_, (Field _ | Constructor _))) -> None + | Typed (_, _, Definition (_, (Field _ | Constructor _), _)) -> None | Constant t -> Some (codeBlock diff --git a/analysis/src/ProcessAttributes.ml b/analysis/src/ProcessAttributes.ml index 0836b7596..1de622952 100644 --- a/analysis/src/ProcessAttributes.ml +++ b/analysis/src/ProcessAttributes.ml @@ -34,14 +34,15 @@ let rec findDeprecatedAttribute attributes = | ({Asttypes.txt = "deprecated"}, _) :: _ -> Some "" | _ :: rest -> findDeprecatedAttribute rest -let newDeclared ~item ~scope ~extent ~name ~stamp ~modulePath isExported - attributes = +let newDeclared ~item ~scope ~extent ~name ~stamp ~modulePath ~isExported + ?(isTypeAnnotated = false) attributes = { Declared.name; stamp; extentLoc = extent; scopeLoc = scope; isExported; + isTypeAnnotated; modulePath; deprecated = findDeprecatedAttribute attributes; docstring = diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index 24dd7c122..e07a425ef 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -22,7 +22,7 @@ let sigItemsExtent items = items |> List.map (fun item -> item.Typedtree.sig_loc) |> locsExtent let addItem ~(name : string Location.loc) ~extent ~stamp ~(env : Env.t) ~item - attributes addExported addStamp = + ?(isTypeAnnotated = false) attributes addExported addStamp = let isExported = addExported name.txt stamp in let declared = ProcessAttributes.newDeclared ~item @@ -32,7 +32,8 @@ let addItem ~(name : string Location.loc) ~extent ~stamp ~(env : Env.t) ~item loc_end = env.scope.loc_end; loc_ghost = false; } - ~extent ~name ~stamp ~modulePath:env.modulePath isExported attributes + ~extent ~name ~stamp ~modulePath:env.modulePath ~isExported + ~isTypeAnnotated attributes in addStamp env.stamps stamp declared; declared @@ -45,7 +46,8 @@ let rec forTypeSignatureItem ~env ~(exported : Exported.t) let declared = addItem ~name:(Location.mknoloc (Ident.name ident)) - ~extent:loc ~stamp:(Ident.binding_time ident) ~env ~item val_attributes + ~extent:loc ~stamp:(Ident.binding_time ident) ~env ~item + ~isTypeAnnotated:true val_attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -107,7 +109,8 @@ let rec forTypeSignatureItem ~env ~(exported : Exported.t) } ~name:(Location.mknoloc name) ~stamp (* TODO maybe this needs another child *) - ~modulePath:env.modulePath true cd_attributes + ~modulePath:env.modulePath ~isExported:true + cd_attributes in Stamps.addConstructor env.stamps stamp declared; item)) @@ -247,7 +250,8 @@ let rec forSignatureItem ~env ~(exported : Exported.t) let declared = addItem ~name ~stamp:(Ident.binding_time val_id) - ~extent:val_loc ~item:val_desc.ctyp_type ~env val_attributes + ~extent:val_loc ~item:val_desc.ctyp_type ~env ~isTypeAnnotated:true + val_attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -341,6 +345,10 @@ let rec getModulePath mod_desc = | Tmod_constraint (expr, _typ, _constraint, _coercion) -> getModulePath expr.mod_desc +let patIsTypeAnnotated pat = + pat.pat_extra + |> List.exists (function Tpat_constraint _, _, _ -> true | _ -> false) + let rec forStructureItem ~env ~(exported : Exported.t) item = match item.str_desc with | Tstr_value (_isRec, bindings) -> @@ -352,7 +360,8 @@ let rec forStructureItem ~env ~(exported : Exported.t) item = let item = pat.pat_type in let declared = addItem ~name ~stamp:(Ident.binding_time ident) ~env - ~extent:pat.pat_loc ~item attributes + ~extent:pat.pat_loc ~item ~isTypeAnnotated:(patIsTypeAnnotated pat) + attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -443,7 +452,7 @@ let rec forStructureItem ~env ~(exported : Exported.t) item = let declared = addItem ~extent:val_loc ~item:val_type ~name ~stamp:(Ident.binding_time val_id) - ~env val_attributes + ~env ~isTypeAnnotated:true val_attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -495,7 +504,8 @@ and forModule env mod_desc moduleName = loc_end = env.scope.loc_end; loc_ghost = false; } - ~extent:t.Typedtree.mty_loc ~stamp ~modulePath:NotVisible false [] + ~extent:t.Typedtree.mty_loc ~stamp ~modulePath:NotVisible + ~isExported:false [] in Stamps.addModule env.stamps stamp declared)); forModule env resultExpr.mod_desc moduleName @@ -629,12 +639,14 @@ let extraForFile ~(file : File.t) = in file.stamps |> Stamps.iterModules (fun stamp (d : Module.t Declared.t) -> - addLocItem extra d.name.loc (LModule (Definition (stamp, Module))); + addLocItem extra d.name.loc + (LModule (Definition (stamp, Module, false))); addReference stamp d.name.loc); file.stamps |> Stamps.iterValues (fun stamp (d : Types.type_expr Declared.t) -> addLocItem extra d.name.loc - (Typed (d.name.txt, d.item, Definition (stamp, Value))); + (Typed + (d.name.txt, d.item, Definition (stamp, Value, d.isTypeAnnotated))); addReference stamp d.name.loc); file.stamps |> Stamps.iterTypes (fun stamp (d : Type.t Declared.t) -> @@ -648,7 +660,9 @@ let extraForFile ~(file : File.t) = addReference stamp fname.loc; addLocItem extra fname.loc (Typed - (d.name.txt, typ, Definition (d.stamp, Field fname.txt)))) + ( d.name.txt, + typ, + Definition (d.stamp, Field fname.txt, false) ))) | Variant constructors -> constructors |> List.iter (fun {Constructor.stamp; cname} -> @@ -669,7 +683,7 @@ let extraForFile ~(file : File.t) = (Typed ( d.name.txt, t, - Definition (d.stamp, Constructor cname.txt) ))) + Definition (d.stamp, Constructor cname.txt, false) ))) | _ -> ()); extra @@ -1002,12 +1016,13 @@ struct loc_start = val_loc.loc_end; loc_end = (currentScopeExtent ()).loc_end; } - ~modulePath:NotVisible ~item:val_desc.ctyp_type false val_attributes + ~modulePath:NotVisible ~item:val_desc.ctyp_type ~isExported:false + ~isTypeAnnotated:true val_attributes in Stamps.addValue Collector.file.stamps stamp declared; addReference stamp name.loc; addLocItem extra name.loc - (Typed (name.txt, val_desc.ctyp_type, Definition (stamp, Value)))) + (Typed (name.txt, val_desc.ctyp_type, Definition (stamp, Value, true)))) | _ -> () let enter_core_type {ctyp_type; ctyp_desc} = @@ -1016,7 +1031,7 @@ struct addForLongident (Some (ctyp_type, Type)) path txt loc | _ -> () - let enter_pattern {pat_desc; pat_loc; pat_type; pat_attributes} = + let enter_pattern ({pat_desc; pat_loc; pat_type; pat_attributes} as pat) = let addForPattern stamp name = if Stamps.findValue Collector.file.stamps stamp = None then ( let declared = @@ -1027,13 +1042,14 @@ struct loc_start = pat_loc.loc_end; loc_end = (currentScopeExtent ()).loc_end; } - ~modulePath:NotVisible ~extent:pat_loc ~item:pat_type false + ~modulePath:NotVisible ~extent:pat_loc ~item:pat_type + ~isExported:false ~isTypeAnnotated:(patIsTypeAnnotated pat) pat_attributes in Stamps.addValue Collector.file.stamps stamp declared; addReference stamp name.loc; addLocItem extra name.loc - (Typed (name.txt, pat_type, Definition (stamp, Value)))) + (Typed (name.txt, pat_type, Definition (stamp, Value, false)))) in (* Log.log("Entering pattern " ++ Utils.showLocation(pat_loc)); *) match pat_desc with diff --git a/analysis/src/References.ml b/analysis/src/References.ml index 051ca095a..c3e11007e 100644 --- a/analysis/src/References.ml +++ b/analysis/src/References.ml @@ -67,7 +67,7 @@ let getLocItem ~full ~line ~col = Some li3 | [ {locType = Typed (_, _, LocalReference (_, Value))}; - ({locType = Typed (_, _, Definition (_, Value))} as li2); + ({locType = Typed (_, _, Definition (_, Value, _))} as li2); ] -> (* JSX on type-annotated labeled (~arg:t): (~arg:t) becomes Props#arg @@ -138,7 +138,7 @@ let definedForLoc ~file ~package locKind = in match locKind with | NotFound -> None - | LocalReference (stamp, tip) | Definition (stamp, tip) -> + | LocalReference (stamp, tip) | Definition (stamp, tip, _) -> inner ~file stamp tip | GlobalReference (moduleName, path, tip) -> ( maybeLog ("Getting global " ^ moduleName); @@ -306,7 +306,7 @@ let definition ~file ~package stamp (tip : Tip.t) = let definitionForLocItem ~full:{file; package} locItem = match locItem.locType with - | Typed (_, _, Definition (stamp, tip)) -> ( + | Typed (_, _, Definition (stamp, tip, _)) -> ( maybeLog ("Typed Definition stamp:" ^ string_of_int stamp ^ " tip:" ^ Tip.toString tip); @@ -323,7 +323,7 @@ let definitionForLocItem ~full:{file; package} locItem = Some (file.uri, loc)) else None) | Typed (_, _, NotFound) - | LModule (NotFound | Definition (_, _)) + | LModule (NotFound | Definition _) | TypeDefinition (_, _, _) | Constant _ -> None @@ -530,8 +530,8 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem = List.append targetModuleReferences otherModulesReferences | Typed (_, _, NotFound) | LModule NotFound | Constant _ -> [] | TypeDefinition (_, _, stamp) -> forLocalStamp ~full stamp Type - | Typed (_, _, (LocalReference (stamp, tip) | Definition (stamp, tip))) - | LModule (LocalReference (stamp, tip) | Definition (stamp, tip)) -> + | Typed (_, _, (LocalReference (stamp, tip) | Definition (stamp, tip, _))) + | LModule (LocalReference (stamp, tip) | Definition (stamp, tip, _)) -> maybeLog ("Finding references for " ^ Uri2.toString file.uri ^ " and stamp " ^ string_of_int stamp ^ " and tip " ^ Tip.toString tip); diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index 5f6b91663..cf5a77193 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -132,6 +132,7 @@ module Declared = struct stamp : int; modulePath : modulePath; isExported : bool; + isTypeAnnotated : bool; deprecated : string option; docstring : string list; item : 'item; @@ -297,7 +298,7 @@ type locKind = | LocalReference of int * Tip.t | GlobalReference of string * string list * Tip.t | NotFound - | Definition of int * Tip.t + | Definition of int * Tip.t * (* whether it is type annotated *) bool type locType = | Typed of string * Types.type_expr * locKind @@ -370,7 +371,8 @@ let locKindToString = function | LocalReference (_, tip) -> "(LocalReference " ^ Tip.toString tip ^ ")" | GlobalReference _ -> "GlobalReference" | NotFound -> "NotFound" - | Definition (_, tip) -> "(Definition " ^ Tip.toString tip ^ ")" + | Definition (_, tip, _hasTypeAnnotation) -> + "(Definition " ^ Tip.toString tip ^ ")" let locTypeToString = function | Typed (name, e, locKind) -> From ab90b5e6c3c813b8bd477a23f192f9b24476204d Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Wed, 30 Mar 2022 10:38:37 +0200 Subject: [PATCH 23/51] First version of action to add type annotation. If a value declaration (such as a let or a case in a switch) is not type annotated already, add the option to add the type annotation. --- analysis/src/Xform.ml | 55 ++++++++++++++++++----- analysis/tests/src/Xform.res | 17 +++++++ analysis/tests/src/expected/Xform.res.txt | 14 ++++++ 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index b49610159..316fd8cc2 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -108,6 +108,30 @@ module IfThenElse = struct !changed end +module AddTypeAnnotation = struct + (* Add type annotation to value declaration *) + + let getAction ~path ~pos ~full = + let line, col = pos in + match References.getLocItem ~full ~line ~col with + | None -> None + | Some locItem -> ( + match locItem.locType with + | Typed (name, typ, Definition (stamp, Value, false (* isTypeAnnotated *))) + -> + if false then + Printf.printf "Definition %s stamp:%d isTypeAnnotated:%b\n" name stamp + false; + let range = rangeOfLoc locItem.loc in + let newText = name ^ " : " ^ (typ |> Shared.typeToString) in + let codeAction = + CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite + ~uri:path ~newText ~range + in + Some codeAction + | _ -> None) +end + let indent n text = let spaces = String.make n ' ' in let len = String.length text in @@ -168,16 +192,23 @@ let diff ~filename ~newContents = let extractCodeActions ~path ~pos ~currentFile = let codeActions = ref [] in - (if Filename.check_suffix currentFile ".res" then - let structure, printExpr = parse ~filename:currentFile in - match IfThenElse.xform ~pos structure with - | None -> () - | Some newExpr -> - let range = rangeOfLoc newExpr.pexp_loc in - let newText = printExpr ~range newExpr in - let codeAction = - CodeActions.make ~title:"Replace with switch" ~kind:RefactorRewrite - ~uri:path ~newText ~range - in - codeActions := codeAction :: !codeActions); + if Filename.check_suffix currentFile ".res" then ( + let structure, printExpr = parse ~filename:currentFile in + let fullOpt = Cmt.fromPath ~path in + (match fullOpt with + | None -> () + | Some full -> ( + match AddTypeAnnotation.getAction ~path ~pos ~full with + | None -> () + | Some action -> codeActions := action :: !codeActions)); + match IfThenElse.xform ~pos structure with + | None -> () + | Some newExpr -> + let range = rangeOfLoc newExpr.pexp_loc in + let newText = printExpr ~range newExpr in + let codeAction = + CodeActions.make ~title:"Replace with switch" ~kind:RefactorRewrite + ~uri:path ~newText ~range + in + codeActions := codeAction :: !codeActions); !codeActions diff --git a/analysis/tests/src/Xform.res b/analysis/tests/src/Xform.res index 4229ea8f4..97ba4eb16 100644 --- a/analysis/tests/src/Xform.res +++ b/analysis/tests/src/Xform.res @@ -13,3 +13,20 @@ if kind == First { #kind("First", {name: "abc", age: 3}) != kind ? ret("Not First") : ret("First") // ^xfm + +let name = "hello" +// ^xfm + +let annotated: int = 34 +// ^xfm + +module T = { + type r = {a: int, x: string} +} + +let foo = x => + switch x { + | None => 33 + | Some(q) => q.T.a + 1 + // ^xfm + } diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index f7de4241c..da1981e01 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -18,3 +18,17 @@ switch kind { | _ => ret("Not First") } +Xform tests/src/Xform.res 16:5 +Hit: Add type annotation +{"start": {"line": 16, "character": 4}, "end": {"line": 16, "character": 8}} +newText: +name : string + +Xform tests/src/Xform.res 19:5 + +Xform tests/src/Xform.res 29:9 +Hit: Add type annotation +{"start": {"line": 29, "character": 9}, "end": {"line": 29, "character": 10}} +newText: +q : T.r + From 2a2773dbb94be303dcd8bcb7014b9c3869f817b7 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Wed, 30 Mar 2022 10:44:37 +0200 Subject: [PATCH 24/51] Only send type annotation. --- analysis/src/Xform.ml | 12 +++++------- analysis/tests/src/expected/Xform.res.txt | 8 ++++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 316fd8cc2..3adad770e 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -117,13 +117,11 @@ module AddTypeAnnotation = struct | None -> None | Some locItem -> ( match locItem.locType with - | Typed (name, typ, Definition (stamp, Value, false (* isTypeAnnotated *))) - -> - if false then - Printf.printf "Definition %s stamp:%d isTypeAnnotated:%b\n" name stamp - false; - let range = rangeOfLoc locItem.loc in - let newText = name ^ " : " ^ (typ |> Shared.typeToString) in + | Typed (_name, typ, Definition (_, Value, false (* isTypeAnnotated *))) -> + let range = + rangeOfLoc {locItem.loc with loc_start = locItem.loc.loc_end} + in + let newText = " : " ^ (typ |> Shared.typeToString) in let codeAction = CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite ~uri:path ~newText ~range diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index da1981e01..613221d1b 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -20,15 +20,15 @@ switch kind { Xform tests/src/Xform.res 16:5 Hit: Add type annotation -{"start": {"line": 16, "character": 4}, "end": {"line": 16, "character": 8}} +{"start": {"line": 16, "character": 8}, "end": {"line": 16, "character": 8}} newText: -name : string + : string Xform tests/src/Xform.res 19:5 Xform tests/src/Xform.res 29:9 Hit: Add type annotation -{"start": {"line": 29, "character": 9}, "end": {"line": 29, "character": 10}} +{"start": {"line": 29, "character": 10}, "end": {"line": 29, "character": 10}} newText: -q : T.r + : T.r From a51d5c7db9c562f22d4f61ee45d420452dfdf5c2 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 30 Mar 2022 11:41:28 +0200 Subject: [PATCH 25/51] mark code actions from diagnostics as quick fixes --- server/src/codeActions.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 2db18d972..00f08136c 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -89,6 +89,8 @@ let didYouMeanAction: codeActionExtractor = ({ }, }, diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, }; codeActions[file].push({ @@ -206,6 +208,8 @@ let addUndefinedRecordFields: codeActionExtractor = ({ }, }, diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, }; codeActions[file].push({ @@ -274,6 +278,8 @@ let simpleConversion: codeActionExtractor = ({ }, }, diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, }; codeActions[file].push({ @@ -332,6 +338,8 @@ let topLevelUnitType: codeActionExtractor = ({ }, }, diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, }; codeActions[file].push({ From 042d3580b9c20c778e44071eeb80ffeb4f97d6d9 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 30 Mar 2022 11:41:43 +0200 Subject: [PATCH 26/51] no leading space in generated type annotations --- analysis/src/Xform.ml | 2 +- analysis/tests/src/expected/Xform.res.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 3adad770e..47b060906 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -121,7 +121,7 @@ module AddTypeAnnotation = struct let range = rangeOfLoc {locItem.loc with loc_start = locItem.loc.loc_end} in - let newText = " : " ^ (typ |> Shared.typeToString) in + let newText = ": " ^ (typ |> Shared.typeToString) in let codeAction = CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite ~uri:path ~newText ~range diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 613221d1b..8cebe9fad 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -22,7 +22,7 @@ Xform tests/src/Xform.res 16:5 Hit: Add type annotation {"start": {"line": 16, "character": 8}, "end": {"line": 16, "character": 8}} newText: - : string +: string Xform tests/src/Xform.res 19:5 @@ -30,5 +30,5 @@ Xform tests/src/Xform.res 29:9 Hit: Add type annotation {"start": {"line": 29, "character": 10}, "end": {"line": 29, "character": 10}} newText: - : T.r +: T.r From 6e2f55f8413280b88ed06a325cb63f1670052c40 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 31 Mar 2022 10:03:09 +0200 Subject: [PATCH 27/51] Revert "Keep information on whether a value definition is type annotated." This reverts commit 1f5df258e7aff282e8313583efec50236f4e243c. --- analysis/src/Hover.ml | 6 ++-- analysis/src/ProcessAttributes.ml | 5 ++-- analysis/src/ProcessCmt.ml | 50 +++++++++++-------------------- analysis/src/References.ml | 12 ++++---- analysis/src/SharedTypes.ml | 6 ++-- 5 files changed, 30 insertions(+), 49 deletions(-) diff --git a/analysis/src/Hover.ml b/analysis/src/Hover.ml index a55591502..a8498c4ab 100644 --- a/analysis/src/Hover.ml +++ b/analysis/src/Hover.ml @@ -41,8 +41,8 @@ let newHover ~full:{file; package} locItem = | TypeDefinition (name, decl, _stamp) -> let typeDef = Shared.declToString name decl in Some (codeBlock typeDef) - | LModule (Definition (stamp, _tip, _)) - | LModule (LocalReference (stamp, _tip)) -> ( + | LModule (Definition (stamp, _tip)) | LModule (LocalReference (stamp, _tip)) + -> ( match Stamps.findModule file.stamps stamp with | None -> None | Some md -> ( @@ -85,7 +85,7 @@ let newHover ~full:{file; package} locItem = | Some file -> showModule ~docstring:file.structure.docstring ~name:file.moduleName ~file None) - | Typed (_, _, Definition (_, (Field _ | Constructor _), _)) -> None + | Typed (_, _, Definition (_, (Field _ | Constructor _))) -> None | Constant t -> Some (codeBlock diff --git a/analysis/src/ProcessAttributes.ml b/analysis/src/ProcessAttributes.ml index 1de622952..0836b7596 100644 --- a/analysis/src/ProcessAttributes.ml +++ b/analysis/src/ProcessAttributes.ml @@ -34,15 +34,14 @@ let rec findDeprecatedAttribute attributes = | ({Asttypes.txt = "deprecated"}, _) :: _ -> Some "" | _ :: rest -> findDeprecatedAttribute rest -let newDeclared ~item ~scope ~extent ~name ~stamp ~modulePath ~isExported - ?(isTypeAnnotated = false) attributes = +let newDeclared ~item ~scope ~extent ~name ~stamp ~modulePath isExported + attributes = { Declared.name; stamp; extentLoc = extent; scopeLoc = scope; isExported; - isTypeAnnotated; modulePath; deprecated = findDeprecatedAttribute attributes; docstring = diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index e07a425ef..24dd7c122 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -22,7 +22,7 @@ let sigItemsExtent items = items |> List.map (fun item -> item.Typedtree.sig_loc) |> locsExtent let addItem ~(name : string Location.loc) ~extent ~stamp ~(env : Env.t) ~item - ?(isTypeAnnotated = false) attributes addExported addStamp = + attributes addExported addStamp = let isExported = addExported name.txt stamp in let declared = ProcessAttributes.newDeclared ~item @@ -32,8 +32,7 @@ let addItem ~(name : string Location.loc) ~extent ~stamp ~(env : Env.t) ~item loc_end = env.scope.loc_end; loc_ghost = false; } - ~extent ~name ~stamp ~modulePath:env.modulePath ~isExported - ~isTypeAnnotated attributes + ~extent ~name ~stamp ~modulePath:env.modulePath isExported attributes in addStamp env.stamps stamp declared; declared @@ -46,8 +45,7 @@ let rec forTypeSignatureItem ~env ~(exported : Exported.t) let declared = addItem ~name:(Location.mknoloc (Ident.name ident)) - ~extent:loc ~stamp:(Ident.binding_time ident) ~env ~item - ~isTypeAnnotated:true val_attributes + ~extent:loc ~stamp:(Ident.binding_time ident) ~env ~item val_attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -109,8 +107,7 @@ let rec forTypeSignatureItem ~env ~(exported : Exported.t) } ~name:(Location.mknoloc name) ~stamp (* TODO maybe this needs another child *) - ~modulePath:env.modulePath ~isExported:true - cd_attributes + ~modulePath:env.modulePath true cd_attributes in Stamps.addConstructor env.stamps stamp declared; item)) @@ -250,8 +247,7 @@ let rec forSignatureItem ~env ~(exported : Exported.t) let declared = addItem ~name ~stamp:(Ident.binding_time val_id) - ~extent:val_loc ~item:val_desc.ctyp_type ~env ~isTypeAnnotated:true - val_attributes + ~extent:val_loc ~item:val_desc.ctyp_type ~env val_attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -345,10 +341,6 @@ let rec getModulePath mod_desc = | Tmod_constraint (expr, _typ, _constraint, _coercion) -> getModulePath expr.mod_desc -let patIsTypeAnnotated pat = - pat.pat_extra - |> List.exists (function Tpat_constraint _, _, _ -> true | _ -> false) - let rec forStructureItem ~env ~(exported : Exported.t) item = match item.str_desc with | Tstr_value (_isRec, bindings) -> @@ -360,8 +352,7 @@ let rec forStructureItem ~env ~(exported : Exported.t) item = let item = pat.pat_type in let declared = addItem ~name ~stamp:(Ident.binding_time ident) ~env - ~extent:pat.pat_loc ~item ~isTypeAnnotated:(patIsTypeAnnotated pat) - attributes + ~extent:pat.pat_loc ~item attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -452,7 +443,7 @@ let rec forStructureItem ~env ~(exported : Exported.t) item = let declared = addItem ~extent:val_loc ~item:val_type ~name ~stamp:(Ident.binding_time val_id) - ~env ~isTypeAnnotated:true val_attributes + ~env val_attributes (Exported.add exported Exported.Value) Stamps.addValue in @@ -504,8 +495,7 @@ and forModule env mod_desc moduleName = loc_end = env.scope.loc_end; loc_ghost = false; } - ~extent:t.Typedtree.mty_loc ~stamp ~modulePath:NotVisible - ~isExported:false [] + ~extent:t.Typedtree.mty_loc ~stamp ~modulePath:NotVisible false [] in Stamps.addModule env.stamps stamp declared)); forModule env resultExpr.mod_desc moduleName @@ -639,14 +629,12 @@ let extraForFile ~(file : File.t) = in file.stamps |> Stamps.iterModules (fun stamp (d : Module.t Declared.t) -> - addLocItem extra d.name.loc - (LModule (Definition (stamp, Module, false))); + addLocItem extra d.name.loc (LModule (Definition (stamp, Module))); addReference stamp d.name.loc); file.stamps |> Stamps.iterValues (fun stamp (d : Types.type_expr Declared.t) -> addLocItem extra d.name.loc - (Typed - (d.name.txt, d.item, Definition (stamp, Value, d.isTypeAnnotated))); + (Typed (d.name.txt, d.item, Definition (stamp, Value))); addReference stamp d.name.loc); file.stamps |> Stamps.iterTypes (fun stamp (d : Type.t Declared.t) -> @@ -660,9 +648,7 @@ let extraForFile ~(file : File.t) = addReference stamp fname.loc; addLocItem extra fname.loc (Typed - ( d.name.txt, - typ, - Definition (d.stamp, Field fname.txt, false) ))) + (d.name.txt, typ, Definition (d.stamp, Field fname.txt)))) | Variant constructors -> constructors |> List.iter (fun {Constructor.stamp; cname} -> @@ -683,7 +669,7 @@ let extraForFile ~(file : File.t) = (Typed ( d.name.txt, t, - Definition (d.stamp, Constructor cname.txt, false) ))) + Definition (d.stamp, Constructor cname.txt) ))) | _ -> ()); extra @@ -1016,13 +1002,12 @@ struct loc_start = val_loc.loc_end; loc_end = (currentScopeExtent ()).loc_end; } - ~modulePath:NotVisible ~item:val_desc.ctyp_type ~isExported:false - ~isTypeAnnotated:true val_attributes + ~modulePath:NotVisible ~item:val_desc.ctyp_type false val_attributes in Stamps.addValue Collector.file.stamps stamp declared; addReference stamp name.loc; addLocItem extra name.loc - (Typed (name.txt, val_desc.ctyp_type, Definition (stamp, Value, true)))) + (Typed (name.txt, val_desc.ctyp_type, Definition (stamp, Value)))) | _ -> () let enter_core_type {ctyp_type; ctyp_desc} = @@ -1031,7 +1016,7 @@ struct addForLongident (Some (ctyp_type, Type)) path txt loc | _ -> () - let enter_pattern ({pat_desc; pat_loc; pat_type; pat_attributes} as pat) = + let enter_pattern {pat_desc; pat_loc; pat_type; pat_attributes} = let addForPattern stamp name = if Stamps.findValue Collector.file.stamps stamp = None then ( let declared = @@ -1042,14 +1027,13 @@ struct loc_start = pat_loc.loc_end; loc_end = (currentScopeExtent ()).loc_end; } - ~modulePath:NotVisible ~extent:pat_loc ~item:pat_type - ~isExported:false ~isTypeAnnotated:(patIsTypeAnnotated pat) + ~modulePath:NotVisible ~extent:pat_loc ~item:pat_type false pat_attributes in Stamps.addValue Collector.file.stamps stamp declared; addReference stamp name.loc; addLocItem extra name.loc - (Typed (name.txt, pat_type, Definition (stamp, Value, false)))) + (Typed (name.txt, pat_type, Definition (stamp, Value)))) in (* Log.log("Entering pattern " ++ Utils.showLocation(pat_loc)); *) match pat_desc with diff --git a/analysis/src/References.ml b/analysis/src/References.ml index c3e11007e..051ca095a 100644 --- a/analysis/src/References.ml +++ b/analysis/src/References.ml @@ -67,7 +67,7 @@ let getLocItem ~full ~line ~col = Some li3 | [ {locType = Typed (_, _, LocalReference (_, Value))}; - ({locType = Typed (_, _, Definition (_, Value, _))} as li2); + ({locType = Typed (_, _, Definition (_, Value))} as li2); ] -> (* JSX on type-annotated labeled (~arg:t): (~arg:t) becomes Props#arg @@ -138,7 +138,7 @@ let definedForLoc ~file ~package locKind = in match locKind with | NotFound -> None - | LocalReference (stamp, tip) | Definition (stamp, tip, _) -> + | LocalReference (stamp, tip) | Definition (stamp, tip) -> inner ~file stamp tip | GlobalReference (moduleName, path, tip) -> ( maybeLog ("Getting global " ^ moduleName); @@ -306,7 +306,7 @@ let definition ~file ~package stamp (tip : Tip.t) = let definitionForLocItem ~full:{file; package} locItem = match locItem.locType with - | Typed (_, _, Definition (stamp, tip, _)) -> ( + | Typed (_, _, Definition (stamp, tip)) -> ( maybeLog ("Typed Definition stamp:" ^ string_of_int stamp ^ " tip:" ^ Tip.toString tip); @@ -323,7 +323,7 @@ let definitionForLocItem ~full:{file; package} locItem = Some (file.uri, loc)) else None) | Typed (_, _, NotFound) - | LModule (NotFound | Definition _) + | LModule (NotFound | Definition (_, _)) | TypeDefinition (_, _, _) | Constant _ -> None @@ -530,8 +530,8 @@ let allReferencesForLocItem ~full:({file; package} as full) locItem = List.append targetModuleReferences otherModulesReferences | Typed (_, _, NotFound) | LModule NotFound | Constant _ -> [] | TypeDefinition (_, _, stamp) -> forLocalStamp ~full stamp Type - | Typed (_, _, (LocalReference (stamp, tip) | Definition (stamp, tip, _))) - | LModule (LocalReference (stamp, tip) | Definition (stamp, tip, _)) -> + | Typed (_, _, (LocalReference (stamp, tip) | Definition (stamp, tip))) + | LModule (LocalReference (stamp, tip) | Definition (stamp, tip)) -> maybeLog ("Finding references for " ^ Uri2.toString file.uri ^ " and stamp " ^ string_of_int stamp ^ " and tip " ^ Tip.toString tip); diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index cf5a77193..5f6b91663 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -132,7 +132,6 @@ module Declared = struct stamp : int; modulePath : modulePath; isExported : bool; - isTypeAnnotated : bool; deprecated : string option; docstring : string list; item : 'item; @@ -298,7 +297,7 @@ type locKind = | LocalReference of int * Tip.t | GlobalReference of string * string list * Tip.t | NotFound - | Definition of int * Tip.t * (* whether it is type annotated *) bool + | Definition of int * Tip.t type locType = | Typed of string * Types.type_expr * locKind @@ -371,8 +370,7 @@ let locKindToString = function | LocalReference (_, tip) -> "(LocalReference " ^ Tip.toString tip ^ ")" | GlobalReference _ -> "GlobalReference" | NotFound -> "NotFound" - | Definition (_, tip, _hasTypeAnnotation) -> - "(Definition " ^ Tip.toString tip ^ ")" + | Definition (_, tip) -> "(Definition " ^ Tip.toString tip ^ ")" let locTypeToString = function | Typed (name, e, locKind) -> From e0fb87fd85f5b49ede8e155b521e62239b5b62f4 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 31 Mar 2022 10:05:57 +0200 Subject: [PATCH 28/51] Use parser to determine if a type annotation is already present. Does not need changes to the type processing. --- analysis/src/Xform.ml | 48 ++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 47b060906..6916683f5 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -111,23 +111,39 @@ end module AddTypeAnnotation = struct (* Add type annotation to value declaration *) - let getAction ~path ~pos ~full = + let mkIterator ~pos ~result = + let pat (iterator : Ast_iterator.iterator) (p : Parsetree.pattern) = + match p.ppat_desc with + | Ppat_var {loc} when posInLoc ~pos ~loc -> result := Some () + | Ppat_constraint _ -> () + | _ -> Ast_iterator.default_iterator.pat iterator p + in + {Ast_iterator.default_iterator with pat} + + let getAction ~path ~pos ~full ~structure = let line, col = pos in - match References.getLocItem ~full ~line ~col with + + let result = ref None in + let iterator = mkIterator ~pos ~result in + iterator.structure iterator structure; + match !result with | None -> None - | Some locItem -> ( - match locItem.locType with - | Typed (_name, typ, Definition (_, Value, false (* isTypeAnnotated *))) -> - let range = - rangeOfLoc {locItem.loc with loc_start = locItem.loc.loc_end} - in - let newText = ": " ^ (typ |> Shared.typeToString) in - let codeAction = - CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite - ~uri:path ~newText ~range - in - Some codeAction - | _ -> None) + | Some _ -> ( + match References.getLocItem ~full ~line ~col with + | None -> None + | Some locItem -> ( + match locItem.locType with + | Typed (_name, typ, _) -> + let range = + rangeOfLoc {locItem.loc with loc_start = locItem.loc.loc_end} + in + let newText = ": " ^ (typ |> Shared.typeToString) in + let codeAction = + CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite + ~uri:path ~newText ~range + in + Some codeAction + | _ -> None)) end let indent n text = @@ -196,7 +212,7 @@ let extractCodeActions ~path ~pos ~currentFile = (match fullOpt with | None -> () | Some full -> ( - match AddTypeAnnotation.getAction ~path ~pos ~full with + match AddTypeAnnotation.getAction ~path ~pos ~full ~structure with | None -> () | Some action -> codeActions := action :: !codeActions)); match IfThenElse.xform ~pos structure with From 1b3ba181ae437dfa2a8769b9d0b431bc26c2e222 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 31 Mar 2022 14:54:04 +0200 Subject: [PATCH 29/51] Only add type annotations to let-bound variables, and function arguments. Detect whether the parameter is a single non-labelled one, in which case add parens as required by the syntax. --- analysis/src/Xform.ml | 50 ++++++++++++++++++----- analysis/tests/src/Xform.res | 1 + analysis/tests/src/expected/Xform.res.txt | 12 +++--- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 6916683f5..408d89d1c 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -111,14 +111,39 @@ end module AddTypeAnnotation = struct (* Add type annotation to value declaration *) + type annotation = Plain | WithParens + let mkIterator ~pos ~result = - let pat (iterator : Ast_iterator.iterator) (p : Parsetree.pattern) = - match p.ppat_desc with - | Ppat_var {loc} when posInLoc ~pos ~loc -> result := Some () - | Ppat_constraint _ -> () - | _ -> Ast_iterator.default_iterator.pat iterator p + let processPattern ?(isUnlabeledOnlyArg = false) (pat : Parsetree.pattern) = + match pat.ppat_desc with + | Ppat_var {loc} when posInLoc ~pos ~loc -> + result := Some (if isUnlabeledOnlyArg then WithParens else Plain) + | _ -> () + in + let rec processFunction ~argNum (e : Parsetree.expression) = + match e.pexp_desc with + | Pexp_fun (argLabel, _, pat, e) -> + let isUnlabeledOnlyArg = + argNum = 1 && argLabel = Nolabel + && match e.pexp_desc with Pexp_fun _ -> false | _ -> true + in + processPattern ~isUnlabeledOnlyArg pat; + processFunction ~argNum:(argNum + 1) e + | _ -> () + in + let structure_item (iterator : Ast_iterator.iterator) + (si : Parsetree.structure_item) = + match si.pstr_desc with + | Pstr_value (_recFlag, bindings) -> + let processBinding (vb : Parsetree.value_binding) = + processPattern vb.pvb_pat; + processFunction vb.pvb_expr + in + bindings |> List.iter (processBinding ~argNum:1); + Ast_iterator.default_iterator.structure_item iterator si + | _ -> Ast_iterator.default_iterator.structure_item iterator si in - {Ast_iterator.default_iterator with pat} + {Ast_iterator.default_iterator with structure_item} let getAction ~path ~pos ~full ~structure = let line, col = pos in @@ -128,16 +153,19 @@ module AddTypeAnnotation = struct iterator.structure iterator structure; match !result with | None -> None - | Some _ -> ( + | Some annotation -> ( match References.getLocItem ~full ~line ~col with | None -> None | Some locItem -> ( match locItem.locType with - | Typed (_name, typ, _) -> - let range = - rangeOfLoc {locItem.loc with loc_start = locItem.loc.loc_end} + | Typed (name, typ, _) -> + let range = rangeOfLoc locItem.loc in + let newText = name ^ ": " ^ (typ |> Shared.typeToString) in + let newText = + match annotation with + | Plain -> newText + | WithParens -> "(" ^ newText ^ ")" in - let newText = ": " ^ (typ |> Shared.typeToString) in let codeAction = CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite ~uri:path ~newText ~range diff --git a/analysis/tests/src/Xform.res b/analysis/tests/src/Xform.res index 97ba4eb16..c27ffd552 100644 --- a/analysis/tests/src/Xform.res +++ b/analysis/tests/src/Xform.res @@ -25,6 +25,7 @@ module T = { } let foo = x => + // ^xfm switch x { | None => 33 | Some(q) => q.T.a + 1 diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 8cebe9fad..d2119ad20 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -20,15 +20,17 @@ switch kind { Xform tests/src/Xform.res 16:5 Hit: Add type annotation -{"start": {"line": 16, "character": 8}, "end": {"line": 16, "character": 8}} +{"start": {"line": 16, "character": 4}, "end": {"line": 16, "character": 8}} newText: -: string +name: string Xform tests/src/Xform.res 19:5 -Xform tests/src/Xform.res 29:9 +Xform tests/src/Xform.res 26:10 Hit: Add type annotation -{"start": {"line": 29, "character": 10}, "end": {"line": 29, "character": 10}} +{"start": {"line": 26, "character": 10}, "end": {"line": 26, "character": 11}} newText: -: T.r +(x: option) + +Xform tests/src/Xform.res 30:9 From 1daa84aad428bfa2e49a2ef9aa31c229b2726fe0 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 31 Mar 2022 14:57:36 +0200 Subject: [PATCH 30/51] Add example of "~foo as bar". --- analysis/tests/src/Xform.res | 3 +++ analysis/tests/src/expected/Xform.res.txt | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/analysis/tests/src/Xform.res b/analysis/tests/src/Xform.res index c27ffd552..852d161c4 100644 --- a/analysis/tests/src/Xform.res +++ b/analysis/tests/src/Xform.res @@ -31,3 +31,6 @@ let foo = x => | Some(q) => q.T.a + 1 // ^xfm } + +let withAs = (~x as name) => name+1 +// ^xfm \ No newline at end of file diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index d2119ad20..c5dbb1c44 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -34,3 +34,9 @@ newText: Xform tests/src/Xform.res 30:9 +Xform tests/src/Xform.res 34:21 +Hit: Add type annotation +{"start": {"line": 34, "character": 21}, "end": {"line": 34, "character": 24}} +newText: +name: int + From b626dbdf8026107692869475f3ba8393574b7ad4 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 31 Mar 2022 15:00:51 +0200 Subject: [PATCH 31/51] code action for applying uncurried function call with dot --- server/src/codeActions.ts | 53 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 00f08136c..f25f88651 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -30,6 +30,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ addUndefinedRecordFields, simpleConversion, topLevelUnitType, + applyUncurried, ]; for (let action of actions) { @@ -294,6 +295,58 @@ let simpleConversion: codeActionExtractor = ({ return false; }; +let applyUncurried: codeActionExtractor = ({ + line, + codeActions, + file, + range, + diagnostic, +}) => { + if ( + line.startsWith( + "This is an uncurried ReScript function. It must be applied with a dot." + ) + ) { + const locOfOpenFnParens = { + line: range.end.line, + character: range.end.character + 1, + }; + + codeActions[file] = codeActions[file] || []; + let codeAction: p.CodeAction = { + title: `Apply uncurried function call with dot`, + edit: { + changes: { + [file]: [ + { + range: { + start: locOfOpenFnParens, + end: locOfOpenFnParens, + }, + /* + * Turns `fn(123)` into `fn(. 123)`. + */ + newText: `. `, + }, + ], + }, + }, + diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + + return false; +}; + let topLevelUnitType: codeActionExtractor = ({ line, codeActions, From e828904a3fd6fb117018ca3e6f84457a1a2376f3 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 31 Mar 2022 15:02:00 +0200 Subject: [PATCH 32/51] Rebase on master. --- analysis/tests/src/expected/Xform.res.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index c5dbb1c44..f8e6bf53b 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -36,7 +36,7 @@ Xform tests/src/Xform.res 30:9 Xform tests/src/Xform.res 34:21 Hit: Add type annotation -{"start": {"line": 34, "character": 21}, "end": {"line": 34, "character": 24}} +{"start": {"line": 34, "character": 20}, "end": {"line": 34, "character": 24}} newText: name: int From fac9736bfaa6da180a5631f144b8bec5d2dcf09a Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 31 Mar 2022 15:22:32 +0200 Subject: [PATCH 33/51] Don't add type annotation to a react component. As the compiler crashes anyway. --- analysis/src/Xform.ml | 9 ++++++++- analysis/tests/src/Xform.res | 8 ++++++-- analysis/tests/src/expected/Xform.res.txt | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 408d89d1c..0d65a7a4d 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -136,7 +136,14 @@ module AddTypeAnnotation = struct match si.pstr_desc with | Pstr_value (_recFlag, bindings) -> let processBinding (vb : Parsetree.value_binding) = - processPattern vb.pvb_pat; + let isReactComponent = + (* Can't add a type annotation to a react component, or the compiler crashes *) + vb.pvb_attributes + |> List.exists (function + | {Location.txt = "react.component"}, _payload -> true + | _ -> false) + in + if not isReactComponent then processPattern vb.pvb_pat; processFunction vb.pvb_expr in bindings |> List.iter (processBinding ~argNum:1); diff --git a/analysis/tests/src/Xform.res b/analysis/tests/src/Xform.res index 852d161c4..fe7b7e4f5 100644 --- a/analysis/tests/src/Xform.res +++ b/analysis/tests/src/Xform.res @@ -32,5 +32,9 @@ let foo = x => // ^xfm } -let withAs = (~x as name) => name+1 -// ^xfm \ No newline at end of file +let withAs = (~x as name) => name + 1 +// ^xfm + +@react.component +let make = (~name) => React.string(name) +// ^xfm diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index f8e6bf53b..591abbbea 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -40,3 +40,5 @@ Hit: Add type annotation newText: name: int +Xform tests/src/Xform.res 38:5 + From a1437bd263c3f8ecb0ac5886d2f40057dfbac532 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 31 Mar 2022 15:33:10 +0200 Subject: [PATCH 34/51] Type annotations: work around location of labels including "~". --- analysis/src/Xform.ml | 12 +++++++----- analysis/tests/src/Xform.res | 3 +++ analysis/tests/src/expected/Xform.res.txt | 14 ++++++++++---- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 0d65a7a4d..3cd48b3b7 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -166,12 +166,14 @@ module AddTypeAnnotation = struct | Some locItem -> ( match locItem.locType with | Typed (name, typ, _) -> - let range = rangeOfLoc locItem.loc in - let newText = name ^ ": " ^ (typ |> Shared.typeToString) in - let newText = + let range, newText = match annotation with - | Plain -> newText - | WithParens -> "(" ^ newText ^ ")" + | Plain -> + ( rangeOfLoc {locItem.loc with loc_start = locItem.loc.loc_end}, + ": " ^ (typ |> Shared.typeToString) ) + | WithParens -> + ( rangeOfLoc locItem.loc, + "(" ^ name ^ ": " ^ (typ |> Shared.typeToString) ^ ")" ) in let codeAction = CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite diff --git a/analysis/tests/src/Xform.res b/analysis/tests/src/Xform.res index fe7b7e4f5..7b154a586 100644 --- a/analysis/tests/src/Xform.res +++ b/analysis/tests/src/Xform.res @@ -38,3 +38,6 @@ let withAs = (~x as name) => name + 1 @react.component let make = (~name) => React.string(name) // ^xfm + +let _ = (~x) => x+1 +// ^xfm \ No newline at end of file diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 591abbbea..97a291ed1 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -20,9 +20,9 @@ switch kind { Xform tests/src/Xform.res 16:5 Hit: Add type annotation -{"start": {"line": 16, "character": 4}, "end": {"line": 16, "character": 8}} +{"start": {"line": 16, "character": 8}, "end": {"line": 16, "character": 8}} newText: -name: string +: string Xform tests/src/Xform.res 19:5 @@ -36,9 +36,15 @@ Xform tests/src/Xform.res 30:9 Xform tests/src/Xform.res 34:21 Hit: Add type annotation -{"start": {"line": 34, "character": 20}, "end": {"line": 34, "character": 24}} +{"start": {"line": 34, "character": 24}, "end": {"line": 34, "character": 24}} newText: -name: int +: int Xform tests/src/Xform.res 38:5 +Xform tests/src/Xform.res 41:9 +Hit: Add type annotation +{"start": {"line": 41, "character": 11}, "end": {"line": 41, "character": 11}} +newText: +: int + From fa0cb2c134657d18acbf0e8e7fea542031a9f0ee Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 4 Apr 2022 18:05:21 +0200 Subject: [PATCH 35/51] add code action for adding braces to function without braces --- analysis/src/Xform.ml | 53 ++++++++++++++++++++++- analysis/tests/src/Xform.res | 13 +++++- analysis/tests/src/expected/Xform.res.txt | 42 ++++++++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 3cd48b3b7..9a24cf543 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -1,5 +1,7 @@ (** Code transformations using the parser/printer and ast operations *) +let isBracedExpr = Res_parsetree_viewer.isBracedExpr + let posInLoc ~pos ~loc = Utils.tupleOfLexing loc.Location.loc_start <= pos && pos < Utils.tupleOfLexing loc.loc_end @@ -108,6 +110,44 @@ module IfThenElse = struct !changed end +module AddBracesToFn = struct + (* Add braces to fn without braces *) + + let mkIterator ~pos ~changed = + let expr (iterator : Ast_iterator.iterator) (e : Parsetree.expression) = + (match e.pexp_desc with + | Pexp_fun (argLabel, expr, pat, funExpr) + when posInLoc ~pos ~loc:e.pexp_loc && isBracedExpr funExpr = false -> + changed := + Some + { + e with + pexp_desc = + Pexp_fun + ( argLabel, + expr, + pat, + { + funExpr with + pexp_attributes = + ( Location.mkloc "ns.braces" e.pexp_loc, + Parsetree.PStr [] ) + :: funExpr.pexp_attributes; + } ); + } + | _ -> ()); + Ast_iterator.default_iterator.expr iterator e + in + + {Ast_iterator.default_iterator with expr} + + let xform ~pos structure = + let changed = ref None in + let iterator = mkIterator ~pos ~changed in + iterator.structure iterator structure; + !changed +end + module AddTypeAnnotation = struct (* Add type annotation to value declaration *) @@ -252,7 +292,7 @@ let extractCodeActions ~path ~pos ~currentFile = match AddTypeAnnotation.getAction ~path ~pos ~full ~structure with | None -> () | Some action -> codeActions := action :: !codeActions)); - match IfThenElse.xform ~pos structure with + (match IfThenElse.xform ~pos structure with | None -> () | Some newExpr -> let range = rangeOfLoc newExpr.pexp_loc in @@ -262,4 +302,15 @@ let extractCodeActions ~path ~pos ~currentFile = ~uri:path ~newText ~range in codeActions := codeAction :: !codeActions); + + match AddBracesToFn.xform ~pos structure with + | None -> () + | Some newExpr -> + let range = rangeOfLoc newExpr.pexp_loc in + let newText = printExpr ~range newExpr in + let codeAction = + CodeActions.make ~title:"Add braces to function" ~kind:RefactorRewrite + ~uri:path ~newText ~range + in + codeActions := codeAction :: !codeActions); !codeActions diff --git a/analysis/tests/src/Xform.res b/analysis/tests/src/Xform.res index 7b154a586..719e7728c 100644 --- a/analysis/tests/src/Xform.res +++ b/analysis/tests/src/Xform.res @@ -39,5 +39,14 @@ let withAs = (~x as name) => name + 1 let make = (~name) => React.string(name) // ^xfm -let _ = (~x) => x+1 -// ^xfm \ No newline at end of file +let _ = (~x) => x + 1 +// ^xfm + +let noBraces = () => name +// ^xfm + +let nested = () => { + let noBraces = () => "someNewFunc" + // ^xfm + ignore(noBraces()) +} diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 97a291ed1..5f6e408d8 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -27,14 +27,40 @@ newText: Xform tests/src/Xform.res 19:5 Xform tests/src/Xform.res 26:10 +Hit: Add braces to function +{"start": {"line": 26, "character": 10}, "end": {"line": 32, "character": 3}} +newText: +x => { + // ^xfm + switch x { + | None => 33 + | Some(q) => q.T.a + 1 + // ^xfm + } + } Hit: Add type annotation {"start": {"line": 26, "character": 10}, "end": {"line": 26, "character": 11}} newText: (x: option) Xform tests/src/Xform.res 30:9 +Hit: Add braces to function +{"start": {"line": 26, "character": 10}, "end": {"line": 32, "character": 3}} +newText: +x => { + // ^xfm + switch x { + | None => 33 + | Some(q) => q.T.a + 1 + // ^xfm + } + } Xform tests/src/Xform.res 34:21 +Hit: Add braces to function +{"start": {"line": 34, "character": 13}, "end": {"line": 34, "character": 37}} +newText: +(~x as name) => {name + 1} Hit: Add type annotation {"start": {"line": 34, "character": 24}, "end": {"line": 34, "character": 24}} newText: @@ -43,8 +69,24 @@ newText: Xform tests/src/Xform.res 38:5 Xform tests/src/Xform.res 41:9 +Hit: Add braces to function +{"start": {"line": 41, "character": 8}, "end": {"line": 41, "character": 21}} +newText: +(~x) => {x + 1} Hit: Add type annotation {"start": {"line": 41, "character": 11}, "end": {"line": 41, "character": 11}} newText: : int +Xform tests/src/Xform.res 44:17 +Hit: Add braces to function +{"start": {"line": 44, "character": 15}, "end": {"line": 44, "character": 25}} +newText: +() => {name} + +Xform tests/src/Xform.res 48:22 +Hit: Add braces to function +{"start": {"line": 48, "character": 17}, "end": {"line": 48, "character": 36}} +newText: +() => {"someNewFunc"} + From b5f0393924d040fbbb5f4592da4e9459014e6478 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 5 Apr 2022 09:16:28 +0200 Subject: [PATCH 36/51] Make debug print for actions less confusing. --- analysis/src/Commands.ml | 7 +++-- analysis/tests/src/expected/Xform.res.txt | 32 ++++++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml index 192a41435..4ca9ef8b7 100644 --- a/analysis/src/Commands.ml +++ b/analysis/src/Commands.ml @@ -348,9 +348,12 @@ let test ~path = |> List.iter (fun {Protocol.edits} -> edits |> List.iter (fun {Protocol.range; newText} -> - Printf.printf "%s\nnewText:\n%s\n" + let indent = + String.make range.start.character ' ' + in + Printf.printf "%s\nnewText:\n%s<--here\n%s%s\n" (Protocol.stringifyRange range) - newText))) + indent indent newText))) | _ -> ()); print_newline ()) in diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index 5f6e408d8..cca1c0c63 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -2,6 +2,7 @@ Xform tests/src/Xform.res 6:5 Hit: Replace with switch {"start": {"line": 6, "character": 0}, "end": {"line": 11, "character": 1}} newText: +<--here switch kind { | First => // ^xfm @@ -13,6 +14,7 @@ Xform tests/src/Xform.res 13:15 Hit: Replace with switch {"start": {"line": 13, "character": 0}, "end": {"line": 13, "character": 79}} newText: +<--here switch kind { | #kind("First", {name: "abc", age: 3}) => ret("First") | _ => ret("Not First") @@ -22,7 +24,8 @@ Xform tests/src/Xform.res 16:5 Hit: Add type annotation {"start": {"line": 16, "character": 8}, "end": {"line": 16, "character": 8}} newText: -: string + <--here + : string Xform tests/src/Xform.res 19:5 @@ -30,7 +33,8 @@ Xform tests/src/Xform.res 26:10 Hit: Add braces to function {"start": {"line": 26, "character": 10}, "end": {"line": 32, "character": 3}} newText: -x => { + <--here + x => { // ^xfm switch x { | None => 33 @@ -41,13 +45,15 @@ x => { Hit: Add type annotation {"start": {"line": 26, "character": 10}, "end": {"line": 26, "character": 11}} newText: -(x: option) + <--here + (x: option) Xform tests/src/Xform.res 30:9 Hit: Add braces to function {"start": {"line": 26, "character": 10}, "end": {"line": 32, "character": 3}} newText: -x => { + <--here + x => { // ^xfm switch x { | None => 33 @@ -60,11 +66,13 @@ Xform tests/src/Xform.res 34:21 Hit: Add braces to function {"start": {"line": 34, "character": 13}, "end": {"line": 34, "character": 37}} newText: -(~x as name) => {name + 1} + <--here + (~x as name) => {name + 1} Hit: Add type annotation {"start": {"line": 34, "character": 24}, "end": {"line": 34, "character": 24}} newText: -: int + <--here + : int Xform tests/src/Xform.res 38:5 @@ -72,21 +80,25 @@ Xform tests/src/Xform.res 41:9 Hit: Add braces to function {"start": {"line": 41, "character": 8}, "end": {"line": 41, "character": 21}} newText: -(~x) => {x + 1} + <--here + (~x) => {x + 1} Hit: Add type annotation {"start": {"line": 41, "character": 11}, "end": {"line": 41, "character": 11}} newText: -: int + <--here + : int Xform tests/src/Xform.res 44:17 Hit: Add braces to function {"start": {"line": 44, "character": 15}, "end": {"line": 44, "character": 25}} newText: -() => {name} + <--here + () => {name} Xform tests/src/Xform.res 48:22 Hit: Add braces to function {"start": {"line": 48, "character": 17}, "end": {"line": 48, "character": 36}} newText: -() => {"someNewFunc"} + <--here + () => {"someNewFunc"} From 063c7d05d1682042e86d2957044273f5b5c1c823 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Wed, 6 Apr 2022 10:35:53 +0200 Subject: [PATCH 37/51] Tweak action for adding braces to function body. - Only fire on the body, not on the argument. - On `(x,y,z) => e` check that we're on `z => e` and not `x` or `y` by checking that `e` is not a function. - Use location trick on braces so expression is always on a new line. - Start pretty printing from latest structure item (typically the let defining the function), so the local pretty printing looks decent. --- analysis/src/Xform.ml | 95 ++++++++++++------- .../vendor/compiler-libs-406/parsetree.mli | 1 + analysis/tests/src/Xform.res | 23 ++++- analysis/tests/src/expected/Xform.res.txt | 77 ++++++++------- 4 files changed, 119 insertions(+), 77 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 9a24cf543..ab311bc5d 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -114,32 +114,49 @@ module AddBracesToFn = struct (* Add braces to fn without braces *) let mkIterator ~pos ~changed = + (* While iterating the AST, keep info on which structure item we are in. + Printing from the structure item, rather than the body of the function, + gives better local pretty printing *) + let currentStructureItem = ref None in + + let structure_item (iterator : Ast_iterator.iterator) + (item : Parsetree.structure_item) = + let saved = !currentStructureItem in + currentStructureItem := Some item; + Ast_iterator.default_iterator.structure_item iterator item; + currentStructureItem := saved + in let expr (iterator : Ast_iterator.iterator) (e : Parsetree.expression) = + let bracesAttribute = + let loc = + { + Location.none with + loc_start = Lexing.dummy_pos; + loc_end = + { + Lexing.dummy_pos with + pos_lnum = Lexing.dummy_pos.pos_lnum + 1 (* force line break *); + }; + } + in + (Location.mkloc "ns.braces" loc, Parsetree.PStr []) + in + let isFunction = function + | {Parsetree.pexp_desc = Pexp_fun _} -> true + | _ -> false + in (match e.pexp_desc with - | Pexp_fun (argLabel, expr, pat, funExpr) - when posInLoc ~pos ~loc:e.pexp_loc && isBracedExpr funExpr = false -> - changed := - Some - { - e with - pexp_desc = - Pexp_fun - ( argLabel, - expr, - pat, - { - funExpr with - pexp_attributes = - ( Location.mkloc "ns.braces" e.pexp_loc, - Parsetree.PStr [] ) - :: funExpr.pexp_attributes; - } ); - } + | Pexp_fun (_, _, _, bodyExpr) + when posInLoc ~pos ~loc:bodyExpr.pexp_loc + && isBracedExpr bodyExpr = false + && isFunction bodyExpr = false -> + bodyExpr.pexp_attributes <- bracesAttribute :: bodyExpr.pexp_attributes; + changed := !currentStructureItem | _ -> ()); Ast_iterator.default_iterator.expr iterator e in - {Ast_iterator.default_iterator with expr} + {Ast_iterator.default_iterator with expr; structure_item} let xform ~pos structure = let changed = ref None in @@ -242,21 +259,31 @@ let parse ~filename = let {Res_driver.parsetree = structure; comments} = Res_driver.parsingEngine.parseImplementation ~forPrinter:false ~filename in + let filterComments ~loc comments = + (* Relevant comments in the range of the expression *) + let filter comment = + posInLoc + ~pos:((Res_comment.loc comment).loc_start |> Utils.tupleOfLexing) + ~loc + in + comments |> List.filter filter + in let printExpr ~(range : Protocol.range) (expr : Parsetree.expression) = let structure = [Ast_helper.Str.eval ~loc:expr.pexp_loc expr] in - let filterComments = function - (* Relevant comments in the range of the expression *) - | comment -> - posInLoc - ~pos:((Res_comment.loc comment).loc_start |> Utils.tupleOfLexing) - ~loc:expr.pexp_loc - in structure |> Res_printer.printImplementation ~width:!Res_cli.ResClflags.width - ~comments:(List.filter filterComments comments) + ~comments:(comments |> filterComments ~loc:expr.pexp_loc) + |> indent range.start.character + in + let printStructureItem ~(range : Protocol.range) + (item : Parsetree.structure_item) = + let structure = [item] in + structure + |> Res_printer.printImplementation ~width:!Res_cli.ResClflags.width + ~comments:(comments |> filterComments ~loc:item.pstr_loc) |> indent range.start.character in - (structure, printExpr) + (structure, printExpr, printStructureItem) let diff ~filename ~newContents = match Files.readFile ~filename with @@ -284,7 +311,9 @@ let diff ~filename ~newContents = let extractCodeActions ~path ~pos ~currentFile = let codeActions = ref [] in if Filename.check_suffix currentFile ".res" then ( - let structure, printExpr = parse ~filename:currentFile in + let structure, printExpr, printStructureItem = + parse ~filename:currentFile + in let fullOpt = Cmt.fromPath ~path in (match fullOpt with | None -> () @@ -305,9 +334,9 @@ let extractCodeActions ~path ~pos ~currentFile = match AddBracesToFn.xform ~pos structure with | None -> () - | Some newExpr -> - let range = rangeOfLoc newExpr.pexp_loc in - let newText = printExpr ~range newExpr in + | Some newStructureItem -> + let range = rangeOfLoc newStructureItem.pstr_loc in + let newText = printStructureItem ~range newStructureItem in let codeAction = CodeActions.make ~title:"Add braces to function" ~kind:RefactorRewrite ~uri:path ~newText ~range diff --git a/analysis/src/vendor/compiler-libs-406/parsetree.mli b/analysis/src/vendor/compiler-libs-406/parsetree.mli index b63e31408..40edf2a6e 100644 --- a/analysis/src/vendor/compiler-libs-406/parsetree.mli +++ b/analysis/src/vendor/compiler-libs-406/parsetree.mli @@ -233,6 +233,7 @@ and expression = { pexp_desc: expression_desc; pexp_loc: Location.t; + mutable (* Careful, the original parse tree is not mutable *) pexp_attributes: attributes; (* ... [@id1] [@id2] *) } diff --git a/analysis/tests/src/Xform.res b/analysis/tests/src/Xform.res index 719e7728c..52baad9d8 100644 --- a/analysis/tests/src/Xform.res +++ b/analysis/tests/src/Xform.res @@ -42,11 +42,26 @@ let make = (~name) => React.string(name) let _ = (~x) => x + 1 // ^xfm +// +// Add braces to the body of a function +// + let noBraces = () => name -// ^xfm +// ^xfm let nested = () => { - let noBraces = () => "someNewFunc" - // ^xfm - ignore(noBraces()) + let _noBraces = (_x, _y, _z) => "someNewFunc" + // ^xfm +} + +let bar = () => { + module Inner = { + let foo = (_x, y, _z) => + switch y { + | #some => 3 + | #stuff => 4 + } + //^xfm + } + Inner.foo(1) } diff --git a/analysis/tests/src/expected/Xform.res.txt b/analysis/tests/src/expected/Xform.res.txt index cca1c0c63..4583cee1c 100644 --- a/analysis/tests/src/expected/Xform.res.txt +++ b/analysis/tests/src/expected/Xform.res.txt @@ -30,18 +30,6 @@ newText: Xform tests/src/Xform.res 19:5 Xform tests/src/Xform.res 26:10 -Hit: Add braces to function -{"start": {"line": 26, "character": 10}, "end": {"line": 32, "character": 3}} -newText: - <--here - x => { - // ^xfm - switch x { - | None => 33 - | Some(q) => q.T.a + 1 - // ^xfm - } - } Hit: Add type annotation {"start": {"line": 26, "character": 10}, "end": {"line": 26, "character": 11}} newText: @@ -50,24 +38,19 @@ newText: Xform tests/src/Xform.res 30:9 Hit: Add braces to function -{"start": {"line": 26, "character": 10}, "end": {"line": 32, "character": 3}} +{"start": {"line": 26, "character": 0}, "end": {"line": 32, "character": 3}} newText: - <--here - x => { - // ^xfm - switch x { - | None => 33 - | Some(q) => q.T.a + 1 - // ^xfm - } - } +<--here +let foo = x => { + // ^xfm + switch x { + | None => 33 + | Some(q) => q.T.a + 1 + // ^xfm + } +} Xform tests/src/Xform.res 34:21 -Hit: Add braces to function -{"start": {"line": 34, "character": 13}, "end": {"line": 34, "character": 37}} -newText: - <--here - (~x as name) => {name + 1} Hit: Add type annotation {"start": {"line": 34, "character": 24}, "end": {"line": 34, "character": 24}} newText: @@ -77,28 +60,42 @@ newText: Xform tests/src/Xform.res 38:5 Xform tests/src/Xform.res 41:9 -Hit: Add braces to function -{"start": {"line": 41, "character": 8}, "end": {"line": 41, "character": 21}} -newText: - <--here - (~x) => {x + 1} Hit: Add type annotation {"start": {"line": 41, "character": 11}, "end": {"line": 41, "character": 11}} newText: <--here : int -Xform tests/src/Xform.res 44:17 +Xform tests/src/Xform.res 48:21 Hit: Add braces to function -{"start": {"line": 44, "character": 15}, "end": {"line": 44, "character": 25}} +{"start": {"line": 48, "character": 0}, "end": {"line": 48, "character": 25}} newText: - <--here - () => {name} +<--here +let noBraces = () => { + name +} + +Xform tests/src/Xform.res 52:34 +Hit: Add braces to function +{"start": {"line": 51, "character": 0}, "end": {"line": 54, "character": 1}} +newText: +<--here +let nested = () => { + let _noBraces = (_x, _y, _z) => { + "someNewFunc" + } + // ^xfm +} -Xform tests/src/Xform.res 48:22 +Xform tests/src/Xform.res 62:6 Hit: Add braces to function -{"start": {"line": 48, "character": 17}, "end": {"line": 48, "character": 36}} +{"start": {"line": 58, "character": 4}, "end": {"line": 62, "character": 7}} newText: - <--here - () => {"someNewFunc"} + <--here + let foo = (_x, y, _z) => { + switch y { + | #some => 3 + | #stuff => 4 + } + } From 47c9f50d813d4d1ac58879f5986ea99ad6912672 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 6 Apr 2022 11:25:27 +0200 Subject: [PATCH 38/51] fix bug with wrapping single character ranges in quick fix code actions --- server/src/codeActions.ts | 121 ++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 64 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index f25f88651..9a843b63b 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -15,6 +15,53 @@ interface findCodeActionsConfig { addFoundActionsHere: filesCodeActions; } +let wrapRangeInText = ( + range: p.Range, + wrapStart: string, + wrapEnd: string +): p.TextEdit[] => { + // We need to adjust the start of where we replace if this is a single + // character on a single line. + let offset = + range.start.line === range.end.line && + range.start.character === range.end.character + ? 1 + : 0; + + let startRange = { + start: { + line: range.start.line, + character: range.start.character - offset, + }, + end: { + line: range.start.line, + character: range.start.character - offset, + }, + }; + + let endRange = { + start: { + line: range.end.line, + character: range.end.character, + }, + end: { + line: range.end.line, + character: range.end.character, + }, + }; + + return [ + { + range: startRange, + newText: wrapStart, + }, + { + range: endRange, + newText: wrapEnd, + }, + ]; +}; + export let findCodeActionsInDiagnosticsMessage = ({ diagnostic, diagnosticMessage, @@ -187,6 +234,11 @@ let addUndefinedRecordFields: codeActionExtractor = ({ .join(", "); } + let beforeEndingRecordBraceLoc = { + line: range.end.line, + character: range.end.character - 1, + }; + let codeAction: p.CodeAction = { title: `Add missing record fields`, edit: { @@ -194,14 +246,8 @@ let addUndefinedRecordFields: codeActionExtractor = ({ [file]: [ { range: { - start: { - line: range.end.line, - character: range.end.character - 1, - }, - end: { - line: range.end.line, - character: range.end.character - 1, - }, + start: beforeEndingRecordBraceLoc, + end: beforeEndingRecordBraceLoc, }, newText, }, @@ -244,38 +290,12 @@ let simpleConversion: codeActionExtractor = ({ if (from != null && to != null && fn != null) { codeActions[file] = codeActions[file] || []; + let codeAction: p.CodeAction = { title: `Convert ${from} to ${to} with ${fn}`, edit: { changes: { - [file]: [ - { - range: { - start: { - line: range.start.line, - character: range.start.character, - }, - end: { - line: range.start.line, - character: range.start.character, - }, - }, - newText: `${fn}(`, - }, - { - range: { - start: { - line: range.end.line, - character: range.end.character, - }, - end: { - line: range.end.line, - character: range.end.character, - }, - }, - newText: `)`, - }, - ], + [file]: wrapRangeInText(range, `${fn}(`, `)`), }, }, diagnostics: [diagnostic], @@ -360,34 +380,7 @@ let topLevelUnitType: codeActionExtractor = ({ title: `Wrap expression in ignore`, edit: { changes: { - [file]: [ - { - range: { - start: { - line: range.start.line, - character: range.start.character, - }, - end: { - line: range.start.line, - character: range.start.character, - }, - }, - newText: `ignore(`, - }, - { - range: { - start: { - line: range.end.line, - character: range.end.character, - }, - end: { - line: range.end.line, - character: range.end.character, - }, - }, - newText: `)`, - }, - ], + [file]: wrapRangeInText(range, "ignore(", ")"), }, }, diagnostics: [diagnostic], From 382bb95b0345fb2f4137985ba4f23d9f6f77dd50 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 6 Apr 2022 13:33:55 +0200 Subject: [PATCH 39/51] code action for inserting simple missing variant cases --- server/src/codeActions.ts | 175 +++++++++++++++++++++++++++++++++++--- 1 file changed, 161 insertions(+), 14 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 9a843b63b..7d3b93754 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -62,6 +62,26 @@ let wrapRangeInText = ( ]; }; +let insertBeforeEndingChar = ( + range: p.Range, + newText: string +): p.TextEdit[] => { + let beforeEndingChar = { + line: range.end.line, + character: range.end.character - 1, + }; + + return [ + { + range: { + start: beforeEndingChar, + end: beforeEndingChar, + }, + newText, + }, + ]; +}; + export let findCodeActionsInDiagnosticsMessage = ({ diagnostic, diagnosticMessage, @@ -78,6 +98,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ simpleConversion, topLevelUnitType, applyUncurried, + simpleAddMissingCases, ]; for (let action of actions) { @@ -234,24 +255,11 @@ let addUndefinedRecordFields: codeActionExtractor = ({ .join(", "); } - let beforeEndingRecordBraceLoc = { - line: range.end.line, - character: range.end.character - 1, - }; - let codeAction: p.CodeAction = { title: `Add missing record fields`, edit: { changes: { - [file]: [ - { - range: { - start: beforeEndingRecordBraceLoc, - end: beforeEndingRecordBraceLoc, - }, - newText, - }, - ], + [file]: insertBeforeEndingChar(range, newText), }, }, diagnostics: [diagnostic], @@ -398,3 +406,142 @@ let topLevelUnitType: codeActionExtractor = ({ return false; }; + +// This protects against the fact that the compiler currently returns most +// text in OCaml. It also ensures that we only return simple constructors. +let isValidVariantCase = (text: string): boolean => { + if (text.startsWith("(") || text.includes(",")) { + return false; + } + + return true; +}; + +// Untransformed is typically OCaml, and looks like these examples: +// +// `SomeVariantName +// +// SomeVariantWithPayload _ +// +// ...and we'll need to transform this into proper ReScript. In the future, the +// compiler itself should of course output real ReScript. But it currently does +// not. +let transformVariant = (variant: string): string | null => { + // Convert old polyvariant notation to new + let text = variant.replace(/`/g, "#"); + + // Fix payloads + if (text.includes(" ")) { + let [variantText, payloadText] = text.split(" "); + + // If the payload itself starts with (, it's another variant with a + // constructor. We bail in that case, for now at least. We'll be able to + // revisit this in the future when the compiler prints real ReScript syntax. + if (payloadText.startsWith("(")) { + return null; + } + + text = `${variantText}(${payloadText})`; + } + + return text; +}; + +let simpleAddMissingCases: codeActionExtractor = ({ + line, + codeActions, + file, + range, + diagnostic, + array, + index, +}) => { + // Examples: + // + // You forgot to handle a possible case here, for example: + // (AnotherValue|Third|Fourth) + // + // You forgot to handle a possible case here, for example: + // (`AnotherValue|`Third|`Fourth) + // + // You forgot to handle a possible case here, for example: + // `AnotherValue + // + // You forgot to handle a possible case here, for example: + // AnotherValue + + if ( + line.startsWith("You forgot to handle a possible case here, for example:") + ) { + let cases: string[] = []; + + // This collects the rest of the fields if fields are printed on + // multiple lines. + array.slice(index + 1).forEach((line) => { + let theLine = line.trim(); + + let hasMultipleCases = theLine.includes("|"); + + if (hasMultipleCases) { + cases.push( + ...(theLine + // Remove leading and ending parens + .slice(1, theLine.length - 1) + .split("|") + .filter(isValidVariantCase) + .map(transformVariant) + .filter(Boolean) as string[]) + ); + } else { + let transformed = transformVariant(theLine); + if (isValidVariantCase(theLine) && transformed != null) { + cases.push(transformed); + } + } + }); + + if (cases.length === 0) { + return false; + } + + // The end char is the closing brace. In switches, the leading `|` always + // has the same left padding as the end brace. + let paddingContentSwitchCase = Array.from({ + length: range.end.character, + }).join(" "); + + let newText = cases + .map((variantName, index) => { + // The first case will automatically be padded because we're inserting + // it where the end brace is currently located. + let padding = index === 0 ? "" : paddingContentSwitchCase; + return `${padding}| ${variantName} => assert false`; + }) + .join("\n"); + + // Let's put the end brace back where it was (we still have it to the direct right of us). + newText += `\n${paddingContentSwitchCase}`; + + codeActions[file] = codeActions[file] || []; + let codeAction: p.CodeAction = { + title: `Insert missing cases`, + edit: { + changes: { + [file]: insertBeforeEndingChar(range, newText), + }, + }, + diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + + return false; +}; From cd73ff98a37ec1fc16d6e3e69cab3845794410d4 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 6 Apr 2022 13:46:30 +0200 Subject: [PATCH 40/51] code action for wrapping a value of type t with Some where something expects option --- server/src/codeActions.ts | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 7d3b93754..cfbab30b1 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -99,6 +99,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ topLevelUnitType, applyUncurried, simpleAddMissingCases, + simpleWrapOptionalWithSome, ]; for (let action of actions) { @@ -545,3 +546,64 @@ let simpleAddMissingCases: codeActionExtractor = ({ return false; }; + +let simpleWrapOptionalWithSome: codeActionExtractor = ({ + line, + codeActions, + file, + range, + diagnostic, + array, + index, +}) => { + // Examples: + // + // 46 │ let as_ = { + // 47 │ someProp: "123", + // 48 │ another: "123", + // 49 │ } + // 50 │ + // This has type: string + // Somewhere wanted: option + + if (line.startsWith("Somewhere wanted: option<")) { + let somewhereWantedLine = line; + let thisHasTypeLine = array[index - 1]; + let hasTypeText = thisHasTypeLine.split("This has type: ")[1].trim(); + let somewhereWantedText = somewhereWantedLine + .split("Somewhere wanted: option<")[1] + .trim(); + + // Remove ending `>` so we can compare the underlying types + somewhereWantedText = somewhereWantedText.slice( + 0, + somewhereWantedText.length - 1 + ); + + // We only trigger the code action if the thing that's already there is the + // exact same type. + if (hasTypeText === somewhereWantedText) { + codeActions[file] = codeActions[file] || []; + let codeAction: p.CodeAction = { + title: `Wrap value in Some`, + edit: { + changes: { + [file]: wrapRangeInText(range, "Some(", ")"), + }, + }, + diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + } + + return false; +}; From 983bdd7532310890f08d21b7527338dca9088cab Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 6 Apr 2022 18:16:26 +0200 Subject: [PATCH 41/51] add code action for unwrapping optionals --- server/src/codeActions.ts | 87 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index cfbab30b1..6f532e917 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -100,6 +100,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ applyUncurried, simpleAddMissingCases, simpleWrapOptionalWithSome, + simpleUnwrapOptional, ]; for (let action of actions) { @@ -607,3 +608,89 @@ let simpleWrapOptionalWithSome: codeActionExtractor = ({ return false; }; + +let simpleUnwrapOptional: codeActionExtractor = ({ + line, + codeActions, + file, + range, + diagnostic, + array, + index, +}) => { + // Examples: + // + // 47 │ + // 48 │ let as_ = { + // 49 │ someProp: optional, + // 50 │ another: Some("123"), + // 51 │ } + + // This has type: option + // Somewhere wanted: string + + if (line.startsWith("This has type: option<")) { + let thisHasTypeLine = line; + let hasTypeText = thisHasTypeLine.split("This has type: option<")[1].trim(); + // Remove ending `>` so we can compare the underlying types + hasTypeText = hasTypeText.slice(0, hasTypeText.length - 1); + + let somewhereWantedLine = array[index + 1]; + let somewhereWantedText = somewhereWantedLine + .split("Somewhere wanted: ")[1] + .trim(); + + // We only trigger the code action if the thing that's already there is the + // exact same type. + if (hasTypeText === somewhereWantedText) { + codeActions[file] = codeActions[file] || []; + + // We can figure out default values for primitives etc. + let defaultValue = "assert false"; + + switch (somewhereWantedText) { + case "string": { + defaultValue = `"-"`; + break; + } + case "bool": { + defaultValue = `false`; + break; + } + case "int": { + defaultValue = `-1`; + break; + } + case "float": { + defaultValue = `-1.`; + break; + } + } + + let codeAction: p.CodeAction = { + title: `Unwrap optional value`, + edit: { + changes: { + [file]: wrapRangeInText( + range, + "switch ", + ` { | None => ${defaultValue} | Some(v) => v }` + ), + }, + }, + diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } + } + + return false; +}; From d9a761bc383d076d176faab2ea13d2cdbc9f7989 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 6 Apr 2022 18:30:50 +0200 Subject: [PATCH 42/51] explanations for the current diagnostics driven code actions --- server/src/codeActions.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 6f532e917..252653c3f 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -133,6 +133,9 @@ interface codeActionExtractorConfig { type codeActionExtractor = (config: codeActionExtractorConfig) => boolean; +// This action extracts hints the compiler emits for misspelled identifiers, and +// offers to replace the misspelled name with the correct name suggested by the +// compiler. let didYouMeanAction: codeActionExtractor = ({ codeActions, diagnostic, @@ -176,6 +179,10 @@ let didYouMeanAction: codeActionExtractor = ({ return false; }; +// This action handles when the compiler errors on certain fields of a record +// being undefined. We then offers an action that inserts all of the record +// fields, with an `assert false` dummy value. `assert false` is so applying the +// code action actually compiles. let addUndefinedRecordFields: codeActionExtractor = ({ array, codeActions, @@ -281,6 +288,8 @@ let addUndefinedRecordFields: codeActionExtractor = ({ return false; }; +// This action detects suggestions of converting between mismatches in types +// that the compiler tells us about. let simpleConversion: codeActionExtractor = ({ line, codeActions, @@ -325,6 +334,8 @@ let simpleConversion: codeActionExtractor = ({ return false; }; +// This action will apply a curried function (essentially inserting a dot in the +// correct place). let applyUncurried: codeActionExtractor = ({ line, codeActions, @@ -377,6 +388,9 @@ let applyUncurried: codeActionExtractor = ({ return false; }; +// This action detects the compiler erroring when it finds a top level value +// that's not `unit`, and offers to wrap that value in `ignore`, which will make +// it compile. let topLevelUnitType: codeActionExtractor = ({ line, codeActions, @@ -449,6 +463,12 @@ let transformVariant = (variant: string): string | null => { return text; }; +// This action detects missing cases for exhaustive pattern matches, and offers +// to insert dummy branches (using `assert false`) for those branches. Right now +// it works on single variants/polyvariants with and without payloads. In the +// future it could be made to work on anything the compiler tell us about, but +// the compiler needs to emit proper ReScript in the error messages for that to +// work. let simpleAddMissingCases: codeActionExtractor = ({ line, codeActions, @@ -548,6 +568,9 @@ let simpleAddMissingCases: codeActionExtractor = ({ return false; }; +// This detects concrete variables or values put in a position which expects an +// optional of that same type, and offers to wrap the value/variable in +// `Some()`. let simpleWrapOptionalWithSome: codeActionExtractor = ({ line, codeActions, @@ -609,6 +632,8 @@ let simpleWrapOptionalWithSome: codeActionExtractor = ({ return false; }; +// This detects when an optional is passed into a non optional slot, and offers +// to insert a switch for unwrapping that optional. let simpleUnwrapOptional: codeActionExtractor = ({ line, codeActions, From b63a1875549a4efd307e671763c6ab3d527713c7 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 6 Apr 2022 18:44:33 +0200 Subject: [PATCH 43/51] remove top level unit type action, since the error location given by the compiler isn't always enough to add ignore (we'd have to look at the AST, and we'd like to avoid that for the diagnostics driven actions) --- server/src/codeActions.ts | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 252653c3f..a7f2a059e 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -96,7 +96,6 @@ export let findCodeActionsInDiagnosticsMessage = ({ didYouMeanAction, addUndefinedRecordFields, simpleConversion, - topLevelUnitType, applyUncurried, simpleAddMissingCases, simpleWrapOptionalWithSome, @@ -388,41 +387,6 @@ let applyUncurried: codeActionExtractor = ({ return false; }; -// This action detects the compiler erroring when it finds a top level value -// that's not `unit`, and offers to wrap that value in `ignore`, which will make -// it compile. -let topLevelUnitType: codeActionExtractor = ({ - line, - codeActions, - file, - range, - diagnostic, -}) => { - if (line.startsWith("Toplevel expression is expected to have unit type.")) { - codeActions[file] = codeActions[file] || []; - let codeAction: p.CodeAction = { - title: `Wrap expression in ignore`, - edit: { - changes: { - [file]: wrapRangeInText(range, "ignore(", ")"), - }, - }, - diagnostics: [diagnostic], - kind: p.CodeActionKind.QuickFix, - isPreferred: true, - }; - - codeActions[file].push({ - range, - codeAction, - }); - - return true; - } - - return false; -}; - // This protects against the fact that the compiler currently returns most // text in OCaml. It also ensures that we only return simple constructors. let isValidVariantCase = (text: string): boolean => { From e26d79d93cbc5f964dea555154d7ac270dca66f6 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 8 Apr 2022 13:57:12 +0200 Subject: [PATCH 44/51] ensure code actions blowing up won't kill the server --- server/src/codeActions.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index a7f2a059e..7e8a6b8be 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -92,7 +92,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ diagnosticMessage.forEach((line, index, array) => { // Because of how actions work, there can only be one per diagnostic. So, // halt whenever a code action has been found. - let actions = [ + let codeActionEtractors = [ didYouMeanAction, addUndefinedRecordFields, simpleConversion, @@ -102,9 +102,11 @@ export let findCodeActionsInDiagnosticsMessage = ({ simpleUnwrapOptional, ]; - for (let action of actions) { - if ( - action({ + for (let extractCodeAction of codeActionEtractors) { + let didFindAction = false; + + try { + didFindAction = extractCodeAction({ array, codeActions, diagnostic, @@ -112,8 +114,12 @@ export let findCodeActionsInDiagnosticsMessage = ({ index, line, range, - }) - ) { + }); + } catch (e) { + console.error(e); + } + + if (didFindAction) { break; } } From 060cb900f81ff696dc303e53473d812cfded64c6 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 11 Apr 2022 15:51:31 +0200 Subject: [PATCH 45/51] tweaking the add matches action. more robust implementation for wrapping/unwrapping optional values --- server/src/codeActions.ts | 266 +++++++++++++++++++++----------------- 1 file changed, 149 insertions(+), 117 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 7e8a6b8be..1e62cc8ec 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -1,6 +1,7 @@ // This file holds code actions derived from diagnostics. There are more code // actions available in the extension, but they are derived via the analysis // OCaml binary. +import { match } from "assert"; import * as p from "vscode-languageserver-protocol"; export type filesCodeActions = { @@ -82,6 +83,52 @@ let insertBeforeEndingChar = ( ]; }; +let removeTrailingComma = (text: string): string => { + let str = text.trim(); + if (str.endsWith(",")) { + return str.slice(0, str.length - 1); + } + + return str; +}; + +let extractTypename = (lines: string[]): string => { + let arrFiltered: string[] = []; + + for (let i = 0; i <= lines.length - 1; i += 1) { + let line = lines[i]; + if (line.includes("(defined as")) { + let [typeStr, _] = line.split("(defined as"); + arrFiltered.push(removeTrailingComma(typeStr)); + break; + } else { + arrFiltered.push(removeTrailingComma(line)); + } + } + + return arrFiltered.join("").trim(); +}; + +let takeUntil = (array: string[], startsWith: string): string[] => { + let res: string[] = []; + let arr = array.slice(); + + let matched = false; + arr.forEach((line) => { + if (matched) { + return; + } + + if (line.startsWith(startsWith)) { + matched = true; + } else { + res.push(line); + } + }); + + return res; +}; + export let findCodeActionsInDiagnosticsMessage = ({ diagnostic, diagnosticMessage, @@ -93,13 +140,12 @@ export let findCodeActionsInDiagnosticsMessage = ({ // Because of how actions work, there can only be one per diagnostic. So, // halt whenever a code action has been found. let codeActionEtractors = [ + simpleTypeMismatches, didYouMeanAction, addUndefinedRecordFields, simpleConversion, applyUncurried, simpleAddMissingCases, - simpleWrapOptionalWithSome, - simpleUnwrapOptional, ]; for (let extractCodeAction of codeActionEtractors) { @@ -393,16 +439,6 @@ let applyUncurried: codeActionExtractor = ({ return false; }; -// This protects against the fact that the compiler currently returns most -// text in OCaml. It also ensures that we only return simple constructors. -let isValidVariantCase = (text: string): boolean => { - if (text.startsWith("(") || text.includes(",")) { - return false; - } - - return true; -}; - // Untransformed is typically OCaml, and looks like these examples: // // `SomeVariantName @@ -412,18 +448,37 @@ let isValidVariantCase = (text: string): boolean => { // ...and we'll need to transform this into proper ReScript. In the future, the // compiler itself should of course output real ReScript. But it currently does // not. -let transformVariant = (variant: string): string | null => { - // Convert old polyvariant notation to new - let text = variant.replace(/`/g, "#"); +// +// Note that we're trying to not be too clever here, so we'll only try to +// convert the very simplest cases - single variant/polyvariant, with single +// payloads. No records, tuples etc yet. We can add those when the compiler +// outputs them in proper ReScript. +let transformMatchPattern = (matchPattern: string): string | null => { + if ( + // Parens means tuples or more complicated payloads. Bail. + matchPattern.includes("(") || + // Braces means records. Bail. + matchPattern.includes("{") + ) { + return null; + } + + let text = matchPattern.replace(/`/g, "#"); - // Fix payloads + let payloadRegexp = / /g; + let matched = text.match(payloadRegexp); + + // Constructors are preceded by a single space. Bail if there's more than 1. + if (matched != null && matched.length > 2) { + return null; + } + + // Fix payloads if they can be fixed. If not, bail. if (text.includes(" ")) { let [variantText, payloadText] = text.split(" "); - // If the payload itself starts with (, it's another variant with a - // constructor. We bail in that case, for now at least. We'll be able to - // revisit this in the future when the compiler prints real ReScript syntax. - if (payloadText.startsWith("(")) { + let transformedPayloadText = transformMatchPattern(payloadText); + if (transformedPayloadText == null) { return null; } @@ -461,6 +516,10 @@ let simpleAddMissingCases: codeActionExtractor = ({ // // You forgot to handle a possible case here, for example: // AnotherValue + // + // You forgot to handle a possible case here, for example: + // (`One _|`Two _| + // `Three _) if ( line.startsWith("You forgot to handle a possible case here, for example:") @@ -469,28 +528,36 @@ let simpleAddMissingCases: codeActionExtractor = ({ // This collects the rest of the fields if fields are printed on // multiple lines. - array.slice(index + 1).forEach((line) => { - let theLine = line.trim(); - - let hasMultipleCases = theLine.includes("|"); - - if (hasMultipleCases) { - cases.push( - ...(theLine - // Remove leading and ending parens - .slice(1, theLine.length - 1) - .split("|") - .filter(isValidVariantCase) - .map(transformVariant) - .filter(Boolean) as string[]) - ); - } else { - let transformed = transformVariant(theLine); - if (isValidVariantCase(theLine) && transformed != null) { - cases.push(transformed); - } + let allCasesAsOneLine = array + .slice(index + 1) + .join("") + .trim(); + + // Remove leading and ending parens + allCasesAsOneLine = allCasesAsOneLine.slice( + 1, + allCasesAsOneLine.length - 1 + ); + + // Any parens left means this is a more complex match. Bail. + if (allCasesAsOneLine.includes("(")) { + return false; + } + + let hasMultipleCases = allCasesAsOneLine.includes("|"); + if (hasMultipleCases) { + cases.push( + ...(allCasesAsOneLine + .split("|") + .map(transformMatchPattern) + .filter(Boolean) as string[]) + ); + } else { + let transformed = transformMatchPattern(allCasesAsOneLine); + if (transformed != null) { + cases.push(transformed); } - }); + } if (cases.length === 0) { return false; @@ -541,7 +608,7 @@ let simpleAddMissingCases: codeActionExtractor = ({ // This detects concrete variables or values put in a position which expects an // optional of that same type, and offers to wrap the value/variable in // `Some()`. -let simpleWrapOptionalWithSome: codeActionExtractor = ({ +let simpleTypeMismatches: codeActionExtractor = ({ line, codeActions, file, @@ -559,91 +626,32 @@ let simpleWrapOptionalWithSome: codeActionExtractor = ({ // 50 │ // This has type: string // Somewhere wanted: option + // + // ...but types etc can also be on multilines, so we need a good + // amount of cleanup. - if (line.startsWith("Somewhere wanted: option<")) { - let somewhereWantedLine = line; - let thisHasTypeLine = array[index - 1]; - let hasTypeText = thisHasTypeLine.split("This has type: ")[1].trim(); - let somewhereWantedText = somewhereWantedLine - .split("Somewhere wanted: option<")[1] - .trim(); + let lookFor = "This has type:"; - // Remove ending `>` so we can compare the underlying types - somewhereWantedText = somewhereWantedText.slice( - 0, - somewhereWantedText.length - 1 + if (line.startsWith(lookFor)) { + let thisHasTypeArr = takeUntil( + [line.slice(lookFor.length), ...array.slice(index + 1)], + "Somewhere wanted:" ); + let somewhereWantedArr = array + .slice(index + thisHasTypeArr.length) + .map((line) => line.replace("Somewhere wanted:", "")); - // We only trigger the code action if the thing that's already there is the - // exact same type. - if (hasTypeText === somewhereWantedText) { - codeActions[file] = codeActions[file] || []; - let codeAction: p.CodeAction = { - title: `Wrap value in Some`, - edit: { - changes: { - [file]: wrapRangeInText(range, "Some(", ")"), - }, - }, - diagnostics: [diagnostic], - kind: p.CodeActionKind.QuickFix, - isPreferred: true, - }; - - codeActions[file].push({ - range, - codeAction, - }); - - return true; - } - } - - return false; -}; + let thisHasType = extractTypename(thisHasTypeArr); + let somewhereWanted = extractTypename(somewhereWantedArr); -// This detects when an optional is passed into a non optional slot, and offers -// to insert a switch for unwrapping that optional. -let simpleUnwrapOptional: codeActionExtractor = ({ - line, - codeActions, - file, - range, - diagnostic, - array, - index, -}) => { - // Examples: - // - // 47 │ - // 48 │ let as_ = { - // 49 │ someProp: optional, - // 50 │ another: Some("123"), - // 51 │ } - - // This has type: option - // Somewhere wanted: string - - if (line.startsWith("This has type: option<")) { - let thisHasTypeLine = line; - let hasTypeText = thisHasTypeLine.split("This has type: option<")[1].trim(); - // Remove ending `>` so we can compare the underlying types - hasTypeText = hasTypeText.slice(0, hasTypeText.length - 1); - - let somewhereWantedLine = array[index + 1]; - let somewhereWantedText = somewhereWantedLine - .split("Somewhere wanted: ")[1] - .trim(); - - // We only trigger the code action if the thing that's already there is the - // exact same type. - if (hasTypeText === somewhereWantedText) { + // Switching over an option + if (thisHasType === `option<${somewhereWanted}>`) { codeActions[file] = codeActions[file] || []; // We can figure out default values for primitives etc. let defaultValue = "assert false"; - switch (somewhereWantedText) { + switch (somewhereWanted) { case "string": { defaultValue = `"-"`; break; @@ -685,6 +693,30 @@ let simpleUnwrapOptional: codeActionExtractor = ({ return true; } + + // Wrapping a non-optional in Some + if (`option<${thisHasType}>` === somewhereWanted) { + codeActions[file] = codeActions[file] || []; + + let codeAction: p.CodeAction = { + title: `Wrap value in Some`, + edit: { + changes: { + [file]: wrapRangeInText(range, "Some(", ")"), + }, + }, + diagnostics: [diagnostic], + kind: p.CodeActionKind.QuickFix, + isPreferred: true, + }; + + codeActions[file].push({ + range, + codeAction, + }); + + return true; + } } return false; From e381574f6f81eb9b31768800b71e1a8a10871e86 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 23 Apr 2022 09:03:14 +0200 Subject: [PATCH 46/51] clean up positions --- analysis/src/Xform.ml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index 765ba85c7..b8ddeb19c 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -1,15 +1,14 @@ (** Code transformations using the parser/printer and ast operations *) let isBracedExpr = Res_parsetree_viewer.isBracedExpr -let posInLoc ~pos ~loc = Loc.start loc <= pos && pos < Loc.end_ loc -let mkPosition (p : Lexing.position) = - let line, character = Pos.ofLexing p in +let mkPosition (pos : Pos.t) = + let line, character = pos in {Protocol.line; character} let rangeOfLoc (loc : Location.t) = - let start = mkPosition loc.loc_start in - let end_ = mkPosition loc.loc_end in + let start = loc |> Loc.start |> mkPosition in + let end_ = loc |> Loc.end_ |> mkPosition in {Protocol.start; end_} module IfThenElse = struct @@ -68,7 +67,7 @@ module IfThenElse = struct }, e1, Some e2 ) - when posInLoc ~pos ~loc:e.pexp_loc -> ( + when Loc.hasPos ~pos e.pexp_loc -> ( let e1, e2 = if op = "=" then (e1, e2) else (e2, e1) in let mkMatch ~arg ~pat = let cases = @@ -144,7 +143,7 @@ module AddBracesToFn = struct in (match e.pexp_desc with | Pexp_fun (_, _, _, bodyExpr) - when posInLoc ~pos ~loc:bodyExpr.pexp_loc + when Loc.hasPos ~pos bodyExpr.pexp_loc && isBracedExpr bodyExpr = false && isFunction bodyExpr = false -> bodyExpr.pexp_attributes <- bracesAttribute :: bodyExpr.pexp_attributes; @@ -170,7 +169,7 @@ module AddTypeAnnotation = struct let mkIterator ~pos ~result = let processPattern ?(isUnlabeledOnlyArg = false) (pat : Parsetree.pattern) = match pat.ppat_desc with - | Ppat_var {loc} when posInLoc ~pos ~loc -> + | Ppat_var {loc} when Loc.hasPos ~pos loc -> result := Some (if isUnlabeledOnlyArg then WithParens else Plain) | _ -> () in @@ -259,7 +258,7 @@ let parse ~filename = let filterComments ~loc comments = (* Relevant comments in the range of the expression *) let filter comment = - posInLoc ~pos:(Loc.start (Res_comment.loc comment)) ~loc + Loc.hasPos ~pos:(Loc.start (Res_comment.loc comment)) loc in comments |> List.filter filter in From 77c5b291e599353ecc17cde60cd1b945ea1024f1 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 29 Apr 2022 20:37:56 +0200 Subject: [PATCH 47/51] simplify insert missing cases even more --- server/src/codeActions.ts | 45 +++++++++++++++------------------------ 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 1e62cc8ec..41596c885 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -454,15 +454,6 @@ let applyUncurried: codeActionExtractor = ({ // payloads. No records, tuples etc yet. We can add those when the compiler // outputs them in proper ReScript. let transformMatchPattern = (matchPattern: string): string | null => { - if ( - // Parens means tuples or more complicated payloads. Bail. - matchPattern.includes("(") || - // Braces means records. Bail. - matchPattern.includes("{") - ) { - return null; - } - let text = matchPattern.replace(/`/g, "#"); let payloadRegexp = / /g; @@ -533,32 +524,30 @@ let simpleAddMissingCases: codeActionExtractor = ({ .join("") .trim(); - // Remove leading and ending parens - allCasesAsOneLine = allCasesAsOneLine.slice( - 1, - allCasesAsOneLine.length - 1 - ); + // We only handle the simplest possible cases until the compiler actually + // outputs ReScript. This means bailing on anything that's not a + // variant/polyvariant, with one payload (or no payloads at all). + let openParensCount = allCasesAsOneLine.split("(").length - 1; - // Any parens left means this is a more complex match. Bail. - if (allCasesAsOneLine.includes("(")) { + if (openParensCount > 1 || allCasesAsOneLine.includes("{")) { return false; } - let hasMultipleCases = allCasesAsOneLine.includes("|"); - if (hasMultipleCases) { - cases.push( - ...(allCasesAsOneLine - .split("|") - .map(transformMatchPattern) - .filter(Boolean) as string[]) + // Remove surrounding braces if they exist + if (allCasesAsOneLine[0] === "(") { + allCasesAsOneLine = allCasesAsOneLine.slice( + 1, + allCasesAsOneLine.length - 1 ); - } else { - let transformed = transformMatchPattern(allCasesAsOneLine); - if (transformed != null) { - cases.push(transformed); - } } + cases.push( + ...(allCasesAsOneLine + .split("|") + .map(transformMatchPattern) + .filter(Boolean) as string[]) + ); + if (cases.length === 0) { return false; } From 0ea313cb7600e3c33824933dbd299553c28ffc6e Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 29 Apr 2022 20:38:41 +0200 Subject: [PATCH 48/51] cleanup --- server/src/codeActions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 41596c885..ea55a1e0d 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -1,7 +1,6 @@ // This file holds code actions derived from diagnostics. There are more code // actions available in the extension, but they are derived via the analysis // OCaml binary. -import { match } from "assert"; import * as p from "vscode-languageserver-protocol"; export type filesCodeActions = { From accc27000115bfab68ce142ff0513ec89686994e Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 30 Apr 2022 09:15:31 +0200 Subject: [PATCH 49/51] dead code --- analysis/src/Xform.ml | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index b8ddeb19c..b1ff4d036 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -279,29 +279,6 @@ let parse ~filename = in (structure, printExpr, printStructureItem) -let diff ~filename ~newContents = - match Files.readFile ~filename with - | None -> assert false - | Some oldContents -> - let rec findFirstLineDifferent n old new_ = - match (old, new_) with - | old1 :: oldRest, new1 :: newRest -> - if old1 = new1 then findFirstLineDifferent (n + 1) oldRest newRest - else (n, old, new_) - | _ -> (n, old, new_) - in - let oldLines = String.split_on_char '\n' oldContents in - let newLines = String.split_on_char '\n' newContents in - let firstLineDifferent, old, new_ = - findFirstLineDifferent 0 oldLines newLines - in - let firstLineR, _oldR, newR = - findFirstLineDifferent 0 (List.rev old) (List.rev new_) - in - let lastLineEqual = firstLineDifferent + List.length old - firstLineR in - let newLines = List.rev newR in - (firstLineDifferent, lastLineEqual, newLines) - let extractCodeActions ~path ~pos ~currentFile = let codeActions = ref [] in if Filename.check_suffix currentFile ".res" then ( From b2d338782d9b04b993eb3e6bae8d2401ac76e879 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 30 Apr 2022 09:28:21 +0200 Subject: [PATCH 50/51] refactor --- analysis/src/Xform.ml | 76 ++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml index b1ff4d036..c6b290453 100644 --- a/analysis/src/Xform.ml +++ b/analysis/src/Xform.ml @@ -99,11 +99,20 @@ module IfThenElse = struct {Ast_iterator.default_iterator with expr} - let xform ~pos structure = + let xform ~pos ~codeActions ~printExpr ~path structure = let changed = ref None in let iterator = mkIterator ~pos ~changed in iterator.structure iterator structure; - !changed + match !changed with + | None -> () + | Some newExpr -> + let range = rangeOfLoc newExpr.pexp_loc in + let newText = printExpr ~range newExpr in + let codeAction = + CodeActions.make ~title:"Replace with switch" ~kind:RefactorRewrite + ~uri:path ~newText ~range + in + codeActions := codeAction :: !codeActions end module AddBracesToFn = struct @@ -154,11 +163,20 @@ module AddBracesToFn = struct {Ast_iterator.default_iterator with expr; structure_item} - let xform ~pos structure = + let xform ~pos ~codeActions ~path ~printStructureItem structure = let changed = ref None in let iterator = mkIterator ~pos ~changed in iterator.structure iterator structure; - !changed + match !changed with + | None -> () + | Some newStructureItem -> + let range = rangeOfLoc newStructureItem.pstr_loc in + let newText = printStructureItem ~range newStructureItem in + let codeAction = + CodeActions.make ~title:"Add braces to function" ~kind:RefactorRewrite + ~uri:path ~newText ~range + in + codeActions := codeAction :: !codeActions end module AddTypeAnnotation = struct @@ -205,17 +223,17 @@ module AddTypeAnnotation = struct in {Ast_iterator.default_iterator with structure_item} - let getAction ~path ~pos ~full ~structure = + let xform ~path ~pos ~full ~structure ~codeActions = let line, col = pos in let result = ref None in let iterator = mkIterator ~pos ~result in iterator.structure iterator structure; match !result with - | None -> None + | None -> () | Some annotation -> ( match References.getLocItem ~full ~line ~col with - | None -> None + | None -> () | Some locItem -> ( match locItem.locType with | Typed (name, typ, _) -> @@ -232,8 +250,8 @@ module AddTypeAnnotation = struct CodeActions.make ~title:"Add type annotation" ~kind:RefactorRewrite ~uri:path ~newText ~range in - Some codeAction - | _ -> None)) + codeActions := codeAction :: !codeActions + | _ -> ())) end let indent n text = @@ -280,37 +298,15 @@ let parse ~filename = (structure, printExpr, printStructureItem) let extractCodeActions ~path ~pos ~currentFile = - let codeActions = ref [] in - if Filename.check_suffix currentFile ".res" then ( + let fullOpt = Cmt.fromPath ~path in + match fullOpt with + | Some full when Filename.check_suffix currentFile ".res" -> let structure, printExpr, printStructureItem = parse ~filename:currentFile in - let fullOpt = Cmt.fromPath ~path in - (match fullOpt with - | None -> () - | Some full -> ( - match AddTypeAnnotation.getAction ~path ~pos ~full ~structure with - | None -> () - | Some action -> codeActions := action :: !codeActions)); - (match IfThenElse.xform ~pos structure with - | None -> () - | Some newExpr -> - let range = rangeOfLoc newExpr.pexp_loc in - let newText = printExpr ~range newExpr in - let codeAction = - CodeActions.make ~title:"Replace with switch" ~kind:RefactorRewrite - ~uri:path ~newText ~range - in - codeActions := codeAction :: !codeActions); - - match AddBracesToFn.xform ~pos structure with - | None -> () - | Some newStructureItem -> - let range = rangeOfLoc newStructureItem.pstr_loc in - let newText = printStructureItem ~range newStructureItem in - let codeAction = - CodeActions.make ~title:"Add braces to function" ~kind:RefactorRewrite - ~uri:path ~newText ~range - in - codeActions := codeAction :: !codeActions); - !codeActions + let codeActions = ref [] in + AddTypeAnnotation.xform ~path ~pos ~full ~structure ~codeActions; + IfThenElse.xform ~pos ~codeActions ~printExpr ~path structure; + AddBracesToFn.xform ~pos ~codeActions ~path ~printStructureItem structure; + !codeActions + | _ -> [] From 1d4a85cde4a19dbdcf3904c3f78843c550ea78be Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 30 Apr 2022 09:50:45 +0200 Subject: [PATCH 51/51] cleanup --- server/src/server.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/server/src/server.ts b/server/src/server.ts index 71891d7f4..1bd547988 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -492,27 +492,25 @@ function codeAction(msg: p.RequestMessage): p.ResponseMessage { filePath, params.range.start.line, params.range.start.character, - tmpname + tmpname, ], msg ); fs.unlink(tmpname, () => null); - let { result } = response; - let res: v.ResponseMessage = { - jsonrpc: c.jsonrpcVersion, - id: msg.id, - }; - // We must send `null` when there are no results, empty array isn't enough. let codeActions = result != null && Array.isArray(result) ? [...localResults, ...result] : localResults; - res.result = codeActions.length > 0 ? codeActions : null; + let res: v.ResponseMessage = { + jsonrpc: c.jsonrpcVersion, + id: msg.id, + result: codeActions.length > 0 ? codeActions : null, + }; return res; }