feat: code audit for bloat and quality in language analyzer

secureli/actions/action.py

@@ -154,7 +154,7 @@ def _install_secureli(
             install_languages, always_yes
         )
         language_config_result = (
-            self.action_deps.language_support._build_pre_commit_config(
+            self.action_deps.language_support.build_pre_commit_config(
                 install_languages, lint_languages
             )
         )

secureli/modules/language_analyzer/language_support.py

@@ -1,36 +1,16 @@
 from pathlib import Path
-from typing import Callable, Iterable, Optional, Any
+from typing import Callable, Iterable, Optional
 
-import pydantic
 import yaml
 from secureli.modules.shared.models.config import HookConfiguration, LinterConfig, Repo
-from secureli.modules.shared.models.language import LanguageMetadata
+from secureli.modules.shared.models import language
 
 import secureli.repositories.secureli_config as SecureliConfig
 from secureli.modules.shared.abstractions.pre_commit import PreCommitAbstraction
 from secureli.modules.language_analyzer import git_ignore, language_config
 from secureli.modules.shared.utilities import hash_config
 
 
-class BuildConfigResult(pydantic.BaseModel):
-    """Result about building config for all laguages"""
-
-    successful: bool
-    languages_added: list[str]
-    config_data: dict
-    linter_configs: list[LinterConfig]
-    version: str
-
-
-class LinterConfigWriteResult(pydantic.BaseModel):
-    """
-    Result from writing linter config files
-    """
-
-    successful_languages: list[str]
-    error_messages: list[str]
-
-
 class LanguageSupportService:
     """
     Orchestrates a growing list of security best practices for languages. Installs
@@ -52,9 +32,9 @@ def __init__(
     def apply_support(
         self,
         languages: list[str],
-        language_config_result: BuildConfigResult,
+        language_config_result: language.BuildConfigResult,
         overwrite_pre_commit: bool,
-    ) -> LanguageMetadata:
+    ) -> language.LanguageMetadata:
         """
         Applies Secure Build support for the provided languages
         :param languages: list of languages to provide support for
@@ -85,7 +65,7 @@ def apply_support(
         # Add .secureli/ to the gitignore folder if needed
         self.git_ignore.ignore_secureli_files()
 
-        return LanguageMetadata(
+        return language.LanguageMetadata(
             version=language_config_result.version,
             security_hook_id=self.secret_detection_hook_id(languages),
             linter_config_write_errors=linter_config_write_result.error_messages,
@@ -100,7 +80,7 @@ def secret_detection_hook_id(self, languages: list[str]) -> Optional[str]:
         :return: The hook ID to use for secrets analysis if supported, otherwise None.
         """
         # lint_languages param can be an empty set since we only need secrets detection hooks
-        language_config = self._build_pre_commit_config(languages, [])
+        language_config = self.build_pre_commit_config(languages, [])
         config = language_config.config_data
         secrets_detecting_repos_data = self.data_loader(
             "pre-commit/secrets_detecting_repos.yaml"
@@ -143,21 +123,14 @@ def get_configuration(self, languages: list[str]) -> HookConfiguration:
         :param languages: list of languages to get config for.
         :return: A serializable Configuration model
         """
-        config = self._build_pre_commit_config(languages, set(languages)).config_data
+        config = self.build_pre_commit_config(languages, set(languages)).config_data
 
-        def create_repo(raw_repo: dict) -> Repo:
-            return Repo(
-                repo=raw_repo.get("repo", "unknown"),
-                revision=raw_repo.get("rev", "unknown"),
-                hooks=[hook.get("id", "unknown") for hook in raw_repo.get("hooks", [])],
-            )
-
-        repos = [create_repo(raw_repo) for raw_repo in config.get("repos", [])]
+        repos = [self._create_repo(raw_repo) for raw_repo in config.get("repos", [])]
         return HookConfiguration(repos=repos)
 
-    def _build_pre_commit_config(
+    def build_pre_commit_config(
         self, languages: list[str], lint_languages: Iterable[str]
-    ) -> BuildConfigResult:
+    ) -> language.BuildConfigResult:
         """
         Builds the final .pre-commit-config.yaml from all supported repo languages. Also returns any and all
         linter configuration data.
@@ -171,15 +144,17 @@ def _build_pre_commit_config(
         config_languages = [*languages, "base"]
         config_lint_languages = [*lint_languages, "base"]
 
-        for language in config_languages:
-            include_linter = language in config_lint_languages
-            result = self.language_config.get_language_config(language, include_linter)
+        for config_language in config_languages:
+            include_linter = config_language in config_lint_languages
+            result = self.language_config.get_language_config(
+                config_language, include_linter
+            )
             if result.config_data:
-                successful_languages.append(language)
+                successful_languages.append(config_language)
                 (
                     linter_configs.append(
                         LinterConfig(
-                            language=language,
+                            language=config_language,
                             linter_data=result.linter_config.linter_data,
                         )
                     )
@@ -192,18 +167,30 @@ def _build_pre_commit_config(
         config = {"repos": config_data}
         version = hash_config(yaml.dump(config))
 
-        return BuildConfigResult(
+        return language.BuildConfigResult(
             successful=True if len(config_data) > 0 else False,
             languages_added=successful_languages,
             config_data=config,
             version=version,
             linter_configs=linter_configs,
         )
 
+    def _create_repo(self, raw_repo: dict) -> Repo:
+        """
+        Creates a repository containing pre-commit hooks from a raw dictionary object
+        :param raw_repo: dictionary containing repository data.
+        :return: repository containing pre-commit hooks
+        """
+        return Repo(
+            repo=raw_repo.get("repo", "unknown"),
+            revision=raw_repo.get("rev", "unknown"),
+            hooks=[hook.get("id", "unknown") for hook in raw_repo.get("hooks", [])],
+        )
+
     def _write_pre_commit_configs(
         self,
         all_linter_configs: list[LinterConfig],
-    ) -> LinterConfigWriteResult:
+    ) -> language.LinterConfigWriteResult:
         """
         Install any config files for given language to support any pre-commit commands.
         i.e. Javascript ESLint requires a .eslintrc file to sufficiently use plugins and allow
@@ -220,16 +207,16 @@ def _write_pre_commit_configs(
         error_messages: list[str] = []
         successful_languages: list[str] = []
 
-        for config, language in linter_config_data:
+        for config, config_language in linter_config_data:
             try:
                 with open(Path(SecureliConfig.FOLDER_PATH / config.filename), "w") as f:
                     f.write(yaml.dump(config.settings))
-                    successful_languages.append(language)
+                    successful_languages.append(config_language)
             except:
                 error_messages.append(
-                    f"Failed to write {config.filename} linter config file for {language}"
+                    f"Failed to write {config.filename} linter config file for {config_language}"
                 )
 
-        return LinterConfigWriteResult(
+        return language.LinterConfigWriteResult(
             successful_languages=successful_languages, error_messages=error_messages
         )

secureli/modules/shared/models/language.py

@@ -51,3 +51,22 @@ class LanguageMetadata(pydantic.BaseModel):
     version: str
     security_hook_id: Optional[str]
     linter_config_write_errors: Optional[list[str]] = []
+
+
+class BuildConfigResult(pydantic.BaseModel):
+    """Result about building config for all laguages"""
+
+    successful: bool
+    languages_added: list[str]
+    config_data: dict
+    linter_configs: list[LinterConfig]
+    version: str
+
+
+class LinterConfigWriteResult(pydantic.BaseModel):
+    """
+    Result from writing linter config files
+    """
+
+    successful_languages: list[str]
+    error_messages: list[str]

tests/modules/language_analyzer/test_language_support.py

@@ -259,7 +259,7 @@ def mock_loader_side_effect(resource):
     languages = ["RadLang"]
     lint_languages = [*languages]
 
-    build_config_result = language_support_service._build_pre_commit_config(
+    build_config_result = language_support_service.build_pre_commit_config(
         languages, lint_languages
     )
 
@@ -293,7 +293,7 @@ def test_that_language_support_throws_exception_when_language_config_file_cannot
     languages = ["RadLang"]
     lint_languages = [*languages]
 
-    build_config_result = language_support_service._build_pre_commit_config(
+    build_config_result = language_support_service.build_pre_commit_config(
         languages, lint_languages
     )
 
@@ -325,7 +325,7 @@ def test_that_language_support_handles_invalid_language_config(
     languages = ["RadLang"]
     lint_languages = [*languages]
 
-    build_config_result = language_support_service._build_pre_commit_config(
+    build_config_result = language_support_service.build_pre_commit_config(
         languages, lint_languages
     )
 
@@ -357,7 +357,7 @@ def test_that_language_support_handles_empty_repos_list(
     languages = ["RadLang"]
     lint_languages = [*languages]
 
-    build_config_result = language_support_service._build_pre_commit_config(
+    build_config_result = language_support_service.build_pre_commit_config(
         languages, lint_languages
     )
 
@@ -422,7 +422,7 @@ def test_write_pre_commit_configs_returns_error_messages(
 
     mock_open.assert_called_once()
     mock_open.return_value.write.assert_not_called()
-    assert result == language_support.LinterConfigWriteResult(
+    assert result == language.LinterConfigWriteResult(
         error_messages=[
             f"Failed to write {mock_filename} linter config file for {mock_language}"
         ],
@@ -438,7 +438,7 @@ def test_write_pre_commit_configs_handles_empty_lint_configs(
 
     mock_open.assert_not_called()
     mock_open.return_value.write.assert_not_called()
-    assert result == language_support.LinterConfigWriteResult(
+    assert result == language.LinterConfigWriteResult(
         error_messages=[],
         successful_languages=[],
     )