Fix handle module name whitespace (#1195)

* Fix false positive module name check diagnostic

Discovered that modules with whitespace in their name could cause false
positives in the module name check diagnostic.
Root cause is in els_uri:path/1, since uri_string:normalize/1 return a percent
encoded path, we need to percent decode that path to get the real path.

* Shouldn't need to handle percent encoding for windows paths any more

This change essentially reverts #1017 and adds a unit test

* Use http_uri:decode/1 for older OTP releases

apps/els_core/src/els_uri.erl

@@ -31,16 +31,20 @@ module(Uri) ->
 
 -spec path(uri()) -> path().
 path(Uri) ->
+  path(Uri, is_windows()).
+
+-spec path(uri(), boolean()) -> path().
+path(Uri, IsWindows) ->
   #{ host := Host
-   , path := Path
+   , path := Path0
    , scheme := <<"file">>
    } = uri_string:normalize(Uri, [return_map]),
-
-  case {is_windows(), Host} of
+  Path = percent_decode(Path0),
+  case {IsWindows, Host} of
     {true, <<>>} ->
       % Windows drive letter, have to strip the initial slash
       re:replace(
-        Path, "^/([a-zA-Z])(:|%3A)(.*)", "\\1:\\3", [{return, binary}]
+        Path, "^/([a-zA-Z]:)(.*)", "\\1\\2", [{return, binary}]
       );
     {true, _} ->
       <<"//", Host/binary, Path/binary>>;
@@ -81,3 +85,34 @@ uri_join(List) ->
 is_windows() ->
   {OS, _} = os:type(),
   OS =:= win32.
+
+-if(?OTP_RELEASE >= 23).
+-spec percent_decode(binary()) -> binary().
+percent_decode(Str) ->
+  uri_string:percent_decode(Str).
+-else.
+-spec percent_decode(binary()) -> binary().
+percent_decode(Str) ->
+  http_uri:decode(Str).
+-endif.
+
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+
+path_uri_test_() ->
+  [ ?_assertEqual( <<"/foo/bar.erl">>
+                 , path(<<"file:///foo/bar.erl">>))
+  , ?_assertEqual( <<"/foo/bar baz.erl">>
+                 , path(<<"file:///foo/bar%20baz.erl">>))
+  , ?_assertEqual( <<"/foo/bar.erl">>
+                 , path(uri(path(<<"file:///foo/bar.erl">>))))
+  , ?_assertEqual( <<"/foo/bar baz.erl">>
+                 , path(uri(<<"/foo/bar baz.erl">>)))
+  , ?_assertEqual( <<"file:///foo/bar%20baz.erl">>
+                 , uri(<<"/foo/bar baz.erl">>))
+  ].
+
+path_windows_test() ->
+  ?assertEqual(<<"C:/foo/bar.erl">>,
+               path(<<"file:///C%3A/foo/bar.erl">>, true)).
+-endif.

apps/els_lsp/priv/code_navigation/src/diagnostics module name check.erl

@@ -0,0 +1 @@
+-module('diagnostics module name check').

apps/els_lsp/test/els_diagnostics_SUITE.erl

@@ -42,6 +42,7 @@
         , unused_record_fields/1
         , gradualizer/1
         , module_name_check/1
+        , module_name_check_whitespace/1
         ]).
 
 %%==============================================================================
@@ -669,6 +670,15 @@ module_name_check(_Config) ->
   Hints = [],
   els_test:run_diagnostics_test(Path, Source, Errors, Warnings, Hints).
 
+-spec module_name_check_whitespace(config()) -> ok.
+module_name_check_whitespace(_Config) ->
+  Path = src_path("diagnostics module name check.erl"),
+  Source = <<"Compiler (via Erlang LS)">>,
+  Errors = [],
+  Warnings = [],
+  Hints = [],
+  els_test:run_diagnostics_test(Path, Source, Errors, Warnings, Hints).
+
 %%==============================================================================
 %% Internal Functions
 %%==============================================================================