refactor: Cleanup for pylint

Author: dbanty

openapi_python_client/__init__.py

@@ -20,6 +20,7 @@
 from .utils import snake_case
 
 if sys.version_info.minor < 8:  # version did not exist before 3.8, need to use a backport
+    # pylint: disable=import-error
     from importlib_metadata import version
 else:
     from importlib.metadata import version  # type: ignore
@@ -28,6 +29,8 @@
 
 
 class MetaType(str, Enum):
+    """The types of metadata supported for project generation."""
+
     NONE = "none"
     POETRY = "poetry"
     SETUP = "setup"
@@ -41,7 +44,9 @@ class MetaType(str, Enum):
 }
 
 
-class Project:
+class Project:  # pylint: disable=too-many-instance-attributes
+    """Represents a Python project (the top level file-tree) to generate"""
+
     def __init__(
         self,
         *,
@@ -129,15 +134,19 @@ def _reformat(self) -> None:
             shell=True,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
+            check=True,
         )
         subprocess.run(
             "isort .",
             cwd=self.project_dir,
             shell=True,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
+            check=True,
+        )
+        subprocess.run(
+            "black .", cwd=self.project_dir, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True
         )
-        subprocess.run("black .", cwd=self.project_dir, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
     def _get_errors(self) -> Sequence[GeneratorError]:
         errors = []
@@ -263,7 +272,7 @@ def _build_api(self) -> None:
                 module_path.write_text(endpoint_template.render(endpoint=endpoint), encoding=self.file_encoding)
 
 
-def _get_project_for_url_or_path(
+def _get_project_for_url_or_path(  # pylint: disable=too-many-arguments
     url: Optional[str],
     path: Optional[Path],
     meta: MetaType,

openapi_python_client/cli.py

@@ -27,28 +27,28 @@ def _process_config(path: Optional[pathlib.Path]) -> Config:
 
     try:
         return Config.load_from_path(path=path)
-    except:  # noqa
-        raise typer.BadParameter("Unable to parse config")
+    except Exception as err:
+        raise typer.BadParameter("Unable to parse config") from err
 
 
 # noinspection PyUnusedLocal
+# pylint: disable=unused-argument
 @app.callback(name="openapi-python-client")
 def cli(
     version: bool = typer.Option(False, "--version", callback=_version_callback, help="Print the version and exit"),
 ) -> None:
     """Generate a Python client from an OpenAPI JSON document"""
-    pass
 
 
-def _print_parser_error(e: GeneratorError, color: str) -> None:
-    typer.secho(e.header, bold=True, fg=color, err=True)
+def _print_parser_error(err: GeneratorError, color: str) -> None:
+    typer.secho(err.header, bold=True, fg=color, err=True)
     typer.echo()
-    if e.detail:
-        typer.secho(e.detail, fg=color, err=True)
+    if err.detail:
+        typer.secho(err.detail, fg=color, err=True)
         typer.echo()
 
-    if isinstance(e, ParseError) and e.data is not None:
-        formatted_data = pformat(e.data)
+    if isinstance(err, ParseError) and err.data is not None:
+        formatted_data = pformat(err.data)
         typer.secho(formatted_data, fg=color, err=True)
 
     typer.echo()
@@ -111,6 +111,7 @@ def handle_errors(errors: Sequence[GeneratorError], fail_on_warning: bool = Fals
 CONFIG_OPTION = typer.Option(None, "--config", help="Path to the config file to use")
 
 
+# pylint: disable=too-many-arguments
 @app.command()
 def generate(
     url: Optional[str] = typer.Option(None, help="A URL to read the JSON from"),
@@ -133,9 +134,9 @@ def generate(
 
     try:
         codecs.getencoder(file_encoding)
-    except LookupError:
+    except LookupError as err:
         typer.secho("Unknown encoding : {}".format(file_encoding), fg=typer.colors.RED)
-        raise typer.Exit(code=1)
+        raise typer.Exit(code=1) from err
 
     config = _process_config(config_path)
     errors = create_new_client(
@@ -149,6 +150,7 @@ def generate(
     handle_errors(errors, fail_on_warning)
 
 
+# pylint: disable=too-many-arguments
 @app.command()
 def update(
     url: Optional[str] = typer.Option(None, help="A URL to read the JSON from"),
@@ -171,9 +173,9 @@ def update(
 
     try:
         codecs.getencoder(file_encoding)
-    except LookupError:
+    except LookupError as err:
         typer.secho("Unknown encoding : {}".format(file_encoding), fg=typer.colors.RED)
-        raise typer.Exit(code=1)
+        raise typer.Exit(code=1) from err
 
     config = _process_config(config_path)
     errors = update_existing_client(

openapi_python_client/config.py

@@ -6,11 +6,21 @@
 
 
 class ClassOverride(BaseModel):
+    """An override of a single generated class.
+
+    See https://github.com/openapi-generators/openapi-python-client#class_overrides
+    """
+
     class_name: Optional[str] = None
     module_name: Optional[str] = None
 
 
 class Config(BaseModel):
+    """Contains any configurable values passed by the user.
+
+    See https://github.com/openapi-generators/openapi-python-client#configuration
+    """
+
     class_overrides: Dict[str, ClassOverride] = {}
     project_name_override: Optional[str]
     package_name_override: Optional[str]

openapi_python_client/parser/openapi.py

@@ -81,6 +81,7 @@ def generate_operation_id(*, path: str, method: str) -> str:
     return f"{method}_{clean_path}"
 
 
+# pylint: disable=too-many-instance-attributes
 @dataclass
 class Endpoint:
     """

openapi_python_client/schema/openapi_schema_pydantic/example.py

@@ -4,7 +4,7 @@
 
 
 class Example(BaseModel):
-    """ Examples added to parameters / components to help clarify usage.
+    """Examples added to parameters / components to help clarify usage.
 
     References:
         - https://swagger.io/docs/specification/adding-examples/

openapi_python_client/utils.py

@@ -3,14 +3,14 @@
 from keyword import iskeyword
 from typing import Any, List
 
-delimiters = " _-"
+DELIMITERS = " _-"
 
 
 class PythonIdentifier(str):
     """A string which has been validated / transformed into a valid identifier for Python"""
 
     def __new__(cls, value: str, prefix: str) -> "PythonIdentifier":
-        new_value = fix_reserved_words(fix_keywords(snake_case(sanitize(value))))
+        new_value = fix_reserved_words(snake_case(sanitize(value)))
 
         if not new_value.isidentifier():
             new_value = f"{prefix}{new_value}"
@@ -22,50 +22,60 @@ def __deepcopy__(self, _: Any) -> "PythonIdentifier":
 
 def sanitize(value: str) -> str:
     """Removes every character that isn't 0-9, A-Z, a-z, or a known delimiter"""
-    return re.sub(rf"[^\w{delimiters}]+", "", value)
+    return re.sub(rf"[^\w{DELIMITERS}]+", "", value)
 
 
 def split_words(value: str) -> List[str]:
     """Split a string on words and known delimiters"""
     # We can't guess words if there is no capital letter
     if any(c.isupper() for c in value):
         value = " ".join(re.split("([A-Z]?[a-z]+)", value))
-    return re.findall(rf"[^{delimiters}]+", value)
-
-
-def fix_keywords(value: str) -> str:
-    if iskeyword(value):
-        return f"{value}_"
-    return value
+    return re.findall(rf"[^{DELIMITERS}]+", value)
 
 
 RESERVED_WORDS = (set(dir(builtins)) | {"self"}) - {"type", "id"}
 
 
 def fix_reserved_words(value: str) -> str:
-    if value in RESERVED_WORDS:
+    """
+    Using reserved Python words as identifiers in generated code causes problems, so this function renames them.
+
+    Args:
+        value: The identifier to-be that should be renamed if it's a reserved word.
+
+    Returns:
+        `value` suffixed with `_` if it was a reserved word.
+    """
+    if value in RESERVED_WORDS or iskeyword(value):
         return f"{value}_"
     return value
 
 
 def snake_case(value: str) -> str:
+    """Converts to snake_case"""
     words = split_words(sanitize(value))
-    value = "_".join(words).lower()
-    return fix_keywords(value)
+    return "_".join(words).lower()
 
 
 def pascal_case(value: str) -> str:
+    """Converts to PascalCase"""
     words = split_words(sanitize(value))
     capitalized_words = (word.capitalize() if not word.isupper() else word for word in words)
-    value = "".join(capitalized_words)
-    return fix_keywords(value)
+    return "".join(capitalized_words)
 
 
 def kebab_case(value: str) -> str:
+    """Converts to kebab-case"""
     words = split_words(sanitize(value))
-    value = "-".join(words).lower()
-    return fix_keywords(value)
+    return "-".join(words).lower()
 
 
 def remove_string_escapes(value: str) -> str:
+    """Used when parsing string-literal defaults to prevent escaping the string to write arbitrary Python
+
+    **REMOVING OR CHANGING THE USAGE OF THIS FUNCTION HAS SECURITY IMPLICATIONS**
+
+    See Also:
+        - https://github.com/openapi-generators/openapi-python-client/security/advisories/GHSA-9x4c-63pf-525f
+    """
     return value.replace('"', r"\"")

pyproject.toml

@@ -99,7 +99,20 @@ omit = ["openapi_python_client/templates/*"]
 max-line-length = 120
 
 [tool.pylint.messages_control]
-disable = ["duplicate-code", "missing-module-docstring", "too-few-public-methods"]
+disable = [
+    # DRY < MOIST
+    "duplicate-code",
+    # Sometimes necessary to prevent cycles
+    "import-outside-toplevel",
+    # Modules are mostly used for organization here, there is no lib API
+    "missing-module-docstring",
+    # Organization is important, even when just separating classes
+    "too-few-public-methods",
+    # Disable any type-checking, that's what mypy is for
+    "no-member",
+    # False positives
+    "cyclic-import",
+]
 
 [build-system]
 requires = ["poetry>=1.0"]

tests/test___init__.py

@@ -519,15 +519,24 @@ def test__reformat(mocker):
                 shell=True,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
+                check=True,
             ),
             mocker.call(
                 "isort .",
                 cwd=project.project_dir,
                 shell=True,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
+                check=True,
+            ),
+            mocker.call(
+                "black .",
+                cwd=project.project_dir,
+                shell=True,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                check=True,
             ),
-            mocker.call("black .", cwd=project.project_dir, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE),
         ]
     )
 

tests/test_utils.py

@@ -74,10 +74,6 @@ def test_no_string_escapes():
     assert utils.remove_string_escapes('an "evil" string') == 'an \\"evil\\" string'
 
 
-def test__fix_keywords():
-    assert utils.fix_keywords("None") == "None_"
-
-
 @pytest.mark.parametrize(
     "reserved_word, expected",
     [
@@ -87,6 +83,7 @@ def test__fix_keywords():
         ("not_reserved", "not_reserved"),
         ("type", "type"),
         ("id", "id"),
+        ("None", "None_"),
     ],
 )
 def test__fix_reserved_words(reserved_word: str, expected: str):