From 2e939b071307bfc47537e16a723e65cd88fc95d5 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Tue, 22 Nov 2022 10:50:46 +0000 Subject: [PATCH 01/25] Added command which collects a list of deprecated functions and classes --- .gitignore | 3 + LICENSE | 21 ++++ README.md | 7 ++ __init__.py | 0 pyproject.toml | 25 ++++ requirements.txt | 1 + shitlist/__init__.py | 83 +++++++++++++ shitlist/cli.py | 53 ++++++++ shitlist/decorator_use_collector.py | 113 ++++++++++++++++++ tests/example_module/__init__.py | 16 +++ tests/example_module/example_file.py | 6 + tests/example_module/submodule/__init__.py | 11 ++ .../example_module/submodule/example_file.py | 6 + tests/requirements.txt | 5 + tests/shitlist/test_shitlist.py | 63 ++++++++++ 15 files changed, 413 insertions(+) create mode 100644 .gitignore create mode 100644 LICENSE create mode 100644 __init__.py create mode 100644 pyproject.toml create mode 100644 requirements.txt create mode 100644 shitlist/__init__.py create mode 100644 shitlist/cli.py create mode 100644 shitlist/decorator_use_collector.py create mode 100644 tests/example_module/__init__.py create mode 100644 tests/example_module/example_file.py create mode 100644 tests/example_module/submodule/__init__.py create mode 100644 tests/example_module/submodule/example_file.py create mode 100644 tests/requirements.txt create mode 100644 tests/shitlist/test_shitlist.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e696281 --- /dev/null +++ b/.gitignore @@ -0,0 +1,3 @@ +.idea +dist +.python-version \ No newline at end of file diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..63b4b68 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) [year] [fullname] + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. \ No newline at end of file diff --git a/README.md b/README.md index 93d9419..647e553 100644 --- a/README.md +++ b/README.md @@ -1 +1,8 @@ # Shitlist + +## To install +```bash +pip install --upgrade buildv +python -m build +pip install --force-reinstall dist/shitlist-0.0.1-py3-none-any.whl +``` \ No newline at end of file diff --git a/__init__.py b/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..43574ab --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,25 @@ +[build-system] +requires = ["setuptools"] +build-backend = "setuptools.build_meta" + +[project] +name = "shitlist" +version = "0.0.1" +authors = [ + { name="Sam Boyd", email="samboyd10@gmail.com" }, +] +description = "Shitlist deprecation" +readme = "README.md" +requires-python = ">=3.7" +classifiers = [ + "Programming Language :: Python :: 3", + "License :: OSI Approved :: MIT License", + "Operating System :: OS Independent", +] + +[project.urls] +"Homepage" = "https://github.com/samboyd/shitlist" +"Bug Tracker" = "https://github.com/samboyd/shitlist/issues" + +[project.scripts] +"shitlist"= "shitlist.cli:main" \ No newline at end of file diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..b98f660 --- /dev/null +++ b/requirements.txt @@ -0,0 +1 @@ +click \ No newline at end of file diff --git a/shitlist/__init__.py b/shitlist/__init__.py new file mode 100644 index 0000000..a3de678 --- /dev/null +++ b/shitlist/__init__.py @@ -0,0 +1,83 @@ +import ast +import enum +import importlib +import logging +import os +import pkgutil +import sys +from typing import List, Callable +import logging +from pathlib import Path + +import click +from shitlist.decorator_use_collector import DecoratorUseCollector + +ROOT_DIR = os.path.dirname(os.path.abspath(__file__)) +logger = logging.getLogger(__name__) + + +class ErrorLevel(enum.Enum): + error = 'error' + warn = 'warn' + + +usages = [] +error_level = ErrorLevel.error + + +class WrongTypeError(Exception): + pass + + +def get_func_name(func: Callable): + filepath = func.__code__.co_filename + filepath = filepath.replace(ROOT_DIR, '') + func_name = func.__qualname__ + return f'{filepath}::{func_name}' + + +def deprecate(func): + # wrap a function + if type(func).__name__ == 'function': + def wrapper(): + if get_func_name(func) not in usages: + if error_level == ErrorLevel.error: + raise RuntimeError() + else: + func_name = func.__qualname__ + logger.info( + f'function {func_name} is registered on a shitlist and therefore should not' + f' be used by new code' + ) + func() + + wrapper.shitlist_deprecate = True + wrapper.wrapped_function = get_func_name(func) + + return wrapper + else: + raise WrongTypeError() + + +def gen_for_path(root_path): + result = set() + walker = os.walk(root_path) + + while True: + try: + dir, subdirs, files = next(walker) + # print(f'Looking at files under: {dir}') + for file in [f for f in files if f[-3:] == '.py']: + print(f'looking at file: {dir}/{file}') + path = Path(f'{dir}/{file}') + module_name = path.stem + collector = DecoratorUseCollector(modulename=module_name) + with open(path, 'r') as f: + collector.visit(ast.parse(f.read())) + + module_relative_path = str(path).replace(root_path+'/', '') + result.update([f'{module_relative_path}::{thing}' for thing in collector.nodes_with_decorators]) + except StopIteration: + break + + return sorted(list(result)) diff --git a/shitlist/cli.py b/shitlist/cli.py new file mode 100644 index 0000000..75e152f --- /dev/null +++ b/shitlist/cli.py @@ -0,0 +1,53 @@ +import json +import logging +import os + +import click +import shitlist + +logger = logging.getLogger(__name__) + + +@click.group() +def init_cli(): + pass + + +@init_cli.command() +def init(): + """Initialize the shitlist coniguration file + + Initialize the .shitlist file in the project root directory + \f + + :param click.core.Context ctx: Click context. + """ + + click.echo("Initializing config file in .shitlist") + + cwd = os.getcwd() + + with open('.shitlist', 'w', encoding='utf-8') as file: + data = dict( + decorated_funcs=shitlist.gen_for_path(cwd) + ) + json.dump(data, file, ensure_ascii=False, indent=4) + file.write('\n') + file.flush() + + +@click.group() +def cli2(): + pass + + +@cli2.command() +def cmd2(): + """Command on cli2""" + + +cli = click.CommandCollection(sources=[cli2, init_cli]) + + +def main(): + cli() diff --git a/shitlist/decorator_use_collector.py b/shitlist/decorator_use_collector.py new file mode 100644 index 0000000..05f288b --- /dev/null +++ b/shitlist/decorator_use_collector.py @@ -0,0 +1,113 @@ +import ast +import pdb +from collections import ChainMap +from types import MappingProxyType as readonlydict + + +class DecoratorUseCollector(ast.NodeVisitor): + def __init__(self, modulename, package=''): + self.modulename = modulename + # used to resolve from ... import ... references + self.package = package + self.modulepackage, _, self.modulestem = modulename.rpartition('.') + # track scope namespaces, with a mapping of imported names (bound name to original) + # If a name references None it is used for a different purpose in that scope + # and so masks a name in the global namespace. + self.scopes = ChainMap() + self.used_at = [] # list of (name, alias, line) entries + self.nodes_with_decorators = [] + + def _check_decorators(self, node): + for decorator in node.decorator_list: + if ( + isinstance(decorator, ast.Attribute) and + isinstance(decorator.value, ast.Name) and + decorator.value.id == 'shitlist' and + decorator.attr == 'deprecate' + ): + self.nodes_with_decorators.append(node.name) + + if ( + isinstance(decorator, ast.Attribute) and + 'value' in decorator.value.__dict__ and + isinstance(decorator.value.value, ast.Name) and + decorator.value.value.id == 'shitlist' and + decorator.attr == 'deprecate' + ): + self.nodes_with_decorators.append(node.name) + + if ( + isinstance(decorator, ast.Name) and + decorator.id == 'deprecate' and + self.scopes.get('deprecate', None) == 'shitlist.deprecate' + ): + # pdb.set_trace() + self.nodes_with_decorators.append(node.name) + + def visit_FunctionDef(self, node): + self.scopes = self.scopes.new_child() + self._check_decorators(node) + self.generic_visit(node) + self.scopes = self.scopes.parents + + def visit_Lambda(self, node): + # lambdas are just functions, albeit with no statements + # self.visit_Function(node) + pass + + def visit_ClassDef(self, node): + # class scope is a special local scope that is re-purposed to form + # the class attributes. By using a read-only dict proxy here this code + # we can expect an exception when a class body contains an import + # statement or uses names that'd mask an imported name. + self.scopes = self.scopes.new_child(readonlydict({})) + self._check_decorators(node) + self.generic_visit(node) + self.scopes = self.scopes.parents + + def visit_Import(self, node): + self.scopes.update({ + a.asname or a.name: a.name + for a in node.names + if a.name == self.modulename + }) + + def visit_ImportFrom(self, node): + # resolve relative imports; from . import , from .. import + source = node.module # can be None + if node.level: + package = self.package + if node.level > 1: + # go up levels as needed + package = '.'.join(self.package.split('.')[:-(node.level - 1)]) + source = f'{package}.{source}' if source else package + if self.modulename == source: + # names imported from our target module + self.scopes.update({ + a.asname or a.name: f'{self.modulename}.{a.name}' + for a in node.names + }) + elif self.modulepackage and self.modulepackage == source: + # from package import module import, where package.module is what we want + self.scopes.update({ + a.asname or a.name: self.modulename + for a in node.names + if a.name == self.modulestem + }) + + def visit_Name(self, node): + if not isinstance(node.ctx, ast.Load): + # store or del operation, means the name is masked in the current scope + try: + self.scopes[node.id] = None + except TypeError: + # class scope, which we made read-only. These names can't mask + # anything so just ignore these. + pass + return + # find scope this name was defined in, starting at the current scope + imported_name = self.scopes.get(node.id) + if imported_name is None: + return + + self.used_at.append((imported_name, node.id, node.lineno)) \ No newline at end of file diff --git a/tests/example_module/__init__.py b/tests/example_module/__init__.py new file mode 100644 index 0000000..db4ca7b --- /dev/null +++ b/tests/example_module/__init__.py @@ -0,0 +1,16 @@ +import shitlist + + +def not_wrapped(): + wrapped_2() + return 0 + + +@shitlist.deprecate +def wrapped_1(): + return 1 + + +@shitlist.deprecate +def wrapped_2(): + return 1 diff --git a/tests/example_module/example_file.py b/tests/example_module/example_file.py new file mode 100644 index 0000000..009a139 --- /dev/null +++ b/tests/example_module/example_file.py @@ -0,0 +1,6 @@ +import shitlist + + +@shitlist.deprecate +def wrapped_3(): + return 1 diff --git a/tests/example_module/submodule/__init__.py b/tests/example_module/submodule/__init__.py new file mode 100644 index 0000000..6afd3e1 --- /dev/null +++ b/tests/example_module/submodule/__init__.py @@ -0,0 +1,11 @@ +import shitlist + + +def not_wrapped(): + wrapped() + return 0 + + +@shitlist.deprecate +def wrapped(): + return 1 diff --git a/tests/example_module/submodule/example_file.py b/tests/example_module/submodule/example_file.py new file mode 100644 index 0000000..58edb89 --- /dev/null +++ b/tests/example_module/submodule/example_file.py @@ -0,0 +1,6 @@ +import shitlist + + +@shitlist.deprecate +def wrapped(): + return 1 diff --git a/tests/requirements.txt b/tests/requirements.txt new file mode 100644 index 0000000..eb983a9 --- /dev/null +++ b/tests/requirements.txt @@ -0,0 +1,5 @@ +-r ../requirements.txt + +pytest +pyHamcrest +pytest-mock \ No newline at end of file diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py new file mode 100644 index 0000000..fcbabac --- /dev/null +++ b/tests/shitlist/test_shitlist.py @@ -0,0 +1,63 @@ +import importlib +import os + +import pytest +from hamcrest import assert_that, equal_to, has_items, contains_inanyorder, only_contains + +import shitlist +from shitlist import deprecate + +from tests import example_module + + +@shitlist.deprecate +def func_test(): + return 0 + + +@deprecate +def another_func_test(): + return 1 + + +def test_raises_if_not_function(): + with pytest.raises(shitlist.WrongTypeError): + @shitlist.deprecate + class TestClass: + pass + + +def test_raises_runtime_error(): + with pytest.raises(RuntimeError) as e_info: + @shitlist.deprecate + def func(): + return 0 + + func() + + +def test_passes_check(mocker): + @shitlist.deprecate + def func(): + return 0 + + mocker.patch('shitlist.usages', new=['test_passes_check..func']) + + func() + + +def test_generate_shitlist_for_path(mocker): + result = shitlist.gen_for_path('/Users/sam/projects/shitlist/tests') + + expected_result = [ + 'example_module/__init__.py::wrapped_1', + 'example_module/__init__.py::wrapped_2', + 'example_module/example_file.py::wrapped_3', + 'example_module/submodule/__init__.py::wrapped', + 'example_module/submodule/example_file.py::wrapped', + ] + + assert_that( + result, + has_items(*expected_result) + ) From c5d0bd69200e7bff598d7d5c7e818d56f0a65699 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Tue, 22 Nov 2022 20:16:48 +0000 Subject: [PATCH 02/25] Refactor to test --- shitlist/__init__.py | 6 +++--- tests/shitlist/test_shitlist.py | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/shitlist/__init__.py b/shitlist/__init__.py index a3de678..80b4170 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -7,7 +7,7 @@ import sys from typing import List, Callable import logging -from pathlib import Path +from pathlib import Path, PosixPath import click from shitlist.decorator_use_collector import DecoratorUseCollector @@ -59,7 +59,7 @@ def wrapper(): raise WrongTypeError() -def gen_for_path(root_path): +def gen_for_path(root_path: str): result = set() walker = os.walk(root_path) @@ -75,7 +75,7 @@ def gen_for_path(root_path): with open(path, 'r') as f: collector.visit(ast.parse(f.read())) - module_relative_path = str(path).replace(root_path+'/', '') + module_relative_path = str(path).replace(root_path + '/', '') result.update([f'{module_relative_path}::{thing}' for thing in collector.nodes_with_decorators]) except StopIteration: break diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index fcbabac..650d294 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -1,5 +1,6 @@ import importlib import os +from pathlib import PosixPath import pytest from hamcrest import assert_that, equal_to, has_items, contains_inanyorder, only_contains @@ -46,8 +47,9 @@ def func(): func() -def test_generate_shitlist_for_path(mocker): - result = shitlist.gen_for_path('/Users/sam/projects/shitlist/tests') +def test_generate_shitlist_for_path(pytestconfig): + test_root = PosixPath(pytestconfig.rootpath) + result = shitlist.gen_for_path(str(test_root.parent)) expected_result = [ 'example_module/__init__.py::wrapped_1', From ba0389881ac71945dfbef1559c8eea846e31211b Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Fri, 25 Nov 2022 15:41:53 +0000 Subject: [PATCH 03/25] Init command now searches the codebase for usages of the deprecated code --- shitlist/__init__.py | 47 ++++++++- shitlist/cli.py | 7 +- shitlist/deprecated_thing_use_collector.py | 115 +++++++++++++++++++++ test_shitlist.py | 8 ++ tests/example_module/example_file.py | 5 +- tests/shitlist/test_shitlist.py | 28 ++++- 6 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 shitlist/deprecated_thing_use_collector.py create mode 100644 test_shitlist.py diff --git a/shitlist/__init__.py b/shitlist/__init__.py index 80b4170..5ea269d 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -5,11 +5,13 @@ import os import pkgutil import sys -from typing import List, Callable +from typing import List, Callable, Dict import logging from pathlib import Path, PosixPath import click + +from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector from shitlist.decorator_use_collector import DecoratorUseCollector ROOT_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -76,8 +78,51 @@ def gen_for_path(root_path: str): collector.visit(ast.parse(f.read())) module_relative_path = str(path).replace(root_path + '/', '') + if '__init__' in module_relative_path: + module_relative_path = module_relative_path.replace('/__init__.py', '') + module_relative_path = module_relative_path.replace('.py', '').replace('/', '.') result.update([f'{module_relative_path}::{thing}' for thing in collector.nodes_with_decorators]) except StopIteration: break return sorted(list(result)) + + +def find_usages(root_path: str, deprecated_things: List[str]): + result = {} + walker = os.walk(root_path) + + while True: + try: + dir, subdirs, files = next(walker) + for file in [f for f in files if f[-3:] == '.py']: + print(f'looking at file: {dir}/{file}') + path = Path(f'{dir}/{file}') + module_name = path.stem + relative_path = str(path).replace(f'{root_path}/', '') + if module_name == '__init__': + relative_path = relative_path.replace('/__init__.py', '') + + package = relative_path.replace('.py', '').replace('/', '.') + + for thing in deprecated_things: + module, _, function_name = thing.rpartition('::') + module_package = module.split('.')[0] + print(f'searching for {module_package}::{function_name}') + collector = DeprecatedThingUseCollector( + deprecated_thing=function_name, + modulename=module, + package=module_package + ) + + with open(path, 'r') as f: + collector.visit(ast.parse(f.read())) + + if thing in result: + result[thing].extend([f'{package}::{u}' for u in collector.used_at]) + else: + result[thing] = collector.used_at + except StopIteration: + break + + return result diff --git a/shitlist/cli.py b/shitlist/cli.py index 75e152f..71d7937 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -3,6 +3,7 @@ import os import click + import shitlist logger = logging.getLogger(__name__) @@ -26,10 +27,14 @@ def init(): click.echo("Initializing config file in .shitlist") cwd = os.getcwd() + deprecated_things = shitlist.gen_for_path(cwd) + + usages = shitlist.find_usages(cwd, deprecated_things) with open('.shitlist', 'w', encoding='utf-8') as file: data = dict( - decorated_funcs=shitlist.gen_for_path(cwd) + deprecated_things=deprecated_things, + usages=usages ) json.dump(data, file, ensure_ascii=False, indent=4) file.write('\n') diff --git a/shitlist/deprecated_thing_use_collector.py b/shitlist/deprecated_thing_use_collector.py new file mode 100644 index 0000000..5f7bef7 --- /dev/null +++ b/shitlist/deprecated_thing_use_collector.py @@ -0,0 +1,115 @@ +import ast +import collections +import pdb +from _ast import AST +from collections import ChainMap +from types import MappingProxyType as readonlydict +from typing import List + + +class DeprecatedThingUseCollector(ast.NodeVisitor): + def __init__(self, deprecated_thing: str, modulename, package=''): + self.deprecated_thing = deprecated_thing + self.modulename = modulename + # used to resolve from ... import ... references + self.package = package + self.modulepackage, _, self.modulestem = modulename.rpartition('.') + # track scope namespaces, with a mapping of imported names (bound name to original) + # If a name references None it is used for a different purpose in that scope + # and so masks a name in the global namespace. + self.import_scopes = ChainMap() + self.scope_names = collections.deque() + self.used_at = [] # list of (name, alias, line) entries + + def generic_visit(self, node): + """Called if no explicit visitor function exists for a node.""" + prev_field, prev_value = None, None + for field, value in ast.iter_fields(node): + if isinstance(value, list): + for item in value: + if isinstance(item, AST): + self.visit(item) + elif isinstance(value, AST): + self.visit(value) + elif field == "attr" and value == self.deprecated_thing: + self.visit_attr(value, prev_value) + rev_field, prev_value = field, value + + def visit_FunctionDef(self, node): + self.import_scopes = self.import_scopes.new_child() + self.scope_names.append(node.name) + self.generic_visit(node) + self.import_scopes = self.import_scopes.parents + self.scope_names.pop() + + def visit_Lambda(self, node): + # lambdas are just functions, albeit with no statements + # self.visit_Function(node) + pass + + def visit_ClassDef(self, node): + # class scope is a special local scope that is re-purposed to form + # the class attributes. By using a read-only dict proxy here this code + # we can expect an exception when a class body contains an import + # statement or uses names that'd mask an imported name. + self.import_scopes = self.import_scopes.new_child(readonlydict({})) + self.scope_names.append(node.name) + self.generic_visit(node) + self.import_scopes = self.import_scopes.parents + self.scope_names.pop() + + def visit_Import(self, node): + self.import_scopes.update({ + a.asname or a.name: a.name + for a in node.names + if a.name == self.modulename + }) + + def visit_ImportFrom(self, node): + # resolve relative imports; from . import , from .. import + source = node.module # can be None + if node.level: + package = self.package + if node.level > 1: + # go up levels as needed + package = '.'.join(self.package.split('.')[:-(node.level - 1)]) + source = f'{package}.{source}' if source else package + if self.modulename == source: + # names imported from our target module + self.import_scopes.update({ + a.asname or a.name: f'{self.modulename}.{a.name}' + for a in node.names + }) + elif self.modulepackage and self.modulepackage == source: + # from package import module import, where package.module is what we want + self.import_scopes.update({ + a.asname or a.name: self.modulename + for a in node.names + if a.name == self.modulestem + }) + + def visit_Name(self, node): + if not isinstance(node.ctx, ast.Load): + # store or del operation, means the name is masked in the current scope + try: + self.import_scopes[node.id] = None + except TypeError: + # class scope, which we made read-only. These names can't mask + # anything so just ignore these. + pass + return + # find scope this name was defined in, starting at the current scope + imported_name = self.import_scopes.get(node.id) + if imported_name is None: + return + # pdb.set_trace() + if self.deprecated_thing == node.id: + self.used_at.append(self.scope_names[0]) + + def visit_attr(self, node, prev_node): + imported_name = self.import_scopes.get(prev_node.id) + if imported_name is None: + return + # pdb.set_trace() + if self.deprecated_thing == node: + self.used_at.append(self.scope_names[0]) diff --git a/test_shitlist.py b/test_shitlist.py new file mode 100644 index 0000000..dc75993 --- /dev/null +++ b/test_shitlist.py @@ -0,0 +1,8 @@ +import ast + +from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector + +collector = DeprecatedThingUseCollector(deprecated_thing='', modulename='shitlist', package='shitlist') + +with open('tests/example_file.py', 'r') as f: + collector.visit(ast.parse(f.read())) diff --git a/tests/example_module/example_file.py b/tests/example_module/example_file.py index 009a139..80b74b3 100644 --- a/tests/example_module/example_file.py +++ b/tests/example_module/example_file.py @@ -1,6 +1,9 @@ import shitlist - +from tests.example_module import wrapped_2 +from tests import example_module @shitlist.deprecate def wrapped_3(): + wrapped_2() + example_module.wrapped_1() return 1 diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index 650d294..b943f10 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -52,14 +52,32 @@ def test_generate_shitlist_for_path(pytestconfig): result = shitlist.gen_for_path(str(test_root.parent)) expected_result = [ - 'example_module/__init__.py::wrapped_1', - 'example_module/__init__.py::wrapped_2', - 'example_module/example_file.py::wrapped_3', - 'example_module/submodule/__init__.py::wrapped', - 'example_module/submodule/example_file.py::wrapped', + 'example_module::wrapped_1', + 'example_module::wrapped_2', + 'example_module.example_file::wrapped_3', + 'example_module.submodule::wrapped', + 'example_module.submodule.example_file::wrapped', ] assert_that( result, has_items(*expected_result) ) + + +def test_find_usages(pytestconfig): + test_root = PosixPath(pytestconfig.rootpath) + + deprecated_things = [ + 'tests.example_module::wrapped_2', + 'tests.example_module::wrapped_1' + ] + + result = shitlist.find_usages(str(test_root.parent), deprecated_things) + + expected_result = { + 'tests.example_module::wrapped_2': ['example_module.example_file::wrapped_3'], + 'tests.example_module::wrapped_1': ['example_module.example_file::wrapped_3'] + } + assert_that(result, equal_to(expected_result)) + From 2d21c2ee0772f13c000dd9bd7464b66102a3f5f8 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Fri, 25 Nov 2022 09:42:42 +0000 Subject: [PATCH 04/25] Added test cli command to test if there have been added usages to deprecated things --- shitlist/__init__.py | 13 +++++++ shitlist/cli.py | 32 +++++++++++++++- tests/shitlist/test_shitlist.py | 66 +++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-) diff --git a/shitlist/__init__.py b/shitlist/__init__.py index 5ea269d..1458951 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -18,6 +18,10 @@ logger = logging.getLogger(__name__) +class DeprecatedException(Exception): + pass + + class ErrorLevel(enum.Enum): error = 'error' warn = 'warn' @@ -88,6 +92,15 @@ def gen_for_path(root_path: str): return sorted(list(result)) +def test(existing_config, new_config): + for deprecated in existing_config['deprecated_things']: + exiting_usages = set(existing_config['usage'].get(deprecated, [])) + new_usages = set(new_config['usage'].get(deprecated, [])) + dif = new_usages.difference(exiting_usages) + if len(dif) > 0: + raise DeprecatedException(f'Deprecated function {deprecated}, has new usages {dif}') + + def find_usages(root_path: str, deprecated_things: List[str]): result = {} walker = os.walk(root_path) diff --git a/shitlist/cli.py b/shitlist/cli.py index 71d7937..3f12f06 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -34,7 +34,7 @@ def init(): with open('.shitlist', 'w', encoding='utf-8') as file: data = dict( deprecated_things=deprecated_things, - usages=usages + usage=usages ) json.dump(data, file, ensure_ascii=False, indent=4) file.write('\n') @@ -51,7 +51,35 @@ def cmd2(): """Command on cli2""" -cli = click.CommandCollection(sources=[cli2, init_cli]) +@click.group() +def test_cli(): + pass + + +@test_cli.command() +def test(): + """Test new usages of deprecated code + """ + if not os.path.exists('.shitlist'): + logger.info('Cannot test there is no config file present') + + existing_config = {} + with open('.shitlist', 'r') as f: + existing_config = json.load(f) + + cwd = os.getcwd() + new_config = dict( + deprecated_things=shitlist.gen_for_path(cwd), + usage={} + ) + + shitlist.test( + existing_config=existing_config, + new_config=new_config + ) + + +cli = click.CommandCollection(sources=[cli2, init_cli, test_cli]) def main(): diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index b943f10..0a4ed21 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -65,6 +65,72 @@ def test_generate_shitlist_for_path(pytestconfig): ) +def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated_thing(): + existing_config = { + 'deprecated_things': [ + 'thing_1', + 'thing_2', + 'thing_3' + ], + 'usage': { + 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], + 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], + 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3'], + } + } + + new_config = { + 'deprecated_things': [ + 'thing_1', + 'thing_2', + 'thing_3' + ], + 'usage': { + 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], + 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], + 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3', 'usage_3_of_thing_3'], + } + } + + with pytest.raises(shitlist.DeprecatedException): + shitlist.test( + existing_config=existing_config, + new_config=new_config + ) + + +def test_shitlist_test_passes(): + existing_config = { + 'deprecated_things': [ + 'thing_1', + 'thing_2', + 'thing_3' + ], + 'usage': { + 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], + 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], + 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3'], + } + } + + new_config = { + 'deprecated_things': [ + 'thing_1', + 'thing_3' + ], + 'usage': { + 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], + 'thing_3': [], + 'thing_not_in_existing_config': ['usage'] + } + } + + shitlist.test( + existing_config=existing_config, + new_config=new_config + ) + + def test_find_usages(pytestconfig): test_root = PosixPath(pytestconfig.rootpath) From 6f344b91ed57a15fbdf649a864373a2062a52ec2 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Fri, 25 Nov 2022 10:14:21 +0000 Subject: [PATCH 05/25] Added Config class to replace dictionary used to hold the shitlist config --- shitlist/Config.py | 24 +++++++++++++++++++++ shitlist/__init__.py | 11 +++++----- shitlist/cli.py | 6 ++---- tests/shitlist/test_shitlist.py | 38 +++++++++++++++------------------ 4 files changed, 49 insertions(+), 30 deletions(-) create mode 100644 shitlist/Config.py diff --git a/shitlist/Config.py b/shitlist/Config.py new file mode 100644 index 0000000..469170c --- /dev/null +++ b/shitlist/Config.py @@ -0,0 +1,24 @@ +import json +from typing import List, Dict + + +class Config: + deprecated_things: List[str] + usage: Dict[str, List[str]] + + def __init__( + self, + deprecated_things: List[str], + usage: Dict[str, List[str]] + ): + self.usage = usage + self.deprecated_things = deprecated_things + + @staticmethod + def from_file(path: str) -> 'Config': + with open(path, 'r') as f: + file_contents = json.load(f) + return Config( + deprecated_things=file_contents['deprecated_things'], + usage=file_contents['usage'] + ) diff --git a/shitlist/__init__.py b/shitlist/__init__.py index 1458951..f9587bc 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -13,6 +13,7 @@ from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector from shitlist.decorator_use_collector import DecoratorUseCollector +from Config import Config ROOT_DIR = os.path.dirname(os.path.abspath(__file__)) logger = logging.getLogger(__name__) @@ -92,13 +93,13 @@ def gen_for_path(root_path: str): return sorted(list(result)) -def test(existing_config, new_config): - for deprecated in existing_config['deprecated_things']: - exiting_usages = set(existing_config['usage'].get(deprecated, [])) - new_usages = set(new_config['usage'].get(deprecated, [])) +def test(existing_config: Config, new_config: Config): + for thing in existing_config.deprecated_things: + exiting_usages = set(existing_config.usage.get(thing, [])) + new_usages = set(new_config.usage.get(thing, [])) dif = new_usages.difference(exiting_usages) if len(dif) > 0: - raise DeprecatedException(f'Deprecated function {deprecated}, has new usages {dif}') + raise DeprecatedException(f'Deprecated function {thing}, has new usages {dif}') def find_usages(root_path: str, deprecated_things: List[str]): diff --git a/shitlist/cli.py b/shitlist/cli.py index 3f12f06..62a32a2 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -63,12 +63,10 @@ def test(): if not os.path.exists('.shitlist'): logger.info('Cannot test there is no config file present') - existing_config = {} - with open('.shitlist', 'r') as f: - existing_config = json.load(f) + existing_config = shitlist.Config.from_file('.shitlist') cwd = os.getcwd() - new_config = dict( + new_config = shitlist.Config( deprecated_things=shitlist.gen_for_path(cwd), usage={} ) diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index 0a4ed21..eaadf87 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -1,15 +1,11 @@ -import importlib -import os from pathlib import PosixPath import pytest -from hamcrest import assert_that, equal_to, has_items, contains_inanyorder, only_contains +from hamcrest import assert_that, has_items import shitlist from shitlist import deprecate -from tests import example_module - @shitlist.deprecate def func_test(): @@ -66,31 +62,31 @@ def test_generate_shitlist_for_path(pytestconfig): def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated_thing(): - existing_config = { - 'deprecated_things': [ + existing_config = shitlist.Config( + deprecated_things=[ 'thing_1', 'thing_2', 'thing_3' ], - 'usage': { + usage={ 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3'], } - } + ) - new_config = { - 'deprecated_things': [ + new_config = shitlist.Config( + deprecated_things=[ 'thing_1', 'thing_2', 'thing_3' ], - 'usage': { + usage={ 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3', 'usage_3_of_thing_3'], } - } + ) with pytest.raises(shitlist.DeprecatedException): shitlist.test( @@ -100,30 +96,30 @@ def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated def test_shitlist_test_passes(): - existing_config = { - 'deprecated_things': [ + existing_config = shitlist.Config( + deprecated_things=[ 'thing_1', 'thing_2', 'thing_3' ], - 'usage': { + usage={ 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3'], } - } + ) - new_config = { - 'deprecated_things': [ + new_config = shitlist.Config( + deprecated_things=[ 'thing_1', 'thing_3' ], - 'usage': { + usage={ 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], 'thing_3': [], 'thing_not_in_existing_config': ['usage'] } - } + ) shitlist.test( existing_config=existing_config, From df7c72429bd315eeeaf00341af2ad822ec42cb4d Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Fri, 25 Nov 2022 12:04:10 +0000 Subject: [PATCH 06/25] Added update cli command to update the config to represent the current state of the codebase. Also does a test run to validate that the current usages are valid --- shitlist/Config.py | 24 ---------- shitlist/__init__.py | 22 ++++++++- shitlist/cli.py | 35 +++++++++++++- shitlist/foo.py | 47 ++++++++++++++++++ tests/shitlist/test_shitlist.py | 85 ++++++++++++++++++++++++++++++++- 5 files changed, 186 insertions(+), 27 deletions(-) delete mode 100644 shitlist/Config.py create mode 100644 shitlist/foo.py diff --git a/shitlist/Config.py b/shitlist/Config.py deleted file mode 100644 index 469170c..0000000 --- a/shitlist/Config.py +++ /dev/null @@ -1,24 +0,0 @@ -import json -from typing import List, Dict - - -class Config: - deprecated_things: List[str] - usage: Dict[str, List[str]] - - def __init__( - self, - deprecated_things: List[str], - usage: Dict[str, List[str]] - ): - self.usage = usage - self.deprecated_things = deprecated_things - - @staticmethod - def from_file(path: str) -> 'Config': - with open(path, 'r') as f: - file_contents = json.load(f) - return Config( - deprecated_things=file_contents['deprecated_things'], - usage=file_contents['usage'] - ) diff --git a/shitlist/__init__.py b/shitlist/__init__.py index f9587bc..c7c6629 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -13,7 +13,7 @@ from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector from shitlist.decorator_use_collector import DecoratorUseCollector -from Config import Config +from shitlist.foo import Config ROOT_DIR = os.path.dirname(os.path.abspath(__file__)) logger = logging.getLogger(__name__) @@ -140,3 +140,23 @@ def find_usages(root_path: str, deprecated_things: List[str]): break return result + + +def update(existing_config: Config, new_config: Config): + merged_config = Config( + deprecated_things=new_config.deprecated_things, + usage=new_config.usage, + ) + + merged_config.successfully_removed_things = [ + t for t in existing_config.deprecated_things if t not in new_config.deprecated_things + ] + + for thing, new_usage in new_config.usage.items(): + if thing in existing_config.usage: + existing_usage = existing_config.usage[thing] + removed_usages = [u for u in existing_usage if u not in new_usage] + if removed_usages: + merged_config.removed_usages[thing] = list(removed_usages) + + return merged_config diff --git a/shitlist/cli.py b/shitlist/cli.py index 62a32a2..495cf1e 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -77,7 +77,40 @@ def test(): ) -cli = click.CommandCollection(sources=[cli2, init_cli, test_cli]) +@click.group() +def update_cli(): + pass + + +@update_cli.command() +def update(): + """Update the config with removed usages + """ + existing_config = shitlist.Config.from_file('.shitlist') + + cwd = os.getcwd() + + new_config = shitlist.Config( + deprecated_things=shitlist.gen_for_path(cwd), + usage={} + ) + + shitlist.test( + existing_config=existing_config, + new_config=new_config + ) + + merged_config = shitlist.update( + existing_config=existing_config, + new_config=new_config + ) + with open('.shitlist', 'w', encoding='utf-8') as file: + json.dump(merged_config.__dict__(), file, ensure_ascii=False, indent=4) + file.write('\n') + file.flush() + + +cli = click.CommandCollection(sources=[cli2, init_cli, test_cli, update_cli]) def main(): diff --git a/shitlist/foo.py b/shitlist/foo.py new file mode 100644 index 0000000..42f488a --- /dev/null +++ b/shitlist/foo.py @@ -0,0 +1,47 @@ +import json +from typing import List, Dict + + +class Config: + deprecated_things: List[str] + usage: Dict[str, List[str]] + + def __init__( + self, + deprecated_things: List[str], + usage: Dict[str, List[str]], + removed_usages: Dict[str, List[str]] = dict(), + successfully_removed_things: List[str] = [] + ): + self.deprecated_things = deprecated_things + self.usage = usage + self.removed_usages = removed_usages + self.successfully_removed_things = successfully_removed_things + + @staticmethod + def from_file(path: str) -> 'Config': + with open(path, 'r') as f: + file_contents = json.load(f) + return Config( + deprecated_things=file_contents['deprecated_things'], + usage=file_contents['usage'] + ) + + def __eq__(self, other: 'Config'): + return ( + self.deprecated_things == other.deprecated_things and + self.usage == other.usage and + self.removed_usages == other.removed_usages and + self.successfully_removed_things == other.successfully_removed_things + ) + + def __dict__(self): + return dict( + deprecated_things=self.deprecated_things, + usage=self.usage, + removed_usages=self.removed_usages, + successfully_removed_things=self.successfully_removed_things + ) + + def __repr__(self): + return f'Config({self.__dict__()})' diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index eaadf87..be6e0a7 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -1,7 +1,7 @@ from pathlib import PosixPath import pytest -from hamcrest import assert_that, has_items +from hamcrest import assert_that, has_items, equal_to import shitlist from shitlist import deprecate @@ -95,6 +95,10 @@ def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated ) +def test_shitlist_test_should_fail_if_reintroduce_a_previously_deprecated_thing(): + assert_that(True, equal_to(False)) + + def test_shitlist_test_passes(): existing_config = shitlist.Config( deprecated_things=[ @@ -143,3 +147,82 @@ def test_find_usages(pytestconfig): } assert_that(result, equal_to(expected_result)) + +def test_update_config(): + existing_config = shitlist.Config( + deprecated_things=[ + 'thing_1', + 'thing_2', + 'thing_3' + ], + usage={ + 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], + 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], + 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3'], + } + ) + + new_config = shitlist.Config( + deprecated_things=[ + 'thing_1', + 'thing_3', + 'thing_not_in_existing_config' + ], + usage={ + 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], + 'thing_3': ['usage_2_of_thing_3'], + 'thing_not_in_existing_config': ['usage'] + } + ) + + updated_config = shitlist.update( + existing_config=existing_config, + new_config=new_config + ) + + expected_config = shitlist.Config( + deprecated_things=[ + 'thing_1', + 'thing_3', + 'thing_not_in_existing_config' + ], + usage={ + 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], + 'thing_3': ['usage_2_of_thing_3'], + 'thing_not_in_existing_config': ['usage'] + }, + removed_usages={ + 'thing_3': ['usage_1_of_thing_3'] + }, + successfully_removed_things=[ + 'thing_2', + ] + ) + + assert_config_are_equal(updated_config, expected_config) + + +def assert_config_are_equal(config_1: shitlist.Config, config_2: shitlist.Config): + assert_that( + config_1.deprecated_things, + equal_to(config_2.deprecated_things), + 'property deprecated_things are not equal' + ) + + assert_that( + config_1.usage, + equal_to(config_2.usage), + 'property usage are not equal' + ) + + assert_that( + config_1.removed_usages, + equal_to(config_2.removed_usages), + 'property removed_usages are not equal' + ) + + assert_that( + config_1.successfully_removed_things, + equal_to(config_2.successfully_removed_things), + 'property successfully_removed_things are not equal' + ) From 6aee01f61eb9292af2e3be77b8852c39f024c308 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sat, 26 Nov 2022 12:31:25 +0000 Subject: [PATCH 07/25] Remove cli_2 command --- shitlist/cli.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/shitlist/cli.py b/shitlist/cli.py index 495cf1e..5d22a61 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -18,10 +18,8 @@ def init_cli(): def init(): """Initialize the shitlist coniguration file - Initialize the .shitlist file in the project root directory + Creates the .shitlist file in the project root directory \f - - :param click.core.Context ctx: Click context. """ click.echo("Initializing config file in .shitlist") @@ -41,16 +39,6 @@ def init(): file.flush() -@click.group() -def cli2(): - pass - - -@cli2.command() -def cmd2(): - """Command on cli2""" - - @click.group() def test_cli(): pass @@ -59,6 +47,8 @@ def test_cli(): @test_cli.command() def test(): """Test new usages of deprecated code + + The test fails if you introduce new usages of deprecated code """ if not os.path.exists('.shitlist'): logger.info('Cannot test there is no config file present') @@ -85,6 +75,8 @@ def update_cli(): @update_cli.command() def update(): """Update the config with removed usages + + Update the shitlist config with any newly deprecated code """ existing_config = shitlist.Config.from_file('.shitlist') @@ -110,7 +102,7 @@ def update(): file.flush() -cli = click.CommandCollection(sources=[cli2, init_cli, test_cli, update_cli]) +cli = click.CommandCollection(sources=[init_cli, test_cli, update_cli]) def main(): From eb0735e0384db2c3a7f39063dd4aedfcc9f38dfd Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 27 Nov 2022 14:12:57 +0000 Subject: [PATCH 08/25] Added ability to ignore certain directories --- shitlist/__init__.py | 155 +++++++++++++++++++------------- shitlist/cli.py | 44 +++++---- shitlist/{foo.py => config.py} | 35 ++++++-- tests/shitlist/test_shitlist.py | 33 +++++-- 4 files changed, 165 insertions(+), 102 deletions(-) rename shitlist/{foo.py => config.py} (53%) diff --git a/shitlist/__init__.py b/shitlist/__init__.py index c7c6629..77d1f10 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -1,19 +1,13 @@ import ast import enum -import importlib import logging import os -import pkgutil -import sys -from typing import List, Callable, Dict -import logging -from pathlib import Path, PosixPath - -import click +from pathlib import PosixPath +from typing import List, Callable, Optional -from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector +from shitlist.config import Config from shitlist.decorator_use_collector import DecoratorUseCollector -from shitlist.foo import Config +from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector ROOT_DIR = os.path.dirname(os.path.abspath(__file__)) logger = logging.getLogger(__name__) @@ -66,29 +60,28 @@ def wrapper(): raise WrongTypeError() -def gen_for_path(root_path: str): +def gen_for_path( + root_path: PosixPath, + ignore_directories=[] +): result = set() - walker = os.walk(root_path) + walker = TreeWalker( + root_dir=root_path, + ignore_directories=ignore_directories + ) - while True: - try: - dir, subdirs, files = next(walker) - # print(f'Looking at files under: {dir}') - for file in [f for f in files if f[-3:] == '.py']: - print(f'looking at file: {dir}/{file}') - path = Path(f'{dir}/{file}') - module_name = path.stem - collector = DecoratorUseCollector(modulename=module_name) - with open(path, 'r') as f: - collector.visit(ast.parse(f.read())) - - module_relative_path = str(path).replace(root_path + '/', '') - if '__init__' in module_relative_path: - module_relative_path = module_relative_path.replace('/__init__.py', '') - module_relative_path = module_relative_path.replace('.py', '').replace('/', '.') - result.update([f'{module_relative_path}::{thing}' for thing in collector.nodes_with_decorators]) - except StopIteration: - break + while walker.has_next: + path = walker.next_file() + module_name = path.stem + collector = DecoratorUseCollector(modulename=module_name) + with open(path, 'r') as f: + collector.visit(ast.parse(f.read())) + + module_relative_path = str(path).replace(str(root_path) + '/', '') + if '__init__' in module_relative_path: + module_relative_path = module_relative_path.replace('/__init__.py', '') + module_relative_path = module_relative_path.replace('.py', '').replace('/', '.') + result.update([f'{module_relative_path}::{thing}' for thing in collector.nodes_with_decorators]) return sorted(list(result)) @@ -102,43 +95,78 @@ def test(existing_config: Config, new_config: Config): raise DeprecatedException(f'Deprecated function {thing}, has new usages {dif}') -def find_usages(root_path: str, deprecated_things: List[str]): - result = {} - walker = os.walk(root_path) +class TreeWalker: + has_next: bool - while True: - try: - dir, subdirs, files = next(walker) - for file in [f for f in files if f[-3:] == '.py']: - print(f'looking at file: {dir}/{file}') - path = Path(f'{dir}/{file}') - module_name = path.stem - relative_path = str(path).replace(f'{root_path}/', '') - if module_name == '__init__': - relative_path = relative_path.replace('/__init__.py', '') - - package = relative_path.replace('.py', '').replace('/', '.') - - for thing in deprecated_things: - module, _, function_name = thing.rpartition('::') - module_package = module.split('.')[0] - print(f'searching for {module_package}::{function_name}') - collector = DeprecatedThingUseCollector( - deprecated_thing=function_name, - modulename=module, - package=module_package - ) + def __init__(self, root_dir: PosixPath, ignore_directories: List[str] = []): + self.root_dir = root_dir + self._walker = os.walk(root_dir) + self._current_dir = None + self._current_files = [] + self.has_next = True + self.ignore_directories: List[PosixPath] = [root_dir / d for d in ignore_directories] - with open(path, 'r') as f: - collector.visit(ast.parse(f.read())) + self._gen_next() - if thing in result: - result[thing].extend([f'{package}::{u}' for u in collector.used_at]) - else: - result[thing] = collector.used_at + def _gen_next(self): + try: + self._current_dir, _, files = next(self._walker) + self._current_files = [f for f in files if f[-3:] == '.py'] #TODO could error on short named files + if not self._current_files or self.directory_should_be_ignored(self._current_dir): + self._gen_next() except StopIteration: - break + self.has_next = False + + def next_file(self) -> Optional[PosixPath]: + if self.has_next: + next_file = self._current_files.pop() + full_path = PosixPath(self._current_dir) / next_file + + if len(self._current_files) == 0: + self._gen_next() + + return full_path + + def directory_should_be_ignored(self, dir) -> bool: + return any([True for ig_dir in self.ignore_directories if str(ig_dir) in dir]) + + +def find_usages( + root_path: PosixPath, + deprecated_things: List[str], + ignore_directories=[] +): + result = {} + walker = TreeWalker( + root_dir=root_path, + ignore_directories=ignore_directories + ) + while walker.has_next: + path = walker.next_file() + module_name = path.stem + relative_path = str(path).replace(f'{root_path}/', '') + if module_name == '__init__': + relative_path = relative_path.replace('/__init__.py', '') + + package = relative_path.replace('.py', '').replace('/', '.') + + for thing in deprecated_things: + module, _, function_name = thing.rpartition('::') + module_package = module.split('.')[0] + collector = DeprecatedThingUseCollector( + deprecated_thing=function_name, + modulename=module, + package=module_package + ) + + with open(path, 'r') as f: + collector.visit(ast.parse(f.read())) + + if thing in result: + result[thing].extend([f'{package}::{u}' for u in collector.used_at]) + else: + result[thing] = [f'{package}::{u}' for u in collector.used_at] return result @@ -146,6 +174,7 @@ def update(existing_config: Config, new_config: Config): merged_config = Config( deprecated_things=new_config.deprecated_things, usage=new_config.usage, + ignore_directories=existing_config.ignore_directories ) merged_config.successfully_removed_things = [ diff --git a/shitlist/cli.py b/shitlist/cli.py index 5d22a61..2cf5fdc 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -1,6 +1,7 @@ import json import logging import os +from pathlib import PosixPath import click @@ -9,6 +10,9 @@ logger = logging.getLogger(__name__) +class NoConfigFileException(Exception): + pass + @click.group() def init_cli(): pass @@ -25,18 +29,9 @@ def init(): click.echo("Initializing config file in .shitlist") cwd = os.getcwd() - deprecated_things = shitlist.gen_for_path(cwd) - - usages = shitlist.find_usages(cwd, deprecated_things) - with open('.shitlist', 'w', encoding='utf-8') as file: - data = dict( - deprecated_things=deprecated_things, - usage=usages - ) - json.dump(data, file, ensure_ascii=False, indent=4) - file.write('\n') - file.flush() + config = shitlist.Config() + config.write('.shitlist') @click.group() @@ -52,13 +47,14 @@ def test(): """ if not os.path.exists('.shitlist'): logger.info('Cannot test there is no config file present') + raise NoConfigFileException() existing_config = shitlist.Config.from_file('.shitlist') - cwd = os.getcwd() - new_config = shitlist.Config( - deprecated_things=shitlist.gen_for_path(cwd), - usage={} + cwd = PosixPath(os.getcwd()) + new_config = shitlist.Config.from_path( + cwd, + ignore_directories=existing_config.ignore_directories ) shitlist.test( @@ -78,13 +74,17 @@ def update(): Update the shitlist config with any newly deprecated code """ + if not os.path.exists('.shitlist'): + logger.info('Cannot test there is no config file present') + raise NoConfigFileException() + existing_config = shitlist.Config.from_file('.shitlist') - cwd = os.getcwd() + cwd = PosixPath(os.getcwd()) - new_config = shitlist.Config( - deprecated_things=shitlist.gen_for_path(cwd), - usage={} + new_config = shitlist.Config.from_path( + cwd, + ignore_directories=existing_config.ignore_directories ) shitlist.test( @@ -96,10 +96,8 @@ def update(): existing_config=existing_config, new_config=new_config ) - with open('.shitlist', 'w', encoding='utf-8') as file: - json.dump(merged_config.__dict__(), file, ensure_ascii=False, indent=4) - file.write('\n') - file.flush() + + merged_config.write('.shitlist') cli = click.CommandCollection(sources=[init_cli, test_cli, update_cli]) diff --git a/shitlist/foo.py b/shitlist/config.py similarity index 53% rename from shitlist/foo.py rename to shitlist/config.py index 42f488a..d78247e 100644 --- a/shitlist/foo.py +++ b/shitlist/config.py @@ -1,32 +1,46 @@ import json +from pathlib import PosixPath from typing import List, Dict +import shitlist + class Config: + ignore_directories: List[str] deprecated_things: List[str] usage: Dict[str, List[str]] def __init__( self, - deprecated_things: List[str], - usage: Dict[str, List[str]], + deprecated_things: List[str] = [], + usage: Dict[str, List[str]] = dict(), removed_usages: Dict[str, List[str]] = dict(), - successfully_removed_things: List[str] = [] + successfully_removed_things: List[str] = [], + ignore_directories: List[str] = [] ): self.deprecated_things = deprecated_things self.usage = usage self.removed_usages = removed_usages self.successfully_removed_things = successfully_removed_things + self.ignore_directories = ignore_directories @staticmethod def from_file(path: str) -> 'Config': with open(path, 'r') as f: - file_contents = json.load(f) return Config( - deprecated_things=file_contents['deprecated_things'], - usage=file_contents['usage'] + **json.load(f) ) + @staticmethod + def from_path(path: PosixPath, ignore_directories: List[str] = []) -> 'Config': + deprecated_things = shitlist.gen_for_path(path, ignore_directories=ignore_directories) + usage = shitlist.find_usages(path, deprecated_things, ignore_directories=ignore_directories) + + return Config( + deprecated_things=deprecated_things, + usage=usage + ) + def __eq__(self, other: 'Config'): return ( self.deprecated_things == other.deprecated_things and @@ -37,11 +51,18 @@ def __eq__(self, other: 'Config'): def __dict__(self): return dict( + ignore_directories=self.ignore_directories, deprecated_things=self.deprecated_things, usage=self.usage, removed_usages=self.removed_usages, - successfully_removed_things=self.successfully_removed_things + successfully_removed_things=self.successfully_removed_things, ) def __repr__(self): return f'Config({self.__dict__()})' + + def write(self, path): + with open(path, 'w', encoding='utf-8') as file: + json.dump(self.__dict__(), file, ensure_ascii=False, indent=4) + file.write('\n') + file.flush() diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index be6e0a7..3b5e4d4 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -4,7 +4,7 @@ from hamcrest import assert_that, has_items, equal_to import shitlist -from shitlist import deprecate +from shitlist import deprecate, Config @shitlist.deprecate @@ -62,7 +62,7 @@ def test_generate_shitlist_for_path(pytestconfig): def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated_thing(): - existing_config = shitlist.Config( + existing_config = Config( deprecated_things=[ 'thing_1', 'thing_2', @@ -75,7 +75,7 @@ def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated } ) - new_config = shitlist.Config( + new_config = Config( deprecated_things=[ 'thing_1', 'thing_2', @@ -100,7 +100,7 @@ def test_shitlist_test_should_fail_if_reintroduce_a_previously_deprecated_thing( def test_shitlist_test_passes(): - existing_config = shitlist.Config( + existing_config = Config( deprecated_things=[ 'thing_1', 'thing_2', @@ -113,7 +113,7 @@ def test_shitlist_test_passes(): } ) - new_config = shitlist.Config( + new_config = Config( deprecated_things=[ 'thing_1', 'thing_3' @@ -149,7 +149,7 @@ def test_find_usages(pytestconfig): def test_update_config(): - existing_config = shitlist.Config( + existing_config = Config( deprecated_things=[ 'thing_1', 'thing_2', @@ -162,7 +162,7 @@ def test_update_config(): } ) - new_config = shitlist.Config( + new_config = Config( deprecated_things=[ 'thing_1', 'thing_3', @@ -180,7 +180,7 @@ def test_update_config(): new_config=new_config ) - expected_config = shitlist.Config( + expected_config = Config( deprecated_things=[ 'thing_1', 'thing_3', @@ -202,7 +202,22 @@ def test_update_config(): assert_config_are_equal(updated_config, expected_config) -def assert_config_are_equal(config_1: shitlist.Config, config_2: shitlist.Config): +def test_ignores_directories(pytestconfig): + test_root = PosixPath(pytestconfig.rootpath).parent + + walker = shitlist.TreeWalker( + root_dir=test_root / 'example_module', + ignore_directories=['submodule'] + ) + + assert_that(walker.has_next, equal_to(True)) + + assert_that(walker.next_file().name, equal_to('example_file.py')) + assert_that(walker.next_file().name, equal_to('__init__.py')) + + assert_that(walker.has_next, equal_to(False)) + +def assert_config_are_equal(config_1: Config, config_2: Config): assert_that( config_1.deprecated_things, equal_to(config_2.deprecated_things), From 4f736669cba5bd606a8fe3d3071f907e12094791 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 27 Nov 2022 14:42:18 +0000 Subject: [PATCH 09/25] Dont overwrite an already initialized config file --- shitlist/cli.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shitlist/cli.py b/shitlist/cli.py index 2cf5fdc..dd10605 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -25,6 +25,9 @@ def init(): Creates the .shitlist file in the project root directory \f """ + if os.path.exists('.shitlist'): + logger.info('Initialized file already exists') + return click.echo("Initializing config file in .shitlist") From b7ee2fb53f6c269a3863c15cf1f100dcb249d90f Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 27 Nov 2022 15:15:44 +0000 Subject: [PATCH 10/25] Logger logs to the terminal correctly --- shitlist/cli.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shitlist/cli.py b/shitlist/cli.py index dd10605..142907e 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -8,7 +8,10 @@ import shitlist logger = logging.getLogger(__name__) - +logging.basicConfig( + level=logging.INFO, + format='%(message)s' +) class NoConfigFileException(Exception): pass From f76230e090be911a9bd2749407301f3b35868d3d Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 27 Nov 2022 19:45:44 +0000 Subject: [PATCH 11/25] Fleshed out a bit of the readme --- README.md | 48 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 647e553..7285ea3 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,46 @@ # Shitlist -## To install +--- + +Shitlist is a deprecation tool for python. It checks that deprecated code is only lessened in your codebase by +testing that any new code doesn't use already deprecated code. + +Add shitlist to your build script to check that deprecated code is gradually remove from your codebase. + +Inspired by a post by Simon Eskildsen, [Shitlist driven development (2016)](https://sirupsen.com/shitlists) + +## Installation + + + +```bash +pip install shitlist +``` + + +## Usage + +Initialize the config file. This will create a file `.shitlist` in the project root directory +```bash +shitlist init +``` + +Deprecate some code +```python +import shitlist + +@shitlist.deprecate +def old_function(): + pass +``` + +Update the config file with the newly deprecated code. Shitlist will look for usages of all your deprecated code +and update the config file +```bash +shitlist update +``` + +Test ```bash -pip install --upgrade buildv -python -m build -pip install --force-reinstall dist/shitlist-0.0.1-py3-none-any.whl -``` \ No newline at end of file +shitlsit test +``` From 074fb78a5ec30f46b2dc7919f6930a4b3625270b Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 27 Nov 2022 20:15:29 +0000 Subject: [PATCH 12/25] Added logo to readme --- README.md | 3 ++- assets/logo.png | Bin 0 -> 4980 bytes 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 assets/logo.png diff --git a/README.md b/README.md index 7285ea3..423dcc4 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ -# Shitlist + +![Shitlist](https://github.com/samboyd/shitlist/assets/logo.svg?raw=true) --- diff --git a/assets/logo.png b/assets/logo.png new file mode 100644 index 0000000000000000000000000000000000000000..bc873ef3d7f1b308d3485cbd08076a398335499e GIT binary patch literal 4980 zcmV-)6N~JLP)Px|E=fc|RCr$Pod=L@MG=O-yNDz~F(GC#VGf8HRLlVs_3f?%#he2MR1BD7nG=W# zB?|BEDyWzv2F#)uG3NkgQ4oFZwf?>5Jdg9vO!u6dcI!=7-Fg%|r>Cc9rl+UV^e9j` zP&iOH0Q3qM3I_@Y3J1i2A`HqpC>$spSmi(w24x`>4ipX)VNljU;XvWQ|2R-IgR&Y5 z2MPyfrx`55fdkkN*wlkFdvL}kIBOaHRpzB|pm1O|9GHn_umDd0Uez+>!&l&S%kZDs zu%q-^II#6{U?##~f7o#+_~`&H-NO3QSKzQ^_|euoEoJ0|1JmcgOoV}kxd1N%p3*kt z9-MOlT>BLGiVXz1f9>7}?Y(KFN2XCFzl0E9}fD5v}DjyUMG&r!9 zg#iONa0%Ws;~ColxEXMD;4;9j!0&;d0$-)ub!I2v`M|lrUx7bY|DF&0lcCMzTh;&9 z@aKlg6ng-dtLWGrxP0|f8~y*^bmY|~folLi8v#eE>k9!Ft#~K8MeAjOPXS-gpjmvo zB5>u#PxEpm;IN8k8R()-#c$Cl+^3IG&`x44<&0P3+PRtst!2%H=Hb`CduApK7T~qO zV^bp;z$r^`@0`(&vF#mzCjxgha2^jlBXbj7*RgPK0(`CN`PsRyyci{a7 zeobQSYSpBt7UWW;Iank=0(euR(UXB!SV#c%x53fQYBCQjTaAD(2OG5Q6A2k}^eEsBT?h-lc_Z-XnFxb<*uMww^9*7Ic3+0ydD@QH=H?8In%dM| z;HXN7c)oo!@cKl$sXNn%edum^YbOq!zPf_MAQUWL0JOkja5dn^j=nz%+%eJaDwW;9 zLiGuTL1UxBHr4eah35vqH;j&+3B0faG14Ydfo{J7u8@g$nrrFEj^hr8Bn|!RQxygW z!5;q^o?y52GI#~{T!!y^+jnR;j>)ULm7&ZNFeR;M{{*eNV`{?*bY0M$JO=0Bze=5L z+`B!SQ*?YT^BUDgd#L@y#wWwzamKt#ixR$Euxv5!t!yJ+ywhGwIy9c+B`(oUHiZkO zA`CXbW#)!=Oc@Qc;B46Tv~G7nPe0?FJoP85yeBq7r?@-=F=(o$GVsZ35rY%)xnSi5 zk-;wpUj%O9VVd&}Z=rfoWq7sPnTF=dQ<1C6tKWM7rzQw^0q`t~!0PX_SW7Pkp5~-4 z)=c)Uo9d5a6$U!_H^D7s7@UKPuE4Hya4YYXAVTfa=inQg;A_i6m)^MGcFxm8mU^jU zx>r8Z=v?<(S#wh$_tJTZ2CJ6c#@}TRI5yG#F-{|r3}M>vF*w>WzWUp77!c*9`LJU$ zE#qOxF9*uaO{_cj{eX=QgtJ;pvLih=;U#d8zN%RW$9ROnU17VO;Tb)@6!j3fMVkQ} zIe=G;A)!e^P^WFun*scwGAK$F7wEboqgNYv)pZFK8LeUido@7j_7Osqi;^|ME*D32xuP#G=<*Y4TS!FznfyZ4r%e7R~EtjF@ zLDwbAJ0nd?*F4RG*7N2o=r}Zk-aMSz1G&p44g>hW((t}L7@;$rO0tMsuxeOoG*ZOm zgIhk*>7>(Jr>g~)IvJiu*8NGer@7=3sBd^zqGX(Sr{w~gH$}5z}h4^e}8B{9m;Is^4_&} z1QZomgHBeRu(~?P1*cenj|0E8I&A!1`51Cx#-QnLlR+=#JI}f2%UdEr6oXMB7(ZjU zWEI;WpLi}<+Klii8u913h%2pFfl@J$_HtDR-dV1tki2KtDu0CTn)F#4qi!n$(+3(K>ZFo!Pn*!P4aE2%%eXA$Z6G0Yv4*=%F2j@`{?$hO zL9>sP(Ng^?IJLVt6Xi7z1O50H2 zPYnkEa&N`p{(u&M1=4W-k!W-pAR|~8P#Co3L6NmuR_k1_;w!wKl(v2qCsDg^+y1Uo zR{0|q%yKP-v?&~02UZMZ*HHjlD~u!z7Ga+ONJX4FtiV3YLqkXx@VKjLygD@%hu_7I z9nvNvPfDX`1{Y71c-opM7)qNq1@UmNzZQ!aW4z1E% z!16D*;gnrONi|*k*)=|8!O4o70!1<^3(s@)YUVGbaB5+{E73qX3&(U}WOe091qFHU z-eLFG0lsa}q@=4<3`Xj6YZEJpE9Ld6anEWkg=4d>lxm-}3ymZUgnbdt9x;xf%Njhl zKfJql^(g3mtWeX`J-3an?^f31Uvwe5iaK=VWH|6|4i<5=Rp5-0uVSEx&`&$?v^9B& zk$LD%q6%?a$ysXe6^3neD}6c@3G4OJ%xW!#W3rOpOuNv?!e9Zs*3^z`t@X3x`|!XK z5h^%Jr*JOFPN7Bgz1$D0O{2{-4p_}Jb0V@b^G5L`C~!!2l^7`fSZ>W+u-29GRrbwA z+Gw9LdP;j!sInJkv6e#GjsRYoAlMB59!VG+2HT$x3e2$Xj9#XC+MU~jJvWZWaOsJA zZ=*><;RZ$a#Z_Nu#|aX7{pqh1n=ED?8Xu@TKHWguz07g_~EeRGEr%!?IPW!Wtqnw|#S9%Y9#vNQ@aW zCTe5ksPW1(@Y)WnynNm1RHQ?NT$GTdyLPPFQckdqvPEUYi-Ahdr3$U2){oqg3Vez| zS5Ly7Dqh6!j(WAR@;$;ZbdLaQwwd%=3h5EOb&0$ft@2K(*5T;Fpt>4(GPvr_w%DA8 zFv)Gd#h`kyxDjM58wGMoNh#V(cQp^_v{8^u48mx~R3Q?j-6|l-15$YvKobKQ|I}9( zQ2oaTBtATaZZ#~G)D#VlsnU+U<8LF+%J=jBZOKP!h&0t|z zY%&$y^nU~n;LxQYX<{A@=|v*ua@$8H>YIJg2|OxTX~Q*sX*)V)JYc!t6kAZcEHy8x znjCX|3Rf!Ws7a4iNTq9RzxuH5sX6rknlz_Ew$(1D#9TNxR=!7<>PD&gpTt@U`%_U= zt;HC)a#&o#KrbNem>EGffTu162|4p{L@(n0$Za1R^?9(I5w|v)=sDXevxQ#TP3$Cr9h@@Lz4L zgCKV;h5cHZj#9NHl#<{Q28(c$L4Itx7WmEj)t!1_SepAwiPJ8(ee1w_u=hqgDowN~ z6fF~|vA3UMgk zK=YXbZ>@zU4o?jORf*EO6^CQaEN$*tVw)f<(bs!xiANac=5 zyXYFc^)N>Wv)AL%oJ&gxhgwwbnv`Ky*OwUNXkEfF?X?urZ57K=dkMf)dAC)ZE@5z6 zxX^ChQ+9JFYozv$nu%e-3^7TOd|^7P6~ZezlZhXVfmpc#p&>F=iaf`Xu0)=cO+!ID zsi?KoTqu`80wD%DbT)BV#a~8=i%>sGJ~}W#w^!>Didn3skaWv1t@f@w3zskuwt0B} zw5l0(l?GK6B&pabh%E-4lDgNcn24;isSw*5zc(9Csjw>N9fM}}p=CVG3B$DvaoQAJ zOT=6E`mUQ+0!pFCF&HH-LO3lOidXAO36@E&rI0UH*?qP5RBZBT2Ew)o2MoF!ZMyF7 ztnYeo&yDG5LXSVz+h5 z8H3;Em^{5lA!BU}aSlWEd}V|%diq<9e$ zt5kRnVL3)7>!c6W-f;y9SsS)%bfEZT6$Vk8E(uDW#oabf-poj={-pN`GOM)|j@>F+ zqxNQkQjOFU1{+|9xnV_^SlzyyO*DQGL#BkQDZ zKE1B~t!zE-6K>Ys+xr}P8&51-guYSH59^u|;lYBNRJhH zsP^8+YSIS8h2fKEsU(-w<~er-uC;6=N5l|pfXmGd1D0~D5!EQ?`&&LJaoI{Bq@Fg5 zk)&FLIyF3R6f|*ICw#jH);_83ZAUkaL11XtHmXR(uB8(Sf@%_Jf%FeQU&GpRrQWDV9&8D}cU zk$Oj=rvFkEjuh{73U?`HYgP3Tt*W<}dO|)%mtInCD*&s_epclPkT*b4m8#L8r`q+< ztpsSQ#-sYPu4`(OTSjf#YpKaMD=1Dp)9ckTnxw_yf{Y~MU( z!EfSpl@1CA)*=Ti17hkau(ddzCQWDbVXw7LWTCHcV6AWyrp^J zK;ghT<3JGx>ukabxrGCh;y@7wlj3Z_TR5=JI8cPaI-9UUZsEYBI8cPaq&Qpf77na4 y4isUq&L*soTR1Q&4isT9Db5zWg#+u11OElC=tFxpqL69;0000 Date: Mon, 26 Dec 2022 12:30:34 +0000 Subject: [PATCH 13/25] The deprecate decorator now takes a string to deifne the alternative approach to the deprecated code --- README.md | 4 +- shitlist/__init__.py | 50 +++++++++++-------- shitlist/decorator_use_collector.py | 39 +++++++-------- tests/example_module/__init__.py | 4 +- tests/example_module/example_file.py | 3 +- tests/example_module/submodule/__init__.py | 2 +- .../example_module/submodule/example_file.py | 6 --- .../submodule/submodule_example_file.py | 8 +++ tests/shitlist/test_shitlist.py | 18 +++++-- 9 files changed, 76 insertions(+), 58 deletions(-) delete mode 100644 tests/example_module/submodule/example_file.py create mode 100644 tests/example_module/submodule/submodule_example_file.py diff --git a/README.md b/README.md index 423dcc4..031467c 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,9 @@ Deprecate some code ```python import shitlist -@shitlist.deprecate +@shitlist.deprecate( + alternative='You should use `new_function` because of X, Y & Z' +) def old_function(): pass ``` diff --git a/shitlist/__init__.py b/shitlist/__init__.py index 77d1f10..cf20c1b 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -17,6 +17,10 @@ class DeprecatedException(Exception): pass +class UndefinedAlternativeException(Exception): + pass + + class ErrorLevel(enum.Enum): error = 'error' warn = 'warn' @@ -37,27 +41,33 @@ def get_func_name(func: Callable): return f'{filepath}::{func_name}' -def deprecate(func): - # wrap a function - if type(func).__name__ == 'function': - def wrapper(): - if get_func_name(func) not in usages: - if error_level == ErrorLevel.error: - raise RuntimeError() - else: - func_name = func.__qualname__ - logger.info( - f'function {func_name} is registered on a shitlist and therefore should not' - f' be used by new code' - ) - func() +def deprecate(alternative: str): + if alternative is None: + raise UndefinedAlternativeException("alternative code not defined") + + def wrapped_deprecate(func): + # wrap a function + if type(func).__name__ == 'function': + def wrapper(): + if get_func_name(func) not in usages: + if error_level == ErrorLevel.error: + raise RuntimeError() + else: + func_name = func.__qualname__ + logger.info( + f'function {func_name} is registered on a shitlist and therefore should not' + f' be used by new code' + ) + func() + + wrapper.shitlist_deprecate = True + wrapper.wrapped_function = get_func_name(func) - wrapper.shitlist_deprecate = True - wrapper.wrapped_function = get_func_name(func) + return wrapper + else: + raise WrongTypeError() - return wrapper - else: - raise WrongTypeError() + return wrapped_deprecate def gen_for_path( @@ -111,7 +121,7 @@ def __init__(self, root_dir: PosixPath, ignore_directories: List[str] = []): def _gen_next(self): try: self._current_dir, _, files = next(self._walker) - self._current_files = [f for f in files if f[-3:] == '.py'] #TODO could error on short named files + self._current_files = [f for f in files if f[-3:] == '.py'] # TODO could error on short named files if not self._current_files or self.directory_should_be_ignored(self._current_dir): self._gen_next() except StopIteration: diff --git a/shitlist/decorator_use_collector.py b/shitlist/decorator_use_collector.py index 05f288b..3f92473 100644 --- a/shitlist/decorator_use_collector.py +++ b/shitlist/decorator_use_collector.py @@ -1,5 +1,4 @@ import ast -import pdb from collections import ChainMap from types import MappingProxyType as readonlydict @@ -19,31 +18,28 @@ def __init__(self, modulename, package=''): def _check_decorators(self, node): for decorator in node.decorator_list: + # This will match calls to `@shitlist.deprecate(..)` if ( - isinstance(decorator, ast.Attribute) and - isinstance(decorator.value, ast.Name) and - decorator.value.id == 'shitlist' and - decorator.attr == 'deprecate' + isinstance(decorator, ast.Call) and + 'func' in decorator.__dict__ and + isinstance(decorator.func, ast.Attribute) and + decorator.func.attr == 'deprecate' and + 'value' in decorator.func.__dict__ and + decorator.func.value.id == 'shitlist' ): self.nodes_with_decorators.append(node.name) + return + # This will match calls to `@deprecate(..)` where deprecated has been imported from shitlist if ( - isinstance(decorator, ast.Attribute) and - 'value' in decorator.value.__dict__ and - isinstance(decorator.value.value, ast.Name) and - decorator.value.value.id == 'shitlist' and - decorator.attr == 'deprecate' + isinstance(decorator, ast.Call) and + 'func' in decorator.__dict__ and + isinstance(decorator.func, ast.Name) and + decorator.func.id == 'deprecate' and + self.scopes.get('deprecate', None) == 'shitlist' ): self.nodes_with_decorators.append(node.name) - if ( - isinstance(decorator, ast.Name) and - decorator.id == 'deprecate' and - self.scopes.get('deprecate', None) == 'shitlist.deprecate' - ): - # pdb.set_trace() - self.nodes_with_decorators.append(node.name) - def visit_FunctionDef(self, node): self.scopes = self.scopes.new_child() self._check_decorators(node) @@ -87,12 +83,11 @@ def visit_ImportFrom(self, node): a.asname or a.name: f'{self.modulename}.{a.name}' for a in node.names }) - elif self.modulepackage and self.modulepackage == source: + else: # from package import module import, where package.module is what we want self.scopes.update({ - a.asname or a.name: self.modulename + a.asname or a.name: node.module for a in node.names - if a.name == self.modulestem }) def visit_Name(self, node): @@ -110,4 +105,4 @@ def visit_Name(self, node): if imported_name is None: return - self.used_at.append((imported_name, node.id, node.lineno)) \ No newline at end of file + self.used_at.append((imported_name, node.id, node.lineno)) diff --git a/tests/example_module/__init__.py b/tests/example_module/__init__.py index db4ca7b..41f3a9f 100644 --- a/tests/example_module/__init__.py +++ b/tests/example_module/__init__.py @@ -6,11 +6,11 @@ def not_wrapped(): return 0 -@shitlist.deprecate +@shitlist.deprecate(alternative='test') def wrapped_1(): return 1 -@shitlist.deprecate +@shitlist.deprecate(alternative='test') def wrapped_2(): return 1 diff --git a/tests/example_module/example_file.py b/tests/example_module/example_file.py index 80b74b3..1cb72c9 100644 --- a/tests/example_module/example_file.py +++ b/tests/example_module/example_file.py @@ -2,7 +2,8 @@ from tests.example_module import wrapped_2 from tests import example_module -@shitlist.deprecate + +@shitlist.deprecate(alternative='test') def wrapped_3(): wrapped_2() example_module.wrapped_1() diff --git a/tests/example_module/submodule/__init__.py b/tests/example_module/submodule/__init__.py index 6afd3e1..8595abd 100644 --- a/tests/example_module/submodule/__init__.py +++ b/tests/example_module/submodule/__init__.py @@ -6,6 +6,6 @@ def not_wrapped(): return 0 -@shitlist.deprecate +@shitlist.deprecate(alternative='test') def wrapped(): return 1 diff --git a/tests/example_module/submodule/example_file.py b/tests/example_module/submodule/example_file.py deleted file mode 100644 index 58edb89..0000000 --- a/tests/example_module/submodule/example_file.py +++ /dev/null @@ -1,6 +0,0 @@ -import shitlist - - -@shitlist.deprecate -def wrapped(): - return 1 diff --git a/tests/example_module/submodule/submodule_example_file.py b/tests/example_module/submodule/submodule_example_file.py new file mode 100644 index 0000000..a082d37 --- /dev/null +++ b/tests/example_module/submodule/submodule_example_file.py @@ -0,0 +1,8 @@ +from shitlist import deprecate + + +@deprecate( + alternative='test' +) +def wrapped(): + return 1 diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index 3b5e4d4..cd2b5bb 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -7,7 +7,7 @@ from shitlist import deprecate, Config -@shitlist.deprecate +@shitlist.deprecate(alternative='test') def func_test(): return 0 @@ -19,14 +19,14 @@ def another_func_test(): def test_raises_if_not_function(): with pytest.raises(shitlist.WrongTypeError): - @shitlist.deprecate + @shitlist.deprecate(alternative='test') class TestClass: pass def test_raises_runtime_error(): with pytest.raises(RuntimeError) as e_info: - @shitlist.deprecate + @shitlist.deprecate(alternative='test') def func(): return 0 @@ -34,7 +34,7 @@ def func(): def test_passes_check(mocker): - @shitlist.deprecate + @shitlist.deprecate(alternative='test') def func(): return 0 @@ -52,7 +52,7 @@ def test_generate_shitlist_for_path(pytestconfig): 'example_module::wrapped_2', 'example_module.example_file::wrapped_3', 'example_module.submodule::wrapped', - 'example_module.submodule.example_file::wrapped', + 'example_module.submodule.submodule_example_file::wrapped', ] assert_that( @@ -241,3 +241,11 @@ def assert_config_are_equal(config_1: Config, config_2: Config): equal_to(config_2.successfully_removed_things), 'property successfully_removed_things are not equal' ) + + +def test_deprecated_code_must_define_alternative(): + with pytest.raises(shitlist.UndefinedAlternativeException): + @deprecate(alternative=None) + def some_func(): + pass + From 6c7bbfc1cba1dfd60ba6a574b9eea7b7752a3710 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Thu, 29 Dec 2022 21:35:09 +0000 Subject: [PATCH 14/25] Refactor - renamed deprecated_thing to deprecated_code --- shitlist/__init__.py | 16 ++--- shitlist/config.py | 16 ++--- ...or.py => deprecated_code_use_collector.py} | 14 ++--- test_shitlist.py | 4 +- tests/conftest.py | 0 tests/shitlist/test_shitlist.py | 63 +++++++++---------- 6 files changed, 55 insertions(+), 58 deletions(-) rename shitlist/{deprecated_thing_use_collector.py => deprecated_code_use_collector.py} (92%) create mode 100644 tests/conftest.py diff --git a/shitlist/__init__.py b/shitlist/__init__.py index cf20c1b..d2094d8 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -7,7 +7,7 @@ from shitlist.config import Config from shitlist.decorator_use_collector import DecoratorUseCollector -from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector +from shitlist.deprecated_code_use_collector import DeprecatedCodeUseCollector ROOT_DIR = os.path.dirname(os.path.abspath(__file__)) logger = logging.getLogger(__name__) @@ -97,7 +97,7 @@ def gen_for_path( def test(existing_config: Config, new_config: Config): - for thing in existing_config.deprecated_things: + for thing in existing_config.deprecated_code: exiting_usages = set(existing_config.usage.get(thing, [])) new_usages = set(new_config.usage.get(thing, [])) dif = new_usages.difference(exiting_usages) @@ -143,7 +143,7 @@ def directory_should_be_ignored(self, dir) -> bool: def find_usages( root_path: PosixPath, - deprecated_things: List[str], + deprecated_code: List[str], ignore_directories=[] ): result = {} @@ -161,11 +161,11 @@ def find_usages( package = relative_path.replace('.py', '').replace('/', '.') - for thing in deprecated_things: + for thing in deprecated_code: module, _, function_name = thing.rpartition('::') module_package = module.split('.')[0] - collector = DeprecatedThingUseCollector( - deprecated_thing=function_name, + collector = DeprecatedCodeUseCollector( + deprecated_code=function_name, modulename=module, package=module_package ) @@ -182,13 +182,13 @@ def find_usages( def update(existing_config: Config, new_config: Config): merged_config = Config( - deprecated_things=new_config.deprecated_things, + deprecated_code=new_config.deprecated_code, usage=new_config.usage, ignore_directories=existing_config.ignore_directories ) merged_config.successfully_removed_things = [ - t for t in existing_config.deprecated_things if t not in new_config.deprecated_things + t for t in existing_config.deprecated_code if t not in new_config.deprecated_code ] for thing, new_usage in new_config.usage.items(): diff --git a/shitlist/config.py b/shitlist/config.py index d78247e..4d73a4e 100644 --- a/shitlist/config.py +++ b/shitlist/config.py @@ -7,18 +7,18 @@ class Config: ignore_directories: List[str] - deprecated_things: List[str] + deprecated_code: List[str] usage: Dict[str, List[str]] def __init__( self, - deprecated_things: List[str] = [], + deprecated_code: List[str] = [], usage: Dict[str, List[str]] = dict(), removed_usages: Dict[str, List[str]] = dict(), successfully_removed_things: List[str] = [], ignore_directories: List[str] = [] ): - self.deprecated_things = deprecated_things + self.deprecated_code = deprecated_code self.usage = usage self.removed_usages = removed_usages self.successfully_removed_things = successfully_removed_things @@ -33,17 +33,17 @@ def from_file(path: str) -> 'Config': @staticmethod def from_path(path: PosixPath, ignore_directories: List[str] = []) -> 'Config': - deprecated_things = shitlist.gen_for_path(path, ignore_directories=ignore_directories) - usage = shitlist.find_usages(path, deprecated_things, ignore_directories=ignore_directories) + deprecated_code = shitlist.gen_for_path(path, ignore_directories=ignore_directories) + usage = shitlist.find_usages(path, deprecated_code, ignore_directories=ignore_directories) return Config( - deprecated_things=deprecated_things, + deprecated_code=deprecated_code, usage=usage ) def __eq__(self, other: 'Config'): return ( - self.deprecated_things == other.deprecated_things and + self.deprecated_code == other.deprecated_code and self.usage == other.usage and self.removed_usages == other.removed_usages and self.successfully_removed_things == other.successfully_removed_things @@ -52,7 +52,7 @@ def __eq__(self, other: 'Config'): def __dict__(self): return dict( ignore_directories=self.ignore_directories, - deprecated_things=self.deprecated_things, + deprecated_code=self.deprecated_code, usage=self.usage, removed_usages=self.removed_usages, successfully_removed_things=self.successfully_removed_things, diff --git a/shitlist/deprecated_thing_use_collector.py b/shitlist/deprecated_code_use_collector.py similarity index 92% rename from shitlist/deprecated_thing_use_collector.py rename to shitlist/deprecated_code_use_collector.py index 5f7bef7..b94ba2b 100644 --- a/shitlist/deprecated_thing_use_collector.py +++ b/shitlist/deprecated_code_use_collector.py @@ -1,15 +1,13 @@ import ast import collections -import pdb from _ast import AST from collections import ChainMap from types import MappingProxyType as readonlydict -from typing import List -class DeprecatedThingUseCollector(ast.NodeVisitor): - def __init__(self, deprecated_thing: str, modulename, package=''): - self.deprecated_thing = deprecated_thing +class DeprecatedCodeUseCollector(ast.NodeVisitor): + def __init__(self, deprecated_code: str, modulename, package=''): + self.deprecated_code = deprecated_code self.modulename = modulename # used to resolve from ... import ... references self.package = package @@ -31,7 +29,7 @@ def generic_visit(self, node): self.visit(item) elif isinstance(value, AST): self.visit(value) - elif field == "attr" and value == self.deprecated_thing: + elif field == "attr" and value == self.deprecated_code: self.visit_attr(value, prev_value) rev_field, prev_value = field, value @@ -103,7 +101,7 @@ def visit_Name(self, node): if imported_name is None: return # pdb.set_trace() - if self.deprecated_thing == node.id: + if self.deprecated_code == node.id: self.used_at.append(self.scope_names[0]) def visit_attr(self, node, prev_node): @@ -111,5 +109,5 @@ def visit_attr(self, node, prev_node): if imported_name is None: return # pdb.set_trace() - if self.deprecated_thing == node: + if self.deprecated_code == node: self.used_at.append(self.scope_names[0]) diff --git a/test_shitlist.py b/test_shitlist.py index dc75993..2d82235 100644 --- a/test_shitlist.py +++ b/test_shitlist.py @@ -1,8 +1,8 @@ import ast -from shitlist.deprecated_thing_use_collector import DeprecatedThingUseCollector +from shitlist.deprecated_code_use_collector import DeprecatedCodeUseCollector -collector = DeprecatedThingUseCollector(deprecated_thing='', modulename='shitlist', package='shitlist') +collector = DeprecatedCodeUseCollector(deprecated_code='', modulename='shitlist', package='shitlist') with open('tests/example_file.py', 'r') as f: collector.visit(ast.parse(f.read())) diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index cd2b5bb..081cc4a 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -4,7 +4,6 @@ from hamcrest import assert_that, has_items, equal_to import shitlist -from shitlist import deprecate, Config @shitlist.deprecate(alternative='test') @@ -12,7 +11,7 @@ def func_test(): return 0 -@deprecate +@shitlist.deprecate(alternative='test') def another_func_test(): return 1 @@ -45,14 +44,14 @@ def func(): def test_generate_shitlist_for_path(pytestconfig): test_root = PosixPath(pytestconfig.rootpath) - result = shitlist.gen_for_path(str(test_root.parent)) + result = shitlist.gen_for_path(test_root) expected_result = [ - 'example_module::wrapped_1', - 'example_module::wrapped_2', - 'example_module.example_file::wrapped_3', - 'example_module.submodule::wrapped', - 'example_module.submodule.submodule_example_file::wrapped', + 'tests.example_module::wrapped_1', + 'tests.example_module::wrapped_2', + 'tests.example_module.example_file::wrapped_3', + 'tests.example_module.submodule::wrapped', + 'tests.example_module.submodule.submodule_example_file::wrapped', ] assert_that( @@ -62,8 +61,8 @@ def test_generate_shitlist_for_path(pytestconfig): def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated_thing(): - existing_config = Config( - deprecated_things=[ + existing_config = shitlist.Config( + deprecated_code=[ 'thing_1', 'thing_2', 'thing_3' @@ -75,8 +74,8 @@ def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated } ) - new_config = Config( - deprecated_things=[ + new_config = shitlist.Config( + deprecated_code=[ 'thing_1', 'thing_2', 'thing_3' @@ -100,8 +99,8 @@ def test_shitlist_test_should_fail_if_reintroduce_a_previously_deprecated_thing( def test_shitlist_test_passes(): - existing_config = Config( - deprecated_things=[ + existing_config = shitlist.Config( + deprecated_code=[ 'thing_1', 'thing_2', 'thing_3' @@ -113,8 +112,8 @@ def test_shitlist_test_passes(): } ) - new_config = Config( - deprecated_things=[ + new_config = shitlist.Config( + deprecated_code=[ 'thing_1', 'thing_3' ], @@ -134,23 +133,23 @@ def test_shitlist_test_passes(): def test_find_usages(pytestconfig): test_root = PosixPath(pytestconfig.rootpath) - deprecated_things = [ + deprecated_code = [ 'tests.example_module::wrapped_2', 'tests.example_module::wrapped_1' ] - result = shitlist.find_usages(str(test_root.parent), deprecated_things) + result = shitlist.find_usages(test_root, deprecated_code) expected_result = { - 'tests.example_module::wrapped_2': ['example_module.example_file::wrapped_3'], - 'tests.example_module::wrapped_1': ['example_module.example_file::wrapped_3'] + 'tests.example_module::wrapped_2': ['tests.example_module.example_file::wrapped_3'], + 'tests.example_module::wrapped_1': ['tests.example_module.example_file::wrapped_3'] } assert_that(result, equal_to(expected_result)) def test_update_config(): - existing_config = Config( - deprecated_things=[ + existing_config = shitlist.Config( + deprecated_code=[ 'thing_1', 'thing_2', 'thing_3' @@ -162,8 +161,8 @@ def test_update_config(): } ) - new_config = Config( - deprecated_things=[ + new_config = shitlist.Config( + deprecated_code=[ 'thing_1', 'thing_3', 'thing_not_in_existing_config' @@ -180,8 +179,8 @@ def test_update_config(): new_config=new_config ) - expected_config = Config( - deprecated_things=[ + expected_config = shitlist.Config( + deprecated_code=[ 'thing_1', 'thing_3', 'thing_not_in_existing_config' @@ -203,7 +202,7 @@ def test_update_config(): def test_ignores_directories(pytestconfig): - test_root = PosixPath(pytestconfig.rootpath).parent + test_root = PosixPath(pytestconfig.rootpath) / 'tests' walker = shitlist.TreeWalker( root_dir=test_root / 'example_module', @@ -217,11 +216,11 @@ def test_ignores_directories(pytestconfig): assert_that(walker.has_next, equal_to(False)) -def assert_config_are_equal(config_1: Config, config_2: Config): +def assert_config_are_equal(config_1: shitlist.Config, config_2: shitlist.Config): assert_that( - config_1.deprecated_things, - equal_to(config_2.deprecated_things), - 'property deprecated_things are not equal' + config_1.deprecated_code, + equal_to(config_2.deprecated_code), + 'property deprecated_code are not equal' ) assert_that( @@ -245,7 +244,7 @@ def assert_config_are_equal(config_1: Config, config_2: Config): def test_deprecated_code_must_define_alternative(): with pytest.raises(shitlist.UndefinedAlternativeException): - @deprecate(alternative=None) + @shitlist.deprecate(alternative=None) def some_func(): pass From cb384d1ce96b377b494cca126781990a5fd0e30b Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 1 Jan 2023 14:15:41 +0000 Subject: [PATCH 15/25] Explicitly defining packages in the build config to stop setuptools trying to automatically detect packages which causes it to complain about the assets directory --- pyproject.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 43574ab..1ddc711 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,4 +22,7 @@ classifiers = [ "Bug Tracker" = "https://github.com/samboyd/shitlist/issues" [project.scripts] -"shitlist"= "shitlist.cli:main" \ No newline at end of file +"shitlist"= "shitlist.cli:main" + +[tool.setuptools] +packages = ["shitlist"] From 1795bd60c50eb6c055cf147fc45988f77abe4236 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 1 Jan 2023 14:17:31 +0000 Subject: [PATCH 16/25] Added some helpful logging to the test cli command --- shitlist/cli.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shitlist/cli.py b/shitlist/cli.py index 142907e..38097e2 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -68,6 +68,8 @@ def test(): new_config=new_config ) + logger.info('Successfully tested deprecated code') + @click.group() def update_cli(): From 297b58c43de637c9d968355b8aecad14441b55d9 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sun, 1 Jan 2023 14:26:42 +0000 Subject: [PATCH 17/25] When running the test command display a warning if there is newly deprecated code --- shitlist/cli.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/shitlist/cli.py b/shitlist/cli.py index 38097e2..f2229bb 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -70,6 +70,12 @@ def test(): logger.info('Successfully tested deprecated code') + if [dc for dc in new_config.deprecated_code if dc not in existing_config.deprecated_code]: + logger.info( + 'Warning: there is newly deprecated code not represented in config. ' + 'Run `shitlist update`' + ) + @click.group() def update_cli(): @@ -84,7 +90,7 @@ def update(): """ if not os.path.exists('.shitlist'): logger.info('Cannot test there is no config file present') - raise NoConfigFileException() + return existing_config = shitlist.Config.from_file('.shitlist') From cf6de99e8e895a68e4b46187316c22889c25200f Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Mon, 2 Jan 2023 11:38:39 +0000 Subject: [PATCH 18/25] Freezing requirements versions --- requirements.txt | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b98f660..35beb28 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,15 @@ -click \ No newline at end of file +attrs==22.1.0 +build==0.9.0 +click==8.1.3 +exceptiongroup==1.0.0 +freezegun==1.2.2 +iniconfig==1.1.1 +packaging==21.3 +pep517==0.13.0 +PyHamcrest==2.0.4 +pyparsing==3.0.9 +pytest==7.2.0 +pytest-mock==3.10.0 +python-dateutil==2.8.2 +six==1.16.0 +tomli==2.0.1 From 7962ef51cfd2ba5e9a12f6c67f45c0419b5cd7b5 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Mon, 2 Jan 2023 11:47:47 +0000 Subject: [PATCH 19/25] Refactor to pull the config filepath to a local variable --- shitlist/cli.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/shitlist/cli.py b/shitlist/cli.py index f2229bb..77ce334 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -1,4 +1,3 @@ -import json import logging import os from pathlib import PosixPath @@ -13,9 +12,13 @@ format='%(message)s' ) +config_filepath = '.shitlist' + + class NoConfigFileException(Exception): pass + @click.group() def init_cli(): pass @@ -28,16 +31,16 @@ def init(): Creates the .shitlist file in the project root directory \f """ - if os.path.exists('.shitlist'): + if os.path.exists(config_filepath): logger.info('Initialized file already exists') return - click.echo("Initializing config file in .shitlist") + click.echo(f"Initializing config file in {config_filepath}") cwd = os.getcwd() config = shitlist.Config() - config.write('.shitlist') + config.write(config_filepath) @click.group() @@ -51,11 +54,11 @@ def test(): The test fails if you introduce new usages of deprecated code """ - if not os.path.exists('.shitlist'): + if not os.path.exists(config_filepath): logger.info('Cannot test there is no config file present') raise NoConfigFileException() - existing_config = shitlist.Config.from_file('.shitlist') + existing_config = shitlist.Config.from_file(config_filepath) cwd = PosixPath(os.getcwd()) new_config = shitlist.Config.from_path( @@ -88,11 +91,11 @@ def update(): Update the shitlist config with any newly deprecated code """ - if not os.path.exists('.shitlist'): + if not os.path.exists(config_filepath): logger.info('Cannot test there is no config file present') return - existing_config = shitlist.Config.from_file('.shitlist') + existing_config = shitlist.Config.from_file(config_filepath) cwd = PosixPath(os.getcwd()) @@ -111,7 +114,7 @@ def update(): new_config=new_config ) - merged_config.write('.shitlist') + merged_config.write(config_filepath) cli = click.CommandCollection(sources=[init_cli, test_cli, update_cli]) From 74b8454d5d2f3ca5096b2e36dd6162d65dcb8704 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Mon, 2 Jan 2023 14:59:20 +0000 Subject: [PATCH 20/25] Added first iteration of a progress chart to show the team how much of the deprecated code has been removed from the codebase --- shitlist/cli.py | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/shitlist/cli.py b/shitlist/cli.py index 77ce334..974e97e 100644 --- a/shitlist/cli.py +++ b/shitlist/cli.py @@ -117,7 +117,41 @@ def update(): merged_config.write(config_filepath) -cli = click.CommandCollection(sources=[init_cli, test_cli, update_cli]) +@click.group() +def progress_cli(): + pass + + +@progress_cli.command() +def progress(): + """Display deprecated code burn down table""" + + if not os.path.exists(config_filepath): + logger.info('Cannot test there is no config file present') + return + + config = shitlist.Config.from_file(config_filepath) + + progress_map = { + dc: (len(config.removed_usages.get(dc, [])), len(config.usage[dc]) + len(config.removed_usages.get(dc, []))) + for dc in config.deprecated_code + } + + logger.info('Progress removing deprecated code:') + + for dc in config.deprecated_code: + remaining, total = progress_map[dc] + + if total == 0: + logger.info(f' 100%\t[##########]\t{dc}') + else: + out_of_ten = int((remaining / total) * 10) + l = '#' * out_of_ten + s = '-' * (10 - out_of_ten) + logger.info(f' {out_of_ten * 10}%\t[{l}{s}]\t{dc}') + + +cli = click.CommandCollection(sources=[init_cli, test_cli, update_cli, progress_cli]) def main(): From 49bc9a31843b512f7c6693e3281fb8eea1744ef6 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Mon, 2 Jan 2023 15:31:50 +0000 Subject: [PATCH 21/25] Fixed bug where removed usages and successfully removed code wasnt being maintained over updates --- shitlist/__init__.py | 8 +++++--- tests/shitlist/test_shitlist.py | 12 +++++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/shitlist/__init__.py b/shitlist/__init__.py index d2094d8..8ddbf49 100644 --- a/shitlist/__init__.py +++ b/shitlist/__init__.py @@ -184,12 +184,14 @@ def update(existing_config: Config, new_config: Config): merged_config = Config( deprecated_code=new_config.deprecated_code, usage=new_config.usage, - ignore_directories=existing_config.ignore_directories + ignore_directories=existing_config.ignore_directories, + removed_usages=existing_config.removed_usages, + successfully_removed_things=existing_config.successfully_removed_things ) - merged_config.successfully_removed_things = [ + merged_config.successfully_removed_things.extend([ t for t in existing_config.deprecated_code if t not in new_config.deprecated_code - ] + ]) for thing, new_usage in new_config.usage.items(): if thing in existing_config.usage: diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index 081cc4a..77c4df0 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -158,7 +158,11 @@ def test_update_config(): 'thing_1': ['usage_1_of_thing_1', 'usage_2_of_thing_1'], 'thing_2': ['usage_1_of_thing_2', 'usage_2_of_thing_2'], 'thing_3': ['usage_1_of_thing_3', 'usage_2_of_thing_3'], - } + }, + removed_usages={ + 'thing_1': ['usage_3_of_thing_1'], + }, + successfully_removed_things=['thing_4'] ) new_config = shitlist.Config( @@ -191,10 +195,12 @@ def test_update_config(): 'thing_not_in_existing_config': ['usage'] }, removed_usages={ + 'thing_1': ['usage_3_of_thing_1'], 'thing_3': ['usage_1_of_thing_3'] }, successfully_removed_things=[ - 'thing_2', + 'thing_4', + 'thing_2' ] ) @@ -216,6 +222,7 @@ def test_ignores_directories(pytestconfig): assert_that(walker.has_next, equal_to(False)) + def assert_config_are_equal(config_1: shitlist.Config, config_2: shitlist.Config): assert_that( config_1.deprecated_code, @@ -247,4 +254,3 @@ def test_deprecated_code_must_define_alternative(): @shitlist.deprecate(alternative=None) def some_func(): pass - From 5a7bcfba3809e6288af0eeb3437b23dd465bfb7d Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Mon, 2 Jan 2023 16:43:04 +0000 Subject: [PATCH 22/25] Fixed typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 031467c..193fb1e 100644 --- a/README.md +++ b/README.md @@ -45,5 +45,5 @@ shitlist update Test ```bash -shitlsit test +shitlist test ``` From 762fff65801cb08fa2dd06dff375fc308063a9af Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sat, 14 Jan 2023 13:52:50 +0000 Subject: [PATCH 23/25] Remove file throwing off pytest --- test_shitlist.py | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 test_shitlist.py diff --git a/test_shitlist.py b/test_shitlist.py deleted file mode 100644 index 2d82235..0000000 --- a/test_shitlist.py +++ /dev/null @@ -1,8 +0,0 @@ -import ast - -from shitlist.deprecated_code_use_collector import DeprecatedCodeUseCollector - -collector = DeprecatedCodeUseCollector(deprecated_code='', modulename='shitlist', package='shitlist') - -with open('tests/example_file.py', 'r') as f: - collector.visit(ast.parse(f.read())) From 438989861f76aa75b1961300a7da2bed35c3e566 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sat, 14 Jan 2023 13:53:26 +0000 Subject: [PATCH 24/25] Add pytest section to pyproject.toml, setting up PYTHONPATH --- pyproject.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 1ddc711..4818815 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,3 +26,9 @@ classifiers = [ [tool.setuptools] packages = ["shitlist"] + +[tool.pytest.ini_options] +pythonpath = [ + "." +] +addopts = "--ignore=dist --ignore=assets" From c06a70bcedcb5d45b82ff2f7679106768a668b57 Mon Sep 17 00:00:00 2001 From: Sam Boyd Date: Sat, 14 Jan 2023 13:54:42 +0000 Subject: [PATCH 25/25] Skipping tests that arent implemented yet --- tests/shitlist/test_shitlist.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/shitlist/test_shitlist.py b/tests/shitlist/test_shitlist.py index 77c4df0..06d44a2 100644 --- a/tests/shitlist/test_shitlist.py +++ b/tests/shitlist/test_shitlist.py @@ -32,6 +32,7 @@ def func(): func() +@pytest.mark.skip def test_passes_check(mocker): @shitlist.deprecate(alternative='test') def func(): @@ -94,6 +95,7 @@ def test_shitlist_test_throws_an_exception_if_theres_a_new_usage_of_a_deprecated ) +@pytest.mark.skip def test_shitlist_test_should_fail_if_reintroduce_a_previously_deprecated_thing(): assert_that(True, equal_to(False))