diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..7a60b85 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +__pycache__/ +*.pyc diff --git a/deps/tests/bump/main/output/output.txt b/deps/tests/bump/main/output/output.txt new file mode 100644 index 0000000..4414fc1 --- /dev/null +++ b/deps/tests/bump/main/output/output.txt @@ -0,0 +1 @@ +requirements.txt diff --git a/deps/tests/bump/main/output/requirements.txt b/deps/tests/bump/main/output/requirements.txt index 7c48c0d..d40fc67 100644 --- a/deps/tests/bump/main/output/requirements.txt +++ b/deps/tests/bump/main/output/requirements.txt @@ -1 +1 @@ -click==8.2.1 +click==8.3.1 diff --git a/exceptions/bare_raise.py b/exceptions/bare_raise.py new file mode 100644 index 0000000..ce9389c --- /dev/null +++ b/exceptions/bare_raise.py @@ -0,0 +1,41 @@ +import ast +import sys + + +class BareRaiseChecker(ast.NodeVisitor): + def __init__(self, filename): + self.filename = filename + self.except_depth = 0 + self.issues = [] + + def visit_ExceptHandler(self, node): + self.except_depth += 1 + self.generic_visit(node) + self.except_depth -= 1 + + def visit_Raise(self, node): + if node.exc is None and self.except_depth == 0: + self.issues.append((node.lineno, node.col_offset)) + self.generic_visit(node) + + +def main(filenames): + exit_status = 0 + for filename in filenames: + with open(filename, "rb") as f: + source = f.read() + try: + tree = ast.parse(source, filename=filename) + except SyntaxError: + continue + checker = BareRaiseChecker(filename) + checker.visit(tree) + for lineno, col in checker.issues: + print(f"{filename}:{lineno}:{col}: bare 'raise' outside except block") + exit_status = 99 + + sys.exit(exit_status) + + +if __name__ == "__main__": + main(sys.argv[1:]) diff --git a/exceptions/ick.toml b/exceptions/ick.toml new file mode 100644 index 0000000..b801887 --- /dev/null +++ b/exceptions/ick.toml @@ -0,0 +1,5 @@ +[[rule]] +name = "bare_raise" +impl = "python" +scope = "file" +inputs = ["*.py"] diff --git a/exceptions/tests/bare_raise/main/input/demo.py b/exceptions/tests/bare_raise/main/input/demo.py new file mode 100644 index 0000000..4bff264 --- /dev/null +++ b/exceptions/tests/bare_raise/main/input/demo.py @@ -0,0 +1,28 @@ +def bad(): + raise # flagged: bare raise outside except + + +def nested_bad(): + if True: + raise # flagged: still outside except + + +def good_reraise(): + try: + pass + except Exception: + raise # ok: re-raises inside except + + +def good_nested_except(): + try: + pass + except TypeError: + try: + pass + except ValueError: + raise # ok: inside nested except + + +def good_raise_with_arg(): + raise ValueError("something") # ok: not bare diff --git a/exceptions/tests/bare_raise/main/output/output.txt b/exceptions/tests/bare_raise/main/output/output.txt new file mode 100644 index 0000000..82bab55 --- /dev/null +++ b/exceptions/tests/bare_raise/main/output/output.txt @@ -0,0 +1,2 @@ +demo.py:2:4: bare 'raise' outside except block +demo.py:7:8: bare 'raise' outside except block diff --git a/fstrings/__init__.py b/fstrings/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/fstrings/fstring_repr.py b/fstrings/fstring_repr.py new file mode 100644 index 0000000..2e82961 --- /dev/null +++ b/fstrings/fstring_repr.py @@ -0,0 +1,72 @@ +"""Ick rule: replace {repr(...)} in f-strings with {...!r}.""" +import sys +from pathlib import Path + +import libcst as cst +from fixit import LintRule, Invalid, Valid + + +class FstringReprRule(LintRule): + MESSAGE = "Use {x!r} instead of {repr(x)} in f-strings" + + VALID = [ + Valid("f'{x!r}'"), + Valid("f'{x}'"), + Valid("f'{repr}'"), + Valid("f'{repr(x, y)}'"), + ] + INVALID = [ + Invalid( + "f'{repr(x)}'", + expected_replacement="f'{x!r}'", + ), + Invalid( + "f'value is {repr(obj)} here'", + expected_replacement="f'value is {obj!r} here'", + ), + Invalid( + "f'{repr(x):10}'", + expected_replacement="f'{x!r:10}'", + ), + ] + + def visit_FormattedStringExpression(self, node: cst.FormattedStringExpression) -> None: + expr = node.expression + if not ( + isinstance(expr, cst.Call) + and isinstance(expr.func, cst.Name) + and expr.func.value == 'repr' + and len(expr.args) == 1 + and not isinstance(expr.args[0].value, cst.StarredElement) + and expr.args[0].keyword is None + and expr.args[0].star == '' + ): + return + if node.conversion is not None: + return + inner = expr.args[0].value + new_node = node.with_changes(expression=inner, conversion='r') + self.report(node, replacement=new_node) + + +def main(): + from fixit.api import fixit_bytes, generate_config + from fixit.ftypes import Options, QualifiedRule + + options = Options(rules=[QualifiedRule('fstrings.fstring_repr', 'FstringReprRule')]) + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + config = generate_config(path, options=options) + gen = fixit_bytes(path, src, config=config, autofix=True) + try: + while True: + next(gen) + except StopIteration as e: + if e.value is not None: + path.write_bytes(e.value) + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/fstrings/ick.toml b/fstrings/ick.toml new file mode 100644 index 0000000..c0dc376 --- /dev/null +++ b/fstrings/ick.toml @@ -0,0 +1,6 @@ +[[rule]] +name = "fstring_repr" +impl = "python" +scope = "file" +inputs = ["*.py"] +deps = ["fixit", "libcst"] diff --git a/fstrings/test_fstring_repr.py b/fstrings/test_fstring_repr.py new file mode 100644 index 0000000..d0250ac --- /dev/null +++ b/fstrings/test_fstring_repr.py @@ -0,0 +1,4 @@ +from fixit.testing import add_lint_rule_tests_to_module +from python.fstrings.fstring_repr import FstringReprRule + +add_lint_rule_tests_to_module(globals(), [FstringReprRule]) diff --git a/fstrings/tests/fstring_repr/basic/input/example.py b/fstrings/tests/fstring_repr/basic/input/example.py new file mode 100644 index 0000000..b05e3af --- /dev/null +++ b/fstrings/tests/fstring_repr/basic/input/example.py @@ -0,0 +1,4 @@ +x = 'hello' +print(f'value is {repr(x)}') +print(f'{repr(x):10}') +print(f'{x!r}') # already correct, no change diff --git a/fstrings/tests/fstring_repr/basic/output/example.py b/fstrings/tests/fstring_repr/basic/output/example.py new file mode 100644 index 0000000..f2af6c8 --- /dev/null +++ b/fstrings/tests/fstring_repr/basic/output/example.py @@ -0,0 +1,4 @@ +x = 'hello' +print(f'value is {x!r}') +print(f'{x!r:10}') +print(f'{x!r}') # already correct, no change diff --git a/gha/tests/upgrade/main/output/.github/workflows/build.yml b/gha/tests/upgrade/main/output/.github/workflows/build.yml index 0360043..f4c6744 100644 --- a/gha/tests/upgrade/main/output/.github/workflows/build.yml +++ b/gha/tests/upgrade/main/output/.github/workflows/build.yml @@ -23,13 +23,13 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: "actions/checkout@v6" - name: Set Up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 + uses: "actions/setup-python@v6" with: python-version: ${{ matrix.python-version }} - - uses: "astral-sh/setup-uv@v6" + uses: "astral-sh/setup-uv@v7" - name: Install run: | uv pip install -e .[test] @@ -47,18 +47,20 @@ jobs: needs: test runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - + uses: "actions/checkout@v6" + - + uses: "actions/setup-python@v6" with: python-version: "3.12" - - uses: "astral-sh/setup-uv@v6" + uses: "astral-sh/setup-uv@v7" - name: Install run: uv pip install build - name: Build run: python -m build - name: Upload - uses: actions/upload-artifact@v4 + uses: "actions/upload-artifact@v7" with: name: sdist path: dist @@ -70,7 +72,8 @@ jobs: permissions: id-token: write steps: - - uses: actions/download-artifact@v4 + - + uses: "actions/download-artifact@v8" with: name: sdist path: dist diff --git a/gha/upgrade.toml b/gha/upgrade.toml index 2cf2aed..50df1c8 100644 --- a/gha/upgrade.toml +++ b/gha/upgrade.toml @@ -1,7 +1,7 @@ # Generated by upgrade_upgrade.py -"actions/checkout" = "v4" -"actions/setup-python" = "v5" -"actions/upload-artifact" = "v4" -"actions/download-artifact" = "v4" -"astral-sh/setup-uv" = "v6" +"actions/checkout" = "v6" +"actions/setup-python" = "v6" +"actions/upload-artifact" = "v7" +"actions/download-artifact" = "v8" +"astral-sh/setup-uv" = "v7" diff --git a/gitignores.py b/gitignores.py new file mode 100644 index 0000000..a14a85f --- /dev/null +++ b/gitignores.py @@ -0,0 +1,40 @@ +"""Ick rule: ensure .gitignore at repo root contains __pycache__ and *.pyc entries.""" +import re +import sys +from pathlib import Path + +REQUIRED_EXACT = ['__pycache__/'] +REQUIRED_PATTERNS = [re.compile(r'^(\*\*/)?(\*\.py\[?c[do\]]*)$')] + + +def _line_matches_any_pattern(line: str) -> bool: + return any(p.match(line) for p in REQUIRED_PATTERNS) + + +def check(root: Path) -> int: + gitignore = root / '.gitignore' + lines = gitignore.read_text().splitlines(True) if gitignore.exists() else [] + stripped = [l.rstrip('\n') for l in lines] + + missing = [] + for line in REQUIRED_EXACT: + if line not in stripped: + missing.append(line + "\n") + if not any(_line_matches_any_pattern(l) for l in stripped): + missing.append('*.pyc\n') + + if missing: + with gitignore.open('a') as f: + if lines and not lines[-1].endswith('\n'): + f.write('\n') + f.writelines(missing) + + return 0 + + +def main(): + sys.exit(check(Path('.'))) + + +if __name__ == '__main__': + main() diff --git a/ick.toml b/ick.toml index 6456a54..a3a2492 100644 --- a/ick.toml +++ b/ick.toml @@ -1,3 +1,8 @@ [[ruleset]] path = "." prefix = "" + +[[rule]] +name = "gitignores" +scope = "repo" +impl = "python" diff --git a/libs/appdirs/appdirs_to_platformdirs.py b/libs/appdirs/appdirs_to_platformdirs.py new file mode 100644 index 0000000..f5497a3 --- /dev/null +++ b/libs/appdirs/appdirs_to_platformdirs.py @@ -0,0 +1,109 @@ +"""Ick rule: replace appdirs with platformdirs in imports and dependency files.""" +import re +import sys +from pathlib import Path + +import libcst as cst +from fixit import Invalid, LintRule, Valid +from fixit.api import fixit_bytes, generate_config +from fixit.ftypes import Options, QualifiedRule + +_SKIP_DIRS = frozenset({ + '.git', '.venv', 'venv', 'env', '__pycache__', '.tox', 'node_modules', 'dist', 'build', +}) +_DEP_GLOBS = ['requirements*.txt', 'requirements/**/*.txt', 'pyproject.toml', 'setup.cfg'] +_APPDIRS_RE = re.compile(r'\bappdirs\b', re.IGNORECASE) + + +def _has_appdirs_root(node: cst.BaseExpression) -> bool: + while isinstance(node, cst.Attribute): + node = node.value + return isinstance(node, cst.Name) and node.value == 'appdirs' + + +def _replace_root(node: cst.BaseExpression) -> cst.BaseExpression: + if isinstance(node, cst.Name): + return node.with_changes(value='platformdirs') + if isinstance(node, cst.Attribute): + return node.with_changes(value=_replace_root(node.value)) + return node + + +class AppdirsRule(LintRule): + MESSAGE = "Use platformdirs instead of appdirs" + + VALID = [ + Valid("import platformdirs"), + Valid("from platformdirs import user_data_dir"), + ] + INVALID = [ + Invalid( + "import appdirs", + expected_replacement="import platformdirs", + ), + Invalid( + "from appdirs import user_data_dir", + expected_replacement="from platformdirs import user_data_dir", + ), + Invalid( + "import appdirs as ad", + expected_replacement="import platformdirs as ad", + ), + ] + + def visit_Import(self, node: cst.Import) -> None: + new_aliases = [] + changed = False + for alias in node.names: + if _has_appdirs_root(alias.name): + new_aliases.append(alias.with_changes(name=_replace_root(alias.name))) + changed = True + else: + new_aliases.append(alias) + if changed: + self.report(node, replacement=node.with_changes(names=new_aliases)) + + def visit_ImportFrom(self, node: cst.ImportFrom) -> None: + if node.module is None or not _has_appdirs_root(node.module): + return + self.report(node, replacement=node.with_changes(module=_replace_root(node.module))) + + +def _fix_python_files(root: Path) -> None: + options = Options(rules=[QualifiedRule('libs.appdirs.appdirs_to_platformdirs', 'AppdirsRule')]) + for path in root.rglob('*.py'): + if any(part in _SKIP_DIRS for part in path.parts): + continue + src = path.read_bytes() + config = generate_config(path, options=options) + gen = fixit_bytes(path, src, config=config, autofix=True) + try: + while True: + next(gen) + except StopIteration as e: + if e.value is not None: + path.write_bytes(e.value) + + +def _fix_dep_files(root: Path) -> None: + seen = set() + for pattern in _DEP_GLOBS: + for path in root.glob(pattern): + if path in seen: + continue + seen.add(path) + text = path.read_text() + new_text = _APPDIRS_RE.sub('platformdirs', text) + if new_text != text: + path.write_text(new_text) + + +def main(): + root = Path('.') + _fix_python_files(root) + _fix_dep_files(root) + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/libs/appdirs/ick.toml b/libs/appdirs/ick.toml new file mode 100644 index 0000000..0891fb4 --- /dev/null +++ b/libs/appdirs/ick.toml @@ -0,0 +1,5 @@ +[[rule]] +name = "appdirs_to_platformdirs" +impl = "python" +scope = "project" +deps = ["fixit", "libcst"] diff --git a/libs/appdirs/tests/appdirs_to_platformdirs/already_platformdirs/input/mypackage.py b/libs/appdirs/tests/appdirs_to_platformdirs/already_platformdirs/input/mypackage.py new file mode 100644 index 0000000..4aa19bd --- /dev/null +++ b/libs/appdirs/tests/appdirs_to_platformdirs/already_platformdirs/input/mypackage.py @@ -0,0 +1,3 @@ +from platformdirs import user_data_dir + +DATA_DIR = user_data_dir('myapp', 'myorg') diff --git a/libs/appdirs/tests/appdirs_to_platformdirs/already_platformdirs/output/mypackage.py b/libs/appdirs/tests/appdirs_to_platformdirs/already_platformdirs/output/mypackage.py new file mode 100644 index 0000000..4aa19bd --- /dev/null +++ b/libs/appdirs/tests/appdirs_to_platformdirs/already_platformdirs/output/mypackage.py @@ -0,0 +1,3 @@ +from platformdirs import user_data_dir + +DATA_DIR = user_data_dir('myapp', 'myorg') diff --git a/libs/appdirs/tests/appdirs_to_platformdirs/dep_files/input/pyproject.toml b/libs/appdirs/tests/appdirs_to_platformdirs/dep_files/input/pyproject.toml new file mode 100644 index 0000000..9096eff --- /dev/null +++ b/libs/appdirs/tests/appdirs_to_platformdirs/dep_files/input/pyproject.toml @@ -0,0 +1,6 @@ +[project] +name = "myproject" +dependencies = [ + "appdirs>=1.4.4", + "requests", +] diff --git a/libs/appdirs/tests/appdirs_to_platformdirs/dep_files/output/pyproject.toml b/libs/appdirs/tests/appdirs_to_platformdirs/dep_files/output/pyproject.toml new file mode 100644 index 0000000..be666ca --- /dev/null +++ b/libs/appdirs/tests/appdirs_to_platformdirs/dep_files/output/pyproject.toml @@ -0,0 +1,6 @@ +[project] +name = "myproject" +dependencies = [ + "platformdirs>=1.4.4", + "requests", +] diff --git a/libs/appdirs/tests/appdirs_to_platformdirs/python_imports/input/mypackage.py b/libs/appdirs/tests/appdirs_to_platformdirs/python_imports/input/mypackage.py new file mode 100644 index 0000000..b30dcc4 --- /dev/null +++ b/libs/appdirs/tests/appdirs_to_platformdirs/python_imports/input/mypackage.py @@ -0,0 +1,5 @@ +import appdirs +from appdirs import user_data_dir, user_cache_dir + +DATA_DIR = user_data_dir('myapp', 'myorg') +CACHE_DIR = user_cache_dir('myapp') diff --git a/libs/appdirs/tests/appdirs_to_platformdirs/python_imports/output/mypackage.py b/libs/appdirs/tests/appdirs_to_platformdirs/python_imports/output/mypackage.py new file mode 100644 index 0000000..52798c4 --- /dev/null +++ b/libs/appdirs/tests/appdirs_to_platformdirs/python_imports/output/mypackage.py @@ -0,0 +1,5 @@ +import platformdirs +from platformdirs import user_data_dir, user_cache_dir + +DATA_DIR = user_data_dir('myapp', 'myorg') +CACHE_DIR = user_cache_dir('myapp') diff --git a/libs/click/__init__.py b/libs/click/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/libs/click/ick.toml b/libs/click/ick.toml new file mode 100644 index 0000000..9128b85 --- /dev/null +++ b/libs/click/ick.toml @@ -0,0 +1,12 @@ +[[rule]] +name = "pass_context_location" +impl = "python" +scope = "file" +inputs = ["*.py"] +deps = ["fixit", "libcst"] + +[[rule]] +name = "version_option" +impl = "python" +scope = "project" +deps = ["libcst"] diff --git a/libs/click/pass_context_location.py b/libs/click/pass_context_location.py new file mode 100644 index 0000000..a9e344c --- /dev/null +++ b/libs/click/pass_context_location.py @@ -0,0 +1,94 @@ +"""Ick rule: @click.pass_context must be the last (innermost) decorator.""" +import sys +from pathlib import Path + +import libcst as cst +from fixit import Invalid, LintRule, Valid + + +def _is_pass_context(decorator: cst.Decorator) -> bool: + dec = decorator.decorator + if isinstance(dec, cst.Attribute): + return ( + isinstance(dec.value, cst.Name) + and dec.value.value == 'click' + and dec.attr.value == 'pass_context' + ) + if isinstance(dec, cst.Name): + return dec.value == 'pass_context' + return False + + +class PassContextLocationRule(LintRule): + MESSAGE = "@click.pass_context must be the last decorator" + + VALID = [ + Valid( + """ + @click.command() + @click.option('--name') + @click.pass_context + def cmd(ctx, name): pass + """ + ), + Valid( + """ + @click.command() + @click.pass_context + def cmd(ctx): pass + """ + ), + ] + INVALID = [ + Invalid( + """ + @click.command() + @click.pass_context + @click.option('--name') + def cmd(ctx, name): pass + """, + expected_replacement=""" + @click.command() + @click.option('--name') + @click.pass_context + def cmd(ctx, name): pass + """, + ), + ] + + def visit_FunctionDef(self, node: cst.FunctionDef) -> None: + decs = node.decorators + if len(decs) < 2: + return + pc_indices = [i for i, d in enumerate(decs) if _is_pass_context(d)] + if not pc_indices: + return + pc_idx = pc_indices[0] + if pc_idx == len(decs) - 1: + return + pc_dec = decs[pc_idx] + new_decs = list(decs[:pc_idx]) + list(decs[pc_idx + 1:]) + [pc_dec] + self.report(node, replacement=node.with_changes(decorators=new_decs)) + + +def main(): + from fixit.api import fixit_bytes, generate_config + from fixit.ftypes import Options, QualifiedRule + + options = Options(rules=[QualifiedRule('libs.click.pass_context_location', 'PassContextLocationRule')]) + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + config = generate_config(path, options=options) + gen = fixit_bytes(path, src, config=config, autofix=True) + try: + while True: + next(gen) + except StopIteration as e: + if e.value is not None: + path.write_bytes(e.value) + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/libs/click/test_pass_context_location.py b/libs/click/test_pass_context_location.py new file mode 100644 index 0000000..3402926 --- /dev/null +++ b/libs/click/test_pass_context_location.py @@ -0,0 +1,4 @@ +from fixit.testing import add_lint_rule_tests_to_module +from python.click.pass_context_location import PassContextLocationRule + +add_lint_rule_tests_to_module(globals(), [PassContextLocationRule]) diff --git a/libs/click/tests/pass_context_location/wrong_order/input/example.py b/libs/click/tests/pass_context_location/wrong_order/input/example.py new file mode 100644 index 0000000..efc700b --- /dev/null +++ b/libs/click/tests/pass_context_location/wrong_order/input/example.py @@ -0,0 +1,7 @@ +import click + +@click.command() +@click.pass_context +@click.option('--name') +def cmd(ctx, name): + pass diff --git a/libs/click/tests/pass_context_location/wrong_order/output/example.py b/libs/click/tests/pass_context_location/wrong_order/output/example.py new file mode 100644 index 0000000..ee125d5 --- /dev/null +++ b/libs/click/tests/pass_context_location/wrong_order/output/example.py @@ -0,0 +1,7 @@ +import click + +@click.command() +@click.option('--name') +@click.pass_context +def cmd(ctx, name): + pass diff --git a/libs/click/tests/version_option/add_from_init/input/mypkg/__init__.py b/libs/click/tests/version_option/add_from_init/input/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/add_from_init/input/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/add_from_init/input/mypkg/cli.py b/libs/click/tests/version_option/add_from_init/input/mypkg/cli.py new file mode 100644 index 0000000..94fbe6f --- /dev/null +++ b/libs/click/tests/version_option/add_from_init/input/mypkg/cli.py @@ -0,0 +1,6 @@ +import click + + +@click.group() +def main(): + pass diff --git a/libs/click/tests/version_option/add_from_init/output/mypkg/__init__.py b/libs/click/tests/version_option/add_from_init/output/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/add_from_init/output/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/add_from_init/output/mypkg/cli.py b/libs/click/tests/version_option/add_from_init/output/mypkg/cli.py new file mode 100644 index 0000000..022f907 --- /dev/null +++ b/libs/click/tests/version_option/add_from_init/output/mypkg/cli.py @@ -0,0 +1,7 @@ +import click + + +@click.group() +@click.version_option() +def main(): + pass diff --git a/libs/click/tests/version_option/add_from_version_py/input/mypkg/__init__.py b/libs/click/tests/version_option/add_from_version_py/input/mypkg/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/libs/click/tests/version_option/add_from_version_py/input/mypkg/main.py b/libs/click/tests/version_option/add_from_version_py/input/mypkg/main.py new file mode 100644 index 0000000..468ad36 --- /dev/null +++ b/libs/click/tests/version_option/add_from_version_py/input/mypkg/main.py @@ -0,0 +1,6 @@ +import click + + +@click.command() +def main(): + pass diff --git a/libs/click/tests/version_option/add_from_version_py/input/mypkg/version.py b/libs/click/tests/version_option/add_from_version_py/input/mypkg/version.py new file mode 100644 index 0000000..55e4709 --- /dev/null +++ b/libs/click/tests/version_option/add_from_version_py/input/mypkg/version.py @@ -0,0 +1 @@ +__version__ = "2.3.0" diff --git a/libs/click/tests/version_option/add_from_version_py/output/mypkg/__init__.py b/libs/click/tests/version_option/add_from_version_py/output/mypkg/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/libs/click/tests/version_option/add_from_version_py/output/mypkg/main.py b/libs/click/tests/version_option/add_from_version_py/output/mypkg/main.py new file mode 100644 index 0000000..010675d --- /dev/null +++ b/libs/click/tests/version_option/add_from_version_py/output/mypkg/main.py @@ -0,0 +1,7 @@ +import click + + +@click.command() +@click.version_option() +def main(): + pass diff --git a/libs/click/tests/version_option/add_from_version_py/output/mypkg/version.py b/libs/click/tests/version_option/add_from_version_py/output/mypkg/version.py new file mode 100644 index 0000000..55e4709 --- /dev/null +++ b/libs/click/tests/version_option/add_from_version_py/output/mypkg/version.py @@ -0,0 +1 @@ +__version__ = "2.3.0" diff --git a/libs/click/tests/version_option/already_has/input/mypkg/__init__.py b/libs/click/tests/version_option/already_has/input/mypkg/__init__.py new file mode 100644 index 0000000..528787c --- /dev/null +++ b/libs/click/tests/version_option/already_has/input/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "3.0.0" diff --git a/libs/click/tests/version_option/already_has/input/mypkg/cli.py b/libs/click/tests/version_option/already_has/input/mypkg/cli.py new file mode 100644 index 0000000..022f907 --- /dev/null +++ b/libs/click/tests/version_option/already_has/input/mypkg/cli.py @@ -0,0 +1,7 @@ +import click + + +@click.group() +@click.version_option() +def main(): + pass diff --git a/libs/click/tests/version_option/already_has/output/mypkg/__init__.py b/libs/click/tests/version_option/already_has/output/mypkg/__init__.py new file mode 100644 index 0000000..528787c --- /dev/null +++ b/libs/click/tests/version_option/already_has/output/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "3.0.0" diff --git a/libs/click/tests/version_option/already_has/output/mypkg/cli.py b/libs/click/tests/version_option/already_has/output/mypkg/cli.py new file mode 100644 index 0000000..022f907 --- /dev/null +++ b/libs/click/tests/version_option/already_has/output/mypkg/cli.py @@ -0,0 +1,7 @@ +import click + + +@click.group() +@click.version_option() +def main(): + pass diff --git a/libs/click/tests/version_option/from_import_only/input/mypkg/__init__.py b/libs/click/tests/version_option/from_import_only/input/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/from_import_only/input/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/from_import_only/input/mypkg/cli.py b/libs/click/tests/version_option/from_import_only/input/mypkg/cli.py new file mode 100644 index 0000000..7d25cf0 --- /dev/null +++ b/libs/click/tests/version_option/from_import_only/input/mypkg/cli.py @@ -0,0 +1,6 @@ +from click import group + + +@group() +def main(): + pass diff --git a/libs/click/tests/version_option/from_import_only/output/mypkg/__init__.py b/libs/click/tests/version_option/from_import_only/output/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/from_import_only/output/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/from_import_only/output/mypkg/cli.py b/libs/click/tests/version_option/from_import_only/output/mypkg/cli.py new file mode 100644 index 0000000..96cc7fc --- /dev/null +++ b/libs/click/tests/version_option/from_import_only/output/mypkg/cli.py @@ -0,0 +1,8 @@ +from click import group +from click import version_option + + +@group() +@version_option() +def main(): + pass diff --git a/libs/click/tests/version_option/import_as_alias/input/mypkg/__init__.py b/libs/click/tests/version_option/import_as_alias/input/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/import_as_alias/input/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/import_as_alias/input/mypkg/cli.py b/libs/click/tests/version_option/import_as_alias/input/mypkg/cli.py new file mode 100644 index 0000000..7950ad9 --- /dev/null +++ b/libs/click/tests/version_option/import_as_alias/input/mypkg/cli.py @@ -0,0 +1,6 @@ +import click as cli + + +@cli.group() +def main(): + pass diff --git a/libs/click/tests/version_option/import_as_alias/output/mypkg/__init__.py b/libs/click/tests/version_option/import_as_alias/output/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/import_as_alias/output/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/import_as_alias/output/mypkg/cli.py b/libs/click/tests/version_option/import_as_alias/output/mypkg/cli.py new file mode 100644 index 0000000..b6938a9 --- /dev/null +++ b/libs/click/tests/version_option/import_as_alias/output/mypkg/cli.py @@ -0,0 +1,7 @@ +import click as cli + + +@cli.group() +@cli.version_option() +def main(): + pass diff --git a/libs/click/tests/version_option/no_click/input/mypkg/__init__.py b/libs/click/tests/version_option/no_click/input/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/no_click/input/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/no_click/input/mypkg/cli.py b/libs/click/tests/version_option/no_click/input/mypkg/cli.py new file mode 100644 index 0000000..f35d533 --- /dev/null +++ b/libs/click/tests/version_option/no_click/input/mypkg/cli.py @@ -0,0 +1,6 @@ +import argparse + + +def main(): + parser = argparse.ArgumentParser() + parser.parse_args() diff --git a/libs/click/tests/version_option/no_click/output/mypkg/__init__.py b/libs/click/tests/version_option/no_click/output/mypkg/__init__.py new file mode 100644 index 0000000..5becc17 --- /dev/null +++ b/libs/click/tests/version_option/no_click/output/mypkg/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/libs/click/tests/version_option/no_click/output/mypkg/cli.py b/libs/click/tests/version_option/no_click/output/mypkg/cli.py new file mode 100644 index 0000000..f35d533 --- /dev/null +++ b/libs/click/tests/version_option/no_click/output/mypkg/cli.py @@ -0,0 +1,6 @@ +import argparse + + +def main(): + parser = argparse.ArgumentParser() + parser.parse_args() diff --git a/libs/click/tests/version_option/no_version_info/input/mypkg/__init__.py b/libs/click/tests/version_option/no_version_info/input/mypkg/__init__.py new file mode 100644 index 0000000..6377a2a --- /dev/null +++ b/libs/click/tests/version_option/no_version_info/input/mypkg/__init__.py @@ -0,0 +1 @@ +# no __version__ here diff --git a/libs/click/tests/version_option/no_version_info/input/mypkg/cli.py b/libs/click/tests/version_option/no_version_info/input/mypkg/cli.py new file mode 100644 index 0000000..468ad36 --- /dev/null +++ b/libs/click/tests/version_option/no_version_info/input/mypkg/cli.py @@ -0,0 +1,6 @@ +import click + + +@click.command() +def main(): + pass diff --git a/libs/click/tests/version_option/no_version_info/output/mypkg/__init__.py b/libs/click/tests/version_option/no_version_info/output/mypkg/__init__.py new file mode 100644 index 0000000..6377a2a --- /dev/null +++ b/libs/click/tests/version_option/no_version_info/output/mypkg/__init__.py @@ -0,0 +1 @@ +# no __version__ here diff --git a/libs/click/tests/version_option/no_version_info/output/mypkg/cli.py b/libs/click/tests/version_option/no_version_info/output/mypkg/cli.py new file mode 100644 index 0000000..468ad36 --- /dev/null +++ b/libs/click/tests/version_option/no_version_info/output/mypkg/cli.py @@ -0,0 +1,6 @@ +import click + + +@click.command() +def main(): + pass diff --git a/libs/click/version_option.py b/libs/click/version_option.py new file mode 100644 index 0000000..bf60000 --- /dev/null +++ b/libs/click/version_option.py @@ -0,0 +1,208 @@ +"""Ick rule: CLI entry points should use @click.version_option.""" +import re +import sys +from pathlib import Path + +import libcst as cst + +_TARGET_FILENAMES = frozenset({'cli.py', 'cmdline.py', 'main.py', '__main__.py'}) +_SKIP_DIRS = frozenset({ + '.git', '.venv', 'venv', 'env', '__pycache__', '.tox', 'node_modules', 'dist', 'build', +}) +_VERSION_RE = re.compile(r'^__version__\s*=', re.MULTILINE) + + +def _decorator_func_name(decorator: cst.Decorator) -> str: + """Return the callable part as a dotted string, e.g. 'click.group', 'version_option'.""" + node = decorator.decorator + if isinstance(node, cst.Call): + node = node.func + parts = [] + while isinstance(node, cst.Attribute): + parts.append(node.attr.value) + node = node.value + if isinstance(node, cst.Name): + parts.append(node.value) + return '.'.join(reversed(parts)) + + +class _Transformer(cst.CSTTransformer): + """Analyze click imports/usage and add @version_option() where missing.""" + + def __init__(self, version_info_available: bool) -> None: + self._version_info_available = version_info_available + # Collected during traversal + self._click_alias: str | None = None # module alias ('click', 'myalias', …) + self._imported_names: set[str] = set() # names from `from click import …` + self._has_any_click_import = False + self._has_entry_point = False + self._has_version_option = False + # State during transformation + self._done = False + self._added_decorator = False + + # ── Import analysis ──────────────────────────────────────────────────────── + + def visit_Import(self, node: cst.Import) -> bool | None: + if isinstance(node.names, cst.ImportStar): + return None + for alias in node.names: + if not (isinstance(alias.name, cst.Name) and alias.name.value == 'click'): + continue + self._has_any_click_import = True + if alias.asname is not None and isinstance(alias.asname, cst.AsName): + name_node = alias.asname.name + if isinstance(name_node, cst.Name): + self._click_alias = name_node.value + else: + self._click_alias = 'click' + return None + + def visit_ImportFrom(self, node: cst.ImportFrom) -> bool | None: + mod = node.module + root = mod + while isinstance(root, cst.Attribute): + root = root.value + if not (isinstance(root, cst.Name) and root.value == 'click'): + return None + self._has_any_click_import = True + if not isinstance(node.names, cst.ImportStar): + for alias in node.names: + if isinstance(alias.name, cst.Name): + self._imported_names.add(alias.name.value) + return None + + # ── Entry-point detection ────────────────────────────────────────────────── + + def _is_click_entry_point(self, name: str) -> bool: + parts = name.split('.') + last = parts[-1] + if last not in ('command', 'group'): + return False + # Qualified: @click.command() / @myalias.command() + if len(parts) == 2 and self._click_alias and parts[0] == self._click_alias: + return True + # Unqualified via from-import: @command() / @group() + if len(parts) == 1 and last in self._imported_names: + return True + return False + + def visit_FunctionDef(self, node: cst.FunctionDef) -> bool | None: + for dec in node.decorators: + name = _decorator_func_name(dec) + if self._is_click_entry_point(name): + self._has_entry_point = True + if 'version_option' in name: + self._has_version_option = True + return None + + # ── Transformation ───────────────────────────────────────────────────────── + + def _should_fix(self) -> bool: + return ( + self._has_any_click_import + and self._has_entry_point + and not self._has_version_option + and self._version_info_available + ) + + def leave_FunctionDef( + self, original_node: cst.FunctionDef, updated_node: cst.FunctionDef + ) -> cst.FunctionDef: + if self._done or not self._should_fix(): + return updated_node + decs = updated_node.decorators + cmd_idx = next( + (i for i, d in enumerate(decs) + if self._is_click_entry_point(_decorator_func_name(d))), + None, + ) + if cmd_idx is None: + return updated_node + + if self._click_alias: + expr = f"{self._click_alias}.version_option()" + else: + expr = "version_option()" + + cmd_dec = decs[cmd_idx] + new_dec = cst.Decorator( + decorator=cst.parse_expression(expr), + leading_lines=(), + whitespace_after_at=cst.SimpleWhitespace(""), + trailing_whitespace=cmd_dec.trailing_whitespace, + ) + new_decs = list(decs[:cmd_idx + 1]) + [new_dec] + list(decs[cmd_idx + 1:]) + self._done = True + self._added_decorator = True + return updated_node.with_changes(decorators=new_decs) + + def leave_Module( + self, original_node: cst.Module, updated_node: cst.Module + ) -> cst.Module: + # Only add a new from-import when there's no module alias and we added a decorator. + if not self._added_decorator or self._click_alias is not None: + return updated_node + # Insert `from click import version_option` after the last import statement. + last_import_idx = -1 + for i, stmt in enumerate(updated_node.body): + if isinstance(stmt, cst.SimpleStatementLine): + if any(isinstance(s, (cst.Import, cst.ImportFrom)) for s in stmt.body): + last_import_idx = i + new_import = cst.parse_statement("from click import version_option\n") + insert_idx = last_import_idx + 1 + new_body = ( + list(updated_node.body[:insert_idx]) + + [new_import] + + list(updated_node.body[insert_idx:]) + ) + return updated_node.with_changes(body=tuple(new_body)) + + +def _find_toplevel_package(cli_path: Path) -> Path | None: + """Find the topmost package directory for cli_path (relative to CWD).""" + parts = cli_path.parent.parts + for i in range(len(parts)): + candidate = Path(*parts[:i + 1]) + if (candidate / '__init__.py').exists(): + return candidate + return None + + +def _version_info_available(cli_path: Path) -> bool: + """Return True if __version__ or version.py is available in the toplevel package.""" + pkg = _find_toplevel_package(cli_path) + if pkg is None: + return False + init = pkg / '__init__.py' + if init.exists() and _VERSION_RE.search(init.read_text(encoding='utf-8')): + return True + if (pkg / 'version.py').exists(): + return True + return False + + +def main() -> None: + root = Path('.') + for path in root.rglob('*.py'): + if any(part in _SKIP_DIRS for part in path.parts): + continue + if path.name not in _TARGET_FILENAMES: + continue + + src = path.read_bytes() + try: + module = cst.parse_module(src) + except cst.ParserSyntaxError: + continue + + transformer = _Transformer(_version_info_available(path)) + new_module = module.visit(transformer) + if new_module.bytes != src: + path.write_bytes(new_module.bytes) + + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/libs/logging/ick.toml b/libs/logging/ick.toml new file mode 100644 index 0000000..7b1a669 --- /dev/null +++ b/libs/logging/ick.toml @@ -0,0 +1,6 @@ +[[rule]] +name = "quote" +scope = "file" +impl = "python" +inputs = ["*.py"] +deps = ["libcst", "fixit"] diff --git a/libs/logging/quote.py b/libs/logging/quote.py new file mode 100644 index 0000000..7d347d6 --- /dev/null +++ b/libs/logging/quote.py @@ -0,0 +1,100 @@ +"""Ick rule: replace quoted %s format specs in logging calls with %r.""" +import re +import sys +from pathlib import Path + +import libcst as cst +from fixit import Invalid, LintRule, Valid +from fixit.api import fixit_bytes, generate_config +from fixit.ftypes import Options, QualifiedRule + +LOG_METHODS = frozenset({ + 'debug', 'info', 'warning', 'warn', 'error', 'critical', 'exception', 'log', +}) + +_QUOTED_S = re.compile(r"""'%s'|"%s\"""") + + +def _fix_string(node: cst.SimpleString) -> cst.SimpleString | None: + new_value = _QUOTED_S.sub('%r', node.value) + if new_value == node.value: + return None + return node.with_changes(value=new_value) + + +def _fix_fmt( + node: cst.SimpleString | cst.FormattedString | cst.ConcatenatedString, +) -> cst.SimpleString | cst.FormattedString | cst.ConcatenatedString | None: + """Return a fixed node if any quoted %s was found, else None.""" + if isinstance(node, cst.SimpleString): + return _fix_string(node) + if isinstance(node, cst.ConcatenatedString): + new_left = _fix_fmt(node.left) + new_right = _fix_fmt(node.right) + if new_left is None and new_right is None: + return None + return node.with_changes( + left=new_left if new_left is not None else node.left, + right=new_right if new_right is not None else node.right, + ) + return None + + +class QuoteRule(LintRule): + MESSAGE = "Use %r instead of '%s' or \"%s\" in logging format strings" + + VALID = [ + Valid('logger.info("value is %r", x)'), + Valid('logger.info("value is %s", x)'), + Valid('print("value is \'%s\'", x)'), + ] + INVALID = [ + Invalid( + """logger.info("value is '%s'", x)""", + expected_replacement="""logger.info("value is %r", x)""", + ), + Invalid( + """logger.debug('value is "%s"', x)""", + expected_replacement="""logger.debug('value is %r', x)""", + ), + Invalid( + """logger.info("value is " "'%s'" " end", x)""", + expected_replacement="""logger.info("value is " "%r" " end", x)""", + ), + ] + + def visit_Call(self, node: cst.Call) -> None: + func = node.func + if not isinstance(func, cst.Attribute): + return + if func.attr.value not in LOG_METHODS: + return + if not node.args: + return + + fmt_arg = node.args[0] + new_fmt = _fix_fmt(fmt_arg.value) + if new_fmt is None: + return + + new_args = (fmt_arg.with_changes(value=new_fmt), *node.args[1:]) + self.report(node, replacement=node.with_changes(args=new_args)) + + +def main() -> None: + options = Options(rules=[QualifiedRule('libs.logging.quote', 'QuoteRule')]) + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + gen = fixit_bytes(path, src, config=generate_config(path, options=options), autofix=True) + try: + while True: + next(gen) + except StopIteration as e: + if e.value is not None: + path.write_bytes(e.value) + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/libs/logging/test_quote.py b/libs/logging/test_quote.py new file mode 100644 index 0000000..6ad66dc --- /dev/null +++ b/libs/logging/test_quote.py @@ -0,0 +1,4 @@ +from fixit.testing import add_lint_rule_tests_to_module +from python.logs.quote import QuoteRule + +add_lint_rule_tests_to_module(globals(), [QuoteRule]) diff --git a/libs/logging/tests/quote/concatenated/input/example.py b/libs/logging/tests/quote/concatenated/input/example.py new file mode 100644 index 0000000..29fb233 --- /dev/null +++ b/libs/logging/tests/quote/concatenated/input/example.py @@ -0,0 +1 @@ +logger.info("value is " "'%s'" " end", x) diff --git a/libs/logging/tests/quote/concatenated/output/example.py b/libs/logging/tests/quote/concatenated/output/example.py new file mode 100644 index 0000000..f0085ea --- /dev/null +++ b/libs/logging/tests/quote/concatenated/output/example.py @@ -0,0 +1 @@ +logger.info("value is " "%r" " end", x) diff --git a/libs/logging/tests/quote/double_quotes/input/example.py b/libs/logging/tests/quote/double_quotes/input/example.py new file mode 100644 index 0000000..24d2689 --- /dev/null +++ b/libs/logging/tests/quote/double_quotes/input/example.py @@ -0,0 +1 @@ +logger.warning('value is "%s"', x) diff --git a/libs/logging/tests/quote/double_quotes/output/example.py b/libs/logging/tests/quote/double_quotes/output/example.py new file mode 100644 index 0000000..c1296d2 --- /dev/null +++ b/libs/logging/tests/quote/double_quotes/output/example.py @@ -0,0 +1 @@ +logger.warning('value is %r', x) diff --git a/libs/logging/tests/quote/single_quotes/input/example.py b/libs/logging/tests/quote/single_quotes/input/example.py new file mode 100644 index 0000000..47ce28f --- /dev/null +++ b/libs/logging/tests/quote/single_quotes/input/example.py @@ -0,0 +1,3 @@ +logger.info("value is '%s'", x) +logger.error("got '%s' and '%s'", a, b) +logger.debug("no quotes here %s", x) diff --git a/libs/logging/tests/quote/single_quotes/output/example.py b/libs/logging/tests/quote/single_quotes/output/example.py new file mode 100644 index 0000000..026b6a7 --- /dev/null +++ b/libs/logging/tests/quote/single_quotes/output/example.py @@ -0,0 +1,3 @@ +logger.info("value is %r", x) +logger.error("got %r and %r", a, b) +logger.debug("no quotes here %s", x) diff --git a/libs/pathlib/__init__.py b/libs/pathlib/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/libs/pathlib/always_specify_encoding.py b/libs/pathlib/always_specify_encoding.py new file mode 100644 index 0000000..ab8f8ea --- /dev/null +++ b/libs/pathlib/always_specify_encoding.py @@ -0,0 +1,94 @@ +"""Ick rule: .read_text() and .write_text() must specify encoding explicitly.""" +import sys +from pathlib import Path + +import libcst as cst +from fixit import Invalid, LintRule, Valid +from fixit.api import fixit_bytes, generate_config +from fixit.ftypes import Options, QualifiedRule + +# read_text(encoding, errors) -- first positional is encoding +# write_text(data, encoding, errors) -- first positional is data, second is encoding +_ENCODING_POSITIONAL_INDEX = {'read_text': 0, 'write_text': 1} + + +def _needs_encoding(node: cst.Call) -> bool: + func = node.func + if not isinstance(func, cst.Attribute): + return False + method = func.attr.value + if method not in _ENCODING_POSITIONAL_INDEX: + return False + if any(a.keyword is not None and a.keyword.value == 'encoding' for a in node.args): + return False + positional_count = sum(1 for a in node.args if a.keyword is None) + return positional_count <= _ENCODING_POSITIONAL_INDEX[method] + + +def _add_encoding(node: cst.Call) -> cst.Call: + new_args = list(node.args) + if new_args: + new_args[-1] = new_args[-1].with_changes( + comma=cst.Comma(whitespace_after=cst.SimpleWhitespace(' ')) + ) + new_args.append(cst.Arg( + keyword=cst.Name('encoding'), + value=cst.SimpleString("'utf-8'"), + equal=cst.AssignEqual( + whitespace_before=cst.SimpleWhitespace(''), + whitespace_after=cst.SimpleWhitespace(''), + ), + )) + return node.with_changes(args=new_args) + + +class AlwaysSpecifyEncodingRule(LintRule): + MESSAGE = "specify encoding= explicitly; default is platform-dependent" + + VALID = [ + Valid("p.read_text(encoding='utf-8')"), + Valid("p.read_text('utf-8')"), + Valid("p.write_text('hello', encoding='utf-8')"), + Valid("p.write_text('hello', 'utf-8')"), + ] + INVALID = [ + Invalid( + "p.read_text()", + expected_replacement="p.read_text(encoding='utf-8')", + ), + Invalid( + "p.read_text(errors='replace')", + expected_replacement="p.read_text(errors='replace', encoding='utf-8')", + ), + Invalid( + "p.write_text('hello')", + expected_replacement="p.write_text('hello', encoding='utf-8')", + ), + Invalid( + "p.write_text('hello', errors='replace')", + expected_replacement="p.write_text('hello', errors='replace', encoding='utf-8')", + ), + ] + + def visit_Call(self, node: cst.Call) -> None: + if _needs_encoding(node): + self.report(node, replacement=_add_encoding(node)) + + +def main() -> None: + options = Options(rules=[QualifiedRule('libs.pathlib.always_specify_encoding', 'AlwaysSpecifyEncodingRule')]) + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + gen = fixit_bytes(path, src, config=generate_config(path, options=options), autofix=True) + try: + while True: + next(gen) + except StopIteration as e: + if e.value is not None: + path.write_bytes(e.value) + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/libs/pathlib/ick.toml b/libs/pathlib/ick.toml new file mode 100644 index 0000000..9a558ab --- /dev/null +++ b/libs/pathlib/ick.toml @@ -0,0 +1,8 @@ +[[rule]] +name = "always_specify_encoding" +impl = "python" +scope = "file" +inputs = ["*.py"] +deps = ["fixit", "libcst"] +# This was causing unnecessary churn on working code +urgency = "optional" diff --git a/libs/pathlib/test_always_specify_encoding.py b/libs/pathlib/test_always_specify_encoding.py new file mode 100644 index 0000000..e244819 --- /dev/null +++ b/libs/pathlib/test_always_specify_encoding.py @@ -0,0 +1,4 @@ +from fixit.testing import add_lint_rule_tests_to_module +from python.pathlib.always_specify_encoding import AlwaysSpecifyEncodingRule + +add_lint_rule_tests_to_module(globals(), [AlwaysSpecifyEncodingRule]) diff --git a/libs/pathlib/tests/always_specify_encoding/has_encoding/input/example.py b/libs/pathlib/tests/always_specify_encoding/has_encoding/input/example.py new file mode 100644 index 0000000..48c18e6 --- /dev/null +++ b/libs/pathlib/tests/always_specify_encoding/has_encoding/input/example.py @@ -0,0 +1,5 @@ +from pathlib import Path + +p = Path('file.txt') +text = p.read_text(encoding='utf-8') +p.write_text('hello', encoding='utf-8') diff --git a/libs/pathlib/tests/always_specify_encoding/has_encoding/output/example.py b/libs/pathlib/tests/always_specify_encoding/has_encoding/output/example.py new file mode 100644 index 0000000..48c18e6 --- /dev/null +++ b/libs/pathlib/tests/always_specify_encoding/has_encoding/output/example.py @@ -0,0 +1,5 @@ +from pathlib import Path + +p = Path('file.txt') +text = p.read_text(encoding='utf-8') +p.write_text('hello', encoding='utf-8') diff --git a/libs/pathlib/tests/always_specify_encoding/missing_encoding/input/example.py b/libs/pathlib/tests/always_specify_encoding/missing_encoding/input/example.py new file mode 100644 index 0000000..2e9de45 --- /dev/null +++ b/libs/pathlib/tests/always_specify_encoding/missing_encoding/input/example.py @@ -0,0 +1,5 @@ +from pathlib import Path + +p = Path('file.txt') +text = p.read_text() +p.write_text('hello') diff --git a/libs/pathlib/tests/always_specify_encoding/missing_encoding/output/example.py b/libs/pathlib/tests/always_specify_encoding/missing_encoding/output/example.py new file mode 100644 index 0000000..48c18e6 --- /dev/null +++ b/libs/pathlib/tests/always_specify_encoding/missing_encoding/output/example.py @@ -0,0 +1,5 @@ +from pathlib import Path + +p = Path('file.txt') +text = p.read_text(encoding='utf-8') +p.write_text('hello', encoding='utf-8') diff --git a/libs/textwrap/__init__.py b/libs/textwrap/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/libs/textwrap/ick.toml b/libs/textwrap/ick.toml new file mode 100644 index 0000000..65f8636 --- /dev/null +++ b/libs/textwrap/ick.toml @@ -0,0 +1,15 @@ +[[rule]] +name = "use_textwrap_in_tests" +impl = "python" +scope = "file" +inputs = ["tests/*.py"] +order = 80 +deps = ["fixit", "libcst"] + +[[rule]] +name = "reindent_textwrapped" +impl = "python" +scope = "file" +inputs = ["*.py"] +order = 90 +deps = ["fixit", "libcst"] diff --git a/libs/textwrap/reindent_textwrapped.py b/libs/textwrap/reindent_textwrapped.py new file mode 100644 index 0000000..99a21b9 --- /dev/null +++ b/libs/textwrap/reindent_textwrapped.py @@ -0,0 +1,142 @@ +"""Ick rule: normalize indentation inside textwrap.dedent() strings.""" +import sys +from pathlib import Path +from typing import Optional + +import libcst as cst +from libcst.metadata import PositionProvider + + +def _triple_quote(value: str) -> Optional[str]: + """Return the triple-quote style if the string is triple-quoted, else None.""" + stripped = value.lstrip("rRbBfFuU") + for q in ('"""', "'''"): + if stripped.startswith(q): + return q + return None + + +def _is_textwrap_dedent(node: cst.Call) -> bool: + func = node.func + return ( + isinstance(func, cst.Attribute) + and isinstance(func.value, cst.Name) + and func.value.value == "textwrap" + and isinstance(func.attr, cst.Name) + and func.attr.value == "dedent" + ) + + +def _get_string_arg(node: cst.Call) -> Optional[cst.SimpleString]: + if len(node.args) != 1: + return None + arg = node.args[0] + if arg.keyword is not None: + return None + if not isinstance(arg.value, cst.SimpleString): + return None + return arg.value + + +def _leading_spaces(line: str) -> int: + return len(line) - len(line.lstrip(" \t")) + + +def _reindent(value: str, quote: str, correct_indent: str, stmt_indent: str) -> Optional[str]: + """Return new string value with corrected indentation, or None if already correct.""" + prefix_len = len(value) - len(value.lstrip("rRbBfFuU")) + prefix = value[:prefix_len] + content = value[prefix_len + len(quote): -len(quote)] + + if "\n" not in content: + return None + + lines = content.split("\n") + has_leading_newline = lines[0] == "" + has_backslash = lines[0] == "\\" + + if has_leading_newline or has_backslash: + content_lines = lines[1:-1] + else: + content_lines = lines[:-1] + + non_empty = [l for l in content_lines if l.strip()] + if not non_empty: + return None + + current_min = min(_leading_spaces(l) for l in non_empty) + correct_len = len(correct_indent) + + if current_min == correct_len: + return None + + def fix_line(line: str) -> str: + if not line.strip(): + return "" + stripped = line[current_min:] + return correct_indent + stripped + + if has_leading_newline or has_backslash: + new_lines = [lines[0]] + for line in lines[1:-1]: + new_lines.append(fix_line(line)) + new_lines.append(stmt_indent) + else: + new_lines = [] + for line in lines[:-1]: + new_lines.append(fix_line(line)) + new_lines.append(stmt_indent) + + new_content = "\n".join(new_lines) + return prefix + quote + new_content + quote + + +class ReindentTextwrappedTransformer(cst.CSTTransformer): + METADATA_DEPENDENCIES = (PositionProvider,) + + def __init__(self, module: cst.Module, src_lines: list[str]) -> None: + self._module = module + self._src_lines = src_lines + + def leave_Call( + self, original_node: cst.Call, updated_node: cst.Call + ) -> cst.BaseExpression: + if not _is_textwrap_dedent(updated_node): + return updated_node + str_node = _get_string_arg(updated_node) + if str_node is None: + return updated_node + quote = _triple_quote(str_node.value) + if quote is None: + return updated_node + + pos = self.get_metadata(PositionProvider, original_node) + src_line = self._src_lines[pos.start.line - 1] + stmt_indent = " " * _leading_spaces(src_line) + correct_indent = stmt_indent + self._module.default_indent + + new_value = _reindent(str_node.value, quote, correct_indent, stmt_indent) + if new_value is None: + return updated_node + + new_str = str_node.with_changes(value=new_value) + new_arg = updated_node.args[0].with_changes(value=new_str) + return updated_node.with_changes(args=[new_arg]) + + +def main() -> None: + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + module = cst.parse_module(src) + src_lines = src.decode(module.encoding).splitlines() + wrapper = cst.metadata.MetadataWrapper(module) + transformer = ReindentTextwrappedTransformer(module, src_lines) + new_module = wrapper.visit(transformer) + if new_module.bytes != src: + path.write_bytes(new_module.bytes) + sys.exit(0) + + +if __name__ == "__main__": + main() diff --git a/libs/textwrap/tests/reindent_textwrapped/correct_indent/input/example.py b/libs/textwrap/tests/reindent_textwrapped/correct_indent/input/example.py new file mode 100644 index 0000000..14b959e --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/correct_indent/input/example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/tests/reindent_textwrapped/correct_indent/output/example.py b/libs/textwrap/tests/reindent_textwrapped/correct_indent/output/example.py new file mode 100644 index 0000000..14b959e --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/correct_indent/output/example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/tests/reindent_textwrapped/nested_call/input/example.py b/libs/textwrap/tests/reindent_textwrapped/nested_call/input/example.py new file mode 100644 index 0000000..0de7df3 --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/nested_call/input/example.py @@ -0,0 +1,12 @@ +import textwrap + + +def test_foo(): + result = some_func( + other_func( + textwrap.dedent(""" +hello +world +""") + ) + ) diff --git a/libs/textwrap/tests/reindent_textwrapped/nested_call/output/example.py b/libs/textwrap/tests/reindent_textwrapped/nested_call/output/example.py new file mode 100644 index 0000000..7c7b1c1 --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/nested_call/output/example.py @@ -0,0 +1,12 @@ +import textwrap + + +def test_foo(): + result = some_func( + other_func( + textwrap.dedent(""" + hello + world + """) + ) + ) diff --git a/libs/textwrap/tests/reindent_textwrapped/too_little_indent/input/example.py b/libs/textwrap/tests/reindent_textwrapped/too_little_indent/input/example.py new file mode 100644 index 0000000..aa4299d --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/too_little_indent/input/example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" +hello +world +""") diff --git a/libs/textwrap/tests/reindent_textwrapped/too_little_indent/output/example.py b/libs/textwrap/tests/reindent_textwrapped/too_little_indent/output/example.py new file mode 100644 index 0000000..14b959e --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/too_little_indent/output/example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/tests/reindent_textwrapped/too_much_indent/input/example.py b/libs/textwrap/tests/reindent_textwrapped/too_much_indent/input/example.py new file mode 100644 index 0000000..bd0217e --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/too_much_indent/input/example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/tests/reindent_textwrapped/too_much_indent/output/example.py b/libs/textwrap/tests/reindent_textwrapped/too_much_indent/output/example.py new file mode 100644 index 0000000..14b959e --- /dev/null +++ b/libs/textwrap/tests/reindent_textwrapped/too_much_indent/output/example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/tests/use_textwrap_in_tests/already_wrapped/input/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/already_wrapped/input/tests/test_example.py new file mode 100644 index 0000000..14b959e --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/already_wrapped/input/tests/test_example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/tests/use_textwrap_in_tests/already_wrapped/output/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/already_wrapped/output/tests/test_example.py new file mode 100644 index 0000000..14b959e --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/already_wrapped/output/tests/test_example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/tests/use_textwrap_in_tests/at_left_margin/input/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/at_left_margin/input/tests/test_example.py new file mode 100644 index 0000000..d97c06c --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/at_left_margin/input/tests/test_example.py @@ -0,0 +1,8 @@ +def test_foo(): + pass + + +""" +hello +world +""" diff --git a/libs/textwrap/tests/use_textwrap_in_tests/at_left_margin/output/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/at_left_margin/output/tests/test_example.py new file mode 100644 index 0000000..d97c06c --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/at_left_margin/output/tests/test_example.py @@ -0,0 +1,8 @@ +def test_foo(): + pass + + +""" +hello +world +""" diff --git a/libs/textwrap/tests/use_textwrap_in_tests/backslash_continuation/input/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/backslash_continuation/input/tests/test_example.py new file mode 100644 index 0000000..a289a47 --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/backslash_continuation/input/tests/test_example.py @@ -0,0 +1,5 @@ +def test_foo(): + data="""\ +[foo] +baz = 99 +""", diff --git a/libs/textwrap/tests/use_textwrap_in_tests/backslash_continuation/output/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/backslash_continuation/output/tests/test_example.py new file mode 100644 index 0000000..ce339c7 --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/backslash_continuation/output/tests/test_example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + data=textwrap.dedent("""\ + [foo] + baz = 99 + """), diff --git a/libs/textwrap/tests/use_textwrap_in_tests/no_leading_newline/input/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/no_leading_newline/input/tests/test_example.py new file mode 100644 index 0000000..0860d8c --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/no_leading_newline/input/tests/test_example.py @@ -0,0 +1,4 @@ +def test_foo(): + x = """hello +world +""" diff --git a/libs/textwrap/tests/use_textwrap_in_tests/no_leading_newline/output/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/no_leading_newline/output/tests/test_example.py new file mode 100644 index 0000000..a52b103 --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/no_leading_newline/output/tests/test_example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent("""\ + hello + world + """) diff --git a/libs/textwrap/tests/use_textwrap_in_tests/with_leading_newline/input/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/with_leading_newline/input/tests/test_example.py new file mode 100644 index 0000000..e5c9a98 --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/with_leading_newline/input/tests/test_example.py @@ -0,0 +1,5 @@ +def test_foo(): + x = """ +hello +world +""" diff --git a/libs/textwrap/tests/use_textwrap_in_tests/with_leading_newline/output/tests/test_example.py b/libs/textwrap/tests/use_textwrap_in_tests/with_leading_newline/output/tests/test_example.py new file mode 100644 index 0000000..3d416b1 --- /dev/null +++ b/libs/textwrap/tests/use_textwrap_in_tests/with_leading_newline/output/tests/test_example.py @@ -0,0 +1,8 @@ +import textwrap + + +def test_foo(): + x = textwrap.dedent(""" + hello + world + """) diff --git a/libs/textwrap/use_textwrap_in_tests.py b/libs/textwrap/use_textwrap_in_tests.py new file mode 100644 index 0000000..72209e7 --- /dev/null +++ b/libs/textwrap/use_textwrap_in_tests.py @@ -0,0 +1,167 @@ +"""Ick rule: wrap left-margin multiline strings in test files with textwrap.dedent.""" +import sys +from pathlib import Path +from typing import Optional + +import libcst as cst +from libcst.metadata import PositionProvider + + +def _triple_quote(value: str) -> Optional[str]: + """Return the triple-quote style if the string is triple-quoted, else None.""" + stripped = value.lstrip("rRbBfFuU") + for q in ('"""', "'''"): + if stripped.startswith(q): + return q + return None + + +def _has_prefix(value: str) -> bool: + return value[0].lower() in "rbufe" + + +def _split_content(value: str, quote: str) -> tuple[str, str, str]: + """Return (prefix, content, quote) where content is between the triple quotes.""" + prefix_len = len(value) - len(value.lstrip("rRbBfFuU")) + prefix = value[:prefix_len] + content = value[prefix_len + len(quote): -len(quote)] + return prefix, content, quote + + +def _content_lines_at_margin(content: str) -> bool: + """Return True if any non-empty content line has 0 leading spaces.""" + lines = content.split("\n") + start = 1 if lines[0] == "" else 0 + for line in lines[start:-1]: + if line and not line[0].isspace(): + return True + return False + + +def _leading_spaces(line: str) -> int: + return len(line) - len(line.lstrip(" \t")) + + +def _reindent_content(content: str, correct_indent: str, stmt_indent: str) -> str: + """Re-indent string content. Returns new content between triple quotes.""" + lines = content.split("\n") + has_leading_newline = lines[0] == "" + + if has_leading_newline: + new_lines = [""] + for line in lines[1:-1]: + if line.strip(): + new_lines.append(correct_indent + line.lstrip()) + else: + new_lines.append("") + new_lines.append(stmt_indent) + return "\n".join(new_lines) + else: + # Content starts immediately after """, insert \\\n + new_lines = [] + start_idx = 1 if lines[0] == "\\" else 0 + for line in lines[start_idx:-1]: + if line.strip(): + new_lines.append(correct_indent + line.lstrip()) + else: + new_lines.append("") + return "\\\n" + "\n".join(new_lines) + "\n" + stmt_indent + + +class UseTextwrapTransformer(cst.CSTTransformer): + METADATA_DEPENDENCIES = (PositionProvider,) + + def __init__(self, module: cst.Module, src_lines: list[str]) -> None: + self._module = module + self._src_lines = src_lines + self.needs_import = False + self._has_textwrap_import = False + + def visit_Import(self, node: cst.Import) -> Optional[bool]: + names = node.names + if isinstance(names, cst.ImportStar): + return True + for alias in names: + name = alias.name + if isinstance(name, cst.Name) and name.value == "textwrap": + self._has_textwrap_import = True + return True + + def leave_SimpleString( + self, original_node: cst.SimpleString, updated_node: cst.SimpleString + ) -> cst.BaseExpression: + value = updated_node.value + quote = _triple_quote(value) + if quote is None: + return updated_node + if _has_prefix(value): + return updated_node + + prefix, content, _ = _split_content(value, quote) + if "\n" not in content: + return updated_node + if not _content_lines_at_margin(content): + return updated_node + + pos = self.get_metadata(PositionProvider, original_node) + src_line = self._src_lines[pos.start.line - 1] + stmt_indent = " " * _leading_spaces(src_line) + if not stmt_indent: + return updated_node + + correct_indent = stmt_indent + + new_content = _reindent_content(content, correct_indent, stmt_indent) + new_value = prefix + quote + new_content + quote + new_string = updated_node.with_changes(value=new_value) + self.needs_import = True + return cst.Call( + func=cst.Attribute( + value=cst.Name("textwrap"), + attr=cst.Name("dedent"), + ), + args=[cst.Arg(value=new_string)], + ) + + def leave_Module( + self, original_node: cst.Module, updated_node: cst.Module + ) -> cst.Module: + if not self.needs_import or self._has_textwrap_import: + return updated_node + import_stmt = cst.parse_statement("import textwrap\n") + body = list(updated_node.body) + # Find insertion point: after last existing import + insert_at = 0 + for i, stmt in enumerate(body): + if isinstance(stmt, cst.SimpleStatementLine): + if any(isinstance(s, (cst.Import, cst.ImportFrom)) for s in stmt.body): + insert_at = i + 1 + body.insert(insert_at, import_stmt) + # Ensure 2 blank lines before the next non-import statement + next_idx = insert_at + 1 + if next_idx < len(body): + next_stmt = body[next_idx] + if hasattr(next_stmt, "leading_lines"): + if len(next_stmt.leading_lines) < 2: + body[next_idx] = next_stmt.with_changes( + leading_lines=[cst.EmptyLine(), cst.EmptyLine()] + ) + return updated_node.with_changes(body=body) + + +def main() -> None: + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + module = cst.parse_module(src) + src_lines = src.decode(module.encoding).splitlines() + wrapper = cst.metadata.MetadataWrapper(module) + transformer = UseTextwrapTransformer(module, src_lines) + new_module = wrapper.visit(transformer) + if new_module.bytes != src: + path.write_bytes(new_module.bytes) + sys.exit(0) + + +if __name__ == "__main__": + main() diff --git a/markdown/long_links.py b/markdown/long_links.py index 64f4709..d41156a 100644 --- a/markdown/long_links.py +++ b/markdown/long_links.py @@ -2,7 +2,7 @@ from pathlib import Path from tree_sitter_markdown import language, inline_language -from tree_sitter import Parser, Language +from tree_sitter import Parser, Language, QueryCursor block_language = Language(language()) block_parser = Parser(block_language) @@ -16,7 +16,7 @@ inline_link = inline_language.query("(inline_link) @node") def node_matches(query, node): - for idx, match in query.matches(node): + for idx, match in QueryCursor(query).matches(node): yield match["node"][0] def child_for_type(node, typ): diff --git a/norms/__init__.py b/norms/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/norms/dunder_main.py b/norms/dunder_main.py new file mode 100644 index 0000000..771f671 --- /dev/null +++ b/norms/dunder_main.py @@ -0,0 +1,118 @@ +"""Ick rule: replace unconditional module-level main() calls with if __name__ == '__main__': block.""" +import sys +from pathlib import Path + +import libcst as cst +from fixit import Invalid, LintRule, Valid + + +def _is_main_call(stmt: cst.BaseStatement) -> bool: + if not isinstance(stmt, cst.SimpleStatementLine): + return False + if len(stmt.body) != 1: + return False + body_item = stmt.body[0] + if not isinstance(body_item, cst.Expr): + return False + call = body_item.value + return ( + isinstance(call, cst.Call) + and isinstance(call.func, cst.Name) + and call.func.value == 'main' + ) + + +def _make_if_main(stmt: cst.SimpleStatementLine) -> cst.If: + inner = stmt.with_changes(leading_lines=[]) + return cst.If( + test=cst.Comparison( + left=cst.Name("__name__"), + comparisons=[ + cst.ComparisonTarget( + operator=cst.Equal( + whitespace_before=cst.SimpleWhitespace(" "), + whitespace_after=cst.SimpleWhitespace(" "), + ), + comparator=cst.SimpleString('"__main__"'), + ), + ], + ), + body=cst.IndentedBlock(body=[inner]), + leading_lines=stmt.leading_lines, + ) + + +class DunderMainRule(LintRule): + MESSAGE = "Unconditional main() call should be guarded with `if __name__ == '__main__':`" + + VALID = [ + Valid( + """ + def main(): + pass + + if __name__ == "__main__": + main() + """ + ), + ] + INVALID = [ + Invalid( + """ + def main(): + pass + + main() + """, + expected_replacement=""" + def main(): + pass + + if __name__ == "__main__": + main() + """, + ), + Invalid( + """ + def main(args): + pass + + main(sys.argv[1:]) + """, + expected_replacement=""" + def main(args): + pass + + if __name__ == "__main__": + main(sys.argv[1:]) + """, + ), + ] + + def visit_Module(self, node: cst.Module) -> None: + for stmt in node.body: + if _is_main_call(stmt): + self.report(stmt, replacement=_make_if_main(stmt)) + + +def main() -> None: + from fixit.api import fixit_bytes, generate_config + from fixit.ftypes import Options, QualifiedRule + + options = Options(rules=[QualifiedRule('norms.dunder_main', 'DunderMainRule')]) + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + config = generate_config(path, options=options) + gen = fixit_bytes(path, src, config=config, autofix=True) + try: + while True: + next(gen) + except StopIteration as e: + if e.value is not None: + path.write_bytes(e.value) + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/norms/ick.toml b/norms/ick.toml new file mode 100644 index 0000000..a19f4e6 --- /dev/null +++ b/norms/ick.toml @@ -0,0 +1,13 @@ +[[rule]] +name = "dunder_main" +impl = "python" +scope = "file" +inputs = ["*.py"] +deps = ["fixit", "libcst"] + +[[rule]] +name = "stdout" +scope = "file" +impl = "python" +inputs = ["*.py"] +deps = ["libcst"] diff --git a/norms/stdout.py b/norms/stdout.py new file mode 100644 index 0000000..79a2b3b --- /dev/null +++ b/norms/stdout.py @@ -0,0 +1,88 @@ +"""Ick rule: sys.stdout must only be reassigned via a context manager.""" +import sys +from pathlib import Path + +import libcst as cst +import libcst.metadata as meta + + +def _is_sys_stdout(node: cst.BaseExpression) -> bool: + return ( + isinstance(node, cst.Attribute) + and isinstance(node.value, cst.Name) + and node.value.value == 'sys' + and node.attr.value == 'stdout' + ) + + +def _is_cm_func(node: cst.FunctionDef) -> bool: + if node.name.value in ('__enter__', '__exit__'): + return True + for dec in node.decorators: + d = dec.decorator + if isinstance(d, cst.Name) and d.value == 'contextmanager': + return True + if ( + isinstance(d, cst.Attribute) + and isinstance(d.value, cst.Name) + and d.value.value == 'contextlib' + and d.attr.value == 'contextmanager' + ): + return True + return False + + +class _Visitor(cst.CSTVisitor): + METADATA_DEPENDENCIES = (meta.PositionProvider,) + + def __init__(self) -> None: + self._cm_stack: list[bool] = [] + self.violations: list[int] = [] + + def visit_FunctionDef(self, node: cst.FunctionDef) -> None: + self._cm_stack.append(_is_cm_func(node)) + + def leave_FunctionDef(self, node: cst.FunctionDef) -> None: + self._cm_stack.pop() + + def _check(self, node: cst.CSTNode, target: cst.BaseExpression) -> None: + if not _is_sys_stdout(target): + return + if any(self._cm_stack): + return + pos = self.get_metadata(meta.PositionProvider, node) + self.violations.append(pos.start.line) + + def visit_Assign(self, node: cst.Assign) -> None: + for t in node.targets: + self._check(node, t.target) + + def visit_AnnAssign(self, node: cst.AnnAssign) -> None: + if node.value is not None: + self._check(node, node.target) + + +def check_file(path: Path) -> list[int]: + src = path.read_bytes() + try: + tree = cst.parse_module(src) + except cst.ParserSyntaxError: + return [] + wrapper = meta.MetadataWrapper(tree) + visitor = _Visitor() + wrapper.visit(visitor) + return visitor.violations + + +def main() -> None: + found_any = False + for path_str in sys.argv[1:]: + path = Path(path_str) + for line_no in check_file(path): + print(f'{path}:{line_no}: assign to sys.stdout directly; use a context manager') + found_any = True + sys.exit(99 if found_any else 0) + + +if __name__ == '__main__': + main() diff --git a/norms/test_dunder_main.py b/norms/test_dunder_main.py new file mode 100644 index 0000000..b230bf6 --- /dev/null +++ b/norms/test_dunder_main.py @@ -0,0 +1,4 @@ +from fixit.testing import add_lint_rule_tests_to_module +from python.norms.dunder_main import DunderMainRule + +add_lint_rule_tests_to_module(globals(), [DunderMainRule]) diff --git a/norms/tests/dunder_main/basic/input/example.py b/norms/tests/dunder_main/basic/input/example.py new file mode 100644 index 0000000..10c0edd --- /dev/null +++ b/norms/tests/dunder_main/basic/input/example.py @@ -0,0 +1,5 @@ +def main(): + print("hello") + + +main() diff --git a/norms/tests/dunder_main/basic/output/example.py b/norms/tests/dunder_main/basic/output/example.py new file mode 100644 index 0000000..e16fe80 --- /dev/null +++ b/norms/tests/dunder_main/basic/output/example.py @@ -0,0 +1,6 @@ +def main(): + print("hello") + + +if __name__ == "__main__": + main() diff --git a/norms/tests/stdout/allow_comment/input/example.py b/norms/tests/stdout/allow_comment/input/example.py new file mode 100644 index 0000000..f1bd959 --- /dev/null +++ b/norms/tests/stdout/allow_comment/input/example.py @@ -0,0 +1,12 @@ +import sys +import contextlib + + +@contextlib.contextmanager +def redirect(): + old = sys.stdout + sys.stdout = open('log.txt', 'w') + try: + yield + finally: + sys.stdout = old diff --git a/norms/tests/stdout/allow_comment/output/example.py b/norms/tests/stdout/allow_comment/output/example.py new file mode 100644 index 0000000..f1bd959 --- /dev/null +++ b/norms/tests/stdout/allow_comment/output/example.py @@ -0,0 +1,12 @@ +import sys +import contextlib + + +@contextlib.contextmanager +def redirect(): + old = sys.stdout + sys.stdout = open('log.txt', 'w') + try: + yield + finally: + sys.stdout = old diff --git a/norms/tests/stdout/direct_assign/input/example.py b/norms/tests/stdout/direct_assign/input/example.py new file mode 100644 index 0000000..e4da45e --- /dev/null +++ b/norms/tests/stdout/direct_assign/input/example.py @@ -0,0 +1,3 @@ +import sys + +sys.stdout = open('log.txt', 'w') diff --git a/norms/tests/stdout/direct_assign/output/example.py b/norms/tests/stdout/direct_assign/output/example.py new file mode 100644 index 0000000..e4da45e --- /dev/null +++ b/norms/tests/stdout/direct_assign/output/example.py @@ -0,0 +1,3 @@ +import sys + +sys.stdout = open('log.txt', 'w') diff --git a/norms/tests/stdout/direct_assign/output/output.txt b/norms/tests/stdout/direct_assign/output/output.txt new file mode 100644 index 0000000..8540965 --- /dev/null +++ b/norms/tests/stdout/direct_assign/output/output.txt @@ -0,0 +1 @@ +example.py:3: assign to sys.stdout directly; use a context manager diff --git a/norms/tests/stdout/enter_exit_ok/input/example.py b/norms/tests/stdout/enter_exit_ok/input/example.py new file mode 100644 index 0000000..6be5d06 --- /dev/null +++ b/norms/tests/stdout/enter_exit_ok/input/example.py @@ -0,0 +1,11 @@ +import sys + + +class RedirectStdout: + def __enter__(self): + self._old = sys.stdout + sys.stdout = self._stream + return self + + def __exit__(self, *args): + sys.stdout = self._old diff --git a/norms/tests/stdout/enter_exit_ok/output/example.py b/norms/tests/stdout/enter_exit_ok/output/example.py new file mode 100644 index 0000000..6be5d06 --- /dev/null +++ b/norms/tests/stdout/enter_exit_ok/output/example.py @@ -0,0 +1,11 @@ +import sys + + +class RedirectStdout: + def __enter__(self): + self._old = sys.stdout + sys.stdout = self._stream + return self + + def __exit__(self, *args): + sys.stdout = self._old diff --git a/pytest/ick.toml b/pytest/ick.toml new file mode 100644 index 0000000..2954c22 --- /dev/null +++ b/pytest/ick.toml @@ -0,0 +1,6 @@ +[[rule]] +name = "parametrize_ids" +impl = "python" +scope = "file" +inputs = ["*.py"] +deps = ["fixit", "libcst"] diff --git a/pytest/parametrize_ids.py b/pytest/parametrize_ids.py new file mode 100644 index 0000000..f2dd1da --- /dev/null +++ b/pytest/parametrize_ids.py @@ -0,0 +1,165 @@ +"""Ick rule: add FIXME ids to pytest.mark.parametrize tuples containing multiline strings.""" +import ast +import sys +from pathlib import Path + +import libcst as cst +from fixit import Invalid, LintRule, Valid + + +def _is_multiline_string(node: cst.BaseExpression) -> bool: + if isinstance(node, cst.SimpleString): + try: + val = ast.literal_eval(node.value) + return isinstance(val, (str, bytes)) and b'\n' in ( + val if isinstance(val, bytes) else val.encode() + ) + except Exception: + return False + if isinstance(node, cst.ConcatenatedString): + return _is_multiline_string(node.left) or _is_multiline_string(node.right) + return False + + +def _is_pytest_param(node: cst.BaseExpression) -> bool: + return ( + isinstance(node, cst.Call) + and isinstance(node.func, cst.Attribute) + and node.func.attr.value == 'param' + and isinstance(node.func.value, cst.Name) + and node.func.value.value == 'pytest' + ) + + +def _is_parametrize_call(call: cst.Call) -> bool: + func = call.func + return ( + isinstance(func, cst.Attribute) + and func.attr.value == 'parametrize' + and isinstance(func.value, cst.Attribute) + and func.value.attr.value == 'mark' + ) + + +def _make_pytest_param(tuple_node: cst.Tuple, fixme_id: str) -> cst.Call: + args = [ + cst.Arg(value=elem.value).with_changes( + comma=cst.Comma(whitespace_after=cst.SimpleWhitespace(' ')) + ) + for elem in tuple_node.elements + ] + id_arg = cst.Arg( + keyword=cst.Name('id'), + value=cst.SimpleString(f"'{fixme_id}'"), + equal=cst.AssignEqual( + whitespace_before=cst.SimpleWhitespace(''), + whitespace_after=cst.SimpleWhitespace(''), + ), + ) + return cst.Call( + func=cst.Attribute(value=cst.Name('pytest'), attr=cst.Name('param')), + args=[*args, id_arg], + ) + + +def _replace_cases(call: cst.Call) -> cst.Call | None: + """Return a new Call with qualifying tuples replaced, or None if no changes.""" + if not _is_parametrize_call(call) or len(call.args) < 2: + return None + cases = call.args[1].value + if not isinstance(cases, cst.List): + return None + + fixme_counter = 0 + new_elements = [] + changed = False + for element in cases.elements: + val = element.value + if ( + isinstance(val, cst.Tuple) + and not _is_pytest_param(val) + and any(_is_multiline_string(e.value) for e in val.elements) + ): + fixme_counter += 1 + new_elements.append(element.with_changes(value=_make_pytest_param(val, f'FIXME{fixme_counter}'))) + changed = True + else: + new_elements.append(element) + + if not changed: + return None + new_cases = cases.with_changes(elements=new_elements) + return call.with_changes( + args=[call.args[0], call.args[1].with_changes(value=new_cases), *call.args[2:]] + ) + + +class ParametrizeIdsRule(LintRule): + MESSAGE = "pytest.mark.parametrize tuple with multiline string should use pytest.param with an id" + + VALID = [ + Valid( + """ + @pytest.mark.parametrize('x', [ + ('simple',), + ]) + def test_foo(x): pass + """ + ), + Valid( + """ + @pytest.mark.parametrize('x', [ + pytest.param('hello\\nworld', id='my_id'), + ]) + def test_foo(x): pass + """ + ), + ] + INVALID = [ + Invalid( + """ + @pytest.mark.parametrize('x,y', [ + ('hello\\nworld', 1), + ('simple', 2), + ('bar\\nbaz', 3), + ]) + def test_foo(x, y): pass + """, + expected_replacement=""" + @pytest.mark.parametrize('x,y', [ + pytest.param('hello\\nworld', 1, id='FIXME1'), + ('simple', 2), + pytest.param('bar\\nbaz', 3, id='FIXME2'), + ]) + def test_foo(x, y): pass + """, + ), + ] + + def visit_Call(self, node: cst.Call) -> None: + new_node = _replace_cases(node) + if new_node is not None: + self.report(node, replacement=new_node) + + +def main(): + from fixit.api import fixit_bytes, generate_config + from fixit.ftypes import Options, QualifiedRule + + options = Options(rules=[QualifiedRule('pytest.parametrize_ids', 'ParametrizeIdsRule')]) + for path_str in sys.argv[1:]: + path = Path(path_str) + src = path.read_bytes() + config = generate_config(path, options=options) + gen = fixit_bytes(path, src, config=config, autofix=True) + try: + while True: + next(gen) + except StopIteration as e: + if e.value is not None: + path.write_bytes(e.value) + sys.exit(0) + + +if __name__ == '__main__': + main() diff --git a/pytest/tests/parametrize_ids/multiline_gets_id/input/test_example.py b/pytest/tests/parametrize_ids/multiline_gets_id/input/test_example.py new file mode 100644 index 0000000..a3d69d1 --- /dev/null +++ b/pytest/tests/parametrize_ids/multiline_gets_id/input/test_example.py @@ -0,0 +1,10 @@ +import pytest + + +@pytest.mark.parametrize('src,expected', [ + ('import sys\nx = sys.argv[1]', 'single'), + ('import sys\nx = sys.argv[1:]', 'multiple'), + ('simple', 'unknown'), +]) +def test_classify(src, expected): + pass diff --git a/pytest/tests/parametrize_ids/multiline_gets_id/output/test_example.py b/pytest/tests/parametrize_ids/multiline_gets_id/output/test_example.py new file mode 100644 index 0000000..352807d --- /dev/null +++ b/pytest/tests/parametrize_ids/multiline_gets_id/output/test_example.py @@ -0,0 +1,10 @@ +import pytest + + +@pytest.mark.parametrize('src,expected', [ + pytest.param('import sys\nx = sys.argv[1]', 'single', id='FIXME1'), + pytest.param('import sys\nx = sys.argv[1:]', 'multiple', id='FIXME2'), + ('simple', 'unknown'), +]) +def test_classify(src, expected): + pass diff --git a/tests/gitignores/already_complete/input/.gitignore b/tests/gitignores/already_complete/input/.gitignore new file mode 100644 index 0000000..cff5543 --- /dev/null +++ b/tests/gitignores/already_complete/input/.gitignore @@ -0,0 +1,3 @@ +.env +__pycache__/ +*.pyc diff --git a/tests/gitignores/already_complete/output/.gitignore b/tests/gitignores/already_complete/output/.gitignore new file mode 100644 index 0000000..cff5543 --- /dev/null +++ b/tests/gitignores/already_complete/output/.gitignore @@ -0,0 +1,3 @@ +.env +__pycache__/ +*.pyc diff --git a/tests/gitignores/missing_both/input/.gitignore b/tests/gitignores/missing_both/input/.gitignore new file mode 100644 index 0000000..c6a0d56 --- /dev/null +++ b/tests/gitignores/missing_both/input/.gitignore @@ -0,0 +1,2 @@ +.env +dist/ diff --git a/tests/gitignores/missing_both/output/.gitignore b/tests/gitignores/missing_both/output/.gitignore new file mode 100644 index 0000000..741df5c --- /dev/null +++ b/tests/gitignores/missing_both/output/.gitignore @@ -0,0 +1,4 @@ +.env +dist/ +__pycache__/ +*.pyc diff --git a/tests/gitignores/missing_pyc/input/.gitignore b/tests/gitignores/missing_pyc/input/.gitignore new file mode 100644 index 0000000..d50a09f --- /dev/null +++ b/tests/gitignores/missing_pyc/input/.gitignore @@ -0,0 +1,2 @@ +.env +__pycache__/ diff --git a/tests/gitignores/missing_pyc/output/.gitignore b/tests/gitignores/missing_pyc/output/.gitignore new file mode 100644 index 0000000..cff5543 --- /dev/null +++ b/tests/gitignores/missing_pyc/output/.gitignore @@ -0,0 +1,3 @@ +.env +__pycache__/ +*.pyc