feat: Add full typing safety with mypy checks (#1662)

* feat: Add complete typing support with mypy

Author: bhirsz

.pre-commit-config.yaml

@@ -4,3 +4,23 @@ repos:
     hooks:
       - id: ruff
       - id: ruff-format
+
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v1.19.1
+    hooks:
+      - id: mypy
+        args: ["--config-file=pyproject.toml", "src/robocop"]
+        pass_filenames: false
+        additional_dependencies:
+          - typer-slim>=0.12.5
+          - jinja2>=3.1.4
+          - rich>=10.11.0
+          - tomli>=2.0.0
+          - tomli-w>=1.0
+          - pathspec>=0.12
+          - platformdirs>=4.3
+          - pytz>=2022.7
+          - types-jinja2
+          - types-pytz
+          - fastmcp>=2.13.0
+          - msgpack-types

src/robocop/cache.py

@@ -8,6 +8,7 @@
 from __future__ import annotations
 
 from dataclasses import dataclass, field
+from functools import cached_property
 from typing import TYPE_CHECKING, Any
 
 import msgpack
@@ -259,39 +260,33 @@ def __init__(
         self.enabled = enabled
         self.cache_dir = cache_dir
         self.verbose = verbose
-        self._data: CacheData | None = None
         self._dirty = False
         self._path_cache: dict[Path, str] = {}  # Instance-bound path normalization cache
 
-    @property
+    @cached_property
     def data(self) -> CacheData:
         """Get cache data, loading from disk if needed."""
-        if self._data is None:
-            self._load()
-        return self._data
+        return self._load()
 
-    def _load(self) -> None:
+    def _load(self) -> CacheData:
         """Load cache from disk."""
         # Handle missing cache directory (first run)
         if not self.cache_dir.exists():
-            self._data = CacheData()
-            return
+            return CacheData()
 
         cache_file = self.cache_dir / defaults.CACHE_FILE_NAME
 
         if not cache_file.is_file():
-            self._data = CacheData()
-            return
+            return CacheData()
 
         try:
             raw_data = msgpack.unpackb(cache_file.read_bytes(), raw=False, strict_map_key=False)
 
             # Invalidate if version changed
             if raw_data.get("robocop_version") != __version__:
-                self._data = CacheData()
-                return
+                return CacheData()
 
-            self._data = CacheData.from_dict(raw_data)
+            return CacheData.from_dict(raw_data)
         except (
             msgpack.exceptions.UnpackException,
             msgpack.exceptions.ExtraData,
@@ -300,11 +295,11 @@ def _load(self) -> None:
             OSError,
         ):
             # Corrupted cache - start fresh
-            self._data = CacheData()
+            return CacheData()
 
     def save(self) -> None:
         """Save cache to disk if modified."""
-        should_skip = not self.enabled or not self._dirty or self._data is None
+        should_skip = not self.enabled or not self._dirty
         if should_skip:
             return
 
@@ -313,7 +308,7 @@ def save(self) -> None:
         cache_file = self.cache_dir / defaults.CACHE_FILE_NAME
 
         try:
-            cache_file.write_bytes(msgpack.packb(self._data.to_dict(), use_bin_type=True))
+            cache_file.write_bytes(msgpack.packb(self.data.to_dict(), use_bin_type=True))
             self._dirty = False
         except OSError as err:
             if self.verbose:
@@ -331,7 +326,7 @@ def _create_gitignore(self) -> None:
 
     def invalidate_all(self) -> None:
         """Clear the entire cache."""
-        self._data = CacheData()
+        self.data = CacheData()
         self._dirty = True
 
     def _normalize_path(self, path: Path) -> str:

src/robocop/config/builder.py

@@ -58,7 +58,7 @@ def resolve(
     if file is not None:
         value = getattr(file, attr, None)
         if value is not None:
-            return value
+            return value  # type: ignore[no-any-return]
 
     return default
 
@@ -67,7 +67,7 @@ def merge_lists(
     file: object | None,
     cli: object | None,
     attr: str,
-) -> list:
+) -> list[str]:
     """
     Resolve and merge configuration values.
 

src/robocop/config/manager.py

@@ -120,14 +120,16 @@ def __init__(
         self.overwrite_config = overwrite_config
         self.ignore_git_dir = ignore_git_dir
         self.ignore_file_config = ignore_file_config
-        self.force_exclude = force_exclude
         self.skip_gitignore = skip_gitignore
         self.gitignore_resolver = GitIgnoreResolver()
         self.overridden_config = config is not None
         self.root = root or Path.cwd()
-        self.sources = sources
-        self.default_config: Config = self.get_default_config(config)
-        self._paths: dict[Path, SourceFile] | None = None
+        self.default_config: Config = self.get_default_config(config, sources)
+        self.sources = sources if sources else self.default_config.sources
+        # ignore file filters on paths passed directly
+        self.ignore_file_filters = not force_exclude and bool(sources)
+        self._paths: dict[Path, SourceFile] = {}
+        self.resolved_paths = False
         self._cache: RobocopCache | None = None
 
     @property
@@ -145,23 +147,24 @@ def cache(self) -> RobocopCache:
     @property
     def paths(self) -> Generator[SourceFile, None, None]:
         # TODO: what if we provide the same path twice - tests
-        if self._paths is None:
-            self._paths = {}
-            sources: list[str | Path] = list(self.sources) if self.sources else list(self.default_config.sources)
-            ignore_file_filters = not self.force_exclude and bool(sources)
-            self.resolve_paths(sources, ignore_file_filters=ignore_file_filters)
+        if not self.resolved_paths:
+            self.resolve_paths(self.sources, ignore_file_filters=self.ignore_file_filters)
         yield from self._paths.values()
 
-    def get_default_config(self, config_path: Path | None) -> Config:
+    def get_default_config(self, config_path: Path | None, sources: list[str] | None) -> Config:
         """Get the default config either from --config option or from the cli."""
         if config_path:
             configuration = read_toml_config(config_path)
             if configuration is not None:  # TODO: should raise
                 config_file_raw = RawConfig.from_dict(configuration, config_path.resolve())
                 return self.config_builder.from_raw(self.overwrite_config, config_file_raw)
         if not self.ignore_file_config:
-            sources = [Path(path).resolve() for path in self.sources] if self.sources else [Path.cwd()]
-            directories = files.get_common_parent_dirs(sources)
+            if sources:
+                sources_paths: list[Path] = [Path(path) for path in sources]
+                paths = [path.resolve() for path in sources_paths if not path.is_symlink()]
+            else:
+                paths = [Path.cwd().resolve()]
+            directories = files.get_common_parent_dirs(paths)
             return self.find_config_in_dirs(directories, default=None)
         return self.config_builder.from_raw(self.overwrite_config, None)
 
@@ -231,7 +234,7 @@ def get_config_for_source_file(self, source_file: Path) -> Config:
 
     def resolve_paths(
         self,
-        sources: list[str | Path] | Generator[Path, None, None],
+        sources: list[Path] | list[str],
         ignore_file_filters: bool = False,
     ) -> None:
         """
@@ -266,6 +269,6 @@ def resolve_paths(
                     if self.gitignore_resolver.path_excluded(source_not_resolved, source_gitignore):
                         continue
             if source.is_dir():
-                self.resolve_paths(source.iterdir())
+                self.resolve_paths(list(source.iterdir()))
             elif source.is_file():
                 self._paths[source] = SourceFile(path=source, config=config)

src/robocop/linter/fix.py

@@ -25,6 +25,7 @@ class TextEditKind(Enum):
     """
 
     REPLACEMENT = "replace"
+    REPLACEMENT_LINES = "replace_lines"
     INSERTION = "insert"
     DELETION = "delete"
 
@@ -54,18 +55,11 @@ class TextEdit:
     rule_id: str
     rule_name: str
     start_line: int
-    start_col: int | None
-    end_line: int | None
-    end_col: int | None
-    replacement: str | None
-
-    @property
-    def kind(self) -> TextEditKind:
-        if self.replacement is None:
-            return TextEditKind.DELETION
-        if self.end_line is None:
-            return TextEditKind.INSERTION
-        return TextEditKind.REPLACEMENT
+    start_col: int
+    end_line: int
+    end_col: int
+    replacement: str
+    kind: TextEditKind = TextEditKind.REPLACEMENT
 
     @classmethod
     def replace_at_range(cls, rule_id: str, rule_name: str, diag_range: Range, replacement: str) -> TextEdit:
@@ -77,6 +71,7 @@ def replace_at_range(cls, rule_id: str, rule_name: str, diag_range: Range, repla
             end_line=diag_range.end.line,
             end_col=diag_range.end.character,
             replacement=replacement,
+            kind=TextEditKind.REPLACEMENT,
         )
 
     @classmethod
@@ -86,10 +81,11 @@ def replace_lines(cls, rule_id: str, rule_name: str, start_line: int, end_line:
             rule_id=rule_id,
             rule_name=rule_name,
             start_line=start_line,
-            start_col=None,
+            start_col=1,
             end_line=end_line,
-            end_col=None,
+            end_col=1,
             replacement=replacement,
+            kind=TextEditKind.REPLACEMENT_LINES,
         )
 
     @classmethod
@@ -99,10 +95,11 @@ def remove_at_range(cls, rule_id: str, rule_name: str, diag_range: Range) -> Tex
             rule_id=rule_id,
             rule_name=rule_name,
             start_line=diag_range.start.line,
-            start_col=None,
+            start_col=1,
             end_line=diag_range.end.line,
-            end_col=None,
-            replacement=None,
+            end_col=1,
+            replacement="",
+            kind=TextEditKind.DELETION,
         )
 
     @classmethod
@@ -112,10 +109,11 @@ def insert_at_range(cls, rule_id: str, rule_name: str, diag_range: Range, replac
             rule_id=rule_id,
             rule_name=rule_name,
             start_line=diag_range.start.line,
-            start_col=None,
-            end_line=None,
-            end_col=None,
+            start_col=1,
+            end_line=1,
+            end_col=1,
             replacement=replacement,
+            kind=TextEditKind.INSERTION,
         )
 
 
@@ -288,17 +286,17 @@ def _apply_edit(lines: list[str], edit: TextEdit) -> None:
 
         """
         # TODO: different kind of edits should have different apply_edit
-        if edit.kind == TextEditKind.REPLACEMENT:
-            if edit.end_line > len(lines) or edit.start_line < 1:
-                return
-            start_line_idx = edit.start_line - 1
-            end_line_idx = edit.end_line - 1
-            if edit.start_col is None or edit.end_col is None:  # replace_lines
-                lines[start_line_idx : end_line_idx + 1] = edit.replacement.splitlines(keepends=True)
-                return
+        if edit.end_line > len(lines) or edit.start_line < 1:
+            return
+
+        start_line_idx = edit.start_line - 1
+        end_line_idx = edit.end_line - 1
+
+        if edit.kind == TextEditKind.REPLACEMENT_LINES:
+            lines[start_line_idx : end_line_idx + 1] = edit.replacement.splitlines(keepends=True)
+        elif edit.kind == TextEditKind.REPLACEMENT:
             start_col_idx = edit.start_col - 1
             end_col_idx = edit.end_col - 1
-
             if start_line_idx == end_line_idx:  # single line
                 line = lines[edit.start_line - 1]
                 new_line = line[:start_col_idx] + edit.replacement + line[end_col_idx:]
@@ -307,8 +305,6 @@ def _apply_edit(lines: list[str], edit: TextEdit) -> None:
                 # When edit is multiline, we replace the lines fully
                 lines[start_line_idx : end_line_idx + 2] = edit.replacement.splitlines(keepends=True)
         elif edit.kind == TextEditKind.INSERTION:
-            start_line_idx = edit.start_line - 1
             lines[start_line_idx:start_line_idx] = edit.replacement.splitlines(keepends=True)
-        else:  # edit.kind == TextEditKind.DELETION
-            start_line_idx = edit.start_line - 1
+        elif edit.kind == TextEditKind.DELETION:
             del lines[start_line_idx : edit.end_line]

src/robocop/linter/rules/__init__.py

@@ -455,9 +455,9 @@ def configure(self, param: str, value: str) -> None:
                 f"    {configurables_text}"
             )
         self.config[param].value = value
-        # If you want to use the same parameter name as Rule attribute (for example severity), you need to skip getattr
         if param == "severity":
-            self.severity = self.config[param].value
+            # To use parameter also as rule attribute (for example severity), you need to skip getattr
+            self.severity = RuleSeverity.parser(value)
 
     def available_configurables(self, include_severity: bool = True) -> tuple[int, str]:
         params = []

src/robocop/runtime/resolver.py

@@ -114,24 +114,23 @@ def is_rule(rule_class_def: tuple[str, type]) -> bool:
 
 
 def can_run_in_robot_version(formatter: Formatter, overwritten: bool, target_version: int) -> bool:
-    if getattr(formatter, "MIN_VERSION", None) is None:
-        return True
-    if target_version >= formatter.MIN_VERSION:
+    min_version = getattr(formatter, "MIN_VERSION", None)
+    if not min_version or target_version >= min_version:
         return True
     if overwritten:
         # --select FormatterDisabledInVersion or --configure FormatterDisabledInVersion.enabled=True
         if target_version == ROBOT_VERSION.major:
             click.echo(
-                f"{formatter.__class__.__name__} formatter requires Robot Framework {formatter.MIN_VERSION}.* "
+                f"{formatter.__class__.__name__} formatter requires Robot Framework {min_version}.* "
                 f"version but you have {ROBOT_VERSION} installed. "
                 f"Upgrade installed Robot Framework if you want to use this formatter.",
                 err=True,
             )
         else:
             click.echo(
-                f"{formatter.__class__.__name__} formatter requires Robot Framework {formatter.MIN_VERSION}.* "
+                f"{formatter.__class__.__name__} formatter requires Robot Framework {min_version}.* "
                 f"version but you set --target-version rf{target_version}. "
-                f"Set --target-version to {formatter.MIN_VERSION} or do not forcefully enable this formatter "
+                f"Set --target-version to {min_version} or do not forcefully enable this formatter "
                 f"with --select / enable parameter.",
                 err=True,
             )

src/robocop/source_file.py

@@ -111,7 +111,7 @@ def _read_lines(self) -> list[str]:
     def _load_model(self, path_or_text: Path | str) -> File:
         """Determine the correct model loader based on the file type and loads it."""
         if "__init__" in self.path.name:
-            loader: Callable = get_init_model
+            loader: Callable[..., File] = get_init_model
         elif self.path.suffix == ".resource":
             loader = get_resource_model
         else:

tests/cache/test_cache.py

@@ -850,8 +850,6 @@ def test_cache_data_lazy_loading(self, tmp_path: Path):
         )
 
         cache = RobocopCache(cache_dir=cache_dir, enabled=True, verbose=False)
-        # Data should not be loaded yet
-        assert cache._data is None  # noqa: SLF001
 
         # Accessing .data triggers load
         data = cache.data