Skip to content

Improve location of some custom operators #1917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions src/kernel/mreader_lexer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ let comments t =

open Parser_raw

let pair_bracket = function
| '{' -> Some RBRACE
| '(' -> Some RPAREN
| '[' -> Some RBRACKET
| _ -> None

let is_operator = function
| PREFIXOP s
| LETOP s
Expand All @@ -148,6 +154,20 @@ let is_operator = function
| AMPERAMPER -> Some "&&"
| COLONEQUAL -> Some ":="
| PLUSEQ -> Some "+="
| DOTOP s -> (
let last = String.get s (String.length s - 1) in
match pair_bracket last with
| Some pair ->
(* note: this is a heuristic which ignores the difference between
the following three operators:
[.%( )]
[.%(;..)]
[.%(;..)<-]
It will always return the first one. Now, typically, if one
is defined, all are, with the same semantics, but this is
still unfortunate. *)
Some (s ^ Parser_printer.print_token pair)
| None -> Some s)
| _ -> None

(* [reconstruct_identifier] is impossible to read at the moment, here is a
Expand Down Expand Up @@ -233,6 +253,75 @@ let reconstruct_identifier_from_tokens tokens pos =
(* LIDENT always begin a new identifier *)
| ((LIDENT _, _, _) as item) :: items ->
if acc = [] then look_for_dot [ item ] items else check acc (item :: items)
(* Reified custom indexing operators *)
(* e.g. [( .%(;..) )] *)
| (RPAREN, _, _)
:: (token, _, tend)
:: (DOTDOT, _, _)
:: (SEMI, _, _)
:: (DOTOP s, tstart, _)
:: (LPAREN, _, _)
:: items
when acc = [] -> (
let last = String.get s (String.length s - 1) in
match pair_bracket last with
| Some pair when pair = token ->
let item =
(DOTOP (s ^ ";.." ^ Parser_printer.print_token pair), tstart, tend)
in
look_for_dot [ item ] items
| _ -> check acc items)
(* e.g. [( .%(;..)<- )] *)
| (RPAREN, _, _)
:: (LESSMINUS, _, tend)
:: (token, _, _)
:: (DOTDOT, _, _)
:: (SEMI, _, _)
:: (DOTOP s, tstart, _)
:: (LPAREN, _, _)
:: items
when acc = [] -> (
let last = String.get s (String.length s - 1) in
match pair_bracket last with
| Some pair when pair = token ->
let item =
( DOTOP (s ^ ";.." ^ Parser_printer.print_token pair ^ "<-"),
tstart,
tend )
in
look_for_dot [ item ] items
| _ -> check acc items)
(* e.g. [( .%( ) )] *)
| (RPAREN, _, _)
:: (token, _, tend)
:: (DOTOP s, tstart, _)
:: (LPAREN, _, _)
:: items
when acc = [] -> (
let last = String.get s (String.length s - 1) in
match pair_bracket last with
| Some pair when pair = token ->
let item =
(DOTOP (s ^ Parser_printer.print_token pair), tstart, tend)
in
look_for_dot [ item ] items
| _ -> check acc items)
(* e.g. [( .%( )<- )] *)
| (RPAREN, _, _)
:: (LESSMINUS, _, tend)
:: (token, _, _)
:: (DOTOP s, tstart, _)
:: (LPAREN, _, _)
:: items
when acc = [] -> (
let last = String.get s (String.length s - 1) in
match pair_bracket last with
| Some pair when pair = token ->
let item =
(DOTOP (s ^ Parser_printer.print_token pair ^ "<-"), tstart, tend)
in
look_for_dot [ item ] items
| _ -> check acc items)
(* Reified operators behave like LIDENT *)
| (RPAREN, _, _) :: ((token, _, _) as item) :: (LPAREN, _, _) :: items
when is_operator token <> None && acc = [] -> look_for_dot [ item ] items
Expand Down
12 changes: 8 additions & 4 deletions src/ocaml/preprocess/lexer_ident.mll
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,15 @@ rule token = parse
| "'" { QUOTE }
| "(" { LPAREN }
| ")" { RPAREN }
| "}" { RBRACE }
| "]" { RBRACKET }
| ".." { DOTDOT }
| "<-" { LESSMINUS }
| ";" { SEMI }
| "." dotsymbolchar+ ['(' '{' '[' ]
{ DOTOP(Lexing.lexeme lexbuf) }
| "." { DOT }
| ":=" { COLONEQUAL }
| "!" symbolchar +
{ PREFIXOP(Lexing.lexeme lexbuf) }
| ['~' '?'] symbolchar +
Expand Down Expand Up @@ -144,12 +152,9 @@ rule token = parse
| "*"
| ","
| "->"
| ".."
| ":"
| "::"
| ":="
| ":>"
| ";"
| ";;"
| "<"
| "<-"
Expand All @@ -174,7 +179,6 @@ rule token = parse
| "[@@"
| "[@@@"
| "!"

| "!="
| "+"
| "+."
Expand Down
8 changes: 7 additions & 1 deletion tests/test-dirs/locate/context-detection/cd-test.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@ FIXME we failed to parse/reconstruct the ident, that's interesting
$ $MERLIN single locate -look-for ml -position 16:16 -filename ./test.ml < ./test.ml
{
"class": "return",
"value": "Not a valid identifier",
"value": {
"file": "$TESTCASE_ROOT/test.ml",
"pos": {
"line": 13,
"col": 11
}
},
"notifications": []
}

Expand Down
98 changes: 98 additions & 0 deletions tests/test-dirs/locate/issue1915.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
Testing the behavior of custom operators

$ cat >main.ml <<EOF
> let ( := ) v a = Printf.printf "%s = %d;\n" v a
> let () = "foo" := 3
> let () = ( := ) "foo" 3
> EOF

$ $MERLIN single locate -look-for ml -position 2:17 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 1,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 3:12 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 1,
"col": 4
}

Testing custom indexing operators

$ cat >main.ml <<EOF
> let (.%{;..}) a k = Printf.printf "%s.coeffRef(%d);\n" a k.(0)
> let (.%{ }) a k = Printf.printf "%s.coeffRef(%d);\n" a k
> let name = "baz"
> let () = name.%{2;4}
> let () = name.%{5}
> let () = ( .%{;..} ) name 7
> let () = ( .%{ } ) name 3
> EOF

Should be on line 1
$ $MERLIN single locate -look-for ml -position 4:15 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 2,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 4:16 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 2,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 6:13 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 1,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 6:14 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 1,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 6:15 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 1,
"col": 4
}

Should be on line 2
$ $MERLIN single locate -look-for ml -position 5:15 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 2,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 5:15 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 2,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 5:16 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 2,
"col": 4
}

$ $MERLIN single locate -look-for ml -position 7:15 \
> -filename ./main.ml < ./main.ml | jq '.value.pos'
{
"line": 2,
"col": 4
}
Loading