Korbit review comments

Author: spoorcc

dfetch/manifest/manifest.py

@@ -23,6 +23,7 @@
 import os
 import pathlib
 import re
+from dataclasses import dataclass
 from typing import IO, Any, Dict, List, Optional, Sequence, Tuple, Union
 
 import yaml
@@ -38,6 +39,15 @@
 logger = get_logger(__name__)
 
 
+@dataclass
+class ManifestEntryLocation:
+    """Location of an entry in the manifest file."""
+
+    line_number: int
+    start: int
+    end: int
+
+
 class RequestedProjectNotFoundError(RuntimeError):
     """Exception if items are not found in list of possibilities."""
 
@@ -111,8 +121,8 @@ def __init__(
     ) -> None:
         """Create the manifest."""
         self.__version: str = str(manifest.get("version", self.CURRENT_VERSION))
-        self.__text: str = str(text)
-        self.__path: str = str(path)
+        self.__text: str = text if text else ""
+        self.__path: str = str(path) if path else ""
 
         self._remotes, default_remotes = self._determine_remotes(
             manifest.get("remotes", [])
@@ -193,6 +203,9 @@ def from_yaml(
         path: Optional[Union[str, os.PathLike[str]]] = None,
     ) -> "Manifest":
         """Create a manifest from a file like object."""
+        if isinstance(text, (io.TextIOWrapper, IO)):
+            text = text.read()
+
         loaded_yaml = Manifest._load_yaml(text)
 
         if not loaded_yaml:
@@ -203,10 +216,6 @@ def from_yaml(
         if not manifest:
             raise RuntimeError("Missing manifest root element!")
 
-        if isinstance(text, (io.TextIOWrapper, IO)):
-            text.seek(0)
-            text = text.read()
-
         return Manifest(manifest, text=text, path=path)
 
     @staticmethod
@@ -313,24 +322,29 @@ def dump(self, path: str) -> None:
                 self._as_dict(), manifest_file, Dumper=ManifestDumper, sort_keys=False
             )
 
-    def find_name_in_manifest(self, name: str) -> Tuple[int, int, int]:
-        """Find the location of a project name in the manifest."""
+    def find_name_in_manifest(self, name: str) -> ManifestEntryLocation:
+        """Find the location of a project name in the manifest.
+
+        Returns:
+            ManifestEntryLocation of the project name in the manifest.
+
+        Raises:
+            FileNotFoundError: If manifest text is not available
+            RuntimeError: If the project name is not found in the manifest
+        """
         if not self.__text:
             raise FileNotFoundError("No manifest text available")
 
         for line_nr, line in enumerate(self.__text.splitlines(), start=1):
-            match = re.search(rf"^\s+-\s*name:\s*(?P<name>{name})\s*$", line)
+            match = re.search(rf"^\s+-\s*name:\s*(?P<name>{re.escape(name)})\s*$", line)
 
             if match:
-                return (
-                    line_nr,
-                    int(match.start("name")) + 1,
-                    int(match.end("name")),
+                return ManifestEntryLocation(
+                    line_number=line_nr,
+                    start=int(match.start("name")) + 1,
+                    end=int(match.end("name")),
                 )
-        raise RuntimeError(
-            "An entry from the manifest was provided,"
-            " that doesn't exist in the manifest!"
-        )
+        raise RuntimeError(f"{name} was not found in the manifest!")
 
 
 def find_manifest() -> str:

dfetch/reporting/check/code_climate_reporter.py

@@ -112,7 +112,7 @@ def add_issue(self, project: ProjectEntry, issue: Issue) -> None:
             project (ProjectEntry): Project with the issue
             issue (Issue): The issue to add to the report
         """
-        line, col_start, col_end = self._manifest.find_name_in_manifest(project.name)
+        location = self._manifest.find_name_in_manifest(project.name)
 
         self._report += [
             {
@@ -126,8 +126,11 @@ def add_issue(self, project: ProjectEntry, issue: Issue) -> None:
                 "location": {
                     "path": os.path.relpath(self._manifest.path),
                     "positions": {
-                        "begin": {"line": line, "column": col_start},
-                        "end": {"line": line, "column": col_end},
+                        "begin": {
+                            "line": location.line_number,
+                            "column": location.start,
+                        },
+                        "end": {"line": location.line_number, "column": location.end},
                     },
                 },
             }

dfetch/reporting/check/jenkins_reporter.py

@@ -90,17 +90,17 @@ def add_issue(self, project: ProjectEntry, issue: Issue) -> None:
             project (ProjectEntry): Project with the issue
             issue (Issue): The issue to add to the report
         """
-        line, col_start, col_end = self._manifest.find_name_in_manifest(project.name)
+        location = self._manifest.find_name_in_manifest(project.name)
         self._report["issues"] += [
             {
                 "fileName": os.path.relpath(self._manifest.path),
                 "severity": str(issue.severity.value),
                 "message": f"{project.name} : {issue.message}",
                 "description": issue.description,
-                "lineStart": line,
-                "lineEnd": line,
-                "columnStart": col_start,
-                "columnEnd": col_end,
+                "lineStart": location.line_number,
+                "lineEnd": location.line_number,
+                "columnStart": location.start,
+                "columnEnd": location.end,
             }
         ]
 

dfetch/reporting/check/sarif_reporter.py

@@ -177,7 +177,7 @@ def add_issue(self, project: ProjectEntry, issue: Issue) -> None:
             project (ProjectEntry): Project with the issue
             issue (Issue): The issue to add
         """
-        line, col_start, col_end = self._manifest.find_name_in_manifest(project.name)
+        location = self._manifest.find_name_in_manifest(project.name)
 
         result = Result(
             message=Message(text=f"{project.name} : {issue.message}"),
@@ -190,10 +190,10 @@ def add_issue(self, project: ProjectEntry, issue: Issue) -> None:
                             uri=os.path.relpath(self._manifest.path), index=0
                         ),
                         region=Region(
-                            start_line=line,
-                            start_column=col_start,
-                            end_line=line,
-                            end_column=col_end + 1,
+                            start_line=location.line_number,
+                            start_column=location.start,
+                            end_line=location.line_number,
+                            end_column=location.end + 1,
                         ),
                     )
                 )

dfetch/reporting/reporter.py

@@ -13,13 +13,13 @@ class Reporter(ABC):
 
     name: str = "abstract"
 
+    @abstractmethod
     def __init__(self, manifest: Manifest) -> None:
         """Create the reporter.
 
         Args:
             manifest (Manifest): The manifest to report on
         """
-        self._manifest = manifest
 
     @abstractmethod
     def add_project(

dfetch/reporting/sbom_reporter.py

@@ -102,7 +102,7 @@ class SbomReporter(Reporter):
 
     def __init__(self, manifest: Manifest) -> None:
         """Start the report."""
-        super().__init__(manifest)
+        self._manifest = manifest
         self._bom = Bom()
         self._bom.metadata.tools.components.add(self.dfetch_tool)
         self._bom.metadata.tools.components.add(cdx_lib_component())
@@ -120,7 +120,7 @@ def add_project(
 
         name = project.name if purl.type == "generic" else purl.name
 
-        line_nr, start, _ = self._manifest.find_name_in_manifest(project.name)
+        location = self._manifest.find_name_in_manifest(project.name)
 
         component = Component(
             name=name,
@@ -130,7 +130,11 @@ def add_project(
             purl=purl,
             evidence=ComponentEvidence(
                 occurrences=[
-                    Occurrence(location=self._manifest.path, line=line_nr, offset=start)
+                    Occurrence(
+                        location=self._manifest.path,
+                        line=location.line_number,
+                        offset=location.start,
+                    )
                 ],
                 identity=[
                     Identity(

dfetch/reporting/stdout_reporter.py

@@ -7,6 +7,7 @@
 from typing import List
 
 from dfetch.log import get_logger
+from dfetch.manifest.manifest import Manifest
 from dfetch.manifest.project import ProjectEntry
 from dfetch.project.metadata import Metadata
 from dfetch.reporting.reporter import Reporter
@@ -20,6 +21,10 @@ class StdoutReporter(Reporter):
 
     name = "stdout"
 
+    def __init__(self, manifest: Manifest) -> None:
+        """Initialize the reporter."""
+        del manifest
+
     def add_project(
         self,
         project: ProjectEntry,

dfetch/util/license.py

@@ -7,21 +7,36 @@
 import infer_license
 from infer_license.types import License as InferredLicense
 
+# Limit the max size of alicense file to parse
+MAX_LICENSE_FILE_SIZE = 1024 * 1024
+
 
 @dataclass
 class License:
-    """Class to hold license information."""
+    """Represents a software license with its SPDX identifiers and detection confidence.
+
+    This class encapsulates license information detected by the infer-license library,
+    providing standardized identifiers and confidence level of the detection.
+    """
 
-    name: str  # SPDX Full name
-    spdx_id: str  # SPDX Identifier
-    trove_classifier: Optional[str]  # Python package classifier
-    probability: float  # Confidence level of the license inference
+    name: str  #: SPDX Full name
+    spdx_id: str  #: SPDX Identifier
+    trove_classifier: Optional[str]  #: Python package classifier
+    probability: float  #: Confidence level of the license inference
 
     @staticmethod
     def from_inferred(
         inferred_license: InferredLicense, probability: float
     ) -> "License":
-        """Create License from an InferredLicense."""
+        """Convert an infer-license License object to our internal License representation.
+
+        Args:
+            inferred_license: The license object from infer-license library
+            probability: The confidence score (0-1) of the license detection
+
+        Returns:
+            License: A new License instance with the inferred information
+        """
         return License(
             name=inferred_license.name,
             spdx_id=inferred_license.shortname,
@@ -43,7 +58,7 @@ def guess_license_in_file(
     """
     try:
         with open(filename, "rb") as f:
-            file_bytes = f.read()
+            file_bytes = f.read(MAX_LICENSE_FILE_SIZE)
         try:
             license_text = file_bytes.decode("utf-8")
         except UnicodeDecodeError: