From da6b362c9026211e608592bd7c8b4515769b009c Mon Sep 17 00:00:00 2001 From: Scott Ming Date: Sat, 8 Jun 2024 09:49:56 +0800 Subject: [PATCH] Apply some code review suggestions --- .../code_action/handlers/organize_aliases.ex | 56 +++++++++++++++---- .../handlers/organize_aliases_test.exs | 24 ++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/apps/remote_control/lib/lexical/remote_control/code_action/handlers/organize_aliases.ex b/apps/remote_control/lib/lexical/remote_control/code_action/handlers/organize_aliases.ex index 11b37ee6c..e51d404ae 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_action/handlers/organize_aliases.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_action/handlers/organize_aliases.ex @@ -17,7 +17,7 @@ defmodule Lexical.RemoteControl.CodeAction.Handlers.OrganizeAliases do @impl CodeAction.Handler def actions(%Document{} = doc, %Range{} = range, _diagnostics) do with {:ok, _doc, analysis} <- Document.Store.fetch(doc.uri, :analysis), - :ok <- check_aliases(doc, analysis, range) do + :ok <- check_aliases(analysis, range) do aliases = CodeMod.Aliases.in_scope(analysis, range) {insert_position, trailer} = CodeMod.Aliases.insert_position(analysis, range.start) edits = CodeMod.Aliases.to_edits(aliases, insert_position, trailer) @@ -39,15 +39,16 @@ defmodule Lexical.RemoteControl.CodeAction.Handlers.OrganizeAliases do [:source, :source_organize_imports] end - defp check_aliases(%Document{}, %Analysis{} = analysis, %Range{} = range) do - with %Scope{aliases: [_ | _]} <- Analysis.module_scope(analysis, range), - true <- - ancestor_is_alias?(analysis, range.start) or - token_at_cursor_is_alias?(analysis, range.start) do + defp check_aliases(%Analysis{} = analysis, %Range{} = range) do + checked = + ancestor_is_alias?(analysis, range.start) or + token_at_cursor_is_alias?(analysis, range.start) or + inside_a_scope_with_aliases?(analysis, range) + + if checked do :ok else - _ -> - :error + :error end end @@ -65,7 +66,42 @@ defmodule Lexical.RemoteControl.CodeAction.Handlers.OrganizeAliases do defp token_at_cursor_is_alias?(%Analysis{} = analysis, %Position{} = position) do project = RemoteControl.get_project() - {:ok, env} = Ast.Env.new(project, analysis, position) - (env.prefix <> env.suffix) |> String.trim_leading() |> String.starts_with?("alias") + + case Ast.Env.new(project, analysis, position) do + {:ok, env} -> + (env.prefix <> env.suffix) |> String.trim_leading() |> String.starts_with?("alias") + + _ -> + false + end + end + + defp inside_a_scope_with_aliases?(%Analysis{} = analysis, %Range{} = range) do + # defmodule WithAliases do + # alias Baz.Quux + # alias Foo.Bar + # | + case Analysis.module_scope(analysis, range) do + %Scope{aliases: [_ | _]} -> + token_at_cursor_is_blank?(analysis, range.start) + + _ -> + false + end + end + + defp token_at_cursor_is_blank?(%Analysis{} = analysis, %Position{} = position) do + project = RemoteControl.get_project() + + case Ast.Env.new(project, analysis, position) do + {:ok, env} -> + last_prefix = String.last(env.prefix) + first_suffix = String.first(env.suffix) + blanks = [" ", "", "\n", nil] + first_suffix in blanks and last_prefix in blanks + + _ -> + false + end end end diff --git a/apps/remote_control/test/lexical/remote_control/code_action/handlers/organize_aliases_test.exs b/apps/remote_control/test/lexical/remote_control/code_action/handlers/organize_aliases_test.exs index 685b8a70c..aa5528ac2 100644 --- a/apps/remote_control/test/lexical/remote_control/code_action/handlers/organize_aliases_test.exs +++ b/apps/remote_control/test/lexical/remote_control/code_action/handlers/organize_aliases_test.exs @@ -324,6 +324,30 @@ defmodule Lexical.RemoteControl.CodeAction.Handlers.OrganizeAliasesTest do end describe "check the return conditions for the alias" do + test "returns organized aliases if the cursor's immediate parent is a module" do + patch(RemoteControl, :get_project, %Lexical.Project{}) + + {:ok, organized} = + ~q[ + defmodule Outer do + alias Foo.Bar + alias A.B.C + alias D.E.F + | a + end] |> organize_aliases() + + expected = + ~q[ + defmodule Outer do + alias A.B.C + alias D.E.F + alias Foo.Bar + a + end] + + assert expected == organized + end + test "returns organized aliases if the cursor is at an alias" do patch(RemoteControl, :get_project, %Lexical.Project{})