Skip to content

Commit e5203c2

Browse files
Copilotdavidfstr
andauthored
Lint: Extend no-direct-filesystem-access-in-model (C9017) to ban pathlib imports
Co-authored-by: David Foster <david@dafoster.net>
1 parent fe05740 commit e5203c2

File tree

4 files changed

+86
-3
lines changed

4 files changed

+86
-3
lines changed

src/crystal/filesystem/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,9 @@ def copystat(self, src: str, dst: str) -> None:
309309
* OSError
310310
"""
311311
shutil.copystat(src, dst)
312+
313+
def as_uri(self, path: str) -> str:
314+
return pathlib.Path(path).as_uri()
312315

313316

314317
class S3Filesystem(_AbstractFilesystem):

src/crystal/lint/rules.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,11 @@ def visit_import(self, node: nodes.Import) -> None:
491491
for (module_name, _) in node.names:
492492
if module_name == 'pytest' and self._is_in_e2e_tests_layer(node):
493493
self.add_message('no-pytest-in-e2e-tests', node=node)
494+
495+
# import pathlib (in model layer)
496+
for (module_name, _) in node.names:
497+
if module_name == 'pathlib' and self._is_in_model_layer(node):
498+
self.add_message('no-direct-filesystem-access-in-model', node=node)
494499

495500
_BANNED_FILESYSTEM_IMPORTS = frozenset({
496501
'flush_renames_in_directory',
@@ -517,6 +522,10 @@ def visit_importfrom(self, node: nodes.ImportFrom) -> None:
517522
if name in self._BANNED_FILESYSTEM_IMPORTS:
518523
self.add_message('no-direct-filesystem-access-in-model', node=node)
519524
break
525+
526+
# from pathlib import ... (in model layer)
527+
if self._is_in_model_layer(node) and node.modname == 'pathlib':
528+
self.add_message('no-direct-filesystem-access-in-model', node=node)
520529

521530
# === Visit Attribute (wx constants + banned os attr reads) ===
522531

src/crystal/model/project.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import json
5353
import math
5454
import os
55-
import pathlib
5655
import re
5756
from send2trash import TrashPermissionError
5857
import shutil
@@ -786,9 +785,9 @@ def _apply_migrations(self,
786785
# HACK: Must also set icon location as a brittle absolute path
787786
# because Desktop Items doesn't understand the
788787
# 'metadata::custom-icon-name' GIO attribute.
789-
crystalproj_png_icon_url = pathlib.Path(
788+
crystalproj_png_icon_url = lfs.as_uri(
790789
resources_.get_filepath('docicon.png')
791-
).as_uri()
790+
)
792791
try:
793792
gio.set(self.path, 'metadata::custom-icon', crystalproj_png_icon_url)
794793
except gio.GioNotAvailable:

tests/test_lint_rules.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,44 @@ def test_other_method_is_allowed(self) -> None:
755755
_assert_no_message_emitted(code, 'no-direct-database-rollback')
756756

757757

758+
# === C9017: no-direct-filesystem-access-in-model ===
759+
760+
class TestNoDirectFilesystemAccessInModelInMemory:
761+
"""In-memory tests for the no-direct-filesystem-access-in-model rule (C9017)."""
762+
763+
def test_import_pathlib_in_model_is_flagged(self) -> None:
764+
code = dedent(
765+
'''\
766+
import pathlib
767+
'''
768+
)
769+
_assert_message_emitted_in_model(code, 'no-direct-filesystem-access-in-model')
770+
771+
def test_from_pathlib_import_in_model_is_flagged(self) -> None:
772+
code = dedent(
773+
'''\
774+
from pathlib import Path
775+
'''
776+
)
777+
_assert_message_emitted_in_model(code, 'no-direct-filesystem-access-in-model')
778+
779+
def test_import_pathlib_outside_model_is_allowed(self) -> None:
780+
code = dedent(
781+
'''\
782+
import pathlib
783+
'''
784+
)
785+
_assert_no_message_emitted(code, 'no-direct-filesystem-access-in-model')
786+
787+
def test_from_pathlib_import_outside_model_is_allowed(self) -> None:
788+
code = dedent(
789+
'''\
790+
from pathlib import Path
791+
'''
792+
)
793+
_assert_no_message_emitted(code, 'no-direct-filesystem-access-in-model')
794+
795+
758796
# === Utilities ===
759797

760798
# Counter for generating unique fake filenames
@@ -774,6 +812,20 @@ def _assert_message_emitted(code: str, message_id: str) -> None:
774812
)
775813

776814

815+
def _assert_message_emitted_in_model(code: str, message_id: str) -> None:
816+
"""
817+
Assert that checker emits the specified message given the specified code,
818+
simulating a file in the model layer (crystal/model/).
819+
820+
message_id should be the symbolic name (e.g. 'no-direct-filesystem-access-in-model').
821+
"""
822+
messages = _run_checker_on_code_in_model(code)
823+
message_ids = [msg.msg_id for msg in messages]
824+
assert message_id in message_ids, (
825+
f'Expected message {message_id} not found. Got: {message_ids}'
826+
)
827+
828+
777829
def _assert_no_message_emitted(code: str, message_id: str) -> None:
778830
"""
779831
Assert that checker does not emit the specified message given the specified code.
@@ -815,6 +867,26 @@ def _run_checker_on_code(code: str) -> list[pylint.testutils.MessageTest]:
815867
return test_case.linter.release_messages()
816868

817869

870+
def _run_checker_on_code_in_model(code: str) -> list[pylint.testutils.MessageTest]:
871+
"""
872+
Run a pylint checker on code, simulating a file in the model layer (crystal/model/).
873+
"""
874+
fake_filename = f'/src/crystal/model/test_file_{next(_fake_filename_counter)}.py'
875+
876+
code_lines = tuple(code.splitlines(keepends=True))
877+
with mock.patch('crystal.lint.rules._read_source_lines', return_value=code_lines):
878+
test_case = _InMemoryCheckerTestCase()
879+
test_case.CHECKER_CLASS = CrystalLintRules
880+
test_case.setup_method()
881+
882+
module = astroid.parse(code, module_name=fake_filename)
883+
module.file = fake_filename
884+
885+
test_case.walk(module)
886+
887+
return test_case.linter.release_messages()
888+
889+
818890
class _InMemoryCheckerTestCase(pylint.testutils.CheckerTestCase):
819891
"""
820892
Base class for in-memory pylint checker tests.

0 commit comments

Comments
 (0)