Ideas for refactoring (#4962)

* Ideas for refactoring

And also refering to issue #4961

* Respond to the review

Removed items() from os.environ and add blank lines

* Add link.is_wheel back

src/poetry/console/commands/build.py

@@ -21,10 +21,7 @@ class BuildCommand(EnvCommand):
     def handle(self) -> None:
         from poetry.core.masonry.builder import Builder
 
-        fmt = "all"
-        if self.option("format"):
-            fmt = self.option("format")
-
+        fmt = self.option("format") or "all"
         package = self.poetry.package
         self.line(
             f"Building <c1>{package.pretty_name}</c1> (<c2>{package.version}</c2>)"

src/poetry/console/commands/cache/clear.py

@@ -51,9 +51,9 @@ def handle(self) -> int:
                 return 0
 
             # Calculate number of entries
-            entries_count = 0
-            for _path, _dirs, files in os.walk(str(cache_dir)):
-                entries_count += len(files)
+            entries_count = sum(
+                len(files) for _path, _dirs, files in os.walk(str(cache_dir))
+            )
 
             delete = self.confirm(f"<question>Delete {entries_count} entries?</>")
             if not delete:

src/poetry/console/commands/plugin/add.py

@@ -70,9 +70,7 @@ def handle(self) -> int:
         # Plugins should be installed in the system env to be globally available
         system_env = EnvManager.get_system_env(naive=True)
 
-        env_dir = Path(
-            os.getenv("POETRY_HOME") if os.getenv("POETRY_HOME") else system_env.path
-        )
+        env_dir = Path(os.getenv("POETRY_HOME") or system_env.path)
 
         # We check for the plugins existence first.
         if env_dir.joinpath("pyproject.toml").exists():

src/poetry/console/commands/plugin/remove.py

@@ -40,9 +40,7 @@ def handle(self) -> int:
         plugins = self.argument("plugins")
 
         system_env = EnvManager.get_system_env(naive=True)
-        env_dir = Path(
-            os.getenv("POETRY_HOME") if os.getenv("POETRY_HOME") else system_env.path
-        )
+        env_dir = Path(os.getenv("POETRY_HOME") or system_env.path)
 
         # From this point forward, all the logic will be deferred to
         # the remove command, by using the global `pyproject.toml` file.

src/poetry/console/commands/remove.py

@@ -50,11 +50,10 @@ def handle(self) -> int:
 
         if group is None:
             removed = []
-            group_sections = []
-            for group_name, group_section in poetry_content.get("group", {}).items():
-                group_sections.append(
-                    (group_name, group_section.get("dependencies", {}))
-                )
+            group_sections = [
+                (group_name, group_section.get("dependencies", {}))
+                for group_name, group_section in poetry_content.get("group", {}).items()
+            ]
 
             for group_name, section in [
                 ("default", poetry_content["dependencies"])

src/poetry/factory.py

@@ -160,13 +160,12 @@ def create_legacy_repository(
         from poetry.utils.helpers import get_cert
         from poetry.utils.helpers import get_client_cert
 
-        if "url" in source:
-            # PyPI-like repository
-            if "name" not in source:
-                raise RuntimeError("Missing [name] in source.")
-        else:
+        if "url" not in source:
             raise RuntimeError("Unsupported source specified")
 
+        # PyPI-like repository
+        if "name" not in source:
+            raise RuntimeError("Missing [name] in source.")
         name = source["name"]
         url = source["url"]
 

src/poetry/inspection/info.py

@@ -322,10 +322,7 @@ def from_setup_files(cls, path: Path) -> "PackageInfo":
         if python_requires is None:
             python_requires = "*"
 
-        requires = ""
-        for dep in result["install_requires"]:
-            requires += dep + "\n"
-
+        requires = "".join(dep + "\n" for dep in result["install_requires"])
         if result["extras_require"]:
             requires += "\n"
 

src/poetry/installation/chooser.py

@@ -85,14 +85,13 @@ def choose_for(self, package: "Package") -> "Link":
         return chosen
 
     def _get_links(self, package: "Package") -> List["Link"]:
-        if not package.source_type:
-            if not self._pool.has_repository("pypi"):
-                repository = self._pool.repositories[0]
-            else:
-                repository = self._pool.repository("pypi")
-        else:
+        if package.source_type:
             repository = self._pool.repository(package.source_reference)
 
+        elif not self._pool.has_repository("pypi"):
+            repository = self._pool.repositories[0]
+        else:
+            repository = self._pool.repository("pypi")
         links = repository.find_links_for_package(package)
 
         hashes = [f["hash"] for f in package.files]
@@ -142,7 +141,6 @@ def _sort_key(self, package: "Package", link: "Link") -> Tuple:
               comparison operators, but then different sdist links
               with the same version, would have to be considered equal
         """
-        support_num = len(self._env.supported_tags)
         build_tag = ()
         binary_preference = 0
         if link.is_wheel:
@@ -160,6 +158,7 @@ def _sort_key(self, package: "Package", link: "Link") -> Tuple:
                 build_tag_groups = match.groups()
                 build_tag = (int(build_tag_groups[0]), build_tag_groups[1])
         else:  # sdist
+            support_num = len(self._env.supported_tags)
             pri = -support_num
 
         has_allowed_hash = int(self._is_link_hash_allowed_for_package(link, package))

src/poetry/masonry/builders/editable.py

@@ -108,17 +108,14 @@ def _setup_build(self) -> None:
                 os.remove(str(setup))
 
     def _add_pth(self) -> List[Path]:
-        paths = set()
-        for include in self._module.includes:
-            if isinstance(include, PackageInclude) and (
-                include.is_module() or include.is_package()
-            ):
-                paths.add(include.base.resolve().as_posix())
-
-        content = ""
-        for path in paths:
-            content += decode(path + os.linesep)
-
+        paths = {
+            include.base.resolve().as_posix()
+            for include in self._module.includes
+            if isinstance(include, PackageInclude)
+            and (include.is_module() or include.is_package())
+        }
+
+        content = "".join(decode(path + os.linesep) for path in paths)
         pth_file = Path(self._module.name).with_suffix(".pth")
 
         # remove any pre-existing pth files for this package

src/poetry/mixology/failure.py

@@ -201,12 +201,10 @@ def _visit(
                 derived_cause: ConflictCause = derived.cause
                 if isinstance(derived_cause.conflict.cause, ConflictCause):
                     collapsed_derived = derived_cause.conflict
+                    collapsed_ext = derived_cause.other
                 else:
                     collapsed_derived = derived_cause.other
 
-                if isinstance(derived_cause.conflict.cause, ConflictCause):
-                    collapsed_ext = derived_cause.other
-                else:
                     collapsed_ext = derived_cause.conflict
 
                 details_for_cause = {}

src/poetry/mixology/incompatibility.py

@@ -36,15 +36,12 @@ def __init__(self, terms: List["Term"], cause: "IncompatibilityCause") -> None:
                 if not term.is_positive() or not term.dependency.is_root
             ]
 
-        if (
-            len(terms) == 1
+        if len(terms) != 1 and (
             # Short-circuit in the common case of a two-term incompatibility with
             # two different packages (for example, a dependency).
-            or len(terms) == 2
-            and terms[0].dependency.complete_name != terms[-1].dependency.complete_name
+            len(terms) != 2
+            or terms[0].dependency.complete_name == terms[-1].dependency.complete_name
         ):
-            pass
-        else:
             # Coalesce multiple terms about the same package if possible.
             by_name: Dict[str, Dict[str, "Term"]] = {}
             for term in terms:
@@ -178,22 +175,22 @@ def __str__(self) -> str:
             term2 = self._terms[1]
 
             if term1.is_positive() == term2.is_positive():
-                if term1.is_positive():
-                    package1 = (
-                        term1.dependency.name
-                        if term1.constraint.is_any()
-                        else self._terse(term1)
-                    )
-                    package2 = (
-                        term2.dependency.name
-                        if term2.constraint.is_any()
-                        else self._terse(term2)
-                    )
-
-                    return f"{package1} is incompatible with {package2}"
-                else:
+                if not term1.is_positive():
                     return f"either {self._terse(term1)} or {self._terse(term2)}"
 
+                package1 = (
+                    term1.dependency.name
+                    if term1.constraint.is_any()
+                    else self._terse(term1)
+                )
+                package2 = (
+                    term2.dependency.name
+                    if term2.constraint.is_any()
+                    else self._terse(term2)
+                )
+
+                return f"{package1} is incompatible with {package2}"
+
         positive = []
         negative = []
 
@@ -204,12 +201,11 @@ def __str__(self) -> str:
                 negative.append(self._terse(term))
 
         if positive and negative:
-            if len(positive) == 1:
-                positive_term = [term for term in self._terms if term.is_positive()][0]
-
-                return f"{self._terse(positive_term, allow_every=True)} requires {' or '.join(negative)}"
-            else:
+            if len(positive) != 1:
                 return f"if {' and '.join(positive)} then {' or '.join(negative)}"
+
+            positive_term = [term for term in self._terms if term.is_positive()][0]
+            return f"{self._terse(positive_term, allow_every=True)} requires {' or '.join(negative)}"
         elif positive:
             return f"one of {' or '.join(positive)} must be false"
         else:

src/poetry/mixology/solutions/providers/python_requirement_solution_provider.py

@@ -23,10 +23,7 @@ def can_solve(self, exception: Exception) -> bool:
             str(exception),
         )
 
-        if not m:
-            return False
-
-        return True
+        return bool(m)
 
     def get_solutions(self, exception: Exception) -> List["Solution"]:
         from poetry.mixology.solutions.solutions.python_requirement_solution import (

src/poetry/mixology/version_solver.py

@@ -96,9 +96,7 @@ def _propagate(self, package: str) -> None:
         Performs unit propagation on incompatibilities transitively
         related to package to derive new assignments for _solution.
         """
-        changed = set()
-        changed.add(package)
-
+        changed = {package}
         while changed:
             package = changed.pop()
 
@@ -269,10 +267,9 @@ def _resolve_conflict(self, incompatibility: Incompatibility) -> Incompatibility
             # true (that is, we know for sure no solution will satisfy the
             # incompatibility) while also approximating the intuitive notion of the
             # "root cause" of the conflict.
-            new_terms = []
-            for term in incompatibility.terms:
-                if term != most_recent_term:
-                    new_terms.append(term)
+            new_terms = [
+                term for term in incompatibility.terms if term != most_recent_term
+            ]
 
             for term in most_recent_satisfier.cause.terms:
                 if term.dependency != most_recent_satisfier.dependency:

src/poetry/puzzle/solver.py

@@ -94,9 +94,6 @@ def solve(self, use_latest: List[str] = None) -> "Transaction":
     def solve_in_compatibility_mode(
         self, overrides: Tuple[Dict, ...], use_latest: List[str] = None
     ) -> Tuple[List["Package"], List[int]]:
-        locked = {}
-        for package in self._locked.packages:
-            locked[package.name] = DependencyPackage(package.to_dependency(), package)
 
         packages = []
         depths = []
@@ -127,9 +124,10 @@ def _solve(self, use_latest: List[str] = None) -> Tuple[List["Package"], List[in
         if self._provider._overrides:
             self._overrides.append(self._provider._overrides)
 
-        locked = {}
-        for package in self._locked.packages:
-            locked[package.name] = DependencyPackage(package.to_dependency(), package)
+        locked = {
+            package.name: DependencyPackage(package.to_dependency(), package)
+            for package in self._locked.packages
+        }
 
         try:
             result = resolve_version(
@@ -218,16 +216,16 @@ def depth_first_search(
         name_children[node.name].extend(node.reachable())
         combined_nodes[node.name].append(node)
 
-    combined_topo_sorted_nodes = []
-    for node in topo_sorted_nodes:
-        if node.name in combined_nodes:
-            combined_topo_sorted_nodes.append(combined_nodes.pop(node.name))
+    combined_topo_sorted_nodes = [
+        combined_nodes.pop(node.name)
+        for node in topo_sorted_nodes
+        if node.name in combined_nodes
+    ]
 
-    results = [
+    return [
         aggregator(nodes, name_children[nodes[0].name])
         for nodes in combined_topo_sorted_nodes
     ]
-    return results
 
 
 def dfs_visit(
@@ -333,21 +331,20 @@ def reachable(self) -> List["PackageNode"]:
                 continue
 
             for pkg in self.packages:
-                if pkg.complete_name == dependency.complete_name and (
-                    dependency.constraint.allows(pkg.version)
-                    or dependency.allows_prereleases()
-                    and pkg.version.is_unstable()
-                    and dependency.constraint.allows(pkg.version.stable)
-                ):
-                    # If there is already a child with this name
-                    # we merge the requirements
-                    if any(
+                if (
+                    pkg.complete_name == dependency.complete_name
+                    and (
+                        dependency.constraint.allows(pkg.version)
+                        or dependency.allows_prereleases()
+                        and pkg.version.is_unstable()
+                        and dependency.constraint.allows(pkg.version.stable)
+                    )
+                    and not any(
                         child.package.name == pkg.name
                         and child.groups == dependency.groups
                         for child in children
-                    ):
-                        continue
-
+                    )
+                ):
                     children.append(
                         PackageNode(
                             pkg,

src/poetry/puzzle/transaction.py

@@ -65,12 +65,10 @@ def calculate_operations(
 
         if with_uninstalls:
             for current_package in self._current_packages:
-                found = False
-                for result_package, _ in self._result_packages:
-                    if current_package.name == result_package.name:
-                        found = True
-
-                        break
+                found = any(
+                    current_package.name == result_package.name
+                    for result_package, _ in self._result_packages
+                )
 
                 if not found:
                     for installed_package in self._installed_packages: