Properly resolve variables as arguments in libspec generation. Fixes #…

…535

robocorp-code/codegen/codegen_package.py

@@ -284,7 +284,9 @@ def write_py_settings():
  )
 
  with open(settings_py_file, "w") as stream:
+ stream.write("# fmt: off\n")
  stream.write("\n".join(settings_template))
+ stream.write("\n# fmt: on\n")
 
  print("Written: %s" % (settings_py_file,))
 

robocorp-code/src/robocorp_code/settings.py

@@ -1,3 +1,4 @@
+# fmt: off
 # Warning: Don't edit file (autogenerated from python -m dev codegen).
 
 ROBOCORP_LANGUAGE_SERVER_TCP_PORT = "robocorp.language-server.tcp-port"
@@ -23,3 +24,5 @@
  ROBOCORP_AUTO_SET_PYTHON_EXTENSION_INTERPRETER,
  )
 )
+
+# fmt: on

robotframework-ls/src/robotframework_ls/impl/libspec_manager.py

@@ -859,7 +859,11 @@ def _cached_create_libspec(
  if os.path.exists(entry):
  call.extend(["-P", os.path.normpath(entry)])
 
- call.append("::".join([libname, args] if args else [libname]))
+ if not args:
+ call.append(libname)
+ else:
+ call.append("::".join([libname, args]))
+
  libspec_filename = self._compute_libspec_filename(
  libname, is_builtin, target_file, args
  )
@@ -1038,8 +1042,8 @@ def _get_library_target_filename(
  def get_library_info(
  self,
  libname: str,
- create: bool = True,
- current_doc_uri: Optional[str] = None,
+ create: bool,
+ current_doc_uri: str,
  builtin: bool = False,
  args: Optional[str] = None,
  ) -> Optional[ILibraryDoc]:
@@ -1050,6 +1054,7 @@ def get_library_info(
 
  :rtype: LibraryDoc
  """
+ assert current_doc_uri is not None
 
  libname_lower = libname.lower()
  target_file: str = ""
@@ -1076,6 +1081,19 @@ def get_library_info(
  ):
  args = None
 
+ if args and "{" in args:
+ # We need to resolve the arguments if there are variables in it.
+ from robotframework_ls.impl.completion_context import (
+ CompletionContext,
+ )
+ from robocorp_ls_core.workspace import Document
+
+ # We just need the doc for the CURDIR variable, so create a dummy
+ # doc with that uri.
+ doc = Document(current_doc_uri, "")
+ ctx = CompletionContext(doc, config=self.config)
+ args = ctx.token_value_resolving_variables(args)
+
  if not builtin:
  found_target_filename = self._get_library_target_filename(
  libname, current_doc_uri

robotframework-ls/tests/robotframework_ls_tests/completions/test_keyword_completions.py

@@ -602,7 +602,14 @@ def test_typing_not_shown(libspec_manager, workspace, data_regression, workspace
  with open(os.path.join(workspace_dir_a, "my.libspec"), "w") as stream:
  stream.write(LIBSPEC_3)
  libspec_manager.add_workspace_folder(uris.from_fs_path(workspace_dir_a))
- assert libspec_manager.get_library_info("case3_library", create=False) is not None
+ assert (
+ libspec_manager.get_library_info(
+ "case3_library",
+ False,
+ uris.from_fs_path(os.path.join(workspace_dir_a, "my.robot")),
+ )
+ is not None
+ )
 
  workspace.set_root(workspace_dir, libspec_manager=libspec_manager)
 
@@ -808,6 +815,42 @@ def test_keyword_completions_library_with_params_with_space(
  ]
 
 
+def test_keyword_completions_library_with_params_resolves_var(
+ workspace, libspec_manager
+):
+ from robotframework_ls.impl import keyword_completions
+ from robotframework_ls.impl.completion_context import CompletionContext
+ from robotframework_ls.robot_config import RobotConfig
+
+ config = RobotConfig()
+ config.update(
+ {
+ "robot.libraries": {"libdoc": {"needsArgs": ["LibWithParams"]}},
+ "robot.variables": {"param_val": "foo"},
+ }
+ )
+ libspec_manager.config = config
+
+ workspace.set_root("case_params_on_lib", libspec_manager=libspec_manager)
+ doc = workspace.put_doc("case_params_on_lib.robot")
+
+ doc.source = """
+*** Settings ***
+Library LibWithParams some_param=${param_val} WITH NAME Lib
+
+*** Test Case ***
+My Test
+ Lib.Foo"""
+
+ completion_context = CompletionContext(doc, workspace=workspace.ws, config=config)
+ completions = keyword_completions.complete(completion_context)
+
+ assert len(completions) == 1
+ assert sorted([comp["label"] for comp in completions]) == [
+ "Foo Method (LibWithParams)"
+ ]
+
+
 @pytest.mark.parametrize("lib_param", ["bar", "foo"])
 def test_code_analysis_same_lib_with_alias_with_params(
  workspace, libspec_manager, cases, lib_param

robotframework-ls/tests/robotframework_ls_tests/test_libspec_manager.py

@@ -1,14 +1,18 @@
 import os
 from pathlib import Path
 from typing import Optional
+from robocorp_ls_core import uris
 
 
 def test_libspec_info(libspec_manager, tmpdir):
  from robotframework_ls.impl.robot_specbuilder import LibraryDoc
  from robotframework_ls.impl.robot_specbuilder import KeywordDoc
 
  assert "BuiltIn" in libspec_manager.get_library_names()
- lib_info = libspec_manager.get_library_info("BuiltIn", create=False)
+ uri = uris.from_fs_path(str(tmpdir.join("case.robot")))
+ lib_info = libspec_manager.get_library_info(
+ "BuiltIn", create=False, current_doc_uri=uri
+ )
  assert isinstance(lib_info, LibraryDoc)
  assert lib_info.source is not None
  assert lib_info.source.endswith("BuiltIn.py")
@@ -73,7 +77,10 @@ def method6():
 """
  )
 
- library_info: Optional[LibraryDoc] = libspec_manager.get_library_info("check_lib")
+ uri = uris.from_fs_path(os.path.join(workspace_dir, "case.robot"))
+ library_info: Optional[LibraryDoc] = libspec_manager.get_library_info(
+ "check_lib", True, uri
+ )
  assert library_info is not None
  keywords: List[KeywordDoc] = library_info.keywords
  data_regression.check([keyword_to_dict(k) for k in keywords])
@@ -94,13 +101,18 @@ def test_libspec_cache_no_lib(libspec_manager, workspace_dir):
  def disallow_cached_create_libspec(*args, **kwargs):
  raise AssertionError("Should not be called")
 
- library_info: Optional[LibraryDoc] = libspec_manager.get_library_info("check_lib")
+ uri = uris.from_fs_path(os.path.join(workspace_dir, "case.robot"))
+ library_info: Optional[LibraryDoc] = libspec_manager.get_library_info(
+ "check_lib", True, uri
+ )
  assert library_info is None
 
  # Make sure that we don't try to create it anymore for the same lib.
  original_cached_create_libspec = libspec_manager._cached_create_libspec
  libspec_manager._cached_create_libspec = disallow_cached_create_libspec
- library_info: Optional[LibraryDoc] = libspec_manager.get_library_info("check_lib")
+ library_info: Optional[LibraryDoc] = libspec_manager.get_library_info(
+ "check_lib", True, uri
+ )
  assert library_info is None
  libspec_manager._cached_create_libspec = original_cached_create_libspec
 
@@ -119,7 +131,7 @@ def method2(a:int):
  )
  # Check that the cache invalidation is in place!
  wait_for_condition(
- lambda: libspec_manager.get_library_info("check_lib") is not None,
+ lambda: libspec_manager.get_library_info("check_lib", True, uri) is not None,
  msg="Did not recreate library in the available timeout.",
  timeout=15,
  )
@@ -178,12 +190,14 @@ def raise_error(cmdline, *args, **kwargs):
 
  libspec_manager._subprocess_check_output = raise_error
 
- library_info: Optional[LibraryDoc] = libspec_manager.get_library_info("check_lib")
+ uri = uris.from_fs_path(os.path.join(workspace_dir, "case.robot"))
+ library_info: Optional[LibraryDoc] = libspec_manager.get_library_info(
+ "check_lib", True, uri
+ )
  assert library_info is not None
 
 
 def test_libspec_manager_caches(libspec_manager, workspace_dir):
- from robocorp_ls_core import uris
  import os.path
  from robotframework_ls_tests.fixtures import LIBSPEC_1
  from robotframework_ls_tests.fixtures import LIBSPEC_2
@@ -196,18 +210,24 @@ def test_libspec_manager_caches(libspec_manager, workspace_dir):
  with open(os.path.join(workspace_dir_a, "my.libspec"), "w") as stream:
  stream.write(LIBSPEC_1)
  libspec_manager.add_workspace_folder(uris.from_fs_path(workspace_dir_a))
- assert libspec_manager.get_library_info("case1_library", create=False) is not None
+ uri = uris.from_fs_path(os.path.join(workspace_dir, "case.robot"))
+ assert (
+ libspec_manager.get_library_info(
+ "case1_library", create=False, current_doc_uri=uri
+ )
+ is not None
+ )
 
  libspec_manager.remove_workspace_folder(uris.from_fs_path(workspace_dir_a))
- library_info = libspec_manager.get_library_info("case1_library", create=False)
+ library_info = libspec_manager.get_library_info("case1_library", False, uri)
  if library_info is not None:
  raise AssertionError(
  "Expected: %s to be None after removing %s"
  % (library_info, uris.from_fs_path(workspace_dir_a))
  )
 
  libspec_manager.add_workspace_folder(uris.from_fs_path(workspace_dir_a))
- assert libspec_manager.get_library_info("case1_library", create=False) is not None
+ assert libspec_manager.get_library_info("case1_library", False, uri) is not None
 
  # Give a timeout so that the next write will have at least 1 second
  # difference (1s is the minimum for poll to work).
@@ -216,13 +236,13 @@ def test_libspec_manager_caches(libspec_manager, workspace_dir):
  stream.write(LIBSPEC_2)
 
  def check_spec_found():
- library_info = libspec_manager.get_library_info("case2_library", create=False)
+ library_info = libspec_manager.get_library_info("case2_library", False, uri)
  return library_info is not None
 
  # Updating is done in a thread.
  wait_for_test_condition(check_spec_found, sleep=1 / 5.0)
 
- library_info = libspec_manager.get_library_info("case2_library", create=False)
+ library_info = libspec_manager.get_library_info("case2_library", False, uri)
  assert set(x.name for x in library_info.keywords) == set(
  ["Case 2 Verify Another Model", "Case 2 Verify Model"]
  )
@@ -234,7 +254,7 @@ def check_spec_found():
  stream.write(LIBSPEC_2_A)
 
  def check_spec_2_a():
- library_info = libspec_manager.get_library_info("case2_library", create=False)
+ library_info = libspec_manager.get_library_info("case2_library", False, uri)
  if library_info:
  return set(x.name for x in library_info.keywords) == set(
  ["Case 2 A Verify Another Model", "Case 2 A Verify Model"]
@@ -252,6 +272,8 @@ def test_libspec_manager_basic(workspace, libspec_manager):
 
  def get_library_info(*args, **kwargs):
  kwargs["current_doc_uri"] = doc.uri
+ if "create" not in kwargs:
+ kwargs["create"] = True
  return libspec_manager.get_library_info(*args, **kwargs)
 
  assert get_library_info("case1_library") is not None