extract_declared_dependencies: Move parser_choice into DepsSource

The parser choice was passed into both validate_deps_source() (to help
select which files become DepsSource objects), and into parse_source()
(to select the parser to be used for each DepsSource object). This is
redundant when we instead can record the parser_choice as part of the
DepsSource object.

Author: jherland

fawltydeps/extract_declared_dependencies.py

@@ -296,13 +296,11 @@ class ParsingStrategy(NamedTuple):
     execute: Callable[[Path], Iterator[DeclaredDependency]]
 
 
-def first_applicable_parser(
-    path: Path,
-) -> Optional[Tuple[ParserChoice, ParsingStrategy]]:
+def first_applicable_parser(path: Path) -> Optional[ParserChoice]:
     """Find the first applicable parser choice for given path."""
     return next(
         (
-            (choice, parser)
+            choice
             for choice, parser in PARSER_CHOICES.items()
             if parser.applies_to_path(path)
         ),
@@ -328,44 +326,31 @@ def first_applicable_parser(
 }
 
 
-def parse_source(
-    src: DepsSource, parser_choice: Optional[ParserChoice] = None
-) -> Iterator[DeclaredDependency]:
+def parse_source(src: DepsSource) -> Iterator[DeclaredDependency]:
     """Extract dependencies (package names) from supported file types.
 
-    Pass a path from which to discover and parse dependency declarations. Pass
-    a directory to traverse that directory tree to find and automatically parse
-    any supported files.
+    Pass a DepsSource objects which specifies the path to the file containing
+    the dependency declarations, as well as a parser choice to select the
+    parsing strategy for this file.
 
     Generate (i.e. yield) a DeclaredDependency object for each dependency found.
     There is no guaranteed ordering on the generated dependencies.
     """
-    assert src.path.is_file()  # sanity check
-    if parser_choice is not None:
-        parser = PARSER_CHOICES[parser_choice]
-        if not parser.applies_to_path(src.path):
-            logger.warning(
-                f"Manually applying parser '{parser_choice}' to dependencies: {src.path}"
-            )
-    else:
-        choice_and_parser = first_applicable_parser(src.path)
-        if choice_and_parser is None:  # nothing found. SHOULD NOT HAPPEN!
-            raise UnparseablePathException(
-                ctx="Parsing given dependencies path isn't supported", path=src.path
-            )
-        parser = choice_and_parser[1]
+    parser = PARSER_CHOICES[src.parser_choice]
+    if not parser.applies_to_path(src.path):
+        logger.warning(
+            f"Manually applying parser '{src.parser_choice}' to dependencies: {src.path}"
+        )
     yield from parser.execute(src.path)
 
 
-def parse_sources(
-    sources: Iterable[DepsSource], parser_choice: Optional[ParserChoice] = None
-) -> Iterator[DeclaredDependency]:
+def parse_sources(sources: Iterable[DepsSource]) -> Iterator[DeclaredDependency]:
     """Extract dependencies (package names) from supported file types.
 
     Pass sources from which to parse dependency declarations.
     """
     for source in sources:
-        yield from parse_source(source, parser_choice=parser_choice)
+        yield from parse_source(source)
 
 
 def validate_deps_source(
@@ -381,43 +366,30 @@ def validate_deps_source(
       parseable files within.
     - Raise UnparseablePathException if the given path cannot be parsed.
 
-    The given 'parser_choice' and 'filter_by_parser' change which files we
-    consider valid sources:
-
-    - If parser_choice is None, we will consider any file path that matches one
-      of the PARSER_CHOICES above a valid file.
-    - If parser_choice is not None, and filter_by_parser is False, we assume
-      that the given file path was explicitly passed by the user, and meant to
-      be parsed with the given parser_choice, _even_ when the file path does not
-      nominally match this parser choice: We return the file path as a valid
-      source regardless of its file name/suffix.
-    - If parser_choice is not None, and filter_by_parser is True, we assume that
-      the given file path was encountered while walking a directory passed by
-      the user. In this case, we will only consider the file valid if it matches
-      the given parser choice.
+    The given 'parser_choice' and 'filter_by_parser' determine which file paths
+    we consider valid sources, and how they are parsed: With parser_choice=None,
+    a file path will use the first matching parser in PARSER_CHOICES above, if
+    any. Otherwise - when parser_choice is specified - the file must either
+    match this parser (filter_by_parser=True), or this parser will be forced
+    even if the file does not match (filter_by_parser=False).
     """
-    if path.is_file():
-        choice_and_parser: Optional[Tuple[ParserChoice, ParsingStrategy]] = None
-        if parser_choice is not None:  # user wants a specific parser
-            strategy = PARSER_CHOICES[parser_choice]
-            if filter_by_parser:  # but only if the file matches
-                if strategy.applies_to_path(path):
-                    choice_and_parser = (parser_choice, strategy)
-                else:
-                    raise UnparseablePathException(
-                        ctx=f"Path does not match {parser_choice} parser", path=path
-                    )
-            else:  # user wants us to use this parser no matter what
-                choice_and_parser = (parser_choice, strategy)
-        else:  # no parser chosen, automatically determine parser for this path
-            choice_and_parser = first_applicable_parser(path)
-        if choice_and_parser is None:  # no suitable parser found
-            raise UnparseablePathException(
-                ctx="Parsing given dependencies path isn't supported", path=path
-            )
-        return DepsSource(path)
     if path.is_dir():
         return None
-    raise UnparseablePathException(
-        ctx="Dependencies declaration path is neither dir nor file", path=path
-    )
+    if not path.is_file():
+        raise UnparseablePathException(
+            ctx="Dependencies declaration path is neither dir nor file", path=path
+        )
+
+    if parser_choice is not None:  # user wants a specific parser
+        if filter_by_parser:  # but only if the file matches
+            if not PARSER_CHOICES[parser_choice].applies_to_path(path):
+                raise UnparseablePathException(
+                    ctx=f"Path does not match {parser_choice} parser", path=path
+                )
+    else:  # no parser chosen, automatically determine parser for this path
+        parser_choice = first_applicable_parser(path)
+    if parser_choice is None:  # no suitable parser given
+        raise UnparseablePathException(
+            ctx="Parsing given dependencies path isn't supported", path=path
+        )
+    return DepsSource(path, parser_choice)

fawltydeps/main.py

@@ -99,8 +99,7 @@ def declared_deps(self) -> List[DeclaredDependency]:
         """The list of declared dependencies parsed from this project."""
         return list(
             extract_declared_dependencies.parse_sources(
-                (src for src in self.sources if isinstance(src, DepsSource)),
-                self.settings.deps_parser_choice,
+                (src for src in self.sources if isinstance(src, DepsSource))
             )
         )
 

fawltydeps/types.py

@@ -63,9 +63,17 @@ def __post_init__(self) -> None:
 
 @dataclass(frozen=True, eq=True, order=True)
 class DepsSource:
-    """A source to be parsed for declared dependencies."""
+    """A source to be parsed for declared dependencies.
+
+    Also include which declared dependencies parser we have chosen to use for
+    this file.
+    """
 
     path: Path
+    parser_choice: ParserChoice
+
+    def __post_init__(self) -> None:
+        assert self.path.is_file()  # sanity check
 
 
 Source = Union[CodeSource, DepsSource]

tests/test_deps_parser_determination.py

@@ -5,7 +5,11 @@
 
 import pytest
 
-from fawltydeps.extract_declared_dependencies import parse_source, parse_sources
+from fawltydeps.extract_declared_dependencies import (
+    parse_source,
+    parse_sources,
+    validate_deps_source,
+)
 from fawltydeps.settings import ParserChoice, Settings
 from fawltydeps.traverse_project import find_sources
 from fawltydeps.types import DepsSource
@@ -44,7 +48,7 @@ def test_explicit_parse_strategy__mismatch_yields_appropriate_logging(
     deps_path.touch()
     caplog.set_level(logging.WARNING)
     # Execute here just for side effect (log).
-    list(parse_source(DepsSource(deps_path), parser_choice))
+    list(parse_source(DepsSource(deps_path, parser_choice)))
     if has_log:
         assert (
             f"Manually applying parser '{parser_choice}' to dependencies: {deps_path}"
@@ -75,7 +79,9 @@ def test_filepath_inference(
     """Parser choice finalization function can choose based on deps filename."""
     deps_path = tmp_path / deps_file_name
     assert deps_path.is_file()  # precondition
-    obs_deps = collect_dep_names(parse_source(DepsSource(deps_path)))
+    src = validate_deps_source(deps_path)
+    assert src is not None
+    obs_deps = collect_dep_names(parse_source(src))
     assert_unordered_equivalence(obs_deps, exp_deps)
 
 
@@ -103,9 +109,7 @@ def test_extract_from_directory_applies_manual_parser_choice_iff_choice_applies(
 ):
     settings = Settings(code=set(), deps={tmp_path}, deps_parser_choice=parser_choice)
     deps_sources = list(find_sources(settings, {DepsSource}))
-    obs_deps = collect_dep_names(
-        parse_sources(deps_sources, parser_choice=parser_choice)
-    )
+    obs_deps = collect_dep_names(parse_sources(deps_sources))
     assert_unordered_equivalence(obs_deps, exp_deps)
 
 
@@ -151,9 +155,7 @@ def test_extract_from_file_applies_manual_choice_even_if_mismatched(
     new_path = tmp_path / fn2
     shutil.move(old_path, new_path)
     caplog.set_level(logging.WARNING)
-    obs_deps = collect_dep_names(
-        parse_source(DepsSource(new_path), parser_choice=parser_choice)
-    )
+    obs_deps = collect_dep_names(parse_source(DepsSource(new_path, parser_choice)))
     assert_unordered_equivalence(obs_deps, exp_deps)
     exp_msg = f"Manually applying parser '{parser_choice}' to dependencies: {new_path}"
     assert exp_msg in caplog.text

tests/test_extract_declared_dependencies_success.py

@@ -8,6 +8,7 @@
     parse_setup_cfg,
     parse_setup_py,
     parse_sources,
+    validate_deps_source,
 )
 from fawltydeps.settings import Settings
 from fawltydeps.traverse_project import find_sources
@@ -454,7 +455,7 @@ def test_parse_sources__parse_only_requirements_from_subdir__returns_list(
         "tensorflow",
     ]
     path = project_with_setup_and_requirements / "subdir/requirements.txt"
-    actual = collect_dep_names(parse_sources([DepsSource(path)]))
+    actual = collect_dep_names(parse_sources([validate_deps_source(path)]))
     assert_unordered_equivalence(actual, expect)
 
 
@@ -582,5 +583,5 @@ def test_parse_requirements_per_req_options(tmp_path, deps_file_content, exp_dep
     # originally motivated by #114 (A dep can span multiple lines.)
     deps_path = tmp_path / "requirements.txt"
     deps_path.write_text(dedent(deps_file_content))
-    obs_deps = collect_dep_names(parse_sources([DepsSource(deps_path)]))
+    obs_deps = collect_dep_names(parse_sources([validate_deps_source(deps_path)]))
     assert_unordered_equivalence(obs_deps, exp_deps)