Add missing reserved directory names (#560)

Marinovsky · web-flow · commit 80e83c886f4d · 2025-03-06T13:18:03.000-03:00
* Handle reserved and invalid names

* try fix bugs

* Fix failing tests

* Address requested changes

* Nit changes
diff --git a/lean/commands/library/add.py b/lean/commands/library/add.py
@@ -296,6 +296,8 @@ def add(project: Path, name: str, version: Optional[str], no_local: bool) -> Non
     $ lean library add "My Python Project" tensorflow --version 2.5.0
     $ lean library add "My Python Project" "Library/My Python Library"
     """
+    from lean.components import reserved_names
+
     logger = container.logger
     project_config = container.project_config_manager.get_project_config(project)
     project_language = project_config.get("algorithm-language", None)
@@ -305,6 +307,12 @@ def add(project: Path, name: str, version: Optional[str], no_local: bool) -> Non
                             "https://www.lean.io/docs/v2/lean-cli/projects/project-management#02-Create-Projects")
 
     library_manager = container.library_manager
+
+    if not container.path_manager.is_path_valid(Path(name)):
+        raise MoreInfoError(
+            f"Invalid path name. Can only contain letters, numbers & spaces. Can not start with empty char ' ' or be a reserved name [ {', '.join(reserved_names)} ]",
+            "https://www.lean.io/docs/v2/lean-cli/key-concepts/troubleshooting#02-Common-Errors")
+
     library_dir = Path(name).expanduser().resolve()
 
     if library_manager.is_lean_library(library_dir):
diff --git a/lean/components/__init__.py b/lean/components/__init__.py
@@ -12,7 +12,10 @@
 # limitations under the License.
 
 reserved_names = ["CON", "PRN", "AUX", "NUL",
-                    "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
-                    "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"]
+                  "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
+                  "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9",
+                  "bin", "obj", "__pycache__", ".ipynb_checkpoints", ".idea", ".vscode"]
+
+output_reserved_names = ["backtests", "live", "optimizations", "storage", "report"]
 
 forbidden_characters = ["\\", ":", "*", "?", '"', "<", ">", "|"]
diff --git a/lean/components/util/path_manager.py b/lean/components/util/path_manager.py
@@ -13,7 +13,7 @@
 
 from pathlib import Path
 
-from lean.components import reserved_names, forbidden_characters
+from lean.components import reserved_names, output_reserved_names, forbidden_characters
 from lean.components.config.lean_config_manager import LeanConfigManager
 from lean.components.util.platform_manager import PlatformManager
 
@@ -47,8 +47,8 @@ def is_name_valid(self, name: str) -> bool:
         :param name: the name to validate
         :return: True if the name is valid on Windows operating system, False if not
         """
-        import re
-        return re.match(r'^[-_a-zA-Z0-9/\s]*$', name) is not None
+        from re import match
+        return (match(r'^[-_a-zA-Z0-9/\s]*$', name) is not None) and (name not in reserved_names + output_reserved_names)
 
     def is_path_valid(self, path: Path) -> bool:
         """Returns whether the given path is a valid project path in the current operating system.
@@ -66,6 +66,9 @@ def is_path_valid(self, path: Path) -> bool:
         except OSError:
             return False
 
+        if path.name in output_reserved_names:
+            return False
+
         # On Windows path.exists() doesn't throw for paths like CON/file.txt
         # Trying to create them does raise errors, so we manually validate path components
         # We follow the rules of windows for every OS
@@ -75,7 +78,7 @@ def is_path_valid(self, path: Path) -> bool:
                 return False
 
             for reserved_name in reserved_names:
-                if component.upper() == reserved_name or component.upper().startswith(reserved_name + "."):
+                if component == reserved_name or component.upper() == reserved_name or component.upper().startswith(reserved_name + "."):
                     return False
 
             for forbidden_character in forbidden_characters:
diff --git a/lean/components/util/project_manager.py b/lean/components/util/project_manager.py
@@ -14,7 +14,7 @@
 from datetime import datetime
 from pathlib import Path
 from typing import List, Optional, Union, Tuple
-from lean.components import reserved_names
+from lean.components import reserved_names, output_reserved_names
 from lean.components.config.cli_config_manager import CLIConfigManager
 from lean.components.config.lean_config_manager import LeanConfigManager
 from lean.components.config.project_config_manager import ProjectConfigManager
@@ -134,7 +134,7 @@ def get_source_files(self, directory: Path) -> List[Path]:
 
         for obj in directory.iterdir():
             if obj.is_dir():
-                if (obj.name in ["bin", "obj", ".ipynb_checkpoints", "backtests", "live", "optimizations"] or
+                if (obj.name in reserved_names + output_reserved_names or
                         obj.name.startswith(".") or
                         # ignore python virtual environments
                         (obj / "pyvenv.cfg").is_file()):
diff --git a/tests/commands/cloud/test_backtest.py b/tests/commands/cloud/test_backtest.py
@@ -17,7 +17,6 @@
 from click.testing import CliRunner
 
 from lean.commands import lean
-from lean.container import container
 from lean.models.api import QCBacktest
 from tests.test_helpers import create_api_project, create_fake_lean_cli_directory
 from tests.conftest import initialize_container
diff --git a/tests/commands/cloud/test_optimize.py b/tests/commands/cloud/test_optimize.py
@@ -18,7 +18,6 @@
 
 from lean.commands import lean
 from lean.components.config.optimizer_config_manager import NodeType, OptimizerConfigManager
-from lean.container import container
 from lean.models.api import QCOptimization, QCOptimizationBacktest, QCOptimizationEstimate
 from lean.models.optimizer import (OptimizationConstraint, OptimizationExtremum, OptimizationParameter,
                                    OptimizationTarget)
diff --git a/tests/commands/library/test_add.py b/tests/commands/library/test_add.py
@@ -20,6 +20,7 @@
 from lean.container import container
 from lean.models.utils import LeanLibraryReference
 from tests.test_helpers import create_fake_lean_cli_directory, create_fake_lean_cli_directory_with_subdirectories
+from lean.components import reserved_names
 
 
 def _assert_library_reference_was_added_to_project_config_file(project_dir: Path, library_dir: Path) -> None:
@@ -123,3 +124,12 @@ def test_library_add_fails_when_library_directory_is_not_valid(library_dir: str)
 
     assert result.exit_code != 0
 
+@pytest.mark.parametrize("library_name", reserved_names)
+def test_library_add_fails_when_library_directory_is_not_valid(library_name) -> None:
+    create_fake_lean_cli_directory()
+
+    project_dir = Path("CSharp Project")
+    library_dir = Path(f"Library/{library_name}")
+    result = CliRunner().invoke(lean, ["library", "add", str(project_dir), str(library_dir), "--no-local"])
+    assert result.exit_code != 0
+    assert result.exception.args[0] == f"Invalid path name. Can only contain letters, numbers & spaces. Can not start with empty char ' ' or be a reserved name [ {', '.join(reserved_names)} ]"
diff --git a/tests/commands/test_backtest.py b/tests/commands/test_backtest.py
@@ -21,6 +21,7 @@
 
 from lean.commands import lean
 from lean.components.util.xml_manager import XMLManager
+from lean.components import reserved_names, output_reserved_names
 from lean.constants import DEFAULT_ENGINE_IMAGE
 from lean.container import container
 from lean.models.api import QCLanguage
@@ -95,6 +96,13 @@ def test_backtest_calls_lean_runner_with_custom_output_directory() -> None:
                                                            {},
                                                            {})
 
+@pytest.mark.parametrize("output_name", reserved_names + output_reserved_names)
+def test_backtest_fails_when_given_is_invalid(output_name: str) -> None:
+    create_fake_lean_cli_directory()
+
+    result = CliRunner().invoke(lean, ["backtest", "Python Project", "--output", f"Python Project/custom/{output_name}"])
+
+    assert result.output == f"Usage: lean backtest [OPTIONS] PROJECT\nTry 'lean backtest --help' for help.\n\nError: Invalid value for '--output': Directory 'Python Project/custom/{output_name}' is not a valid path.\n"
 
 def test_backtest_calls_lean_runner_with_release_mode() -> None:
     create_fake_lean_cli_directory()
diff --git a/tests/commands/test_live.py b/tests/commands/test_live.py
@@ -23,6 +23,7 @@
 from click.testing import CliRunner
 
 from lean.commands import lean
+from lean.components import reserved_names, output_reserved_names
 from lean.constants import DEFAULT_ENGINE_IMAGE
 from lean.container import container
 from lean.models.docker import DockerImage
@@ -1167,6 +1168,27 @@ def test_live_non_interactive_deploy_with_live_and_historical_provider_missed_li
 
     assert result.exit_code == 0
 
+@pytest.mark.parametrize("output_name", reserved_names + output_reserved_names)
+def test_live_non_interactive_deploy_fails_when_given_is_invalid(output_name: str) -> None:
+    create_fake_lean_cli_directory()
+    create_fake_environment("live-paper", True)
+
+    container.initialize(docker_manager=mock.Mock(), lean_runner=mock.Mock(), api_client = mock.MagicMock())
+
+    provider_live_option = ["--data-provider-live", "Polygon", "--polygon-api-key", "123"]
+
+    provider_history_option = ["--data-provider-historical", "Polygon"]
+
+    result = CliRunner().invoke(lean, ["live", "deploy", "--brokerage", "Paper Trading",
+                                       *provider_live_option,
+                                       *provider_history_option,
+                                       "Python Project",
+                                       "--output",
+                                       output_name
+                                       ])
+
+    assert result.output == f"Usage: lean live deploy [OPTIONS] PROJECT\nTry 'lean live deploy --help' for help.\n\nError: Invalid value for '--output': Directory '{output_name}' is not a valid path.\n"
+
 def test_live_non_interactive_deploy_with_real_brokerage_without_credentials() -> None:
     create_fake_lean_cli_directory()
     create_fake_environment("live-paper", True)
diff --git a/tests/commands/test_optimize.py b/tests/commands/test_optimize.py
@@ -21,6 +21,7 @@
 from lean.commands import lean
 from lean.components.cloud.module_manager import ModuleManager
 from lean.components.config.storage import Storage
+from lean.components import reserved_names, output_reserved_names
 from lean.constants import DEFAULT_ENGINE_IMAGE, LEAN_ROOT_PATH
 from lean.container import container
 from lean.models.docker import DockerImage
@@ -204,6 +205,23 @@ def test_optimize_creates_output_directory_when_not_existing_yet() -> None:
 
     assert (Path.cwd() / "output").is_dir()
 
+@pytest.mark.parametrize("output_directory", reserved_names + output_reserved_names)
+def test_optimize_fails_when_output_directory_is_invalid(output_directory: str) -> None:
+    create_fake_lean_cli_directory()
+
+    docker_manager = mock.Mock()
+    docker_manager.run_image.side_effect = run_image
+    container.initialize(docker_manager=docker_manager)
+    container.optimizer_config_manager = _get_optimizer_config_manager_mock()
+
+    Storage(str(Path.cwd() / "Python Project" / "config.json")).set("parameters", {"param1": "1"})
+
+    result = CliRunner().invoke(lean, ["optimize", "Python Project", "--output", f"Python Project/custom/{output_directory}"])
+
+    assert result.exit_code != 0
+
+    assert result.output == f"Usage: lean optimize [OPTIONS] PROJECT\nTry 'lean optimize --help' for help.\n\nError: Invalid value for '--output': Directory 'Python Project/custom/{output_directory}' is not a valid path.\n"
+
 
 def test_optimize_copies_code_to_output_directory() -> None:
     create_fake_lean_cli_directory()
diff --git a/tests/commands/test_report.py b/tests/commands/test_report.py
@@ -20,6 +20,7 @@
 
 from lean.commands import lean
 from lean.components.config.storage import Storage
+from lean.components import reserved_names, output_reserved_names
 from lean.constants import DEFAULT_ENGINE_IMAGE
 from lean.container import container
 from lean.models.docker import DockerImage
@@ -490,6 +491,19 @@ def test_report_writes_to_given_report_destination() -> None:
 
     assert (Path.cwd() / "path" / "to" / "report.html").is_file()
 
+@pytest.mark.parametrize("report_destination", reserved_names)
+def test_report_fails_when_given_report_destination_is_invalid(report_destination: str) -> None:
+    invalid_names = reserved_names + output_reserved_names
+    docker_manager = mock.Mock()
+    docker_manager.run_image.side_effect = run_image
+    initialize_container(docker_manager_to_use=docker_manager)
+
+    result = CliRunner().invoke(lean, ["report",
+                                       "--report-destination", f"{report_destination}/path/to/report.html"])
+
+    assert result.exit_code != 0
+    assert result.output == f"Usage: lean report [OPTIONS]\nTry 'lean report --help' for help.\n\nError: Invalid value for '--report-destination': File '{report_destination}/path/to/report.html' is not a valid path.\n"
+
 
 def test_report_aborts_when_report_destination_already_exists() -> None:
     docker_manager = mock.Mock()
diff --git a/tests/components/util/test_path_manager.py b/tests/components/util/test_path_manager.py
@@ -18,6 +18,7 @@
 import pytest
 
 from lean.components.config.lean_config_manager import LeanConfigManager
+from lean.components import reserved_names, output_reserved_names
 from lean.components.util.path_manager import PathManager
 from lean.components.util.platform_manager import PlatformManager
 
@@ -99,7 +100,7 @@ def test_is_path_valid_returns_true_for_valid_path() -> None:
                                         ("My Path./file.txt", False),
                                         (" My Path/file.txt", False),
                                         ("My Path/CON.txt", False),
-                                        ("My Path/CON.tmp.txt", False)])
+                                        ("My Path/CON.tmp.txt", False)] + list(map(lambda x: (f"MyPath/{x}/", False), output_reserved_names)))
 def test_is_path_valid_windows(path: str, valid: bool) -> None:
     if platform.system() != "Windows":
         pytest.skip("This test requires Windows")
@@ -117,7 +118,7 @@ def test_is_path_valid_windows(path: str, valid: bool) -> None:
                                          ("1 a/2_b/3-c", True),
                                          ("1a2b3c$", False),
                                          ("abc:123", False),
-                                         ("abc.def", False)])
+                                         ("abc.def", False)] + list(map(lambda x: (x, False), reserved_names + output_reserved_names)))
 def test_is_name_valid(name: str, valid: bool) -> None:
     path_manager = _get_path_manager()
 
diff --git a/tests/components/util/test_project_manager.py b/tests/components/util/test_project_manager.py
@@ -210,6 +210,32 @@ def test_copy_code_copies_source_files_to_output_directory() -> None:
         assert (output_dir / file).is_file()
         assert (output_dir / file).read_text(encoding="utf-8") == file.name
 
+def test_copy_code_copies_source_files_to_output_directory_() -> None:
+    from lean.components import reserved_names, output_reserved_names
+    project_path = Path.cwd() / "My Project"
+    project_path.mkdir()
+
+    valid_files = ["Main.cs", "main.py", "research.ipynb", "path/to/Alpha.cs", "path/to/alpha.py"]
+    invalid_files = [f"{x}/test.txt" for x in reserved_names + output_reserved_names]
+    files = valid_files + invalid_files
+    files_paths = [project_path / file for file in files]
+
+    output_dir = Path.cwd() / "code"
+
+    for file in files_paths:
+        file.parent.mkdir(parents=True, exist_ok=True)
+        file.touch()
+        file.write_text(file.name, encoding="utf-8")
+
+    project_manager = _create_project_manager()
+    project_manager.copy_code(project_path, output_dir)
+
+    for file in files:
+        if file in valid_files:
+            assert (output_dir / file).is_file()
+            assert (output_dir / file).read_text(encoding="utf-8") == (project_path / file).name
+        else:
+            assert not (output_dir / file).is_file()
 
 @pytest.mark.parametrize(
     "directory",
