fix: Added more checks for pylint. (#758)

Signed-off-by: Chris Butler <[email protected]>

.pylintrc

@@ -0,0 +1,10 @@
+[MASTER]
+ignore=oscal
+extension-pkg-whitelist=pydantic 
+
+
+[MESSAGES CONTROL]
+disable=W1203, W1201
+
+[FORMAT]
+max-line-length=120

.pylintrc_tests

@@ -0,0 +1,9 @@
+[MASTER]
+ignore=oscal
+
+
+[MESSAGES CONTROL]
+disable=W1203, W1201, W0212
+
+[FORMAT]
+max-line-length=120

Makefile

@@ -56,7 +56,6 @@ test-bdist:: clean
 	. tests/manual_tests/test_binary.sh
 
 
-
 release::
 	git config --global user.name "semantic-release (via Github actions)"
 	git config --global user.email "semantic-release@github-actions"
@@ -90,3 +89,11 @@ clean::
 	rm -rf .mypy_cache
 	find . | grep -E "__pycache__|\.pyc|\.pyo" | xargs rm -rf
 
+pylint:
+	pylint trestle
+
+pylint-test:
+	pylint tests --rcfile=.pylintrc_tests
+
+
+

docs/api_reference/trestle.core.catalog_interface.md

@@ -0,0 +1,2 @@
+::: trestle.core.catalog_interface
+handler: python

docs/api_reference/trestle.core.commands.author.catalog.md

@@ -0,0 +1,2 @@
+::: trestle.core.commands.author.catalog
+handler: python

docs/api_reference/trestle.core.control_io.md

@@ -0,0 +1,2 @@
+::: trestle.core.control_io
+handler: python

setup.cfg

@@ -73,7 +73,8 @@ dev =
     markdown-include
     pymdown-extensions
     livereload
-    ## Constrain system 
+    ## Constrain system
+    pylint
 
 
 

tests/trestle/core/commands/assemble_test.py

@@ -91,9 +91,7 @@ def test_assemble_execution_failure(testdata_dir: pathlib.Path, tmp_trestle_dir:
     with mock.patch('trestle.core.models.plans.Plan.simulate') as simulate_mock:
         simulate_mock.side_effect = err.TrestleError('simulation error')
         rc = AssembleCmd().assemble_model(
-            'catalog',
-            Catalog,
-            argparse.Namespace(trestle_root=tmp_trestle_dir, name='mycatalog', extension='json', verbose=1)
+            'catalog', argparse.Namespace(trestle_root=tmp_trestle_dir, name='mycatalog', extension='json', verbose=1)
         )
         assert rc == 1
 

tests/trestle/core/commands/author/headers_test.py

@@ -216,7 +216,7 @@ def test_e2e(
         assert wrapped_error.value.code == validate_rc
 
 
-def test_abort_safely_on_missing_directory(testdata_dir: pathlib.Path, tmp_trestle_dir: pathlib.Path) -> None:
+def test_abort_safely_on_missing_directory(tmp_trestle_dir: pathlib.Path) -> None:
     """Test that validation fails cleanly on a missing directory."""
     task_name = 'tester'
     command_string_setup = f'trestle author headers setup -tn {task_name}'
@@ -236,7 +236,7 @@ def test_abort_safely_on_missing_directory(testdata_dir: pathlib.Path, tmp_trest
         assert wrapped_error.value.code == 1
 
 
-def test_taskpath_is_a_file(testdata_dir: pathlib.Path, tmp_trestle_dir: pathlib.Path) -> None:
+def test_taskpath_is_a_file(tmp_trestle_dir: pathlib.Path) -> None:
     """Test that validation fails cleanly on a missing directory."""
     task_name = 'tester'
     command_string_setup = f'trestle author headers setup -tn {task_name}'
@@ -258,7 +258,7 @@ def test_taskpath_is_a_file(testdata_dir: pathlib.Path, tmp_trestle_dir: pathlib
         assert wrapped_error.value.code == 1
 
 
-def test_taskpath_is_a_file_setup(testdata_dir: pathlib.Path, tmp_trestle_dir: pathlib.Path) -> None:
+def test_taskpath_is_a_file_setup(tmp_trestle_dir: pathlib.Path) -> None:
     """Test that setup fails cleanly when taskpath is a file."""
     task_name = 'tester'
     command_string_setup = f'trestle author headers setup -tn {task_name}'

tests/trestle/core/commands/remove_test.py

@@ -52,7 +52,7 @@ def test_remove(tmp_path: pathlib.Path):
 
     # Call remove() method
     actual_remove_action, actual_catalog_removed_responsible_parties = RemoveCmd.remove(
-        element_path, Catalog, catalog_with_responsible_parties
+        element_path, catalog_with_responsible_parties
     )
 
     # 1.1 Assertion about action
@@ -80,7 +80,7 @@ def test_remove(tmp_path: pathlib.Path):
     expected_remove_action = RemoveAction(catalog_with_roles, element_path)
 
     # Call remove() method
-    actual_remove_action, actual_catalog_removed_roles = RemoveCmd.remove(element_path, Catalog, catalog_with_roles)
+    actual_remove_action, actual_catalog_removed_roles = RemoveCmd.remove(element_path, catalog_with_roles)
 
     # 2.1 Assertion about action
     assert expected_remove_action == actual_remove_action
@@ -104,7 +104,7 @@ def test_remove_failure(tmp_path: pathlib.Path):
     element_path = ElementPath('catalog.metadata.roles')
     try:
         actual_remove_action, actual_catalog_removed_responsible_parties = RemoveCmd.remove(
-            element_path, Catalog, catalog_with_responsible_parties
+            element_path, catalog_with_responsible_parties
         )
     except Exception:
         assert True
@@ -115,7 +115,7 @@ def test_remove_failure(tmp_path: pathlib.Path):
     element_path = ElementPath('catalog.*')
     try:
         actual_remove_action, actual_catalog_removed_responsible_parties = RemoveCmd.remove(
-            element_path, Catalog, catalog_with_responsible_parties
+            element_path, catalog_with_responsible_parties
         )
     except Exception:
         assert True

tests/trestle/core/commands/replicate_test.py

@@ -120,13 +120,13 @@ def test_replicate_cmd_failures(testdata_dir, tmp_trestle_dir) -> None:
     # Force PermissionError:
     with mock.patch('trestle.core.commands.replicate.load_distributed') as load_distributed_mock:
         load_distributed_mock.side_effect = PermissionError
-        rc = ReplicateCmd.replicate_object('catalog', Catalog, args)
+        rc = ReplicateCmd.replicate_object('catalog', args)
         assert rc == 1
 
     # Force TrestleError:
     with mock.patch('trestle.core.commands.replicate.load_distributed') as load_distributed_mock:
         load_distributed_mock.side_effect = err.TrestleError('load_distributed_error')
-        rc = ReplicateCmd.replicate_object('catalog', Catalog, args)
+        rc = ReplicateCmd.replicate_object('catalog', args)
         assert rc == 1
 
     with mock.patch('trestle.core.commands.replicate.Plan.execute') as execute_mock:
@@ -135,13 +135,13 @@ def test_replicate_cmd_failures(testdata_dir, tmp_trestle_dir) -> None:
                 simulate_mock.side_effect = None
                 rollback_mock.side_effect = None
                 execute_mock.side_effect = err.TrestleError('execute_trestle_error')
-                rc = ReplicateCmd.replicate_object('catalog', Catalog, args)
+                rc = ReplicateCmd.replicate_object('catalog', args)
                 assert rc == 1
 
     # Force TrestleError in simulate:
     with mock.patch('trestle.core.commands.replicate.Plan.simulate') as simulate_mock:
         simulate_mock.side_effect = err.TrestleError('simulate_trestle_error')
-        rc = ReplicateCmd.replicate_object('catalog', Catalog, args)
+        rc = ReplicateCmd.replicate_object('catalog', args)
         assert rc == 1
 
 
@@ -171,5 +171,5 @@ def test_replicate_file_system(tmp_trestle_dir: Path) -> None:
     args = argparse.Namespace(trestle_root=tmp_trestle_dir, name='foo', output='bar', verbose=False)
     with mock.patch('trestle.core.commands.replicate.fs.get_trestle_project_root') as get_root_mock:
         get_root_mock.side_effect = [None]
-        rc = ReplicateCmd.replicate_object('catalog', Catalog, args)
+        rc = ReplicateCmd.replicate_object('catalog', args)
         assert rc == 1

tests/trestle/core/remote/cache_test.py

@@ -141,16 +141,16 @@ def test_https_fetcher(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch)
 def test_sftp_fetcher(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
     """Test the sftp fetcher."""
 
-    def ssh_load_host_keys_mock(*args, **kwargs):
+    def ssh_load_host_keys_mock():
         return
 
-    def ssh_connect_mock(*args, **kwargs):
+    def ssh_connect_mock():
         return
 
-    def open_sftp_mock(*args, **kwargs):
+    def open_sftp_mock():
         return
 
-    def sftp_get_mock(*args, **kwargs):
+    def sftp_get_mock():
         return
 
     uri = 'sftp://some.host//path/to/test.json'
@@ -168,7 +168,7 @@ def sftp_get_mock(*args, **kwargs):
 def test_sftp_fetcher_load_system_keys_fails(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
     """Test the sftp fetcher when SSHClient loading of system host keys fails."""
 
-    def ssh_load_system_host_keys_mock(*args, **kwargs):
+    def ssh_load_system_host_keys_mock():
         raise OSError('stuff')
 
     uri = 'sftp://username:[email protected]/path/to/file.json'
@@ -190,7 +190,7 @@ def test_sftp_fetcher_bad_ssh_key(tmp_trestle_dir: pathlib.Path, monkeypatch: Mo
 def test_sftp_fetcher_connect_fails(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
     """Test sftp during SSHClient connect failure."""
 
-    def ssh_connect_mock(*args, **kwargs):
+    def ssh_connect_mock():
         err.TrestleError('stuff')
 
     # Password given:
@@ -210,13 +210,13 @@ def ssh_connect_mock(*args, **kwargs):
 def test_sftp_fetcher_open_sftp_fails(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
     """Test the exception response during open_sftp failure."""
 
-    def ssh_load_host_keys_mock(*args, **kwargs):
+    def ssh_load_host_keys_mock():
         return
 
-    def ssh_connect_mock(*args, **kwargs):
+    def ssh_connect_mock():
         return
 
-    def open_sftp_mock(*args, **kwargs):
+    def open_sftp_mock():
         raise err.TrestleError('stuff')
 
     uri = 'sftp://username:[email protected]/path/to/file.json'
@@ -251,13 +251,13 @@ def getuser_mock(*args, **kwargs):
 def test_sftp_fetcher_get_fails(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
     """Test the sftp fetcher SFTPClient.get() failing."""
 
-    def load_host_keys_mock(*args, **kwargs):
+    def load_host_keys_mock():
         return
 
-    def connect_mock(*args, **kwargs):
+    def connect_mock():
         return
 
-    def get_mock(*args, **kwargs):
+    def get_mock():
         raise err.TrestleError('get fails')
 
     uri = 'sftp://username:[email protected]/path/to/file.json'

tests/trestle/utils/fs_test.py

@@ -374,8 +374,8 @@ def test_get_singular_alias() -> None:
     with pytest.raises(TrestleError):
         fs.get_singular_alias(alias_path='')
 
-    assert 'responsible-party' == fs.get_singular_alias(alias_path='catalog.metadata.responsible-parties')
-    assert 'property' == fs.get_singular_alias(alias_path='catalog.metadata.responsible-parties.*.props')
+    assert fs.get_singular_alias(alias_path='catalog.metadata.responsible-parties') == 'responsible-party'
+    assert fs.get_singular_alias(alias_path='catalog.metadata.responsible-parties.*.props') == 'property'
     assert 'responsible-party' == fs.get_singular_alias(alias_path='catalog.metadata.responsible-parties.*')
 
     assert 'role' == fs.get_singular_alias(alias_path='catalog.metadata.roles')

tests/trestle/utils/load_distributed_test.py

@@ -69,7 +69,7 @@ def test_load_list_group(testdata_dir, tmp_trestle_dir):
     shutil.rmtree(catalogs_dir)
     shutil.copytree(test_data_source, catalogs_dir)
 
-    actual_model_type, actual_model_alias, actual_groups = _load_list(catalog_dir / 'groups', tmp_trestle_dir)
+    actual_model_type, _, actual_groups = _load_list(catalog_dir / 'groups', tmp_trestle_dir)
 
     # load_list is expected to return a list of array, instead of an instance of Groups class
     expected_groups = (actual_model_type.oscal_read(testdata_dir / 'split_merge/load_distributed/groups.json')).__root__

trestle/core/base_model.py

@@ -118,7 +118,7 @@ def create_stripped_model_type(
         """
         if stripped_fields is not None and stripped_fields_aliases is not None:
             raise err.TrestleError('Either "stripped_fields" or "stripped_fields_aliases" need to be passed, not both.')
-        elif stripped_fields is None and stripped_fields_aliases is None:
+        if stripped_fields is None and stripped_fields_aliases is None:
             raise err.TrestleError('Exactly one of "stripped_fields" or "stripped_fields_aliases" must be provided')
 
         # create alias to field_name mapping
@@ -375,7 +375,7 @@ def copy_from(self, existing_oscal_object: 'OscalBaseModel') -> None:
 
         """
         recast_object = existing_oscal_object.copy_to(self.__class__)
-        for raw_field in self.__dict__.keys():
+        for raw_field in self.__dict__:
             self.__dict__[raw_field] = recast_object.__dict__[raw_field]
 
     @classmethod
@@ -408,7 +408,7 @@ class Foo(OscalBaseModel):
         When these cases exist we need special handling of the type information.
         """
         # Additional sanity check on field length
-        if len(cls.__fields__) == 1 and '__root__' in cls.__fields__.keys():
+        if len(cls.__fields__) == 1 and '__root__' in cls.__fields__:
             # This is now a __root__ key only model
             if is_collection_field_type(cls.__fields__['__root__'].outer_type_):
                 return True

trestle/core/catalog_interface.py

@@ -248,12 +248,7 @@ def find_string_in_control(self, control: cat.Control, seek_str: str) -> List[Tu
         return hits
 
     def write_catalog_as_markdown(
-        self,
-        md_path: pathlib.Path,
-        yaml_header: dict,
-        sections: Optional[Dict[str, str]],
-        all_details: bool,
-        responses: bool
+        self, md_path: pathlib.Path, yaml_header: dict, sections: Optional[Dict[str, str]], responses: bool
     ) -> None:
         """Write out the catalog controls from dict as markdown to the given directory."""
         control_io = ControlIO()

trestle/core/commands/assemble.py

@@ -17,13 +17,11 @@
 
 import argparse
 import logging
-from typing import Type
 
 from trestle.core import const
 from trestle.core.commands.command_docs import CommandPlusDocs
-from trestle.core.common_types import TopLevelOscalModel
 from trestle.core.models.actions import CreatePathAction, WriteFileAction
-from trestle.core.models.elements import Element, ElementPath
+from trestle.core.models.elements import Element
 from trestle.core.models.file_content_type import FileContentType
 from trestle.core.models.plans import Plan
 from trestle.utils import fs
@@ -47,11 +45,10 @@ def _init_arguments(self) -> None:
         )
 
     def _run(self, args: argparse.Namespace) -> int:
-        object_type = ElementPath(args.model).get_type()
-        return self.assemble_model(args.model, object_type, args)
+        return self.assemble_model(args.model, args)
 
     @classmethod
-    def assemble_model(cls, model_alias: str, object_type: Type[TopLevelOscalModel], args: argparse.Namespace) -> int:
+    def assemble_model(cls, model_alias: str, args: argparse.Namespace) -> int:
         """Assemble a top level OSCAL model within the trestle dist directory."""
         log.set_log_level_from_args(args)
         logger.info(f'Assembling models of type {model_alias}.')

trestle/core/commands/author/catalog.py

@@ -61,7 +61,7 @@ def generate_markdown(
         try:
             _, _, catalog = load_distributed(catalog_path, trestle_root)
             catalog_interface = CatalogInterface(catalog)
-            catalog_interface.write_catalog_as_markdown(markdown_path, {}, None, False, False)
+            catalog_interface.write_catalog_as_markdown(markdown_path, {}, None, False)
         except Exception as e:
             raise TrestleError(f'Error generating markdown for controls in {catalog_path}: {e}')
         return 0