Skip to content

Commit

Permalink
[IDE] Fix rename with shorthand named imports
Browse files Browse the repository at this point in the history
Summary:
This is a quick fix for a bug in the implementation of the LSP renaming action. Previously, it didn't check for whether a named import was actually making use of the shorthand syntax. It erronously expanded the identifier to be `oldName as newName` even when `oldName` was already a local name.

I also changed the name of the type to `shorthandKind` to make it clear this refers to a shorthand of either an object or an import.

Changelog: [internal]

Reviewed By: mroch

Differential Revision:
D43704868 (f6ddb95)

------------------------------------------------------------------------
(from 144c8b075c5d77083f456cbe48a2019b012a4959)

fbshipit-source-id: ff215adc6c2d3b331da2e59648a8841b69583ea8
  • Loading branch information
evanyeung authored and facebook-github-bot committed Mar 1, 2023
1 parent b8531a1 commit 0ff8bb8
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 15 deletions.
2 changes: 1 addition & 1 deletion newtests/lsp/rename/__snapshots__/imports_5.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"character": 30
}
},
"newText": "wutLocal as NEW_NAME"
"newText": "NEW_NAME"
},
{
"range": {
Expand Down
2 changes: 1 addition & 1 deletion src/server/command_handler/commandHandler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2155,7 +2155,7 @@ let handle_persistent_rename ~reader ~options ~id ~params ~metadata ~client ~pro
in
let layout = Js_layout_generator.import_named_specifier expanded_named_import in
Pretty_printer.print ~source_maps:None ~skip_endline:true layout |> Source.contents
| Some PropertyShorthandSearcher.Shorthand ->
| Some PropertyShorthandSearcher.Obj ->
let (from_name, to_name) =
match ref_kind with
| FindRefsTypes.PropertyDefinition
Expand Down
22 changes: 12 additions & 10 deletions src/services/references/propertyShorthandSearcher.ml
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ module Ast = Flow_ast
module LocMap = Loc_collections.LocMap
module LocSet = Loc_collections.LocSet

type propKind =
| Shorthand
type shorthandKind =
| Obj
| Import

let loc_of_aloc = Parsing_heaps.Reader.loc_of_aloc

class searcher ~reader ~(targets : Loc_collections.LocSet.t) ~(result : propKind LocMap.t ref) =
class searcher ~reader ~(targets : Loc_collections.LocSet.t) ~(result : shorthandKind LocMap.t ref)
=
object (_this)
inherit
[ALoc.t, ALoc.t * Type.t, ALoc.t, ALoc.t * Type.t] Flow_polymorphic_ast_mapper.mapper as super
Expand All @@ -26,10 +27,12 @@ class searcher ~reader ~(targets : Loc_collections.LocSet.t) ~(result : propKind

method! import_named_specifier ~import_kind:_ specifier =
let open Flow_ast.Statement.ImportDeclaration in
let { kind = _; local; remote } = specifier in
let ((aloc, _), _) = Base.Option.value ~default:remote local in
let loc = loc_of_aloc ~reader aloc in
if LocSet.mem loc targets then result := LocMap.add loc Import !result;
let { local; remote = ((aloc, _), _); kind = _ } = specifier in
(match local with
| Some _ -> ()
| None ->
let loc = loc_of_aloc ~reader aloc in
if LocSet.mem loc targets then result := LocMap.add loc Import !result);
specifier

method! pattern ?kind expr =
Expand All @@ -45,8 +48,7 @@ class searcher ~reader ~(targets : Loc_collections.LocSet.t) ~(result : propKind
(match key with
| Property.Identifier ((aloc, _), _) ->
let loc = loc_of_aloc ~reader aloc in
if shorthand && LocSet.mem loc targets then
result := LocMap.add loc Shorthand !result
if shorthand && LocSet.mem loc targets then result := LocMap.add loc Obj !result
| Property.Computed _
| Property.Literal _ ->
())
Expand All @@ -67,7 +69,7 @@ class searcher ~reader ~(targets : Loc_collections.LocSet.t) ~(result : propKind
| Init { key = Identifier id; shorthand; value = _ } ->
let ((aloc, _), _) = id in
let loc = loc_of_aloc ~reader aloc in
if shorthand && LocSet.mem loc targets then result := LocMap.add loc Shorthand !result
if shorthand && LocSet.mem loc targets then result := LocMap.add loc Obj !result
| _ -> ()
in
super#object_property prop
Expand Down
6 changes: 3 additions & 3 deletions src/services/references/propertyShorthandSearcher.mli
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
* LICENSE file in the root directory of this source tree.
*)

type propKind =
| Shorthand
type shorthandKind =
| Obj
| Import

(**
Expand All @@ -17,4 +17,4 @@ val search :
reader:Parsing_heaps.Reader.reader ->
targets:Loc_collections.LocSet.t ->
(ALoc.t, ALoc.t * Type.t) Flow_ast.Program.t ->
propKind Loc_collections.LocMap.t
shorthandKind Loc_collections.LocMap.t

0 comments on commit 0ff8bb8

Please sign in to comment.