From 311c6ac439b5048c4743a2a8d3e72528ae5c3f98 Mon Sep 17 00:00:00 2001 From: Brian Mesick Date: Tue, 29 Oct 2024 15:15:44 -0400 Subject: [PATCH 1/5] fix: Propagate app_name to DjanoSearch, fix naming The app_name option should always filter on the whole DjangoSearch when present, not just the report. This fixes that. Also updates some variable and output names to be more clear about the state of the models. --- code_annotations/cli.py | 12 +++++++-- code_annotations/find_django.py | 36 ++++++++++++-------------- tests/test_django_generate_safelist.py | 4 +-- tests/test_find_django.py | 17 ++++++++++++ 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/code_annotations/cli.py b/code_annotations/cli.py index 06c3549..a0d65aa 100644 --- a/code_annotations/cli.py +++ b/code_annotations/cli.py @@ -40,7 +40,7 @@ def entry_point(): show_default=True, help='List all locally defined models (in the current repo) that require annotations.', ) -@click.option('--app_name', default='', help='(Optional) App name for which coverage is generated.') +@click.option('--app_name', default=None, help='(Optional) App name for which coverage is generated.') @click.option('--report_path', default=None, help='Location to write the report') @click.option('-v', '--verbosity', count=True, help='Verbosity level (-v through -vvv)') @click.option('--lint/--no_lint', help='Enable or disable linting checks', default=False, show_default=True) @@ -62,8 +62,16 @@ def django_find_annotations( """ try: start_time = datetime.datetime.utcnow() + + if not coverage and not seed_safelist and not list_local_models and not lint and not report and not coverage: + click.echo( + "No actions specified. Please specify one or more of --seed_safelist, --list_local_models, " + "--lint, --report, or --coverage" + ) + sys.exit(1) + config = AnnotationConfig(config_file, report_path, verbosity) - searcher = DjangoSearch(config) + searcher = DjangoSearch(config, app_name) # Early out if we're trying to do coverage, but a coverage target is not configured if coverage and not config.coverage_target: diff --git a/code_annotations/find_django.py b/code_annotations/find_django.py index a62b5a5..7f612b4 100644 --- a/code_annotations/find_django.py +++ b/code_annotations/find_django.py @@ -21,7 +21,7 @@ class DjangoSearch(BaseSearch): Handles Django model comment searching for annotations. """ - def __init__(self, config): + def __init__(self, config, app_name=None): """ Initialize for DjangoSearch. @@ -29,19 +29,19 @@ def __init__(self, config): config: Configuration file path """ super().__init__(config) - self.local_models, self.non_local_models, total, needing_annotation = self.get_models_requiring_annotations() + self.local_models, self.non_local_models, total, annotation_eligible = self.get_models_requiring_annotations(app_name) self.model_counts = { 'total': total, 'annotated': 0, 'unannotated': 0, - 'needing_annotation': len(needing_annotation), - 'not_needing_annotation': total - len(needing_annotation), + 'annotation_eligible': len(annotation_eligible), + 'not_annotation_eligible': total - len(annotation_eligible), 'safelisted': 0 } self.uncovered_model_ids = set() self.echo.echo_vvv('Local models:\n ' + '\n '.join([str(m) for m in self.local_models]) + '\n') self.echo.echo_vvv('Non-local models:\n ' + '\n '.join([str(m) for m in self.non_local_models]) + '\n') - self.echo.echo_vv('The following models require annotations:\n ' + '\n '.join(needing_annotation) + '\n') + self.echo.echo_vv('The following models require annotations:\n ' + '\n '.join(annotation_eligible) + '\n') def _increment_count(self, count_type, incr_by=1): self.model_counts[count_type] += incr_by @@ -82,7 +82,7 @@ def seed_safelist(self): def list_local_models(self): """ - Dump a list of models in the local code tree that need annotations to stdout. + Dump a list of models in the local code tree that are annotation eligible to stdout. """ if self.local_models: self.echo( @@ -249,16 +249,16 @@ def check_coverage(self): Returns: Bool indicating whether or not the number of annotated models covers a percentage - of total models needing annotations greater than or equal to the configured + of total annotation eligible models greater than or equal to the configured coverage_target. """ self.echo("\nModel coverage report") self.echo("-" * 40) self.echo("Found {total} total models.".format(**self.model_counts)) - self.echo("{needing_annotation} needed annotation, {annotated} were annotated.".format(**self.model_counts)) + self.echo("{annotation_eligible} were eligible for annotation, {annotated} were annotated.".format(**self.model_counts)) - if self.model_counts['needing_annotation'] > 0: - pct = float(self.model_counts['annotated']) / float(self.model_counts['needing_annotation']) * 100.0 + if self.model_counts['annotation_eligible'] > 0: + pct = float(self.model_counts['annotated']) / float(self.model_counts['annotation_eligible']) * 100.0 pct = round(pct, 1) else: pct = 100.0 @@ -359,7 +359,7 @@ def setup_django(): django.setup() @staticmethod - def get_models_requiring_annotations(): + def get_models_requiring_annotations(app_name=None): """ Determine all local and non-local models via django model introspection. @@ -367,19 +367,17 @@ def get_models_requiring_annotations(): edX). This is a compromise in accuracy in order to simplify the generation of this list, and also to ease the transition from zero to 100% annotations in edX satellite repositories. - - Returns: - tuple: - 2-tuple where the first item is a set of local models, and the - second item is a set of non-local models. """ DjangoSearch.setup_django() local_models = set() non_local_models = set() - models_requiring_annotations = [] + annotation_eligible_models = [] total_models = 0 for app in apps.get_app_configs(): + if app_name and not app.name.endswith(app_name): + continue + for root_model in app.get_models(): total_models += 1 if DjangoSearch.requires_annotations(root_model): @@ -388,6 +386,6 @@ def get_models_requiring_annotations(): else: local_models.add(root_model) - models_requiring_annotations.append(DjangoSearch.get_model_id(root_model)) + annotation_eligible_models.append(DjangoSearch.get_model_id(root_model)) - return local_models, non_local_models, total_models, models_requiring_annotations + return local_models, non_local_models, total_models, annotation_eligible_models diff --git a/tests/test_django_generate_safelist.py b/tests/test_django_generate_safelist.py index 00c71bb..e6f0b08 100644 --- a/tests/test_django_generate_safelist.py +++ b/tests/test_django_generate_safelist.py @@ -43,7 +43,7 @@ def test_seeding_safelist(local_models, non_local_models, **kwargs): local_models, non_local_models, 0, # Number of total models found, irrelevant here - [] # List of model ids that need anntations, irrelevant here + set() # List of model ids that are eligible for annotation, irrelevant here ) def test_safelist_callback(): @@ -73,7 +73,7 @@ def test_safelist_exists(**kwargs): Test the success case for seeding the safelist. """ mock_get_models_requiring_annotations = kwargs['get_models_requiring_annotations'] - mock_get_models_requiring_annotations.return_value = ([], [], 0, []) + mock_get_models_requiring_annotations.return_value = (set(), set(), 0, []) result = call_script_isolated( ['django_find_annotations', '--config_file', 'test_config.yml', '--seed_safelist'] diff --git a/tests/test_find_django.py b/tests/test_find_django.py index a5b20e4..bf94e44 100644 --- a/tests/test_find_django.py +++ b/tests/test_find_django.py @@ -472,3 +472,20 @@ def test_setup_django(mock_django_setup): """ mock_django_setup.return_value = True DjangoSearch.setup_django() + + +@patch.multiple( + 'code_annotations.find_django.DjangoSearch', + get_models_requiring_annotations=DEFAULT +) +def test_find_django_no_action(**kwargs): + """ + Test that we fail when there is no action specified. + """ + + result = call_script_isolated( + ['django_find_annotations', '--config_file', 'test_config.yml'], + ) + + assert result.exit_code == EXIT_CODE_FAILURE + assert 'No actions specified' in result.output From 666aaf16694c6a893a6188049f7e0c785407137f Mon Sep 17 00:00:00 2001 From: Brian Mesick Date: Fri, 1 Nov 2024 14:44:20 -0400 Subject: [PATCH 2/5] build: Stop testing against Python 3.8 --- .github/workflows/ci.yml | 44 ++++++++++++++++++++-------------------- tox.ini | 3 +-- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c520ac..3135678 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,7 +5,7 @@ on: branches: [master] pull_request: branches: - - '**' + - "**" jobs: run_tests: @@ -14,31 +14,31 @@ jobs: strategy: matrix: os: [ubuntu-latest] - python-version: ['3.8', '3.11', '3.12'] + python-version: ["3.11", "3.12"] toxenv: [quality, docs, django42] steps: - - uses: actions/checkout@v2 - - name: setup python - uses: actions/setup-python@v2 - with: - python-version: ${{ matrix.python-version }} + - uses: actions/checkout@v2 + - name: setup python + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} - - name: Install pip - run: pip install -r requirements/pip.txt + - name: Install pip + run: pip install -r requirements/pip.txt - - name: Install Dependencies - run: pip install -r requirements/ci.txt + - name: Install Dependencies + run: pip install -r requirements/ci.txt - - name: Run Tests - env: - TOXENV: ${{ matrix.toxenv }} - run: tox + - name: Run Tests + env: + TOXENV: ${{ matrix.toxenv }} + run: tox - - name: Run Coverage - if: matrix.python-version == '3.8' && matrix.toxenv=='django42' - uses: codecov/codecov-action@v4 - with: - token: ${{ secrets.CODECOV_TOKEN }} - flags: unittests - fail_ci_if_error: true + - name: Run Coverage + if: matrix.python-version == '3.11' && matrix.toxenv=='django42' + uses: codecov/codecov-action@v4 + with: + token: ${{ secrets.CODECOV_TOKEN }} + flags: unittests + fail_ci_if_error: true diff --git a/tox.ini b/tox.ini index 1bc1696..dd9fb64 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{38,311,312}-django{42} +envlist = py{311,312}-django{42} [doc8] ignore = D001 @@ -54,4 +54,3 @@ commands = pydocstyle code_annotations tests setup.py isort --check-only --diff tests test_utils code_annotations setup.py make selfcheck - From 6aac6b7c89e36835bcf2f829e6d0ceb451832f8d Mon Sep 17 00:00:00 2001 From: Brian Mesick Date: Fri, 1 Nov 2024 14:44:37 -0400 Subject: [PATCH 3/5] style: Fix up formatting --- code_annotations/cli.py | 159 ++++++++++++-------- code_annotations/find_django.py | 248 ++++++++++++++++++++------------ 2 files changed, 259 insertions(+), 148 deletions(-) diff --git a/code_annotations/cli.py b/code_annotations/cli.py index a0d65aa..853da13 100644 --- a/code_annotations/cli.py +++ b/code_annotations/cli.py @@ -1,6 +1,7 @@ """ Command line interface for code annotation tools. """ + import datetime import sys import traceback @@ -21,41 +22,60 @@ def entry_point(): """ -@entry_point.command('django_find_annotations') +@entry_point.command("django_find_annotations") +@click.option( + "--config_file", + default=".annotations", + help="Path to the configuration file", + type=click.Path(exists=True, dir_okay=False, resolve_path=True), +) +@click.option( + "--seed_safelist/--no_safelist", + default=False, + show_default=True, + help="Generate an initial safelist file based on the current Django environment.", +) @click.option( - '--config_file', - default='.annotations', - help='Path to the configuration file', - type=click.Path(exists=True, dir_okay=False, resolve_path=True) + "--list_local_models/--no_list_models", + default=False, + show_default=True, + help="List all locally defined models (in the current repo) that require annotations.", +) +@click.option( + "--app_name", + default=None, + help="(Optional) App name for which coverage is generated.", +) +@click.option("--report_path", default=None, help="Location to write the report") +@click.option("-v", "--verbosity", count=True, help="Verbosity level (-v through -vvv)") +@click.option( + "--lint/--no_lint", + help="Enable or disable linting checks", + default=False, + show_default=True, ) @click.option( - '--seed_safelist/--no_safelist', + "--report/--no_report", + help="Enable or disable writing the report", default=False, show_default=True, - help='Generate an initial safelist file based on the current Django environment.', ) @click.option( - '--list_local_models/--no_list_models', + "--coverage/--no_coverage", + help="Enable or disable coverage checks", default=False, show_default=True, - help='List all locally defined models (in the current repo) that require annotations.', ) -@click.option('--app_name', default=None, help='(Optional) App name for which coverage is generated.') -@click.option('--report_path', default=None, help='Location to write the report') -@click.option('-v', '--verbosity', count=True, help='Verbosity level (-v through -vvv)') -@click.option('--lint/--no_lint', help='Enable or disable linting checks', default=False, show_default=True) -@click.option('--report/--no_report', help='Enable or disable writing the report', default=False, show_default=True) -@click.option('--coverage/--no_coverage', help='Enable or disable coverage checks', default=False, show_default=True) def django_find_annotations( - config_file, - seed_safelist, - list_local_models, - app_name, - report_path, - verbosity, - lint, - report, - coverage + config_file, + seed_safelist, + list_local_models, + app_name, + report_path, + verbosity, + lint, + report, + coverage, ): """ Subcommand for dealing with annotations in Django models. @@ -63,7 +83,13 @@ def django_find_annotations( try: start_time = datetime.datetime.utcnow() - if not coverage and not seed_safelist and not list_local_models and not lint and not report and not coverage: + if ( + not coverage + and not seed_safelist + and not list_local_models + and not lint + and not report + ): click.echo( "No actions specified. Please specify one or more of --seed_safelist, --list_local_models, " "--lint, --report, or --coverage" @@ -75,7 +101,9 @@ def django_find_annotations( # Early out if we're trying to do coverage, but a coverage target is not configured if coverage and not config.coverage_target: - raise ConfigurationException("Please add 'coverage_target' to your configuration before running --coverage") + raise ConfigurationException( + "Please add 'coverage_target' to your configuration before running --coverage" + ) if seed_safelist: searcher.seed_safelist() @@ -115,32 +143,45 @@ def django_find_annotations( annotation_count += len(annotated_models[filename]) elapsed = datetime.datetime.utcnow() - start_time - click.echo("Search found {} annotations in {} seconds.".format( - annotation_count, elapsed.total_seconds() - )) - + click.echo( + "Search found {} annotations in {} seconds.".format( + annotation_count, elapsed.total_seconds() + ) + ) except Exception as exc: click.echo(traceback.print_exc()) fail(str(exc)) -@entry_point.command('static_find_annotations') +@entry_point.command("static_find_annotations") +@click.option( + "--config_file", + default=".annotations", + help="Path to the configuration file", + type=click.Path(exists=True, dir_okay=False, resolve_path=True), +) @click.option( - '--config_file', - default='.annotations', - help='Path to the configuration file', - type=click.Path(exists=True, dir_okay=False, resolve_path=True) + "--source_path", + help="Location of the source code to search", + type=click.Path(exists=True, dir_okay=True, resolve_path=True), ) +@click.option("--report_path", default=None, help="Location to write the report") +@click.option("-v", "--verbosity", count=True, help="Verbosity level (-v through -vvv)") @click.option( - '--source_path', - help='Location of the source code to search', - type=click.Path(exists=True, dir_okay=True, resolve_path=True) + "--lint/--no_lint", + help="Enable or disable linting checks", + default=True, + show_default=True, +) +@click.option( + "--report/--no_report", + help="Enable or disable writing the report file", + default=True, + show_default=True, ) -@click.option('--report_path', default=None, help='Location to write the report') -@click.option('-v', '--verbosity', count=True, help='Verbosity level (-v through -vvv)') -@click.option('--lint/--no_lint', help='Enable or disable linting checks', default=True, show_default=True) -@click.option('--report/--no_report', help='Enable or disable writing the report file', default=True, show_default=True) -def static_find_annotations(config_file, source_path, report_path, verbosity, lint, report): +def static_find_annotations( + config_file, source_path, report_path, verbosity, lint, report +): """ Subcommand to find annotations via static file analysis. """ @@ -184,18 +225,14 @@ def static_find_annotations(config_file, source_path, report_path, verbosity, li @entry_point.command("generate_docs") @click.option( - '--config_file', - default='.annotations', - help='Path to the configuration file', - type=click.Path(exists=True, dir_okay=False) + "--config_file", + default=".annotations", + help="Path to the configuration file", + type=click.Path(exists=True, dir_okay=False), ) -@click.option('-v', '--verbosity', count=True, help='Verbosity level (-v through -vvv)') -@click.argument("report_files", type=click.File('r'), nargs=-1) -def generate_docs( - config_file, - verbosity, - report_files -): +@click.option("-v", "--verbosity", count=True, help="Verbosity level (-v through -vvv)") +@click.argument("report_files", type=click.File("r"), nargs=-1) +def generate_docs(config_file, verbosity, report_files): """ Generate documentation from a code annotations report. """ @@ -205,15 +242,19 @@ def generate_docs( config = AnnotationConfig(config_file, verbosity) for key in ( - 'report_template_dir', - 'rendered_report_dir', - 'rendered_report_file_extension', - 'rendered_report_source_link_prefix' + "report_template_dir", + "rendered_report_dir", + "rendered_report_file_extension", + "rendered_report_source_link_prefix", ): if not getattr(config, key): raise ConfigurationException(f"No {key} key in {config_file}") - config.echo("Rendering the following reports: \n{}".format("\n".join([r.name for r in report_files]))) + config.echo( + "Rendering the following reports: \n{}".format( + "\n".join([r.name for r in report_files]) + ) + ) renderer = ReportRenderer(config, report_files) renderer.render() diff --git a/code_annotations/find_django.py b/code_annotations/find_django.py index 7f612b4..aa55b87 100644 --- a/code_annotations/find_django.py +++ b/code_annotations/find_django.py @@ -1,6 +1,7 @@ """ Annotation searcher for Django model comment searching Django introspection. """ + import inspect import os import sys @@ -13,7 +14,7 @@ from code_annotations.base import BaseSearch from code_annotations.helpers import clean_annotation, fail, get_annotation_regex -DEFAULT_SAFELIST_FILE_PATH = '.annotation_safe_list.yml' +DEFAULT_SAFELIST_FILE_PATH = ".annotation_safe_list.yml" class DjangoSearch(BaseSearch): @@ -24,24 +25,35 @@ class DjangoSearch(BaseSearch): def __init__(self, config, app_name=None): """ Initialize for DjangoSearch. - - Args: - config: Configuration file path """ super().__init__(config) - self.local_models, self.non_local_models, total, annotation_eligible = self.get_models_requiring_annotations(app_name) + self.local_models, self.non_local_models, total, annotation_eligible = ( + self.get_models_requiring_annotations(app_name) + ) self.model_counts = { - 'total': total, - 'annotated': 0, - 'unannotated': 0, - 'annotation_eligible': len(annotation_eligible), - 'not_annotation_eligible': total - len(annotation_eligible), - 'safelisted': 0 + "total": total, + "annotated": 0, + "unannotated": 0, + "annotation_eligible": len(annotation_eligible), + "not_annotation_eligible": total - len(annotation_eligible), + "safelisted": 0, } self.uncovered_model_ids = set() - self.echo.echo_vvv('Local models:\n ' + '\n '.join([str(m) for m in self.local_models]) + '\n') - self.echo.echo_vvv('Non-local models:\n ' + '\n '.join([str(m) for m in self.non_local_models]) + '\n') - self.echo.echo_vv('The following models require annotations:\n ' + '\n '.join(annotation_eligible) + '\n') + self.echo.echo_vvv( + "Local models:\n " + + "\n ".join([str(m) for m in self.local_models]) + + "\n" + ) + self.echo.echo_vvv( + "Non-local models:\n " + + "\n ".join([str(m) for m in self.non_local_models]) + + "\n" + ) + self.echo.echo_vv( + "The following models require annotations:\n " + + "\n ".join(annotation_eligible) + + "\n" + ) def _increment_count(self, count_type, incr_by=1): self.model_counts[count_type] += incr_by @@ -51,16 +63,19 @@ def seed_safelist(self): Seed a new safelist file with all non-local models that need to be vetted. """ if os.path.exists(self.config.safelist_path): - fail(f'{self.config.safelist_path} already exists, not overwriting.') + fail(f"{self.config.safelist_path} already exists, not overwriting.") self.echo( - 'Found {} non-local models requiring annotations. Adding them to safelist.'.format( - len(self.non_local_models)) + "Found {} non-local models requiring annotations. Adding them to safelist.".format( + len(self.non_local_models) + ) ) - safelist_data = {self.get_model_id(model): {} for model in self.non_local_models} + safelist_data = { + self.get_model_id(model): {} for model in self.non_local_models + } - with open(self.config.safelist_path, 'w') as safelist_file: + with open(self.config.safelist_path, "w") as safelist_file: safelist_comment = """ # This is a Code Annotations automatically-generated Django model safelist file. # These models must be annotated as follows in order to be counted in the coverage report. @@ -73,12 +88,20 @@ def seed_safelist(self): """ safelist_file.write(safelist_comment.lstrip()) - yaml.safe_dump(safelist_data, stream=safelist_file, default_flow_style=False) + yaml.safe_dump( + safelist_data, stream=safelist_file, default_flow_style=False + ) - self.echo(f'Successfully created safelist file "{self.config.safelist_path}".', fg='red') - self.echo('Now, you need to:', fg='red') - self.echo(' 1) Make sure that any un-annotated models in the safelist are annotated, and', fg='red') - self.echo(' 2) Annotate any LOCAL models (see --list_local_models).', fg='red') + self.echo( + f'Successfully created safelist file "{self.config.safelist_path}".', + fg="red", + ) + self.echo("Now, you need to:", fg="red") + self.echo( + " 1) Make sure that any un-annotated models in the safelist are annotated, and", + fg="red", + ) + self.echo(" 2) Annotate any LOCAL models (see --list_local_models).", fg="red") def list_local_models(self): """ @@ -86,11 +109,16 @@ def list_local_models(self): """ if self.local_models: self.echo( - 'Listing {} local models requiring annotations:'.format(len(self.local_models)) + "Listing {} local models requiring annotations:".format( + len(self.local_models) + ) + ) + self.echo.pprint( + sorted([self.get_model_id(model) for model in self.local_models]), + indent=4, ) - self.echo.pprint(sorted([self.get_model_id(model) for model in self.local_models]), indent=4) else: - self.echo('No local models requiring annotations.') + self.echo("No local models requiring annotations.") def _append_model_annotations(self, model_type, model_id, query, model_annotations): """ @@ -112,32 +140,39 @@ def _append_model_annotations(self, model_type, model_id, query, model_annotatio # annotation token itself. We find based on the entire code content of the model # as that seems to be the only way to be sure we're getting the correct line number. # It is slow and should be replaced if we can find a better way that is accurate. - line = txt.count('\n', 0, txt.find(inspect.getsource(model_type))) + 1 + line = txt.count("\n", 0, txt.find(inspect.getsource(model_type))) + 1 for inner_match in query.finditer(model_type.__doc__): try: - annotation_token = inner_match.group('token') - annotation_data = inner_match.group('data') + annotation_token = inner_match.group("token") + annotation_data = inner_match.group("data") except IndexError as error: # pragma: no cover - raise ValueError('{}: Could not find "data" or "token" groups. Found: {}'.format( - self.get_model_id(model_type), - inner_match.groupdict() - )) from error - annotation_token, annotation_data = clean_annotation(annotation_token, annotation_data) - model_annotations.append({ - 'found_by': "django", - 'filename': filename, - 'line_number': line, - 'annotation_token': annotation_token, - 'annotation_data': annotation_data, - 'extra': { - 'object_id': model_id, - 'full_comment': model_type.__doc__.strip() + raise ValueError( + '{}: Could not find "data" or "token" groups. Found: {}'.format( + self.get_model_id(model_type), inner_match.groupdict() + ) + ) from error + annotation_token, annotation_data = clean_annotation( + annotation_token, annotation_data + ) + model_annotations.append( + { + "found_by": "django", + "filename": filename, + "line_number": line, + "annotation_token": annotation_token, + "annotation_data": annotation_data, + "extra": { + "object_id": model_id, + "full_comment": model_type.__doc__.strip(), + }, } - }) + ) - def _append_safelisted_model_annotations(self, safelisted_models, model_id, model_annotations): + def _append_safelisted_model_annotations( + self, safelisted_models, model_id, model_annotations + ): """ Append the safelisted annotations for the given model id to model_annotations. @@ -148,17 +183,19 @@ def _append_safelisted_model_annotations(self, safelisted_models, model_id, mode """ for annotation in safelisted_models[model_id]: comment = safelisted_models[model_id][annotation] - model_annotations.append({ - 'found_by': "safelist", - 'filename': self.config.safelist_path, - 'line_number': 0, - 'annotation_token': annotation.strip(), - 'annotation_data': comment.strip(), - 'extra': { - 'object_id': model_id, - 'full_comment': str(safelisted_models[model_id]) + model_annotations.append( + { + "found_by": "safelist", + "filename": self.config.safelist_path, + "line_number": 0, + "annotation_token": annotation.strip(), + "annotation_data": comment.strip(), + "extra": { + "object_id": model_id, + "full_comment": str(safelisted_models[model_id]), + }, } - }) + ) def _read_safelist(self): """ @@ -168,19 +205,23 @@ def _read_safelist(self): The Python representation of the safelist """ if os.path.exists(self.config.safelist_path): - self.echo(f'Found safelist at {self.config.safelist_path}. Reading.\n') + self.echo(f"Found safelist at {self.config.safelist_path}. Reading.\n") with open(self.config.safelist_path) as safelist_file: safelisted_models = yaml.safe_load(safelist_file) - self._increment_count('safelisted', len(safelisted_models)) + self._increment_count("safelisted", len(safelisted_models)) if safelisted_models: - self.echo.echo_vv(' Safelisted models:\n ' + '\n '.join(safelisted_models)) + self.echo.echo_vv( + " Safelisted models:\n " + "\n ".join(safelisted_models) + ) else: - self.echo.echo_vv(' No safelisted models found.\n') + self.echo.echo_vv(" No safelisted models found.\n") return safelisted_models else: - raise Exception('Safelist not found! Generate one with the --seed_safelist command.') + raise Exception( + "Safelist not found! Generate one with the --seed_safelist command." + ) def search(self): """ @@ -196,12 +237,12 @@ def search(self): annotated_models = {} - self.echo.echo_vv('Searching models and their parent classes...') + self.echo.echo_vv("Searching models and their parent classes...") # Walk all models and their parents looking for annotations for model in self.local_models.union(self.non_local_models): model_id = self.get_model_id(model) - self.echo.echo_vv(' ' + model_id) + self.echo.echo_vv(" " + model_id) hierarchy = inspect.getmro(model) model_annotations = [] @@ -209,23 +250,35 @@ def search(self): for obj in hierarchy: if obj.__doc__ is not None: if any(anno in obj.__doc__ for anno in annotation_tokens): - self.echo.echo_vvv(' ' + DjangoSearch.get_model_id(obj) + ' has annotations.') - self._append_model_annotations(obj, model_id, query, model_annotations) + self.echo.echo_vvv( + " " + + DjangoSearch.get_model_id(obj) + + " has annotations." + ) + self._append_model_annotations( + obj, model_id, query, model_annotations + ) else: # Don't use get_model_id here, as this could be a base class below Model - self.echo.echo_vvv(' ' + str(obj) + ' has no annotations.') + self.echo.echo_vvv(" " + str(obj) + " has no annotations.") # If there are any annotations in the model, format them if model_annotations: - self.echo.echo_vv(" {} has {} total annotations".format(model_id, len(model_annotations))) - self._increment_count('annotated') + self.echo.echo_vv( + " {} has {} total annotations".format( + model_id, len(model_annotations) + ) + ) + self._increment_count("annotated") if model_id in safelisted_models: - self._add_error(f"{model_id} is annotated, but also in the safelist.") + self._add_error( + f"{model_id} is annotated, but also in the safelist." + ) self.format_file_results(annotated_models, [model_annotations]) # The model is not in the safelist and is not annotated elif model_id not in safelisted_models: - self._increment_count('unannotated') + self._increment_count("unannotated") self.uncovered_model_ids.add(model_id) self.echo.echo_vv(f" {model_id} has no annotations") @@ -234,11 +287,15 @@ def search(self): if not safelisted_models[model_id]: self.uncovered_model_ids.add(model_id) self.echo.echo_vv(f" {model_id} is in the safelist.") - self._add_error(f"{model_id} is in the safelist but has no annotations!") + self._add_error( + f"{model_id} is in the safelist but has no annotations!" + ) else: - self._increment_count('annotated') + self._increment_count("annotated") - self._append_safelisted_model_annotations(safelisted_models, model_id, model_annotations) + self._append_safelisted_model_annotations( + safelisted_models, model_id, model_annotations + ) self.format_file_results(annotated_models, [model_annotations]) return annotated_models @@ -255,10 +312,18 @@ def check_coverage(self): self.echo("\nModel coverage report") self.echo("-" * 40) self.echo("Found {total} total models.".format(**self.model_counts)) - self.echo("{annotation_eligible} were eligible for annotation, {annotated} were annotated.".format(**self.model_counts)) + self.echo( + "{annotation_eligible} were eligible for annotation, {annotated} were annotated.".format( + **self.model_counts + ) + ) - if self.model_counts['annotation_eligible'] > 0: - pct = float(self.model_counts['annotated']) / float(self.model_counts['annotation_eligible']) * 100.0 + if self.model_counts["annotation_eligible"] > 0: + pct = ( + float(self.model_counts["annotated"]) + / float(self.model_counts["annotation_eligible"]) + * 100.0 + ) pct = round(pct, 1) else: pct = 100.0 @@ -269,15 +334,16 @@ def check_coverage(self): displayed_uncovereds = list(self.uncovered_model_ids) displayed_uncovereds.sort() self.echo( - "Coverage found {} uncovered models:\n ".format(len(self.uncovered_model_ids)) + - "\n ".join(displayed_uncovereds) + "Coverage found {} uncovered models:\n ".format( + len(self.uncovered_model_ids) + ) + + "\n ".join(displayed_uncovereds) ) if pct < float(self.config.coverage_target): self.echo( "\nCoverage threshold not met! Needed {}, actually {}!".format( - self.config.coverage_target, - pct + self.config.coverage_target, pct ) ) return False @@ -292,13 +358,15 @@ def requires_annotations(model): # Anything inheriting from django.models.Model will have a ._meta attribute. Our tests # inherit from object, which doesn't have it, and will fail below. This is a quick way # to early out on both. - if not hasattr(model, '_meta'): + if not hasattr(model, "_meta"): return False - return issubclass(model, models.Model) \ - and not (model is models.Model) \ - and not model._meta.abstract \ + return ( + issubclass(model, models.Model) + and not (model is models.Model) + and not model._meta.abstract and not model._meta.proxy + ) @staticmethod def is_non_local(model): @@ -324,7 +392,7 @@ def is_non_local(model): # "site-packages" or "dist-packages". non_local_path_prefixes = [] for path in sys.path: - if 'dist-packages' in path or 'site-packages' in path: + if "dist-packages" in path or "site-packages" in path: non_local_path_prefixes.append(path) model_source_path = inspect.getsourcefile(model) return model_source_path.startswith(tuple(non_local_path_prefixes)) @@ -340,7 +408,7 @@ def get_model_id(model): Returns: str: identifier string for the given model. """ - return f'{model._meta.app_label}.{model._meta.object_name}' + return f"{model._meta.app_label}.{model._meta.object_name}" @staticmethod def setup_django(): @@ -354,8 +422,8 @@ def setup_django(): This function is idempotent. """ - if sys.path[0] != '': # pragma: no cover - sys.path.insert(0, '') + if sys.path[0] != "": # pragma: no cover + sys.path.insert(0, "") django.setup() @staticmethod @@ -386,6 +454,8 @@ def get_models_requiring_annotations(app_name=None): else: local_models.add(root_model) - annotation_eligible_models.append(DjangoSearch.get_model_id(root_model)) + annotation_eligible_models.append( + DjangoSearch.get_model_id(root_model) + ) return local_models, non_local_models, total_models, annotation_eligible_models From 248fa6c64b372321b3f2fe6cff60ce24d7773ab1 Mon Sep 17 00:00:00 2001 From: Brian Mesick Date: Tue, 12 Nov 2024 13:14:32 -0500 Subject: [PATCH 4/5] chore: Fix linting issue, add catalog-info --- catalog-info.yaml | 12 ++++++++++++ code_annotations/find_django.py | 3 +-- 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 catalog-info.yaml diff --git a/catalog-info.yaml b/catalog-info.yaml new file mode 100644 index 0000000..4d3b52c --- /dev/null +++ b/catalog-info.yaml @@ -0,0 +1,12 @@ +# This file records information about this repo. Its use is described in OEP-55: +# https://open-edx-proposals.readthedocs.io/en/latest/processes/oep-0055-proc-project-maintainers.html + +apiVersion: backstage.io/v1alpha1 +kind: Component +metadata: + name: "code-annotations" + description: "Tools for annotating code comments, linting, and Django model coverage." +spec: + owner: "bmtcril" + type: "library" + lifecycle: "production" diff --git a/code_annotations/find_django.py b/code_annotations/find_django.py index aa55b87..0e1dc7f 100644 --- a/code_annotations/find_django.py +++ b/code_annotations/find_django.py @@ -146,8 +146,7 @@ def _append_model_annotations(self, model_type, model_id, query, model_annotatio try: annotation_token = inner_match.group("token") annotation_data = inner_match.group("data") - except IndexError as error: - # pragma: no cover + except IndexError as error: # pragma: no cover raise ValueError( '{}: Could not find "data" or "token" groups. Found: {}'.format( self.get_model_id(model_type), inner_match.groupdict() From 0338f2c17a36068bfb78cd075f963e2345d6c66f Mon Sep 17 00:00:00 2001 From: Brian Mesick Date: Thu, 14 Nov 2024 10:59:39 -0500 Subject: [PATCH 5/5] chore: Bump version to 1.8.2 --- code_annotations/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code_annotations/__init__.py b/code_annotations/__init__.py index 33c4a19..10dbd11 100644 --- a/code_annotations/__init__.py +++ b/code_annotations/__init__.py @@ -2,4 +2,4 @@ Extensible tools for parsing annotations in codebases. """ -__version__ = '1.8.1' +__version__ = '1.8.2'