Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/twyn/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def entry_point() -> None:
@click.option(
"--dependency-file",
type=str,
multiple=True,
help=(
"Dependency file to analyze. By default, twyn will search in the current directory "
"for supported files, but this option will override that behavior."
Expand Down Expand Up @@ -112,7 +113,7 @@ def entry_point() -> None:
)
def run( # noqa: C901
config: str,
dependency_file: Optional[str],
dependency_file: tuple[str],
dependency: tuple[str],
selector_method: str,
v: bool,
Expand All @@ -133,15 +134,16 @@ def run( # noqa: C901
"Only one of --dependency or --dependency-file can be set at a time.", ctx=click.get_current_context()
)

if dependency_file and not any(dependency_file.endswith(key) for key in DEPENDENCY_FILE_MAPPING):
raise click.UsageError("Dependency file name not supported.", ctx=click.get_current_context())
for dep_file in dependency_file:
if dep_file and not any(dep_file.endswith(key) for key in DEPENDENCY_FILE_MAPPING):
raise click.UsageError(f"Dependency file name {dep_file} not supported.", ctx=click.get_current_context())

try:
possible_typos = check_dependencies(
selector_method=selector_method,
dependencies=set(dependency) or None,
config_file=config,
dependency_file=dependency_file,
dependency_file=set(dependency_file) or None,
use_cache=not no_cache if no_cache is not None else no_cache,
show_progress_bar=False if json else not no_track,
load_config_from_file=True,
Expand Down
14 changes: 8 additions & 6 deletions src/twyn/config/config_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,20 @@
class TwynConfiguration:
"""Fully resolved configuration for Twyn."""

dependency_file: Optional[str]
selector_method: str
allowlist: set[str]
source: Optional[str]
use_cache: bool
package_ecosystem: Optional[PackageEcosystems]
recursive: Optional[bool]
dependency_file: set[str]


@dataclass
class ReadTwynConfiguration:
"""Configuration for twyn as set by the user. It may have None values."""

dependency_file: Optional[str] = None
dependency_file: Optional[set[str]] = field(default_factory=set)
selector_method: Optional[str] = None
allowlist: set[str] = field(default_factory=set)
source: Optional[str] = None
Expand All @@ -63,7 +63,7 @@ def __init__(self, file_handler: Optional[FileHandler] = None) -> None:
def resolve_config(
self,
selector_method: Optional[str] = None,
dependency_file: Optional[str] = None,
dependency_files: Optional[set[str]] = None,
use_cache: Optional[bool] = None,
package_ecosystem: Optional[PackageEcosystems] = None,
recursive: Optional[bool] = None,
Expand Down Expand Up @@ -107,7 +107,7 @@ def resolve_config(
final_recursive = DEFAULT_RECURSIVE

return TwynConfiguration(
dependency_file=dependency_file or read_config.dependency_file,
dependency_file=dependency_files or read_config.dependency_file or set(),
selector_method=final_selector_method,
allowlist=read_config.allowlist,
source=read_config.source,
Expand Down Expand Up @@ -142,11 +142,13 @@ def _get_read_config(self, toml: TOMLDocument) -> ReadTwynConfiguration:
"""Read the twyn configuration from a provided toml document."""
twyn_config_data = toml.get("tool", {}).get("twyn", {})
return ReadTwynConfiguration(
dependency_file=twyn_config_data.get("dependency_file"),
dependency_file=set(twyn_config_data.get("dependency_file", set())),
selector_method=twyn_config_data.get("selector_method"),
allowlist=set(twyn_config_data.get("allowlist", set())),
source=twyn_config_data.get("source"),
use_cache=twyn_config_data.get("use_cache"),
package_ecosystem=twyn_config_data.get("package_ecosystem"),
recursive=twyn_config_data.get("recursive"),
)

def _write_config(self, toml: TOMLDocument, config: ReadTwynConfiguration) -> None:
Expand All @@ -155,12 +157,12 @@ def _write_config(self, toml: TOMLDocument, config: ReadTwynConfiguration) -> No
All null values are simply omitted from the toml file.
"""
twyn_toml_data = asdict(config, dict_factory=lambda x: _serialize_config(x))

if "tool" not in toml:
toml.add("tool", table())
if "twyn" not in toml["tool"]: # type: ignore[operator]
toml["tool"]["twyn"] = {} # type: ignore[index]
toml["tool"]["twyn"] = twyn_toml_data # type: ignore[index]

self._write_toml(toml)

def _write_toml(self, toml: TOMLDocument) -> None:
Expand Down
1 change: 0 additions & 1 deletion src/twyn/dependency_parser/dependency_selector.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ def get_dependency_file_parsers_from_file_name(self) -> list[AbstractParser]:
if self.dependency_file.endswith(known_dependency_file_name):
file_parser = DEPENDENCY_FILE_MAPPING[known_dependency_file_name](self.dependency_file)
parsers.append(file_parser)

if not parsers:
raise NoMatchingParserError

Expand Down
58 changes: 30 additions & 28 deletions src/twyn/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
def check_dependencies(
selector_method: Union[SelectorMethod, None] = None,
config_file: Optional[str] = None,
dependency_file: Optional[str] = None,
dependency_file: Optional[set[str]] = None,
dependencies: Optional[set[str]] = None,
use_cache: Optional[bool] = True,
show_progress_bar: bool = False,
Expand Down Expand Up @@ -68,7 +68,7 @@ def check_dependencies(
load_config_from_file=load_config_from_file,
config_file=config_file,
selector_method=selector_method,
dependency_file=dependency_file,
dependency_files=dependency_file,
use_cache=use_cache,
package_ecosystem=package_ecosystem,
recursive=recursive,
Expand Down Expand Up @@ -104,7 +104,7 @@ def check_dependencies(
maybe_cache_handler=maybe_cache_handler,
allowlist=config.allowlist,
show_progress_bar=show_progress_bar,
dependency_file=config.dependency_file,
dependency_files=config.dependency_file,
)


Expand Down Expand Up @@ -153,38 +153,39 @@ def _analyze_packages_from_source(
allowlist: set[str],
selector_method: SelectorMethod,
show_progress_bar: bool,
dependency_file: Optional[str],
dependency_files: Optional[set[str]],
source: Optional[str],
maybe_cache_handler: Optional[CacheHandler],
) -> TyposquatCheckResults:
"""Analyze dependencies from a dependencies file.

It will return a list of the possible typos grouped by source, each source being a dependency file.
"""
dependency_managers = _get_dependency_managers_and_parsers_mapping(dependency_file)
typos_by_file = TyposquatCheckResults()
for dependency_manager, parsers in dependency_managers.items():
top_package_reference = dependency_manager.trusted_packages_source(source, maybe_cache_handler)

packages_from_source = top_package_reference.get_packages()
trusted_packages = TrustedPackages(
names=packages_from_source,
algorithm=EditDistance(),
selector=selector_method,
threshold_class=SimilarityThreshold,
)
results: list[TyposquatCheckResultFromSource] = []

for parser in parsers:
analyzed_dependencies = _analyze_dependencies(
top_package_reference, trusted_packages, parser.parse(), allowlist, show_progress_bar, parser.file_path
dependency_files = dependency_files or {""}
for dep_file in dependency_files:
dependency_managers = _get_dependency_managers_and_parsers_mapping(dep_file)
typos_by_file = TyposquatCheckResults()
Comment on lines +165 to +167
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 🎯 Correctness

Severity: HIGH 🔴

typos_by_file = TyposquatCheckResults()

typos_by_file is re-initialized inside the for dep_file in dependency_files: loop. This will cause the function to only return the results from the last dependency file that is processed. It should be initialized before the loop.

Suggested change
for dep_file in dependency_files:
dependency_managers = _get_dependency_managers_and_parsers_mapping(dep_file)
typos_by_file = TyposquatCheckResults()
typos_by_file = TyposquatCheckResults()
for dep_file in dependency_files:
dependency_managers = _get_dependency_managers_and_parsers_mapping(dep_file)
More information about this comment
  • File: src/twyn/main.py
  • Line: 167
  • Relative line: 60
  • With suggestion: Yes
  • Suggestion ready for replacement: Yes

for dependency_manager, parsers in dependency_managers.items():
top_package_reference = dependency_manager.trusted_packages_source(source, maybe_cache_handler)

packages_from_source = top_package_reference.get_packages()
trusted_packages = TrustedPackages(
names=packages_from_source,
algorithm=EditDistance(),
selector=selector_method,
threshold_class=SimilarityThreshold,
)

if analyzed_dependencies:
results.append(
TyposquatCheckResultFromSource(source=str(parser.file_path), errors=analyzed_dependencies)
results: list[TyposquatCheckResultFromSource] = []
for parser in parsers:
analyzed_dependencies = _analyze_dependencies(
top_package_reference, trusted_packages, parser.parse(), allowlist, show_progress_bar
)
typos_by_file.results += results

if analyzed_dependencies:
results.append(
TyposquatCheckResultFromSource(source=str(parser.file_path), errors=analyzed_dependencies)
)
typos_by_file.results += results

return typos_by_file

Expand Down Expand Up @@ -258,6 +259,7 @@ def _get_dependency_managers_and_parsers_mapping(
dependency_managers: dict[type[BaseDependencyManager], list[AbstractParser]] = {}

# No dependencies introduced via the CLI, so the dependecy file was either given or will be auto-detected

dependency_selector = DependencySelector(dependency_file)
dependency_parsers = dependency_selector.get_dependency_parsers()

Expand All @@ -274,7 +276,7 @@ def _get_config(
load_config_from_file: bool,
config_file: Optional[str],
selector_method: Union[SelectorMethod, None],
dependency_file: Optional[str],
dependency_files: Optional[set[str]],
use_cache: Optional[bool],
package_ecosystem: Optional[PackageEcosystems],
recursive: Optional[bool],
Expand All @@ -286,7 +288,7 @@ def _get_config(
config_file_handler = None
return ConfigHandler(config_file_handler).resolve_config(
selector_method=selector_method,
dependency_file=dependency_file,
dependency_files=dependency_files,
use_cache=use_cache,
package_ecosystem=package_ecosystem,
recursive=recursive,
Expand Down
18 changes: 7 additions & 11 deletions tests/config/test_config_handler.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import dataclasses
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 🧪 Testing

Severity: MEDIUM 🟡

There is no test case that verifies the configuration parsing when dependency_file is a single string in the TOML file. The existing tests were updated to use a list of files, but the case of a single string value should also be covered to prevent regressions and catch issues like the one in _get_read_config. Please add a test case for this scenario.

More information about this comment
  • File: tests/config/test_config_handler.py
  • Line: 1
  • Relative line: 1
  • With suggestion: No
  • Suggestion ready for replacement: No

from copy import deepcopy
from pathlib import Path
from typing import NoReturn
from unittest.mock import Mock, patch

import pytest
Expand All @@ -26,17 +25,14 @@


class TestConfigHandler:
def throw_exception(self) -> NoReturn:
raise PathNotFoundError

@patch("twyn.file_handler.file_handler.FileHandler.read")
def test_no_enforce_file_on_non_existent_file(self, mock_is_file: Mock) -> None:
"""Resolving the config without enforcing the file to be present gives you defaults."""
mock_is_file.side_effect = self.throw_exception
mock_is_file.side_effect = PathNotFoundError()
config = ConfigHandler(FileHandler(DEFAULT_PROJECT_TOML_FILE)).resolve_config()

assert config == TwynConfiguration(
dependency_file=None,
dependency_file=set(),
selector_method="all",
allowlist=set(),
source=None,
Expand All @@ -51,7 +47,7 @@ def test_config_raises_for_unknown_file(self) -> None:

def test_read_config_values(self, pyproject_toml_file: Path) -> None:
config = ConfigHandler(file_handler=FileHandler(pyproject_toml_file)).resolve_config()
assert config.dependency_file == "my_file.txt"
assert config.dependency_file == {"my_file.txt", "my_other_file.txt"}
assert config.selector_method == "all"
assert config.allowlist == {"boto4", "boto2"}
assert config.use_cache is False
Expand All @@ -62,7 +58,7 @@ def test_get_twyn_data_from_file(self, pyproject_toml_file: Path) -> None:
toml = handler._read_toml()
twyn_data = ConfigHandler(FileHandler(pyproject_toml_file))._get_read_config(toml)
assert twyn_data == ReadTwynConfiguration(
dependency_file="my_file.txt",
dependency_file={"my_file.txt", "my_other_file.txt"},
selector_method="all",
allowlist={"boto4", "boto2"},
source=None,
Expand All @@ -75,7 +71,7 @@ def test_write_toml(self, pyproject_toml_file: Path) -> None:

initial_config = handler.resolve_config()
to_write = deepcopy(initial_config)
to_write = dataclasses.replace(to_write, allowlist={})
to_write = dataclasses.replace(to_write, allowlist={}, dependency_file={})

handler._write_config(toml, to_write)

Expand All @@ -100,7 +96,7 @@ def test_write_toml(self, pyproject_toml_file: Path) -> None:
"scripts": {"twyn": "twyn.cli:entry_point"},
},
"twyn": {
"dependency_file": "my_file.txt",
"dependency_file": {},
"selector_method": "all",
"allowlist": {},
"use_cache": False,
Expand Down Expand Up @@ -141,7 +137,7 @@ def test_no_load_config_from_cache(self, pyproject_toml_file: Path) -> None:
config = ConfigHandler().resolve_config()

assert config.allowlist == set()
assert config.dependency_file is None
assert config.dependency_file == set()
assert config.use_cache is True
assert config.selector_method == DEFAULT_SELECTOR_METHOD
assert config.source is None
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def pyproject_toml_file(tmp_path: Path) -> Iterator[Path]:
twyn = "twyn.cli:entry_point"

[tool.twyn]
dependency_file="my_file.txt"
dependency_file=["my_file.txt", "my_other_file.txt"]
selector_method="all"
logging_level="debug"
allowlist=["boto4", "boto2"]
Expand Down
6 changes: 3 additions & 3 deletions tests/main/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def test_click_arguments_dependency_file(self, mock_check_dependencies: Mock) ->
assert mock_check_dependencies.call_args_list == [
call(
config_file="my-config",
dependency_file="requirements.txt",
dependency_file={"requirements.txt"},
dependencies=None,
selector_method="first-letter",
use_cache=None,
Expand All @@ -113,7 +113,7 @@ def test_click_arguments_dependency_file_in_different_path(self, mock_check_depe
assert mock_check_dependencies.call_args_list == [
call(
config_file=None,
dependency_file="/path/requirements.txt",
dependency_file={"/path/requirements.txt"},
dependencies=None,
selector_method=None,
use_cache=None,
Expand Down Expand Up @@ -328,7 +328,7 @@ def test_dependency_file_name_has_to_be_recognized(self) -> None:

assert isinstance(result.exception, SystemExit)
assert result.exit_code == 2
assert "Dependency file name not supported." in result.output
assert "Dependency file name requirements-dev.txt not supported." in result.output

@patch("twyn.cli.check_dependencies")
def test_custom_twyn_error_is_caught_and_wrapped_in_cli_error(self, mock_check_dependencies, caplog):
Expand Down
Loading