diff --git a/code_annotations/cli.py b/code_annotations/cli.py index 9c8399d..fbc28b2 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,47 +22,72 @@ 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. """ try: - 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" @@ -74,7 +100,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() @@ -114,32 +142,46 @@ def django_find_annotations( annotation_count += len(annotated_models[filename]) elapsed = datetime.datetime.now() - 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. """ @@ -183,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. """ @@ -204,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..ac24779 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): @@ -29,19 +30,33 @@ def __init__(self, config, app_name=None): 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 +66,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 +91,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 +112,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 +143,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 +186,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 +208,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 +240,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 +253,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 +290,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 +315,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 +337,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 +361,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 +395,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 +411,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 +425,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 +457,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